On Thu, Apr 29, 2021 at 8:32 AM Jussi Maki <joamaki@xxxxxxxxx> wrote:
>
> Ports test_tc_redirect.sh to the test_progs framework and removes the
> old test. This makes it more in line with rest of the tests and makes
> it possible to run this test with vmtest.sh and under the bpf CI.
>
> Signed-off-by: Jussi Maki <joamaki@xxxxxxxxx>
> ---
Aren't there any Makefile changes that need to be done as well, given
you are removing "old style" test script?
> tools/testing/selftests/bpf/network_helpers.c | 2 +-
> tools/testing/selftests/bpf/network_helpers.h | 1 +
> .../selftests/bpf/prog_tests/tc_redirect.c | 481 ++++++++++++++++++
> .../selftests/bpf/progs/test_tc_neigh.c | 33 +-
> .../selftests/bpf/progs/test_tc_neigh_fib.c | 9 +-
> .../selftests/bpf/progs/test_tc_peer.c | 33 +-
> .../testing/selftests/bpf/test_tc_redirect.sh | 216 --------
> 7 files changed, 509 insertions(+), 266 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_redirect.c
> delete mode 100755 tools/testing/selftests/bpf/test_tc_redirect.sh
>
[...]
> +
> +#define TIMEOUT_MILLIS 10000
> +
> +static const char * const namespaces[] = {NS_SRC, NS_FWD, NS_DST, NULL};
> +static int root_netns_fd = -1;
> +static __u32 duration;
> +
> +static void restore_root_netns(void)
> +{
> + CHECK_FAIL(setns(root_netns_fd, CLONE_NEWNET));
can you please also switch to ASSERT_xxx() macros while converting
this selftest? Thanks!
> +}
> +
> +int setns_by_name(const char *name)
static?
> +{
> + int nsfd;
> + char nspath[PATH_MAX];
> + int err;
> +
> + snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name);
> + nsfd = open(nspath, O_RDONLY | O_CLOEXEC);
> + if (CHECK(nsfd < 0, nspath, "failed to open\n"))
> + return -EINVAL;
> +
> + err = setns(nsfd, CLONE_NEWNET);
> + close(nsfd);
> +
> + if (CHECK(err, name, "failed to setns\n"))
> + return -1;
> +
> + return 0;
> +}
> +
[...]
> +
> +#define SYS(fmt, ...) \
> + ({ \
> + char cmd[1024]; \
> + snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__); \
> + if (CHECK(system(cmd), cmd, "failed\n")) \
> + goto fail; \
> + })
> +
> +static int netns_setup_links_and_routes(struct netns_setup_result *result)
> +{
> + char veth_src_fwd_addr[IFADDR_STR_LEN+1] = {0,};
> + char veth_dst_fwd_addr[IFADDR_STR_LEN+1] = {0,};
> +
> + SYS("ip link add veth_src type veth peer name veth_src_fwd");
> + SYS("ip link add veth_dst type veth peer name veth_dst_fwd");
> + if (CHECK_FAIL(get_ifaddr("veth_src_fwd", veth_src_fwd_addr)))
please no CHECK_FAIL, they are invisible in test_progs logs, so it's
harder to debug any failures (and use ASSERT_xxx() instead of CHECK).
> + goto fail;
> + if (CHECK_FAIL(get_ifaddr("veth_dst_fwd", veth_dst_fwd_addr)))
> + goto fail;
> +
> + result->ifindex_veth_src_fwd = get_ifindex("veth_src_fwd");
> + if (CHECK_FAIL(result->ifindex_veth_src_fwd < 0))
> + goto fail;
> + result->ifindex_veth_dst_fwd = get_ifindex("veth_dst_fwd");
> + if (CHECK_FAIL(result->ifindex_veth_dst_fwd < 0))
> + goto fail;
> +
[...]
> +
> + /* bpf_fib_lookup() checks if forwarding is enabled */
> + system("ip netns exec " NS_FWD " sysctl -q -w "
> + "net.ipv4.ip_forward=1 "
> + "net.ipv6.conf.veth_src_fwd.forwarding=1 "
> + "net.ipv6.conf.veth_dst_fwd.forwarding=1");
no SYS() and/or error checking?
> +
> + test_connectivity();
> +done:
> + system("ip netns exec " NS_FWD " sysctl -q -w "
> + "net.ipv4.ip_forward=0 "
> + "net.ipv6.conf.veth_src_fwd.forwarding=0 "
> + "net.ipv6.conf.veth_dst_fwd.forwarding=0");
> +
same?
> + bpf_program__unpin(skel->progs.tc_src, SRC_PROG_PIN_FILE);
> + bpf_program__unpin(skel->progs.tc_chk, CHK_PROG_PIN_FILE);
> + bpf_program__unpin(skel->progs.tc_dst, DST_PROG_PIN_FILE);
> + test_tc_neigh_fib__destroy(skel);
> + netns_unload_bpf();
> + restore_root_netns();
> +}
> +
[...]
