[PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Good news! I finally solved the unstable issue of MPTCP BPF sched selftests
I reported a year ago, #487 "send() fails with EAGAIN in blocking IO mode".

The fix is simple, it can be solved by explicitly setting SO_SNDBUF
sockopt, but be sure not to set SO_RCVBUF at the same time
(see sk->sk_userlocks & SOCK_RCVBUF_LOCK in mptcp_rcv_space_adjust()).

With this fix, BPF sched selftests are now very stable, I run loop testing
using mptcp-upstream-virtme-docker (run_loop run_bpftest_all), and can run
it normally for hundreds of times without error:


	=== Attempt: 465 (Thu, 29 May 2025 04:04:09 +0000) ===

BPF Test: test_progs -t mptcp
TAP version 13
1..1
# #198/1   mptcp/base:OK
# #198/2   mptcp/mptcpify:OK
# #198/3   mptcp/subflow:OK
# #198/4   mptcp/iters_subflow:OK
# #198/5   mptcp/default:OK
# #198/6   mptcp/first:OK
# #198/7   mptcp/bkup:OK
# #198/8   mptcp/rr:OK
# #198/9   mptcp/red:OK
# #198/10  mptcp/burst:OK
# #198     mptcp:OK
# Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs_mptcp
# time=3
BPF Test: test_progs-cpuv4 -t mptcp
TAP version 13
1..1
# #198/1   mptcp/base:OK
# #198/2   mptcp/mptcpify:OK
# #198/3   mptcp/subflow:OK
# #198/4   mptcp/iters_subflow:OK
# #198/5   mptcp/default:OK
# #198/6   mptcp/first:OK
# #198/7   mptcp/bkup:OK
# #198/8   mptcp/rr:OK
# #198/9   mptcp/red:OK
# #198/10  mptcp/burst:OK
# #198     mptcp:OK
# Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-cpuv4_mptcp
# time=4
BPF Test: test_progs-no_alu32 -t mptcp
TAP version 13
1..1
# #198/1   mptcp/base:OK
# #198/2   mptcp/mptcpify:OK
# #198/3   mptcp/subflow:OK
# #198/4   mptcp/iters_subflow:OK
# #198/5   mptcp/default:OK
# #198/6   mptcp/first:OK
# #198/7   mptcp/bkup:OK
# #198/8   mptcp/rr:OK
# #198/9   mptcp/red:OK
# #198/10  mptcp/burst:OK
# #198     mptcp:OK
# Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-no_alu32_mptcp
# time=3

	=== Attempt: 466 (Thu, 29 May 2025 04:04:19 +0000) ===


This set also invalidates the following set named "add io thread mode
tests":
https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/

v2:
 - mptcp: fix the default value of scaling_ratio
 https://patchwork.kernel.org/project/mptcp/patch/0ccc1c26d27d6ee7be22806a97983d37c6ca548c.1715053270.git.tanggeliang@kylinos.cn/

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/487

Geliang Tang (3):
  selftests/bpf: Add sndbuf for send_recv_data
  Squash to "selftests/bpf: Add bpf scheduler test"
  DO-NOT-MERGE: selftests/bpf: Increase total_bytes of bpf sched tests

 tools/testing/selftests/bpf/network_helpers.c | 20 ++++++++++++++++++-
 tools/testing/selftests/bpf/network_helpers.h |  2 +-
 .../selftests/bpf/prog_tests/bpf_qdisc.c      |  2 +-
 .../selftests/bpf/prog_tests/bpf_tcp_ca.c     |  4 ++--
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 13 ++++++++++--
 5 files changed, 34 insertions(+), 7 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v3 1/3] selftests/bpf: Add sndbuf for send_recv_data

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This helper send_recv_data() is used by MPTCP BPF sched tests, but these
tests require explicit setting of sockopt SO_SNDBUF, so this patch adds
'sndbuf' parameter to this helper to set this sockopt of the accept socket.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/network_helpers.c | 20 ++++++++++++++++++-
 tools/testing/selftests/bpf/network_helpers.h |  2 +-
 .../selftests/bpf/prog_tests/bpf_qdisc.c      |  2 +-
 .../selftests/bpf/prog_tests/bpf_tcp_ca.c     |  4 ++--
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 72b5c174ab3b..9cdb80127804 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -92,6 +92,17 @@ int settimeo(int fd, int timeout_ms)
 	return 0;
 }
 
+static int setsndbuf(int fd, uint32_t sndbuf)
+{
+	if (sndbuf > 0 &&
+	    setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf))) {
+		log_err("Failed to set SO_SNDBUF");
+		return -1;
+	}
+
+	return 0;
+}
+
 #define save_errno_close(fd) ({ int __save = errno; close(fd); errno = __save; })
 
 int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen,
@@ -667,6 +678,7 @@ int set_hw_ring_size(char *ifname, struct ethtool_ringparam *ring_param)
 struct send_recv_arg {
 	int		fd;
 	uint32_t	bytes;
+	uint32_t	sndbuf;
 	int		stop;
 };
 
@@ -690,6 +702,11 @@ static void *send_recv_server(void *arg)
 		goto done;
 	}
 
+	if (setsndbuf(fd, a->sndbuf)) {
+		err = -errno;
+		goto done;
+	}
+
 	while (bytes < a->bytes && !READ_ONCE(a->stop)) {
 		nr_sent = send(fd, &batch,
 			       MIN(a->bytes - bytes, sizeof(batch)), 0);
@@ -718,12 +735,13 @@ static void *send_recv_server(void *arg)
 	return NULL;
 }
 
