Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

11 Nov 2025 02:59:28 Geliang Tang <geliang@kernel.org>:

> Hi Paolo,
>
> Thanks for this v2.
>
> On Fri, 2025-11-07 at 08:23 +0100, Paolo Abeni wrote:
>> The CI reports sporadic failures of the fastclose self-tests. The
>> root
>> cause is a duplicate reset, not carrying the relevant MPTCP option.
>> In the failing scenario the bad reset is received by the peer before
>> the fastclose one, preventing the reception of the latter.
>>
>> Indeed there is window of opportunity at fastclose time for the
>> following
>> race:
>>
>> mptcp_do_fastclose
>>   __mptcp_close_ssk
>>     __tcp_close()
>>       tcp_set_state() [1]
>>       tcp_send_active_reset() [2]
>>
>> After [1] the stack will send reset to in-flight data reaching the
>> now
>> closed port. Such reset may race with [2].
>>
>> Address the issue explicitly sending a single reset on fastclose
>> before
>> explicitly moving the subflow to close status.
>>
>> Fixes: d21f83485518 ("mptcp: use fastclose on more edge scenarios");
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/596
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> v1 -> v2:
>>  - test subflow->send_fastclose in __mptcp_subflow_disconnect()
>> instead
>>    of MPTCP_CF_FASTCLOSE
>> ---
>>  net/mptcp/protocol.c | 37 +++++++++++++++++++++++--------------
>>  1 file changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index 0301e0b0de05..24d4fa8227b7 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -2437,7 +2437,6 @@ bool __mptcp_retransmit_pending_data(struct
>> sock *sk)
>>  
>>  /* flags for __mptcp_close_ssk() */
>>  #define MPTCP_CF_PUSH      BIT(1)
>> -#define MPTCP_CF_FASTCLOSE BIT(2)
>>  
>>  /* be sure to send a reset only if the caller asked for it, also
>>   * clean completely the subflow status when the subflow reaches
>> @@ -2448,7 +2447,7 @@ static void __mptcp_subflow_disconnect(struct
>> sock *ssk,
>>                        unsigned int flags)
>
> nit:
>
> The 3rd argument "flags" of __mptcp_subflow_disconnect is useless now.
> We can drop it.

I don't think it is useless: __mptcp_close_ssk() is still called with either
the push flag or no flag (0).

Because this patch is for net, we should avoid unnecessary modifications:
we should not change the type or the name of the function argument for
"cosmetic" reasons.

>
> No need to send v3, Matt or I can handle it.
>
> Thanks,
> -Geliang
>
>>  {
>>     if (((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
>> -       (flags & MPTCP_CF_FASTCLOSE)) {
>> +       subflow->send_fastclose) {
>>         /* The MPTCP code never wait on the subflow sockets,
>> TCP-level
>>          * disconnect should never fail
>>          */
>> @@ -2511,20 +2510,13 @@ static void __mptcp_close_ssk(struct sock
>> *sk, struct sock *ssk,
>>     if (dispose_it)
>>         list_del(&subflow->node);
>>  
>> -   if ((flags & MPTCP_CF_FASTCLOSE) &&
>> !__mptcp_check_fallback(msk)) {
>> -       /* be sure to force the tcp_close path
>> -        * to generate the egress reset
>> -        */
>> -       ssk->sk_lingertime = 0;
>> -       sock_set_flag(ssk, SOCK_LINGER);
>> -       subflow->send_fastclose = 1;
>> -   }
>> +   if (subflow->send_fastclose && ssk->sk_state != TCP_CLOSE)
>> +       tcp_set_state(ssk, TCP_CLOSE);
>>  
>>     need_push = (flags & MPTCP_CF_PUSH) &&
>> __mptcp_retransmit_pending_data(sk);
>>     if (!dispose_it) {
>>         __mptcp_subflow_disconnect(ssk, subflow, flags);
>>         release_sock(ssk);
>> -

(This line should not be removed but this can be fixed when applying the patch.)

Cheers,
Matt