| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <665825df-b995-45ee-9e0c-2b40cc4897ee@kernel.org>
Date: Wed, 5 Nov 2025 15:40:24 +0100
From: Matthieu Baerts <matttbe@...nel.org>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>, mptcp@...ts.linux.dev
Cc: Mat Martineau <martineau@...nel.org>, Geliang Tang <geliang@...nel.org>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
<eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>, 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,
netdev@...r.kernel.org, bpf@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: 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@...ux.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
--
Sponsored by the NGI0 Core fund.
Powered by blists - more mailing lists