-int send_recv_data(int lfd, int fd, uint32_t total_bytes)
+int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf)
 {
 	ssize_t nr_recv = 0, bytes = 0;
 	struct send_recv_arg arg = {
 		.fd	= lfd,
 		.bytes	= total_bytes,
+		.sndbuf = sndbuf,
 		.stop	= 0,
 	};
 	pthread_t srv_thread;
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index ef208eefd571..cc0e5f812de2 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -98,7 +98,7 @@ struct nstoken;
  */
 struct nstoken *open_netns(const char *name);
 void close_netns(struct nstoken *token);
-int send_recv_data(int lfd, int fd, uint32_t total_bytes);
+int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf);
 int make_netns(const char *name);
 int remove_netns(const char *name);
 
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
index 730357cd0c9a..2c35ce21a4a5 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
@@ -35,7 +35,7 @@ static void do_test(char *qdisc)
 	if (!ASSERT_OK_FD(cli_fd, "connect to client"))
 		goto done;
 
-	err = send_recv_data(srv_fd, cli_fd, total_bytes);
+	err = send_recv_data(srv_fd, cli_fd, total_bytes, 0);
 	ASSERT_OK(err, "send_recv_data");
 
 done:
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
index b7d1b52309d0..090d876e0da4 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
@@ -70,7 +70,7 @@ static void do_test(const struct network_helper_opts *opts)
 	if (!start_test(NULL, opts, opts, &lfd, &fd))
 		goto done;
 
-	ASSERT_OK(send_recv_data(lfd, fd, total_bytes), "send_recv_data");
+	ASSERT_OK(send_recv_data(lfd, fd, total_bytes, 0), "send_recv_data");
 
 done:
 	if (lfd != -1)
@@ -169,7 +169,7 @@ static void test_dctcp(void)
 			!ASSERT_EQ(errno, ENOENT, "bpf_map_lookup_elem(sk_stg_map)"))
 		goto done;
 
-	ASSERT_OK(send_recv_data(lfd, fd, total_bytes), "send_recv_data");
+	ASSERT_OK(send_recv_data(lfd, fd, total_bytes, 0), "send_recv_data");
 	ASSERT_EQ(dctcp_skel->bss->stg_result, expected_stg, "stg_result");
 
 done:
-- 
2.43.0

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

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

BPF tests fail sometimes (a probability of approximately 1%) with
"bytes != total_bytes" errors:

 test_burst:PASS:open_and_load:burst 0 nsec
 test_bpf_sched:PASS:Scheduler name too long 0 nsec
 test_bpf_sched:PASS:burst 0 nsec
 create_netns:PASS:ip netns add mptcp_ns 0 nsec
 create_netns:PASS:ip -net mptcp_ns link set dev lo up 0 nsec
 sched_init:PASS:create_netns 0 nsec
 endpoint_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name
 endpoint_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
 endpoint_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
 endpoint_init:PASS:ip -net mptcp_ns addr add 10.0.1.2/24 dev veth2 0 nsec
 endpoint_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
 endpoint_init:PASS:ip -net mptcp_ns mptcp endpoint add 10.0.1.2 subflow
 sched_init:PASS:endpoint_init 0 nsec
 test_bpf_sched:PASS:burst 0 nsec
 send_data_and_verify:PASS:burst 0 nsec
 send_data_and_verify:PASS:burst 0 nsec
 (network_helpers.c:613: errno: Resource temporarily unavailable) \
                                        send 5608500 expected 10485760
 (network_helpers.c:661: errno: None) recv 2755984 expected 10485760
 (network_helpers.c:669: errno: None) Failed in thread_ret -11
 send_data_and_verify:FAIL:send_recv_data unexpected error: -4 (errno 0)
 multipath-tcp#162/9   mptcp/burst:FAIL
 multipath-tcp#162     mptcp:FAIL

This patch fixes this issue by setting SO_SNDBUF, while deliberately not
setting SO_RCVBUF.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/487
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 7c51250e7161..5402d42335c3 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -590,6 +590,8 @@ static void send_data_and_verify(char *sched, bool addr1, bool addr2)
 	struct timespec start, end;
 	int server_fd, client_fd;
 	unsigned int delta_ms;
+	unsigned int rcvbuf;
+	socklen_t optlen;
 
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
 	if (!ASSERT_OK_FD(server_fd, "start_mptcp_server"))
@@ -602,7 +604,14 @@ static void send_data_and_verify(char *sched, bool addr1, bool addr2)
 	if (clock_gettime(CLOCK_MONOTONIC, &start) < 0)
 		goto fail;
 
