Hello, this series is a small follow-up to the test_tc_tunnel recent integration, to address some small missing details raised during the final review ([1]). This is mostly about adding some missing checks on net namespaces management.
[1] https://lore.kernel.org/bpf/1ac9d14e-4250-480c-b863-410be78ac6c6@linux.dev/
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- Alexis Lothoré (eBPF Foundation) (3): selftests/bpf: skip tc_tunnel subtest if its setup fails selftests/bpf: add checks in tc_tunnel when entering net namespaces selftests/bpf: use start_server_str rather than start_reuseport_server in tc_tunnel
.../selftests/bpf/prog_tests/test_tc_tunnel.c | 162 ++++++++++++++------- 1 file changed, 107 insertions(+), 55 deletions(-) --- base-commit: 1e2d874b04ba46a3b9fe6697097aa437641f4339 change-id: 20251030-tc_tunnel_improv-6b9d1c22c6f6
Best regards,
A subtest setup can fail in a wide variety of ways, so make sure not to run it if an issue occurs during its setup. The return value is already representing whether the setup succeeds or fails, it is just about wiring it.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c index cf2e088bfe8e..1d8d38e67f8b 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c @@ -666,8 +666,8 @@ void test_tc_tunnel(void) ret = build_subtest_name(cfg, cfg->name, TEST_NAME_MAX_LEN); if (ret < 0 || !test__start_subtest(cfg->name)) continue; - subtest_setup(skel, cfg); - run_test(cfg); + if (subtest_setup(skel, cfg) == 0) + run_test(cfg); subtest_cleanup(cfg); } cleanup();
test_tc_tunnel is missing checks on any open_netns. Add those checks anytime we try to enter a net namespace, and skip the related operations if we fail. While at it, reduce the number of open_netns/close_netns for cases involving operations in two distinct namespaces: the test currently does the following:
nstoken = open_netns("foo") do_operation(); close(nstoken); nstoken = open_netns("bar") do_another_operation(); close(nstoken);
As already stated in reviews for the initial test, we don't need to go back to the root net namespace to enter a second namespace, so just do:
ntoken_client = open_netns("foo") do_operation(); nstoken_server = open_netns("bar") do_another_operation(); close(nstoken_server); close(nstoken_client);
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- .../selftests/bpf/prog_tests/test_tc_tunnel.c | 134 ++++++++++++++------- 1 file changed, 88 insertions(+), 46 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c index 1d8d38e67f8b..deea90aaefad 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c @@ -133,8 +133,12 @@ static void set_subtest_addresses(struct subtest_cfg *cfg)
static int run_server(struct subtest_cfg *cfg) { - struct nstoken *nstoken = open_netns(SERVER_NS); int family = cfg->ipproto == 6 ? AF_INET6 : AF_INET; + struct nstoken *nstoken; + + nstoken = open_netns(SERVER_NS); + if (!ASSERT_OK_PTR(nstoken, "open server ns")) + return -1;
cfg->server_fd = start_reuseport_server(family, SOCK_STREAM, cfg->server_addr, TEST_PORT, @@ -319,6 +323,10 @@ static int configure_encapsulation(struct subtest_cfg *cfg) static int configure_kernel_decapsulation(struct subtest_cfg *cfg) { struct nstoken *nstoken = open_netns(SERVER_NS); + int ret = -1; + + if (!ASSERT_OK_PTR(nstoken, "open server ns")) + return ret;
if (cfg->configure_fou_rx_port && !ASSERT_OK(add_fou_rx_port(cfg), "configure FOU RX port")) @@ -337,11 +345,11 @@ static int configure_kernel_decapsulation(struct subtest_cfg *cfg) SYS(fail, "sysctl -qw net.ipv4.conf.all.rp_filter=0"); SYS(fail, "sysctl -qw net.ipv4.conf.testtun0.rp_filter=0"); SYS(fail, "ip link set dev testtun0 up"); - close_netns(nstoken); - return 0; + + ret = 0; fail: close_netns(nstoken); - return -1; + return ret; }
static void remove_kernel_decapsulation(struct subtest_cfg *cfg) @@ -356,6 +364,10 @@ static void remove_kernel_decapsulation(struct subtest_cfg *cfg) static int configure_ebpf_decapsulation(struct subtest_cfg *cfg) { struct nstoken *nstoken = open_netns(SERVER_NS); + int ret = -1; + + if (!ASSERT_OK_PTR(nstoken, "open server ns")) + return ret;
if (!cfg->expect_kern_decap_failure) SYS(fail, "ip link del testtun0"); @@ -363,17 +375,20 @@ static int configure_ebpf_decapsulation(struct subtest_cfg *cfg) if (!ASSERT_OK(tc_prog_attach("veth2", cfg->server_ingress_prog_fd, -1), "attach_program")) goto fail; - close_netns(nstoken); - return 0; + + ret = 0; fail: close_netns(nstoken); - return -1; + return ret; }
static void run_test(struct subtest_cfg *cfg) { struct nstoken *nstoken = open_netns(CLIENT_NS);
+ if (!ASSERT_OK_PTR(nstoken, "open client ns")) + return; + if (!ASSERT_OK(run_server(cfg), "run server")) goto fail;
@@ -407,7 +422,7 @@ static void run_test(struct subtest_cfg *cfg)
static int setup(void) { - struct nstoken *nstoken = NULL; + struct nstoken *nstoken_client, *nstoken_server; int fd, err;
fd = open("/dev/urandom", O_RDONLY); @@ -424,52 +439,75 @@ static int setup(void) !ASSERT_OK(make_netns(SERVER_NS), "create server ns")) goto fail;
- nstoken = open_netns(CLIENT_NS); - SYS(fail, "ip link add %s type veth peer name %s", + nstoken_client = open_netns(CLIENT_NS); + if (!ASSERT_OK_PTR(nstoken_client, "open client ns")) + goto fail_delete_ns; + SYS(fail_close_ns_client, "ip link add %s type veth peer name %s", "veth1 mtu 1500 netns " CLIENT_NS " address " MAC_ADDR_VETH1, "veth2 mtu 1500 netns " SERVER_NS " address " MAC_ADDR_VETH2); - SYS(fail, "ethtool -K veth1 tso off"); - SYS(fail, "ip link set veth1 up"); - close_netns(nstoken); - nstoken = open_netns(SERVER_NS); - SYS(fail, "ip link set veth2 up"); - close_netns(nstoken); - + SYS(fail_close_ns_client, "ethtool -K veth1 tso off"); + SYS(fail_close_ns_client, "ip link set veth1 up"); + nstoken_server = open_netns(SERVER_NS); + if (!ASSERT_OK_PTR(nstoken_server, "open server ns")) + goto fail_close_ns_client; + SYS(fail_close_ns_server, "ip link set veth2 up"); + + close_netns(nstoken_server); + close_netns(nstoken_client); return 0; + +fail_close_ns_server: + close_netns(nstoken_server); +fail_close_ns_client: + close_netns(nstoken_client); +fail_delete_ns: + SYS_NOFAIL("ip netns del " CLIENT_NS); + SYS_NOFAIL("ip netns del " SERVER_NS); fail: - close_netns(nstoken); - return 1; + return -1; }
static int subtest_setup(struct test_tc_tunnel *skel, struct subtest_cfg *cfg) { - struct nstoken *nstoken; + struct nstoken *nstoken_client, *nstoken_server; + int ret = -1;
set_subtest_addresses(cfg); if (!ASSERT_OK(set_subtest_progs(cfg, skel), "find subtest progs")) - return -1; + goto fail; if (cfg->extra_decap_mod_args_cb) cfg->extra_decap_mod_args_cb(cfg, cfg->extra_decap_mod_args);
- nstoken = open_netns(CLIENT_NS); - SYS(fail, "ip -4 addr add " IP4_ADDR_VETH1 "/24 dev veth1"); - SYS(fail, "ip -4 route flush table main"); - SYS(fail, "ip -4 route add " IP4_ADDR_VETH2 " mtu 1450 dev veth1"); - SYS(fail, "ip -6 addr add " IP6_ADDR_VETH1 "/64 dev veth1 nodad"); - SYS(fail, "ip -6 route flush table main"); - SYS(fail, "ip -6 route add " IP6_ADDR_VETH2 " mtu 1430 dev veth1"); - close_netns(nstoken); - - nstoken = open_netns(SERVER_NS); - SYS(fail, "ip -4 addr add " IP4_ADDR_VETH2 "/24 dev veth2"); - SYS(fail, "ip -6 addr add " IP6_ADDR_VETH2 "/64 dev veth2 nodad"); - close_netns(nstoken); - - return 0; + nstoken_client = open_netns(CLIENT_NS); + if (!ASSERT_OK_PTR(nstoken_client, "open client ns")) + goto fail; + SYS(fail_close_client_ns, + "ip -4 addr add " IP4_ADDR_VETH1 "/24 dev veth1"); + SYS(fail_close_client_ns, "ip -4 route flush table main"); + SYS(fail_close_client_ns, + "ip -4 route add " IP4_ADDR_VETH2 " mtu 1450 dev veth1"); + SYS(fail_close_client_ns, + "ip -6 addr add " IP6_ADDR_VETH1 "/64 dev veth1 nodad"); + SYS(fail_close_client_ns, "ip -6 route flush table main"); + SYS(fail_close_client_ns, + "ip -6 route add " IP6_ADDR_VETH2 " mtu 1430 dev veth1"); + nstoken_server = open_netns(SERVER_NS); + if (!ASSERT_OK_PTR(nstoken_server, "open server ns")) + goto fail_close_client_ns; + SYS(fail_close_server_ns, + "ip -4 addr add " IP4_ADDR_VETH2 "/24 dev veth2"); + SYS(fail_close_server_ns, + "ip -6 addr add " IP6_ADDR_VETH2 "/64 dev veth2 nodad"); + + ret = 0; + +fail_close_server_ns: + close_netns(nstoken_server); +fail_close_client_ns: + close_netns(nstoken_client); fail: - close_netns(nstoken); - return -1; + return ret; }
@@ -478,15 +516,19 @@ static void subtest_cleanup(struct subtest_cfg *cfg) struct nstoken *nstoken;
nstoken = open_netns(CLIENT_NS); - SYS_NOFAIL("tc qdisc delete dev veth1 parent ffff:fff1"); - SYS_NOFAIL("ip a flush veth1"); - close_netns(nstoken); + if (ASSERT_OK_PTR(nstoken, "open clien ns")) { + SYS_NOFAIL("tc qdisc delete dev veth1 parent ffff:fff1"); + SYS_NOFAIL("ip a flush veth1"); + close_netns(nstoken); + } nstoken = open_netns(SERVER_NS); - SYS_NOFAIL("tc qdisc delete dev veth2 parent ffff:fff1"); - SYS_NOFAIL("ip a flush veth2"); - if (!cfg->expect_kern_decap_failure) - remove_kernel_decapsulation(cfg); - close_netns(nstoken); + if (ASSERT_OK_PTR(nstoken, "open clien ns")) { + SYS_NOFAIL("tc qdisc delete dev veth2 parent ffff:fff1"); + SYS_NOFAIL("ip a flush veth2"); + if (!cfg->expect_kern_decap_failure) + remove_kernel_decapsulation(cfg); + close_netns(nstoken); + } }
static void cleanup(void)
test_tc_tunnel currently uses start_reuseport_server because it needs to frequently start and stop the server, so we need SO_REUSEPORT to avoid getting errors on server restart due to the socket being in TIME_WAIT state. But the test is only using one server at a time, so it is a bit confusing to use this API.
Replace start_reuseport with start_sever_str, and provided the relevant callback to set SO_REUSEPORT.
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- .../selftests/bpf/prog_tests/test_tc_tunnel.c | 24 +++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c index deea90aaefad..8e3fe6dc6221 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c @@ -69,7 +69,7 @@ struct subtest_cfg { int client_egress_prog_fd; int server_ingress_prog_fd; char extra_decap_mod_args[TUNNEL_ARGS_MAX_LEN]; - int *server_fd; + int server_fd; };
struct connection { @@ -131,20 +131,30 @@ static void set_subtest_addresses(struct subtest_cfg *cfg) } }
+static int reuseport_cb(int fd, void *opts) +{ + int one = 1; + + return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)); +} + static int run_server(struct subtest_cfg *cfg) { int family = cfg->ipproto == 6 ? AF_INET6 : AF_INET; + struct network_helper_opts opts = { + .timeout_ms = TIMEOUT_MS, + .post_socket_cb = reuseport_cb, + }; struct nstoken *nstoken;
nstoken = open_netns(SERVER_NS); if (!ASSERT_OK_PTR(nstoken, "open server ns")) return -1;
- cfg->server_fd = start_reuseport_server(family, SOCK_STREAM, - cfg->server_addr, TEST_PORT, - TIMEOUT_MS, 1); + cfg->server_fd = start_server_str(family, SOCK_STREAM, cfg->server_addr, + TEST_PORT, &opts); close_netns(nstoken); - if (!ASSERT_OK_PTR(cfg->server_fd, "start server")) + if (!ASSERT_OK_FD(cfg->server_fd, "start server")) return -1;
return 0; @@ -152,7 +162,7 @@ static int run_server(struct subtest_cfg *cfg)
static void stop_server(struct subtest_cfg *cfg) { - free_fds(cfg->server_fd, 1); + close(cfg->server_fd); }
static int check_server_rx_data(struct subtest_cfg *cfg, @@ -188,7 +198,7 @@ static struct connection *connect_client_to_server(struct subtest_cfg *cfg) return NULL; }
- server_fd = accept(*cfg->server_fd, NULL, NULL); + server_fd = accept(cfg->server_fd, NULL, NULL); if (server_fd < 0) { close(client_fd); free(conn);
linux-kselftest-mirror@lists.linaro.org
-
Alexis Lothoré (eBPF Foundation)