[PATCH mptcp-next v9 0/4] setsockopt per subflow: BPF

[PATCH mptcp-next v9 0/4] setsockopt per subflow: BPF

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v9:
 - validate the return value of snprintf in patch 2 as Mat suggested.
 - patch 2 depends on "selftests/bpf: Add RUN_MPTCP_TEST macro", so this
   series could be merged in this order:
   1 selftests/bpf: Add mptcp subflow example
   2 selftests/bpf: Add RUN_MPTCP_TEST macro
   3 selftests/bpf: Add mptcp subflow subtest
   4 selftests/bpf: Handle SIGINT when creating netns

v8:
 - update the git metadata to add Nicolas as the git "Author" and add a
   "Co-authored-by" tag with my name in patch 1 as Mat suggested in v7.
 - use send_byte in patch 2 instead of send_recv_data, to make this
   series independent of the commit "selftests/bpf: Export send_recv_data
   helper", which is in bpf-next, but not in mptcp_net-next at this moment.

v7:
 - address Mat's comments in v6 (thanks): drop MPTCP_BASE_TEST and rebased.
 - depends on:
 "refactor mptcp bpf tests" v7
 "export send_byte and send_recv_data" v2

v6:
 - Deponds on "[bpf-next] export send_byte and send_recv_data" series.

v5:
 - #1-#7 Address #76: add new macros MPTCP_BASE_TEST and RUN_MPTCP_TEST.
 - #8-#14 add MPTCP_SCHED_TEST macro.

v4:
 - #1-#5 Address #76: add a new macro RUN_MPTCP_TEST.
 - #6-#11 Cleanups: add send_data_and_verify and SCHED_TEST.

v3:
 - deal with EAGAIN.
 - add _ss_search helper.

v2:
 - fix checkpatch warnings.
 - cleanups.

Geliang Tang (3):
  selftests/bpf: Add mptcp subflow subtest
  selftests/bpf: Handle SIGINT when creating netns
  Squash to "selftests/bpf: Add bpf scheduler test"

Nicolas Rybowski (1):
  selftests/bpf: Add mptcp subflow example

 .../testing/selftests/bpf/prog_tests/mptcp.c  | 143 +++++++++++++++---
 .../selftests/bpf/progs/mptcp_subflow.c       |  70 +++++++++
 2 files changed, 195 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_subflow.c

-- 
2.40.1

[PATCH mptcp-next v9 1/4] selftests/bpf: Add mptcp subflow example

[Geliang Tang]

From: Nicolas Rybowski <nicolas.rybowski@tessares.net>

Move Nicolas's patch into bpf selftests directory. This example added a
test that was adding a different mark (SO_MARK) on each subflow, and
changing the TCP CC only on the first subflow.

This example shows how it is possible to:

    Identify the parent msk of an MPTCP subflow.
    Put different sockopt for each subflow of a same MPTCP connection.

Here especially, we implemented two different behaviours:

    A socket mark (SOL_SOCKET SO_MARK) is put on each subflow of a same
    MPTCP connection. The order of creation of the current subflow defines
    its mark. The TCP CC algorithm of the very first subflow of an MPTCP
    connection is set to "reno".

The code comes from

    commit 4d120186e4d6 ("bpf:examples: update mptcp_set_mark_kern.c")

in MPTCP repo https://github.com/multipath-tcp/mptcp_net-next (the
"scripts" branch).

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/76
Co-developed-by: Geliang Tang <tanggeliang@kylinos.cn>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
---
 .../selftests/bpf/progs/mptcp_subflow.c       | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_subflow.c