-	if (!ASSERT_OK(send_recv_data(server_fd, client_fd, total_bytes),
+	optlen = sizeof(rcvbuf);
+	if (getsockopt(server_fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf, &optlen) < 0)
+		goto fail;
+
+	if (setsockopt(server_fd, SOL_SOCKET, SO_SNDBUF, &rcvbuf, sizeof(rcvbuf)) < 0)
+		goto fail;
+
+	if (!ASSERT_OK(send_recv_data(server_fd, client_fd, total_bytes, rcvbuf),
 		       "send_recv_data"))
 		goto fail;
 
-- 
2.43.0

[PATCH mptcp-next v3 3/3] DO-NOT-MERGE: selftests/bpf: Increase total_bytes of bpf sched tests

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Increase total_bytes to make #487 easier to reproduce, to verify that the
previous two patches can indeed fix #487. This patch is only for testing,
please do not merge it, it will make mptcp bpf sched testing particularly
time-consuming.

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 5402d42335c3..0b40a9a07b5c 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -56,7 +56,7 @@
 #endif
 #define MPTCP_SCHED_NAME_MAX	16
 
-static const unsigned int total_bytes = 10 * 1024 * 1024;
+static const unsigned int total_bytes = 100 * 1024 * 1024;
 static int duration;
 
 struct __mptcp_info {
-- 
2.43.0

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[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-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/15316193959

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


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 0/3] send() fails with EAGAIN in blocking IO mode #487

[Mat Martineau]

On Thu, 29 May 2025, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Good news! I finally solved the unstable issue of MPTCP BPF sched selftests
> I reported a year ago, #487 "send() fails with EAGAIN in blocking IO mode".
>
> The fix is simple, it can be solved by explicitly setting SO_SNDBUF
> sockopt, but be sure not to set SO_RCVBUF at the same time
> (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in mptcp_rcv_space_adjust()).
>
> With this fix, BPF sched selftests are now very stable, I run loop testing
> using mptcp-upstream-virtme-docker (run_loop run_bpftest_all), and can run
> it normally for hundreds of times without error:
>

Hi Geliang -

I can see how changing SO_SNDBUF on the sending socket side would shift 
timing behavior in a way that affect the test outcome, but it doesn't 
address the root issue with bug #487:

It is either OK to get an EAGAIN from a blocking send(), or it's not OK.


If it's not ok to ever return EAGAIN from a blocking send, the existing 
test code is a reproducer for a bug, and changing the test is hiding that 
bug.

If EAGAIN is ok, then we should change the code in send_recv_server() to 
allow it.


As Matthieu noted in the comment here 
(https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2083123666), 
it may be due to the BPF context that the socket is really non-blocking. 
Did we ever figure out if that was the case - I couldn't find the 
discussion on the mailing list?


- Mat


>
> 	=== Attempt: 465 (Thu, 29 May 2025 04:04:09 +0000) ===
>
> BPF Test: test_progs -t mptcp
> TAP version 13
> 1..1
> # #198/1   mptcp/base:OK
> # #198/2   mptcp/mptcpify:OK
> # #198/3   mptcp/subflow:OK
> # #198/4   mptcp/iters_subflow:OK
> # #198/5   mptcp/default:OK
> # #198/6   mptcp/first:OK
> # #198/7   mptcp/bkup:OK
> # #198/8   mptcp/rr:OK
> # #198/9   mptcp/red:OK
> # #198/10  mptcp/burst:OK
> # #198     mptcp:OK
> # Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
> ok 1 test: bpftest_test_progs_mptcp
> # time=3
> BPF Test: test_progs-cpuv4 -t mptcp
> TAP version 13
> 1..1
> # #198/1   mptcp/base:OK
> # #198/2   mptcp/mptcpify:OK
> # #198/3   mptcp/subflow:OK
> # #198/4   mptcp/iters_subflow:OK
> # #198/5   mptcp/default:OK
> # #198/6   mptcp/first:OK
> # #198/7   mptcp/bkup:OK
> # #198/8   mptcp/rr:OK
> # #198/9   mptcp/red:OK
> # #198/10  mptcp/burst:OK
> # #198     mptcp:OK
> # Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
> ok 1 test: bpftest_test_progs-cpuv4_mptcp
> # time=4
> BPF Test: test_progs-no_alu32 -t mptcp
> TAP version 13
> 1..1
> # #198/1   mptcp/base:OK
> # #198/2   mptcp/mptcpify:OK
> # #198/3   mptcp/subflow:OK
> # #198/4   mptcp/iters_subflow:OK
> # #198/5   mptcp/default:OK
> # #198/6   mptcp/first:OK
> # #198/7   mptcp/bkup:OK
> # #198/8   mptcp/rr:OK
> # #198/9   mptcp/red:OK
> # #198/10  mptcp/burst:OK
> # #198     mptcp:OK
> # Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
> ok 1 test: bpftest_test_progs-no_alu32_mptcp
> # time=3
>
> 	=== Attempt: 466 (Thu, 29 May 2025 04:04:19 +0000) ===
>
>
> This set also invalidates the following set named "add io thread mode
> tests":
> https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/
>
> v2:
> - mptcp: fix the default value of scaling_ratio
> https://patchwork.kernel.org/project/mptcp/patch/0ccc1c26d27d6ee7be22806a97983d37c6ca548c.1715053270.git.tanggeliang@kylinos.cn/
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/487
>
> Geliang Tang (3):
>  selftests/bpf: Add sndbuf for send_recv_data
>  Squash to "selftests/bpf: Add bpf scheduler test"
>  DO-NOT-MERGE: selftests/bpf: Increase total_bytes of bpf sched tests
>
> tools/testing/selftests/bpf/network_helpers.c | 20 ++++++++++++++++++-
> tools/testing/selftests/bpf/network_helpers.h |  2 +-
> .../selftests/bpf/prog_tests/bpf_qdisc.c      |  2 +-
> .../selftests/bpf/prog_tests/bpf_tcp_ca.c     |  4 ++--
> .../testing/selftests/bpf/prog_tests/mptcp.c  | 13 ++++++++++--
> 5 files changed, 34 insertions(+), 7 deletions(-)
>
> -- 
> 2.43.0
>
>
>

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Matthieu Baerts]

Hi Mat, Geliang,

On 14/06/2025 01:11, Mat Martineau wrote:
> On Thu, 29 May 2025, Geliang Tang wrote:
> 
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> Good news! I finally solved the unstable issue of MPTCP BPF sched
>> selftests
>> I reported a year ago, #487 "send() fails with EAGAIN in blocking IO
>> mode".
>>
>> The fix is simple, it can be solved by explicitly setting SO_SNDBUF
>> sockopt, but be sure not to set SO_RCVBUF at the same time
>> (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in mptcp_rcv_space_adjust()).
>>
>> With this fix, BPF sched selftests are now very stable, I run loop
>> testing
>> using mptcp-upstream-virtme-docker (run_loop run_bpftest_all), and can
>> run
>> it normally for hundreds of times without error:
>>
> 
> Hi Geliang -
> 
> I can see how changing SO_SNDBUF on the sending socket side would shift
> timing behavior in a way that affect the test outcome, but it doesn't
> address the root issue with bug #487:
> 
> It is either OK to get an EAGAIN from a blocking send(), or it's not OK.
> 
> 
> If it's not ok to ever return EAGAIN from a blocking send, the existing
> test code is a reproducer for a bug, and changing the test is hiding
> that bug.
> 
> If EAGAIN is ok, then we should change the code in send_recv_server() to
> allow it.

It is now a bit hidden in the middle of #487, but if I'm not mistaken,
it is OK to get EAGAIN with a blocking send() **if** SO_SNDTIMEO is
used, and in case of timeout.

See:
https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2485577676

So I think the question should be: is it normal to block for longer than
the timeout period (which is a "long" period, no?)? If yes, then
limiting the send buffer might be a solution, but as Mat said, it looks
better to understand the root cause than hiding a bug :)

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

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Geliang Tang]

