1. Patchew
  2. MPTCP
  3. View series

[PATCH mptcp-next v2] selftests/bpf: use random netns name for mptcp

Geliang Tang posted 1 patch 2 weeks, 3 days ago
Failed in applying to current master (apply log)
.../testing/selftests/bpf/prog_tests/mptcp.c  | 32 ++++++++++---------
1 file changed, 17 insertions(+), 15 deletions(-)
[PATCH mptcp-next v2] selftests/bpf: use random netns name for mptcp
Posted by Geliang Tang 2 weeks, 3 days ago 
This patch moves the netns operations code from subtest function
test_base() to the main test function test_mptcp(), then this dedicated
test netns can be used for all the subtests. Other subtests will be
added later.

Use rand() to generate a random netns name instead of using the fixed
name "mptcp_ns" for every test.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
This patch should be merged before the commit "mptcp: refactor push_pending logic".
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 32 ++++++++++---------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index cd0c42fff7c0..d989613c821d 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -7,8 +7,6 @@
 #include "network_helpers.h"
 #include "mptcp_sock.skel.h"
 
-#define NS_TEST "mptcp_ns"
-
 #ifndef TCP_CA_NAME_MAX
 #define TCP_CA_NAME_MAX	16
 #endif
@@ -140,20 +138,12 @@ 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(fail, "ip netns add %s", NS_TEST);
-	SYS(fail, "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"))
@@ -174,16 +164,28 @@ static void test_base(void)
 	close(server_fd);
 
 fail:
-	if (nstoken)
-		close_netns(nstoken);
-
-	SYS_NOFAIL("ip netns del " NS_TEST " &> /dev/null");
-
 	close(cgroup_fd);
 }
 
 void test_mptcp(void)
 {
+	struct nstoken *nstoken = NULL;
+	char NS_TEST[32] = { 0 };
+
+	srand(time(NULL));
+	snprintf(NS_TEST, sizeof(NS_TEST), "mptcp_ns_%d", rand());
+	SYS(fail, "ip netns add %s", NS_TEST);
+	SYS(fail, "ip -net %s link set dev lo up", NS_TEST);
+
+	nstoken = open_netns(NS_TEST);
+	if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+		goto fail;
+
 	if (test__start_subtest("base"))
 		test_base();
+
+	if (nstoken)
+		close_netns(nstoken);
+fail:
+	SYS_NOFAIL("ip netns del %s &> /dev/null", NS_TEST);
 }
-- 
2.35.3
[PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_first test"
Posted by Geliang Tang 2 weeks, 3 days ago 
Run mptcp sched test in a dedicated netns.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 50 ++++++++-----------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 77a6997fc82f..bea9c799a531 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -253,40 +253,34 @@ static void send_data(int lfd, int fd)
 #define ADDR_1	"10.0.1.1"
 #define ADDR_2	"10.0.1.2"
 
-static void sched_init(char *flags, char *sched)
+static void sched_init(char *ns, char *flags, char *sched)
 {
-	char cmd[64];
-
-	system("ip link add veth1 type veth peer name veth2");
-	snprintf(cmd, sizeof(cmd), "ip addr add %s/24 dev veth1", ADDR_1);
-	system(cmd);
-	system("ip link set veth1 up");
-	snprintf(cmd, sizeof(cmd), "ip addr add %s/24 dev veth2", ADDR_2);
-	system(cmd);
-	system("ip link set veth2 up");
-
-	snprintf(cmd, sizeof(cmd), "ip mptcp endpoint add %s %s", ADDR_2, flags);
-	system(cmd);
-	snprintf(cmd, sizeof(cmd), "sysctl -qw net.mptcp.scheduler=%s", sched);
-	system(cmd);
+	SYS_NOFAIL("ip -net %s link add veth1 type veth peer name veth2", ns);
+	SYS_NOFAIL("ip -net %s addr add %s/24 dev veth1", ns, ADDR_1);
+	SYS_NOFAIL("ip -net %s link set dev veth1 up", ns);
+	SYS_NOFAIL("ip -net %s addr add %s/24 dev veth2", ns, ADDR_2);
+	SYS_NOFAIL("ip -net %s link set dev veth2 up", ns);
+	SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", ns, ADDR_2, flags);
+	SYS_NOFAIL("ip netns exec %s sysctl -qw net.mptcp.scheduler=%s", ns, sched);
 }
 
-static void sched_cleanup(void)
+static void sched_cleanup(char *ns)
 {
-	system("sysctl -qw net.mptcp.scheduler=default");
-	system("ip mptcp endpoint flush");
-	system("ip link del veth1");
+	SYS_NOFAIL("ip netns exec %s sysctl -qw net.mptcp.scheduler=default", ns);
+	SYS_NOFAIL("ip -net %s mptcp endpoint flush", ns);
+	SYS_NOFAIL("ip -net %s link del veth1", ns);
 }
 
-static int has_bytes_sent(char *addr)
+static int has_bytes_sent(char *ns, char *addr)
 {
-	char cmd[64];
+	char cmd[128];
 
-	snprintf(cmd, sizeof(cmd), "ss -it dst %s | grep -q 'bytes_sent:'", addr);
+	snprintf(cmd, sizeof(cmd), "ip netns exec %s ss -it dst %s | grep -q bytes_sent:",
+		 ns, addr);
 	return system(cmd);
 }
 
-static void test_first(void)
+static void test_first(char *ns)
 {
 	struct mptcp_bpf_first *first_skel;
 	int server_fd, client_fd;
@@ -302,17 +296,17 @@ static void test_first(void)
 		return;
 	}
 
-	sched_init("subflow", "bpf_first");
+	sched_init(ns, "subflow", "bpf_first");
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, 0, 0);
 	client_fd = connect_to_fd(server_fd, 0);
 
 	send_data(server_fd, client_fd);
