Message-ID: <87v7k5vs3h.fsf@cloudflare.com>
Date: Thu, 23 Oct 2025 12:18:42 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>
Cc: mptcp@...ts.linux.dev,  netdev@...r.kernel.org,  bpf@...r.kernel.org,
  John Fastabend <john.fastabend@...il.com>,  Eric Dumazet
 <edumazet@...gle.com>,  Kuniyuki Iwashima <kuniyu@...gle.com>,  Paolo
 Abeni <pabeni@...hat.com>,  Willem de Bruijn <willemb@...gle.com>,  "David
 S. Miller" <davem@...emloft.net>,  Jakub Kicinski <kuba@...nel.org>,
  Simon Horman <horms@...nel.org>,  Matthieu Baerts <matttbe@...nel.org>,
  Mat Martineau <martineau@...nel.org>,  Geliang Tang <geliang@...nel.org>,
  Andrii Nakryiko <andrii@...nel.org>,  Eduard Zingerman
 <eddyz87@...il.com>,  Alexei Starovoitov <ast@...nel.org>,  Daniel
 Borkmann <daniel@...earbox.net>,  Martin KaFai Lau <martin.lau@...ux.dev>,
  Song Liu <song@...nel.org>,  Yonghong Song <yonghong.song@...ux.dev>,  KP
 Singh <kpsingh@...nel.org>,  Stanislav Fomichev <sdf@...ichev.me>,  Hao
 Luo <haoluo@...gle.com>,  Jiri Olsa <jolsa@...nel.org>,  Shuah Khan
 <shuah@...nel.org>,  Florian Westphal <fw@...len.de>,
  linux-kernel@...r.kernel.org,  linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net v2 3/3] selftests/bpf: Add mptcp test with sockmap

On Mon, Oct 20, 2025 at 02:04 PM +08, 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@...ux.dev>
> ---
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 136 ++++++++++++++++++
>  .../selftests/bpf/progs/mptcp_sockmap.c       |  43 ++++++
>  2 files changed, 179 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..54459b385439 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>
>  #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,137 @@ 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];
> +
> +	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;
> +	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_fd1 = xaccept_nonblock(listen_fd, NULL, NULL);
> +	/* test normal redirect behavior: the 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 byte 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);
> +	close(listen_fd);
> +}
> +
> +static void test_sockmap_reject_mptcp(struct mptcp_sockmap *skel)
> +{
> +	int listen_fd = -1, server_fd = -1;
> +	int client_fd1 = -1, client_fd2 = -1;
> +	int err, zero = 0;
> +
> +	listen_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
> +	if (!ASSERT_OK_FD(listen_fd, "start_mptcp_server"))
> +		return;
> +
> +	skel->bss->trace_port = ntohs(get_socket_local_port(listen_fd));
> +	skel->bss->sk_index = 0;
> +	client_fd1 = connect_to_fd(listen_fd, 0);
> +	if (!ASSERT_OK_FD(client_fd1, "connect_to_fd client_fd1"))
> +		goto end;
> +	/* sockmap helper called from sockops prog should reject mptcp sk */
> +	if (ASSERT_EQ(skel->bss->helper_ret, -EOPNOTSUPP, "should reject"))
> +		goto end;

I'm confused. Should we bail out (goto end) if EOPNOTSUPP is *not*
returned? That is "if (!ASSERT_EQ(...))".

> +
> +	/* skip sockops prog */
> +	skel->bss->trace_port = -1;
> +	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, "should reject"))
> +		goto end;

Same here. The check seems backward.

> +end:
> +	if (client_fd1 > 0)
> +		close(client_fd1);
> +	if (client_fd2 > 0)
> +		close(client_fd2);
> +	if (server_fd > 0)
> +		close(server_fd);
> +	close(listen_fd);
> +}
> +
> +static void test_mptcp_sockmap(void)
> +{
> +	struct mptcp_sockmap *skel;
> +	struct netns_obj *netns;
> +	int cgroup_fd, err;
> +
> +	cgroup_fd = test__join_cgroup("/mptcp_sockmap");
> +	if (!ASSERT_OK_FD(cgroup_fd, "join_cgroup: mptcp_sockmap"))
> +		return;
> +
> +	skel = mptcp_sockmap__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open_load: mptcp_sockmap"))
> +		goto close_cgroup;
> +
> +	skel->links.mptcp_sockmap_inject =
> +		bpf_program__attach_cgroup(skel->progs.mptcp_sockmap_inject, cgroup_fd);
> +	if (!ASSERT_OK_PTR(skel->links.mptcp_sockmap_inject, "attach sockmap"))
> +		goto skel_destroy;
> +
> +	err = bpf_prog_attach(bpf_program__fd(skel->progs.mptcp_sockmap_redirect),
> +			      bpf_map__fd(skel->maps.sock_map),
> +			      BPF_SK_SKB_STREAM_VERDICT, 0);
> +	if (!ASSERT_OK(err, "bpf_prog_attach stream verdict"))
> +		goto skel_destroy;
> +
> +	netns = netns_new(NS_TEST, true);
> +	if (!ASSERT_OK_PTR(netns, "netns_new: mptcp_sockmap"))
> +		goto skel_destroy;
> +
> +	if (endpoint_init("subflow") < 0)
> +		goto close_netns;
> +
> +	test_sockmap_with_mptcp_fallback(skel);
> +	test_sockmap_reject_mptcp(skel);
> +
> +close_netns:
> +	netns_free(netns);
> +skel_destroy:
> +	mptcp_sockmap__destroy(skel);
> +close_cgroup:
> +	close(cgroup_fd);
> +}
> +
>  void test_mptcp(void)
>  {
>  	if (test__start_subtest("base"))
> @@ -444,4 +578,6 @@ void test_mptcp(void)
>  		test_mptcpify();
>  	if (test__start_subtest("subflow"))
>  		test_subflow();
> +	if (test__start_subtest("sockmap"))
> +		test_mptcp_sockmap();
>  }
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sockmap.c b/tools/testing/selftests/bpf/progs/mptcp_sockmap.c
> new file mode 100644
> index 000000000000..d4eef0cbadb9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/mptcp_sockmap.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "bpf_tracing_net.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int sk_index;
> +int redirect_idx;
> +int trace_port;
> +int helper_ret;
> +struct {
> +	__uint(type, BPF_MAP_TYPE_SOCKMAP);
> +	__uint(key_size, sizeof(__u32));
> +	__uint(value_size, sizeof(__u32));
> +	__uint(max_entries, 100);
> +} sock_map SEC(".maps");
> +
> +SEC("sockops")
> +int mptcp_sockmap_inject(struct bpf_sock_ops *skops)
> +{
> +	struct bpf_sock *sk;
> +
> +	/* only accept specified connection */
> +	if (skops->local_port != trace_port ||
> +	    skops->op != BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB)
> +		return 1;
> +
> +	sk = skops->sk;
> +	if (!sk)
> +		return 1;
> +
> +	/* update sk handler */
> +	helper_ret = bpf_sock_map_update(skops, &sock_map, &sk_index, BPF_NOEXIST);
> +
> +	return 1;
> +}
> +
> +SEC("sk_skb/stream_verdict")
> +int mptcp_sockmap_redirect(struct __sk_buff *skb)
> +{
> +	/* redirect skb to the sk under sock_map[redirect_idx] */
> +	return bpf_sk_redirect_map(skb, &sock_map, redirect_idx, 0);
> +}

Powered by blists - more mailing lists