Re: [PATCH resend mptcp-net 4/4] mptcp: do not reschedule the RTX timer for fallback sockets

On 8/14/26 4:03 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The MPTCP_RTX_DISABLED flag makes the TCP_CLOSE state sticky for the RTX timer, permanently disabling retransmissions on reused connections and their accepted children.
> --
> 
> --- Patch [4]: [PATCH resend mptcp-net 4/4] mptcp: do not reschedule the RTX timer for fallback sockets ---
> commit 227375cb381dd3864273cd7170dd28e0f079ff8b
> Author: Paolo Abeni <[email protected]>
> 
> mptcp: do not reschedule the RTX timer for fallback sockets
>     
> This patch introduces the MPTCP_RTX_DISABLED flag to prevent the RTX timer
> from being rescheduled on fallback sockets and when the socket state is
> TCP_CLOSE.
> 
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index b311fe30c7855..f22d64ab1c53b 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
> [ ... ]
>> @@ -1121,13 +1122,14 @@ static bool mptcp_rtx_timer_pending(struct sock *sk)
>>  
>>  static void mptcp_reset_rtx_timer(struct sock *sk)
>>  {
>> +	struct mptcp_sock *msk = mptcp_sk(sk);
>>  	unsigned long tout;
>>  
>> -	/* prevent rescheduling on close */
>> -	if (unlikely(inet_sk_state_load(sk) == TCP_CLOSE))
>> +	/* Prevent rescheduling on close and in case of fallback. */
>> +	if (test_bit(MPTCP_RTX_DISABLED, &msk->flags))
>>  		return;
> 
> [Severity: High]
> Since this flag is checked here to prevent rescheduling, will the timer be
> permanently disabled if the flag is never cleared?
> 
> [ ... ]
>> @@ -3360,6 +3362,9 @@ void mptcp_set_state(struct sock *sk, int state)
>>  		 * transition from TCP_SYN_RECV to TCP_CLOSE_WAIT.
>>  		 */
>>  		break;
>> +	case TCP_CLOSE:
>> +		set_bit(MPTCP_RTX_DISABLED, &mptcp_sk(sk)->flags);
>> +		fallthrough;
>>  	default:
> 
> [Severity: High]
> Does setting MPTCP_RTX_DISABLED here without a corresponding clear logic make
> the TCP_CLOSE state sticky for the RTX timer? 
> 
> If an MPTCP socket transitions to TCP_CLOSE (like a failed connect() attempt
> or an explicit disconnect()), and the application then reuses the socket to 
> connect() again, it appears the flag will remain set.
> 
> Furthermore, if listen() is called on such a reused socket, would
> sk_clone_lock() copy the flags to all child sockets? This could cause every
> accepted connection to inherit the MPTCP_RTX_DISABLED flag, permanently
> breaking MPTCP-level retransmissions for all clients.

On nice, I did not read an hallucination this allucinated since a while...

without disconnect() a closed socket can not transition to other non
closed states, because the network-level ops check the 'struct socket'
state, which in turn is cleared only by disconnect().

mptcp_disconnect() explicitly clears msk->flags.

No real issues above.

/P