diff --git a/tools/testing/selftests/bpf/progs/mptcp_subflow.c b/tools/testing/selftests/bpf/progs/mptcp_subflow.c
new file mode 100644
index 000000000000..de9dbba37133
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcp_subflow.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Tessares SA. */
+/* Copyright (c) 2024, Kylin Software */
+
+#include <sys/socket.h> // SOL_SOCKET, SO_MARK, ...
+#include <linux/tcp.h>  // TCP_CONGESTION
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+#ifndef SOL_TCP
+#define SOL_TCP 6
+#endif
+
+#ifndef TCP_CA_NAME_MAX
+#define TCP_CA_NAME_MAX 16
+#endif
+
+char cc[TCP_CA_NAME_MAX] = "reno";
+
+/* Associate a subflow counter to each token */
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(__u32));
+	__uint(max_entries, 100);
+} mptcp_sf SEC(".maps");
+
+SEC("sockops")
+int mptcp_subflow(struct bpf_sock_ops *skops)
+{
+	__u32 init = 1, key, mark, *cnt;
+	struct mptcp_sock *msk;
+	struct bpf_sock *sk;
+	int err;
+
+	if (skops->op != BPF_SOCK_OPS_TCP_CONNECT_CB)
+		return 1;
+
+	sk = skops->sk;
+	if (!sk)
+		return 1;
+
+	msk = bpf_skc_to_mptcp_sock(sk);
+	if (!msk)
+		return 1;
+
+	key = msk->token;
+	cnt = bpf_map_lookup_elem(&mptcp_sf, &key);
+	if (cnt) {
+		/* A new subflow is added to an existing MPTCP connection */
+		__sync_fetch_and_add(cnt, 1);
+		mark = *cnt;
+	} else {
+		/* A new MPTCP connection is just initiated and this is its primary subflow */
+		bpf_map_update_elem(&mptcp_sf, &key, &init, BPF_ANY);
+		mark = init;
+	}
+
+	/* Set the mark of the subflow's socket based on appearance order */
+	err = bpf_setsockopt(skops, SOL_SOCKET, SO_MARK, &mark, sizeof(mark));
+	if (err < 0)
+		return 1;
+	if (mark == 1)
+		err = bpf_setsockopt(skops, SOL_TCP, TCP_CONGESTION, cc, TCP_CA_NAME_MAX);
+
+	return 1;
+}
-- 
2.40.1

[PATCH mptcp-next v9 2/4] selftests/bpf: Add mptcp subflow subtest

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a subtest named test_subflow to load and verify the newly
added mptcp subflow example in test_mptcp. Add a helper endpoint_init()
to add a new subflow endpoint. Add another helper ss_search() to verify the
fwmark and congestion values set by mptcp_subflow prog using setsockopts.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/76
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 118 +++++++++++++++++-
 1 file changed, 112 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index d3cc3541dd4f..6d94cda6b97a 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -9,6 +9,7 @@
 #include "network_helpers.h"
 #include "mptcp_sock.skel.h"
 #include "mptcpify.skel.h"
+#include "mptcp_subflow.skel.h"
 #include "mptcp_bpf_first.skel.h"
 #include "mptcp_bpf_bkup.skel.h"
 #include "mptcp_bpf_rr.skel.h"
@@ -16,6 +17,9 @@
 #include "mptcp_bpf_burst.skel.h"
 
 #define NS_TEST "mptcp_ns"
+#define ADDR_1	"10.0.1.1"
+#define ADDR_2	"10.0.1.2"
+#define PORT_1	10001
 #define WITH_DATA	true
 #define WITHOUT_DATA	false
 
@@ -41,6 +45,9 @@
 #endif
 #define MPTCP_SCHED_NAME_MAX	16
 
+static const unsigned int total_bytes = 10 * 1024 * 1024;
+static int duration;
+
 struct __mptcp_info {
 	__u8	mptcpi_subflows;
 	__u8	mptcpi_add_addr_signal;
@@ -332,8 +339,110 @@ static void test_mptcpify(void)
 	close(cgroup_fd);
 }
 
