* [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage
@ 2026-10-30 8:41 Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 1/5] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
v18:
- Re-add patch 2 "mptcp: drop the cant_coalesce CB field".
- Patch 3: simplify TFO sync by relying on patch 2's __mptcp_sync_rcv_sequence()
instead of complex copied_seq checks. Use copied_seq += mptcp_iasn(msk).
- Patch 4: move subflow.c changes to patch 2, use map_seq64 for TFO init.
v17:
- Patch 2: add TFO synchronization: subflow_set_remote_key() preserves
partial read progress by checking copied_seq against the initial TFO
value before updating. Add WRITE_ONCE/READ_ONCE for ordering between
BH (3rd ACK processing) and user-space readers. Add underflow guard
(copied_seq >= map_seq) before offset subtraction in
__mptcp_recvmsg_mskq() to protect MSG_PEEK traversals. Skip skbs
entirely before copied_seq. Update subflow.c to set both map_seq
(u32) and map_seq64 (u64) for TFO.
- Patch 3: adapt to the new map_seq (u32) + map_seq64 (u64) layout
introduced in patch 2.
- https://patchwork.kernel.org/project/mptcp/cover/cover.1793326853.git.tanggeliang@kylinos.cn/
v16:
- Simplified the series to retain only the core changes required by both
kTLS and sockmap: drop the CB offset field, add copied_seq tracking,
and sync the MPTCP skb CB layout with the TCP one.
- Dropped patch 2, leaving the fastopen code unchanged.
- Moved patches 6-11 into a separate series "prepare interfaces for kTLS
and sockmap support".
- https://patchwork.kernel.org/project/mptcp/cover/cover.1793255126.git.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().
- https://patchwork.kernel.org/project/mptcp/cover/cover.1792718041.git.tanggeliang@kylinos.cn/
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/
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 | 261 +++++++++++++++++++++++++------------------
net/mptcp/protocol.h | 21 +++-
net/mptcp/subflow.c | 18 ++-
4 files changed, 199 insertions(+), 118 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH mptcp-next v18 1/5] mptcp: drop the mptcp_ooo_try_coalesce() helper
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
@ 2026-10-30 8:41 ` Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field Geliang Tang
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
It's used to save an additional comparison for in-order skbs, but is
also a barrier to remove CB offset. Remove the helper, let
__mptcp_try_coalesce() always perform the sequence check and remove
duplicate checks from the callers.
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 02a4f479d9d8..528a72da646c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -167,7 +167,8 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
{
int limit = READ_ONCE(sk->sk_rcvbuf);
- if (unlikely(MPTCP_SKB_CB(to)->cant_coalesce) ||
+ if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
+ unlikely(MPTCP_SKB_CB(to)->cant_coalesce) ||
MPTCP_SKB_CB(from)->offset ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
@@ -200,15 +201,6 @@ static bool mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
return true;
}
-static bool mptcp_ooo_try_coalesce(struct mptcp_sock *msk, struct sk_buff *to,
- struct sk_buff *from)
-{
- if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq)
- return false;
-
- return mptcp_try_coalesce((struct sock *)msk, to, from);
-}
-
/* "inspired" by tcp_rcvbuf_grow(), main difference:
* - mptcp does not maintain a msk-level window clamp
* - returns true when the receive buffer is actually updated
@@ -348,7 +340,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
/* with 2 subflows, adding at end of ooo queue is quite likely
* Use of ooo_last_skb avoids the O(Log(N)) rbtree lookup.
*/
- if (mptcp_ooo_try_coalesce(msk, msk->ooo_last_skb, skb)) {
+ if (mptcp_try_coalesce(sk, msk->ooo_last_skb, skb)) {
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOMERGE);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOQUEUETAIL);
return;
@@ -394,7 +386,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
goto merge_right;
}
- } else if (mptcp_ooo_try_coalesce(msk, skb1, skb)) {
+ } else if (mptcp_try_coalesce(sk, skb1, skb)) {
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOMERGE);
return;
}
@@ -777,8 +769,7 @@ static void __mptcp_add_backlog(struct sock *sk,
if (!list_empty(&msk->backlog_list))
tail = list_last_entry(&msk->backlog_list, struct sk_buff, list);
- if (tail && MPTCP_SKB_CB(skb)->map_seq == MPTCP_SKB_CB(tail)->end_seq &&
- ssk == tail->sk &&
+ if (tail && ssk == tail->sk &&
__mptcp_try_coalesce(sk, tail, skb, &fragstolen, &delta)) {
skb->truesize -= delta;
kfree_skb_partial(skb, fragstolen);
@@ -902,7 +893,7 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
end_seq = MPTCP_SKB_CB(skb)->end_seq;
tail = skb_peek_tail(&sk->sk_receive_queue);
- if (!tail || !mptcp_ooo_try_coalesce(msk, tail, skb)) {
+ if (!tail || !mptcp_try_coalesce(sk, tail, skb)) {
int delta = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
/* skip overlapping data, if any */
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 1/5] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
@ 2026-10-30 8:41 ` Geliang Tang
2026-10-30 9:02 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field Geliang Tang
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
Such field is used to ensure in-sequence processing in case of fastopen.
Instead let's perform synchronization of the fastopen skb sequence
when the IASN becomes available with the 3rd ack.
When the `cant_coalesce` field has been introduced, commit f03afb3aeb9d
("mptcp: drop __mptcp_fastopen_gen_msk_ackseq()") noted that updating the
already queued skb for passive fastopen socket at 3rd ack time would be
difficult and race prone. The main point is that such update don't need
to be synchronously performed at 3rd ack time, but is sufficient to
perform it before the next segment is introduced into the msk.
To such extent, add an explicit test in __mptcp_move_skb(). Performance
wise this trades a conditional in the fast path - in __mptcp_try_coalesce()
- with a similar one in __mptcp_move_skb() and a couple more in slow paths.
After this change the user-space will always observe consistent sequence
numbers in the receive queue, even in the TFO dummy mapping case.
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 6 ++++--
net/mptcp/protocol.c | 31 +++++++++++++++++++++++++++++--
net/mptcp/protocol.h | 4 +++-
net/mptcp/subflow.c | 7 +++++++
4 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index f717750906ff..e18b31028e7e 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -9,6 +9,7 @@
void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subflow,
struct request_sock *req)
{
+ struct mptcp_sock *msk;
struct sock *sk, *ssk;
struct sk_buff *skb;
struct tcp_sock *tp;
@@ -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;
mptcp_borrow_fwdmem(sk, skb);
skb_set_owner_r(skb, sk);
__skb_queue_tail(&sk->sk_receive_queue, skb);
- mptcp_sk(sk)->bytes_received += skb->len;
+ msk->bytes_received += skb->len;
sk->sk_data_ready(sk);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 528a72da646c..fe154b181683 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,7 +168,6 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
int limit = READ_ONCE(sk->sk_rcvbuf);
if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
- unlikely(MPTCP_SKB_CB(to)->cant_coalesce) ||
MPTCP_SKB_CB(from)->offset ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
@@ -430,7 +429,6 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + copy_len;
MPTCP_SKB_CB(skb)->offset = offset;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
- MPTCP_SKB_CB(skb)->cant_coalesce = 0;
__skb_unlink(skb, &ssk->sk_receive_queue);
@@ -438,6 +436,26 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
skb_dst_drop(skb);
}
+void __mptcp_sync_rcv_sequence(struct sock *sk)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ struct sk_buff *skb;
+ u32 offset;
+
+ if (likely(!msk->rcvd_dummy_seq))
+ return;
+
+ /* User space can have already received the TFO skb. */
+ msk->rcvd_dummy_seq = false;
+ skb = skb_peek_tail(&sk->sk_receive_queue);
+ if (!skb)
+ return;
+
+ offset = MPTCP_SKB_CB(skb)->offset;
+ MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len + offset;
+ MPTCP_SKB_CB(skb)->end_seq = msk->ack_seq;
+}
+
static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
{
u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
@@ -446,6 +464,12 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
mptcp_borrow_fwdmem(sk, skb);
+ /* 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);
+
if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
/* in sequence */
insert:
@@ -3680,6 +3704,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->bytes_retrans = 0;
msk->rcvspace_init = 0;
msk->fastclosing = 0;
+ msk->rcvd_dummy_seq = false;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
@@ -3930,6 +3955,8 @@ static void mptcp_release_cb(struct sock *sk)
__mptcp_error_report(sk);
if (__test_and_clear_bit(MPTCP_SYNC_SNDBUF, &msk->cb_flags))
__mptcp_sync_sndbuf(sk);
+ if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
+ __mptcp_sync_rcv_sequence(sk);
}
}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3be60da12924..c317d8b59fff 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -126,13 +126,13 @@
#define MPTCP_FLUSH_JOIN_LIST 5
#define MPTCP_SYNC_STATE 6
#define MPTCP_SYNC_SNDBUF 7
+#define MPTCP_SYNC_SEQ 8
struct mptcp_skb_cb {
u64 map_seq;
u64 end_seq;
u32 offset;
u8 has_rxtstamp;
- u8 cant_coalesce;
};
#define MPTCP_SKB_CB(__skb) ((struct mptcp_skb_cb *)&((__skb)->cb[0]))
@@ -336,6 +336,7 @@ struct mptcp_sock {
bool use_64bit_ack; /* Set when we received a 64-bit DSN */
bool csum_enabled;
bool allow_infinite_fallback;
+ bool rcvd_dummy_seq;
u8 pending_state; /* A subflow asked to set this sk_state,
* protected by the msk data lock
*/
@@ -1193,6 +1194,7 @@ void mptcp_event_pm_listener(const struct sock *ssk,
enum mptcp_event_type event);
bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
+void __mptcp_sync_rcv_sequence(struct sock *sk);
void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subflow,
struct request_sock *req);
int mptcp_pm_genl_fill_addr(struct sk_buff *msg,
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 78e886b20258..49a000b8dcb8 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -515,6 +515,8 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
struct mptcp_subflow_context *subflow,
const struct mptcp_options_received *mp_opt)
{
+ struct sock *sk = (struct sock *)msk;
+
/* active MPC subflow will reach here multiple times:
* at subflow_finish_connect() time and at 4th ack time
*/
@@ -533,6 +535,11 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
WRITE_ONCE(msk->ack_seq, subflow->iasn);
WRITE_ONCE(msk->can_ack, true);
atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
+
+ if (!sock_owned_by_user(sk))
+ __mptcp_sync_rcv_sequence(sk);
+ else
+ set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
}
static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 1/5] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field Geliang Tang
@ 2026-10-30 8:41 ` Geliang Tang
2026-10-30 9:18 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
Instead, use a new msk-level field to track the bytes already consumed
inside each skb, carrying the amount of bytes already copied to
user-space, alike what TCP is already doing.
The newly introduce `copied_seq` field is always accessed under the msk
socket lock, delegating the synchronization with IASN to the msk release
CB, when the socket is owned by the user-space at remote key reception
time. Such synchronization preserves any partial progress (copy) made on
the TFO packet.
Note that the explicit synchronization in __mptcp_move_skb() is needed to
ensure that the TFO skb in the receive queue got its map_seq synched
before the next skb lands into the receive queue when spooling the backlog
at mptcp_release_cb() time, as the release CB synchronization will happen
later.
Prior to this patch, the TFO skb dummy mapping (map_seq = -(u32)skb->len,
zero-extended to u64) caused mptcp_inq_hint() to compute a wrapped
ack_seq - map_seq, so SIOCINQ and TCP_CM_INQ returned INT_MAX before the
MPC ACK. This patch fixes that behaviour through both the -(u64)skb->len
sign fix and the new ack_seq - copied_seq hint.
Overall this simplifies a bit the __mptcp_recvmsg_mskq(), mptcp_inq_hint()
and the __mptcp_move_skb() code and will also make possible the next
patch.
Initialize MPTCP sequence space to 0 in mptcp_propagate_state() when mp_opt
is NULL, ensuring SKB's map_seq starts from 0 to match msk->copied_seq and
prevent offset calculation underflow in fallback mode. The writes are safe
without sock_owned_by_user() deferral because mptcp_propagate_state()
already holds mptcp_data_lock().
Note: mptcp_iasn() assumes ack_seq and bytes_received advance in lockstep.
If mptcp_check_data_fin() increments ack_seq for a DATA_FIN before the
deferred SYNC_SEQ sync runs, copied_seq ends up one byte past the real
IASN. In practice this requires a DATA_FIN to arrive before the MPC third
ACK on a passive TFO socket, which cannot happen because the connection
is not yet established.
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 9 ++--
net/mptcp/protocol.c | 102 +++++++++++++++++++------------------------
net/mptcp/protocol.h | 8 +++-
net/mptcp/subflow.c | 15 +++++--
4 files changed, 70 insertions(+), 64 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index e18b31028e7e..421a50a85547 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -45,10 +45,11 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
subflow->ssn_offset += skb->len;
has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
- /* Only the sequence delta is relevant */
- MPTCP_SKB_CB(skb)->map_seq = -skb->len;
+ /* The TFO segment data sits before the IASN; before receiving
+ * the remote key, IASN is assumed being 0.
+ */
+ MPTCP_SKB_CB(skb)->map_seq = -(u64)skb->len;
MPTCP_SKB_CB(skb)->end_seq = 0;
- MPTCP_SKB_CB(skb)->offset = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
mptcp_data_lock(sk);
@@ -56,6 +57,8 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
msk = mptcp_sk(sk);
msk->rcvd_dummy_seq = true;
+ msk->copied_seq = MPTCP_SKB_CB(skb)->map_seq;
+ msk->tfo_skb_len = skb->len;
mptcp_borrow_fwdmem(sk, skb);
skb_set_owner_r(skb, sk);
__skb_queue_tail(&sk->sk_receive_queue, skb);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index fe154b181683..792147b5279f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,7 +168,6 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
int limit = READ_ONCE(sk->sk_rcvbuf);
if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
- MPTCP_SKB_CB(from)->offset ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
return false;
@@ -415,8 +414,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
skb_set_owner_r(skb, sk);
}
-static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
- int copy_len)
+static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
@@ -425,9 +423,9 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
* mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
* value
*/
- MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
- MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + copy_len;
- MPTCP_SKB_CB(skb)->offset = offset;
+ MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow) -
+ offset;
+ MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
__skb_unlink(skb, &ssk->sk_receive_queue);
@@ -440,7 +438,6 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct sk_buff *skb;
- u32 offset;
if (likely(!msk->rcvd_dummy_seq))
return;
@@ -451,9 +448,8 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
if (!skb)
return;
- offset = MPTCP_SKB_CB(skb)->offset;
- MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len + offset;
- MPTCP_SKB_CB(skb)->end_seq = msk->ack_seq;
+ MPTCP_SKB_CB(skb)->map_seq = mptcp_iasn(msk) - skb->len;
+ MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
}
static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
@@ -496,10 +492,6 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
/* Partial packet */
if (after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
copy_len = MPTCP_SKB_CB(skb)->end_seq - msk->ack_seq;
- MPTCP_SKB_CB(skb)->offset += msk->ack_seq -
- MPTCP_SKB_CB(skb)->map_seq;
- MPTCP_SKB_CB(skb)->map_seq += msk->ack_seq -
- MPTCP_SKB_CB(skb)->map_seq;
goto insert;
}
@@ -857,7 +849,7 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
if (offset < skb->len) {
size_t len = skb->len - offset;
- mptcp_init_skb(ssk, skb, offset, len);
+ mptcp_init_skb(ssk, skb, offset);
if (own_msk) {
mptcp_subflow_lend_fwdmem(subflow, skb);
@@ -920,12 +912,9 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
if (!tail || !mptcp_try_coalesce(sk, tail, skb)) {
int delta = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
- /* skip overlapping data, if any */
pr_debug("uncoalesced seq=%llx ack seq=%llx delta=%d\n",
MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq,
delta);
- MPTCP_SKB_CB(skb)->offset += delta;
- MPTCP_SKB_CB(skb)->map_seq += delta;
__skb_queue_tail(&sk->sk_receive_queue, skb);
}
msk->bytes_received += end_seq - msk->ack_seq;
@@ -2209,33 +2198,24 @@ static void mptcp_eat_recv_skb(struct sock *sk, struct sk_buff *skb)
}
static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
- size_t len, int flags, int copied_total,
+ size_t len, int flags, u64 *seq,
struct scm_timestamping_internal *tss,
int *cmsg_flags, struct sk_buff **last)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct sk_buff *skb, *tmp;
- int total_data_len = 0;
int copied = 0;
skb_queue_walk_safe(&sk->sk_receive_queue, skb, tmp) {
- u32 delta, offset = MPTCP_SKB_CB(skb)->offset;
+ u64 offset = *seq - MPTCP_SKB_CB(skb)->map_seq;
u32 data_len = skb->len - offset;
u32 count;
int err;
- if (flags & MSG_PEEK) {
- /* skip already peeked skbs */
- if (total_data_len + data_len <= copied_total) {
- total_data_len += data_len;
- *last = skb;
- continue;
- }
-
- /* skip the already peeked data in the current skb */
- delta = copied_total - total_data_len;
- offset += delta;
- data_len -= delta;
+ /* Skip the already peeked data. */
+ if (offset >= skb->len) {
+ *last = skb;
+ continue;
}
count = min_t(size_t, len - copied, data_len);
@@ -2254,14 +2234,12 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
}
copied += count;
+ *seq += count;
if (!(flags & MSG_PEEK)) {
msk->bytes_consumed += count;
- if (count < data_len) {
- MPTCP_SKB_CB(skb)->offset += count;
- MPTCP_SKB_CB(skb)->map_seq += count;
+ if (count < data_len)
break;
- }
mptcp_eat_recv_skb(sk, skb);
} else {
@@ -2417,22 +2395,17 @@ static bool mptcp_move_skbs(struct sock *sk)
static unsigned int mptcp_inq_hint(const struct sock *sk)
{
const struct mptcp_sock *msk = mptcp_sk(sk);
- const struct sk_buff *skb;
-
- skb = skb_peek(&sk->sk_receive_queue);
- if (skb) {
- u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
+ u64 hint_val;
- if (hint_val >= INT_MAX)
- return INT_MAX;
+ hint_val = READ_ONCE(msk->ack_seq) - READ_ONCE(msk->copied_seq);
+ if (hint_val >= INT_MAX)
+ return INT_MAX;
- return (unsigned int)hint_val;
- }
-
- if (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN))
+ if (!hint_val &&
+ (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN)))
return 1;
- return 0;
+ return (unsigned int)hint_val;
}
static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
@@ -2441,6 +2414,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
struct mptcp_sock *msk = mptcp_sk(sk);
struct scm_timestamping_internal tss;
int copied = 0, cmsg_flags = 0;
+ u64 peek_seq, *seq;
int target;
long timeo;
@@ -2459,6 +2433,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
len = min_t(size_t, len, INT_MAX);
target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);
+ seq = &msk->copied_seq;
+ if (flags & MSG_PEEK) {
+ peek_seq = msk->copied_seq;
+ seq = &peek_seq;
+ }
if (unlikely(msk->recvmsg_inq))
cmsg_flags = MPTCP_CMSG_INQ;
@@ -2468,7 +2447,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int err, bytes_read;
bytes_read = __mptcp_recvmsg_mskq(sk, msg, len - copied, flags,
- copied, &tss, &cmsg_flags,
+ seq, &tss, &cmsg_flags,
&last);
if (unlikely(bytes_read < 0)) {
if (!copied)
@@ -2478,8 +2457,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
copied += bytes_read;
- if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk))
+ if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk)) {
+ if (flags & MSG_PEEK)
+ peek_seq = msk->copied_seq + copied;
continue;
+ }
/* only the MPTCP socket status is relevant here. The exit
* conditions mirror closely tcp_recvmsg()
@@ -2522,6 +2504,10 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
copied = copied ? : err;
goto out_err;
}
+
+ /* Recompute peek offset after eventual seq resync. */
+ if (flags & MSG_PEEK)
+ peek_seq = msk->copied_seq + copied;
}
mptcp_cleanup_rbuf(msk, copied);
@@ -3698,6 +3684,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
mptcp_pm_data_reset(msk);
mptcp_ca_reset(sk);
msk->bytes_consumed = 0;
+ msk->copied_seq = 0;
msk->bytes_acked = 0;
msk->bytes_received = 0;
msk->bytes_sent = 0;
@@ -3705,6 +3692,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->rcvspace_init = 0;
msk->fastclosing = 0;
msk->rcvd_dummy_seq = false;
+ msk->tfo_skb_len = 0;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
@@ -3955,8 +3943,10 @@ static void mptcp_release_cb(struct sock *sk)
__mptcp_error_report(sk);
if (__test_and_clear_bit(MPTCP_SYNC_SNDBUF, &msk->cb_flags))
__mptcp_sync_sndbuf(sk);
- if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
+ if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
+ msk->copied_seq += mptcp_iasn(msk);
__mptcp_sync_rcv_sequence(sk);
+ }
}
}
@@ -4628,7 +4618,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
mptcp_move_skbs(sk);
while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
- offset = MPTCP_SKB_CB(skb)->offset;
+ offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
if (offset < skb->len) {
*off = offset;
return skb;
@@ -4670,11 +4660,9 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
copied += count;
msk->bytes_consumed += count;
- if (count < data_len) {
- MPTCP_SKB_CB(skb)->offset += count;
- MPTCP_SKB_CB(skb)->map_seq += count;
+ msk->copied_seq += count;
+ if (count < data_len)
break;
- }
mptcp_eat_recv_skb(sk, skb);
if (!desc->count)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index c317d8b59fff..985ef15275eb 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -131,7 +131,6 @@
struct mptcp_skb_cb {
u64 map_seq;
u64 end_seq;
- u32 offset;
u8 has_rxtstamp;
};
@@ -311,6 +310,7 @@ struct mptcp_sock {
u64 rcv_data_fin_seq;
u64 bytes_retrans;
u64 bytes_consumed;
+ u64 copied_seq;
int snd_burst;
int old_wspace;
u64 recovery_snd_nxt; /* in recovery mode accept up to this seq;
@@ -325,6 +325,7 @@ struct mptcp_sock {
u32 last_ack_recv;
unsigned long timer_ival;
u32 token;
+ u32 tfo_skb_len;
unsigned long flags;
unsigned long cb_flags;
bool recovery; /* closing subflow write queue reinjected */
@@ -884,6 +885,11 @@ struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk);
int mptcp_sched_get_send(struct mptcp_sock *msk);
int mptcp_sched_get_retrans(struct mptcp_sock *msk);
+static inline u64 mptcp_iasn(const struct mptcp_sock *msk)
+{
+ return msk->ack_seq - msk->bytes_received + msk->tfo_skb_len;
+}
+
static inline u64 mptcp_data_avail(const struct mptcp_sock *msk)
{
return READ_ONCE(msk->bytes_received) - READ_ONCE(msk->bytes_consumed);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 49a000b8dcb8..899ffaba0b2b 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -536,10 +536,12 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
WRITE_ONCE(msk->can_ack, true);
atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
- if (!sock_owned_by_user(sk))
+ if (!sock_owned_by_user(sk)) {
+ msk->copied_seq += subflow->iasn;
__mptcp_sync_rcv_sequence(sk);
- else
+ } else {
set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
+ }
}
static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
@@ -556,6 +558,13 @@ static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
WRITE_ONCE(msk->snd_una, subflow->idsn + 1);
WRITE_ONCE(msk->wnd_end, subflow->idsn + 1 + tcp_sk(ssk)->snd_wnd);
subflow_set_remote_key(msk, subflow, mp_opt);
+ } else {
+ /* Fallback (no MPTCP options at SYN-ACK) */
+ subflow->map_seq = 0;
+ subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq -
+ subflow->ssn_offset;
+ WRITE_ONCE(msk->ack_seq, 0);
+ msk->copied_seq = 0;
}
if (!sock_owned_by_user(sk)) {
@@ -1492,7 +1501,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
skb = skb_peek(&ssk->sk_receive_queue);
subflow->map_valid = 1;
subflow->map_data_len = skb->len;
- subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
+ subflow->map_subflow_seq = TCP_SKB_CB(skb)->seq - subflow->ssn_offset;
subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
subflow->iasn +
TCP_SKB_CB(skb)->seq -
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (2 preceding siblings ...)
2026-10-30 8:41 ` [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field Geliang Tang
@ 2026-10-30 8:41 ` Geliang Tang
2026-10-30 9:19 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
2026-10-30 10:45 ` [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
5 siblings, 1 reply; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
The MPTCP protocol uses a significantly different CB layout WRT TCP, as it
includes different information and use 64 bits for the sequence numbers.
As the msk-level rcvbuf buffer size is limited by the core socket code the
INT_MAX; after validating the incoming skb vs the current receive window,
we can safely use 32 bits for MPTCP-level sequence number. This allow
updating the MPTCP CB layout so that fields with a corresponding TCP-level
data use the same area inside the CB itself.
The 32-bit sequence numbers are safe because all SKBs in the receive queue
and OFO queue have been validated against the receive window (max INT_MAX),
so their sequence numbers are guaranteed to be within 2^31 of the current
ack_seq or copied_seq. This ensures that 32-bit arithmetic (comparisons
and subtractions) produces correct results even when 64-bit values cross
the 2^32 boundary, as the relative distance remains small.
Add build time check to ensure the latter invariant.
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 6 ++-
net/mptcp/protocol.c | 92 +++++++++++++++++++++++++++-----------------
net/mptcp/protocol.h | 7 +++-
3 files changed, 65 insertions(+), 40 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 421a50a85547..0d339dd454f9 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -48,8 +48,10 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
/* The TFO segment data sits before the IASN; before receiving
* the remote key, IASN is assumed being 0.
*/
- MPTCP_SKB_CB(skb)->map_seq = -(u64)skb->len;
+ MPTCP_SKB_CB(skb)->map_seq64 = -(u64)skb->len;
+ MPTCP_SKB_CB(skb)->map_seq = MPTCP_SKB_CB(skb)->map_seq64;
MPTCP_SKB_CB(skb)->end_seq = 0;
+ MPTCP_SKB_CB(skb)->flags = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
mptcp_data_lock(sk);
@@ -57,7 +59,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
msk = mptcp_sk(sk);
msk->rcvd_dummy_seq = true;
- msk->copied_seq = MPTCP_SKB_CB(skb)->map_seq;
+ msk->copied_seq = MPTCP_SKB_CB(skb)->map_seq64;
msk->tfo_skb_len = skb->len;
mptcp_borrow_fwdmem(sk, skb);
skb_set_owner_r(skb, sk);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 792147b5279f..7195da74d5c7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,11 +168,13 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
int limit = READ_ONCE(sk->sk_rcvbuf);
if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
+ MPTCP_SKB_CB(from)->map_seq64 != MPTCP_SKB_CB(to)->map_seq64 +
+ to->len ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
return false;
- pr_debug("colesced seq %llx into %llx new len %d new end seq %llx\n",
+ pr_debug("colesced seq %x into %x new len %d new end seq %x\n",
MPTCP_SKB_CB(from)->map_seq, MPTCP_SKB_CB(to)->map_seq,
to->len, MPTCP_SKB_CB(from)->end_seq);
MPTCP_SKB_CB(to)->end_seq = MPTCP_SKB_CB(from)->end_seq;
@@ -254,8 +256,8 @@ static void mptcp_prune_ofo_queue(struct sock *sk,
struct sk_buff *skb = rb_to_skb(node);
/* Stop pruning if the incoming skb would land in OoO tail. */
- if (after64(MPTCP_SKB_CB(in_skb)->map_seq,
- MPTCP_SKB_CB(skb)->map_seq))
+ if (after(MPTCP_SKB_CB(in_skb)->map_seq,
+ MPTCP_SKB_CB(skb)->map_seq))
break;
pruned = true;
@@ -301,16 +303,21 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
{
struct sock *sk = (struct sock *)msk;
struct rb_node **p, *parent;
- u64 seq, end_seq, max_seq;
+ u64 end_seq, max_seq;
struct sk_buff *skb1;
+ u32 seq;
seq = MPTCP_SKB_CB(skb)->map_seq;
- end_seq = MPTCP_SKB_CB(skb)->end_seq;
+ end_seq = MPTCP_SKB_CB(skb)->map_seq64 + skb->len;
max_seq = atomic64_read(&msk->rcv_wnd_sent);
- pr_debug("msk=%p seq=%llx limit=%llx empty=%d\n", msk, seq, max_seq,
+ pr_debug("msk=%p seq=%x limit=%llx empty=%d\n", msk, seq, max_seq,
RB_EMPTY_ROOT(&msk->out_of_order_queue));
- if (after64(end_seq, max_seq)) {
+ /* Use the full sequence space to perform the admission checks, to
+ * protect vs possible wrap-arounds.
+ */
+ if (after64(MPTCP_SKB_CB(skb)->map_seq64, max_seq) ||
+ after64(end_seq, max_seq)) {
/* out of window */
mptcp_drop(sk, skb);
pr_debug("oow by %lld, rcv_wnd_sent %llu\n",
@@ -345,7 +352,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
}
/* Can avoid an rbtree lookup if we are adding skb after ooo_last_skb */
- if (!before64(seq, MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq)) {
+ if (!before(seq, MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq)) {
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOQUEUETAIL);
parent = &msk->ooo_last_skb->rbnode;
p = &parent->rb_right;
@@ -357,18 +364,18 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
while (*p) {
parent = *p;
skb1 = rb_to_skb(parent);
- if (before64(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
+ if (before(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
p = &parent->rb_left;
continue;
}
- if (before64(seq, MPTCP_SKB_CB(skb1)->end_seq)) {
- if (!after64(end_seq, MPTCP_SKB_CB(skb1)->end_seq)) {
+ if (before(seq, MPTCP_SKB_CB(skb1)->end_seq)) {
+ if (!after((u32)end_seq, MPTCP_SKB_CB(skb1)->end_seq)) {
/* All the bits are present. Drop. */
mptcp_drop(sk, skb);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
return;
}
- if (after64(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
+ if (after(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
/* partial overlap:
* | skb |
* | skb1 |
@@ -399,7 +406,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
merge_right:
/* Remove other segments covered by skb. */
while ((skb1 = skb_rb_next(skb)) != NULL) {
- if (before64(end_seq, MPTCP_SKB_CB(skb1)->end_seq))
+ if (before((u32)end_seq, MPTCP_SKB_CB(skb1)->end_seq))
break;
rb_erase(&skb1->rbnode, &msk->out_of_order_queue);
mptcp_drop(sk, skb1);
@@ -421,11 +428,13 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset)
/* the skb map_seq accounts for the skb offset:
* mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
- * value
+ * value; note that end seq number is only available in 32bits format.
*/
- MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow) -
- offset;
+ MPTCP_SKB_CB(skb)->map_seq64 = mptcp_subflow_get_mapped_dsn(subflow) -
+ offset;
+ MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
+ MPTCP_SKB_CB(skb)->flags = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
__skb_unlink(skb, &ssk->sk_receive_queue);
@@ -448,13 +457,14 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
if (!skb)
return;
- MPTCP_SKB_CB(skb)->map_seq = mptcp_iasn(msk) - skb->len;
+ MPTCP_SKB_CB(skb)->map_seq64 = mptcp_iasn(msk) - skb->len;
+ MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
}
static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
{
- u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
+ u32 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
struct mptcp_sock *msk = mptcp_sk(sk);
struct sk_buff *tail;
@@ -466,7 +476,7 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
if (unlikely(msk->rcvd_dummy_seq))
__mptcp_sync_rcv_sequence(sk);
- if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
+ if (MPTCP_SKB_CB(skb)->map_seq64 == msk->ack_seq) {
/* in sequence */
insert:
if (!mptcp_try_rmem_schedule(sk, skb)) {
@@ -484,14 +494,14 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
skb_set_owner_r(skb, sk);
__skb_queue_tail(&sk->sk_receive_queue, skb);
return true;
- } else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq)) {
+ } else if (after64(MPTCP_SKB_CB(skb)->map_seq64, msk->ack_seq)) {
mptcp_data_queue_ofo(msk, skb);
return false;
}
/* Partial packet */
- if (after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
- copy_len = MPTCP_SKB_CB(skb)->end_seq - msk->ack_seq;
+ if (after64(MPTCP_SKB_CB(skb)->map_seq64 + skb->len, msk->ack_seq)) {
+ copy_len = MPTCP_SKB_CB(skb)->end_seq - (u32)msk->ack_seq;
goto insert;
}
@@ -886,39 +896,37 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
{
struct sock *sk = (struct sock *)msk;
struct sk_buff *skb, *tail;
+ u32 seq_delta, ack_seq;
bool moved = false;
struct rb_node *p;
- u64 end_seq;
p = rb_first(&msk->out_of_order_queue);
pr_debug("msk=%p empty=%d\n", msk, RB_EMPTY_ROOT(&msk->out_of_order_queue));
while (p) {
+ ack_seq = msk->ack_seq;
skb = rb_to_skb(p);
- if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq))
+ if (after(MPTCP_SKB_CB(skb)->map_seq, ack_seq))
break;
p = rb_next(p);
rb_erase(&skb->rbnode, &msk->out_of_order_queue);
- if (unlikely(!after64(MPTCP_SKB_CB(skb)->end_seq,
- msk->ack_seq))) {
+ if (unlikely(!after(MPTCP_SKB_CB(skb)->end_seq, ack_seq))) {
mptcp_drop(sk, skb);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
continue;
}
- end_seq = MPTCP_SKB_CB(skb)->end_seq;
+ seq_delta = MPTCP_SKB_CB(skb)->end_seq - ack_seq;
tail = skb_peek_tail(&sk->sk_receive_queue);
if (!tail || !mptcp_try_coalesce(sk, tail, skb)) {
- int delta = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
-
- pr_debug("uncoalesced seq=%llx ack seq=%llx delta=%d\n",
- MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq,
- delta);
+ pr_debug("uncoalesced seq=%x ack seq=%x delta=%d\n",
+ MPTCP_SKB_CB(skb)->map_seq, ack_seq,
+ ack_seq - MPTCP_SKB_CB(skb)->map_seq);
__skb_queue_tail(&sk->sk_receive_queue, skb);
}
- msk->bytes_received += end_seq - msk->ack_seq;
- WRITE_ONCE(msk->ack_seq, end_seq);
+ msk->bytes_received += seq_delta;
+ WRITE_ONCE(msk->ack_seq, msk->ack_seq + seq_delta);
moved = true;
}
return moved;
@@ -2207,7 +2215,7 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
int copied = 0;
skb_queue_walk_safe(&sk->sk_receive_queue, skb, tmp) {
- u64 offset = *seq - MPTCP_SKB_CB(skb)->map_seq;
+ u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq;
u32 data_len = skb->len - offset;
u32 count;
int err;
@@ -4618,7 +4626,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
mptcp_move_skbs(sk);
while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
- offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
+ offset = (u32)msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
if (offset < skb->len) {
*off = offset;
return skb;
@@ -4868,11 +4876,23 @@ static int mptcp_napi_poll(struct napi_struct *napi, int budget)
return work_done;
}
+#define CHK_CB_FIELD(mptcp_field, tcp_field) \
+ ({ \
+ BUILD_BUG_ON(offsetof(struct mptcp_skb_cb, mptcp_field) != \
+ offsetof(struct tcp_skb_cb, tcp_field)); \
+ BUILD_BUG_ON(offsetofend(struct mptcp_skb_cb, mptcp_field) != \
+ offsetofend(struct tcp_skb_cb, tcp_field)); \
+ })
+
void __init mptcp_proto_init(void)
{
struct mptcp_delegated_action *delegated;
int cpu;
+ CHK_CB_FIELD(map_seq, seq);
+ CHK_CB_FIELD(end_seq, end_seq);
+ CHK_CB_FIELD(flags, tcp_flags);
+
mptcp_prot.h.hashinfo = tcp_prot.h.hashinfo;
if (percpu_counter_init(&mptcp_sockets_allocated, 0, GFP_KERNEL))
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 985ef15275eb..9b073cbdf76e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -129,9 +129,12 @@
#define MPTCP_SYNC_SEQ 8
struct mptcp_skb_cb {
- u64 map_seq;
- u64 end_seq;
+ u32 map_seq;
+ u32 end_seq;
+ u32 unused;
+ u16 flags;
u8 has_rxtstamp;
+ u64 map_seq64;
};
#define MPTCP_SKB_CB(__skb) ((struct mptcp_skb_cb *)&((__skb)->cb[0]))
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (3 preceding siblings ...)
2026-10-30 8:41 ` [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-10-30 8:41 ` Geliang Tang
2026-10-30 9:08 ` sashiko-bot
2026-10-30 10:45 ` [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
5 siblings, 1 reply; 11+ messages in thread
From: Geliang Tang @ 2026-10-30 8:41 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
When MPTCP carries TLS, __mptcp_read_sock() is reached from BH context
(via tls_strp_read_copyin() -> sk->sk_read_sock()), where calling
mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() is not safe as both
expect process context.
Move both calls to mptcp_worker(): __mptcp_read_sock() accumulates the
consumed bytes in msk->read_copied and signals the worker through a new
MPTCP_WORK_READ_COMPLETE bit. The worker drains the accumulator under
mptcp_data_lock() and runs rcvbuf autotuning and RBU update in process
context.
Also remove the backlog flush from mptcp_recv_skb() (covered by
mptcp_release_cb()), drop the msk_owned_by_me() assertion (BH caller
does not own the msk lock), and drop the redundant mptcp_recv_skb()
call after the main loop.
Initialize msk->read_copied in __mptcp_init_sock() and reset it in
mptcp_disconnect().
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 37 ++++++++++++++++++++++++++++---------
net/mptcp/protocol.h | 2 ++
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7195da74d5c7..a59247495987 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3136,6 +3136,21 @@ static void mptcp_backlog_purge(struct sock *sk)
sk_mem_reclaim(sk);
}
+static void mptcp_read_complete(struct sock *sk)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ int read_copied;
+
+ mptcp_data_lock(sk);
+ read_copied = msk->read_copied;
+ msk->read_copied = 0;
+ mptcp_data_unlock(sk);
+
+ mptcp_rcv_space_adjust(msk, read_copied);
+ if (read_copied > 0)
+ mptcp_cleanup_rbuf(msk, read_copied);
+}
+
static void mptcp_do_fastclose(struct sock *sk)
{
struct mptcp_subflow_context *subflow, *tmp;
@@ -3211,6 +3226,9 @@ static void mptcp_worker(struct work_struct *work)
if (test_and_clear_bit(MPTCP_WORK_RTX, &msk->flags))
__mptcp_retrans(sk);
+ if (test_and_clear_bit(MPTCP_WORK_READ_COMPLETE, &msk->flags))
+ mptcp_read_complete(sk);
+
fail_tout = msk->first ? READ_ONCE(mptcp_subflow_ctx(msk->first)->fail_tout) : 0;
if (fail_tout && time_after(jiffies, fail_tout))
mptcp_mp_fail_no_response(msk);
@@ -3254,6 +3272,7 @@ static void __mptcp_init_sock(struct sock *sk)
msk->last_data_sent = tcp_jiffies32;
msk->last_data_recv = tcp_jiffies32;
msk->last_ack_recv = tcp_jiffies32;
+ msk->read_copied = 0;
mptcp_pm_data_init(msk);
spin_lock_init(&msk->fallback_lock);
@@ -3701,6 +3720,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->fastclosing = 0;
msk->rcvd_dummy_seq = false;
msk->tfo_skb_len = 0;
+ msk->read_copied = 0;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
@@ -4622,9 +4642,6 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
struct sk_buff *skb;
u32 offset;
- if (!list_empty(&msk->backlog_list))
- mptcp_move_skbs(sk);
-
while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
offset = (u32)msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
if (offset < skb->len) {
@@ -4639,6 +4656,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
/*
* Note:
* - It is assumed that the socket was locked by the caller.
+ * - Can be invoked in BH scope.
*/
static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
sk_read_actor_t recv_actor, bool noack)
@@ -4648,8 +4666,6 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
int copied = 0;
u32 offset;
- msk_owned_by_me(msk);
-
if (sk->sk_state == TCP_LISTEN)
return -ENOTCONN;
while ((skb = mptcp_recv_skb(sk, &offset)) != NULL) {
@@ -4680,11 +4696,14 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
if (noack)
goto out;
- mptcp_rcv_space_adjust(msk, copied);
-
+ /* The backlog flushing is only needed when some data is actually
+ * moved and will take place in mptcp_release_cb() via
+ * release_sock() in the worker.
+ */
if (copied > 0) {
- mptcp_recv_skb(sk, &offset);
- mptcp_cleanup_rbuf(msk, copied);
+ msk->read_copied += copied;
+ set_bit(MPTCP_WORK_READ_COMPLETE, &msk->flags);
+ mptcp_schedule_work(sk);
}
out:
return copied;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 9b073cbdf76e..7441da7704fe 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -117,6 +117,7 @@
#define MPTCP_FALLBACK_DONE 2
#define MPTCP_WORK_CLOSE_SUBFLOW 3
#define MPTCP_RTX_ENABLED 4
+#define MPTCP_WORK_READ_COMPLETE 5
/* MPTCP socket release cb flags */
#define MPTCP_PUSH_PENDING 1
@@ -326,6 +327,7 @@ struct mptcp_sock {
u32 last_data_sent;
u32 last_data_recv;
u32 last_ack_recv;
+ int read_copied;
unsigned long timer_ival;
u32 token;
u32 tfo_skb_len;
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field
2026-10-30 8:41 ` [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field Geliang Tang
@ 2026-10-30 9:02 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-30 9:02 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mptcp: mptcp_inq_hint() seq mismatch when subflow_set_remote_key() defers sync
- [Medium] mptcp: off-by-one in __mptcp_sync_rcv_sequence() after mptcp_check_data_fin()
--
commit f64a32260362fb1c8168b46cd47f03ba4852fb6c
Author: Paolo Abeni <pabeni@redhat.com>
mptcp: drop the cant_coalesce CB field
This commit removes the cant_coalesce skb control buffer field and instead
synchronizes the sequence numbers of a passive TCP Fast Open skb once the
initial ACK sequence number becomes available on the third ACK.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 528a72da646cc..fe154b1816834 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -438,6 +436,26 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
> skb_dst_drop(skb);
> }
>
> +void __mptcp_sync_rcv_sequence(struct sock *sk)
> +{
> + struct mptcp_sock *msk = mptcp_sk(sk);
> + struct sk_buff *skb;
> + u32 offset;
> +
> + if (likely(!msk->rcvd_dummy_seq))
> + return;
> +
> + /* User space can have already received the TFO skb. */
> + msk->rcvd_dummy_seq = false;
> + skb = skb_peek_tail(&sk->sk_receive_queue);
> + if (!skb)
> + return;
> +
> + offset = MPTCP_SKB_CB(skb)->offset;
> + MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len + offset;
> + MPTCP_SKB_CB(skb)->end_seq = msk->ack_seq;
> +}
[Severity: Medium]
As reported by Sashiko on v15 [1], can __mptcp_sync_rcv_sequence() compute
an off-by-one map_seq and end_seq if mptcp_check_data_fin() runs before
mptcp_release_cb()?
When mptcp_worker() holds lock_sock(sk) on a passive TFO socket with SYN data
queued in sk->sk_receive_queue (msk->rcvd_dummy_seq is true), the peer's third
ACK carrying MP_CAPABLE and a zero-payload DATA_FIN (with data_seq equal to
subflow->iasn and data_len equal to 1) can arrive in BH context.
In that case, subflow_set_remote_key() sets msk->ack_seq to subflow->iasn and
sets MPTCP_SYNC_SEQ in msk->cb_flags, while mptcp_incoming_options() /
get_mapping_status() calls mptcp_update_rcv_data_fin() to record
msk->rcv_data_fin_seq = subflow->iasn and msk->rcv_data_fin = 1 without
queuing an skb to msk->backlog_list.
Before release_sock(sk) invokes mptcp_release_cb() in net/mptcp/protocol.c,
mptcp_worker() calls mptcp_check_data_fin():
net/mptcp/protocol.c:mptcp_worker()
mptcp_check_send_data_fin(sk);
mptcp_check_data_fin_ack(sk);
mptcp_check_data_fin(sk);
which sees msk->ack_seq == rcv_data_fin_seq and advances msk->ack_seq by 1:
net/mptcp/protocol.c:mptcp_check_data_fin()
if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) {
WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
WRITE_ONCE(msk->rcv_data_fin, 0);
When release_sock(sk) subsequently runs mptcp_release_cb() ->
__mptcp_sync_rcv_sequence(sk), msk->ack_seq is already subflow->iasn + 1, so
both MPTCP_SKB_CB(skb)->map_seq and MPTCP_SKB_CB(skb)->end_seq on the TFO skb
will be shifted forward by 1 byte.
[ ... ]
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 78e886b202585..49a000b8dcb8e 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -515,6 +515,8 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> struct mptcp_subflow_context *subflow,
> const struct mptcp_options_received *mp_opt)
> {
> + struct sock *sk = (struct sock *)msk;
> +
> /* active MPC subflow will reach here multiple times:
> * at subflow_finish_connect() time and at 4th ack time
> */
> @@ -533,6 +535,11 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> WRITE_ONCE(msk->ack_seq, subflow->iasn);
> WRITE_ONCE(msk->can_ack, true);
> atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
> +
> + if (!sock_owned_by_user(sk))
> + __mptcp_sync_rcv_sequence(sk);
> + else
> + set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
> }
[Severity: Medium]
As reported by Sashiko on v15 [2], does deferring __mptcp_sync_rcv_sequence()
here while immediately updating msk->ack_seq cause mptcp_inq_hint() to return
a bogus value when sock_owned_by_user(sk) is true?
In mptcp_fastopen_subflow_synack_set_params() in net/mptcp/fastopen.c, the TFO
skb is queued onto sk->sk_receive_queue with MPTCP_SKB_CB(skb)->map_seq set to
-skb->len and end_seq set to 0 while msk->ack_seq is 0.
If user space holds lock_sock(sk) in mptcp_ioctl() (for SIOCINQ) or
mptcp_recvmsg() when the peer's third ACK arrives in BH context,
subflow_set_remote_key() updates msk->ack_seq to subflow->iasn immediately,
but defers __mptcp_sync_rcv_sequence(sk) via MPTCP_SYNC_SEQ until
release_sock(sk) runs mptcp_release_cb().
When the third ACK carries no payload (or arrives after mptcp_move_skbs() has
checked msk->backlog_list), __mptcp_move_skb() is not called before
mptcp_inq_hint(sk) runs under lock_sock(sk):
net/mptcp/protocol.c:mptcp_ioctl()
lock_sock(sk);
if (mptcp_move_skbs(sk))
mptcp_cleanup_rbuf(msk, 0);
*karg = mptcp_inq_hint(sk);
release_sock(sk);
and similarly in mptcp_recvmsg() (for TCP_CM_INQ) and
__mptcp_recvmsg_mskq() -> mptcp_rcv_space_adjust().
In mptcp_inq_hint() in net/mptcp/protocol.c:
skb = skb_peek(&sk->sk_receive_queue);
if (skb) {
u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
if (hint_val >= INT_MAX)
return INT_MAX;
return (unsigned int)hint_val;
}
hint_val subtracts the unsynchronized dummy map_seq (-skb->len + offset) from
subflow->iasn, evaluating to subflow->iasn + skb->len - offset and returning
INT_MAX (or a corrupted value) instead of the unread TFO payload length.
[1] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn
[2] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793349369.git.geliang@kernel.org?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker
2026-10-30 8:41 ` [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-10-30 9:08 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-30 9:08 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mptcp: unsynchronized copied_seq when MPTCP_SYNC_SEQ is deferred
- [High] mptcp: wrong map_seq64 in mptcp_init_skb() on non-zero skb offset
--
commit 68366c7579c4f5b2392dcf76c7e487086dcb48df
Author: Paolo Abeni <pabeni@redhat.com>
mptcp: defer read_sock cleanup to mptcp_worker
This series replaces the MPTCP SKB control buffer offset tracking with
msk->copied_seq, aligns struct mptcp_skb_cb with struct tcp_skb_cb using
32-bit sequence numbers, and defers __mptcp_read_sock() receive buffer
cleanup to mptcp_worker().
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..a59247495987a 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -424,21 +421,21 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
> skb_set_owner_r(skb, sk);
> }
>
> -static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
> - int copy_len)
> +static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset)
> {
> struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
>
> /* the skb map_seq accounts for the skb offset:
> * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
> - * value
> + * value; note that end seq number is only available in 32bits format.
> */
> - MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
> - MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + copy_len;
> - MPTCP_SKB_CB(skb)->offset = offset;
> + MPTCP_SKB_CB(skb)->map_seq64 = mptcp_subflow_get_mapped_dsn(subflow) -
> + offset;
> + MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
> + MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
> + MPTCP_SKB_CB(skb)->flags = 0;
> MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
> - MPTCP_SKB_CB(skb)->cant_coalesce = 0;
>
> __skb_unlink(skb, &ssk->sk_receive_queue);
>
> @@ -446,15 +443,40 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
> skb_dst_drop(skb);
> }
[ ... ]
> static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
> {
> - u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
> + u32 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct sk_buff *tail;
>
> mptcp_borrow_fwdmem(sk, skb);
>
> - if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
> + /* 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);
> +
> + if (MPTCP_SKB_CB(skb)->map_seq64 == msk->ack_seq) {
> /* in sequence */
> insert:
> if (!mptcp_try_rmem_schedule(sk, skb)) {
> @@ -472,18 +494,14 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
> skb_set_owner_r(skb, sk);
> __skb_queue_tail(&sk->sk_receive_queue, skb);
> return true;
> - } else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq)) {
> + } else if (after64(MPTCP_SKB_CB(skb)->map_seq64, msk->ack_seq)) {
> mptcp_data_queue_ofo(msk, skb);
> return false;
> }
>
> /* Partial packet */
> - if (after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
> - copy_len = MPTCP_SKB_CB(skb)->end_seq - msk->ack_seq;
> - MPTCP_SKB_CB(skb)->offset += msk->ack_seq -
> - MPTCP_SKB_CB(skb)->map_seq;
> - MPTCP_SKB_CB(skb)->map_seq += msk->ack_seq -
> - MPTCP_SKB_CB(skb)->map_seq;
> + if (after64(MPTCP_SKB_CB(skb)->map_seq64 + skb->len, msk->ack_seq)) {
> + copy_len = MPTCP_SKB_CB(skb)->end_seq - (u32)msk->ack_seq;
> goto insert;
> }
[Severity: High]
When __mptcp_move_skbs_from_subflow() passes a non-zero offset to
mptcp_init_skb() (for example, when the leading offset bytes of skb on
ssk->sk_receive_queue belonged to a prior or duplicate DSS mapping that was
already consumed or discarded via subflow_check_data_avail() ->
mptcp_subflow_discard_data()), mptcp_init_skb() subtracts offset from
mptcp_subflow_get_mapped_dsn(subflow):
MPTCP_SKB_CB(skb)->map_seq64 = mptcp_subflow_get_mapped_dsn(subflow) -
offset;
MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
If the new DSS mapping starting at offset has mapped_dsn > msk->ack_seq
(out-of-order at the MPTCP level because the range [msk->ack_seq, mapped_dsn)
is still in flight on another subflow), doesn't subtracting offset assign the
DSN range [mapped_dsn - offset, mapped_dsn) to the already-discarded bytes
0..offset of skb?
Specifically, in __mptcp_move_skb():
1. When mapped_dsn - offset <= msk->ack_seq < mapped_dsn, __mptcp_move_skb()
sees map_seq64 <= msk->ack_seq and map_seq64 + skb->len > msk->ack_seq.
Instead of queueing skb to msk->out_of_order_queue, it takes the in-sequence
or partial-packet insert path, advances msk->ack_seq across the unreceived
DSN gap, and causes __mptcp_recvmsg_mskq() to copy the stale subflow bytes at
0..offset to user-space.
2. When mapped_dsn - offset > msk->ack_seq, __mptcp_move_skb() queues skb into
msk->out_of_order_queue with map_seq = mapped_dsn - offset, so
mptcp_data_queue_ofo() and __mptcp_ofo_queue() treat bytes 0..offset as valid
data for [mapped_dsn - offset, mapped_dsn) and drop the real segments arriving
on other subflows as duplicate data.
Can this corrupt the MPTCP receive stream when an out-of-order subflow SKB has
a non-zero offset?
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 78e886b202585..899ffaba0b2b6 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -515,6 +515,8 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> struct mptcp_subflow_context *subflow,
> const struct mptcp_options_received *mp_opt)
> {
> + struct sock *sk = (struct sock *)msk;
> +
> /* active MPC subflow will reach here multiple times:
> * at subflow_finish_connect() time and at 4th ack time
> */
> @@ -533,6 +535,13 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> WRITE_ONCE(msk->ack_seq, subflow->iasn);
> WRITE_ONCE(msk->can_ack, true);
> atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
> +
> + if (!sock_owned_by_user(sk)) {
> + msk->copied_seq += subflow->iasn;
> + __mptcp_sync_rcv_sequence(sk);
> + } else {
> + set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
> + }
> }
[Severity: High]
When subflow_set_remote_key() runs in BH context while sock_owned_by_user(sk)
is true, msk->ack_seq is updated immediately to subflow->iasn while updating
msk->copied_seq is deferred via MPTCP_SYNC_SEQ until mptcp_release_cb().
Could leaving msk->copied_seq unsynchronized while the user holds the socket
lock cause several issues?
First, in net/mptcp/protocol.c:mptcp_recvmsg():
bytes_read = __mptcp_recvmsg_mskq(sk, msg, len - copied, flags,
seq, &tss, &cmsg_flags,
&last);
...
if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk)) {
if (flags & MSG_PEEK)
peek_seq = msk->copied_seq + copied;
continue;
}
If payload arrives with or right after the MP_CAPABLE packet while the user
holds lock_sock(sk) in mptcp_recvmsg(), mptcp_data_ready() queues the
IASN-based SKBs onto msk->backlog_list. Before release_sock(sk) and
mptcp_release_cb() run, mptcp_recvmsg() calls mptcp_move_skbs(sk) and
continues to the next iteration of __mptcp_recvmsg_mskq(). Inside
mptcp_move_skbs() -> __mptcp_move_skb(), __mptcp_sync_rcv_sequence(sk) updates
any TFO SKB in sk->sk_receive_queue to the IASN sequence space and clears
msk->rcvd_dummy_seq, and queues the new IASN-based SKBs onto
sk->sk_receive_queue, while MPTCP_SYNC_SEQ is still pending and
msk->copied_seq has not been advanced by subflow->iasn.
On the next iteration, __mptcp_recvmsg_mskq() computes:
u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq;
using the unsynchronized msk->copied_seq against the IASN-based map_seq. When
offset >= skb->len, __mptcp_recvmsg_mskq() treats the unread SKB as
already-peeked data, sets *last = skb, and skips it; sk_wait_data(sk, &timeo,
last) then blocks waiting for skb_peek_tail(&sk->sk_receive_queue) != last
even after release_sock(sk) runs MPTCP_SYNC_SEQ, stalling the reader with
unread data in the queue. When offset < skb->len, __mptcp_recvmsg_mskq()
copies from a bogus offset inside skb and eats the remainder.
Second, any call to mptcp_inq_hint(sk) while MPTCP_SYNC_SEQ is pending (via
mptcp_ioctl() for SIOCINQ, mptcp_recvmsg() for TCP_CM_INQ, or
mptcp_rcv_space_adjust()) subtracts the unsynchronized msk->copied_seq from
the IASN-based msk->ack_seq and returns INT_MAX.
Third, in net/mptcp/protocol.c:mptcp_release_cb():
do {
...
if (__mptcp_move_skbs(sk))
need_push = true;
...
} while (!list_empty(&msk->backlog_list));
if (need_push)
__mptcp_push_pending(sk, 0);
if (msk->cb_flags) {
...
if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
msk->copied_seq += mptcp_iasn(msk);
__mptcp_sync_rcv_sequence(sk);
}
}
Because __mptcp_move_skbs(sk) runs before MPTCP_SYNC_SEQ is handled, if a
spooled backlog SKB carries a DATA_FIN (arriving with or right after the 3rd
ACK while the user holds lock_sock(sk)), __mptcp_move_skbs() ->
mptcp_check_data_fin() increments msk->ack_seq by 1 without incrementing
msk->bytes_received, causing mptcp_iasn(msk) to shift msk->copied_seq one byte
past the real IASN.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793349369.git.geliang@kernel.org?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field
2026-10-30 8:41 ` [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field Geliang Tang
@ 2026-10-30 9:18 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-30 9:18 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mptcp: unsynchronized copied_seq when MPTCP_SYNC_SEQ is deferred
- [High] mptcp: off-by-one in mptcp_iasn() when DATA_FIN precedes MPTCP_SYNC_SEQ
- [High] mptcp: stream corruption in mptcp_init_skb() on non-zero skb offset
--
commit 1889ab1df71af9d2613dfc80ed132ac8e6d02071
Author: Paolo Abeni <pabeni@redhat.com>
mptcp: remove CB offset field
This commit replaces the per-skb MPTCP_SKB_CB(skb)->offset field with an
msk-level copied_seq counter to track consumed bytes in the MPTCP receive
queue. It also adds deferred sequence synchronization via MPTCP_SYNC_SEQ and
mptcp_iasn() for passive FastOpen sockets when the remote key arrives while
the socket is owned by user space.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fe154b1816834..792147b5279f9 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -168,7 +168,6 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
> int limit = READ_ONCE(sk->sk_rcvbuf);
>
> if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
> - MPTCP_SKB_CB(from)->offset ||
> ((to->len + from->len) > (limit >> 3)) ||
> !skb_try_coalesce(to, from, fragstolen, delta))
> return false;
> @@ -415,8 +414,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
> skb_set_owner_r(skb, sk);
> }
>
> -static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
> - int copy_len)
> +static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset)
> {
> struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> @@ -425,9 +423,9 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
> * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
> * value
> */
> - MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
> - MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + copy_len;
> - MPTCP_SKB_CB(skb)->offset = offset;
> + MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow) -
> + offset;
> + MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
> MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>
> __skb_unlink(skb, &ssk->sk_receive_queue);
[Severity: High]
As reported by Sashiko on v17 [1], can subtracting offset from
mptcp_subflow_get_mapped_dsn(subflow) in mptcp_init_skb() and dropping the
offset check in __mptcp_try_coalesce() corrupt the MPTCP data stream when an
skb has a non-zero subflow offset?
An skb at the head of ssk->sk_receive_queue can reach
__mptcp_move_skbs_from_subflow() with offset = tp->copied_seq -
TCP_SKB_CB(skb)->seq > 0 while carrying a new DSS mapping M2 starting at
tp->copied_seq (skb->data[offset..skb->len)) with mapped_dsn =
mptcp_subflow_get_mapped_dsn(subflow):
1. In subflow_check_data_avail() -> get_mapping_status(), a single skb can
span the tail of a previous DSS mapping M1 in skb->data[0..offset) and the
start of a new DSS mapping M2 in skb->data[offset..skb->len):
net/mptcp/subflow.c:get_mapping_status() {
...
if (!skb_is_fully_mapped(ssk, skb)) {
/* will validate the next map after consuming the current one */
goto validate_csum;
}
}
If M1 was already received on another subflow (ack_seq < msk->ack_seq),
mptcp_subflow_discard_data() advances tp->copied_seq by offset without
eating the skb and resets subflow->map_valid = 0. On the next iteration,
get_mapping_status() installs M2 starting offset bytes into the skb.
2. Similarly, tcp_data_queue() or tcp_ofo_queue() can queue a partially
overlapping TCP segment with TCP_SKB_CB(skb)->seq < tp->copied_seq <
TCP_SKB_CB(skb)->end_seq at a DSS mapping boundary.
Because MPTCP stripes mappings across subflows, M1 and M2 on the same subflow
have contiguous SSNs but non-contiguous MPTCP DSNs, so skb->data[0..offset)
does not belong to the DSN range [mapped_dsn - offset, mapped_dsn). Setting
MPTCP_SKB_CB(skb)->map_seq = mapped_dsn - offset (and map_seq64 = mapped_dsn -
offset in the follow-up commit syncing the skb cb layout with tcp) causes
several problems when M2 is out of order (mapped_dsn > msk->ack_seq) or
queued to msk->backlog_list:
- If mapped_dsn - offset <= msk->ack_seq < mapped_dsn, __mptcp_move_skb()
treats the out-of-order skb as in-sequence or partially overlapping,
advances msk->ack_seq and msk->bytes_received across the unreceived gap
[msk->ack_seq, mapped_dsn), coalesces skb->data[0..skb->len) onto the
receive queue tail via mptcp_try_coalesce() if mapped_dsn - offset ==
MPTCP_SKB_CB(tail)->end_seq, delivers the stale M1 bytes in
skb->data[0..offset) to user space, and causes the genuine data for
[msk->ack_seq, mapped_dsn) to be dropped later as MPTCP_MIB_DUPDATA.
- If mapped_dsn - offset > msk->ack_seq, mptcp_data_queue_ofo() inserts or
coalesces the skb in msk->out_of_order_queue under the fabricated range
[mapped_dsn - offset, end_seq), dropping or replacing legitimate
out-of-order segments covering [mapped_dsn - offset, mapped_dsn) and
delivering skb->data[0..offset) to user space when __mptcp_ofo_queue()
dequeues it.
- In __mptcp_add_backlog(), if mapped_dsn - offset ==
MPTCP_SKB_CB(tail)->end_seq for a prior backlog skb, __mptcp_try_coalesce()
coalesces skb->data[0..skb->len) including the stale prefix
skb->data[0..offset) into tail.
> @@ -440,7 +438,6 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
> {
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct sk_buff *skb;
> - u32 offset;
>
> if (likely(!msk->rcvd_dummy_seq))
> return;
> @@ -451,9 +448,8 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
> if (!skb)
> return;
>
> - offset = MPTCP_SKB_CB(skb)->offset;
> - MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len + offset;
> - MPTCP_SKB_CB(skb)->end_seq = msk->ack_seq;
> + MPTCP_SKB_CB(skb)->map_seq = mptcp_iasn(msk) - skb->len;
> + MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
> }
[ ... ]
> @@ -2209,33 +2198,24 @@ static void mptcp_eat_recv_skb(struct sock *sk, struct sk_buff *skb)
> }
>
> static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
> - size_t len, int flags, int copied_total,
> + size_t len, int flags, u64 *seq,
> struct scm_timestamping_internal *tss,
> int *cmsg_flags, struct sk_buff **last)
> {
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct sk_buff *skb, *tmp;
> - int total_data_len = 0;
> int copied = 0;
>
> skb_queue_walk_safe(&sk->sk_receive_queue, skb, tmp) {
> - u32 delta, offset = MPTCP_SKB_CB(skb)->offset;
> + u64 offset = *seq - MPTCP_SKB_CB(skb)->map_seq;
> u32 data_len = skb->len - offset;
> u32 count;
> int err;
>
> - if (flags & MSG_PEEK) {
> - /* skip already peeked skbs */
> - if (total_data_len + data_len <= copied_total) {
> - total_data_len += data_len;
> - *last = skb;
> - continue;
> - }
> -
> - /* skip the already peeked data in the current skb */
> - delta = copied_total - total_data_len;
> - offset += delta;
> - data_len -= delta;
> + /* Skip the already peeked data. */
> + if (offset >= skb->len) {
> + *last = skb;
> + continue;
> }
[ ... ]
> @@ -2417,22 +2395,17 @@ static bool mptcp_move_skbs(struct sock *sk)
> static unsigned int mptcp_inq_hint(const struct sock *sk)
> {
> const struct mptcp_sock *msk = mptcp_sk(sk);
> - const struct sk_buff *skb;
> -
> - skb = skb_peek(&sk->sk_receive_queue);
> - if (skb) {
> - u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
> + u64 hint_val;
>
> - if (hint_val >= INT_MAX)
> - return INT_MAX;
> + hint_val = READ_ONCE(msk->ack_seq) - READ_ONCE(msk->copied_seq);
> + if (hint_val >= INT_MAX)
> + return INT_MAX;
>
> - return (unsigned int)hint_val;
> - }
> -
> - if (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN))
> + if (!hint_val &&
> + (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN)))
> return 1;
>
> - return 0;
> + return (unsigned int)hint_val;
> }
[ ... ]
> @@ -2478,8 +2457,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>
> copied += bytes_read;
>
> - if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk))
> + if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk)) {
> + if (flags & MSG_PEEK)
> + peek_seq = msk->copied_seq + copied;
> continue;
> + }
[ ... ]
> @@ -3955,8 +3943,10 @@ static void mptcp_release_cb(struct sock *sk)
> __mptcp_error_report(sk);
> if (__test_and_clear_bit(MPTCP_SYNC_SNDBUF, &msk->cb_flags))
> __mptcp_sync_sndbuf(sk);
> - if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
> + if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
> + msk->copied_seq += mptcp_iasn(msk);
> __mptcp_sync_rcv_sequence(sk);
> + }
> }
> }
>
> @@ -4628,7 +4618,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
> mptcp_move_skbs(sk);
>
> while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
> - offset = MPTCP_SKB_CB(skb)->offset;
> + offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
> if (offset < skb->len) {
> *off = offset;
> return skb;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index c317d8b59fffb..985ef15275eb0 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
[ ... ]
> @@ -884,6 +885,11 @@ struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk);
> int mptcp_sched_get_send(struct mptcp_sock *msk);
> int mptcp_sched_get_retrans(struct mptcp_sock *msk);
>
> +static inline u64 mptcp_iasn(const struct mptcp_sock *msk)
> +{
> + return msk->ack_seq - msk->bytes_received + msk->tfo_skb_len;
> +}
> +
> static inline u64 mptcp_data_avail(const struct mptcp_sock *msk)
> {
> return READ_ONCE(msk->bytes_received) - READ_ONCE(msk->bytes_consumed);
[Severity: High]
As reported by Sashiko on v15 [2], won't mptcp_iasn() be off by one if a
DATA_FIN is processed before the deferred MPTCP_SYNC_SEQ runs in
mptcp_release_cb()?
The commit message notes that mptcp_check_data_fin() incrementing ack_seq
before MPTCP_SYNC_SEQ runs would require a DATA_FIN to arrive before the MPC
third ACK on a passive TFO socket. However, subflow_set_remote_key() sets
MPTCP_SYNC_SEQ at the moment the MPC third ACK (or SYN-ACK on an active
socket) arrives while sock_owned_by_user(sk) is true, which also establishes
the connection and sets msk->ack_seq = subflow->iasn.
If the peer sends a payload with DATA_FIN (or a subflow closes immediately)
together with or right after the MPC third ACK while lock_sock(sk) is still
held:
1. When data + DATA_FIN is queued to msk->backlog_list while lock_sock(sk) is
held, release_sock(sk) invokes mptcp_release_cb(sk), which spools the
backlog via __mptcp_move_skbs() in its loop before checking MPTCP_SYNC_SEQ
at the end of the function:
net/mptcp/protocol.c:__mptcp_move_skbs() {
...
__mptcp_ofo_queue(msk);
if (moved)
mptcp_check_data_fin((struct sock *)msk);
return moved;
}
Inside __mptcp_move_skbs(), __mptcp_move_skb() syncs any TFO skb using
mptcp_iasn(msk) == subflow->iasn and queues the new skb at map_seq =
subflow->iasn, and then mptcp_check_data_fin() increments msk->ack_seq by 1
without incrementing msk->bytes_received. When mptcp_release_cb() then
processes MPTCP_SYNC_SEQ, msk->copied_seq += mptcp_iasn(msk) adds
subflow->iasn + 1, advancing msk->copied_seq one byte past the queued skb's
map_seq and skipping the first byte of unread payload.
2. Similarly, if a 0-payload DATA_FIN arrives after the third ACK while
mptcp_worker() holds lock_sock(sk), mptcp_worker() calls
mptcp_check_data_fin(sk) and increments msk->ack_seq to subflow->iasn + 1
before release_sock(sk) runs mptcp_release_cb(sk).
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 49a000b8dcb8e..899ffaba0b2b6 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -536,10 +536,12 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> WRITE_ONCE(msk->can_ack, true);
> atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
>
> - if (!sock_owned_by_user(sk))
> + if (!sock_owned_by_user(sk)) {
> + msk->copied_seq += subflow->iasn;
> __mptcp_sync_rcv_sequence(sk);
> - else
> + } else {
> set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
> + }
> }
[Severity: High]
As reported by Sashiko on v15 [2], when subflow_set_remote_key() runs in BH
context while sock_owned_by_user(sk) is true, it immediately updates
msk->ack_seq to subflow->iasn and sets msk->can_ack = true, but defers
updating msk->copied_seq via MPTCP_SYNC_SEQ until mptcp_release_cb(). What
happens if the user context holding lock_sock(sk) accesses msk->copied_seq or
spools the backlog before release_sock(sk) runs?
Specifically, if the peer's MP_CAPABLE third ACK (on a passive TFO socket) or
SYN-ACK (on an active client socket) arrives with or followed by data while
user space holds lock_sock(sk):
1. In mptcp_recvmsg(), mptcp_recv_skb(), and mptcp_ioctl(SIOCINQ),
mptcp_move_skbs(sk) is called while holding lock_sock(sk). Inside
mptcp_move_skbs() -> __mptcp_move_skbs() -> __mptcp_move_skb(),
__mptcp_sync_rcv_sequence(sk) rebases any queued TFO skb's map_seq from
-(u64)skb->len to mptcp_iasn(msk) - skb->len and clears
msk->rcvd_dummy_seq = false, and __mptcp_move_skb() moves the IASN-mapped
backlog skbs into sk->sk_receive_queue. However, MPTCP_SYNC_SEQ is still
pending, so msk->copied_seq (and peek_seq = msk->copied_seq + copied in
mptcp_recvmsg()) remains in the pre-IASN sequence space.
2. In mptcp_recv_skb(), the loop then computes:
offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
if (offset < skb->len) {
*off = offset;
return skb;
}
mptcp_eat_recv_skb(sk, skb);
Because msk->copied_seq has not been rebased by subflow->iasn, offset
underflows to a value >= skb->len, and mptcp_eat_recv_skb() frees and drops
every unread skb in sk->sk_receive_queue without advancing msk->copied_seq
or msk->bytes_consumed. Even after the later commit in this series ("mptcp:
defer read_sock cleanup to mptcp_worker") removes mptcp_move_skbs() from
mptcp_recv_skb(), mptcp_release_cb() still runs __mptcp_move_skbs() and
calls sk->sk_data_ready(sk) in its loop before processing MPTCP_SYNC_SEQ,
so kTLS RX synchronously invokes __mptcp_read_sock() -> mptcp_recv_skb()
with an unsynchronized msk->copied_seq and drops the queued skbs.
3. In mptcp_recvmsg(), the next iteration of __mptcp_recvmsg_mskq() computes
offset = *seq - MPTCP_SKB_CB(skb)->map_seq, which similarly underflows by
subflow->iasn; when offset >= skb->len, it treats all unread skbs in
sk->sk_receive_queue as already-peeked data, sets *last = skb, and blocks
in sk_wait_data() (or copies from a corrupted mid-skb offset if a 32-bit
offset wraps below skb->len in the follow-up commit).
4. In mptcp_inq_hint() (called under lock_sock(sk) by mptcp_ioctl(SIOCINQ),
TCP_CM_INQ, and mptcp_rcv_space_adjust()), READ_ONCE(msk->ack_seq) -
READ_ONCE(msk->copied_seq) subtracts the pre-IASN msk->copied_seq from the
post-IASN msk->ack_seq, returning INT_MAX and corrupting receive buffer
autotuning.
[1] https://lore.kernel.org/r/9a98772b14e535454de5bafbf113aa72a3ee6f44.1793326853.git.tanggeliang@kylinos.cn
[2] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793349369.git.geliang@kernel.org?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one
2026-10-30 8:41 ` [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-10-30 9:19 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-30 9:19 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mptcp: subtracting offset from map_seq64 in mptcp_init_skb() corrupts OFO data
- [High] mptcp: copied_seq desync when mptcp_move_skbs() runs before MPTCP_SYNC_SEQ
- [High] mptcp: 64-bit window wrap-around in mptcp_data_queue_ofo() when ack_seq > max_seq
--
commit 7989e41ea583368f6b9348be07447be36c4632d3
Author: Paolo Abeni <pabeni@redhat.com>
mptcp: sync mptcp skb cb layout with tcp one
This commit updates struct mptcp_skb_cb to store 32-bit map_seq and end_seq
fields alongside a 64-bit map_seq64 field so that the MPTCP control buffer
layout matches TCP's sequence number and flags offsets. It also converts the
out-of-order queue and receive queue helpers to use 32-bit sequence
arithmetic after window validation.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 792147b5279f9..7195da74d5c73 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -301,16 +303,21 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
> {
> struct sock *sk = (struct sock *)msk;
> struct rb_node **p, *parent;
> - u64 seq, end_seq, max_seq;
> + u64 end_seq, max_seq;
> struct sk_buff *skb1;
> + u32 seq;
>
> seq = MPTCP_SKB_CB(skb)->map_seq;
> - end_seq = MPTCP_SKB_CB(skb)->end_seq;
> + end_seq = MPTCP_SKB_CB(skb)->map_seq64 + skb->len;
> max_seq = atomic64_read(&msk->rcv_wnd_sent);
>
> - pr_debug("msk=%p seq=%llx limit=%llx empty=%d\n", msk, seq, max_seq,
> + pr_debug("msk=%p seq=%x limit=%llx empty=%d\n", msk, seq, max_seq,
> RB_EMPTY_ROOT(&msk->out_of_order_queue));
> - if (after64(end_seq, max_seq)) {
> + /* Use the full sequence space to perform the admission checks, to
> + * protect vs possible wrap-arounds.
> + */
> + if (after64(MPTCP_SKB_CB(skb)->map_seq64, max_seq) ||
> + after64(end_seq, max_seq)) {
[Severity: High]
Can a 64-bit sequence number wrap-around bypass this admission check when
msk->ack_seq has advanced past max_seq (msk->rcv_wnd_sent), allowing
out-of-window data to be queued and then dequeued via 32-bit truncation in
__mptcp_ofo_queue()?
In subflow_set_remote_key(), both msk->ack_seq and msk->rcv_wnd_sent are
initialized to subflow->iasn:
net/mptcp/subflow.c:subflow_set_remote_key() {
...
WRITE_ONCE(msk->ack_seq, subflow->iasn);
WRITE_ONCE(msk->can_ack, true);
atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
}
On an active MPTCP connection before the first outgoing DSS ACK updates
msk->rcv_wnd_sent via mptcp_set_rwin(), if in-order data of length N arrives
and is accepted by __mptcp_move_skb(), msk->ack_seq advances to
subflow->iasn + N while max_seq remains at subflow->iasn.
When msk->ack_seq > max_seq, a segment carrying a 64-bit data_seq in the
range [max_seq + 2^63 + 1, msk->ack_seq + 2^63 - 1] (for example,
subflow->iasn + 0x8000000000000000ULL + k with 1 <= k < N and k + len > N)
satisfies after64(map_seq64, msk->ack_seq) in __mptcp_move_skb(), but in
mptcp_data_queue_ofo() both after64(map_seq64, max_seq) and
after64(end_seq, max_seq) evaluate to false because max_seq - map_seq64 is
positive in signed 64-bit arithmetic.
Once queued into msk->out_of_order_queue, MPTCP_SKB_CB(skb)->map_seq and
end_seq are truncated to 32 bits, dropping bit 63 so that map_seq becomes
(u32)subflow->iasn + k. When __mptcp_ofo_queue() runs, after(map_seq, ack_seq)
is false and after(end_seq, ack_seq) is true, so the out-of-window SKB is
moved into sk->sk_receive_queue and advances msk->ack_seq.
Should mptcp_data_queue_ofo() clamp or validate max_seq against msk->ack_seq
(or check the distance from msk->ack_seq) before performing the window check?
[ ... ]
> @@ -421,11 +428,13 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset)
>
> /* the skb map_seq accounts for the skb offset:
> * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
> - * value
> + * value; note that end seq number is only available in 32bits format.
> */
> - MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow) -
> - offset;
> + MPTCP_SKB_CB(skb)->map_seq64 = mptcp_subflow_get_mapped_dsn(subflow) -
> + offset;
> + MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
> MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
[Severity: Critical]
As reported by Sashiko on v17 [1], does subtracting offset from map_seq64 and
map_seq without trimming the prefix bytes from skb corrupt the MPTCP data
stream when an SKB with offset > 0 carries an out-of-order mapping?
In __mptcp_move_skbs_from_subflow(), offset = tp->copied_seq - TCP_SKB_CB(skb)->seq
is non-zero when the first offset bytes of the head SKB on
ssk->sk_receive_queue were already consumed or discarded (for instance, when
a single subflow SKB spans two DSS mappings and mptcp_subflow_discard_data()
advances tp->copied_seq past the first mapping). Only the bytes in
[offset, skb->len) belong to the active DSS mapping starting at
mapped_dsn = mptcp_subflow_get_mapped_dsn(subflow).
When mapped_dsn > msk->ack_seq (out-of-order MPTCP data) and offset > 0,
subtracting offset in mptcp_init_skb() shifts map_seq64 and map_seq backwards
into the unreceived MPTCP sequence range [mapped_dsn - offset, mapped_dsn):
1. If mapped_dsn - offset <= msk->ack_seq, __mptcp_move_skb() treats the SKB
as in-sequence or partially overlapping msk->ack_seq, queues it directly
to sk->sk_receive_queue (or coalesces it into the tail SKB via
__mptcp_try_coalesce()), and advances msk->ack_seq across the unreceived
gap [msk->ack_seq, mapped_dsn), delivering the stale subflow prefix bytes
[0, offset) to user space.
2. If mapped_dsn - offset > msk->ack_seq, mptcp_data_queue_ofo() inserts the
SKB into out_of_order_queue covering [mapped_dsn - offset, mapped_dsn + len).
From there, __mptcp_try_coalesce() can merge the unpulled [0, offset)
prefix into a preceding OFO SKB ending at mapped_dsn - offset, subsequent
legitimate segments for [mapped_dsn - offset, mapped_dsn) are dropped as
duplicate data, and __mptcp_ofo_queue() dequeues the SKB as soon as
msk->ack_seq reaches mapped_dsn - offset rather than mapped_dsn.
> + MPTCP_SKB_CB(skb)->flags = 0;
> MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>
> __skb_unlink(skb, &ssk->sk_receive_queue);
> @@ -448,13 +457,14 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
> if (!skb)
> return;
>
> - MPTCP_SKB_CB(skb)->map_seq = mptcp_iasn(msk) - skb->len;
> + MPTCP_SKB_CB(skb)->map_seq64 = mptcp_iasn(msk) - skb->len;
> + MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
> MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
> }
[ ... ]
> @@ -2207,7 +2215,7 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
> int copied = 0;
>
> skb_queue_walk_safe(&sk->sk_receive_queue, skb, tmp) {
> - u64 offset = *seq - MPTCP_SKB_CB(skb)->map_seq;
> + u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq;
> u32 data_len = skb->len - offset;
> u32 count;
> int err;
> @@ -4618,7 +4626,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
> mptcp_move_skbs(sk);
>
> while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
> - offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
> + offset = (u32)msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
> if (offset < skb->len) {
> *off = offset;
> return skb;
[Severity: High]
What happens in mptcp_recvmsg() and mptcp_recv_skb() if mptcp_move_skbs()
spools the backlog while MPTCP_SYNC_SEQ is still pending on msk->cb_flags?
When subflow_set_remote_key() runs in softirq while the MPTCP socket is owned
by user space (for example, on a passive TFO socket during mptcp_recvmsg() or
__mptcp_read_sock()), it sets msk->ack_seq to subflow->iasn immediately, but
defers updating msk->copied_seq by setting MPTCP_SYNC_SEQ in msk->cb_flags:
net/mptcp/subflow.c:subflow_set_remote_key() {
...
if (!sock_owned_by_user(sk)) {
msk->copied_seq += subflow->iasn;
__mptcp_sync_rcv_sequence(sk);
} else {
set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
}
}
If data arrives in the 3rd ACK or immediately afterward while user space still
holds the socket lock, the incoming SKBs are queued to msk->backlog_list with
map_seq64 and map_seq in the subflow->iasn sequence space. Before
release_sock() runs mptcp_release_cb(), both mptcp_recvmsg() and
mptcp_recv_skb() call mptcp_move_skbs(sk):
1. Inside mptcp_move_skbs() -> __mptcp_move_skbs() -> __mptcp_move_skb(),
__mptcp_sync_rcv_sequence(sk) updates any remaining TFO SKB in
sk->sk_receive_queue to map_seq = (u32)(mptcp_iasn(msk) - skb->len) and
clears msk->rcvd_dummy_seq = false, while queuing the spooled SKBs with
IASN-based map_seq values into sk->sk_receive_queue. However,
msk->copied_seq is still in the pre-IASN sequence space.
2. In mptcp_recv_skb(), offset = (u32)msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq
is then off by -(u32)iasn, so offset >= skb->len holds whenever
(u32)(-iasn) >= skb->len and the loop eats and drops every queued SKB via
mptcp_eat_recv_skb(). Follow-up commit 68366c7579c4 ("mptcp: defer
read_sock cleanup to mptcp_worker") later removes the mptcp_move_skbs()
call from mptcp_recv_skb(), but does not address mptcp_recvmsg() or
mptcp_release_cb().
3. In mptcp_recvmsg(), after mptcp_move_skbs(sk) returns true, the loop
continues into __mptcp_recvmsg_mskq() with *seq still unsynchronized,
causing u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq to skip
unread SKBs or copy from a corrupted offset.
4. Even when release_sock() runs mptcp_release_cb(), __mptcp_move_skbs() runs
before the test for MPTCP_SYNC_SEQ:
net/mptcp/protocol.c:mptcp_release_cb() {
for (;;) {
...
if (__mptcp_move_skbs(sk, &join_list, &moved))
...
if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
msk->copied_seq += mptcp_iasn(msk);
__mptcp_sync_rcv_sequence(sk);
}
}
}
If the spooled backlog carries a DATA_FIN, __mptcp_move_skbs() calls
mptcp_check_data_fin(), which increments msk->ack_seq by 1 without
incrementing msk->bytes_received. When MPTCP_SYNC_SEQ then adds
mptcp_iasn(msk) (which is msk->ack_seq - msk->bytes_received +
msk->tfo_skb_len), msk->copied_seq is advanced by iasn + 1, skipping 1
byte of unread payload.
Should MPTCP_SYNC_SEQ be synchronized inside __mptcp_sync_rcv_sequence() (or
before spooling backlog SKBs in mptcp_move_skbs() and mptcp_release_cb()) so
that msk->copied_seq and the receive queue SKBs never end up in different
sequence spaces?
[1] https://lore.kernel.org/r/fe11356d3e5637de94fbe89540271d5f4ee381b4.1793326853.git.tanggeliang@kylinos.cn
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793349369.git.geliang@kernel.org?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (4 preceding siblings ...)
2026-10-30 8:41 ` [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-10-30 10:45 ` MPTCP CI
5 siblings, 0 replies; 11+ messages in thread
From: MPTCP CI @ 2026-10-30 10:45 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
Hi Paolo,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Unstable: 1 failed test(s): selftest_simult_flows(2) ⚠️
- 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/41030711574
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/81f9c2ee233f
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1196971
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] 11+ messages in thread
end of thread, other threads:[~2026-10-30 10:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 8:41 [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 1/5] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
2026-10-30 8:41 ` [PATCH mptcp-next v18 2/5] mptcp: drop the cant_coalesce CB field Geliang Tang
2026-10-30 9:02 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 3/5] mptcp: remove CB offset field Geliang Tang
2026-10-30 9:18 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 4/5] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
2026-10-30 9:19 ` sashiko-bot
2026-10-30 8:41 ` [PATCH mptcp-next v18 5/5] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
2026-10-30 9:08 ` sashiko-bot
2026-10-30 10:45 ` [PATCH mptcp-next v18 0/5] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
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.