[-- Attachment #1: Type: text/plain, Size: 3398 bytes --]

Hi Matt, Mat,

On Sun, 2025-06-15 at 23:29 +0200, Matthieu Baerts wrote:
> Hi Mat, Geliang,
> 
> On 14/06/2025 01:11, Mat Martineau wrote:
> > On Thu, 29 May 2025, Geliang Tang wrote:
> > 
> > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > 
> > > Good news! I finally solved the unstable issue of MPTCP BPF sched
> > > selftests
> > > I reported a year ago, #487 "send() fails with EAGAIN in blocking
> > > IO
> > > mode".
> > > 
> > > The fix is simple, it can be solved by explicitly setting
> > > SO_SNDBUF
> > > sockopt, but be sure not to set SO_RCVBUF at the same time
> > > (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in
> > > mptcp_rcv_space_adjust()).
> > > 
> > > With this fix, BPF sched selftests are now very stable, I run
> > > loop
> > > testing
> > > using mptcp-upstream-virtme-docker (run_loop run_bpftest_all),
> > > and can
> > > run
> > > it normally for hundreds of times without error:
> > > 
> > 
> > Hi Geliang -
> > 
> > I can see how changing SO_SNDBUF on the sending socket side would
> > shift
> > timing behavior in a way that affect the test outcome, but it
> > doesn't
> > address the root issue with bug #487:
> > 
> > It is either OK to get an EAGAIN from a blocking send(), or it's
> > not OK.
> > 
> > 
> > If it's not ok to ever return EAGAIN from a blocking send, the
> > existing
> > test code is a reproducer for a bug, and changing the test is
> > hiding
> > that bug.
> > 
> > If EAGAIN is ok, then we should change the code in
> > send_recv_server() to
> > allow it.

I did try to handle EAGAIN in send_recv_server() but it didn't work.
MPTCP BPF sched selftests still fail. Test code and results are
attached.

I added this in send_recv_server():

	if (errno == EAGAIN && again < 5) {
		again++;
		continue;
	}

And still got the EAGAIN error:

# (network_helpers.c:728: errno: Resource temporarily unavailable) send
7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv
3469500 ​​expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable)
Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno
11)

In addition, BPF selftests adds a new mechanism that does not allow any
test item to run for more than 10 seconds. Otherwise, the following
error will be reported:

# WATCHDOG: test case mptcp/default executes for 10 seconds...

In my testing, I have not found any other solution besides limiting the
send buffer. This allows data to be sent at a constant rate, which
ensures the stability of MPTCP BPF sched selftests.

In order to avoid hiding this bug, we can add a test item for this in
mptcp selftest in the future, like in [1].

WDYT?

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/

> 
> It is now a bit hidden in the middle of #487, but if I'm not
> mistaken,
> it is OK to get EAGAIN with a blocking send() **if** SO_SNDTIMEO is
> used, and in case of timeout.
> 
> See:
> https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2485577676
> 
> So I think the question should be: is it normal to block for longer
> than
> the timeout period (which is a "long" period, no?)? If yes, then
> limiting the send buffer might be a solution, but as Mat said, it
> looks
> better to understand the root cause than hiding a bug :)
> 
> Cheers,
> Matt

[-- Attachment #2: bpf-sched-selftests-eagain-err.diff --]
[-- Type: text/x-patch, Size: 1693 bytes --]

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 9cdb80127804..e9966282b664 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -94,11 +94,11 @@ int settimeo(int fd, int timeout_ms)
 
 static int setsndbuf(int fd, uint32_t sndbuf)
 {
-	if (sndbuf > 0 &&
-	    setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf))) {
-		log_err("Failed to set SO_SNDBUF");
-		return -1;
-	}
+	//if (sndbuf > 0 &&
+	//    setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf))) {
+	//	log_err("Failed to set SO_SNDBUF");
+	//	return -1;
+	//}
 
 	return 0;
 }
@@ -688,6 +688,7 @@ static void *send_recv_server(void *arg)
 	ssize_t nr_sent = 0, bytes = 0;
 	char batch[1500];
 	int err = 0, fd;
+	int again = 0;
 
 	fd = accept(a->fd, NULL, NULL);
 	while (fd == -1) {
@@ -713,6 +714,10 @@ static void *send_recv_server(void *arg)
 		if (nr_sent == -1 && errno == EINTR)
 			continue;
 		if (nr_sent == -1) {
+			if (errno == EAGAIN && again < 5) {
+				again++;
+				continue;
+			}
 			err = -errno;
 			break;
 		}
@@ -747,6 +752,7 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf)
 	pthread_t srv_thread;
 	void *thread_ret;
 	char batch[1500];
+	int again = 0;
 	int err = 0;
 
 	err = pthread_create(&srv_thread, NULL, send_recv_server, (void *)&arg);
@@ -762,6 +768,10 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf)
 		if (nr_recv == -1 && errno == EINTR)
 			continue;
 		if (nr_recv == -1) {
+			if (errno == EAGAIN && again < 5) {
+				again++;
+				continue;
+			}
 			err = -errno;
 			break;
 		}

