[PATCH mptcp-next v3 0/1] add netns helpers

[PATCH mptcp-next v3 0/1] add netns helpers

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

The commit 1e115a58be0ff ("selftests/bpf: netns_new() and netns_free()
helpers.") now in bpf-next makes v2 of this set useless. v3 only uses
these new helpers in mptcp selftests and drops other patches.

v2:
https://patchwork.kernel.org/project/mptcp/cover/cover.1715821541.git.tanggeliang@kylinos.cn/

Geliang Tang (1):
  selftests/bpf: Use make/remove netns helpers in mptcp

 tools/testing/selftests/bpf/prog_tests/mptcp.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v3 1/1] selftests/bpf: Use make/remove netns helpers in mptcp

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

New netns selftest helpers make_netns() and remove_netns() are added in
network_helpers.c, let's use them in mptcp selftests too.

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))
+		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,
-- 
2.43.0

Re: [PATCH mptcp-next v3 0/1] add netns helpers

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/11267085466

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/ff5b50d17a96
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=897427


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next v3 1/1] selftests/bpf: Use make/remove netns helpers in mptcp

[Matthieu Baerts]

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
-- 
Sponsored by the NGI0 Core fund.