All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
@ 2026-10-08 12:16 T S Rameshkumar
  2026-10-08 12:29 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: T S Rameshkumar @ 2026-10-08 12:16 UTC (permalink / raw)
  To: matttbe, mptcp; +Cc: petar.sakic, T S Rameshkumar

With TCP Fast Open on an MPTCP listener, if the server application
writes data while the passive subflow is still in SYN_RECV (after
consuming the client's SYN data but before the MP_CAPABLE third ACK
arrives), __mptcp_subflow_active() refuses transmission and the data
is queued into the msk write queue.

When the MPC third ACK arrives, the subflow transitions to
TCP_ESTABLISHED and is marked fully established in
check_fully_established(), but because the third ACK carries no
DSS data, the queued bytes remain stranded until the peer sends
more data.

Fix this in check_fully_established() by checking if the subflow
was doing passive TFO (subflow->is_mptfo). Under mptcp_data_lock(),
flush queued bytes via __mptcp_check_push() and clear is_mptfo.

Reported-by: Petar Sakic <petar.sakic@ink.fish>
Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
Assisted-by: Gemini 3.7
Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
---
 net/mptcp/options.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index ce0de02f5..b865ee6af 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 
 	mptcp_data_lock((struct sock *)msk);
 	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
+	/* Passive TFO: the application may have written data while the
+	 * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
+	 * it then and nothing else spools the msk write queue when the
+	 * MPC third ack (no DSS) arrives. Push it now.
+	 */
+	if (subflow->is_mptfo) {
+		subflow->is_mptfo = 0;
+		__mptcp_check_push((struct sock *)msk, ssk);
+	}
 	mptcp_data_unlock((struct sock *)msk);
 
 check_notify:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 12:16 [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
@ 2026-10-08 12:29 ` sashiko-bot
  2026-10-08 13:42 ` MPTCP CI
       [not found] ` <CADRhS2uk-P8TSmyiM=X5JQXHtzdm68vg6+vyBDr-UCf0EAu0HA@mail.gmail.com>
  2 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-08 12:29 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: mptcp

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/mptcp: data race in subflow bitfields causes list corruption
--

commit 91c44c97db7bbddaa206314ca428111bec1fb73f
Author: T S Rameshkumar <rameshsv06@gmail.com>

mptcp: push queued data on passive TFO subflows becoming established
    
With TCP Fast Open on an MPTCP listener, passive subflows that receive
data during SYN_RECV queue it into the msk write queue. This patch flushes
the queued bytes when the MPC third ACK arrives by checking if the subflow
was doing passive TFO.

Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/

> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>  
>  	mptcp_data_lock((struct sock *)msk);
>  	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
> +	/* Passive TFO: the application may have written data while the
> +	 * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
> +	 * it then and nothing else spools the msk write queue when the
> +	 * MPC third ack (no DSS) arrives. Push it now.
> +	 */
> +	if (subflow->is_mptfo) {
> +		subflow->is_mptfo = 0;

[Severity: High]
Could this bitfield modification race with mptcp_close_ssk() and corrupt
the conn_list linked list?

In check_fully_established(), the new modification to subflow->is_mptfo is
protected by mptcp_data_lock(msk) and lock_sock(ssk).

However, there are asynchronous paths (like mptcp_pm_nl_rm_subflow_received()
or mptcp_worker()) that call mptcp_close_ssk(). That function modifies the
subflow->close_event_done bitfield while holding only lock_sock(msk).

In struct mptcp_subflow_context, these two bitfields share the same memory
word:

        valid_csum_seen : 1,        /* at least one csum validated */
        is_mptfo : 1,               /* subflow is doing TFO */
        close_event_done : 1,       /* has done the post-closed part */
        mpc_drop : 1,               /* the MPC option has been dropped */

Since the lock_sock(msk) mutex does not overlap with the locks held in this
code block, concurrent execution could result in a read-modify-write data
race. 

If the close_event_done = true update is overwritten by is_mptfo = 0,
mptcp_close_ssk() could be re-executed on the same subflow later. This
would result in a double list_del(&subflow->node) inside __mptcp_close_ssk(),
corrupting the conn_list.

Is there a safe way to update is_mptfo without introducing a read-modify-write
race on the shared bitfield word?

> +		__mptcp_check_push((struct sock *)msk, ssk);
> +	}
>  	mptcp_data_unlock((struct sock *)msk);
>  
>  check_notify:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008121602.206507-1-rameshkumar.t@phytecembedded.in?part=1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
@ 2026-10-08 12:42 T S Rameshkumar
  2026-10-08 12:56 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: T S Rameshkumar @ 2026-10-08 12:42 UTC (permalink / raw)
  To: matttbe, mptcp; +Cc: petar.sakic, T S Rameshkumar

With TCP Fast Open on an MPTCP listener, if the server application
writes data while the passive subflow is still in SYN_RECV (after
consuming the client's SYN data but before the MP_CAPABLE third ACK
arrives), __mptcp_subflow_active() refuses transmission and the data
is queued into the msk write queue.

When the MPC third ACK arrives, the subflow transitions to
TCP_ESTABLISHED and is marked fully established in
check_fully_established(), but because the third ACK carries no
DSS data, the queued bytes remain stranded until the peer sends
more data.

Fix this in check_fully_established() by checking if the subflow
was doing passive TFO (subflow->is_mptfo). Under mptcp_data_lock(),
flush queued bytes via __mptcp_check_push() and clear is_mptfo.

Also move is_mptfo from the shared u32 bitfield to a standalone
bool in struct mptcp_subflow_context to avoid any read-modify-write
data races with concurrent bitfield modifications (e.g.
close_event_done in mptcp_close_ssk()).

Reported-by: Petar Sakic <petar.sakic@ink.fish>
Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
Assisted-by: Gemini 3.7
Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
---
 net/mptcp/fastopen.c | 2 +-
 net/mptcp/options.c  | 9 +++++++++
 net/mptcp/protocol.h | 4 ++--
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index f71775090..c11fc3c5c 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -29,7 +29,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
 	if (!skb)
 		return;
 
-	subflow->is_mptfo = 1;
+	subflow->is_mptfo = true;
 
 	/* dequeue the skb from sk receive queue */
 	__skb_unlink(skb, &ssk->sk_receive_queue);
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index ce0de02f5..7ea97b283 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 
 	mptcp_data_lock((struct sock *)msk);
 	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
+	/* Passive TFO: the application may have written data while the
+	 * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
+	 * it then and nothing else spools the msk write queue when the
+	 * MPC third ack (no DSS) arrives. Push it now.
+	 */
+	if (subflow->is_mptfo) {
+		subflow->is_mptfo = false;
+		__mptcp_check_push((struct sock *)msk, ssk);
+	}
 	mptcp_data_unlock((struct sock *)msk);
 
 check_notify:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0384d6a02..0e44de129 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -582,15 +582,15 @@ struct mptcp_subflow_context {
 		disposable : 1,	    /* ctx can be free at ulp release time */
 		closing : 1,	    /* must not pass rx data to msk anymore */
 		valid_csum_seen : 1,        /* at least one csum validated */
-		is_mptfo : 1,	    /* subflow is doing TFO */
 		close_event_done : 1,       /* has done the post-closed part */
 		mpc_drop : 1,	    /* the MPC option has been dropped in a rtx */
 		resetting : 1,	    /* subflow is resetting */
-		__unused : 8;
+		__unused : 9;
 	bool	data_avail;
 	bool	scheduled;
 	bool	pm_listener;	    /* a listener managed by the kernel PM? */
 	bool	fully_established;  /* path validated */
+	bool	is_mptfo;	    /* subflow is doing TFO */
 	u32	lent_mem_frag;
 	u32	remote_nonce;
 	u64	thmac;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 12:42 T S Rameshkumar
@ 2026-10-08 12:56 ` sashiko-bot
  2026-10-08 13:03 ` Matthieu Baerts
  2026-10-10  7:40 ` netdev-bot+sashiko
  2 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-08 12:56 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: mptcp

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mptcp: data race in mptcp_subflow_context bitfield on close_event_done
--

commit 5833b858d2930ce8da576be275d9bd667c3816cd
Author: T S Rameshkumar <rameshsv06@gmail.com>

mptcp: push queued data on passive TFO subflows becoming established

With TCP Fast Open on an MPTCP listener, if the server application
writes data while the passive subflow is still in SYN_RECV,
__mptcp_subflow_active() refuses transmission and the data is queued
into the msk write queue.

Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0384d6a023f9d..0e44de1298007 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -582,15 +582,15 @@ struct mptcp_subflow_context {
>  		disposable : 1,	    /* ctx can be free at ulp release time */
>  		closing : 1,	    /* must not pass rx data to msk anymore */
>  		valid_csum_seen : 1,        /* at least one csum validated */
> -		is_mptfo : 1,	    /* subflow is doing TFO */
>  		close_event_done : 1,       /* has done the post-closed part */

[Severity: High]
Does leaving close_event_done in this bitfield still result in a data race?

In net/mptcp/protocol.c:mptcp_close_ssk(), subflow->close_event_done is set
to true while holding only the msk lock.

Concurrently, the TCP RX path calls subflow_data_ready() ->
mptcp_subflow_data_available() -> subflow_check_data_avail() in
net/mptcp/subflow.c, which sets subflow->map_valid = 1 holding only the
ssk (subflow) lock.

Since close_event_done and map_valid are part of the same u32 bitfield,
could these uncoordinated read-modify-write operations cause concurrent
updates to be silently lost, potentially dropping valid data or executing
mptcp_close_ssk() multiple times?

>  		mpc_drop : 1,	    /* the MPC option has been dropped in a rtx */
>  		resetting : 1,	    /* subflow is resetting */
> -		__unused : 8;
> +		__unused : 9;
>  	bool	data_avail;
>  	bool	scheduled;
>  	bool	pm_listener;	    /* a listener managed by the kernel PM? */
>  	bool	fully_established;  /* path validated */
> +	bool	is_mptfo;	    /* subflow is doing TFO */
>  	u32	lent_mem_frag;
>  	u32	remote_nonce;
>  	u64	thmac;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008124220.213016-1-rameshkumar.t@phytecembedded.in?part=1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 12:42 T S Rameshkumar
  2026-10-08 12:56 ` sashiko-bot
@ 2026-10-08 13:03 ` Matthieu Baerts
       [not found]   ` <PN0PR01MB9382C18780E137E3013A097CF4932@PN0PR01MB9382.INDPRD01.PROD.OUTLOOK.COM>
  2026-10-10  7:40 ` netdev-bot+sashiko
  2 siblings, 1 reply; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-08 13:03 UTC (permalink / raw)
  To: T S Rameshkumar, mptcp; +Cc: petar.sakic, T S Rameshkumar

Hi T S,

Thank you for the patch, but *please* stop sending multiple version the
same day without waiting for reviews. The patch traffic is already high:
if everybody sends so many versions like that, we (human) cannot deal
with that, plus it is unlikely the patch can be tested properly. Also,
please increment the patch version and include a changelog ("b4" can
help with that).

  https://docs.kernel.org/process/maintainer-netdev.html

Also, if you send your patch to the MPTCP list only, please base your
patch on top of our "export" branch: this version doesn't apply on top
of it.

On 08/10/2026 14:42, T S Rameshkumar wrote:
> With TCP Fast Open on an MPTCP listener, if the server application
> writes data while the passive subflow is still in SYN_RECV (after
> consuming the client's SYN data but before the MP_CAPABLE third ACK
> arrives), __mptcp_subflow_active() refuses transmission and the data
> is queued into the msk write queue.
> 
> When the MPC third ACK arrives, the subflow transitions to
> TCP_ESTABLISHED and is marked fully established in
> check_fully_established(), but because the third ACK carries no
> DSS data, the queued bytes remain stranded until the peer sends
> more data.
> 
> Fix this in check_fully_established() by checking if the subflow
> was doing passive TFO (subflow->is_mptfo). Under mptcp_data_lock(),
> flush queued bytes via __mptcp_check_push() and clear is_mptfo.

Why do you need to clear is_mptfo?

If it is not needed, you probably don't need to move is_mptfo. Also, we
would go back to the version shared by Petar.

@Petar: are you working with T S? Because if you suggested the patch, we
should at least add a "Suggested-by" tag. Is it OK with the
attributions? (Next time, please send a formal patch to avoid any
conflicts here ;))

> Also move is_mptfo from the shared u32 bitfield to a standalone
> bool in struct mptcp_subflow_context to avoid any read-modify-write
> data races with concurrent bitfield modifications (e.g.
> close_event_done in mptcp_close_ssk()).
> 
> Reported-by: Petar Sakic <petar.sakic@ink.fish>
> Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
> Assisted-by: Gemini 3.7
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
> ---
>  net/mptcp/fastopen.c | 2 +-
>  net/mptcp/options.c  | 9 +++++++++
>  net/mptcp/protocol.h | 4 ++--
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index f71775090..c11fc3c5c 100644
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c
> @@ -29,7 +29,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
>  	if (!skb)
>  		return;
>  
> -	subflow->is_mptfo = 1;
> +	subflow->is_mptfo = true;
>  
>  	/* dequeue the skb from sk receive queue */
>  	__skb_unlink(skb, &ssk->sk_receive_queue);
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index ce0de02f5..7ea97b283 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>  
>  	mptcp_data_lock((struct sock *)msk);
>  	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
> +	/* Passive TFO: the application may have written data while the
> +	 * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
> +	 * it then and nothing else spools the msk write queue when the
> +	 * MPC third ack (no DSS) arrives. Push it now.
> +	 */
> +	if (subflow->is_mptfo) {
> +		subflow->is_mptfo = false;
> +		__mptcp_check_push((struct sock *)msk, ssk);
> +	}
>  	mptcp_data_unlock((struct sock *)msk);
>  
>  check_notify:
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0384d6a02..0e44de129 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -582,15 +582,15 @@ struct mptcp_subflow_context {
>  		disposable : 1,	    /* ctx can be free at ulp release time */
>  		closing : 1,	    /* must not pass rx data to msk anymore */
>  		valid_csum_seen : 1,        /* at least one csum validated */
> -		is_mptfo : 1,	    /* subflow is doing TFO */
>  		close_event_done : 1,       /* has done the post-closed part */
>  		mpc_drop : 1,	    /* the MPC option has been dropped in a rtx */
>  		resetting : 1,	    /* subflow is resetting */
> -		__unused : 8;
> +		__unused : 9;
>  	bool	data_avail;
>  	bool	scheduled;
>  	bool	pm_listener;	    /* a listener managed by the kernel PM? */
>  	bool	fully_established;  /* path validated */
> +	bool	is_mptfo;	    /* subflow is doing TFO */
>  	u32	lent_mem_frag;
>  	u32	remote_nonce;
>  	u64	thmac;

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
       [not found]   ` <PN0PR01MB9382C18780E137E3013A097CF4932@PN0PR01MB9382.INDPRD01.PROD.OUTLOOK.COM>
@ 2026-10-08 13:31     ` Matthieu Baerts
  2026-10-08 14:39       ` Petar Sakic
  2026-10-12  9:43       ` Matthieu Baerts
  0 siblings, 2 replies; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-08 13:31 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: MPTCP Linux, petar.sakic

On 08/10/2026 15:06, T S Rameshkumar wrote:
> Thank you for the detailed feedback and guidance on the workflow!
> I apologize for sending revisions too quickly without giving enough time
> for review and discussion. I will make sure to pace subsequent
> submissions properly, base patches on top of the MPTCP "export" branch,
> increment version tags (v2, v3, etc.), and include changelogs using b4.

Thank you!

Please keep the discussions on the ML (and reply in plain text, and no
top-post [1]).

[1] https://subspace.kernel.org/etiquette.html

> Regarding your technical questions:
> 
>  1.
>     Clearing is_mptfo:
>      1.
>         You are completely right. An automated bot flagged a potential
>         bitfield data race if is_mptfo were cleared concurrently.
>         However, since check_fully_established() is only evaluated once
>         upon receiving the MPC third-ACK, clearing is_mptfo is not
>         needed at all. Since is_mptfo is only read after subflow
>         creation, leaving it in the bitfield and uncleared (exactly as
>         Petar drafted) is completely race-free and clean.

Thanks!

Please also send a regression test for this, ideally with Packetdrill:

 https://lore.kernel.org/bfd64e10-7f1e-41f6-ad12-12f06c4d2880@kernel.org

>  2.
>     Attribution:
>      1.
>         I will definitely add "Suggested-by: Petar Sakic
>         <petar.sakic@ink.fish>".
> 
> I will wait for Petar's response and any further review comments before
> submitting v2 on top of the export branch.

Sounds good to be. But it will be a "v6" then.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 12:16 [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
  2026-10-08 12:29 ` sashiko-bot
@ 2026-10-08 13:42 ` MPTCP CI
       [not found] ` <CADRhS2uk-P8TSmyiM=X5JQXHtzdm68vg6+vyBDr-UCf0EAu0HA@mail.gmail.com>
  2 siblings, 0 replies; 14+ messages in thread
From: MPTCP CI @ 2026-10-08 13:42 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: mptcp

Hi T,

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): Success! ✅
- 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! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/37777427552

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e76f38387d94
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1181596


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] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 13:31     ` Matthieu Baerts
@ 2026-10-08 14:39       ` Petar Sakic
  2026-10-08 16:10         ` Matthieu Baerts
  2026-10-12  9:43       ` Matthieu Baerts
  1 sibling, 1 reply; 14+ messages in thread
From: Petar Sakic @ 2026-10-08 14:39 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: T S Rameshkumar, MPTCP Linux

Hi Matt,

> @Petar: are you working with T S? Because if you suggested the patch, we
> should at least add a "Suggested-by" tag. Is it OK with the
> attributions?

No, I'm not working with T S. I don't know him; he picked up my report
from the list.

Suggested-by is fine with me, and so is T S carrying the patch. I'm not
a kernel developer, so I'm glad someone who can do the formal
submission and the packetdrill test is taking it on.

> (Next time, please send a formal patch to avoid any conflicts here ;))

Understood, will do.

For context: we hit this with xray Shadowsocks-2022 proxies over MPTCP.
The client puts the whole request in the first write, so small requests
ride the SYN, and replies from targets close to the server hang. We
reproduced it in network namespaces at ~120 ms RTT, and the single
options.c hunk fixed 30/30 requests on 6.12.111. Happy to run our
reproducer against v6 when it is posted.

Thanks,
Petar

On Thu, Oct 8, 2026 at 3:31 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> On 08/10/2026 15:06, T S Rameshkumar wrote:
> > Thank you for the detailed feedback and guidance on the workflow!
> > I apologize for sending revisions too quickly without giving enough time
> > for review and discussion. I will make sure to pace subsequent
> > submissions properly, base patches on top of the MPTCP "export" branch,
> > increment version tags (v2, v3, etc.), and include changelogs using b4.
>
> Thank you!
>
> Please keep the discussions on the ML (and reply in plain text, and no
> top-post [1]).
>
> [1] https://subspace.kernel.org/etiquette.html
>
> > Regarding your technical questions:
> >
> >  1.
> >     Clearing is_mptfo:
> >      1.
> >         You are completely right. An automated bot flagged a potential
> >         bitfield data race if is_mptfo were cleared concurrently.
> >         However, since check_fully_established() is only evaluated once
> >         upon receiving the MPC third-ACK, clearing is_mptfo is not
> >         needed at all. Since is_mptfo is only read after subflow
> >         creation, leaving it in the bitfield and uncleared (exactly as
> >         Petar drafted) is completely race-free and clean.
>
> Thanks!
>
> Please also send a regression test for this, ideally with Packetdrill:
>
>  https://lore.kernel.org/bfd64e10-7f1e-41f6-ad12-12f06c4d2880@kernel.org
>
> >  2.
> >     Attribution:
> >      1.
> >         I will definitely add "Suggested-by: Petar Sakic
> >         <petar.sakic@ink.fish>".
> >
> > I will wait for Petar's response and any further review comments before
> > submitting v2 on top of the export branch.
>
> Sounds good to be. But it will be a "v6" then.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 14:39       ` Petar Sakic
@ 2026-10-08 16:10         ` Matthieu Baerts
  0 siblings, 0 replies; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-08 16:10 UTC (permalink / raw)
  To: Petar Sakic; +Cc: T S Rameshkumar, MPTCP Linux

On 08/10/2026 16:39, Petar Sakic wrote:
>> @Petar: are you working with T S? Because if you suggested the patch, we
>> should at least add a "Suggested-by" tag. Is it OK with the
>> attributions?
> 
> No, I'm not working with T S. I don't know him; he picked up my report
> from the list.
> 
> Suggested-by is fine with me, and so is T S carrying the patch. I'm not
> a kernel developer, so I'm glad someone who can do the formal
> submission and the packetdrill test is taking it on.

Great!

>> (Next time, please send a formal patch to avoid any conflicts here ;))
> 
> Understood, will do.

Note that you can also report issues with MPTCP on GitHub:

  https://github.com/multipath-tcp/mptcp_net-next

> For context: we hit this with xray Shadowsocks-2022 proxies over MPTCP.
> The client puts the whole request in the first write, so small requests
> ride the SYN, and replies from targets close to the server hang. We
> reproduced it in network namespaces at ~120 ms RTT, and the single
> options.c hunk fixed 30/30 requests on 6.12.111. Happy to run our
> reproducer against v6 when it is posted.

Interesting use-case! That's a shame we didn't find this issue earlier.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 12:42 T S Rameshkumar
  2026-10-08 12:56 ` sashiko-bot
  2026-10-08 13:03 ` Matthieu Baerts
@ 2026-10-10  7:40 ` netdev-bot+sashiko
  2 siblings, 0 replies; 14+ messages in thread
From: netdev-bot+sashiko @ 2026-10-10  7:40 UTC (permalink / raw)
  To: rameshsv06; +Cc: matttbe, mptcp, petar.sakic, rameshkumar.t, kuba

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: data-less passive TFO subflows are still
  left with stranded data.
- [Medium] Sibling path left unfixed: fallback on the third ACK.
- [Low] The Fixes: tag 'fb7084501a61 ("mptcp: add support for
  TCP_FASTOPEN sockopt")' points to a SHA that does not exist in the…
- [Low] The commit author is 'T S Rameshkumar <rameshsv06@gmail.com>',
  but the only Signed-off-by is 'T S Rameshkumar…

--

mptcp: push queued data on passive TFO subflows becoming established

When a passive TFO subflow with is_mptfo set becomes fully established,
this patch pushes the msk write queue from check_fully_established().
Without that push, data written while the subflow was still in SYN_RECV
stays stranded. The patch also moves is_mptfo out of the shared u32
bitfield and makes it a standalone bool.

Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/

> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")

[Severity: Low]
Is this Fixes: tag correct? fb7084501a61 doesn't resolve to a commit in
the tree.

By subject, the closest match is 4ffb0a02346c ("mptcp: add TCP_FASTOPEN
sock option"). The passive TFO path changed here, including is_mptfo and
mptcp_fastopen_subflow_synack_set_params(), came from:

  36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")

The closely related fix e00b63056fb4 ("mptcp: fastopen: only mark MPTFO
subflows with SYN data") also uses 36b122baf6a8 as its Fixes: target.

Should this be:

  Fixes: 36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")

> Assisted-by: Gemini 3.7
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>

[Severity: Low]
The author is rameshsv06@gmail.com, but the only Signed-off-by uses
rameshkumar.t@phytecembedded.in. checkpatch reports this as a
"From:/Signed-off-by: email address mismatch".

Could the author and Signed-off-by identities be aligned? The patch also
carries an Assisted-by: tag, so a matching DCO sign-off from the author
is worth having.

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index ce0de02f5a3a1..7ea97b283fda1 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>  
>  	mptcp_data_lock((struct sock *)msk);
>  	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
> +	/* Passive TFO: the application may have written data while the
> +	 * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
> +	 * it then and nothing else spools the msk write queue when the
> +	 * MPC third ack (no DSS) arrives. Push it now.
> +	 */
> +	if (subflow->is_mptfo) {
> +		subflow->is_mptfo = false;
> +		__mptcp_check_push((struct sock *)msk, ssk);
> +	}

[Severity: High]
Does this cover passive TFO children whose SYN carried no data?

Since e00b63056fb4 ("mptcp: fastopen: only mark MPTFO subflows with SYN
data"), mptcp_fastopen_subflow_synack_set_params() returns before setting
is_mptfo when the receive queue is empty:

net/mptcp/fastopen.c:mptcp_fastopen_subflow_synack_set_params() {
    ...
	/* A valid TFO cookie does not guarantee SYN data. */
	skb = skb_peek(&ssk->sk_receive_queue);
	if (!skb)
		return;

	subflow->is_mptfo = true;
    ...
}

But tcp_try_fastopen() still creates a full Fast Open child for a
data-less SYN. This happens when the SYN has a valid cookie, or when the
listener has TFO_SERVER_COOKIE_NOT_REQD and the SYN carries a cookie
option:

net/ipv4/tcp_fastopen.c:tcp_try_fastopen() {
    ...
			 * Note: Data-less SYN with valid cookie is allowed to
			 * send data in SYN_RECV state.
			 */
fastopen:
			child = tcp_fastopen_create_child(sk, skb, req);
    ...
}

That child can be accept()ed while it is still in SYN_RECV.
mptcp_sk_clone_init() has already moved the msk to TCP_ESTABLISHED, so
mptcp_sendmsg() queues the data without waiting. Every push attempt is
then refused by __mptcp_subflow_active()->__tcp_can_send(), which only
allows TCPF_ESTABLISHED and TCPF_CLOSE_WAIT.

The sequence looks like this:

tcp_try_fastopen()
  tcp_fastopen_create_child()
    subflow_syn_recv_sock()->mptcp_sk_clone_init()  /* msk ESTABLISHED */
subflow_prep_synack()
  mptcp_fastopen_subflow_synack_set_params()        /* is_mptfo stays false */
accept() + write()                                  /* queued, ssk SYN_RECV */
mptcp_incoming_options()                            /* MPC third ACK */
  check_fully_established()                         /* reaches set_fully_established */

When the MPC third ACK arrives, seq == ssn_offset + 1 because there was
no SYN data to offset. There is no DSS and MPC is present, so the code
reaches set_fully_established. At that point is_mptfo is false and
__mptcp_check_push() is never called.

Wouldn't the queued data then stay in the msk write queue until the peer
sends again? That is the same symptom the commit message describes.

Would it work better to gate this on something that covers every passive
Fast Open child? Options include tcp_sk(ssk)->syn_fastopen_child, or
just checking for pending data with mptcp_send_head(), rather than
is_mptfo.

>  	mptcp_data_unlock((struct sock *)msk);
>  
>  check_notify:

[Severity: Medium]
What happens when the in-sequence third ACK on a passive TFO subflow
arrives without an MP_CAPABLE option? That could happen if a middlebox
strips it or the peer leaves it out.

In that case check_fully_established() takes the earlier fallback
branch. It returns before reaching set_fully_established, so it skips
the new push:

	if (!(mp_opt->suboptions & OPTIONS_MPTCP_MPC)) {
		if (subflow->mp_join)
			goto reset;
		subflow->mp_capable = 0;
		if (!mptcp_try_fallback(ssk, MPTCP_MIB_MPCAPABLEDATAFALLBACK)) {
			...
		}
		return false;
	}

mptcp_incoming_options() then returns without pushing:

	if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
		return sk->sk_state != TCP_CLOSE;

The fallback branch in mptcp_incoming_options() that does spool the
write queue is checked before check_fully_established():

	if (__mptcp_check_fallback(msk)) {
		...
		mptcp_data_lock(subflow->conn);
		if (sk_stream_memory_free(sk))
			__mptcp_check_push(subflow->conn, sk);

So that branch only runs on the next incoming segment.
__mptcp_try_fallback() itself only updates msk->flags and does not
schedule a push.

In this fallback case, would the data written while the subflow was in
SYN_RECV also stay queued until the peer sends something else?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008124220.213016-1-rameshkumar.t%40phytecembedded.in

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-08 13:31     ` Matthieu Baerts
  2026-10-08 14:39       ` Petar Sakic
@ 2026-10-12  9:43       ` Matthieu Baerts
  1 sibling, 0 replies; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-12  9:43 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: MPTCP Linux, petar.sakic

Hi T S,

On 08/10/2026 15:31, Matthieu Baerts wrote:
> On 08/10/2026 15:06, T S Rameshkumar wrote:

(...)

>> Regarding your technical questions:
>>
>>  1.
>>     Clearing is_mptfo:
>>      1.
>>         You are completely right. An automated bot flagged a potential
>>         bitfield data race if is_mptfo were cleared concurrently.
>>         However, since check_fully_established() is only evaluated once
>>         upon receiving the MPC third-ACK, clearing is_mptfo is not
>>         needed at all. Since is_mptfo is only read after subflow
>>         creation, leaving it in the bitfield and uncleared (exactly as
>>         Petar drafted) is completely race-free and clean.
> 
> Thanks!
> 
> Please also send a regression test for this, ideally with Packetdrill:
> 
>  https://lore.kernel.org/bfd64e10-7f1e-41f6-ad12-12f06c4d2880@kernel.org

Just to avoid a deadlock, no urgency, but do you plan to send a new
version? If there are some issues with Packetdrill, please describe
them, someone can also help you there.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
       [not found] ` <CADRhS2uk-P8TSmyiM=X5JQXHtzdm68vg6+vyBDr-UCf0EAu0HA@mail.gmail.com>
@ 2026-10-12 13:28   ` Matthieu Baerts
  2026-10-12 19:48     ` Petar Sakic
  0 siblings, 1 reply; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-12 13:28 UTC (permalink / raw)
  To: T S RAMESHKUMAR; +Cc: Petar Sakic, mptcp

Hi T S,

+cc Petar.

On 12/10/2026 12:00, T S RAMESHKUMAR wrote:
> Hi Matt, Petar,
> 
> Thank you both!
> 
> I have prepared [PATCH mptcp-net v2] based on the export branch.
> In v2:
> - Reverted bitfield modifications: check_fully_established() is only
> called once upon receiving the MPC 3rd-ACK, so is_mptfo is read-only at
> that stage and safe from concurrency races.
> - Added "Suggested-by: Petar Sakic <petar.sakic@ink.fish>" and
> "Reported-by: Petar Sakic <petar.sakic@ink.fish>".
> - Kept the Fixes tag pointing to fb7084501a61 ("mptcp: add support for
> TCP_FASTOPEN sockopt") to ensure backports cover 6.8 and all stable trees.

Thank you. I will try to review this after the automated tests.

> Regarding Packetdrill:
> I have prepared the packetdrill test case (server-TCP_FASTOPEN-early-
> write.pkt) which simulates the server writing data in SYN_RECV before
> the 3rd ACK arrives and verifies that the 100 bytes are pushed
> immediately with DSS mapping once the 3rd ACK is received. All 51
> fastopen tests pass.
> 
> I can submit the packetdrill test via a PR on https://github.com/
> multipath-tcp/packetdrill <https://github.com/multipath-tcp/
> packetdrill>, or send it directly to the list if preferred. Here is the
> test script for reference:

Please open a new PR.

> --- server-TCP_FASTOPEN-early-write.pkt ---
> // SPDX-License-Identifier: GPL-2.0
> // Test MPTCP passive Fast Open early write before 3rd ACK arrives
> --tolerance_usecs=100000
> `../common/defaults.sh`
> 
>  0.0    socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
> +0.0    setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0.0    setsockopt(3, SOL_TCP, TCP_FASTOPEN, [2], 4) = 0
> 
> +0.0    bind(3, ..., ...) = 0
> +0.0    listen(3, 1) = 0
> 
> // Client connects with TFO Cookie and 500 bytes of SYN data
> +0.1      <  S   0:500(500)                win 65535  <mss 1460, sackOK,
> TS val 100 ecr 0,   nop, wscale 8, FO TFO_COOKIE, nop, nop, mpcapable v1
> flags[flag_h] nokey>
> +0.0      >  S.  0:0(0)          ack 501              <mss 1460, sackOK,
> TS val 100 ecr 100, nop, wscale 8,                          mpcapable v1
> flags[flag_h] key[skey]>
> 
> // Server accepts while still in SYN_RECV and reads the SYN data
> +0.0    accept(3, ..., ...) = 4
> +0.0    read(4, ..., 500) = 500
> 
> // Server writes early reply data before receiving client's 3rd ACK
> +0.0    write(4, ..., 100) = 100
> 
> // Client's 3rd ACK arrives with no data, establishing the MPTCP connection
> +0.2      <   .  501:501(0)      ack 1     win 450    <                
>                                                              mpcapable
> v1 flags[flag_h] key[ckey, skey]>

Here, you are dropping TCP Timestamps (TS) negotiated before. Probably
better to keep it (or you don't inject it in he first SYN, clearer).

> // Server must immediately push the queued 100 bytes with DSS mapping
> +0.0      >  P.  1:101(100)      ack 501              <nop, nop, TS val
> 100 ecr 100, dss dack8=0 dsn8=1 ssn=1 dll=100 nocs>

"dack8=0"? That's strange...

Also, TCP options are not padded correctly to ensure TCP header is a
multiple of 4 bytes: 2 excess bytes → you are missing two "nop" at the end.

How did you test it?

 $ cd <Packetdrill DIR>/gtests/net
 $ run_all.py -lv mptcp/fastopen/server-TCP_FASTOPEN-early-write.pkt

Don't forget to add the "Assisted-by: LLM" tag if used, and mention what
you did, how you tested things. You can use this docker image to ease
the testing part:

  https://github.com/multipath-tcp/mptcp-upstream-virtme-docker

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-12 13:28   ` Matthieu Baerts
@ 2026-10-12 19:48     ` Petar Sakic
  2026-10-13  9:28       ` Matthieu Baerts
  0 siblings, 1 reply; 14+ messages in thread
From: Petar Sakic @ 2026-10-12 19:48 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: T S RAMESHKUMAR, mptcp

Hi Matthieu and T S

I hope replying here is OK.

Tested v2 on top of 6.12.111 (applies with a 15-line offset, no other change).

Setup: two netns over a veth pair, netem 50 ms each way (100 ms RTT). MPTCP
listener with TCP_FASTOPEN=16; the server replies as soon as accept() returns,
while the TFO child is still SYN_RECV (checked via TCP_INFO). Client: first
connection fetches the cookie, the next ones use TCP_FASTOPEN_CONNECT so the
request rides the SYN. Checked per run: TCPFastOpenActive/Passive,
MPCapableSYNTX/ACKRX, MPCapableFallbackACK = 0.

- 6.12.111 unpatched: reply stranded on every TFO connection (3/3).
- 6.12.111 + v2: reply delivered on every TFO connection (3/3), ~2 RTT.
  Plain TCP control unchanged.

Tested-by: Petar Sakic <petar.sakic@ink.fish>

One case v2 does not cover, same setup: a cookie-bearing SYN with no data
(sendmsg MSG_FASTOPEN, zero bytes), server speaking first.
tcp_fastopen_create_child() still creates the child (TCPFastOpenPassive
increments, accept() returns in SYN_RECV), but
mptcp_fastopen_subflow_synack_set_params() returns before setting
subflow->is_mptfo because the receive queue is empty, so the new check
never fires and the reply is stranded again (2/2, with and without v2).
Plain TCP handles it. Moving the assignment above the early return makes
both cases pass here (3/3 and 2/2), no splats:

--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -25,13 +25,13 @@
        tp = tcp_sk(ssk);

+       subflow->is_mptfo = 1;
+
        /* A valid TFO cookie does not guarantee SYN data. */
        skb = skb_peek(&ssk->sk_receive_queue);
        if (!skb)
                return;

-       subflow->is_mptfo = 1;
-
        /* dequeue the skb from sk receive queue */

Up to you whether that belongs in this patch or a follow-up.

Cheers,
- Petar


On Mon, Oct 12, 2026 at 3:28 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi T S,
>
> +cc Petar.
>
> On 12/10/2026 12:00, T S RAMESHKUMAR wrote:
> > Hi Matt, Petar,
> >
> > Thank you both!
> >
> > I have prepared [PATCH mptcp-net v2] based on the export branch.
> > In v2:
> > - Reverted bitfield modifications: check_fully_established() is only
> > called once upon receiving the MPC 3rd-ACK, so is_mptfo is read-only at
> > that stage and safe from concurrency races.
> > - Added "Suggested-by: Petar Sakic <petar.sakic@ink.fish>" and
> > "Reported-by: Petar Sakic <petar.sakic@ink.fish>".
> > - Kept the Fixes tag pointing to fb7084501a61 ("mptcp: add support for
> > TCP_FASTOPEN sockopt") to ensure backports cover 6.8 and all stable trees.
>
> Thank you. I will try to review this after the automated tests.
>
> > Regarding Packetdrill:
> > I have prepared the packetdrill test case (server-TCP_FASTOPEN-early-
> > write.pkt) which simulates the server writing data in SYN_RECV before
> > the 3rd ACK arrives and verifies that the 100 bytes are pushed
> > immediately with DSS mapping once the 3rd ACK is received. All 51
> > fastopen tests pass.
> >
> > I can submit the packetdrill test via a PR on https://github.com/
> > multipath-tcp/packetdrill <https://github.com/multipath-tcp/
> > packetdrill>, or send it directly to the list if preferred. Here is the
> > test script for reference:
>
> Please open a new PR.
>
> > --- server-TCP_FASTOPEN-early-write.pkt ---
> > // SPDX-License-Identifier: GPL-2.0
> > // Test MPTCP passive Fast Open early write before 3rd ACK arrives
> > --tolerance_usecs=100000
> > `../common/defaults.sh`
> >
> >  0.0    socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
> > +0.0    setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> > +0.0    setsockopt(3, SOL_TCP, TCP_FASTOPEN, [2], 4) = 0
> >
> > +0.0    bind(3, ..., ...) = 0
> > +0.0    listen(3, 1) = 0
> >
> > // Client connects with TFO Cookie and 500 bytes of SYN data
> > +0.1      <  S   0:500(500)                win 65535  <mss 1460, sackOK,
> > TS val 100 ecr 0,   nop, wscale 8, FO TFO_COOKIE, nop, nop, mpcapable v1
> > flags[flag_h] nokey>
> > +0.0      >  S.  0:0(0)          ack 501              <mss 1460, sackOK,
> > TS val 100 ecr 100, nop, wscale 8,                          mpcapable v1
> > flags[flag_h] key[skey]>
> >
> > // Server accepts while still in SYN_RECV and reads the SYN data
> > +0.0    accept(3, ..., ...) = 4
> > +0.0    read(4, ..., 500) = 500
> >
> > // Server writes early reply data before receiving client's 3rd ACK
> > +0.0    write(4, ..., 100) = 100
> >
> > // Client's 3rd ACK arrives with no data, establishing the MPTCP connection
> > +0.2      <   .  501:501(0)      ack 1     win 450    <
> >                                                              mpcapable
> > v1 flags[flag_h] key[ckey, skey]>
>
> Here, you are dropping TCP Timestamps (TS) negotiated before. Probably
> better to keep it (or you don't inject it in he first SYN, clearer).
>
> > // Server must immediately push the queued 100 bytes with DSS mapping
> > +0.0      >  P.  1:101(100)      ack 501              <nop, nop, TS val
> > 100 ecr 100, dss dack8=0 dsn8=1 ssn=1 dll=100 nocs>
>
> "dack8=0"? That's strange...
>
> Also, TCP options are not padded correctly to ensure TCP header is a
> multiple of 4 bytes: 2 excess bytes → you are missing two "nop" at the end.
>
> How did you test it?
>
>  $ cd <Packetdrill DIR>/gtests/net
>  $ run_all.py -lv mptcp/fastopen/server-TCP_FASTOPEN-early-write.pkt
>
> Don't forget to add the "Assisted-by: LLM" tag if used, and mention what
> you did, how you tested things. You can use this docker image to ease
> the testing part:
>
>   https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-12 19:48     ` Petar Sakic
@ 2026-10-13  9:28       ` Matthieu Baerts
  0 siblings, 0 replies; 14+ messages in thread
From: Matthieu Baerts @ 2026-10-13  9:28 UTC (permalink / raw)
  To: Petar Sakic; +Cc: T S RAMESHKUMAR, mptcp

On 12/10/2026 21:48, Petar Sakic wrote:
> Hi Matthieu and T S
> 
> I hope replying here is OK.
> 
> Tested v2 on top of 6.12.111 (applies with a 15-line offset, no other change).
> 
> Setup: two netns over a veth pair, netem 50 ms each way (100 ms RTT). MPTCP
> listener with TCP_FASTOPEN=16; the server replies as soon as accept() returns,
> while the TFO child is still SYN_RECV (checked via TCP_INFO). Client: first
> connection fetches the cookie, the next ones use TCP_FASTOPEN_CONNECT so the
> request rides the SYN. Checked per run: TCPFastOpenActive/Passive,
> MPCapableSYNTX/ACKRX, MPCapableFallbackACK = 0.
> 
> - 6.12.111 unpatched: reply stranded on every TFO connection (3/3).
> - 6.12.111 + v2: reply delivered on every TFO connection (3/3), ~2 RTT.
>   Plain TCP control unchanged.
> 
> Tested-by: Petar Sakic <petar.sakic@ink.fish>

Thank you for having tested that.

> One case v2 does not cover, same setup: a cookie-bearing SYN with no data
> (sendmsg MSG_FASTOPEN, zero bytes), server speaking first.
> tcp_fastopen_create_child() still creates the child (TCPFastOpenPassive
> increments, accept() returns in SYN_RECV), but
> mptcp_fastopen_subflow_synack_set_params() returns before setting
> subflow->is_mptfo because the receive queue is empty, so the new check
> never fires and the reply is stranded again (2/2, with and without v2).
> Plain TCP handles it. Moving the assignment above the early return makes
> both cases pass here (3/3 and 2/2), no splats:
> 
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c
> @@ -25,13 +25,13 @@
>         tp = tcp_sk(ssk);
> 
> +       subflow->is_mptfo = 1;
> +
>         /* A valid TFO cookie does not guarantee SYN data. */
>         skb = skb_peek(&ssk->sk_receive_queue);
>         if (!skb)
>                 return;
> 
> -       subflow->is_mptfo = 1;
> -
>         /* dequeue the skb from sk receive queue */
> 
> Up to you whether that belongs in this patch or a follow-up.
Probably better in a follow-up in this form, but not with this patch:
here, you are reverting this:

  https://git.kernel.org/torvalds/c/e00b63056fb4

What about calling __mptcp_check_push() if (subflow->mp_capable) instead?

@T S: please wait for the discussions to settle before sending new versions.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-10-13  9:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 12:16 [PATCH v4] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
2026-10-08 12:29 ` sashiko-bot
2026-10-08 13:42 ` MPTCP CI
     [not found] ` <CADRhS2uk-P8TSmyiM=X5JQXHtzdm68vg6+vyBDr-UCf0EAu0HA@mail.gmail.com>
2026-10-12 13:28   ` Matthieu Baerts
2026-10-12 19:48     ` Petar Sakic
2026-10-13  9:28       ` Matthieu Baerts
  -- strict thread matches above, loose matches on Subject: below --
2026-10-08 12:42 T S Rameshkumar
2026-10-08 12:56 ` sashiko-bot
2026-10-08 13:03 ` Matthieu Baerts
     [not found]   ` <PN0PR01MB9382C18780E137E3013A097CF4932@PN0PR01MB9382.INDPRD01.PROD.OUTLOOK.COM>
2026-10-08 13:31     ` Matthieu Baerts
2026-10-08 14:39       ` Petar Sakic
2026-10-08 16:10         ` Matthieu Baerts
2026-10-12  9:43       ` Matthieu Baerts
2026-10-10  7:40 ` netdev-bot+sashiko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.