Re: [PATCH net v4 3/3] selftests/bpf: Add mptcp test with sockmap

Hi Jiayuan,

Thank you for this new test!

I'm not very familiar with the BPF selftests: it would be nice if someone else can have a quick look.

On 05/11/2025 12:36, Jiayuan Chen wrote:

Add test cases to verify that when MPTCP falls back to plain TCP sockets, they can properly work with sockmap.

Additionally, add test cases to ensure that sockmap correctly rejects MPTCP sockets as expected.

Signed-off-by: Jiayuan Chen jiayuan.chen@linux.dev

.../testing/selftests/bpf/prog_tests/mptcp.c | 150 ++++++++++++++++++ .../selftests/bpf/progs/mptcp_sockmap.c | 43 +++++ 2 files changed, 193 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sockmap.c

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index f8eb7f9d4fd2..56c556f603cc 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -6,11 +6,14 @@ #include <netinet/in.h> #include <test_progs.h> #include <unistd.h> +#include <error.h>

Do you use this new include?

#include "cgroup_helpers.h" #include "network_helpers.h" +#include "socket_helpers.h" #include "mptcp_sock.skel.h" #include "mptcpify.skel.h" #include "mptcp_subflow.skel.h" +#include "mptcp_sockmap.skel.h" #define NS_TEST "mptcp_ns" #define ADDR_1 "10.0.1.1" @@ -436,6 +439,151 @@ static void test_subflow(void) close(cgroup_fd); } +/* Test sockmap on MPTCP server handling non-mp-capable clients. */ +static void test_sockmap_with_mptcp_fallback(struct mptcp_sockmap *skel) +{

• int listen_fd = -1, client_fd1 = -1, client_fd2 = -1;
• int server_fd1 = -1, server_fd2 = -1, sent, recvd;
• char snd[9] = "123456789";
• char rcv[10];
• /* start server with MPTCP enabled */
• listen_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
• if (!ASSERT_OK_FD(listen_fd, "redirect:start_mptcp_server"))

• return;
  

• skel->bss->trace_port = ntohs(get_socket_local_port(listen_fd));
• skel->bss->sk_index = 0;
• /* create client without MPTCP enabled */
• client_fd1 = connect_to_fd_opts(listen_fd, NULL);
• if (!ASSERT_OK_FD(client_fd1, "redirect:connect_to_fd"))

• goto end;
  

• server_fd1 = xaccept_nonblock(listen_fd, NULL, NULL);
• skel->bss->sk_index = 1;
• client_fd2 = connect_to_fd_opts(listen_fd, NULL);
• if (!ASSERT_OK_FD(client_fd2, "redirect:connect_to_fd"))

• goto end;
  

• server_fd2 = xaccept_nonblock(listen_fd, NULL, NULL);
• /* test normal redirect behavior: data sent by client_fd1 can be

• * received by client_fd2
  

• */
  

• skel->bss->redirect_idx = 1;
• sent = xsend(client_fd1, snd, sizeof(snd), 0);
• if (!ASSERT_EQ(sent, sizeof(snd), "redirect:xsend(client_fd1)"))

• goto end;
  

• /* try to recv more bytes to avoid truncation check */
• recvd = recv_timeout(client_fd2, rcv, sizeof(rcv), MSG_DONTWAIT, 2);
• if (!ASSERT_EQ(recvd, sizeof(snd), "redirect:recv(client_fd2)"))

• goto end;
  

+end:

• if (client_fd1 > 1)

• close(client_fd1);
  

• if (client_fd2 > 1)

• close(client_fd2);
  

• if (server_fd1 > 0)

• close(server_fd1);
  

• if (server_fd2 > 0)

• close(server_fd2);
  

Why do you check if it is above 0 or 1? Should you not always check if it is >= 0 for each fd?

• close(listen_fd);

+}

+/* Test sockmap rejection of MPTCP sockets - both server and client sides. */ +static void test_sockmap_reject_mptcp(struct mptcp_sockmap *skel) +{

• int client_fd1 = -1, client_fd2 = -1;
• int listen_fd = -1, server_fd = -1;
• int err, zero = 0;
• /* start server with MPTCP enabled */
• listen_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
• if (!ASSERT_OK_FD(listen_fd, "start_mptcp_server"))

In test_sockmap_with_mptcp_fallback(), you prefixed each error with 'redirect:'. Should you also have a different prefix here? 'sockmap-fb:' vs 'sockmap-mptcp:' eventually?

• return;
  

• skel->bss->trace_port = ntohs(get_socket_local_port(listen_fd));
• skel->bss->sk_index = 0;
• /* create client with MPTCP enabled */
• client_fd1 = connect_to_fd(listen_fd, 0);
• if (!ASSERT_OK_FD(client_fd1, "connect_to_fd client_fd1"))

• goto end;
  

• /* bpf_sock_map_update() called from sockops should reject MPTCP sk */
• if (!ASSERT_EQ(skel->bss->helper_ret, -EOPNOTSUPP, "should reject"))

• goto end;
  

So here, the client is connected, but sockmap doesn't operate on it, right? So most likely, the connection is stalled until the userspace realises that and takes an action?

• /* set trace_port = -1 to stop sockops */
• skel->bss->trace_port = -1;

What do you want to demonstrate from here? That without the sockmap injection, there are no new entries added? Is it worth checking that here?

• client_fd2 = connect_to_fd(listen_fd, 0);
• if (!ASSERT_OK_FD(client_fd2, "connect_to_fd client_fd2"))

• goto end;
  

• server_fd = xaccept_nonblock(listen_fd, NULL, NULL);
• err = bpf_map_update_elem(bpf_map__fd(skel->maps.sock_map),

• 		  &zero, &server_fd, BPF_NOEXIST);
  

• if (!ASSERT_EQ(err, -EOPNOTSUPP, "server should be disallowed"))

• goto end;
  

• /* MPTCP client should also be disallowed */
• err = bpf_map_update_elem(bpf_map__fd(skel->maps.sock_map),

• 		  &zero, &client_fd1, BPF_NOEXIST);
  

• if (!ASSERT_EQ(err, -EOPNOTSUPP, "client should be disallowed"))

• goto end;
  

+end:

• if (client_fd1 > 0)

• close(client_fd1);
  

• if (client_fd2 > 0)

• close(client_fd2);
  

• if (server_fd > 0)

• close(server_fd);
  

Same here: should it not be "*fd >= 0"?

• close(listen_fd);

+}

(...)

Cheers, Matt