All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
@ 2026-10-12 11:10 T S Rameshkumar
  2026-10-12 12:24 ` MPTCP CI
  2026-10-14  0:11 ` netdev-bot+sashiko
  0 siblings, 2 replies; 6+ messages in thread
From: T S Rameshkumar @ 2026-10-12 11:10 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, 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().

Suggested-by: Petar Sakic <petar.sakic@ink.fish>
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")
Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
---
v2:
 - Reverted moving/clearing is_mptfo since check_fully_established() is
   only called once and is_mptfo is read-only at this stage (Matthieu Baerts).
 - Added Suggested-by tag for Petar Sakic.
 - Rebased cleanly on top of mptcp_net-next export branch.
---
 net/mptcp/options.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 2bf368670..f5a258c7d 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1043,6 +1043,8 @@ 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);
+	if (subflow->is_mptfo)
+		__mptcp_check_push((struct sock *)msk, ssk);
 	mptcp_data_unlock((struct sock *)msk);
 
 check_notify:
-- 
2.34.1

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

* Re: [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-12 11:10 [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
@ 2026-10-12 12:24 ` MPTCP CI
  2026-10-14  0:11 ` netdev-bot+sashiko
  1 sibling, 0 replies; 6+ messages in thread
From: MPTCP CI @ 2026-10-12 12:24 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: Unstable ⚠️
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38243594367

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


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

* Re: [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-12 11:10 [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
  2026-10-12 12:24 ` MPTCP CI
@ 2026-10-14  0:11 ` netdev-bot+sashiko
  2026-10-14  8:28   ` Matthieu Baerts
  1 sibling, 1 reply; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-10-14  0:11 UTC (permalink / raw)
  To: rameshsv06; +Cc: mptcp, matttbe, petar.sakic, rameshkumar.t, kuba

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

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

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.
- [Medium] The fix also misses the fallback path.
- [Low] The commit tags are wrong.

--

mptcp: push queued data on passive TFO subflows becoming established

This commit makes check_fully_established() call __mptcp_check_push()
when a passive TFO subflow (subflow->is_mptfo) becomes fully
established. The goal is to flush data the server wrote while the
subflow was still in SYN_RECV. Without the flush, that data stays in
the msk write queue because the MPC third ACK carries no DSS.

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

> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>

[Severity: Low]
This isn't a bug, but does fb7084501a61 exist? The hash doesn't resolve
in the tree, and no commit has that subject.

The passive TFO and is_mptfo handling seem to have come from these
commits:

  36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
  dfc8d0603033 ("mptcp: implement delayed seq generation for passive fastopen")
  4ffb0a02346c ("mptcp: add TCP_FASTOPEN sock option")

Should the Fixes tag point at 36b122baf6a8 or dfc8d0603033 instead?

Also, the Signed-off-by address (rameshkumar.t@phytecembedded.in) doesn't
match the author address (rameshsv06@gmail.com). Checkpatch reports this
as FROM_SIGN_OFF_MISMATCH.

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 20490410fa22f..33eede084af8d 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1046,6 +1046,8 @@ 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);
> +	if (subflow->is_mptfo)
> +		__mptcp_check_push((struct sock *)msk, ssk);

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

mptcp_fastopen_subflow_synack_set_params() sets is_mptfo only after it
finds SYN data in the subflow receive queue:

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 = 1;
    ...
}

The TCP core still creates a full TFO child for a data-less SYN that has
a valid cookie. It also does this when TFO_SERVER_COOKIE_NOT_REQD is set:

net/ipv4/tcp_fastopen.c:tcp_try_fastopen() {
    ...
	if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
	      (syn_data || foc->len >= 0) &&
	      tcp_fastopen_queue_check(sk))) {
    ...
}

tcp_conn_request() then puts that child straight into the accept queue,
while the subflow is still in SYN_RECV.

Suppose the application accepts and writes at once, as a
server-speaks-first protocol does. mptcp_sendmsg_frag() returns -EAGAIN
because !__tcp_can_send(ssk). __mptcp_push_pending() then exits with
nothing copied, so the rtx timer is never armed.

When the pure MPC third ACK arrives, is_mptfo is 0 at this point and the
push is skipped. mptcp_incoming_options() then returns before
ack_update_msk():

	if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
		return true;

So won't the queued bytes stay stuck until the peer sends something,
exactly as before this patch?

Would it work better to replace the is_mptfo test with one that covers
every passive TFO child, such as tcp_sk(ssk)->syn_fastopen_child? Another
option is to push whenever data is pending.

[Severity: Medium]
What happens on the fallback path of a passive TFO subflow?

The third ACK may carry no MP_CAPABLE option. That happens if a middlebox
stripped MPC and the client fell back, or if the peer doesn't conform. In
that case check_fully_established() never reaches set_fully_established:

net/mptcp/options.c:check_fully_established() {
    ...
	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)) {
			MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_FALLBACKFAILED);
			goto reset;
		}
		return false;
	}
    ...
}

mptcp_try_fallback() switches the msk to fallback mode, but it doesn't
push pending data.

The fallback push in mptcp_incoming_options() is checked before
check_fully_established() runs, so it only fires on later segments:

net/mptcp/options.c:mptcp_incoming_options() {
	if (__mptcp_check_fallback(msk)) {
		...
		if (sk_stream_memory_free(sk))
			__mptcp_check_push(subflow->conn, sk);
		...
	}
	...
	if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
		return sk->sk_state != TCP_CLOSE;
	...
}

In a typical TFO request/response, the client already sent its request in
the SYN and has nothing more to send.

Could a server reply written during SYN_RECV also get stuck in the msk
write queue on this path, with no rtx timer armed?

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

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

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

* Re: [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-14  0:11 ` netdev-bot+sashiko
@ 2026-10-14  8:28   ` Matthieu Baerts
  2026-10-21 17:56     ` Matthieu Baerts
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2026-10-14  8:28 UTC (permalink / raw)
  To: netdev-bot+sashiko; +Cc: rameshsv06, mptcp, petar.sakic, rameshkumar.t, kuba

Hi T S,

14 Oct 2026 02:11:13 netdev-bot+sashiko@kernel.org:

> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 1 · Low: 1
>
> 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.
> - [Medium] The fix also misses the fallback path.
> - [Low] The commit tags are wrong.
>
> --
>
> mptcp: push queued data on passive TFO subflows becoming established
>
> This commit makes check_fully_established() call __mptcp_check_push()
> when a passive TFO subflow (subflow->is_mptfo) becomes fully
> established. The goal is to flush data the server wrote while the
> subflow was still in SYN_RECV. Without the flush, that data stays in
> the msk write queue because the MPC third ACK carries no DSS.
>
> Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
>
>> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
>> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
>
> [Severity: Low]
> This isn't a bug, but does fb7084501a61 exist? The hash doesn't resolve
> in the tree, and no commit has that subject.

Nice, the LLM invented a plausible commit...

BTW, don't forget the "Assisted-by: LLM" tag.

> The passive TFO and is_mptfo handling seem to have come from these
> commits:
>
>   36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")

Yes

>   dfc8d0603033 ("mptcp: implement delayed seq generation for passive fastopen")
>   4ffb0a02346c ("mptcp: add TCP_FASTOPEN sock option")
>
> Should the Fixes tag point at 36b122baf6a8 or dfc8d0603033 instead?
>
> Also, the Signed-off-by address (rameshkumar.t@phytecembedded.in) doesn't
> match the author address (rameshsv06@gmail.com). Checkpatch reports this
> as FROM_SIGN_OFF_MISMATCH.

Indeed: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38243594334

>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 20490410fa22f..33eede084af8d 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -1046,6 +1046,8 @@ 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);
>> +   if (subflow->is_mptfo)
>> +       __mptcp_check_push((struct sock *)msk, ssk);
>
> [Severity: High]
> Does this check miss passive TFO children whose SYN carried no data?
>
> mptcp_fastopen_subflow_synack_set_params() sets is_mptfo only after it
> finds SYN data in the subflow receive queue:
>
> 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 = 1;
>     ...
> }
>
> The TCP core still creates a full TFO child for a data-less SYN that has
> a valid cookie. It also does this when TFO_SERVER_COOKIE_NOT_REQD is set:
>
> net/ipv4/tcp_fastopen.c:tcp_try_fastopen() {
>     ...
>     if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
>           (syn_data || foc->len >= 0) &&
>           tcp_fastopen_queue_check(sk))) {
>     ...
> }
>
> tcp_conn_request() then puts that child straight into the accept queue,
> while the subflow is still in SYN_RECV.
>
> Suppose the application accepts and writes at once, as a
> server-speaks-first protocol does. mptcp_sendmsg_frag() returns -EAGAIN
> because !__tcp_can_send(ssk). __mptcp_push_pending() then exits with
> nothing copied, so the rtx timer is never armed.
>
> When the pure MPC third ACK arrives, is_mptfo is 0 at this point and the
> push is skipped. mptcp_incoming_options() then returns before
> ack_update_msk():
>
>     if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
>         return true;
>
> So won't the queued bytes stay stuck until the peer sends something,
> exactly as before this patch?

Indeed (as mentioned in the human review).

> Would it work better to replace the is_mptfo test with one that covers
> every passive TFO child, such as tcp_sk(ssk)->syn_fastopen_child? Another

Good idea to use syn_fastopen_child here.

> option is to push whenever data is pending.
>
> [Severity: Medium]
> What happens on the fallback path of a passive TFO subflow?
>
> The third ACK may carry no MP_CAPABLE option. That happens if a middlebox
> stripped MPC and the client fell back, or if the peer doesn't conform. In
> that case check_fully_established() never reaches set_fully_established:
>
> net/mptcp/options.c:check_fully_established() {
>     ...
>     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)) {
>             MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_FALLBACKFAILED);
>             goto reset;
>         }
>         return false;
>     }
>     ...
> }
>
> mptcp_try_fallback() switches the msk to fallback mode, but it doesn't
> push pending data.
>
> The fallback push in mptcp_incoming_options() is checked before
> check_fully_established() runs, so it only fires on later segments:
>
> net/mptcp/options.c:mptcp_incoming_options() {
>     if (__mptcp_check_fallback(msk)) {
>         ...
>         if (sk_stream_memory_free(sk))
>             __mptcp_check_push(subflow->conn, sk);
>         ...
>     }
>     ...
>     if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
>         return sk->sk_state != TCP_CLOSE;
>     ...
> }
>
> In a typical TFO request/response, the client already sent its request in
> the SYN and has nothing more to send.
>
> Could a server reply written during SYN_RECV also get stuck in the msk
> write queue on this path, with no rtx timer armed?
>
>>     mptcp_data_unlock((struct sock *)msk);
>>
>> check_notify:

