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

[sashiko-bot@kernel.org]

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