[-- Attachment #3: bpf-sched-selftests-eagain-err.log --]
[-- Type: text/x-log, Size: 4834 bytes --]


	=== Attempt: 28 (Sat, 14 Jun 2025 12:31:17 +0000) ===


BPF Test: test_progs -t mptcp
TAP version 13
1..1
# WATCHDOG: test case mptcp/default executes for 10 seconds...
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# address_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:1::1/64 dev veth1 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.2.1/24 dev veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:2::1/64 dev veth2 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
# address_init:PASS:ip -net mptcp_ns link add veth3 type veth peer name veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.3.1/24 dev veth3 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:3::1/64 dev veth3 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth3 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.4.1/24 dev veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:4::1/64 dev veth4 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth4 up 0 nsec
# sched_init:PASS:ip netns exec mptcp_ns sysctl -qw net.mptcp.scheduler=default 0 nsec
# test_default:PASS:sched_init 0 nsec
# send_data_and_verify:PASS:start_mptcp_server 0 nsec
# send_data_and_verify:PASS:connect_to_fd 0 nsec
# (network_helpers.c:728: errno: Resource temporarily unavailable) send 7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv 3469500 expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable) Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno 11)
# #203/4   mptcp/default:FAIL
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:FAIL
# 
# All error logs:
# address_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:1::1/64 dev veth1 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.2.1/24 dev veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:2::1/64 dev veth2 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
# address_init:PASS:ip -net mptcp_ns link add veth3 type veth peer name veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.3.1/24 dev veth3 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:3::1/64 dev veth3 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth3 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.4.1/24 dev veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:4::1/64 dev veth4 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth4 up 0 nsec
# sched_init:PASS:ip netns exec mptcp_ns sysctl -qw net.mptcp.scheduler=default 0 nsec
# test_default:PASS:sched_init 0 nsec
# send_data_and_verify:PASS:start_mptcp_server 0 nsec
# send_data_and_verify:PASS:connect_to_fd 0 nsec
# (network_helpers.c:728: errno: Resource temporarily unavailable) send 7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv 3469500 expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable) Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno 11)
# #203/4   mptcp/default:FAIL
# #203     mptcp:FAIL
# Summary: 0/8 PASSED, 0 SKIPPED, 1 FAILED
not ok 1 test: bpftest_test_progs_mptcp # FAIL
# time=21
BPF Test: test_progs-cpuv4 -t mptcp
TAP version 13
1..1
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# #203/4   mptcp/default:OK
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:OK
# Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-cpuv4_mptcp
# time=4
BPF Test: test_progs-no_alu32 -t mptcp
TAP version 13
1..1
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# #203/4   mptcp/default:OK
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:OK
# Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-no_alu32_mptcp
# time=4


	=== ERROR after 28 attempts (Sat, 14 Jun 2025 12:31:46 +0000) ===

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Geliang Tang]

Hi Mat, Matt,

On Mon, 2025-06-16 at 14:34 +0800, Geliang Tang wrote:
> Hi Matt, Mat,
> 
> On Sun, 2025-06-15 at 23:29 +0200, Matthieu Baerts wrote:
> > Hi Mat, Geliang,
> > 
> > On 14/06/2025 01:11, Mat Martineau wrote:
> > > On Thu, 29 May 2025, Geliang Tang wrote:
> > > 
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > Good news! I finally solved the unstable issue of MPTCP BPF
> > > > sched
> > > > selftests
> > > > I reported a year ago, #487 "send() fails with EAGAIN in
> > > > blocking
> > > > IO
> > > > mode".
> > > > 
> > > > The fix is simple, it can be solved by explicitly setting
> > > > SO_SNDBUF
> > > > sockopt, but be sure not to set SO_RCVBUF at the same time
> > > > (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in
> > > > mptcp_rcv_space_adjust()).
> > > > 
> > > > With this fix, BPF sched selftests are now very stable, I run
> > > > loop
> > > > testing
> > > > using mptcp-upstream-virtme-docker (run_loop run_bpftest_all),
> > > > and can
> > > > run
> > > > it normally for hundreds of times without error:
> > > > 
> > > 
> > > Hi Geliang -
> > > 
> > > I can see how changing SO_SNDBUF on the sending socket side would
> > > shift
> > > timing behavior in a way that affect the test outcome, but it
> > > doesn't
> > > address the root issue with bug #487:
> > > 
> > > It is either OK to get an EAGAIN from a blocking send(), or it's
> > > not OK.
> > > 
> > > 
> > > If it's not ok to ever return EAGAIN from a blocking send, the
> > > existing
> > > test code is a reproducer for a bug, and changing the test is
> > > hiding
> > > that bug.
> > > 
> > > If EAGAIN is ok, then we should change the code in
> > > send_recv_server() to
> > > allow it.
> 
> I did try to handle EAGAIN in send_recv_server() but it didn't work.
> MPTCP BPF sched selftests still fail. Test code and results are
> attached.
> 
> I added this in send_recv_server():
> 
> 	if (errno == EAGAIN && again < 5) {
> 		again++;
> 		continue;
> 	}
> 
> And still got the EAGAIN error:
> 
> # (network_helpers.c:728: errno: Resource temporarily unavailable)
> send
> 7867500 expected 10485760
> # (network_helpers.c:782: errno: Resource temporarily unavailable)
> recv
> 3469500 ​​expected 10485760
> # (network_helpers.c:790: errno: Resource temporarily unavailable)
> Failed in thread_ret -11
> # send_data_and_verify:FAIL:send_recv_data unexpected error: -11
> (errno
> 11)
> 
> In addition, BPF selftests adds a new mechanism that does not allow
> any
> test item to run for more than 10 seconds. Otherwise, the following
> error will be reported:
> 
> # WATCHDOG: test case mptcp/default executes for 10 seconds...
> 
> In my testing, I have not found any other solution besides limiting
> the
> send buffer. This allows data to be sent at a constant rate, which
> ensures the stability of MPTCP BPF sched selftests.
> 
> In order to avoid hiding this bug, we can add a test item for this in
> mptcp selftest in the future, like in [1].

Last week I debugged this issue further and found something (Thanks to
Gang Yan for his help): when mptcp bpf sched selftests fail, the memory
limit check (if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)) is always true
in __mptcp_move_skbs_from_subflow(), at this time sk->sk_receive_queue
is empty, but no skb is moved from this subflow to sk-
>sk_receive_queue, which causes the transmission to fail.

