On Fri, Jan 27, 2023 at 11:18 AM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> Test skb and xdp dynptr functionality in the following ways:
>
> 1) progs/test_cls_redirect_dynptr.c
> * Rewrite "progs/test_cls_redirect.c" test to use dynptrs to parse
> skb data
>
> * This is a great example of how dynptrs can be used to simplify a
> lot of the parsing logic for non-statically known values.
>
> When measuring the user + system time between the original version
> vs. using dynptrs, and averaging the time for 10 runs (using
> "time ./test_progs -t cls_redirect"):
> original version: 0.092 sec
> with dynptrs: 0.078 sec
>
> 2) progs/test_xdp_dynptr.c
> * Rewrite "progs/test_xdp.c" test to use dynptrs to parse xdp data
>
> When measuring the user + system time between the original version
> vs. using dynptrs, and averaging the time for 10 runs (using
> "time ./test_progs -t xdp_attach"):
> original version: 0.118 sec
> with dynptrs: 0.094 sec
>
> 3) progs/test_l4lb_noinline_dynptr.c
> * Rewrite "progs/test_l4lb_noinline.c" test to use dynptrs to parse
> skb data
>
> When measuring the user + system time between the original version
> vs. using dynptrs, and averaging the time for 10 runs (using
> "time ./test_progs -t l4lb_all"):
> original version: 0.062 sec
> with dynptrs: 0.081 sec
>
> For number of processed verifier instructions:
> original version: 6268 insns
> with dynptrs: 2588 insns
>
> 4) progs/test_parse_tcp_hdr_opt_dynptr.c
> * Add sample code for parsing tcp hdr opt lookup using dynptrs.
> This logic is lifted from a real-world use case of packet parsing
> in katran [0], a layer 4 load balancer. The original version
> "progs/test_parse_tcp_hdr_opt.c" (not using dynptrs) is included
> here as well, for comparison.
>
> When measuring the user + system time between the original version
> vs. using dynptrs, and averaging the time for 10 runs (using
> "time ./test_progs -t parse_tcp_hdr_opt"):
> original version: 0.031 sec
> with dynptrs: 0.045 sec
>
> 5) progs/dynptr_success.c
> * Add test case "test_skb_readonly" for testing attempts at writes /
> data slices on a prog type with read-only skb ctx.
>
> 6) progs/dynptr_fail.c
> * Add test cases "skb_invalid_data_slice{1,2}" and
> "xdp_invalid_data_slice" for testing that helpers that modify the
> underlying packet buffer automatically invalidate the associated
> data slice.
> * Add test cases "skb_invalid_ctx" and "xdp_invalid_ctx" for testing
> that prog types that do not support bpf_dynptr_from_skb/xdp don't
> have access to the API.
> * Add test case "skb_invalid_write" for testing that writes to a
> read-only data slice are rejected by the verifier.
>
> [0]
> https://github.com/facebookincubator/katran/blob/main/katran/lib/bpf/pckt_parsing.h
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/cls_redirect.c | 25 +
> .../testing/selftests/bpf/prog_tests/dynptr.c | 63 +-
> .../selftests/bpf/prog_tests/l4lb_all.c | 2 +
> .../bpf/prog_tests/parse_tcp_hdr_opt.c | 93 ++
> .../selftests/bpf/prog_tests/xdp_attach.c | 11 +-
> .../testing/selftests/bpf/progs/dynptr_fail.c | 124 +++
> .../selftests/bpf/progs/dynptr_success.c | 28 +
> .../bpf/progs/test_cls_redirect_dynptr.c | 973 ++++++++++++++++++
> .../bpf/progs/test_l4lb_noinline_dynptr.c | 474 +++++++++
> .../bpf/progs/test_parse_tcp_hdr_opt.c | 119 +++
> .../bpf/progs/test_parse_tcp_hdr_opt_dynptr.c | 112 ++
> .../selftests/bpf/progs/test_xdp_dynptr.c | 237 +++++
> .../selftests/bpf/test_tcp_hdr_options.h | 1 +
> 13 files changed, 2248 insertions(+), 14 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_l4lb_noinline_dynptr.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
>
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
> index 062fbc8c8e5e..28c453bbb84a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
> @@ -4,11 +4,10 @@
> #define IFINDEX_LO 1
> #define XDP_FLAGS_REPLACE (1U << 4)
>
> -void serial_test_xdp_attach(void)
> +static void serial_test_xdp_attach(const char *file)
> {
> __u32 duration = 0, id1, id2, id0 = 0, len;
> struct bpf_object *obj1, *obj2, *obj3;
> - const char *file = "./test_xdp.bpf.o";
> struct bpf_prog_info info = {};
> int err, fd1, fd2, fd3;
> LIBBPF_OPTS(bpf_xdp_attach_opts, opts);
> @@ -85,3 +84,11 @@ void serial_test_xdp_attach(void)
> out_1:
> bpf_object__close(obj1);
> }
> +
> +void test_xdp_attach(void)
this test was marked as serial (it starts with "serial_test_"), so we
probably want to preserve that? Just keep this as
"serial_test_xdp_attach" and rename above serial_test_xdp_attach to
something like "subtest_xdp_attach" (this name doesn't matter to
test_progs runner).
> +{
> + if (test__start_subtest("xdp_attach"))
> + serial_test_xdp_attach("./test_xdp.bpf.o");
> + if (test__start_subtest("xdp_attach_dynptr"))
> + serial_test_xdp_attach("./test_xdp_dynptr.bpf.o");
> +}
[...]
