Re: [RFC PATCH mptcp-next v3 1/3] selftests: mptcp: connect: retry write() on EAGAIN

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: mptcp_connect fails on EAGAIN when poll_timeout is infinite
--

commit c4cb29705a0b8f55f650eb00f7f218f1708de879
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

selftests: mptcp: connect: retry write() on EAGAIN

mptcp_connect treats any write() error as fatal. But write() can fail
with EAGAIN even after poll() said the socket was writable: the MPTCP
send buffer shrinks when a subflow is closed.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..03fff859e4d1d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
[ ... ]
> @@ -711,6 +712,20 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>  
>  				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
>  				if (bw < 0) {
> +					/* POLLOUT does not guarantee that
> +					 * the next write will not block:
> +					 * the MPTCP send buffer can shrink
> +					 * in between, e.g. when a subflow
> +					 * is closed. Wait for more space,
> +					 * but not forever: poll() does not
> +					 * check every cause of EAGAIN.
> +					 */
> +					if (errno == EAGAIN &&
> +					    ++eagain_ms <= poll_timeout) {
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
Will this condition incorrectly abort the test immediately if poll_timeout
is set to -1 (infinite timeout)?

If the tool is invoked with a timeout of 0 or less (e.g., -t 0), poll_timeout
is set to -1. When write() returns EAGAIN, the new condition evaluates to
1 <= -1, which is false. This bypasses the intended retry logic, immediately
failing the transfer instead of waiting.

I noticed a similar issue in the preexisting code in xdisconnect():

tools/testing/selftests/net/mptcp/mptcp_connect.c:xdisconnect() {
	...
	for (i = 0; ; i += msec_sleep) {
		/* closed socket are not listed by 'ss' */
		if (system(cmd) != 0)
			break;

		if (i > poll_timeout)
			xerror("timeout while waiting for spool to complete");
		usleep(msec_sleep * 1000);
	}
	...
}

Does this code also incorrectly assume poll_timeout is positive, causing an
immediate crash if poll_timeout is -1?

> +						usleep(1000);
> +						continue;
> +					}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014143349.768138-1-anilkaushikwireless@gmail.com?part=1