One fix is ​​to also consider the case where sk->sk_receive_queue is
empty when doing the memory limit check:

  -               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
  +               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf &&
  +                   !skb_queue_empty(&sk->sk_receive_queue))
                          break;

In addition, this memory limit check was moved from the end of do {}
while (more_data_avail) to the front in the commit e0ca4057e0ec
("mptcp: micro-optimize __mptcp_move_skb()"), so another better fix is
​​to restore this check to the end of do {} while () so that move skbs
from this subflow to sk->sk_receive_queue always has a chance to do at
least once:

@@ -587,9 +587,6 @@ static bool __mptcp_move_skbs_from_subflow(struct
mptcp_sock *msk,
                struct sk_buff *skb;
                bool fin;
 
-               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
-                       break;
-
                /* try to move as much data as available */
                map_remaining = subflow->map_data_len -
                                mptcp_subflow_get_map_offset(subflow);
@@ -634,6 +631,8 @@ static bool __mptcp_move_skbs_from_subflow(struct
mptcp_sock *msk,
                WRITE_ONCE(tp->copied_seq, seq);
                more_data_avail = mptcp_subflow_data_available(ssk);
 
+               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
+                       break;
        } while (more_data_avail);

I think this is the root cause of #487, I would like to hear your
opinions on which fix is ​​better.

Thanks,
-Geliang

> 
> WDYT?
> 
> Thanks,
> -Geliang
> 
> [1]
> https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/
> 
> > 
> > It is now a bit hidden in the middle of #487, but if I'm not
> > mistaken,
> > it is OK to get EAGAIN with a blocking send() **if** SO_SNDTIMEO is
> > used, and in case of timeout.
> > 
> > See:
> > https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2485577676
> > 
> > So I think the question should be: is it normal to block for longer
> > than
> > the timeout period (which is a "long" period, no?)? If yes, then
> > limiting the send buffer might be a solution, but as Mat said, it
> > looks
> > better to understand the root cause than hiding a bug :)
> > 
> > Cheers,
> > Matt

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 6600 bytes --]


Hi Geliang -

On Sun, 29 Jun 2025, Geliang Tang wrote:

> Hi Mat, Matt,
>
> On Mon, 2025-06-16 at 14:34 +0800, Geliang Tang wrote:
>> Hi Matt, Mat,
>>
>> On Sun, 2025-06-15 at 23:29 +0200, Matthieu Baerts wrote:
>>> Hi Mat, Geliang,
>>>
>>> On 14/06/2025 01:11, Mat Martineau wrote:
>>>> On Thu, 29 May 2025, Geliang Tang wrote:
>>>>
>>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>>
>>>>> Good news! I finally solved the unstable issue of MPTCP BPF
>>>>> sched
>>>>> selftests
>>>>> I reported a year ago, #487 "send() fails with EAGAIN in
>>>>> blocking
>>>>> IO
>>>>> mode".
>>>>>
>>>>> The fix is simple, it can be solved by explicitly setting
>>>>> SO_SNDBUF
>>>>> sockopt, but be sure not to set SO_RCVBUF at the same time
>>>>> (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in
>>>>> mptcp_rcv_space_adjust()).
>>>>>
>>>>> With this fix, BPF sched selftests are now very stable, I run
>>>>> loop
>>>>> testing
>>>>> using mptcp-upstream-virtme-docker (run_loop run_bpftest_all),
>>>>> and can
>>>>> run
>>>>> it normally for hundreds of times without error:
>>>>>
>>>>
>>>> Hi Geliang -
>>>>
>>>> I can see how changing SO_SNDBUF on the sending socket side would
>>>> shift
>>>> timing behavior in a way that affect the test outcome, but it
>>>> doesn't
>>>> address the root issue with bug #487:
>>>>
>>>> It is either OK to get an EAGAIN from a blocking send(), or it's
>>>> not OK.
>>>>
>>>>
>>>> If it's not ok to ever return EAGAIN from a blocking send, the
>>>> existing
>>>> test code is a reproducer for a bug, and changing the test is
>>>> hiding
>>>> that bug.
>>>>
>>>> If EAGAIN is ok, then we should change the code in
>>>> send_recv_server() to
>>>> allow it.
>>
>> I did try to handle EAGAIN in send_recv_server() but it didn't work.
>> MPTCP BPF sched selftests still fail. Test code and results are
>> attached.
>>
>> I added this in send_recv_server():
>>
>> 	if (errno == EAGAIN && again < 5) {
>> 		again++;
>> 		continue;
>> 	}
>>
>> And still got the EAGAIN error:
>>
>> # (network_helpers.c:728: errno: Resource temporarily unavailable)
>> send
>> 7867500 expected 10485760
>> # (network_helpers.c:782: errno: Resource temporarily unavailable)
>> recv
>> 3469500 ​​expected 10485760
>> # (network_helpers.c:790: errno: Resource temporarily unavailable)
>> Failed in thread_ret -11
>> # send_data_and_verify:FAIL:send_recv_data unexpected error: -11
>> (errno
>> 11)
>>
>> In addition, BPF selftests adds a new mechanism that does not allow
>> any
>> test item to run for more than 10 seconds. Otherwise, the following
>> error will be reported:
>>
>> # WATCHDOG: test case mptcp/default executes for 10 seconds...
>>
>> In my testing, I have not found any other solution besides limiting
>> the
>> send buffer. This allows data to be sent at a constant rate, which
>> ensures the stability of MPTCP BPF sched selftests.
>>
>> In order to avoid hiding this bug, we can add a test item for this in
>> mptcp selftest in the future, like in [1].
>
> Last week I debugged this issue further and found something (Thanks to
> Gang Yan for his help): when mptcp bpf sched selftests fail, the memory
> limit check (if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)) is always true
> in __mptcp_move_skbs_from_subflow(), at this time sk->sk_receive_queue
> is empty,

Thanks for tracking down some more details!

