Re: [PATCH mptcp-next v8 6/6] selftests: mptcp: join: validate subflow retry after link failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: mptcp: blocking FIFO stdin stalls server in copyfd_io_poll()
--

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

selftests: mptcp: join: validate subflow retry after link failure

This commit adds two selftests in mptcp_join.sh to validate subflow retries
after packet drops using netfilter rules. The first test verifies that
additional and initial subflows reconnect after silent packet drops, and the
second test verifies closing the client connection while a subflow is being
re-created.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c3ccce96b45b1..58a48f50ba152 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
[ ... ]
> @@ -1038,7 +1039,9 @@ do_transfer()
>  	fi
>  
>  	extra_srv_args="$extra_args $extra_srv_args"
> -	if [ "$test_linkfail" -gt 1 ];then
> +	if [ -n "${server_in}" ]; then
> +		listener_in="${server_in}"
> +	elif [ "$test_linkfail" -gt 1 ]; then
>  		listener_in="${sinfail}"
>  	fi
>  	ip netns exec ${listener_ns} \
[ ... ]
> @@ -2585,6 +2769,69 @@ link_failure_tests()
[ ... ]
> +	# close the connection while a subflow is being re-created
> +	if reset_with_events "subflow retry, close while connecting" &&
> +	   continue_if has_subflow_retries && init_nft_filter; then
> +		sf_retry_init
> +		# The server sends nothing, but does not close its side: the
> +		# client has nothing to read, and the connection stays
> +		# established.
> +		local srv_in srv_in_pid
> +		srv_in=$(mktemp -u)
> +		mkfifo "${srv_in}"
> +		sleep 600 > "${srv_in}" &
> +		srv_in_pid=$!
> +		{ timeout_test=600 test_linkfail=131072 server_in="${srv_in}" \
> +			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null

[Severity: Medium]
Does redirecting the server's stdin from the srv_in FIFO cause the server
mptcp_connect process to block in read() and stop draining incoming data from
the client?

In link_failure_tests() -> run_tests() -> do_transfer(), the server is started
in poll mode (-j / CFG_MODE_POLL) with stdin redirected from listener_in
(srv_in), while sleep 600 holds the write end of the FIFO open without writing
any bytes.

In tools/testing/selftests/net/mptcp/mptcp_connect.c:copyfd_io_poll(), only
peerfd is set to non-blocking mode and passed to poll(), while infd (stdin)
remains blocking:

	set_nonblock(peerfd, true);

	for (;;) {
		...
		switch (poll(&fds, 1, poll_timeout)) {
		...
		}

		if (fds.revents & POLLIN) {
			...
		}

		if (fds.revents & POLLOUT) {
			if (winfo->len == 0) {
				winfo->off = 0;
				winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
			}

Because peerfd has POLLOUT set on the first poll() iteration after accept(),
the server calls read(infd, ...) on the FIFO and blocks for 600 seconds. As a
result, the server never returns to poll() and never reads from peerfd again.

Once the client fills the initial ~64 KB receive window, the server advertises
a zero receive window and stops acknowledging new data. If wait_counter_inc
for MPTcpExtMPJoinAckRx and nft_drop_from 10.0.3.2 run after that initial
window has been sent and acknowledged, subflow 10.0.3.2 has no unacknowledged
data in flight (and zero-window probes are only sent on the initial subflow
10.0.1.2), so subflow 10.0.3.2 never times out or gets re-created, causing
wait_counter_inc on MPTcpExtMPJoinSynTx to time out.

[ ... ]

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