-	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
-	ASSERT_GT(has_bytes_sent(ADDR_2), 0, "has_bytes_sent addr_2");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_1), "has_bytes_sent addr_1");
+	ASSERT_GT(has_bytes_sent(ns, ADDR_2), 0, "has_bytes_sent addr_2");
 
 	close(client_fd);
 	close(server_fd);
-	sched_cleanup();
+	sched_cleanup(ns);
 	bpf_link__destroy(link);
 	mptcp_bpf_first__destroy(first_skel);
 }
@@ -334,7 +328,7 @@ void test_mptcp(void)
 	if (test__start_subtest("base"))
 		test_base();
 	if (test__start_subtest("first"))
-		test_first();
+		test_first(NS_TEST);
 
 	if (nstoken)
 		close_netns(nstoken);
-- 
2.35.3
Re: [PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_first test"
Posted by Matthieu Baerts 2 weeks, 3 days ago 
Hi Geliang,

On 17/05/2023 10:40, Geliang Tang wrote:
> Run mptcp sched test in a dedicated netns.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 50 ++++++++-----------
>  1 file changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index 77a6997fc82f..bea9c799a531 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -253,40 +253,34 @@ static void send_data(int lfd, int fd)
>  #define ADDR_1	"10.0.1.1"
>  #define ADDR_2	"10.0.1.2"
>  
> -static void sched_init(char *flags, char *sched)
> +static void sched_init(char *ns, char *flags, char *sched)

I think it is cleaner to use a new netns for each test: by doing that,
we are sure to start from a fresh and controlled environment just in
case something wrong happened with the previous test. No?

So in the sched_init(), the netns is created and in sched_cleanup(), the
netns is destroyed (and no need to do anything else I guess).

I guess you can also simply use a helper to create the netns and set
NS_TEST which would be a "global" variable. WDYT?

>  {
> -	char cmd[64];
> -
> -	system("ip link add veth1 type veth peer name veth2");
> -	snprintf(cmd, sizeof(cmd), "ip addr add %s/24 dev veth1", ADDR_1);
> -	system(cmd);
> -	system("ip link set veth1 up");
> -	snprintf(cmd, sizeof(cmd), "ip addr add %s/24 dev veth2", ADDR_2);
> -	system(cmd);
> -	system("ip link set veth2 up");
> -
> -	snprintf(cmd, sizeof(cmd), "ip mptcp endpoint add %s %s", ADDR_2, flags);
> -	system(cmd);
> -	snprintf(cmd, sizeof(cmd), "sysctl -qw net.mptcp.scheduler=%s", sched);
> -	system(cmd);
> +	SYS_NOFAIL("ip -net %s link add veth1 type veth peer name veth2", ns);
> +	SYS_NOFAIL("ip -net %s addr add %s/24 dev veth1", ns, ADDR_1);
> +	SYS_NOFAIL("ip -net %s link set dev veth1 up", ns);
> +	SYS_NOFAIL("ip -net %s addr add %s/24 dev veth2", ns, ADDR_2);
> +	SYS_NOFAIL("ip -net %s link set dev veth2 up", ns);
> +	SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", ns, ADDR_2, flags);
> +	SYS_NOFAIL("ip netns exec %s sysctl -qw net.mptcp.scheduler=%s", ns, sched);

Why not using SYS() instead? By doing that, the test will be marked as
failed if one of these commands fails.

You just need to add:

  fail:
          return;

at the end I guess.

>  }
>  
> -static void sched_cleanup(void)
> +static void sched_cleanup(char *ns)>  {
> -	system("sysctl -qw net.mptcp.scheduler=default");
> -	system("ip mptcp endpoint flush");
> -	system("ip link del veth1");
> +	SYS_NOFAIL("ip netns exec %s sysctl -qw net.mptcp.scheduler=default", ns);
> +	SYS_NOFAIL("ip -net %s mptcp endpoint flush", ns);
> +	SYS_NOFAIL("ip -net %s link del veth1", ns);

Same here with SYS() to catch unexpected behaviours.

(but see above, maybe enough to just close nstoken and delete the netns)

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
[PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_red test"
Posted by Geliang Tang 2 weeks, 3 days ago 
From: Matthieu Baerts <matthieu.baerts@tessares.net>

Run mptcp sched test in a dedicated netns.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index f0f9aa517ec1..83af690facef 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -376,7 +376,7 @@ static void test_rr(char *ns)
 	mptcp_bpf_rr__destroy(rr_skel);
 }
 
-static void test_red(void)
+static void test_red(char *ns)
 {
 	struct mptcp_bpf_red *red_skel;
 	int server_fd, client_fd;
@@ -392,17 +392,17 @@ static void test_red(void)
 		return;
 	}
 
-	sched_init("subflow", "bpf_red");
+	sched_init(ns, "subflow", "bpf_red");
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, 0, 0);
 	client_fd = connect_to_fd(server_fd, 0);
 
 	send_data(server_fd, client_fd);
-	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr 1");
-	ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr 2");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_1), "has_bytes_sent addr 1");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_2), "has_bytes_sent addr 2");
 
 	close(client_fd);
 	close(server_fd);
-	sched_cleanup();
+	sched_cleanup(ns);
 	bpf_link__destroy(link);
 	mptcp_bpf_red__destroy(red_skel);
 }