> but no skb is moved from this subflow to sk->
> sk_receive_queue, which causes the transmission to fail.
>

I'm trying to understand this part (above) better. 
__mptcp_move_skbs_from_subflow() is on the receive path. What is the path 
to causing transmission to fail? Does exiting the loop early on the rx 
side leave ssk->sk_err set, so an error propagates in the msk and 
sendmsg() returns early with EAGAIN?

> One fix is ​​to also consider the case where sk->sk_receive_queue is
> empty when doing the memory limit check:
>
>  -               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
>  +               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf &&
>  +                   !skb_queue_empty(&sk->sk_receive_queue))
>                          break;

The msk sk_receive_queue is probably empty a lot of the time, does it look 
like the empty queue is related to the failure?

>
> In addition, this memory limit check was moved from the end of do {}
> while (more_data_avail) to the front in the commit e0ca4057e0ec
> ("mptcp: micro-optimize __mptcp_move_skb()"), so another better fix is
> ​​to restore this check to the end of do {} while () so that move skbs
> from this subflow to sk->sk_receive_queue always has a chance to do at
> least once:
>
> @@ -587,9 +587,6 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>                struct sk_buff *skb;
>                bool fin;
>
> -               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
> -                       break;
> -
>                /* try to move as much data as available */
>                map_remaining = subflow->map_data_len -
>                                mptcp_subflow_get_map_offset(subflow);
> @@ -634,6 +631,8 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>                WRITE_ONCE(tp->copied_seq, seq);
>                more_data_avail = mptcp_subflow_data_available(ssk);
>
> +               if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
> +                       break;
>        } while (more_data_avail);
>
> I think this is the root cause of #487, I would like to hear your
> opinions on which fix is ​​better.

I have a slight preference for the second possible fix since it still 
provides an exit from the loop even when the sk_receive_queue is empty. 
Can you send that patch?

Before we merge a fix I'm still hoping to understand how letting this loop 
body run once on the rx path is changing the tx side behavior!

Thanks,
Mat

>
> Thanks,
> -Geliang
>
>>
>> WDYT?
>>
>> Thanks,
>> -Geliang
>>
>> [1]
>> https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/
>>
>>>
>>> It is now a bit hidden in the middle of #487, but if I'm not
>>> mistaken,
>>> it is OK to get EAGAIN with a blocking send() **if** SO_SNDTIMEO is
>>> used, and in case of timeout.
>>>
>>> See:
>>> https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2485577676
>>>
>>> So I think the question should be: is it normal to block for longer
>>> than
>>> the timeout period (which is a "long" period, no?)? If yes, then
>>> limiting the send buffer might be a solution, but as Mat said, it
>>> looks
>>> better to understand the root cause than hiding a bug :)
>>>
>>> Cheers,
>>> Matt
>

Re: [PATCH mptcp-next v3 0/3] send() fails with EAGAIN in blocking IO mode #487

[Geliang Tang]

[-- Attachment #1: Type: text/plain, Size: 3398 bytes --]

Hi Matt, Mat,

On Sun, 2025-06-15 at 23:29 +0200, Matthieu Baerts wrote:
> Hi Mat, Geliang,
> 
> On 14/06/2025 01:11, Mat Martineau wrote:
> > On Thu, 29 May 2025, Geliang Tang wrote:
> > 
> > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > 
> > > Good news! I finally solved the unstable issue of MPTCP BPF sched
> > > selftests
> > > I reported a year ago, #487 "send() fails with EAGAIN in blocking
> > > IO
> > > mode".
> > > 
> > > The fix is simple, it can be solved by explicitly setting
> > > SO_SNDBUF
> > > sockopt, but be sure not to set SO_RCVBUF at the same time
> > > (see sk->sk_userlocks & SOCK_RCVBUF_LOCK in
> > > mptcp_rcv_space_adjust()).
> > > 
> > > With this fix, BPF sched selftests are now very stable, I run
> > > loop
> > > testing
> > > using mptcp-upstream-virtme-docker (run_loop run_bpftest_all),
> > > and can
> > > run
> > > it normally for hundreds of times without error:
> > > 
> > 
> > Hi Geliang -
> > 
> > I can see how changing SO_SNDBUF on the sending socket side would
> > shift
> > timing behavior in a way that affect the test outcome, but it
> > doesn't
> > address the root issue with bug #487:
> > 
> > It is either OK to get an EAGAIN from a blocking send(), or it's
> > not OK.
> > 
> > 
> > If it's not ok to ever return EAGAIN from a blocking send, the
> > existing
> > test code is a reproducer for a bug, and changing the test is
> > hiding
> > that bug.
> > 
> > If EAGAIN is ok, then we should change the code in
> > send_recv_server() to
> > allow it.

I did try to handle EAGAIN in send_recv_server() but it didn't work.
MPTCP BPF sched selftests still fail. Test code and results are
attached.

I added this in send_recv_server():

	if (errno == EAGAIN && again < 5) {
		again++;
		continue;
	}

And still got the EAGAIN error:

# (network_helpers.c:728: errno: Resource temporarily unavailable) send
7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv
3469500 ​​expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable)
Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno
11)

In addition, BPF selftests adds a new mechanism that does not allow any
test item to run for more than 10 seconds. Otherwise, the following
error will be reported:

# WATCHDOG: test case mptcp/default executes for 10 seconds...

In my testing, I have not found any other solution besides limiting the
send buffer. This allows data to be sent at a constant rate, which
ensures the stability of MPTCP BPF sched selftests.

In order to avoid hiding this bug, we can add a test item for this in
mptcp selftest in the future, like in [1].

WDYT?

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/cover/cover.1722502941.git.tanggeliang@kylinos.cn/

