Re: [PATCH bpf-next v1 3/3] bpf/selftests: Add xdp bpf_load_tcp_hdr_options tests

On Fri, Oct 1, 2021 at 3:04 PM Joanne Koong [off-list ref] wrote:

This patch adds tests for bpf_load_tcp_hdr_options used by xdp
programs.

test_xdp_tcp_hdr_options.c:
- Tests ipv4 and ipv6 packets with TCPOPT_EXP and non-TCPOPT_EXP
tcp options set. Verify that options can be parsed and loaded
successfully.
- Tests error paths: TCPOPT_EXP with invalid magic, option with
invalid kind_len, non-existent option, invalid flags, option size
smaller than kind_len, invalid packet

Signed-off-by: Joanne Koong <redacted>
---
 .../bpf/prog_tests/xdp_tcp_hdr_options.c      | 158 ++++++++++++++
 .../bpf/progs/test_xdp_tcp_hdr_options.c      | 198 ++++++++++++++++++
 2 files changed, 356 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_tcp_hdr_options.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_tcp_hdr_options.c

[...]

+static void check_opt_out(struct test_xdp_tcp_hdr_options *skel)
+{
+       struct bpf_test_option *opt_out;
+       __u32 duration = 0;
+
+       opt_out = &skel->bss->exprm_opt_out;
+       CHECK(opt_out->flags != opt_flags, "exprm flags",
+             "flags = 0x%x", opt_out->flags);
+       CHECK(opt_out->max_delack_ms != exprm_max_delack_ms, "exprm max_delack_ms",
+             "max_delack_ms = 0x%x", opt_out->max_delack_ms);
+       CHECK(opt_out->rand != exprm_rand, "exprm rand",
+             "rand = 0x%x", opt_out->rand);
+
+       opt_out = &skel->bss->regular_opt_out;
+       CHECK(opt_out->flags != opt_flags, "regular flags",
+             "flags = 0x%x", opt_out->flags);
+       CHECK(opt_out->max_delack_ms != regular_max_delack_ms, "regular max_delack_ms",
+             "max_delack_ms = 0x%x", opt_out->max_delack_ms);
+       CHECK(opt_out->rand != regular_rand, "regular rand",
+             "rand = 0x%x", opt_out->rand);

Please use ASSERT_xxx() macros for new tests. CHECK()s are confusing
and actually require more typing and work to output actual arguments
that failed. In this case, you'd just write

ASSERT_EQ(opt_out->rand, regular_rand, "regular_rand");

And if they are not equal, ASSERT_EQ() will print actual values of
both opt_out->rand and regular_rand.

+}
+
+void test_xdp_tcp_hdr_options(void)
+{
+       int err, prog_fd, prog_err_path_fd, prog_invalid_pkt_fd;
+       struct xdp_ipv6_packet ipv6_pkt, invalid_pkt;
+       struct test_xdp_tcp_hdr_options *skel;
+       struct xdp_ipv4_packet ipv4_pkt;
+       struct xdp_test_opt test_opt;
+       __u32 duration, retval, size;
+       char buf[128];
+

[...]