-static const unsigned int total_bytes = 10 * 1024 * 1024;
-static int stop, duration;
+static int endpoint_init(char *flags)
+{
+	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);
+	SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
+
+	return 0;
+fail:
+	return -1;
+}
+
+static int _ss_search(char *src, char *dst, char *port, char *keyword)
+{
+	char cmd[128];
+	int n;
+
+	n = snprintf(cmd, sizeof(cmd),
+		     "ip netns exec %s ss -Menita src %s dst %s %s %d | grep -q '%s'",
+		     NS_TEST, src, dst, port, PORT_1, keyword);
+	if (n < 0 || n >= sizeof(cmd))
+		return -1;
+
+	return system(cmd);
+}
+
+static int ss_search(char *src, char *keyword)
+{
+	return _ss_search(src, ADDR_1, "dport", keyword);
+}
+
+static void run_subflow(char *new)
+{
+	int server_fd, client_fd, err;
+	char cc[TCP_CA_NAME_MAX];
+	socklen_t len = sizeof(cc);
+
+	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
+	if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
+		return;
+
+	client_fd = connect_to_fd(server_fd, 0);
+	if (!ASSERT_GE(client_fd, 0, "connect to fd"))
+		goto fail;
+
+	err = getsockopt(server_fd, SOL_TCP, TCP_CONGESTION, cc, &len);
+	if (!ASSERT_OK(err, "getsockopt(srv_fd, TCP_CONGESTION)"))
+		goto fail;
+
+	send_byte(client_fd);
+
+	ASSERT_OK(ss_search(ADDR_1, "fwmark:0x1"), "ss_search fwmark:0x1");
+	ASSERT_OK(ss_search(ADDR_2, "fwmark:0x2"), "ss_search fwmark:0x2");
+	ASSERT_OK(ss_search(ADDR_1, new), "ss_search new cc");
+	ASSERT_OK(ss_search(ADDR_2, cc), "ss_search default cc");
+
+	close(client_fd);
+fail:
+	close(server_fd);
+}
+
+static void test_subflow(void)
+{
+	int cgroup_fd, prog_fd, err;
+	struct mptcp_subflow *skel;
+	struct nstoken *nstoken;
+
+	cgroup_fd = test__join_cgroup("/mptcp_subflow");
+	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup: mptcp_subflow"))
+		return;
+
+	skel = mptcp_subflow__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_load: mptcp_subflow"))
+		goto close_cgroup;
+
+	err = mptcp_subflow__attach(skel);
+	if (!ASSERT_OK(err, "skel_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"))
+		goto skel_destroy;
+
+	nstoken = create_netns();
+	if (!ASSERT_OK_PTR(nstoken, "create_netns: mptcp_subflow"))
+		goto skel_destroy;
+
+	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
+		goto close_netns;
+
+	run_subflow(skel->data->cc);
+
+close_netns:
+	cleanup_netns(nstoken);
+skel_destroy:
+	mptcp_subflow__destroy(skel);
+close_cgroup:
+	close(cgroup_fd);
+}
+
+static int stop;
 
 static void *server(void *arg)
 {
@@ -414,10 +523,6 @@ static void send_data(int lfd, int fd, char *msg)
 	      PTR_ERR(thread_ret));
 }
 
-#define ADDR_1	"10.0.1.1"
-#define ADDR_2	"10.0.1.2"
-#define PORT_1	10001
-
 static struct nstoken *sched_init(char *flags, char *sched)
 {
 	struct nstoken *nstoken;
@@ -558,6 +663,7 @@ void test_mptcp(void)
 {
 	RUN_MPTCP_TEST(base);
 	RUN_MPTCP_TEST(mptcpify);
+	RUN_MPTCP_TEST(subflow);
 	RUN_MPTCP_TEST(default);
 	RUN_MPTCP_TEST(first);
 	RUN_MPTCP_TEST(bkup);
-- 
2.40.1

[PATCH mptcp-next v9 3/4] selftests/bpf: Handle SIGINT when creating netns

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

It's necessary to delete netns during the MPTCP bpf tests interrupt,
otherwise the next tests run will fail due to unable to create netns.

This patch adds a new SIGINT handle sig_int, and deletes NS_TEST in it.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 6d94cda6b97a..7409f7c8b1f6 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -79,11 +79,18 @@ struct mptcp_storage {
 	char ca_name[TCP_CA_NAME_MAX];
 };
 
+static void sig_int(int sig)
+{
+	signal(sig, SIG_IGN);
+	SYS_NOFAIL("ip netns del %s", NS_TEST);
+}
+
 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);
 
+	signal(SIGINT, sig_int);
 	return open_netns(NS_TEST);
 fail:
 	return NULL;
-- 
2.40.1

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

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Use endpoint_init in sched_init and use _ss_search in has_bytes_sent.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 7409f7c8b1f6..f84c74b87d23 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -536,28 +536,22 @@ static struct nstoken *sched_init(char *flags, char *sched)
 
 	nstoken = create_netns();
 	if (!ASSERT_OK_PTR(nstoken, "create_netns"))
+		return NULL;
+
+	if (!ASSERT_OK(endpoint_init("subflow"), "endpoint_init"))
 		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);
-	SYS(fail, "ip -net %s mptcp endpoint add %s %s", NS_TEST, ADDR_2, flags);
 	SYS(fail, "ip netns exec %s sysctl -qw net.mptcp.scheduler=%s", NS_TEST, sched);
 
 	return nstoken;
 fail:
+	cleanup_netns(nstoken);
 	return NULL;
 }
 
-static int has_bytes_sent(char *addr)
+static int has_bytes_sent(char *dst)
 {
-	char cmd[128];
-
-	snprintf(cmd, sizeof(cmd), "ip netns exec %s ss -it src %s sport %d dst %s | %s",
-		 NS_TEST, ADDR_1, PORT_1, addr, "grep -q bytes_sent:");
-	return system(cmd);
+	return _ss_search(ADDR_1, dst, "sport", "bytes_sent:");
 }
 
 static void send_data_and_verify(char *sched, bool addr1, bool addr2)
