All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next v15 00/11] Reduce the differences between TCP and MPTCP for TLS usage
@ 2026-10-23  1:58 Geliang Tang
  2026-10-23  1:58 ` [PATCH mptcp-next v15 01/11] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Geliang Tang @ 2026-10-23  1:58 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

v15:
 - Patch 2: drop mptcp_data_lock() in mptcp_inq_hint(), use READ_ONCE()
   and __test_and_clear_bit() instead. This issue is unrelated to this
   series and will be addressed in a separate patch later.
 - Patch 5: Guard mptcp_cleanup_rbuf() with read_copied > 0. Initialize/
   reset msk->read_copied. Drop redundant mptcp_recv_skb() in
   __mptcp_read_sock().
 - Patch 6: simplify memory ordering, drop smp_mb()/smp_rmb(), keep
   existing smp_mb__before_atomic(). Check SOCK_DONE inline.
 - Patch 9: drop tcp_sock_rate_check_app_limited() split, call
   tcp_rate_check_app_limited() directly as Paolo suggested. Add
   write_seq != snd_nxt early bail-out.
 - Patch 10: wrap assertion behind expect_all_features().

v14:
 - v13 can no longer be applied to the export branch, so it has been
   rebased here with the following corrections:
 - Patch 2: move locking in mptcp_inq_hint() to avoid races with ack_seq
   change from patch 3 to here. Update commit log.
 - Patch 3: add mptcp_data_lock() in __mptcp_move_skb() around copied_seq
   update. Remove local delta variable in __mptcp_ofo_queue(). Initialize
   msk->copied_seq to 0 in mptcp_propagate_state() fallback path.
 - Patch 4: add map_seq64 comparison in __mptcp_try_coalesce() for 64-bit
   sequence consistency.
 - New patch 11: rename mptcp_check_readable to mptcp_stream_is_readable,
   and align with TCP by calling sk_is_readable() for KTLS support.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1790734083.git.tanggeliang@kylinos.cn/

v13:
 - use smp_mb instead of smp_mb__before_atomic in mptcp_check_data_fin.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1788245079.git.tanggeliang@kylinos.cn/

v12:
 - Move sock_set_flag(SOCK_DONE) to after smp_mb__before_atomic() in
   mptcp_check_data_fin() and reuse the existing barrier rather than adding
   a new smp_wmb().
 - Sample SOCK_DONE before smp_rmb() in mptcp_inq_hint() and mptcp_inq(),
   so the smp_rmb() only orders SOCK_DONE read relative to the subsequent
   ack_seq/copied_seq reads.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1788171508.git.tanggeliang@kylinos.cn/

v11:
 - __mptcp_move_skb(): gate __mptcp_sync_rcv_sequence() on
   test_and_clear_bit(MPTCP_SYNC_SEQ) so the dummy mapping is only re-based
   after ack_seq = iasn.
 - a new patch to set and test SOCK_DONE.
 - use SOCK_DONE in mptcp_inq_hint() and mptcp_inq().
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1788099847.git.tanggeliang@kylinos.cn/

v10:
 - Drop "mptcp: use atomic64_t for locklessly accessed u64 fields"
   (v9 patch 1) and the squash to bpf_burst test (v9 patch 11).
   Series reduced from 11 to 9 patches.
 - Revert all atomic64_read()/atomic64_set() on msk->ack_seq back to
   plain reads and WRITE_ONCE() in patches 1-4. Remove the local u64
   ack_seq variables that only existed to hold atomic64_read() results.
 - mptcp_inq_hint: move test_bit(MPTCP_SYNC_SEQ) inside mptcp_data_lock()
   to close the race between the flag check and the ack_seq read. Revert
   atomic64_read() to plain msk->ack_seq under the lock.
 - mptcp_inq (patch 6): use READ_ONCE() for both ack_seq and copied_seq
   instead of atomic64_read(). Add comment for the FIN subtraction,
   mirroring tcp_inq(). Update commit log.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787888115.git.tanggeliang@kylinos.cn/

v9:
 - Extend atomic64_t conversion from only msk->ack_seq to all locklessly
   accessed u64 fields: write_seq, snd_nxt, snd_una, wnd_end,
   bytes_received, bytes_consumed.
 - Add mptcp_data_lock() in mptcp_inq_hint() to read ack_seq and
   copied_seq atomically.
 - Add read_copied field and drain it under mptcp_data_lock() in
   mptcp_read_complete() to fix the BH vs worker data race.
 - New patches: mptcp_inq/peek_len and sendmsg_locked proto_ops.
 - Squash to "selftests/bpf: Add bpf_burst scheduler & test".
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787827525.git.tanggeliang@kylinos.cn/

v8:
 - check MPTCP_SYNC_SEQ in mptcp_inq_hint:
       if (hint_val >= INT_MAX) {
               if (test_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
                       return 0;
               return INT_MAX;
       }
 - rename mptcp_sock_rate_check_app_limited to
   mptcp_rate_check_app_limited.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787644449.git.tanggeliang@kylinos.cn/

v7:
 - Patch 1 is a new one to fix the existing issue Sashiko mentioned -
   using atomic64_t for msk->ack_seq.
 - Fix the map_subflow_seq setting in fallback mode in patch 4.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787537436.git.tanggeliang@kylinos.cn/

v6:
 - Memory ordering for MPTCP_SYNC_SEQ: Add smp_wmb() before set_bit() in
   subflow_set_remote_key() and smp_rmb() after test_and_clear_bit() in
   __mptcp_move_skb() and mptcp_release_cb() to prevent data races on
   weakly-ordered architectures, ensuring ack_seq updates are visible
   before the flag is set and readers see the updated sequence after
   observing the flag.
 - Lockdep nested locking: Use lock_sock_fast_nested(ssk) instead of
   lock_sock_fast(ssk) in mptcp_sock_rate_check_app_limited() to suppress
   false positive recursive locking warnings when acquiring subflow socket
   locks while holding the parent MPTCP socket lock.
 - Receive window advertisement order: Swap mptcp_rcv_space_adjust() and
   mptcp_cleanup_rbuf() in mptcp_read_complete() to expand the receive
   buffer before evaluating window updates, ensuring ACKs advertise the
   new (larger) window size instead of delaying the advertisement until
   the next cycle.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787446274.git.tanggeliang@kylinos.cn/

v5:
 - only patch 3 changed.
 - Fallback offset underflow: Initialize MPTCP sequence space to 0 in
   mptcp_propagate_state() when mp_opt == NULL, ensuring SKB's map_seq
   starts from 0 to match msk->copied_seq and prevent offset calculation
   underflow in fallback mode.
 - Stale peek_seq: Move peek_seq recomputation into the mptcp_move_skbs()
   continue path and add another recomputation after sk_wait_data(),
   ensuring peek_seq stays synchronized with msk->copied_seq updates from
   MPTCP_SYNC_SEQ processing in both paths.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787368526.git.tanggeliang@kylinos.cn/

v4:
 - mptcp_recvmsg: move the peek_seq recompute from after sk_wait_data()
   to before the mptcp_move_skbs() continue check.
   mptcp_move_skbs() -> __mptcp_move_skb() may shift the head skb's
   map_seq to the IASN frame via __mptcp_sync_rcv_sequence(); if
   'continue' then exits the loop, the recompute is skipped, and the
   next __mptcp_recvmsg_mskq() computes offset with peek_seq in the old
   frame minus map_seq in the new frame. The underflow makes the offset
   test fail and the skb is skipped (and mptcp_recv_skb() actually frees
   the TFO skb via mptcp_eat_recv_skb()).
 - Make all three MPTCP_SYNC_SEQ bit accesses atomic:
   - subflow_set_remote_key:   __set_bit            -> set_bit
   - __mptcp_move_skb:         __test_and_clear_bit -> test_and_clear_bit
   - mptcp_release_cb:         __test_and_clear_bit -> test_and_clear_bit
   The flag is set in BH under mptcp_data_lock() (msk slock held) and
   cleared in user context under lock_sock() slow path (only owned=1,
   slock not held). The non-atomic RMW on the two sides races, which can
   lose the IASN sync.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1787295147.git.tanggeliang@kylinos.cn/

v3:
 - Patch 2, handle "offset" in __mptcp_sync_rcv_sequence().
 - Patch 3, update __mptcp_move_skb() in response to Sashiko comments:

 	if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
		msk->copied_seq += mptcp_iasn(msk);

 - Patch 4, replace after64() to after() in mptcp_prune_ofo_queue(). The
   pre-existing issue raised by Sashiko regarding changing the type of
   ack_seq to atomic64_t is not addressed in this series.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1786445142.git.tanggeliang@kylinos.cn/

v2:
 - Patch 3, updated in response to Sashiko comments:

        if (unlikely(msk->rcvd_dummy_seq)) {
                msk->copied_seq += mptcp_iasn(msk);
                __mptcp_sync_rcv_sequence(sk);
                /* Release cb() would otherwise re-base copied_seq
                 * again.
                 */
                test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
        }

        /* Skip the already peeked data. */
        if (offset >= skb->len) {
                *last = skb;
                continue;
        }
 - Patch 5, replaced with Paolo's patch.
 - Patch 6, 7, new patches addressing app-limited conditions.
 - The patch "trim the duplicated skb head at receive enqueue" has been
   dropped, as it is no longer needed after Paolo updated the "mptcp:
   out-of-order queue pruning" series.
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1786158416.git.tanggeliang@kylinos.cn/

v1:
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1785150300.git.tanggeliang@kylinos.cn/

The goal of this series is to reduce the differences between TCP and MPTCP
for TLS usage, in preparation for adding TLS over MPTCP support in the
future.

In previous versions [1], a struct tls_prot_ops was defined to represent
the interface differences between TCP and MPTCP, which contained the
following callbacks:

	struct sk_buff *(*recv_skb)(struct sock *sk, u32 *off);
	bool (*lock_is_held)(struct sock *sk);
	void (*read_done)(struct sock *sk, size_t len);
	u32 (*get_skb_seq)(struct sk_buff *skb);
	int (*skb_get_header)(const struct sk_buff *skb, int offset,
	                      void *to, int len);
	bool (*epollin_ready)(const struct sock *sk);
	void (*check_app_limited)(struct sock *sk);

In reality, some of these callbacks are unnecessary. This series aims to
eliminate the get_skb_seq(), skb_get_header(), and lock_is_held()
callbacks.

The first four patches come from Paolo's "mptcp: address stall under memory
pressure" series v5 [2], with only minor cleanup from my side.

They remove the CB offset field and sync the MPTCP skb CB layout with the
TCP one, so that we can obtain the TCP or MPTCP sequence number in a
unified way, e.g.:

	struct tls_skb_cb {
	    u32 seq;
	};

	#define TLS_SKB_CB(__skb) ((struct tls_skb_cb *)&((__skb)->cb[0]))

This eliminates the need for a separate get_skb_seq() callback.

Building on the removal of the CB offset field, I also added patch 5 that
trims the duplicated skb head at receive enqueue. With that in place, KTLS
can retrieve the record header via skb_copy_bits() directly, so there is no
longer any need for a dedicated MPTCP helper like mptcp_skb_get_header().
The skb_get_header() callback can thus be removed.

Patch 6 defers sk_data_ready to the worker, which avoids recursive locking
when TLS calls back into MPTCP under mptcp_data_lock(). With this change,
the lock_is_held() callback is no longer needed and can be removed.

[1]
https://patchwork.kernel.org/project/mptcp/cover/cover.1782123118.git.tanggeliang@kylinos.cn/
[2]
https://patchwork.kernel.org/project/mptcp/cover/cover.1778446731.git.pabeni@redhat.com/

Gang Yan (1):
  mptcp: update mptcp_check_readable

Geliang Tang (5):
  mptcp: align FIN handling with TCP via SOCK_DONE
  mptcp: implement peek_len for proto_ops
  mptcp: add sendmsg_locked to proto_ops
  mptcp: track app-limited state in mptcp_sendmsg
  selftests: mptcp: sockopt: check app_limited

Paolo Abeni (5):
  mptcp: drop the mptcp_ooo_try_coalesce() helper
  mptcp: drop the cant_coalesce CB field
  mptcp: remove CB offset field
  mptcp: sync mptcp skb cb layout with tcp one
  mptcp: defer read_sock cleanup to mptcp_worker

 net/mptcp/fastopen.c                          |  17 +-
 net/mptcp/protocol.c                          | 365 ++++++++++++------
 net/mptcp/protocol.h                          |  21 +-
 net/mptcp/subflow.c                           |  23 ++
 .../selftests/net/mptcp/mptcp_sockopt.c       |   5 +
 5 files changed, 309 insertions(+), 122 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [PATCH mptcp-next v15 02/11] mptcp: drop the cant_coalesce CB field
@ 2026-10-24 16:26 netdev-bot+sashiko
  0 siblings, 0 replies; 19+ messages in thread
From: netdev-bot+sashiko @ 2026-10-24 16:26 UTC (permalink / raw)
  To: geliang; +Cc: mptcp, yangang, kuba

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

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

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] __mptcp_move_skb() (net/mptcp/protocol.c:468) runs
  `test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)` in process…
- [Medium] In this commit, the passive TFO dummy mapping is still wrong
  when user space reads before the IASN is known.
- [Low] The commit message says the patch trades the cant_coalesce check
  in __mptcp_try_coalesce() for 'a similar one in __mptcp_move_skb()'.
- [Low] The concern is that the commit message does not disclose the
  user-visible change to mptcp_inq_hint() (TCP_INQ cmsg / SIOCINQ) for…
- [Low] The new per-connection flag msk->rcvd_dummy_seq is set only in
  mptcp_fastopen_subflow_synack_set_params() (fastopen.c:58).
- [Low] __mptcp_sync_rcv_sequence() is documented with '/* Caller must
  hold the msk data lock */' (protocol.c:439), but __mptcp_move_skb()…

--

mptcp: drop the cant_coalesce CB field

This patch removes the cant_coalesce skb CB field, which kept the passive
TFO skb from being coalesced. Instead, the TFO skb sequence numbers are
rewritten once the IASN is known at 3rd ack time. If the msk is owned by
the user at that point, the sync is deferred through a new MPTCP_SYNC_SEQ
cb_flags bit that __mptcp_move_skb() and mptcp_release_cb() consume.

> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index f717750906ffa..e18b31028e7e3 100644
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c

[ ... ]

> @@ -49,15 +50,16 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
>  	MPTCP_SKB_CB(skb)->end_seq = 0;
>  	MPTCP_SKB_CB(skb)->offset = 0;
>  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
> -	MPTCP_SKB_CB(skb)->cant_coalesce = 1;
>  
>  	mptcp_data_lock(sk);
>  	DEBUG_NET_WARN_ON_ONCE(sock_owned_by_user_nocheck(sk));
>  
> +	msk = mptcp_sk(sk);
> +	msk->rcvd_dummy_seq = true;

[Severity: Low]
This is the only place rcvd_dummy_seq is set. The only place it is cleared
is __mptcp_sync_rcv_sequence(), and that needs subflow_set_remote_key() to
run first. Suppose the passive TFO connection falls back at the 3rd ack
in check_fully_established():

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

The remote key never arrives, so the flag stays true.

Should mptcp_disconnect() also reset rcvd_dummy_seq? It resets cb_flags,
can_ack, fully_established, bytes_received, ack_seq and other
per-connection state, but not the new field.

__mptcp_init_sock() and mptcp_sk_clone_init() don't reset it either. So
after connect(AF_UNSPEC) and then listen(), sk_clone_lock() copies the
stale value into every child.

Today this looks harmless: on non-TFO connections the receive queue is
empty at every sync point, so the helper only clears the flag. Still,
__mptcp_sync_rcv_sequence() takes the flag to mean the queue tail is the
TFO skb.

At the end of the series, mptcp_disconnect() resets tfo_skb_len and
copied_seq, but rcvd_dummy_seq is still not reset.

>  	mptcp_borrow_fwdmem(sk, skb);

[ ... ]

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 528a72da646cc..96deaed109a2f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -438,6 +436,27 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
>  	skb_dst_drop(skb);
>  }
>  
> +/* Caller must hold the msk data lock */
> +void __mptcp_sync_rcv_sequence(struct sock *sk)

[Severity: Low]
Is this comment accurate? __mptcp_move_skb() calls this helper from paths
where the data lock has already been dropped:

mptcp_move_skbs()
    mptcp_data_unlock(sk);
    __mptcp_move_skbs()
        __mptcp_move_skb()
            __mptcp_sync_rcv_sequence()

It does the same from the mptcp_release_cb() loop, after
spin_unlock_bh(&sk->sk_lock.slock).

On those paths the helper reads and writes msk->rcvd_dummy_seq and
rewrites the CB of the receive queue tail while holding only socket
ownership. That seems safe in practice, because BH writers don't touch
the receive queue while the msk is owned. The comment still doesn't
match the callers.

Would "caller must hold the msk data lock or own the msk socket" be more
accurate? The same comment and caller layout are still present at the
end of the series.

[ ... ]

> @@ -446,6 +465,17 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>  
>  	mptcp_borrow_fwdmem(sk, skb);
>  
> +	if (test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {

[Severity: High]
Can this lockless atomic clear race with the non-atomic cb_flags writers?

Here __mptcp_move_skb() runs in process context without the msk data
lock. It is reached from mptcp_move_skbs() after mptcp_data_unlock(),
and from the mptcp_release_cb() loop after spin_unlock_bh().

The other cb_flags writers use non-atomic RMWs under the data lock while
the msk is owned, for example:

mptcp_propagate_state()
    __set_bit(MPTCP_SYNC_STATE, &msk->cb_flags);

__mptcp_data_acked()
    __set_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->cb_flags);

The MPTCP_ERROR_REPORT, MPTCP_PUSH_PENDING, MPTCP_RETRANSMIT and
MPTCP_FLUSH_JOIN_LIST writers work the same way.

Before this patch, every cb_flags change except the owner-side reset in
mptcp_disconnect() happened under the data lock. So the __ ops had the
word to themselves.

One possible interleaving:

CPU1 (BH, data lock held)          CPU2 (owner, no data lock)
__set_bit(X): load cb_flags
  (MPTCP_SYNC_SEQ set)
                                   test_and_clear_bit(MPTCP_SYNC_SEQ)
store old | X
  (MPTCP_SYNC_SEQ set again)

Once the bit is back, mptcp_inq_hint() takes the new early return and
reports only the head skb length for TCP_INQ and SIOCINQ.

Some architectures (parisc, sparc32) implement atomic bitops with hashed
spinlocks. There the BH bit could be lost instead, which would drop a
deferred push, error report, state sync or clean_una.

At the end of the series, both places that consume MPTCP_SYNC_SEQ, in
__mptcp_move_skb() and mptcp_release_cb(), do:

	msk->copied_seq += mptcp_iasn(msk);

A resurrected bit then adds the IASN twice. mptcp_recv_skb() would
compute an offset past skb->len and could discard queued skbs.
__mptcp_recvmsg_mskq() would compute a bogus offset and data_len.

Could MPTCP_SYNC_SEQ be consumed under the data lock instead, for example
in mptcp_can_spool_backlog() or before the lock is dropped? Another
option would be to keep this state outside the shared cb_flags word.

> +		/* Ensure we see the updated ack_seq after seeing the flag */
> +		smp_rmb();
> +
> +		/* Be sure to sync the eventual fastopen dummy mapping before
> +		 * any other skb lands into the msk.
> +		 */
> +		if (unlikely(msk->rcvd_dummy_seq))
> +			__mptcp_sync_rcv_sequence(sk);
> +	}

[Severity: Low]
The commit message says:

  Performance wise this trades a conditional in the fast path - in
  __mptcp_try_coalesce() - with a similar one in __mptcp_move_skb()

Is the new check really similar? The test_and_clear_bit() above is an
unconditional, fully ordered atomic RMW on the shared cb_flags word. It
runs for every skb moved into the msk, on both the
__mptcp_move_skbs_from_subflow() path and the backlog __mptcp_move_skbs()
path. The old check was a plain read of a byte in the cache-hot skb->cb.

subflow_set_remote_key() also sets the bit whenever the msk is owned by
the user, even when rcvd_dummy_seq is false:

	} else {
		/* Ensure ack_seq is visible before setting the flag */
		smp_wmb();
		set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
	}

So non-TFO connections take this path too.

Would doing a test_bit() before the test_and_clear_bit(), or checking
rcvd_dummy_seq first, match the description better? The atomic RMW is
still there at the end of the series.

[ ... ]

> @@ -2394,11 +2424,14 @@ static unsigned int mptcp_inq_hint(const struct sock *sk)
>  {
>  	const struct mptcp_sock *msk = mptcp_sk(sk);
>  	const struct sk_buff *skb;
> +	u64 hint_val;
>  
>  	skb = skb_peek(&sk->sk_receive_queue);
>  	if (skb) {
> -		u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
> +		if (test_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
> +			return skb->len - MPTCP_SKB_CB(skb)->offset;

[Severity: Low]
This isn't a bug, but this changes what user space sees through the
TCP_INQ cmsg and SIOCINQ on passive TFO sockets. The commit message only
covers it indirectly ("the user-space will always observe consistent
sequence numbers in the receive queue"), and there is no Fixes: tag.

Would it be worth describing the mptcp_inq_hint() change in the commit
message? The later commit "mptcp: remove CB offset field" in this series
does document the inq_hint behaviour explicitly. Since this is net-next
refactoring, leaving out the Fixes: tag may be intentional.

>  
> +		hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
>  		if (hint_val >= INT_MAX)
>  			return INT_MAX;

[Severity: Medium]
Can a passive TFO skb still reach this INT_MAX return before the 3rd
ack?

mptcp_fastopen_subflow_synack_set_params() still does:

	MPTCP_SKB_CB(skb)->map_seq = -skb->len;

skb->len is an unsigned int, so this negates in 32 bits and then
zero-extends into the u64 map_seq. The result is 2^32 - len, not
-(u64)len.

Until the 3rd ack, msk->ack_seq is 0. MPTCP_SYNC_SEQ is only set by
subflow_set_remote_key(), so the early return above is not taken. The
code then computes 0 - (2^32 - len) and returns INT_MAX. That value
reaches TCP_CM_INQ, SIOCINQ through mptcp_ioctl(), and
mptcp_rcv_space_adjust().

A second case is a 3rd ack without MPC, which makes the connection fall
back in check_fully_established(). subflow_set_remote_key() never runs,
so rcvd_dummy_seq is never cleared.

Fallback data maps to map_seq 0, which equals ack_seq. With the
cant_coalesce check gone, __mptcp_try_coalesce() merges that data into
the TFO skb, whose end_seq is 0:

	if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
	    MPTCP_SKB_CB(from)->offset ||

The bogus map_seq then covers more data, so the INT_MAX result applies
to more data.

The INT_MAX value before the 3rd ack already existed before this patch.
The extra coverage after fallback is new here. Both cases seem to
conflict with the commit message:

  After this change the user-space will always observe consistent
  sequence numbers in the receive queue, even in the TFO dummy mapping
  case.

The later commit "mptcp: remove CB offset field" in this series seems to
fix both cases. It sets map_seq64 to -(u64)skb->len and computes the hint
as ack_seq - copied_seq.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792718041.git.tanggeliang%40kylinos.cn

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-10-24 16:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23  1:58 [PATCH mptcp-next v15 00/11] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 01/11] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 02/11] mptcp: drop the cant_coalesce CB field Geliang Tang
2026-10-23  2:35   ` sashiko-bot
2026-10-23  1:58 ` [PATCH mptcp-next v15 03/11] mptcp: remove CB offset field Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 04/11] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
2026-10-23  2:35   ` sashiko-bot
2026-10-23  1:58 ` [PATCH mptcp-next v15 05/11] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
2026-10-23  2:29   ` sashiko-bot
2026-10-23  1:58 ` [PATCH mptcp-next v15 06/11] mptcp: align FIN handling with TCP via SOCK_DONE Geliang Tang
2026-10-23  2:29   ` sashiko-bot
2026-10-23  1:58 ` [PATCH mptcp-next v15 07/11] mptcp: implement peek_len for proto_ops Geliang Tang
2026-10-23  2:19   ` sashiko-bot
2026-10-23  1:58 ` [PATCH mptcp-next v15 08/11] mptcp: add sendmsg_locked to proto_ops Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 09/11] mptcp: track app-limited state in mptcp_sendmsg Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 10/11] selftests: mptcp: sockopt: check app_limited Geliang Tang
2026-10-23  1:58 ` [PATCH mptcp-next v15 11/11] mptcp: update mptcp_check_readable Geliang Tang
2026-10-23  3:11 ` [PATCH mptcp-next v15 00/11] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
  -- strict thread matches above, loose matches on Subject: below --
2026-10-24 16:26 [PATCH mptcp-next v15 02/11] mptcp: drop the cant_coalesce CB field 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.