[PATCH mptcp-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[PATCH mptcp-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[Quanye Yang via B4 Relay]

From: Quanye Yang <quanyeyang@proton.me>

process_one_client() used a single read() and write() before waiting
for hangup. A SOCK_STREAM read() can return a short count when the
payload is segmented, so the server echoed only part of the data and
then blocked on FIN.

The client keeps reading until it has the full length and only then
closes the socket. The two sides wait on each other and the test
hangs until SIGALRM.

Loop the server read/write until read() returns 0, and accumulate the
copied byte counts for the existing sockopt checks.

Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/639
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 34 +++++++++++++----------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index b6e58d936ebe..5e1ec3336859 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -638,7 +638,7 @@ static void connect_one_server(int fd, int pipefd)
 
 static void process_one_client(int fd, int pipefd)
 {
-	ssize_t ret, ret2, ret3;
+	ssize_t ret, ret2, n, w;
 	struct so_state s;
 	char buf[4096];
 
@@ -648,23 +648,27 @@ static void process_one_client(int fd, int pipefd)
 	ret = write(pipefd, "xmit", 4);
 	assert(ret == 4);
 
-	ret = read(fd, buf, sizeof(buf));
-	if (ret < 0)
-		die_perror("read");
+	ret = 0;
+	ret2 = 0;
+	while ((n = read(fd, buf, sizeof(buf))) > 0) {
+		if (!ret) {
+			assert(s.mptcpi_rcv_delta <= (uint64_t)n);
 
-	assert(s.mptcpi_rcv_delta <= (uint64_t)ret);
-
-	if (s.tcpi_rcv_delta)
-		assert(s.tcpi_rcv_delta == (uint64_t)ret);
+			if (s.tcpi_rcv_delta)
+				assert(s.tcpi_rcv_delta <= (uint64_t)n);
+		}
 
-	ret2 = write(fd, buf, ret);
-	if (ret2 < 0)
-		die_perror("write");
+		w = write(fd, buf, n);
+		if (w < 0)
+			die_perror("write");
+		if (w != n)
+			xerror("short write");
 
-	/* wait for hangup */
-	ret3 = read(fd, buf, 1);
-	if (ret3 != 0)
-		xerror("expected EOF, got %lu", ret3);
+		ret += n;
+		ret2 += w;
+	}
+	if (n < 0)
+		die_perror("read");
 
 	do_getsockopts(&s, fd, ret, ret2);
 	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)

---
base-commit: a22035be824449c4921ec406f6383089016a628c
change-id: 20261017-fix-mptcp-issue-639-96339a3d921a

Best regards,
--  
Quanye Yang <quanyeyang@proton.me>

Re: [PATCH mptcp-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: mptcp: connect_one_server() passes partial ret to do_getsockopts()
--

commit 522b0ff16267ee948ab4943b8089644ce3f33c32
Author: Quanye Yang <quanyeyang@proton.me>

selftests: mptcp: sockopt: echo until EOF on partial reads

This commit updates process_one_client() in mptcp_sockopt.c to loop over
read() and write() until EOF, preventing a hang when payload segmentation
causes a short read on the server.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index b6e58d936ebe5..5e1ec33368590 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -638,7 +638,7 @@ static void connect_one_server(int fd, int pipefd)
>  
>  static void process_one_client(int fd, int pipefd)
>  {
> -	ssize_t ret, ret2, ret3;
> +	ssize_t ret, ret2, n, w;
>  	struct so_state s;
>  	char buf[4096];
>  
> @@ -648,23 +648,27 @@ static void process_one_client(int fd, int pipefd)
>  	ret = write(pipefd, "xmit", 4);
>  	assert(ret == 4);
>  
> -	ret = read(fd, buf, sizeof(buf));
> -	if (ret < 0)
> -		die_perror("read");
> +	ret = 0;
> +	ret2 = 0;
> +	while ((n = read(fd, buf, sizeof(buf))) > 0) {
> +		if (!ret) {
> +			assert(s.mptcpi_rcv_delta <= (uint64_t)n);
>  
> -	assert(s.mptcpi_rcv_delta <= (uint64_t)ret);
> -
> -	if (s.tcpi_rcv_delta)
> -		assert(s.tcpi_rcv_delta == (uint64_t)ret);
> +			if (s.tcpi_rcv_delta)
> +				assert(s.tcpi_rcv_delta <= (uint64_t)n);
> +		}
>  
> -	ret2 = write(fd, buf, ret);
> -	if (ret2 < 0)
> -		die_perror("write");
> +		w = write(fd, buf, n);
> +		if (w < 0)
> +			die_perror("write");
> +		if (w != n)
> +			xerror("short write");
>  
> -	/* wait for hangup */
> -	ret3 = read(fd, buf, 1);
> -	if (ret3 != 0)
> -		xerror("expected EOF, got %lu", ret3);
> +		ret += n;
> +		ret2 += w;
> +	}

[Severity: Medium]
When a partial read occurs in process_one_client(), it immediately echoes
the partial chunk n back to the client before reading and echoing the rest
of the payload in subsequent loop iterations.

If the client in connect_one_server() reads the echoed data across more
than one read() call, ret only holds the byte count of the final chunk
while total accumulates the full length:

tools/testing/selftests/net/mptcp/mptcp_sockopt.c:connect_one_server() {
	...
	total = 0;
	do {
		ret = read(fd, buf2 + total, sizeof(buf2) - total);
		if (ret < 0)
			die_perror("read");
		if (ret == 0) {
			eof = true;
			break;
		}

		total += ret;
	} while (total < len);
	...
	do_getsockopts(&s, fd, ret, ret);
}

Because connect_one_server() passes ret instead of total to
do_getsockopts(), won't connect_one_server() -> do_getsockopts() ->
do_getsockopt_mptcp_info() fail on:

	assert(s->mi.mptcpi_write_seq + w == i.mptcpi_write_seq);

and do_getsockopt_tcp_info() fail when checking tcpi_bytes_sent and
tcpi_bytes_received against r and w whenever the echoed response is read in
multiple chunks? Should connect_one_server() pass total to do_getsockopts()
as well?

> +	if (n < 0)
> +		die_perror("read");
>  
>  	do_getsockopts(&s, fd, ret, ret2);
>  	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-fix-mptcp-issue-639-v1-1-0554d07ab8f8@proton.me?part=1

Re: [PATCH mptcp-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: the segmented-payload case still fails.
- [Medium] The commit message describes a failure the old code could not
  produce.

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but the xerror()
  calls in process_one_client() pass `ssize_t` values (`ret + 1`,…

--

selftests: mptcp: sockopt: echo until EOF on partial reads

This commit changes process_one_client() in the mptcp_sockopt selftest.
The old code did a single read() and write(), then a one-byte read to wait
for hangup. The new code loops on read()/write() until EOF and passes the
accumulated byte counts to the existing sockopt checks.

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

> process_one_client() used a single read() and write() before waiting
> for hangup. A SOCK_STREAM read() can return a short count when the
> payload is segmented, so the server echoed only part of the data and
> then blocked on FIN.
>
> The client keeps reading until it has the full length and only then
> closes the socket. The two sides wait on each other and the test
> hangs until SIGALRM.

[Severity: Medium]
Is this description of the old behavior accurate?

In the old process_one_client(), the server did this after a short first
read and the partial echo:

	/* wait for hangup */
	ret3 = read(fd, buf, 1);
	if (ret3 != 0)
		xerror("expected EOF, got %lu", ret3);

By then connect_one_server() has already written all len bytes, since it
calls xerror() on a short write. So the remaining bytes reach the server,
read(fd, buf, 1) returns 1, and the server exits through xerror().

The server is a separate forked process. When it exits, its socket is
closed and the client's blocked read() returns. That is a quick failure,
not both sides waiting on each other until the alarm fires.

The old read(fd, buf, 1) could only block forever if the rest of the data
never arrived. In that case the new read() loop would block in the same
way.

If issue #639 really shows a timeout, is the root cause somewhere else?
If not, could the commit message describe the failure as the "expected
EOF" error instead of a hang?

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index b6e58d936ebe5..5e1ec33368590 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c

[ ... ]

> @@ -648,23 +648,27 @@ static void process_one_client(int fd, int pipefd)
>  	ret = write(pipefd, "xmit", 4);
>  	assert(ret == 4);
>  
> -	ret = read(fd, buf, sizeof(buf));
> -	if (ret < 0)
> -		die_perror("read");
> +	ret = 0;
> +	ret2 = 0;
> +	while ((n = read(fd, buf, sizeof(buf))) > 0) {
> +		if (!ret) {
> +			assert(s.mptcpi_rcv_delta <= (uint64_t)n);
>  
> -	assert(s.mptcpi_rcv_delta <= (uint64_t)ret);
> -
> -	if (s.tcpi_rcv_delta)
> -		assert(s.tcpi_rcv_delta == (uint64_t)ret);
> +			if (s.tcpi_rcv_delta)
> +				assert(s.tcpi_rcv_delta <= (uint64_t)n);
> +		}
>  
> -	ret2 = write(fd, buf, ret);
> -	if (ret2 < 0)
> -		die_perror("write");
> +		w = write(fd, buf, n);
> +		if (w < 0)
> +			die_perror("write");
> +		if (w != n)
> +			xerror("short write");

[Severity: High]
When the payload is segmented, the server now sends the echo as several
write() calls. Can the client still fail in connect_one_server() in that
case?

The client read loop adds up total, but then passes ret, which holds only
the size of the last read() chunk:

	total = 0;
	do {
		ret = read(fd, buf2 + total, sizeof(buf2) - total);
		...
		total += ret;
	} while (total < len);
	...
	do_getsockopts(&s, fd, ret, ret);

For a plain read(), mptcp_recvmsg() uses a sock_rcvlowat() target of 1.
So if the echo arrives as n1 and then n2, the client can return n1
without waiting for n2. That leaves ret equal to n2, which is less than
len.

do_getsockopt_mptcp_info() then checks:

	assert(s->mi.mptcpi_write_seq + w == i.mptcpi_write_seq);

Here w is n2 but write_seq has advanced by len, so the client aborts.

Without that assert, do_getsockopt_tcp_info() would still fail. It
compares tcpi_bytes_sent and tcpi_bytes_received (both len) against n2,
never matches, and ends in xerror() after the retries.

Before this patch the echo was a single write(), so ret == len usually
held. Should connect_one_server() pass total (or len) to do_getsockopts()
as part of this fix?

>  
> -	/* wait for hangup */
> -	ret3 = read(fd, buf, 1);
> -	if (ret3 != 0)
> -		xerror("expected EOF, got %lu", ret3);
> +		ret += n;
> +		ret2 += w;
> +	}
> +	if (n < 0)
> +		die_perror("read");
>  
>  	do_getsockopts(&s, fd, ret, ret2);
>  	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)

[Severity: Low]
This is a pre-existing issue, not one introduced by this patch. The
xerror() calls that follow pass ssize_t values to PRIu64 conversions:

		xerror("mptcpi_rcv_delta %" PRIu64 ", expect %" PRIu64 ", diff %" PRId64,
		       s.mptcpi_rcv_delta, ret + 1, s.mptcpi_rcv_delta - (ret + 1));

ret2 and ret are passed the same way in the mptcpi_bytes_sent,
mptcpi_bytes_received and mptcpi_bytes_acked messages.

On 32-bit builds ssize_t is 32 bits, so will these messages print
garbage values? The declaration of ret and ret2 is being rewritten
anyway. Could they be cast to uint64_t in these calls, or declared with a
64-bit type?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261017-fix-mptcp-issue-639-v1-1-0554d07ab8f8%40proton.me

Re: [PATCH mptcp-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[MPTCP CI]

Hi Quanye,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Critical: No conclusion ❓
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/39578925751

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


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-net] selftests: mptcp: sockopt: echo until EOF on partial reads

[Matthieu Baerts]

Hi Quanye,

On 17/10/2026 13:22, Quanye Yang via B4 Relay wrote:
> From: Quanye Yang <quanyeyang@proton.me>
> 
> process_one_client() used a single read() and write() before waiting
> for hangup. A SOCK_STREAM read() can return a short count when the
> payload is segmented, so the server echoed only part of the data and
> then blocked on FIN.
> 
> The client keeps reading until it has the full length and only then
> closes the socket. The two sides wait on each other and the test
> hangs until SIGALRM.
> 
> Loop the server read/write until read() returns 0, and accumulate the
> copied byte counts for the existing sockopt checks.

Thank you for the fix.

> Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")

Note, you can keep the Fixes tag, but target mptcp-next because I don't
think I saw the issue in our and the netdev CI, and we got asked to
delay "non-urgent" fixes like this one found by AI without user reports.
(Not that the fix is useless, but it is not urgent, and there are
already too many urgent ones ;) )

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/639
> Signed-off-by: Quanye Yang <quanyeyang@proton.me>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 34 +++++++++++++----------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index b6e58d936ebe..5e1ec3336859 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -638,7 +638,7 @@ static void connect_one_server(int fd, int pipefd)
>  
>  static void process_one_client(int fd, int pipefd)
>  {
> -	ssize_t ret, ret2, ret3;
> +	ssize_t ret, ret2, n, w;
>  	struct so_state s;
>  	char buf[4096];
>  
> @@ -648,23 +648,27 @@ static void process_one_client(int fd, int pipefd)
>  	ret = write(pipefd, "xmit", 4);
>  	assert(ret == 4);
>  
> -	ret = read(fd, buf, sizeof(buf));
> -	if (ret < 0)
> -		die_perror("read");
> +	ret = 0;
> +	ret2 = 0;
> +	while ((n = read(fd, buf, sizeof(buf))) > 0) {
> +		if (!ret) {

Instead of 'ret' and 'ret2', maybe clearer to use 'read' and 'written'?

(Also, with an integer, it is clearer to use > 0 or != 0)

But regarding the modification: if the 'read' is partial, will we not
have an issue with the assert below?
Do we know in advance how much we are supposed to receive? If yes, we
can retry if "0 < n < expected"?

(Note: you can maybe avoid the big loop and use a label + goto?)

> +			assert(s.mptcpi_rcv_delta <= (uint64_t)n);
>  
> -	assert(s.mptcpi_rcv_delta <= (uint64_t)ret);
> -
> -	if (s.tcpi_rcv_delta)
> -		assert(s.tcpi_rcv_delta == (uint64_t)ret);
> +			if (s.tcpi_rcv_delta)
> +				assert(s.tcpi_rcv_delta <= (uint64_t)n);
> +		}
>  
> -	ret2 = write(fd, buf, ret);
> -	if (ret2 < 0)
> -		die_perror("write");
> +		w = write(fd, buf, n);
> +		if (w < 0)
> +			die_perror("write");
> +		if (w != n)
> +			xerror("short write");

Same here: if we fail in this case, then the modification doesn't seem
useful, no?
Instead, could we try to write again? (goto)

>  
> -	/* wait for hangup */
> -	ret3 = read(fd, buf, 1);

Without the big loop, we can keep this explicit "wait for hangup" which
is perhaps clearer.

WDYT?

> -	if (ret3 != 0)
> -		xerror("expected EOF, got %lu", ret3);
> +		ret += n;
> +		ret2 += w;
> +	}
> +	if (n < 0)
> +		die_perror("read");
>  
>  	do_getsockopts(&s, fd, ret, ret2);
>  	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)
> 
> ---
> base-commit: a22035be824449c4921ec406f6383089016a628c
> change-id: 20261017-fix-mptcp-issue-639-96339a3d921a
> 
> Best regards,
> --  
> Quanye Yang <quanyeyang@proton.me>

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