From: Geliang Tang tanggeliang@kylinos.cn
This patchset addresses Alexei's comment for commit "Handle SIGINT when creating netns" [1]. Export local helpers create_netns() and cleanup_netns() defined in mptcp.c into network_helpers.c as generic ones. For this another helper unshare_netns() is added to replace the existing local helpers create_netns().
v2: - drop patch 9, it breaks CI - delete netns first before adding it in patch 4 - move cleanup_netns behind create_netns in patch 6
[1] https://patchwork.kernel.org/project/mptcp/patch/20240507-upstream-bpf-next-...
Geliang Tang (8): selftests/bpf: Add unshare_netns helper selftests/bpf: Use unshare_netns helper selftests/bpf: Drop duplicate create_netns selftests/bpf: Export create_netns helper selftests/bpf: Use create_netns helper selftests/bpf: Export cleanup_netns helper selftests/bpf: Use cleanup_netns helper selftests/bpf: Use netns helpers in lwt tests
tools/testing/selftests/bpf/network_helpers.c | 68 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 3 + .../selftests/bpf/prog_tests/assign_reuse.c | 12 +--- .../selftests/bpf/prog_tests/bind_perm.c | 11 +-- .../bpf/prog_tests/bpf_iter_setsockopt.c | 13 +--- .../bpf/prog_tests/btf_skc_cls_ingress.c | 6 +- .../selftests/bpf/prog_tests/crypto_sanity.c | 3 +- .../selftests/bpf/prog_tests/decap_sanity.c | 6 +- .../selftests/bpf/prog_tests/fib_lookup.c | 8 +-- .../selftests/bpf/prog_tests/lwt_helpers.h | 26 ++----- .../selftests/bpf/prog_tests/lwt_redirect.c | 2 - .../selftests/bpf/prog_tests/lwt_reroute.c | 2 - .../testing/selftests/bpf/prog_tests/mptcp.c | 22 +----- .../bpf/prog_tests/ns_current_pid_tgid.c | 9 +-- .../selftests/bpf/prog_tests/setget_sockopt.c | 10 +-- .../selftests/bpf/prog_tests/sk_assign.c | 4 +- .../selftests/bpf/prog_tests/sock_destroy.c | 9 +-- .../selftests/bpf/prog_tests/sock_fields.c | 13 +--- .../bpf/prog_tests/sock_iter_batch.c | 9 +-- .../bpf/prog_tests/tcp_custom_syncookie.c | 5 +- .../bpf/prog_tests/tcp_hdr_options.c | 13 +--- .../bpf/prog_tests/xdp_dev_bound_only.c | 3 +- .../bpf/prog_tests/xdp_do_redirect.c | 4 +- 23 files changed, 106 insertions(+), 155 deletions(-)
From: Geliang Tang tanggeliang@kylinos.cn
Many BPF selftests create new test network namespaces by using CLONE_NEWNET flag to unshare the network namespace, so that the calling process is moved into a new network namespace which is not shared with any previously existing process. So this patch adds a new helper in network_helpers.c named unshare_netns(), which create a new network namespace and set the net device lo up.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 19 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 1 + 2 files changed, 20 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 35250e6cde7f..919bb2a0c6a6 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -670,3 +670,22 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes)
return err; } + +int unshare_netns(void) +{ + int err; + + err = unshare(CLONE_NEWNET); + if (err) { + log_err("unshare netns failed"); + return err; + } + + err = SYS_NOFAIL("ip link set dev lo up"); + if (err) { + log_err("set dev lo up failed"); + return err; + } + + return 0; +} diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 883c7ea9d8d5..b087330f2861 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -82,6 +82,7 @@ struct nstoken; struct nstoken *open_netns(const char *name); void close_netns(struct nstoken *token); int send_recv_data(int lfd, int fd, uint32_t total_bytes); +int unshare_netns(void);
static __u16 csum_fold(__u32 csum) {
On 5/15/24 6:13 PM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
Many BPF selftests create new test network namespaces by using CLONE_NEWNET flag to unshare the network namespace, so that the calling process is moved into a new network namespace which is not shared with any previously existing process. So this patch adds a new helper in network_helpers.c named unshare_netns(), which create a new network namespace and set the net device lo up.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/network_helpers.c | 19 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 1 + 2 files changed, 20 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 35250e6cde7f..919bb2a0c6a6 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -670,3 +670,22 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes) return err; }
+int unshare_netns(void) +{
- int err;
- err = unshare(CLONE_NEWNET);
- if (err) {
log_err("unshare netns failed");return err;- }
- err = SYS_NOFAIL("ip link set dev lo up");
Losing useful error output is not good. Please find a way to keep them available such that it is easier to debug. The same goes for similar cases in this patch set. I think the only exception could be the "ip netns del" during cleanup.
Also, bpf CI reports a crash (should be the test #18 bpf_ip_check_defrag). Not sure how this set indirectly triggered it more easily:
https://github.com/kernel-patches/bpf/actions/runs/9290828674/job/2556900264...
pw-bot: cr
- if (err) {
log_err("set dev lo up failed");return err;- }
- return 0;
+} diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 883c7ea9d8d5..b087330f2861 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -82,6 +82,7 @@ struct nstoken; struct nstoken *open_netns(const char *name); void close_netns(struct nstoken *token); int send_recv_data(int lfd, int fd, uint32_t total_bytes); +int unshare_netns(void); static __u16 csum_fold(__u32 csum) {
From: Geliang Tang tanggeliang@kylinos.cn
The newly added helper unshare_netns() can be used to replace unshare() and "ip link set dev lo up" in sk_assign.c, btf_skc_cls_ingress.c and tcp_custom_syncookie.c to simplify the code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 6 +----- tools/testing/selftests/bpf/prog_tests/sk_assign.c | 4 +--- .../testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index ef4d6a3ae423..f697397c641e 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -27,15 +27,11 @@ static int prepare_netns(void) LIBBPF_OPTS(bpf_tc_opts, tc_attach, .prog_fd = bpf_program__fd(skel->progs.cls_ingress));
- if (CHECK(unshare(CLONE_NEWNET), "create netns", + if (CHECK(unshare_netns(), "create netns", "unshare(CLONE_NEWNET): %s (%d)", strerror(errno), errno)) return -1;
- if (CHECK(system("ip link set dev lo up"), - "ip link set dev lo up", "failed\n")) - return -1; - qdisc_lo.ifindex = if_nametoindex("lo"); if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo clsact")) return -1; diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index 0b9bd1d6f7cc..56df5861059f 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -47,12 +47,10 @@ configure_stack(void) return false;
/* Move to a new networking namespace */ - if (CHECK_FAIL(unshare(CLONE_NEWNET))) + if (CHECK_FAIL(unshare_netns())) return false;
/* Configure necessary links, routes */ - if (CHECK_FAIL(system("ip link set dev lo up"))) - return false; if (CHECK_FAIL(system("ip route add local default dev lo"))) return false; if (CHECK_FAIL(system("ip -6 route add local default dev lo"))) diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c index eaf441dc7e79..07817d9c039c 100644 --- a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c +++ b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c @@ -32,12 +32,9 @@ static struct test_tcp_custom_syncookie_case {
static int setup_netns(void) { - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) + if (!ASSERT_OK(unshare_netns(), "create netns")) return -1;
- if (!ASSERT_OK(system("ip link set dev lo up"), "ip")) - goto err; - if (!ASSERT_OK(write_sysctl("/proc/sys/net/ipv4/tcp_ecn", "1"), "write_sysctl")) goto err;
From: Geliang Tang tanggeliang@kylinos.cn
This patch drops the duplicate local functions create_netns() in bind_perm.c, bpf_iter_setsockopt.c, setget_sockopt.c, sock_fields.c and tcp_hdr_options.c and uses unshare_netns() instead.
A new helper create_netns() will be added in network_helpers.c as a public one.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/bind_perm.c | 11 ++--------- .../selftests/bpf/prog_tests/bpf_iter_setsockopt.c | 13 +------------ .../selftests/bpf/prog_tests/setget_sockopt.c | 10 ++-------- .../testing/selftests/bpf/prog_tests/sock_fields.c | 13 +------------ .../selftests/bpf/prog_tests/tcp_hdr_options.c | 13 +------------ 5 files changed, 7 insertions(+), 53 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bind_perm.c b/tools/testing/selftests/bpf/prog_tests/bind_perm.c index f7cd129cb82b..bc80c2773b67 100644 --- a/tools/testing/selftests/bpf/prog_tests/bind_perm.c +++ b/tools/testing/selftests/bpf/prog_tests/bind_perm.c @@ -7,16 +7,9 @@
#include "test_progs.h" #include "cap_helpers.h" +#include "network_helpers.h" #include "bind_perm.skel.h"
-static int create_netns(void) -{ - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) - return -1; - - return 0; -} - void try_bind(int family, int port, int expected_errno) { struct sockaddr_storage addr = {}; @@ -54,7 +47,7 @@ void test_bind_perm(void) __u64 old_caps = 0; int cgroup_fd;
- if (create_netns()) + if (unshare_netns()) return;
cgroup_fd = test__join_cgroup("/bind_perm"); diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c index b52ff8ce34db..eff5d445896b 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter_setsockopt.c @@ -8,17 +8,6 @@ #include "bpf_cubic.skel.h" #include "bpf_iter_setsockopt.skel.h"
-static int create_netns(void) -{ - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) - return -1; - - if (!ASSERT_OK(system("ip link set dev lo up"), "bring up lo")) - return -1; - - return 0; -} - static unsigned int set_bpf_cubic(int *fds, unsigned int nr_fds) { unsigned int i; @@ -187,7 +176,7 @@ void serial_test_bpf_iter_setsockopt(void) struct bpf_link *cubic_link = NULL; struct bpf_link *dctcp_link = NULL;
- if (create_netns()) + if (unshare_netns()) return;
/* Load iter_skel */ diff --git a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c index 7d4a9b3d3722..a44b6e3cec15 100644 --- a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c +++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c @@ -20,14 +20,8 @@ static const char addr6_str[] = "::1"; static struct setget_sockopt *skel; static int cg_fd;
-static int create_netns(void) +static int create_veth(void) { - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) - return -1; - - if (!ASSERT_OK(system("ip link set dev lo up"), "set lo up")) - return -1; - if (!ASSERT_OK(system("ip link add dev binddevtest1 type veth peer name binddevtest2"), "add veth")) return -1; @@ -160,7 +154,7 @@ void test_setget_sockopt(void) if (cg_fd < 0) return;
- if (create_netns()) + if (unshare_netns() || create_veth()) goto done;
skel = setget_sockopt__open(); diff --git a/tools/testing/selftests/bpf/prog_tests/sock_fields.c b/tools/testing/selftests/bpf/prog_tests/sock_fields.c index 7d23166c77af..3499f54b14d1 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_fields.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_fields.c @@ -45,17 +45,6 @@ static __u64 child_cg_id; static int linum_map_fd; static __u32 duration;
-static bool create_netns(void) -{ - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) - return false; - - if (!ASSERT_OK(system("ip link set dev lo up"), "bring up lo")) - return false; - - return true; -} - static void print_sk(const struct bpf_sock *sk, const char *prefix) { char src_ip4[24], dst_ip4[24]; @@ -350,7 +339,7 @@ void serial_test_sock_fields(void) struct bpf_link *link;
/* Use a dedicated netns to have a fixed listen port */ - if (!create_netns()) + if (unshare_netns()) return;
/* Create a cgroup, get fd, and join it */ diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c index 56685fc03c7e..6b4d8fd0f087 100644 --- a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c +++ b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c @@ -40,17 +40,6 @@ struct sk_fds { int active_lport; };
-static int create_netns(void) -{ - if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) - return -1; - - if (!ASSERT_OK(system("ip link set dev lo up"), "run ip cmd")) - return -1; - - return 0; -} - static void print_hdr_stg(const struct hdr_stg *hdr_stg, const char *prefix) { fprintf(stderr, "%s{active:%u, resend_syn:%u, syncookie:%u, fastopen:%u}\n", @@ -548,7 +537,7 @@ void test_tcp_hdr_options(void) if (!test__start_subtest(tests[i].desc)) continue;
- if (create_netns()) + if (unshare_netns()) break;
tests[i].run();
From: Geliang Tang tanggeliang@kylinos.cn
This patch moves create_netns() from mptcp.c into network_helpers.c, and export it in network_helpers.h as a public helper.
The new helper accepts a string parameter, and uses SYS_NOFAIL() instead of SYS() to execute the command, since SYS() deponds on test__fail() which is defined in test_progs.c. SYS() is not suitable to be used in network_helpers.c. Also using "ip netns del" command to delete netns before runing "ip netns add" to add it, since the next tests run may fail due to unable to create netns when last run tests interrupt.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 28 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 1 + .../testing/selftests/bpf/prog_tests/mptcp.c | 14 ++-------- 3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 919bb2a0c6a6..5555b109899f 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -498,6 +498,34 @@ void close_netns(struct nstoken *token) free(token); }
+struct nstoken *create_netns(const char *name) +{ + struct nstoken *token = NULL; + + SYS_NOFAIL("ip netns del %s", name); + if (SYS_NOFAIL("ip netns add %s", name)) { + log_err("add netns %s failed", name); + goto fail; + } + + if (SYS_NOFAIL("ip -net %s link set dev lo up", name)) { + log_err("set dev lo up failed"); + goto fail; + } + + token = open_netns(name); + if (!token) { + log_err("open netns %s failed", name); + goto fail; + } + + return token; + +fail: + SYS_NOFAIL("ip netns del %s", name); + return NULL; +} + int get_socket_local_port(int sock_fd) { struct sockaddr_storage addr; diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index b087330f2861..5c0b082153fd 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -81,6 +81,7 @@ struct nstoken; */ struct nstoken *open_netns(const char *name); void close_netns(struct nstoken *token); +struct nstoken *create_netns(const char *name); int send_recv_data(int lfd, int fd, uint32_t total_bytes); int unshare_netns(void);
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index 4472aa404da0..abb4372114b6 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -79,16 +79,6 @@ struct mptcp_storage { char ca_name[TCP_CA_NAME_MAX]; };
-static struct nstoken *create_netns(void) -{ - SYS(fail, "ip netns add %s", NS_TEST); - SYS(fail, "ip -net %s link set dev lo up", NS_TEST); - - return open_netns(NS_TEST); -fail: - return NULL; -} - static void cleanup_netns(struct nstoken *nstoken) { if (nstoken) @@ -228,7 +218,7 @@ static void test_base(void) if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) return;
- nstoken = create_netns(); + nstoken = create_netns(NS_TEST); if (!ASSERT_OK_PTR(nstoken, "create_netns")) goto fail;
@@ -344,7 +334,7 @@ static void test_mptcpify(void) if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) return;
- nstoken = create_netns(); + nstoken = create_netns(NS_TEST); if (!ASSERT_OK_PTR(nstoken, "create_netns")) goto fail;
From: Geliang Tang tanggeliang@kylinos.cn
The newly added helper create_netns() can be used in assign_reuse.c, fib_lookup.c, ns_current_pid_tgid.c, sock_destroy.c and sock_iter_batch.c to simplify the code.
It changes the behavior of test fib_lookup.c a little bit, but doesn't affect the results.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/assign_reuse.c | 9 ++------- tools/testing/selftests/bpf/prog_tests/fib_lookup.c | 4 +--- .../selftests/bpf/prog_tests/ns_current_pid_tgid.c | 5 +---- tools/testing/selftests/bpf/prog_tests/sock_destroy.c | 5 +---- tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c | 6 +----- 5 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c index 989ee4d9785b..21e1549d9749 100644 --- a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c +++ b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c @@ -175,12 +175,9 @@ void test_assign_reuse(void) { struct nstoken *tok = NULL;
- SYS(out, "ip netns add %s", NS_TEST); - SYS(cleanup, "ip -net %s link set dev lo up", NS_TEST); - - tok = open_netns(NS_TEST); + tok = create_netns(NS_TEST); if (!ASSERT_OK_PTR(tok, "netns token")) - return; + goto cleanup;
if (test__start_subtest("tcpv4")) run_assign_reuse(AF_INET, SOCK_STREAM, "127.0.0.1", PORT); @@ -194,6 +191,4 @@ void test_assign_reuse(void) cleanup: close_netns(tok); SYS_NOFAIL("ip netns delete %s", NS_TEST); -out: - return; } diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c index bd7658958004..b7eac1fce746 100644 --- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c @@ -315,9 +315,7 @@ void test_fib_lookup(void) return; prog_fd = bpf_program__fd(skel->progs.fib_lookup);
- SYS(fail, "ip netns add %s", NS_TEST); - - nstoken = open_netns(NS_TEST); + nstoken = create_netns(NS_TEST); if (!ASSERT_OK_PTR(nstoken, "open_netns")) goto fail;
diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c index e72d75d6baa7..6dae14d106ac 100644 --- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c +++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c @@ -204,10 +204,7 @@ static void test_in_netns(int (*fn)(void *), void *arg) { struct nstoken *nstoken = NULL;
- SYS(cleanup, "ip netns add ns_current_pid_tgid"); - SYS(cleanup, "ip -net ns_current_pid_tgid link set dev lo up"); - - nstoken = open_netns("ns_current_pid_tgid"); + nstoken = create_netns("ns_current_pid_tgid"); if (!ASSERT_OK_PTR(nstoken, "open_netns")) goto cleanup;
diff --git a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c index 9c11938fe597..c156218c7280 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c @@ -193,10 +193,7 @@ void test_sock_destroy(void) if (!ASSERT_OK_PTR(skel->links.sock_connect, "prog_attach")) goto cleanup;
- SYS(cleanup, "ip netns add %s", TEST_NS); - SYS(cleanup, "ip -net %s link set dev lo up", TEST_NS); - - nstoken = open_netns(TEST_NS); + nstoken = create_netns(TEST_NS); if (!ASSERT_OK_PTR(nstoken, "open_netns")) goto cleanup;
diff --git a/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c b/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c index d56e18b25528..b82ab5ddd9e6 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c @@ -112,11 +112,7 @@ void test_sock_iter_batch(void) { struct nstoken *nstoken = NULL;
- SYS_NOFAIL("ip netns del " TEST_NS); - SYS(done, "ip netns add %s", TEST_NS); - SYS(done, "ip -net %s link set dev lo up", TEST_NS); - - nstoken = open_netns(TEST_NS); + nstoken = create_netns(TEST_NS); if (!ASSERT_OK_PTR(nstoken, "open_netns")) goto done;
From: Geliang Tang tanggeliang@kylinos.cn
This patch adds a new struct member name in struct nstoken, to save the name of the given network namespace. It dups the name string in open_netns() and freed in close_netns().
Then move cleanup_netns() from mptcp.c into network_helpers.c as a public helper. In it the newly added name field is passed to command "ip netns del" to delete this network namespace. This makes it more flexible.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 21 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 1 + .../testing/selftests/bpf/prog_tests/mptcp.c | 8 ------- 3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 5555b109899f..2293eac2dcec 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -444,6 +444,7 @@ char *ping_command(int family)
struct nstoken { int orig_netns_fd; + char *name; };
struct nstoken *open_netns(const char *name) @@ -459,6 +460,13 @@ struct nstoken *open_netns(const char *name) return NULL; }
+ token->name = strdup(name); + if (!token->name) { + log_err("Failed to dup name"); + free(token); + return NULL; + } + token->orig_netns_fd = open("/proc/self/ns/net", O_RDONLY); if (token->orig_netns_fd == -1) { log_err("Failed to open(/proc/self/ns/net)"); @@ -483,6 +491,7 @@ struct nstoken *open_netns(const char *name) fail: if (token->orig_netns_fd != -1) close(token->orig_netns_fd); + free(token->name); free(token); return NULL; } @@ -494,6 +503,8 @@ void close_netns(struct nstoken *token)
if (setns(token->orig_netns_fd, CLONE_NEWNET)) log_err("Failed to setns(orig_netns_fd)"); + if (token->name) + free(token->name); close(token->orig_netns_fd); free(token); } @@ -526,6 +537,16 @@ struct nstoken *create_netns(const char *name) return NULL; }
+void cleanup_netns(struct nstoken *token) +{ + if (!token) + return; + + if (SYS_NOFAIL("ip netns del %s", token->name)) + log_err("del netns %s failed", token->name); + close_netns(token); +} + int get_socket_local_port(int sock_fd) { struct sockaddr_storage addr; diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 5c0b082153fd..42d503a9f55e 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -82,6 +82,7 @@ struct nstoken; struct nstoken *open_netns(const char *name); void close_netns(struct nstoken *token); struct nstoken *create_netns(const char *name); +void cleanup_netns(struct nstoken *token); int send_recv_data(int lfd, int fd, uint32_t total_bytes); int unshare_netns(void);
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index abb4372114b6..c7f61fd9e7a5 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -79,14 +79,6 @@ struct mptcp_storage { char ca_name[TCP_CA_NAME_MAX]; };
-static void cleanup_netns(struct nstoken *nstoken) -{ - if (nstoken) - close_netns(nstoken); - - SYS_NOFAIL("ip netns del %s", NS_TEST); -} - static int start_mptcp_server(int family, const char *addr_str, __u16 port, int timeout_ms) {
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses cleanup_netns() helper in BPF tests wide to replace close_netns() and "ip netns del", included assign_reuse.c, crypto_sanity.c, decap_sanity.c, fib_lookup.c, ns_current_pid_tgid.c, sock_destroy.c, sock_iter_batch.c, xdp_dev_bound_only.c and xdp_do_redirect.c. This can simplify the code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/assign_reuse.c | 3 +-- tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 3 +-- tools/testing/selftests/bpf/prog_tests/decap_sanity.c | 6 ++---- tools/testing/selftests/bpf/prog_tests/fib_lookup.c | 4 +--- .../testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c | 4 +--- tools/testing/selftests/bpf/prog_tests/sock_destroy.c | 4 +--- tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c | 3 +-- tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c | 3 +-- tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c | 4 +--- 9 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c index 21e1549d9749..6fa08c47357c 100644 --- a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c +++ b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c @@ -189,6 +189,5 @@ void test_assign_reuse(void) run_assign_reuse(AF_INET6, SOCK_DGRAM, "::1", PORT);
cleanup: - close_netns(tok); - SYS_NOFAIL("ip netns delete %s", NS_TEST); + cleanup_netns(tok); } diff --git a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c index b1a3a49a822a..ce6ceac8a812 100644 --- a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c @@ -190,8 +190,7 @@ void test_crypto_sanity(void) ASSERT_OK(err, "bpf_tc_detach decrypt");
fail: - close_netns(nstoken); + cleanup_netns(nstoken); deinit_afalg(); - SYS_NOFAIL("ip netns del " NS_TEST " &> /dev/null"); crypto_sanity__destroy(skel); } diff --git a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c index dcb9e5070cc3..9528b039ad33 100644 --- a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c @@ -68,10 +68,8 @@ void test_decap_sanity(void) ASSERT_FALSE(skel->bss->broken_csum_start, "broken_csum_start");
fail: - if (nstoken) { + if (nstoken) bpf_tc_hook_destroy(&qdisc_hook); - close_netns(nstoken); - } - SYS_NOFAIL("ip netns del " NS_TEST); + cleanup_netns(nstoken); decap_sanity__destroy(skel); } diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c index b7eac1fce746..9de05603098d 100644 --- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c @@ -368,8 +368,6 @@ void test_fib_lookup(void) }
fail: - if (nstoken) - close_netns(nstoken); - SYS_NOFAIL("ip netns del " NS_TEST); + cleanup_netns(nstoken); fib_lookup__destroy(skel); } diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c index 6dae14d106ac..b4af39dbc449 100644 --- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c +++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c @@ -211,9 +211,7 @@ static void test_in_netns(int (*fn)(void *), void *arg) test_ns_current_pid_tgid_new_ns(fn, arg);
cleanup: - if (nstoken) - close_netns(nstoken); - SYS_NOFAIL("ip netns del ns_current_pid_tgid"); + cleanup_netns(nstoken); }
/* TODO: use a different tracepoint */ diff --git a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c index c156218c7280..1d3abc7a92c4 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c @@ -209,9 +209,7 @@ void test_sock_destroy(void) RUN_TESTS(sock_destroy_prog_fail);
cleanup: - if (nstoken) - close_netns(nstoken); - SYS_NOFAIL("ip netns del " TEST_NS); + cleanup_netns(nstoken); if (cgroup_fd >= 0) close(cgroup_fd); sock_destroy_prog__destroy(skel); diff --git a/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c b/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c index b82ab5ddd9e6..f37e6450d08b 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_iter_batch.c @@ -124,8 +124,7 @@ void test_sock_iter_batch(void) do_test(SOCK_DGRAM, true); do_test(SOCK_DGRAM, false); } - close_netns(nstoken);
done: - SYS_NOFAIL("ip netns del " TEST_NS); + cleanup_netns(nstoken); } diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c b/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c index 7dd18c6d06c6..315777a4c7ce 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c @@ -53,9 +53,8 @@ void test_xdp_dev_bound_only_offdev(void) out: close(fd1); close(fd2); - close_netns(tok); /* eth42 was added inside netns, removing the netns will * also remove eth42 veth pair. */ - SYS_NOFAIL("ip netns del " LOCAL_NETNS); + cleanup_netns(tok); } diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c index bad0ea167be7..27cb409a237f 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c @@ -241,8 +241,6 @@ void test_xdp_do_redirect(void) out_tc: bpf_tc_hook_destroy(&tc_hook); out: - if (nstoken) - close_netns(nstoken); - SYS_NOFAIL("ip netns del testns"); + cleanup_netns(nstoken); test_xdp_do_redirect__destroy(skel); }
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses netns helpers create_netns() and cleanup_netns() in lwt_helpers.h instead of using the local function netns_create() and netns_delete().
For using these helpers. network_helpers.h needs to be included in lwt_helpers.h. Then '#include "network_helpers.h"' in lwt_redirect.c and lwt_reroute.c can be dropped.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/lwt_helpers.h | 26 +++++-------------- .../selftests/bpf/prog_tests/lwt_redirect.c | 2 -- .../selftests/bpf/prog_tests/lwt_reroute.c | 2 -- 3 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h index fb1eb8c67361..602a268502e2 100644 --- a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h +++ b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h @@ -9,6 +9,7 @@ #include <linux/icmp.h>
#include "test_progs.h" +#include "network_helpers.h"
#define log_err(MSG, ...) \ fprintf(stderr, "(%s:%d: errno: %s) " MSG "\n", \ @@ -16,27 +17,14 @@
#define RUN_TEST(name) \ ({ \ - if (test__start_subtest(#name)) \ - if (ASSERT_OK(netns_create(), "netns_create")) { \ - struct nstoken *token = open_netns(NETNS); \ - if (ASSERT_OK_PTR(token, "setns")) { \ - test_ ## name(); \ - close_netns(token); \ - } \ - netns_delete(); \ - } \ + if (test__start_subtest(#name)) { \ + struct nstoken *token = create_netns(NETNS); \ + if (ASSERT_OK_PTR(token, "setns")) \ + test_ ## name(); \ + cleanup_netns(token); \ + } \ })
-static inline int netns_create(void) -{ - return system("ip netns add " NETNS); -} - -static inline int netns_delete(void) -{ - return system("ip netns del " NETNS ">/dev/null 2>&1"); -} - static int open_tuntap(const char *dev_name, bool need_mac) { int err = 0; diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c b/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c index 835a1d756c16..70b80171f7f4 100644 --- a/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/lwt_redirect.c @@ -57,7 +57,6 @@ #define NETNS "ns_lwt_redirect" #include "lwt_helpers.h" #include "test_progs.h" -#include "network_helpers.h"
#define BPF_OBJECT "test_lwt_redirect.bpf.o" #define INGRESS_SEC(need_mac) ((need_mac) ? "redir_ingress" : "redir_ingress_nomac") @@ -308,7 +307,6 @@ static void test_lwt_redirect_dev_carrier_down(void)
static void *test_lwt_redirect_run(void *arg) { - netns_delete(); RUN_TEST(lwt_redirect_normal); RUN_TEST(lwt_redirect_normal_nomac); RUN_TEST(lwt_redirect_dev_down); diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_reroute.c b/tools/testing/selftests/bpf/prog_tests/lwt_reroute.c index 03825d2b45a8..f51cbde7d8b3 100644 --- a/tools/testing/selftests/bpf/prog_tests/lwt_reroute.c +++ b/tools/testing/selftests/bpf/prog_tests/lwt_reroute.c @@ -50,7 +50,6 @@ */ #define NETNS "ns_lwt_reroute" #include "lwt_helpers.h" -#include "network_helpers.h" #include <linux/net_tstamp.h>
#define BPF_OBJECT "test_lwt_reroute.bpf.o" @@ -242,7 +241,6 @@ static void test_lwt_reroute_qdisc_dropped(void)
static void *test_lwt_reroute_run(void *arg) { - netns_delete(); RUN_TEST(lwt_reroute_normal_xmit); RUN_TEST(lwt_reroute_qdisc_dropped); return NULL;
linux-kselftest-mirror@lists.linaro.org
-
Geliang Tang -
Martin KaFai Lau