@@ -430,7 +430,7 @@ void test_mptcp(void)
 	if (test__start_subtest("rr"))
 		test_rr(NS_TEST);
 	if (test__start_subtest("red"))
-		test_red();
+		test_red(NS_TEST);
 
 	if (nstoken)
 		close_netns(nstoken);
-- 
2.35.3
Re: [PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_red test"
Posted by Geliang Tang 2 weeks, 3 days ago 
Geliang Tang <geliang.tang@suse.com> 于2023年5月17日周三 16:41写道：
>
> From: Matthieu Baerts <matthieu.baerts@tessares.net>

Sorry, my bad. This line shouldn't be here:)

>
> Run mptcp sched test in a dedicated netns.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/mptcp.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index f0f9aa517ec1..83af690facef 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -376,7 +376,7 @@ static void test_rr(char *ns)
>         mptcp_bpf_rr__destroy(rr_skel);
>  }
>
> -static void test_red(void)
> +static void test_red(char *ns)
>  {
>         struct mptcp_bpf_red *red_skel;
>         int server_fd, client_fd;
> @@ -392,17 +392,17 @@ static void test_red(void)
>                 return;
>         }
>
> -       sched_init("subflow", "bpf_red");
> +       sched_init(ns, "subflow", "bpf_red");
>         server_fd = start_mptcp_server(AF_INET, ADDR_1, 0, 0);
>         client_fd = connect_to_fd(server_fd, 0);
>
>         send_data(server_fd, client_fd);
> -       ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr 1");
> -       ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr 2");
> +       ASSERT_OK(has_bytes_sent(ns, ADDR_1), "has_bytes_sent addr 1");
> +       ASSERT_OK(has_bytes_sent(ns, ADDR_2), "has_bytes_sent addr 2");
>
>         close(client_fd);
>         close(server_fd);
> -       sched_cleanup();
> +       sched_cleanup(ns);
>         bpf_link__destroy(link);
>         mptcp_bpf_red__destroy(red_skel);
>  }
> @@ -430,7 +430,7 @@ void test_mptcp(void)
>         if (test__start_subtest("rr"))
>                 test_rr(NS_TEST);
>         if (test__start_subtest("red"))
> -               test_red();
> +               test_red(NS_TEST);
>
>         if (nstoken)
>                 close_netns(nstoken);
> --
> 2.35.3
>
>
[PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_bkup test"
Posted by Geliang Tang 2 weeks, 3 days ago 
Run mptcp sched test in a dedicated netns.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 6f69abcadebb..f318aba9e410 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -312,7 +312,7 @@ static void test_first(char *ns)
 	mptcp_bpf_first__destroy(first_skel);
 }
 