> 
> It is now a bit hidden in the middle of #487, but if I'm not
> mistaken,
> it is OK to get EAGAIN with a blocking send() **if** SO_SNDTIMEO is
> used, and in case of timeout.
> 
> See:
> https://github.com/multipath-tcp/mptcp_net-next/issues/487#issuecomment-2485577676
> 
> So I think the question should be: is it normal to block for longer
> than
> the timeout period (which is a "long" period, no?)? If yes, then
> limiting the send buffer might be a solution, but as Mat said, it
> looks
> better to understand the root cause than hiding a bug :)
> 
> Cheers,
> Matt

[-- Attachment #2: bpf-sched-selftests-eagain-err.diff --]
[-- Type: text/x-patch, Size: 1693 bytes --]

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 9cdb80127804..e9966282b664 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -94,11 +94,11 @@ int settimeo(int fd, int timeout_ms)
 
 static int setsndbuf(int fd, uint32_t sndbuf)
 {
-	if (sndbuf > 0 &&
-	    setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf))) {
-		log_err("Failed to set SO_SNDBUF");
-		return -1;
-	}
+	//if (sndbuf > 0 &&
+	//    setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf))) {
+	//	log_err("Failed to set SO_SNDBUF");
+	//	return -1;
+	//}
 
 	return 0;
 }
@@ -688,6 +688,7 @@ static void *send_recv_server(void *arg)
 	ssize_t nr_sent = 0, bytes = 0;
 	char batch[1500];
 	int err = 0, fd;
+	int again = 0;
 
 	fd = accept(a->fd, NULL, NULL);
 	while (fd == -1) {
@@ -713,6 +714,10 @@ static void *send_recv_server(void *arg)
 		if (nr_sent == -1 && errno == EINTR)
 			continue;
 		if (nr_sent == -1) {
+			if (errno == EAGAIN && again < 5) {
+				again++;
+				continue;
+			}
 			err = -errno;
 			break;
 		}
@@ -747,6 +752,7 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf)
 	pthread_t srv_thread;
 	void *thread_ret;
 	char batch[1500];
+	int again = 0;
 	int err = 0;
 
 	err = pthread_create(&srv_thread, NULL, send_recv_server, (void *)&arg);
@@ -762,6 +768,10 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes, uint32_t sndbuf)
 		if (nr_recv == -1 && errno == EINTR)
 			continue;
 		if (nr_recv == -1) {
+			if (errno == EAGAIN && again < 5) {
+				again++;
+				continue;
+			}
 			err = -errno;
 			break;
 		}

[-- Attachment #3: bpf-sched-selftests-eagain-err.log --]
[-- Type: text/x-log, Size: 4834 bytes --]


	=== Attempt: 28 (Sat, 14 Jun 2025 12:31:17 +0000) ===


BPF Test: test_progs -t mptcp
TAP version 13
1..1
# WATCHDOG: test case mptcp/default executes for 10 seconds...
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# address_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:1::1/64 dev veth1 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.2.1/24 dev veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:2::1/64 dev veth2 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
# address_init:PASS:ip -net mptcp_ns link add veth3 type veth peer name veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.3.1/24 dev veth3 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:3::1/64 dev veth3 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth3 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.4.1/24 dev veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:4::1/64 dev veth4 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth4 up 0 nsec
# sched_init:PASS:ip netns exec mptcp_ns sysctl -qw net.mptcp.scheduler=default 0 nsec
# test_default:PASS:sched_init 0 nsec
# send_data_and_verify:PASS:start_mptcp_server 0 nsec
# send_data_and_verify:PASS:connect_to_fd 0 nsec
# (network_helpers.c:728: errno: Resource temporarily unavailable) send 7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv 3469500 expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable) Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno 11)
# #203/4   mptcp/default:FAIL
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:FAIL
# 
# All error logs:
# address_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:1::1/64 dev veth1 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.2.1/24 dev veth2 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:2::1/64 dev veth2 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
# address_init:PASS:ip -net mptcp_ns link add veth3 type veth peer name veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.3.1/24 dev veth3 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:3::1/64 dev veth3 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth3 up 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add 10.0.4.1/24 dev veth4 0 nsec
# address_init:PASS:ip -net mptcp_ns addr add dead:beef:4::1/64 dev veth4 nodad 0 nsec
# address_init:PASS:ip -net mptcp_ns link set dev veth4 up 0 nsec
# sched_init:PASS:ip netns exec mptcp_ns sysctl -qw net.mptcp.scheduler=default 0 nsec
# test_default:PASS:sched_init 0 nsec
# send_data_and_verify:PASS:start_mptcp_server 0 nsec
# send_data_and_verify:PASS:connect_to_fd 0 nsec
# (network_helpers.c:728: errno: Resource temporarily unavailable) send 7867500 expected 10485760
# (network_helpers.c:782: errno: Resource temporarily unavailable) recv 3469500 expected 10485760
# (network_helpers.c:790: errno: Resource temporarily unavailable) Failed in thread_ret -11
# send_data_and_verify:FAIL:send_recv_data unexpected error: -11 (errno 11)
# #203/4   mptcp/default:FAIL
# #203     mptcp:FAIL
# Summary: 0/8 PASSED, 0 SKIPPED, 1 FAILED
not ok 1 test: bpftest_test_progs_mptcp # FAIL
# time=21
BPF Test: test_progs-cpuv4 -t mptcp
TAP version 13
1..1
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# #203/4   mptcp/default:OK
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:OK
# Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-cpuv4_mptcp
# time=4
BPF Test: test_progs-no_alu32 -t mptcp
TAP version 13
1..1
# #203/1   mptcp/base:OK
# #203/2   mptcp/mptcpify:OK
# #203/3   mptcp/subflow:OK
# #203/4   mptcp/default:OK
# #203/5   mptcp/first:OK
# #203/6   mptcp/bkup:OK
# #203/7   mptcp/rr:OK
# #203/8   mptcp/red:OK
# #203/9   mptcp/burst:OK
# #203     mptcp:OK
# Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
ok 1 test: bpftest_test_progs-no_alu32_mptcp
# time=4


	=== ERROR after 28 attempts (Sat, 14 Jun 2025 12:31:46 +0000) ===