| Message ID | af42ab81074d8a56f5fac8bcb4637400777f5ad5.1728530836.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | add netns helpers | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
Hi Geliang, On 10/10/2024 05:31, Geliang Tang wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > New netns selftest helpers make_netns() and remove_netns() are added in (s/are/have been/) > network_helpers.c, let's use them in mptcp selftests too. Thank you for the patch! I have one small comment below. The rest looks good to me: Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Because it is just a single patch, and just a clean-up, do you mind sending it directly to BPF maintainers please? > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > --- > tools/testing/selftests/bpf/prog_tests/mptcp.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c > index acd79be134cd..7f70f8ec76aa 100644 > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > @@ -82,12 +82,19 @@ struct mptcp_storage { > > 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); > + struct nstoken *nstoken = NULL; > + > + if (make_netns(NS_TEST)) I think it is clearer to explicitly add "< 0", so people would interpret this line as "if there is an error when creating the netns". Otherwise, it is not clear if we are going to take the error or the success branch: if (make_netns(NS_TEST) < 0) > + goto fail; Maybe clearer with either: return NULL; or below, also use a goto just for the error path: struct nstoken *nstoken = NULL; if (make_netns(NS_TEST) < 0) goto fail; nstoken = open_netns(NS_TEST); if (!nstoken) goto fail; return nstoken; fail: log_err("open netns %s failed", NS_TEST); remove_netns(NS_TEST); return NULL; Up to you. Cheers, Matt
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index acd79be134cd..7f70f8ec76aa 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -82,12 +82,19 @@ struct mptcp_storage { 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); + struct nstoken *nstoken = NULL; + + if (make_netns(NS_TEST)) + goto fail; + + nstoken = open_netns(NS_TEST); + if (!nstoken) { + log_err("open netns %s failed", NS_TEST); + remove_netns(NS_TEST); + } - return open_netns(NS_TEST); fail: - return NULL; + return nstoken; } static void cleanup_netns(struct nstoken *nstoken) @@ -95,7 +102,7 @@ static void cleanup_netns(struct nstoken *nstoken) if (nstoken) close_netns(nstoken); - SYS_NOFAIL("ip netns del %s", NS_TEST); + remove_netns(NS_TEST); } static int start_mptcp_server(int family, const char *addr_str, __u16 port,