[PATCH mptcp-next v3 0/3] fixes for "new MPTCP subflow subtest v6"

[PATCH mptcp-next v3 0/3] fixes for "new MPTCP subflow subtest v6"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v3:
 - update endpoint_init
 - squash 2-3 into one
 - drop 5-6, they will be sent in the next series.

v2:
 - two more cleanups, 5-6, use __bpf_kfunc_start_defs/__bpf_kfunc_end_defs
 and drop the declarations of __bpf_kfunc.

1-2 address Martin's comments
3-4 cleanups for endpoint_init

Geliang Tang (3):
  Squash to "selftests/bpf: Add getsockopt to inspect mptcp subflow"
  Squash to "selftests/bpf: Add mptcp subflow subtest"
  Squash to "selftests/bpf: Add bpf scheduler test"

 .../testing/selftests/bpf/prog_tests/mptcp.c  | 58 +++++++++++++------
 tools/testing/selftests/bpf/progs/mptcp_bpf.h |  4 +-
 2 files changed, 41 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v3 1/3] Squash to "selftests/bpf: Add getsockopt to inspect mptcp subflow"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Use can_loop instead of cond_break as Martin suggested.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/progs/mptcp_bpf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf.h b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
index 928a1e5ad8db..c3800f986ae1 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf.h
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf.h
@@ -26,10 +26,10 @@ static inline int list_is_head(const struct list_head *list,
 #define list_entry_is_head(pos, head, member)				\
 	list_is_head(&pos->member, (head))
 
-/* small difference: 'cond_break' has been added in the conditions */
+/* small difference: 'can_loop' has been added in the conditions */
 #define list_for_each_entry(pos, head, member)				\
 	for (pos = list_first_entry(head, typeof(*pos), member);	\
-	     cond_break, !list_entry_is_head(pos, head, member);	\
+	     !list_entry_is_head(pos, head, member) && can_loop;	\
 	     pos = list_next_entry(pos, member))
 
 /* mptcp helpers from protocol.h */
-- 
2.43.0

[PATCH mptcp-next v3 2/3] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Address Martin's comments:

Drop mptcp_subflow__attach.
Use bpf_program__attach_cgroup instead of bpf_prog_attach.
Use the skel->links.{mptcp_subflow, _getsockopt_subflow}, instead of declaring a
local "link".

More subflows for endpoint_init:

Add two more test addresses ADDR_3 and ADDR_4, and adds a new parameter
"subflows" for endpoint_init() to control how many subflows are used for the
tests. This makes it more flexible.

Update the parameters of endpoint_init() in test_subflow().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 56 +++++++++++++------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index a3e68bc6afa3..167fd9b190ee 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -20,6 +20,8 @@
 #define NS_TEST "mptcp_ns"
 #define ADDR_1	"10.0.1.1"
 #define ADDR_2	"10.0.1.2"
+#define ADDR_3	"10.0.1.3"
+#define ADDR_4	"10.0.1.4"
 #define PORT_1	10001
 #define WITH_DATA	true
 #define WITHOUT_DATA	false
@@ -351,22 +353,46 @@ static void test_mptcpify(void)
 	close(cgroup_fd);
 }
 
