Re: [PATCH mptcp-net 2/2] Squash-to: "mptcp: do not reschedule the RTX timer for fallback sockets"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Non-blocking MPTCP Fastopen bypasses `mptcp_connect`, leaving the socket permanently in `TCP_CLOSE` and failing to enable the RTX timer.
--

commit 4679eada053d3ad7385facbcb9d12c98ccd146f2
Author: Paolo Abeni <pabeni@redhat.com>
Subject: Squash-to: "mptcp: do not reschedule the RTX timer for fallback sockets"

This commit replaces the MPTCP_RTX_DISABLED flag with MPTCP_RTX_ENABLED to
explicitly control the retransmit timer status across the socket lifecycle.
It sets the enabled flag during active connect and passive initialization.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 2f17c5daa704e..a28a0506d28da 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -4154,6 +4155,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  		return PTR_ERR(ssk);
>  
>  	mptcp_set_state(sk, TCP_SYN_SENT);
> +	set_bit(MPTCP_RTX_ENABLED, &msk->flags);

[Severity: High]
Does this change leave the RTX timer disabled during non-blocking Fastopen
connections?

When a non-blocking TCP Fast Open connection is initiated on an MPTCP socket
(using MSG_FASTOPEN and MSG_DONTWAIT), mptcp_sendmsg_fastopen() skips the call
to __inet_stream_connect(), which bypasses mptcp_connect() entirely:

net/mptcp/protocol.c:mptcp_sendmsg_fastopen() {
    ...
	if (ret == -EINPROGRESS && !(msg->msg_flags & MSG_DONTWAIT)) {
		ret = __inet_stream_connect(sk->sk_socket, msg->msg_name,
					    msg->msg_namelen, msg->msg_flags, 1);
    ...
}

Because mptcp_connect() is bypassed, MPTCP_RTX_ENABLED is never set, and the
socket state remains TCP_CLOSE instead of TCP_SYN_SENT.

This causes __mptcp_sync_state() to skip transitioning the socket to
TCP_ESTABLISHED:

net/mptcp/subflow.c:__mptcp_sync_state() {
    ...
	if (sk->sk_state == TCP_SYN_SENT) {
		/* subflow->idsn is always available is TCP_SYN_SENT state,
		 * even for the FASTOPEN scenarios
		 */
    ...
}

Will this cause non-blocking Fastopen connections to become permanently stuck
in TCP_CLOSE without an active retransmit timer if packet loss occurs?

>  	subflow = mptcp_subflow_ctx(ssk);
>  #ifdef CONFIG_TCP_MD5SIG
>  	/* no MPTCP if MD5SIG is enabled on this socket or we may run out of

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/f1e8ffe44c3756195391a25ecf60e56e02e0f34f.1787739122.git.pabeni@redhat.com?part=2