| Message ID | 20230219070124.3900561-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [PATCHv2,bpf-next] selftests/bpf: run mptcp in a dedicated netns | expand |
Hi Hangbin, On 19/02/2023 08:01, Hangbin Liu wrote: > The current mptcp test is run in init netns. If the user or default > system config disabled mptcp, the test will fail. Let's run the mptcp > test in a dedicated netns to avoid none kernel default mptcp setting. > > Suggested-by: Martin KaFai Lau <martin.lau@linux.dev> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > v2: remove unneed close_cgroup_fd goto label. Thank you for the update! Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net> Cheers, Matt
On 2/18/23 11:01 PM, Hangbin Liu wrote: > The current mptcp test is run in init netns. If the user or default > system config disabled mptcp, the test will fail. Let's run the mptcp > test in a dedicated netns to avoid none kernel default mptcp setting. > > Suggested-by: Martin KaFai Lau <martin.lau@linux.dev> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > v2: remove unneed close_cgroup_fd goto label. > --- > .../testing/selftests/bpf/prog_tests/mptcp.c | 27 +++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c > index 59f08d6d1d53..dbe2bcfd3b38 100644 > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > @@ -7,6 +7,16 @@ > #include "network_helpers.h" > #include "mptcp_sock.skel.h" > > +#define SYS(fmt, ...) \ > + ({ \ > + char cmd[1024]; \ > + snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__); \ > + if (!ASSERT_OK(system(cmd), cmd)) \ > + goto fail; \ > + }) > + > +#define NS_TEST "mptcp_ns" > + > #ifndef TCP_CA_NAME_MAX > #define TCP_CA_NAME_MAX 16 > #endif > @@ -138,12 +148,20 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > > static void test_base(void) > { > + struct nstoken *nstoken = NULL; > int server_fd, cgroup_fd; > > cgroup_fd = test__join_cgroup("/mptcp"); > if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) > return; > > + SYS("ip netns add %s", NS_TEST); > + SYS("ip -net %s link set dev lo up", NS_TEST); > + > + nstoken = open_netns(NS_TEST); > + if (!ASSERT_OK_PTR(nstoken, "open_netns")) > + goto fail; > + > /* without MPTCP */ > server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0); > if (!ASSERT_GE(server_fd, 0, "start_server")) > @@ -157,13 +175,18 @@ static void test_base(void) > /* with MPTCP */ > server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); > if (!ASSERT_GE(server_fd, 0, "start_mptcp_server")) > - goto close_cgroup_fd; > + goto fail; > > ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp"); > > close(server_fd); > > -close_cgroup_fd: > +fail: > + if (nstoken) > + close_netns(nstoken); > + > + system("ip netns del " NS_TEST " >& /dev/null"); It needs to be "&>", like the fix in commit 98e13848cf43 ("selftests/bpf: Fix decap_sanity_ns cleanup"). Since it needs to respin, could you help and take this chance to put the above SYS() macro into the test_progs.h. Other selftests are doing similar thing also. If possible, it may be easier to have a configurable "goto_label" as the first arg.
On Wed, Feb 22, 2023 at 03:44:17PM -0800, Martin KaFai Lau wrote: > > + system("ip netns del " NS_TEST " >& /dev/null"); > > It needs to be "&>", like the fix in commit 98e13848cf43 ("selftests/bpf: > Fix decap_sanity_ns cleanup"). :Shame, Didn't notice this when do copy/paste... > > Since it needs to respin, could you help and take this chance to put the > above SYS() macro into the test_progs.h. Other selftests are doing similar > thing also. If possible, it may be easier to have a configurable > "goto_label" as the first arg. OK, I will fix it. Thanks Hangbin
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index 59f08d6d1d53..dbe2bcfd3b38 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -7,6 +7,16 @@ #include "network_helpers.h" #include "mptcp_sock.skel.h" +#define SYS(fmt, ...) \ + ({ \ + char cmd[1024]; \ + snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__); \ + if (!ASSERT_OK(system(cmd), cmd)) \ + goto fail; \ + }) + +#define NS_TEST "mptcp_ns" + #ifndef TCP_CA_NAME_MAX #define TCP_CA_NAME_MAX 16 #endif @@ -138,12 +148,20 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) static void test_base(void) { + struct nstoken *nstoken = NULL; int server_fd, cgroup_fd; cgroup_fd = test__join_cgroup("/mptcp"); if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) return; + SYS("ip netns add %s", NS_TEST); + SYS("ip -net %s link set dev lo up", NS_TEST); + + nstoken = open_netns(NS_TEST); + if (!ASSERT_OK_PTR(nstoken, "open_netns")) + goto fail; + /* without MPTCP */ server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0); if (!ASSERT_GE(server_fd, 0, "start_server")) @@ -157,13 +175,18 @@ static void test_base(void) /* with MPTCP */ server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); if (!ASSERT_GE(server_fd, 0, "start_mptcp_server")) - goto close_cgroup_fd; + goto fail; ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp"); close(server_fd); -close_cgroup_fd: +fail: + if (nstoken) + close_netns(nstoken); + + system("ip netns del " NS_TEST " >& /dev/null"); + close(cgroup_fd); }
The current mptcp test is run in init netns. If the user or default system config disabled mptcp, the test will fail. Let's run the mptcp test in a dedicated netns to avoid none kernel default mptcp setting. Suggested-by: Martin KaFai Lau <martin.lau@linux.dev> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- v2: remove unneed close_cgroup_fd goto label. --- .../testing/selftests/bpf/prog_tests/mptcp.c | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)