-static int endpoint_init(char *flags)
+static int endpoint_add(char *addr, char *flags)
 {
+	return SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s",
+			  NS_TEST, addr, flags);
+}
+
+static int endpoint_init(char *flags, u8 subflows)
+{
+	int ret = -1;
+
+	if (!subflows || subflows > 4)
+		goto fail;
+
 	SYS(fail, "ip -net %s link add veth1 type veth peer name veth2", NS_TEST);
 	SYS(fail, "ip -net %s addr add %s/24 dev veth1", NS_TEST, ADDR_1);
 	SYS(fail, "ip -net %s link set dev veth1 up", NS_TEST);
 	SYS(fail, "ip -net %s addr add %s/24 dev veth2", NS_TEST, ADDR_2);
 	SYS(fail, "ip -net %s link set dev veth2 up", NS_TEST);
-	if (SYS_NOFAIL("ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags)) {
+
+	SYS(fail, "ip -net %s link add veth3 type veth peer name veth4", NS_TEST);
+	SYS(fail, "ip -net %s addr add %s/24 dev veth3", NS_TEST, ADDR_3);
+	SYS(fail, "ip -net %s link set dev veth3 up", NS_TEST);
+	SYS(fail, "ip -net %s addr add %s/24 dev veth4", NS_TEST, ADDR_4);
+	SYS(fail, "ip -net %s link set dev veth4 up", NS_TEST);
+
+	if (SYS_NOFAIL("ip -net %s mptcp limits set subflows 4", NS_TEST)) {
 		printf("'ip mptcp' not supported, skip this test.\n");
 		test__skip();
 		goto fail;
 	}
 
-	return 0;
+	if (subflows > 1)
+		ret = endpoint_add(ADDR_2, flags);
+	if (subflows > 2)
+		ret = ret ?: endpoint_add(ADDR_3, flags);
+	if (subflows > 3)
+		ret = ret ?: endpoint_add(ADDR_4, flags);
+
 fail:
-	return -1;
+	return ret;
 }
 
 static void wait_for_new_subflows(int fd)
@@ -424,10 +450,9 @@ static void run_subflow(void)
 
 static void test_subflow(void)
 {
-	int cgroup_fd, prog_fd, err;
 	struct mptcp_subflow *skel;
 	struct nstoken *nstoken;
-	struct bpf_link *link;
+	int cgroup_fd;
 
 	cgroup_fd = test__join_cgroup("/mptcp_subflow");
 	if (!ASSERT_OK_FD(cgroup_fd, "join_cgroup: mptcp_subflow"))
@@ -439,30 +464,25 @@ static void test_subflow(void)
 
 	skel->bss->pid = getpid();
 
-	err = mptcp_subflow__attach(skel);
-	if (!ASSERT_OK(err, "skel_attach: mptcp_subflow"))
+	skel->links.mptcp_subflow =
+		bpf_program__attach_cgroup(skel->progs.mptcp_subflow, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links.mptcp_subflow, "attach mptcp_subflow"))
 		goto skel_destroy;
 
-	prog_fd = bpf_program__fd(skel->progs.mptcp_subflow);
-	err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
-	if (!ASSERT_OK(err, "prog_attach"))
+	skel->links._getsockopt_subflow =
+		bpf_program__attach_cgroup(skel->progs._getsockopt_subflow, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links._getsockopt_subflow, "attach _getsockopt_subflow"))
 		goto skel_destroy;
 
 	nstoken = create_netns();
 	if (!ASSERT_OK_PTR(nstoken, "create_netns: mptcp_subflow"))
 		goto skel_destroy;
 
-	if (endpoint_init("subflow") < 0)
-		goto close_netns;
-
-	link = bpf_program__attach_cgroup(skel->progs._getsockopt_subflow,
-					  cgroup_fd);
-	if (!ASSERT_OK_PTR(link, "getsockopt prog"))
+	if (endpoint_init("subflow", 2) < 0)
 		goto close_netns;
 
 	run_subflow();
 
-	bpf_link__destroy(link);
 close_netns:
 	cleanup_netns(nstoken);
 skel_destroy:
-- 
2.43.0

[PATCH mptcp-next v3 3/3] Squash to "selftests/bpf: Add bpf scheduler test"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Update endpoint_init() in sched_init().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 167fd9b190ee..ce18da1a6330 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -499,7 +499,7 @@ static struct nstoken *sched_init(char *flags, char *sched)
 	if (!ASSERT_OK_PTR(nstoken, "create_netns"))
 		return NULL;
 
-	if (endpoint_init("subflow") < 0)
+	if (endpoint_init("subflow", 2) < 0)
 		goto fail;
 
 	SYS(fail, "ip netns exec %s sysctl -qw net.mptcp.scheduler=%s", NS_TEST, sched);
-- 
2.43.0

Re: [PATCH mptcp-next v3 0/3] fixes for "new MPTCP subflow subtest v6"

[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/10977099962

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


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 2/3] Squash to "selftests/bpf: Add mptcp subflow subtest"

[Matthieu Baerts]

Hi Geliang,

On 22/09/2024 03:00, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Address Martin's comments:

Thank you for that!

> Drop mptcp_subflow__attach.
> Use bpf_program__attach_cgroup instead of bpf_prog_attach.
> Use the skel->links.{mptcp_subflow, _getsockopt_subflow}, instead of declaring a
> local "link".
> 
> More subflows for endpoint_init:
> 
> Add two more test addresses ADDR_3 and ADDR_4, and adds a new parameter
> "subflows" for endpoint_init() to control how many subflows are used for the
> tests. This makes it more flexible.

Should we not split that and keep it for later? I mean: I think it would
be better to only address Martin's comments, and modify endpoint_init()
later because this is not needed for the moment if I'm not mistaken.
When we will need more than 2 subflows, we can add these patches, no?

It's just to minimise the differences between the versions already
reviewed by Martin, and the future one. WDYT?

I can already apply patches 1 and 2/6 from your v2. Then check later
what is preferred before sending a new version upstream.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.