* [PATCH v2 mptcp-net 1/2] mptcp: decouple mptcp fastclose from tcp close
2025-11-07 7:23 [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset Paolo Abeni
@ 2025-11-07 7:23 ` Paolo Abeni
2025-11-11 11:24 ` Matthieu Baerts
2025-11-07 7:23 ` [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose Paolo Abeni
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paolo Abeni @ 2025-11-07 7:23 UTC (permalink / raw)
To: mptcp; +Cc: geliang
With the current fastclose implementation, the mptcp_do_fastclose() helper
is in charge of two distinct actions: send the fastclose reset and cleanup
the subflows.
Formally decouple the two steps, ensuring that mptcp explicitly closes all
the subflows after the mentioned helper.
This will make the upcoming fix simpler, and allows dropping the 2nd
argument from mptcp_destroy_common().
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1506fde9a6da..0301e0b0de05 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2884,7 +2884,11 @@ static void mptcp_worker(struct work_struct *work)
__mptcp_close_subflow(sk);
if (mptcp_close_tout_expired(sk)) {
+ struct mptcp_subflow_context *subflow, *tmp;
+
mptcp_do_fastclose(sk);
+ mptcp_for_each_subflow_safe(msk, subflow, tmp)
+ __mptcp_close_ssk(sk, subflow->tcp_sock, subflow, 0);
mptcp_close_wake_up(sk);
}
@@ -3289,7 +3293,7 @@ static void mptcp_copy_inaddrs(struct sock *msk, const struct sock *ssk)
inet_sk(msk)->inet_rcv_saddr = inet_sk(ssk)->inet_rcv_saddr;
}
-static void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)
+static void mptcp_destroy_common(struct mptcp_sock *msk)
{
struct mptcp_subflow_context *subflow, *tmp;
struct sock *sk = (struct sock *)msk;
@@ -3299,7 +3303,7 @@ static void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)
/* join list will be eventually flushed (with rst) at sock lock release time */
mptcp_for_each_subflow_safe(msk, subflow, tmp)
- __mptcp_close_ssk(sk, mptcp_subflow_tcp_sock(subflow), subflow, flags);
+ __mptcp_close_ssk(sk, mptcp_subflow_tcp_sock(subflow), subflow, 0);
__skb_queue_purge(&sk->sk_receive_queue);
skb_rbtree_purge(&msk->out_of_order_queue);
@@ -3333,7 +3337,8 @@ static int mptcp_disconnect(struct sock *sk, int flags)
/* msk->subflow is still intact, the following will not free the first
* subflow
*/
- mptcp_destroy_common(msk, MPTCP_CF_FASTCLOSE);
+ mptcp_do_fastclose(sk);
+ mptcp_destroy_common(msk);
/* The first subflow is already in TCP_CLOSE status, the following
* can't overlap with a fallback anymore
@@ -3521,7 +3526,7 @@ static void mptcp_destroy(struct sock *sk)
/* allow the following to close even the initial subflow */
msk->free_first = 1;
- mptcp_destroy_common(msk, 0);
+ mptcp_destroy_common(msk);
sk_sockets_allocated_dec(sk);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 1/2] mptcp: decouple mptcp fastclose from tcp close
2025-11-07 7:23 ` [PATCH v2 mptcp-net 1/2] mptcp: decouple mptcp fastclose from tcp close Paolo Abeni
@ 2025-11-11 11:24 ` Matthieu Baerts
0 siblings, 0 replies; 12+ messages in thread
From: Matthieu Baerts @ 2025-11-11 11:24 UTC (permalink / raw)
To: Paolo Abeni, mptcp; +Cc: geliang
Hi Paolo,
On 07/11/2025 08:23, Paolo Abeni wrote:
> With the current fastclose implementation, the mptcp_do_fastclose() helper
> is in charge of two distinct actions: send the fastclose reset and cleanup
> the subflows.
>
> Formally decouple the two steps, ensuring that mptcp explicitly closes all
> the subflows after the mentioned helper.
>
> This will make the upcoming fix simpler, and allows dropping the 2nd
> argument from mptcp_destroy_common().
I suggest adding the same Fixes tag here, just to ease the backports (to
ensure this patch is automatically picked up):
Fixes: d21f83485518 ("mptcp: use fastclose on more edge scenarios")
I'm going to add the following text to the previous paragraph:
The Fixes tag is the same as in the next commit, simply to ease the
backports.
Thank you for the patch!
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-07 7:23 [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset Paolo Abeni
2025-11-07 7:23 ` [PATCH v2 mptcp-net 1/2] mptcp: decouple mptcp fastclose from tcp close Paolo Abeni
@ 2025-11-07 7:23 ` Paolo Abeni
2025-11-11 1:59 ` Geliang Tang
2025-11-11 11:24 ` Matthieu Baerts
2025-11-07 8:30 ` [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset MPTCP CI
` (2 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-11-07 7:23 UTC (permalink / raw)
To: mptcp; +Cc: geliang
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)
{
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);
-
goto out;
}
@@ -2855,9 +2847,26 @@ static void mptcp_do_fastclose(struct sock *sk)
mptcp_set_state(sk, TCP_CLOSE);
mptcp_backlog_purge(sk);
- mptcp_for_each_subflow_safe(msk, subflow, tmp)
- __mptcp_close_ssk(sk, mptcp_subflow_tcp_sock(subflow),
- subflow, MPTCP_CF_FASTCLOSE);
+
+ /* Explicitly send the fastclose reset as need */
+ if (__mptcp_check_fallback(msk))
+ return;
+
+ mptcp_for_each_subflow_safe(msk, subflow, tmp) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ lock_sock(ssk);
+
+ /* Some subflow socket states don't allow/need a reset.*/
+ if ((1 << ssk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))
+ goto unlock;
+
+ subflow->send_fastclose = 1;
+ tcp_send_active_reset(ssk, ssk->sk_allocation,
+ SK_RST_REASON_TCP_ABORT_ON_CLOSE);
+unlock:
+ release_sock(ssk);
+ }
}
static void mptcp_worker(struct work_struct *work)
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-07 7:23 ` [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose Paolo Abeni
@ 2025-11-11 1:59 ` Geliang Tang
2025-11-11 6:14 ` Matthieu Baerts
2025-11-11 11:24 ` Matthieu Baerts
1 sibling, 1 reply; 12+ messages in thread
From: Geliang Tang @ 2025-11-11 1:59 UTC (permalink / raw)
To: Paolo Abeni, mptcp
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.
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);
> -
> goto out;
> }
>
> @@ -2855,9 +2847,26 @@ static void mptcp_do_fastclose(struct sock
> *sk)
>
> mptcp_set_state(sk, TCP_CLOSE);
> mptcp_backlog_purge(sk);
> - mptcp_for_each_subflow_safe(msk, subflow, tmp)
> - __mptcp_close_ssk(sk,
> mptcp_subflow_tcp_sock(subflow),
> - subflow, MPTCP_CF_FASTCLOSE);
> +
> + /* Explicitly send the fastclose reset as need */
> + if (__mptcp_check_fallback(msk))
> + return;
> +
> + mptcp_for_each_subflow_safe(msk, subflow, tmp) {
> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +
> + lock_sock(ssk);
> +
> + /* Some subflow socket states don't allow/need a
> reset.*/
> + if ((1 << ssk->sk_state) & (TCPF_LISTEN |
> TCPF_CLOSE))
> + goto unlock;
> +
> + subflow->send_fastclose = 1;
> + tcp_send_active_reset(ssk, ssk->sk_allocation,
> +
> SK_RST_REASON_TCP_ABORT_ON_CLOSE);
> +unlock:
> + release_sock(ssk);
> + }
> }
>
> static void mptcp_worker(struct work_struct *work)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-11 1:59 ` Geliang Tang
@ 2025-11-11 6:14 ` Matthieu Baerts
2025-11-11 7:27 ` Paolo Abeni
0 siblings, 1 reply; 12+ messages in thread
From: Matthieu Baerts @ 2025-11-11 6:14 UTC (permalink / raw)
To: Geliang Tang; +Cc: Paolo Abeni, mptcp
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-11 6:14 ` Matthieu Baerts
@ 2025-11-11 7:27 ` Paolo Abeni
2025-11-11 7:49 ` Geliang Tang
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Abeni @ 2025-11-11 7:27 UTC (permalink / raw)
To: Matthieu Baerts, Geliang Tang; +Cc: mptcp
On 11/11/25 7:14 AM, Matthieu Baerts wrote:
> 11 Nov 2025 02:59:28 Geliang Tang <geliang@kernel.org>:
>> On Fri, 2025-11-07 at 08:23 +0100, Paolo Abeni wrote:
>>> @@ -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.
FWIW, I agree with Mat.
>>> @@ -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.)
Thanks!
/P
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-11 7:27 ` Paolo Abeni
@ 2025-11-11 7:49 ` Geliang Tang
0 siblings, 0 replies; 12+ messages in thread
From: Geliang Tang @ 2025-11-11 7:49 UTC (permalink / raw)
To: Paolo Abeni, Matthieu Baerts; +Cc: mptcp
On Tue, 2025-11-11 at 08:27 +0100, Paolo Abeni wrote:
> On 11/11/25 7:14 AM, Matthieu Baerts wrote:
> > 11 Nov 2025 02:59:28 Geliang Tang <geliang@kernel.org>:
> > > On Fri, 2025-11-07 at 08:23 +0100, Paolo Abeni wrote:
> > > > @@ -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.
>
> FWIW, I agree with Mat.
I agree too. Let's apply it as is.
Thanks,
-Geliang
>
> > > > @@ -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.)
>
> Thanks!
>
> /P
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose
2025-11-07 7:23 ` [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose Paolo Abeni
2025-11-11 1:59 ` Geliang Tang
@ 2025-11-11 11:24 ` Matthieu Baerts
1 sibling, 0 replies; 12+ messages in thread
From: Matthieu Baerts @ 2025-11-11 11:24 UTC (permalink / raw)
To: Paolo Abeni, mptcp; +Cc: geliang
Hi Paolo,
On 07/11/2025 08:23, 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.
Thank you for the proper fix!
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Fixes: d21f83485518 ("mptcp: use fastclose on more edge scenarios");
When applying this patch, I will remove the extra ';' at the end of this
line, plus not removing a line below.
I will also revert "selftests: mptcp: join: fastclose: drop plain RST"
and only keep the removal of the two 'MPTCP_LIB_SUBTEST_FLAKY=1' lines.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset
2025-11-07 7:23 [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset Paolo Abeni
2025-11-07 7:23 ` [PATCH v2 mptcp-net 1/2] mptcp: decouple mptcp fastclose from tcp close Paolo Abeni
2025-11-07 7:23 ` [PATCH v2 mptcp-net 2/2] mptcp: fix duplicate reset on fastclose Paolo Abeni
@ 2025-11-07 8:30 ` MPTCP CI
2025-11-11 2:31 ` Geliang Tang
2025-11-11 15:54 ` Matthieu Baerts
4 siblings, 0 replies; 12+ messages in thread
From: MPTCP CI @ 2025-11-07 8:30 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
Hi Paolo,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Unstable: 1 failed test(s): packetdrill_mp_capable 🔴
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/19161574021
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/92f738c5d9cf
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1020769
If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:
$ cd [kernel source code]
$ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
--pull always mptcp/mptcp-upstream-virtme-docker:latest \
auto-normal
For more details:
https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)
Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset
2025-11-07 7:23 [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset Paolo Abeni
` (2 preceding siblings ...)
2025-11-07 8:30 ` [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset MPTCP CI
@ 2025-11-11 2:31 ` Geliang Tang
2025-11-11 15:54 ` Matthieu Baerts
4 siblings, 0 replies; 12+ messages in thread
From: Geliang Tang @ 2025-11-11 2:31 UTC (permalink / raw)
To: Paolo Abeni, mptcp
Hi Paolo,
On Fri, 2025-11-07 at 08:23 +0100, Paolo Abeni wrote:
> The actual fix is in patch 2/2, while patch 1/2 is there just to make
> the change tidy/more easily reviewable.
> ---
> v1 -> v2:
> - drop FASTCLOSE flag entirely
Apart from a nit in patch 2, this set looks good to me.
Reviewed-by: Geliang Tang <geliang@kernel.org>
Note: As discussed in v1, with this set, "selftests: mptcp: join:
fastclose: drop plain RST" should be reverted.
Thanks,
-Geliang
>
> Paolo Abeni (2):
> mptcp: decouple mptcp fastclose from tcp close
> mptcp: fix duplicate reset on fastclose
>
> net/mptcp/protocol.c | 50 ++++++++++++++++++++++++++++--------------
> --
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset
2025-11-07 7:23 [PATCH v2 mptcp-net 0/2] mptcp: fix duplicate reset Paolo Abeni
` (3 preceding siblings ...)
2025-11-11 2:31 ` Geliang Tang
@ 2025-11-11 15:54 ` Matthieu Baerts
4 siblings, 0 replies; 12+ messages in thread
From: Matthieu Baerts @ 2025-11-11 15:54 UTC (permalink / raw)
To: Paolo Abeni, mptcp; +Cc: geliang
Hi Paolo, Geliang,
(I just noticed this email was unsent, the patches have been applied a
few hours ago)
On 07/11/2025 08:23, Paolo Abeni wrote:
> The actual fix is in patch 2/2, while patch 1/2 is there just to make
> the change tidy/more easily reviewable.
Thank you for the patches and the reviews! I had a few conflicts, but I
think I got it right.
Now in our tree (fixes for -net):
New patches for t/upstream-net and t/upstream:
- 408b267c6bd8: Revert "selftests: mptcp: join: fastclose: drop plain RST"
- 0ef8d2d40280: mptcp: decouple mptcp fastclose from tcp close (with
conflicts)
- 5413ee2c5a4b: mptcp: fix duplicate reset on fastclose (with conflicts)
- a5fd6152dbe2: conflict in t/mptcp-make-mptcp_destroy_common-static
- 69ec39f082d0: conflict in
t/mptcp-handle-first-subflow-closing-consistently
- 8a23d7a4757f: conflict in t/mptcp-introduce-mptcp-level-backlog
- e54a7311c74b: selftests: mptcp: join: fastclose: remove flaky marks
- Results: 0a46d88dd962..751a2432b7d8 (export-net)
- Results: 3ba6375d918e..247540d77d20 (export)
Tests are now in progress:
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/7b747795627aab975ffff54ee796ccd77f593292/checks
- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/3f20649f33461a575a29a4da06d29ec9c50058cb/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/cf56fe8fbd0dde4dcb53f37ba20d59ac543bbf74/checks
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 12+ messages in thread