-static void test_bkup(void)
+static void test_bkup(char *ns)
 {
 	struct mptcp_bpf_bkup *bkup_skel;
 	int server_fd, client_fd;
@@ -328,17 +328,17 @@ static void test_bkup(void)
 		return;
 	}
 
-	sched_init("subflow backup", "bpf_bkup");
+	sched_init(ns, "subflow backup", "bpf_bkup");
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, 0, 0);
 	client_fd = connect_to_fd(server_fd, 0);
 
 	send_data(server_fd, client_fd);
-	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
-	ASSERT_GT(has_bytes_sent(ADDR_2), 0, "has_bytes_sent addr_2");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_1), "has_bytes_sent addr_1");
+	ASSERT_GT(has_bytes_sent(ns, ADDR_2), 0, "has_bytes_sent addr_2");
 
 	close(client_fd);
 	close(server_fd);
-	sched_cleanup();
+	sched_cleanup(ns);
 	bpf_link__destroy(link);
 	mptcp_bpf_bkup__destroy(bkup_skel);
 }
@@ -362,7 +362,7 @@ void test_mptcp(void)
 	if (test__start_subtest("first"))
 		test_first(NS_TEST);
 	if (test__start_subtest("bkup"))
-		test_bkup();
+		test_bkup(NS_TEST);
 
 	if (nstoken)
 		close_netns(nstoken);
-- 
2.35.3
[PATCH mptcp-next v2] Squash to "selftests/bpf: Add bpf_rr test"
Posted by Geliang Tang 2 weeks, 3 days ago 
Run mptcp sched test in a dedicated netns.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 6d728edce349..ad1c46b35715 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -344,7 +344,7 @@ static void test_bkup(char *ns)
 	mptcp_bpf_bkup__destroy(bkup_skel);
 }
 
-static void test_rr(void)
+static void test_rr(char *ns)
 {
 	struct mptcp_bpf_rr *rr_skel;
 	int server_fd, client_fd;
@@ -360,17 +360,17 @@ static void test_rr(void)
 		return;
 	}
 
-	sched_init("subflow", "bpf_rr");
+	sched_init(ns, "subflow", "bpf_rr");
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, 0, 0);
 	client_fd = connect_to_fd(server_fd, 0);
 
 	send_data(server_fd, client_fd);
-	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr 1");
-	ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr 2");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_1), "has_bytes_sent addr 1");
+	ASSERT_OK(has_bytes_sent(ns, ADDR_2), "has_bytes_sent addr 2");
 
 	close(client_fd);
 	close(server_fd);
-	sched_cleanup();
+	sched_cleanup(ns);
 	bpf_link__destroy(link);
 	mptcp_bpf_rr__destroy(rr_skel);
 }
@@ -396,7 +396,7 @@ void test_mptcp(void)
 	if (test__start_subtest("bkup"))
 		test_bkup(NS_TEST);
 	if (test__start_subtest("rr"))
-		test_rr();
+		test_rr(NS_TEST);
 
 	if (nstoken)
 		close_netns(nstoken);
-- 
2.35.3

Patchew 2.1-devel-0870f84

© 2016 - 2023 Red Hat, Inc.