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

[Matthieu Baerts <matttbe@kernel.org>]

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.