Re: [PATCH mptcp-net 19/34] mptcp: do not fallback while old data is in flight

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] mptcp: infinite map emission stall in
  mptcp_pm_mp_fail_received() [1]
--

commit 4c9a52a9f4dc2474ec08fa066289f109db0a1859
Author: Chris Mason <mason@kernel.org>

mptcp: do not fallback while old data is in flight

This commit prevents an MPTCP connection from falling back to plain TCP
while a zero window probe or reinjected data is still in flight on the
subflow, avoiding duplicate data delivery on the receiver.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8f954a752797d..7329deb43c622 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -1582,7 +1594,10 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
>  out:
>  	if (READ_ONCE(msk->csum_enabled))
>  		mptcp_update_data_checksum(skb, copy);
> -	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
> +
> +	/* the infinite map implies a fallback: wait for old data acks */
> +	if (mptcp_subflow_ctx(ssk)->send_infinite_map &&
> +	    !mptcp_subflow_ctx(ssk)->old_data_pending)
>  		mptcp_update_infinite_map(msk, ssk, mpext);

[Severity: High]
This problem wasn't introduced by this patch, but can subflow->send_infinite_map
remain pending forever [1] when an MP_FAIL arrives while the connection
has unacknowledged data and no unsent data left on the send queue?

mptcp_pm_mp_fail_received() sets subflow->send_infinite_map = 1 and
msk->allow_subflows = false:

net/mptcp/pm.c:mptcp_pm_mp_fail_received() {
	...
	msk->allow_subflows = false;
	spin_unlock_bh(&msk->fallback_lock);

	if (!subflow->fail_tout) {
		pr_debug("send MP_FAIL response and infinite map\n");

		subflow->send_mp_fail = 1;
		subflow->send_infinite_map = 1;
		tcp_send_ack(sk);
	}
	...
}

Attaching the infinite map in mptcp_sendmsg_frag() requires sending a data
skb.  When there is no unsent data, mptcp_send_head(sk) is NULL so
__mptcp_subflow_push_pending() does not call mptcp_sendmsg_frag().  Meanwhile,
the MPTCP retransmission timer cannot retransmit the unacknowledged data to
carry the infinite map because __mptcp_push_retrans() aborts when
!msk->allow_subflows:

net/mptcp/protocol.c:__mptcp_push_retrans() {
	...
			spin_lock_bh(&msk->fallback_lock);
			if (__mptcp_check_fallback(msk) ||
			    !msk->allow_subflows) {
				spin_unlock_bh(&msk->fallback_lock);
				release_sock(ssk);
				return -1;
			}
	...
}

As a result, the infinite map is never transmitted and the connection stalls
indefinitely.

[1] https://sashiko.dev/#/bug/linux-906f0960-8875-45cb-8066-cc9808d83b26

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-mptcp-cmason-fixes-v1-0-9205b77e9a2f@kernel.org?part=19