-- 
2.40.1

Re: [PATCH mptcp-next v9 0/4] setsockopt per subflow: BPF

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

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


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 v9 0/4] setsockopt per subflow: BPF

[Mat Martineau]

On Wed, 24 Apr 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> v9:
> - validate the return value of snprintf in patch 2 as Mat suggested.
> - patch 2 depends on "selftests/bpf: Add RUN_MPTCP_TEST macro", so this
>   series could be merged in this order:
>   1 selftests/bpf: Add mptcp subflow example
>   2 selftests/bpf: Add RUN_MPTCP_TEST macro
>   3 selftests/bpf: Add mptcp subflow subtest
>   4 selftests/bpf: Handle SIGINT when creating netns

Hi Geliang -

v9 looks good to me, thanks!

Reviewed-by: Mat Martineau <martineau@kernel.org>

>
> v8:
> - update the git metadata to add Nicolas as the git "Author" and add a
>   "Co-authored-by" tag with my name in patch 1 as Mat suggested in v7.
> - use send_byte in patch 2 instead of send_recv_data, to make this
>   series independent of the commit "selftests/bpf: Export send_recv_data
>   helper", which is in bpf-next, but not in mptcp_net-next at this moment.
>
> v7:
> - address Mat's comments in v6 (thanks): drop MPTCP_BASE_TEST and rebased.
> - depends on:
> "refactor mptcp bpf tests" v7
> "export send_byte and send_recv_data" v2
>
> v6:
> - Deponds on "[bpf-next] export send_byte and send_recv_data" series.
>
> v5:
> - #1-#7 Address #76: add new macros MPTCP_BASE_TEST and RUN_MPTCP_TEST.
> - #8-#14 add MPTCP_SCHED_TEST macro.
>
> v4:
> - #1-#5 Address #76: add a new macro RUN_MPTCP_TEST.
> - #6-#11 Cleanups: add send_data_and_verify and SCHED_TEST.
>
> v3:
> - deal with EAGAIN.
> - add _ss_search helper.
>
> v2:
> - fix checkpatch warnings.
> - cleanups.
>
> Geliang Tang (3):
>  selftests/bpf: Add mptcp subflow subtest
>  selftests/bpf: Handle SIGINT when creating netns
>  Squash to "selftests/bpf: Add bpf scheduler test"
>
> Nicolas Rybowski (1):
>  selftests/bpf: Add mptcp subflow example
>
> .../testing/selftests/bpf/prog_tests/mptcp.c  | 143 +++++++++++++++---
> .../selftests/bpf/progs/mptcp_subflow.c       |  70 +++++++++
> 2 files changed, 195 insertions(+), 18 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/mptcp_subflow.c
>
> -- 
> 2.40.1
>
>
>

Re: [PATCH mptcp-next v9 0/4] setsockopt per subflow: BPF

[Matthieu Baerts]

Hi Geliang, Mat,

On 24/04/2024 03:13, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v9:
>  - validate the return value of snprintf in patch 2 as Mat suggested.
>  - patch 2 depends on "selftests/bpf: Add RUN_MPTCP_TEST macro", so this
>    series could be merged in this order:
>    1 selftests/bpf: Add mptcp subflow example
>    2 selftests/bpf: Add RUN_MPTCP_TEST macro
>    3 selftests/bpf: Add mptcp subflow subtest
>    4 selftests/bpf: Handle SIGINT when creating netns

Thank you for the v9, and for the reviews!

New patches for t/upstream:
- fbbcd761fba3: selftests/bpf: Add mptcp subflow example
- Results: a11773e3c938..97e0136c8e33 (export)

New patches for t/upstream:
- 9961ad9244d7: selftests/bpf: Add mptcp subflow subtest
- 2f83b94e9407: selftests/bpf: Handle SIGINT when creating netns
- 84add332488a: conflict in t/selftests-bpf-Add-bpf-scheduler-test
- bdc876b898a8: conflict in t/selftests-bpf-add-bpf_first-scheduler
- Results: 97e0136c8e33..d3dcecbbf741 (export)

- d7b5a88080f9: "squashed" in "selftests/bpf: Add bpf scheduler test"
- 774ca6584944: Squash to "selftests/bpf: Add bpf scheduler test"
  (removed unused variable)
- Results: d3dcecbbf741..4e475b683f9a (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/de55140562593e303216dad6fd1e925cd618164d/checks

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