Re: [PATCH bpf-next v2 2/3] selftests/bpf: Test for IPv6 ext header parsing

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Dec 7, 2021 at 2:58 PM Mathieu Jadin <mathjadin@gmail.com> wrote:
>
> This test creates a client and a server exchanging a single byte
> with a Segment Routing Header and the eBPF program saves
> the inner segment in a sk_storage. The test program checks that
> the segment is correct.
>
> Signed-off-by: Mathieu Jadin <mathjadin@gmail.com>
> ---
>  .../bpf/prog_tests/tcp_ipv6_exthdr_srh.c      | 171 ++++++++++++++++++
>  .../selftests/bpf/progs/tcp_ipv6_exthdr_srh.c |  78 ++++++++
>  2 files changed, 249 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/tcp_ipv6_exthdr_srh.c
>  create mode 100644 tools/testing/selftests/bpf/progs/tcp_ipv6_exthdr_srh.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_ipv6_exthdr_srh.c b/tools/testing/selftests/bpf/prog_tests/tcp_ipv6_exthdr_srh.c
> new file mode 100644
> index 000000000000..70f7ee230975
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/tcp_ipv6_exthdr_srh.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <linux/seg6.h>
> +#include "cgroup_helpers.h"
> +#include "network_helpers.h"
> +
> +struct tcp_srh_storage {
> +       struct in6_addr inner_segment;
> +};
> +
> +static void send_byte(int fd)
> +{
> +       char b = 0x55;
> +
> +       if (CHECK_FAIL(send(fd, &b, sizeof(b), 0) != 1))
> +               perror("Failed to send single byte");
> +}
> +
> +static int verify_srh(int map_fd, int server_fd, struct ipv6_sr_hdr *client_srh)
> +{
> +       int err = 0;
> +       struct tcp_srh_storage val;
> +
> +       if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &server_fd, &val) < 0)) {

please use ASSERT_XXX() macros instead of CHECK and especially instead
of CHECK_FAIL

> +               perror("Failed to read socket storage");
> +               return -1;
> +       }
> +
> +       if (memcmp(&val.inner_segment, &client_srh->segments[1],
> +                  sizeof(struct in6_addr))) {
> +               log_err("The inner segment of the received SRH differs from the sent one");
> +               err++;
> +       }
> +
> +       return err;
> +}
> +
> +static int run_test(int cgroup_fd, int listen_fd)
> +{
> +       struct bpf_prog_load_attr attr = {
> +               .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> +               .file = "./tcp_ipv6_exthdr_srh.o",
> +               .expected_attach_type = BPF_CGROUP_SOCK_OPS,
> +       };
> +       size_t srh_size = sizeof(struct ipv6_sr_hdr) +
> +               2 * sizeof(struct in6_addr);
> +       struct ipv6_sr_hdr *client_srh;
> +       struct bpf_object *obj;
> +       struct bpf_map *map;
> +       struct timeval tv;
> +       int client_fd;
> +       int server_fd;
> +       int prog_fd;
> +       int map_fd;
> +       char byte;
> +       int err;
> +
> +       err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);

bpf_prog_load_xattr() is deprecated, please use BPF skeleton for the test

> +       if (err) {
> +               log_err("Failed to load BPF object");
> +               return -1;
> +       }
> +
> +       map = bpf_object__next_map(obj, NULL);
> +       map_fd = bpf_map__fd(map);
> +

[...]

> +
> +void test_tcp_ipv6_exthdr_srh(void)
> +{
> +       int server_fd, cgroup_fd;
> +
> +       cgroup_fd = test__join_cgroup("/tcp_ipv6_exthdr_srh");
> +       if (CHECK_FAIL(cgroup_fd < 0))
> +               return;
> +
> +       server_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
> +       if (CHECK_FAIL(server_fd < 0))
> +               goto close_cgroup_fd;
> +
> +       if (CHECK_FAIL(system("sysctl net.ipv6.conf.all.seg6_enabled=1")))
> +               goto close_server;
> +
> +       if (CHECK_FAIL(system("sysctl net.ipv6.conf.lo.seg6_enabled=1")))
> +               goto reset_sysctl;
> +
> +       CHECK_FAIL(run_test(cgroup_fd, server_fd));
> +
> +       if (CHECK_FAIL(system("sysctl net.ipv6.conf.lo.seg6_enabled=0")))
> +               log_err("Cannot reset sysctl net.ipv6.conf.lo.seg6_enabled to 0");
> +
> +reset_sysctl:
> +       if (CHECK_FAIL(system("sysctl net.ipv6.conf.all.seg6_enabled=0")))
> +               log_err("Cannot reset sysctl net.ipv6.conf.all.seg6_enabled to 0");
> +

same here, please no CHECK_FAIL()s


> +close_server:
> +       close(server_fd);
> +close_cgroup_fd:
> +       close(cgroup_fd);
> +}

[...]