It's plausible, but: to be confirmed with a packetdrill test, and it should
be part of another fix. Do you want to work on that too?

Cheers,
Matt

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


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

* Re: [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
  2026-10-14  8:28   ` Matthieu Baerts
@ 2026-10-21 17:56     ` Matthieu Baerts
       [not found]       ` <CADRhS2to+Z-ZztOpfYhum5z4jXkUHFhnQL3+nVBJHJxe3NZjZw@mail.gmail.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2026-10-21 17:56 UTC (permalink / raw)
  To: rameshsv06; +Cc: mptcp, petar.sakic, rameshkumar.t

Hi T S,

On 14/10/2026 10:28, Matthieu Baerts wrote:
> Hi T S,

Just to follow-up here: do you still plan to send a v4 with the
different fixes (Fixes tag, Signed-off-by/author, using
syn_fastopen_child), the packetdrill test for that case, and eventually
another fix for the fallback case (that can be for another series).

If you don't plan to send a new version, or you have issues, please comment.

Cheers,
Matt

> 14 Oct 2026 02:11:13 netdev-bot+sashiko@kernel.org:
> 
>> Thank you for your contribution! Sashiko AI review found 3 potential
>> issue(s) to consider.
>>
>> Critical: 0 · High: 1 · Medium: 1 · Low: 1
>>
>> 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.
>> - [Medium] The fix also misses the fallback path.
>> - [Low] The commit tags are wrong.
>>
>> --
>>
>> mptcp: push queued data on passive TFO subflows becoming established
>>
>> This commit makes check_fully_established() call __mptcp_check_push()
>> when a passive TFO subflow (subflow->is_mptfo) becomes fully
>> established. The goal is to flush data the server wrote while the
>> subflow was still in SYN_RECV. Without the flush, that data stays in
>> the msk write queue because the MPC third ACK carries no DSS.
>>
>> Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
>>
>>> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
>>> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
>>
>> [Severity: Low]
>> This isn't a bug, but does fb7084501a61 exist? The hash doesn't resolve
>> in the tree, and no commit has that subject.
> 
> Nice, the LLM invented a plausible commit...
> 
> BTW, don't forget the "Assisted-by: LLM" tag.
> 
>> The passive TFO and is_mptfo handling seem to have come from these
>> commits:
>>
>>   36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
> 
> Yes
> 
>>   dfc8d0603033 ("mptcp: implement delayed seq generation for passive fastopen")
>>   4ffb0a02346c ("mptcp: add TCP_FASTOPEN sock option")
>>
>> Should the Fixes tag point at 36b122baf6a8 or dfc8d0603033 instead?
>>
>> Also, the Signed-off-by address (rameshkumar.t@phytecembedded.in) doesn't
>> match the author address (rameshsv06@gmail.com). Checkpatch reports this
>> as FROM_SIGN_OFF_MISMATCH.
> 
> Indeed: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38243594334
> 
>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>>> index 20490410fa22f..33eede084af8d 100644
>>> --- a/net/mptcp/options.c
>>> +++ b/net/mptcp/options.c
>>> @@ -1046,6 +1046,8 @@ 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);
>>> +   if (subflow->is_mptfo)
>>> +       __mptcp_check_push((struct sock *)msk, ssk);
>>
>> [Severity: High]
>> Does this check miss passive TFO children whose SYN carried no data?
>>
>> mptcp_fastopen_subflow_synack_set_params() sets is_mptfo only after it
>> finds SYN data in the subflow receive queue:
>>
>> 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 = 1;
>>     ...
>> }
>>
>> The TCP core still creates a full TFO child for a data-less SYN that has
>> a valid cookie. It also does this when TFO_SERVER_COOKIE_NOT_REQD is set:
>>
>> net/ipv4/tcp_fastopen.c:tcp_try_fastopen() {
>>     ...
>>     if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
>>           (syn_data || foc->len >= 0) &&
>>           tcp_fastopen_queue_check(sk))) {
>>     ...
>> }
>>
>> tcp_conn_request() then puts that child straight into the accept queue,
>> while the subflow is still in SYN_RECV.
>>
>> Suppose the application accepts and writes at once, as a
>> server-speaks-first protocol does. mptcp_sendmsg_frag() returns -EAGAIN
>> because !__tcp_can_send(ssk). __mptcp_push_pending() then exits with
>> nothing copied, so the rtx timer is never armed.
>>
>> When the pure MPC third ACK arrives, is_mptfo is 0 at this point and the
>> push is skipped. mptcp_incoming_options() then returns before
>> ack_update_msk():
>>
>>     if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
>>         return true;
>>
>> So won't the queued bytes stay stuck until the peer sends something,
>> exactly as before this patch?
> 
> Indeed (as mentioned in the human review).
> 
>> Would it work better to replace the is_mptfo test with one that covers
>> every passive TFO child, such as tcp_sk(ssk)->syn_fastopen_child? Another
> 
> Good idea to use syn_fastopen_child here.
> 
>> option is to push whenever data is pending.
>>
>> [Severity: Medium]
>> What happens on the fallback path of a passive TFO subflow?
>>
>> The third ACK may carry no MP_CAPABLE option. That happens if a middlebox
>> stripped MPC and the client fell back, or if the peer doesn't conform. In
>> that case check_fully_established() never reaches set_fully_established:
>>
>> net/mptcp/options.c:check_fully_established() {
>>     ...
>>     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)) {
>>             MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_FALLBACKFAILED);
>>             goto reset;
>>         }
>>         return false;
>>     }
>>     ...
>> }
>>
>> mptcp_try_fallback() switches the msk to fallback mode, but it doesn't
>> push pending data.
>>
>> The fallback push in mptcp_incoming_options() is checked before
>> check_fully_established() runs, so it only fires on later segments:
>>
>> net/mptcp/options.c:mptcp_incoming_options() {
>>     if (__mptcp_check_fallback(msk)) {
>>         ...
>>         if (sk_stream_memory_free(sk))
>>             __mptcp_check_push(subflow->conn, sk);
>>         ...
>>     }
>>     ...
>>     if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
>>         return sk->sk_state != TCP_CLOSE;
>>     ...
>> }
>>
>> In a typical TFO request/response, the client already sent its request in
>> the SYN and has nothing more to send.
>>
>> Could a server reply written during SYN_RECV also get stuck in the msk
>> write queue on this path, with no rtx timer armed?
>>
>>>     mptcp_data_unlock((struct sock *)msk);
>>>
>>> check_notify:
> 
> It's plausible, but: to be confirmed with a packetdrill test, and it should
> be part of another fix. Do you want to work on that too?
> 
> Cheers,
> Matt
> 
>>
>> --
>> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012111001.542847-1-rameshkumar.t%40phytecembedded.in
> 

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


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

* Re: [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established
       [not found]       ` <CADRhS2to+Z-ZztOpfYhum5z4jXkUHFhnQL3+nVBJHJxe3NZjZw@mail.gmail.com>
@ 2026-10-27 10:48         ` Matthieu Baerts
  0 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2026-10-27 10:48 UTC (permalink / raw)
  To: T S RAMESHKUMAR; +Cc: MPTCP Linux, Petar Sakic, T S Rameshkumar

Hi T S,

On 22/10/2026 04:29, T S RAMESHKUMAR wrote:
> Yes, I am working on it. 
> 
> I planned that the fixes that the commit has been made on the Friday 22
> Oct 2026 

I'm sorry for this reminder, but everything is still OK on your side?

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


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

end of thread, other threads:[~2026-10-27 10:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 11:10 [PATCH mptcp-net v2] mptcp: push queued data on passive TFO subflows becoming established T S Rameshkumar
2026-10-12 12:24 ` MPTCP CI
2026-10-14  0:11 ` netdev-bot+sashiko
2026-10-14  8:28   ` Matthieu Baerts
2026-10-21 17:56     ` Matthieu Baerts
     [not found]       ` <CADRhS2to+Z-ZztOpfYhum5z4jXkUHFhnQL3+nVBJHJxe3NZjZw@mail.gmail.com>
2026-10-27 10:48         ` Matthieu Baerts

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.