* [PATCH mptcp-next 1/6] mptcp: drop the mptcp_ooo_try_coalesce() helper
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 2/6] mptcp: drop the cant_coalesce CB field Geliang Tang
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni
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.
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 954e20bb27de..d7838ab334fd 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -159,7 +159,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))
@@ -192,15 +193,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
@@ -275,7 +267,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;
@@ -321,7 +313,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;
}
@@ -751,8 +743,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);
@@ -876,7 +867,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.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next 2/6] mptcp: drop the cant_coalesce CB field
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 1/6] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 3/6] mptcp: remove CB offset field Geliang Tang
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni
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.
There is still a potential race in mptcp_inq_hint() that will be addressed
by a later patch in the series.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 2 +-
net/mptcp/protocol.c | 28 ++++++++++++++++++++++++++--
net/mptcp/protocol.h | 4 +++-
net/mptcp/subflow.c | 7 +++++++
4 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index f717750906ff..d6895c2200cc 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -49,11 +49,11 @@ 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));
+ mptcp_sk(sk)->rcvd_dummy_seq = true;
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 d7838ab334fd..c0b6e312816f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -160,7 +160,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))
@@ -357,7 +356,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);
@@ -408,6 +406,24 @@ static bool mptcp_prune_ofo_queue(struct sock *sk, u64 seq)
return mem <= sk->sk_rcvbuf;
}
+void __mptcp_sync_rcv_sequence(struct sock *sk)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ struct sk_buff *skb;
+
+ 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;
+
+ MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len;
+ 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;
@@ -416,6 +432,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);
+
/* Can't drop packets for fallback socket this late, or the stream
* will break.
*/
@@ -3833,6 +3855,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 da40c6f3705f..19b6eafece71 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -124,13 +124,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]))
@@ -310,6 +310,7 @@ struct mptcp_sock {
u32 token;
unsigned long flags;
unsigned long cb_flags;
+ bool rcvd_dummy_seq;
bool recovery; /* closing subflow write queue reinjected */
bool can_ack;
bool fully_established;
@@ -1169,6 +1170,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 8e386899ceb9..ea9b697c0300 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -478,6 +478,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
*/
@@ -496,6 +498,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.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next 3/6] mptcp: remove CB offset field
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 1/6] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 2/6] mptcp: drop the cant_coalesce CB field Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-30 1:32 ` Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 4/6] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni
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 was always ignored, now it
affects the `copied_seq` initial update: be sure to extends the sign
correctly of such mapping initialization time.
Overall this simplify a bit the __mptcp_recvmsg_mskq(), mptcp_inq_hint()
and the __mptcp_move_skb() code and will also make possible the next
patch.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 15 ++++--
net/mptcp/protocol.c | 126 +++++++++++++++++++------------------------
net/mptcp/protocol.h | 8 ++-
net/mptcp/subflow.c | 7 ++-
4 files changed, 77 insertions(+), 79 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index d6895c2200cc..421a50a85547 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;
@@ -44,20 +45,24 @@ 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);
DEBUG_NET_WARN_ON_ONCE(sock_owned_by_user_nocheck(sk));
- mptcp_sk(sk)->rcvd_dummy_seq = true;
+ 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);
- 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 c0b6e312816f..74a426bf6680 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -28,7 +28,7 @@
#include "protocol.h"
#include "mib.h"
-static unsigned int mptcp_inq_hint(const struct sock *sk);
+static unsigned int mptcp_inq_hint(struct sock *sk);
#define CREATE_TRACE_POINTS
#include <trace/events/mptcp.h>
@@ -160,7 +160,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;
@@ -342,8 +341,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;
@@ -352,9 +350,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);
@@ -420,8 +418,8 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
if (!skb)
return;
- MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len;
- 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)
@@ -450,6 +448,7 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
}
if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
+add_queue:
/* in sequence */
msk->bytes_received += copy_len;
WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
@@ -463,28 +462,18 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
} else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq)) {
mptcp_data_queue_ofo(msk, skb);
return false;
- }
+ } else if (after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
+ /* Partial packet: map_seq < ack_seq < end_seq. */
+ int delta = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
- /* Completely old data? */
- if (!after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
- mptcp_drop(sk, skb);
- return false;
+ copy_len -= delta;
+ goto add_queue;
}
- /* Partial packet: map_seq < ack_seq < end_seq.
- * Skip the already-acked bytes and enqueue the new data.
- */
- 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;
- msk->bytes_received += copy_len;
- WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
-
- skb_set_owner_r(skb, sk);
- __skb_queue_tail(&sk->sk_receive_queue, skb);
- return true;
+ /* Completely old data. */
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
+ mptcp_drop(sk, skb);
+ return false;
}
static void mptcp_stop_rtx_timer(struct sock *sk)
@@ -829,7 +818,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);
@@ -896,8 +885,6 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
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;
@@ -2134,34 +2121,23 @@ 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)
+ continue;
count = min_t(size_t, len - copied, data_len);
if (!(flags & MSG_TRUNC)) {
@@ -2179,14 +2155,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 {
@@ -2339,25 +2313,23 @@ static bool mptcp_move_skbs(struct sock *sk)
return enqueued;
}
-static unsigned int mptcp_inq_hint(const struct sock *sk)
+static unsigned int mptcp_inq_hint(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;
-
- return (unsigned int)hint_val;
- }
+ /* Avoid races vs ack_seq updates. */
+ mptcp_data_lock(sk);
+ hint_val = msk->ack_seq - msk->copied_seq;
+ mptcp_data_unlock(sk);
+ if (hint_val >= INT_MAX)
+ return INT_MAX;
- 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,
@@ -2366,6 +2338,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;
@@ -2385,6 +2358,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;
@@ -2394,7 +2372,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)
@@ -2449,6 +2427,10 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
err = 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);
@@ -3626,11 +3608,13 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->bytes_retrans = 0;
msk->rcvspace_init = 0;
msk->fastclosing = 0;
+ msk->tfo_skb_len = 0;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
WRITE_ONCE(msk->ack_seq, 0);
atomic64_set(&msk->rcv_wnd_sent, 0);
+ msk->copied_seq = 0;
WRITE_ONCE(sk->sk_shutdown, 0);
sk_error_report(sk);
@@ -3855,8 +3839,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);
+ }
}
}
@@ -4517,7 +4503,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;
@@ -4559,11 +4545,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 19b6eafece71..730af40ec9bc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -129,7 +129,6 @@
struct mptcp_skb_cb {
u64 map_seq;
u64 end_seq;
- u32 offset;
u8 has_rxtstamp;
};
@@ -289,6 +288,7 @@ struct mptcp_sock {
u64 bytes_sent;
u64 snd_nxt;
u64 bytes_received;
+ u64 copied_seq;
u64 ack_seq;
atomic64_t rcv_wnd_sent;
u64 rcv_data_fin_seq;
@@ -308,6 +308,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 rcvd_dummy_seq;
@@ -860,6 +861,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 ea9b697c0300..d0af5cb6f1b3 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -499,10 +499,13 @@ 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)) {
+ /* User space could have already read partially the TFO skb */
+ 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,
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 3/6] mptcp: remove CB offset field
2026-07-27 11:29 ` [PATCH mptcp-next 3/6] mptcp: remove CB offset field Geliang Tang
@ 2026-07-30 1:32 ` Geliang Tang
0 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2026-07-30 1:32 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni
Hi Paolo,
On Mon, 2026-07-27 at 19:29 +0800, Geliang Tang wrote:
> 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 was always ignored,
> now it
> affects the `copied_seq` initial update: be sure to extends the sign
> correctly of such mapping initialization time.
>
> Overall this simplify a bit the __mptcp_recvmsg_mskq(),
> mptcp_inq_hint()
> and the __mptcp_move_skb() code and will also make possible the next
> patch.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/fastopen.c | 15 ++++--
> net/mptcp/protocol.c | 126 +++++++++++++++++++----------------------
> --
> net/mptcp/protocol.h | 8 ++-
> net/mptcp/subflow.c | 7 ++-
> 4 files changed, 77 insertions(+), 79 deletions(-)
>
> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index d6895c2200cc..421a50a85547 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;
> @@ -44,20 +45,24 @@ 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);
> DEBUG_NET_WARN_ON_ONCE(sock_owned_by_user_nocheck(sk));
>
> - mptcp_sk(sk)->rcvd_dummy_seq = true;
> + 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);
> - 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 c0b6e312816f..74a426bf6680 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -28,7 +28,7 @@
> #include "protocol.h"
> #include "mib.h"
>
> -static unsigned int mptcp_inq_hint(const struct sock *sk);
> +static unsigned int mptcp_inq_hint(struct sock *sk);
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/mptcp.h>
> @@ -160,7 +160,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;
> @@ -342,8 +341,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;
> @@ -352,9 +350,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);
> @@ -420,8 +418,8 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
> if (!skb)
> return;
>
> - MPTCP_SKB_CB(skb)->map_seq = msk->ack_seq - skb->len;
> - 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)
Here Sashiko complained that the fastopen code does not update
msk->copied_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);
So I changed it to in v2:
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);
}
I'm not sure if this change is correct.
> @@ -450,6 +448,7 @@ static bool __mptcp_move_skb(struct sock *sk,
> struct sk_buff *skb)
> }
>
> if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
> +add_queue:
> /* in sequence */
> msk->bytes_received += copy_len;
> WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
> @@ -463,28 +462,18 @@ static bool __mptcp_move_skb(struct sock *sk,
> struct sk_buff *skb)
> } else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk-
> >ack_seq)) {
> mptcp_data_queue_ofo(msk, skb);
> return false;
> - }
> + } else if (after64(MPTCP_SKB_CB(skb)->end_seq, msk-
> >ack_seq)) {
> + /* Partial packet: map_seq < ack_seq < end_seq. */
> + int delta = msk->ack_seq - MPTCP_SKB_CB(skb)-
> >map_seq;
>
> - /* Completely old data? */
> - if (!after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
> - MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
> - mptcp_drop(sk, skb);
> - return false;
> + copy_len -= delta;
> + goto add_queue;
> }
>
> - /* Partial packet: map_seq < ack_seq < end_seq.
> - * Skip the already-acked bytes and enqueue the new data.
> - */
> - 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;
> - msk->bytes_received += copy_len;
> - WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
> -
> - skb_set_owner_r(skb, sk);
> - __skb_queue_tail(&sk->sk_receive_queue, skb);
> - return true;
> + /* Completely old data. */
> + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
> + mptcp_drop(sk, skb);
> + return false;
> }
>
> static void mptcp_stop_rtx_timer(struct sock *sk)
> @@ -829,7 +818,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);
> @@ -896,8 +885,6 @@ static bool __mptcp_ofo_queue(struct mptcp_sock
> *msk)
> 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;
> @@ -2134,34 +2121,23 @@ 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)
> + continue;
And here Sashiko complained about a risk of an infinite loop. So in the
next version, v2, I added *last = skb here:
/* Skip the already peeked data. */
if (offset >= skb->len) {
*last = skb;
continue;
}
Please give me some feedback.
Thanks,
-Geliang
>
> count = min_t(size_t, len - copied, data_len);
> if (!(flags & MSG_TRUNC)) {
> @@ -2179,14 +2155,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 {
> @@ -2339,25 +2313,23 @@ static bool mptcp_move_skbs(struct sock *sk)
> return enqueued;
> }
>
> -static unsigned int mptcp_inq_hint(const struct sock *sk)
> +static unsigned int mptcp_inq_hint(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;
> -
> - return (unsigned int)hint_val;
> - }
> + /* Avoid races vs ack_seq updates. */
> + mptcp_data_lock(sk);
> + hint_val = msk->ack_seq - msk->copied_seq;
> + mptcp_data_unlock(sk);
> + if (hint_val >= INT_MAX)
> + return INT_MAX;
>
> - 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,
> @@ -2366,6 +2338,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;
>
> @@ -2385,6 +2358,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;
> @@ -2394,7 +2372,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)
> @@ -2449,6 +2427,10 @@ static int mptcp_recvmsg(struct sock *sk,
> struct msghdr *msg, size_t len,
> err = 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);
> @@ -3626,11 +3608,13 @@ static int mptcp_disconnect(struct sock *sk,
> int flags)
> msk->bytes_retrans = 0;
> msk->rcvspace_init = 0;
> msk->fastclosing = 0;
> + msk->tfo_skb_len = 0;
> mptcp_init_rtt_est(msk);
>
> /* for fallback's sake */
> WRITE_ONCE(msk->ack_seq, 0);
> atomic64_set(&msk->rcv_wnd_sent, 0);
> + msk->copied_seq = 0;
>
> WRITE_ONCE(sk->sk_shutdown, 0);
> sk_error_report(sk);
> @@ -3855,8 +3839,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);
> + }
> }
> }
>
> @@ -4517,7 +4503,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;
> @@ -4559,11 +4545,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 19b6eafece71..730af40ec9bc 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -129,7 +129,6 @@
> struct mptcp_skb_cb {
> u64 map_seq;
> u64 end_seq;
> - u32 offset;
> u8 has_rxtstamp;
> };
>
> @@ -289,6 +288,7 @@ struct mptcp_sock {
> u64 bytes_sent;
> u64 snd_nxt;
> u64 bytes_received;
> + u64 copied_seq;
> u64 ack_seq;
> atomic64_t rcv_wnd_sent;
> u64 rcv_data_fin_seq;
> @@ -308,6 +308,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 rcvd_dummy_seq;
> @@ -860,6 +861,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 ea9b697c0300..d0af5cb6f1b3 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -499,10 +499,13 @@ 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)) {
> + /* User space could have already read partially the
> TFO skb */
> + 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,
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH mptcp-next 4/6] mptcp: sync mptcp skb cb layout with tcp one
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (2 preceding siblings ...)
2026-07-27 11:29 ` [PATCH mptcp-next 3/6] mptcp: remove CB offset field Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-27 11:29 ` [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue Geliang Tang
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni
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.
Add build time check to ensure the latter invariant.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 6 ++--
net/mptcp/protocol.c | 82 +++++++++++++++++++++++++++-----------------
net/mptcp/protocol.h | 7 ++--
3 files changed, 60 insertions(+), 35 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 74a426bf6680..b9e44c64c620 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -164,7 +164,7 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
!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;
@@ -234,14 +234,18 @@ 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,
+ /* Use the full sequence space to perform the admission checks, to
+ * protect vs possible wrap-arounds.
+ */
+ 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)) {
/* out of window */
@@ -272,7 +276,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;
@@ -284,18 +288,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(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 |
@@ -326,7 +330,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);
@@ -348,11 +352,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);
@@ -418,13 +424,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;
@@ -447,7 +454,7 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
return false;
}
- if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
+ if (MPTCP_SKB_CB(skb)->map_seq64 == msk->ack_seq) {
add_queue:
/* in sequence */
msk->bytes_received += copy_len;
@@ -459,12 +466,13 @@ 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;
- } else if (after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
+ } else if (after64(MPTCP_SKB_CB(skb)->map_seq64 + skb->len,
+ msk->ack_seq)) {
/* Partial packet: map_seq < ack_seq < end_seq. */
- int delta = msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
+ int delta = (u32)msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
copy_len -= delta;
goto add_queue;
@@ -855,40 +863,40 @@ 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;
+ int delta = 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,
+ pr_debug("uncoalesced seq=%x ack seq=%x delta=%d\n",
+ MPTCP_SKB_CB(skb)->map_seq, ack_seq,
delta);
__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;
@@ -2130,7 +2138,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;
@@ -4503,7 +4511,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;
@@ -4754,11 +4762,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 730af40ec9bc..8b16b0a4eb9f 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -127,9 +127,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.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (3 preceding siblings ...)
2026-07-27 11:29 ` [PATCH mptcp-next 4/6] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-29 8:08 ` Paolo Abeni
2026-07-27 11:29 ` [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker Geliang Tang
2026-07-27 12:37 ` [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
6 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
From: Geliang Tang <tanggeliang@kylinos.cn>
After the CB offset field removal, the msk receive queue tracks the
consumed position with the msk-level copied_seq and each skb map_seq
points at its first byte (skb->data[0]). When the same DSN range is
delivered more than once - a partially-acked segment, or duplicate
data arriving on a second subflow - the skb is queued whole: its
map_seq sits below msk->ack_seq and the duplicated bytes stay
physically at the front of the skb.
The linear readers (recvmsg, read_sock, read_done) cope with that by
computing a per-skb offset = copied_seq - map_seq and skipping it. But
consumers that treat the receive queue as a single contiguous byte
stream cannot: the TLS strparser builds an anchor whose frag_list is
the receive-queue skbs and reads it with a plain skb_copy_bits(), which
has no per-skb offset knowledge. A record spanning such an skb boundary
then reads the duplicated prefix and gets corrupted.
Physically drop the duplicated leading bytes at enqueue time instead,
so the receive queue is always contiguous. Add mptcp_trim_dup_head(),
modelled on tcp_trim_head()/__pskb_trim_head() but tolerating a
non-empty linear area: it pulls the linear head first, then eats the
remaining bytes from the paged frags, and the caller bumps map_seq
accordingly. The skb truesize is left unchanged on purpose - dropping
only skb->len/data_len keeps the memory accounting over-reserved, hence
always safe, at both callers and avoids any rmem/fwd_alloc fixup.
Two callers trim the overlap:
- __mptcp_move_skb() partial-packet branch (map_seq < ack_seq <
end_seq). The skb is not owned yet, so on the -ENOMEM unclone
failure refund the truesize borrowed by mptcp_borrow_fwdmem()
before dropping it.
- __mptcp_ofo_queue() uncoalesced branch. The skb is already msk
owned, so mptcp_drop() refunds it; skip advancing ack_seq on
failure.
After trimming, map_seq equals the queue tail end_seq, so the segment
can be coalesced normally.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
net/mptcp/protocol.c | 79 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 78 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b9e44c64c620..09b888ff33c7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -11,6 +11,7 @@
#include <linux/netdevice.h>
#include <linux/sched/signal.h>
#include <linux/atomic.h>
+#include <linux/skbuff_ref.h>
#include <net/aligned_data.h>
#include <net/rps.h>
#include <net/sock.h>
@@ -429,6 +430,62 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;
}
+static int mptcp_trim_dup_head(struct sk_buff *skb, int delta)
+{
+ struct skb_shared_info *shinfo;
+ int headlen, eat, i, k;
+
+ if (delta <= 0)
+ return 0;
+
+ /* Received skbs are not expected to carry a frag_list; the frag loop
+ * below only handles the linear area and the paged frags.
+ */
+ DEBUG_NET_WARN_ON_ONCE(skb_has_frag_list(skb));
+
+ if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
+ return -ENOMEM;
+
+ /* Eat the linear head first, then the paged frags. Note the skb
+ * truesize is left unchanged on purpose: dropping only skb->len /
+ * skb->data_len keeps the memory accounting over-reserved (hence
+ * always safe) at both callers.
+ */
+ headlen = skb_headlen(skb);
+ eat = min(delta, headlen);
+ if (eat) {
+ __skb_pull(skb, eat);
+ delta -= eat;
+ }
+ if (!delta)
+ return 0;
+
+ shinfo = skb_shinfo(skb);
+ eat = delta;
+ k = 0;
+ for (i = 0; i < shinfo->nr_frags; i++) {
+ int size = skb_frag_size(&shinfo->frags[i]);
+
+ if (size <= eat) {
+ skb_frag_unref(skb, i);
+ eat -= size;
+ } else {
+ shinfo->frags[k] = shinfo->frags[i];
+ if (eat) {
+ skb_frag_off_add(&shinfo->frags[k], eat);
+ skb_frag_size_sub(&shinfo->frags[k], eat);
+ eat = 0;
+ }
+ k++;
+ }
+ }
+ shinfo->nr_frags = k;
+
+ skb->data_len -= delta;
+ skb->len -= delta;
+ return 0;
+}
+
static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
{
u32 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
@@ -474,6 +531,16 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
/* Partial packet: map_seq < ack_seq < end_seq. */
int delta = (u32)msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
+ if (mptcp_trim_dup_head(skb, delta)) {
+ /* skb is not owned yet: mptcp_borrow_fwdmem() added its
+ * truesize to sk_forward_alloc and cleared skb->sk, so
+ * mptcp_drop() won't refund it. Do it here.
+ */
+ sk_forward_alloc_add(sk, -skb->truesize);
+ mptcp_drop(sk, skb);
+ return false;
+ }
+ MPTCP_SKB_CB(skb)->map_seq += delta;
copy_len -= delta;
goto add_queue;
}
@@ -889,10 +956,20 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
if (!tail || !mptcp_try_coalesce(sk, tail, skb)) {
int delta = ack_seq - MPTCP_SKB_CB(skb)->map_seq;
- /* skip overlapping data, if any */
+ /* Physically trim the overlapping prefix, if any,
+ * so the receive queue stays contiguous.
+ */
pr_debug("uncoalesced seq=%x ack seq=%x delta=%d\n",
MPTCP_SKB_CB(skb)->map_seq, ack_seq,
delta);
+ if (mptcp_trim_dup_head(skb, delta)) {
+ /* skb is msk-owned here; mptcp_drop() refunds
+ * it. Skip advancing ack_seq/bytes_received.
+ */
+ mptcp_drop(sk, skb);
+ continue;
+ }
+ MPTCP_SKB_CB(skb)->map_seq += delta;
__skb_queue_tail(&sk->sk_receive_queue, skb);
}
msk->bytes_received += seq_delta;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue
2026-07-27 11:29 ` [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue Geliang Tang
@ 2026-07-29 8:08 ` Paolo Abeni
2026-07-30 1:15 ` Geliang Tang
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2026-07-29 8:08 UTC (permalink / raw)
To: Geliang Tang, mptcp; +Cc: Geliang Tang
On 7/27/26 1:29 PM, Geliang Tang wrote:
> The linear readers (recvmsg, read_sock, read_done) cope with that by
> computing a per-skb offset = copied_seq - map_seq and skipping it. But
> consumers that treat the receive queue as a single contiguous byte
> stream cannot: the TLS strparser builds an anchor whose frag_list is
> the receive-queue skbs and reads it with a plain skb_copy_bits(), which
> has no per-skb offset knowledge. A record spanning such an skb boundary
> then reads the duplicated prefix and gets corrupted.
The above raises a question.
AFAICS, the critical skb layout is also possible with plain TCP -
possibly is just less likely. How does TLS deal with that? I read the
above as the TLS stream get corrupted, which sounds suspiciously too
fragile to me?!? Or did I miss something?
This change adds a lot of complexity to the rx path, we want to avoid it.
/P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue
2026-07-29 8:08 ` Paolo Abeni
@ 2026-07-30 1:15 ` Geliang Tang
2026-07-30 15:00 ` Paolo Abeni
0 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2026-07-30 1:15 UTC (permalink / raw)
To: Paolo Abeni, mptcp; +Cc: Geliang Tang
Hi Paolo,
On Wed, 2026-07-29 at 10:08 +0200, Paolo Abeni wrote:
> On 7/27/26 1:29 PM, Geliang Tang wrote:
> > The linear readers (recvmsg, read_sock, read_done) cope with that
> > by
> > computing a per-skb offset = copied_seq - map_seq and skipping it.
> > But
> > consumers that treat the receive queue as a single contiguous byte
> > stream cannot: the TLS strparser builds an anchor whose frag_list
> > is
> > the receive-queue skbs and reads it with a plain skb_copy_bits(),
> > which
> > has no per-skb offset knowledge. A record spanning such an skb
> > boundary
> > then reads the duplicated prefix and gets corrupted.
>
> The above raises a question.
>
> AFAICS, the critical skb layout is also possible with plain TCP -
> possibly is just less likely. How does TLS deal with that? I read the
> above as the TLS stream get corrupted, which sounds suspiciously too
> fragile to me?!? Or did I miss something?
My description was inaccurate. This only occurs in the MPTCP out-of-
order scenario. It does not happen with TCP.
>
> This change adds a lot of complexity to the rx path, we want to avoid
> it.
I agree with you. This helper mptcp_trim_dup_head() does not need to be
called in __mptcp_move_skb(). It only needs to be called when
overlapping data occurs in __mptcp_ofo_queue(). This way, it won't
affect the efficiency of the rx path.
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;
p = rb_first(&msk->out_of_order_queue);
while (p) {
... ...
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 = ack_seq - MPTCP_SKB_CB(skb)->map_seq;
/* trim overlapping prefix, if any */
pr_debug("uncoalesced seq=%x ack seq=%x delta=%d\n",
MPTCP_SKB_CB(skb)->map_seq, ack_seq,
delta);
if (mptcp_trim_head(skb, delta)) {
mptcp_drop(sk, skb);
continue;
}
MPTCP_SKB_CB(skb)->map_seq += delta;
__skb_queue_tail(&sk->sk_receive_queue, skb);
}
msk->bytes_received += seq_delta;
WRITE_ONCE(msk->ack_seq, msk->ack_seq + seq_delta);
moved = true;
}
return moved;
}
This helper is actually a mirror of the tcp_trim_head(). If we could
export and reuse TCP's __pskb_trim_head(), this helper would become
much simpler:
static int mptcp_trim_head(struct sk_buff *skb, int delta)
{
int eat;
if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
return -ENOMEM;
eat = min_t(int, delta, skb_headlen(skb));
if (eat) {
__skb_pull(skb, eat);
delta -= eat;
}
if (delta) {
__pskb_trim_head(skb, delta);
skb->len += skb_headlen(skb);
}
return 0;
}
Would this implementation be better? Please give me some feedback.
Thanks,
-Geliang
>
> /P
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue
2026-07-30 1:15 ` Geliang Tang
@ 2026-07-30 15:00 ` Paolo Abeni
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Abeni @ 2026-07-30 15:00 UTC (permalink / raw)
To: Geliang Tang, mptcp; +Cc: Geliang Tang
On 7/30/26 3:15 AM, Geliang Tang wrote:
> On Wed, 2026-07-29 at 10:08 +0200, Paolo Abeni wrote:
>> On 7/27/26 1:29 PM, Geliang Tang wrote:
>>> The linear readers (recvmsg, read_sock, read_done) cope with that
>>> by
>>> computing a per-skb offset = copied_seq - map_seq and skipping it.
>>> But
>>> consumers that treat the receive queue as a single contiguous byte
>>> stream cannot: the TLS strparser builds an anchor whose frag_list
>>> is
>>> the receive-queue skbs and reads it with a plain skb_copy_bits(),
>>> which
>>> has no per-skb offset knowledge. A record spanning such an skb
>>> boundary
>>> then reads the duplicated prefix and gets corrupted.
>>
>> The above raises a question.
>>
>> AFAICS, the critical skb layout is also possible with plain TCP -
>> possibly is just less likely. How does TLS deal with that? I read the
>> above as the TLS stream get corrupted, which sounds suspiciously too
>> fragile to me?!? Or did I miss something?
>
> My description was inaccurate. This only occurs in the MPTCP out-of-
> order scenario. It does not happen with TCP.
I mean: even for plain TCP skbs can sits in the receive queue with some
heading bytes overlapping with already received ones, and skipped at
read time due to `copied_seq`, how comes that TLS has no problem is such
a case?
/P
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (4 preceding siblings ...)
2026-07-27 11:29 ` [PATCH mptcp-next 5/6] mptcp: trim the duplicated skb head at receive enqueue Geliang Tang
@ 2026-07-27 11:29 ` Geliang Tang
2026-07-29 8:55 ` Paolo Abeni
2026-07-27 12:37 ` [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
6 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2026-07-27 11:29 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
From: Geliang Tang <tanggeliang@kylinos.cn>
When MPTCP carries TLS, the data path runs under mptcp_data_lock().
Reaching sk->sk_data_ready(sk) synchronously ends up at
tls_strp_check_rcv() -> mptcp_recv_skb() -> mptcp_move_skbs(), which
calls mptcp_data_lock() on the same sk and recurses on sk_lock.slock.
Fix this by deferring sk->sk_data_ready(sk) to mptcp_worker() via a
new MPTCP_WORK_DATA_READY bit, re-using the existing
mptcp_schedule_work()/mptcp_cancel_work() infrastructure. The wakeup
bit is consumed after the SOCK_DEAD && TCP_CLOSE destroy branch, so a
socket that reaches the destroy path drops the pending wakeup rather
than running it post-free.
No new work_struct, workqueue or cancel path is introduced; only a new
flag bit and the corresponding set_bit()/test_and_clear_bit() in the
mptcp_worker() body.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
net/mptcp/protocol.c | 9 +++++++--
net/mptcp/protocol.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 09b888ff33c7..be139718ed15 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1085,8 +1085,10 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
mptcp_rcv_rtt_update(msk, subflow);
if (!sock_owned_by_user(sk)) {
/* Wake-up the reader only for in-sequence data */
- if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk))
- sk->sk_data_ready(sk);
+ if (move_skbs_to_msk(msk, ssk) && mptcp_epollin_ready(sk)) {
+ set_bit(MPTCP_WORK_DATA_READY, &msk->flags);
+ mptcp_schedule_work(sk);
+ }
} else {
__mptcp_move_skbs_from_subflow(msk, ssk, false);
}
@@ -3223,6 +3225,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_DATA_READY, &msk->flags))
+ sk->sk_data_ready(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);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 8b16b0a4eb9f..7ba8b78ac6e4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -115,6 +115,7 @@
#define MPTCP_WORK_RTX 1
#define MPTCP_FALLBACK_DONE 2
#define MPTCP_WORK_CLOSE_SUBFLOW 3
+#define MPTCP_WORK_DATA_READY 4
/* MPTCP socket release cb flags */
#define MPTCP_PUSH_PENDING 1
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker
2026-07-27 11:29 ` [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker Geliang Tang
@ 2026-07-29 8:55 ` Paolo Abeni
2026-07-30 0:57 ` Geliang Tang
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2026-07-29 8:55 UTC (permalink / raw)
To: Geliang Tang, mptcp; +Cc: Geliang Tang
On 7/27/26 1:29 PM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> When MPTCP carries TLS, the data path runs under mptcp_data_lock().
> Reaching sk->sk_data_ready(sk) synchronously ends up at
> tls_strp_check_rcv() -> mptcp_recv_skb() -> mptcp_move_skbs(), which
> calls mptcp_data_lock() on the same sk and recurses on sk_lock.slock.
Is mptcp_move_skbs() really needed in mptcp_recv_skb()? Anyway it looks
like that is not the only constraint: AFAICS, before the mptcp_recv_skb()
calls, the TLS code would call __mptcp_read_sock() which in turns calls
mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() that requires holding
the msk socket lock in process context, while the mptcp/TLS caller is in
BH scope.
> Fix this by deferring sk->sk_data_ready(sk) to mptcp_worker() via a
> new MPTCP_WORK_DATA_READY bit, re-using the existing
> mptcp_schedule_work()/mptcp_cancel_work() infrastructure. The wakeup
> bit is consumed after the SOCK_DEAD && TCP_CLOSE destroy branch, so a
> socket that reaches the destroy path drops the pending wakeup rather
> than running it post-free.
I think that unconditionally adding the work latency for non-TLS application
is a no-go.
Instead I *think* that the constraints in __mptcp_read_sock() could be relaxed
with something alike the following (completely untested):
---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ca644ec53eed..b09e267f5d46 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2986,6 +2986,15 @@ static void mptcp_do_fastclose(struct sock *sk)
}
}
+static void mptcp_read_complete(struct sock *sk)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+
+ mptcp_cleanup_rbuf(msk, msk->read_copied);
+ mptcp_rcv_space_adjust(msk, msk->read_copied);
+ msk->read_copied = 0;
+}
+
static void mptcp_worker(struct work_struct *work)
{
struct mptcp_sock *msk = container_of(work, struct mptcp_sock, work);
@@ -3026,6 +3035,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);
@@ -4381,9 +4393,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 = MPTCP_SKB_CB(skb)->offset;
if (offset < skb->len) {
@@ -4395,10 +4404,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
return NULL;
}
-/*
- * 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)
{
@@ -4441,11 +4447,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 the workers's release callback.
+ */
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 4a2d40cd7b13..89902a98d383 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -115,6 +115,7 @@
#define MPTCP_WORK_RTX 1
#define MPTCP_FALLBACK_DONE 2
#define MPTCP_WORK_CLOSE_SUBFLOW 3
+#define MPTCP_WORK_READ_COMPLETE 4
/* MPTCP socket release cb flags */
#define MPTCP_PUSH_PENDING 1
@@ -305,6 +306,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;
unsigned long flags;
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker
2026-07-29 8:55 ` Paolo Abeni
@ 2026-07-30 0:57 ` Geliang Tang
2026-07-30 15:18 ` Paolo Abeni
0 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2026-07-30 0:57 UTC (permalink / raw)
To: Paolo Abeni, mptcp; +Cc: Geliang Tang
Hi Paolo,
On Wed, 2026-07-29 at 10:55 +0200, Paolo Abeni wrote:
> On 7/27/26 1:29 PM, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > When MPTCP carries TLS, the data path runs under mptcp_data_lock().
> > Reaching sk->sk_data_ready(sk) synchronously ends up at
> > tls_strp_check_rcv() -> mptcp_recv_skb() -> mptcp_move_skbs(),
> > which
> > calls mptcp_data_lock() on the same sk and recurses on
> > sk_lock.slock.
>
> Is mptcp_move_skbs() really needed in mptcp_recv_skb()? Anyway it
> looks
> like that is not the only constraint: AFAICS, before the
> mptcp_recv_skb()
> calls, the TLS code would call __mptcp_read_sock() which in turns
> calls
> mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() that requires
> holding
> the msk socket lock in process context, while the mptcp/TLS caller is
> in
> BH scope.
>
> > Fix this by deferring sk->sk_data_ready(sk) to mptcp_worker() via a
> > new MPTCP_WORK_DATA_READY bit, re-using the existing
> > mptcp_schedule_work()/mptcp_cancel_work() infrastructure. The
> > wakeup
> > bit is consumed after the SOCK_DEAD && TCP_CLOSE destroy branch, so
> > a
> > socket that reaches the destroy path drops the pending wakeup
> > rather
> > than running it post-free.
>
> I think that unconditionally adding the work latency for non-TLS
> application
> is a no-go.
>
> Instead I *think* that the constraints in __mptcp_read_sock() could
> be relaxed
> with something alike the following (completely untested):
Thank you for your patch. It is very useful, but when running TLS
tests, it deadlocks with the mptcp_data_lock in mptcp_inq_hint(). TLS
calls mptcp_inq(), and my implementation of mptcp_inq() is a wrapper
around mptcp_inq_hint():
static int mptcp_inq(struct sock *sk)
{
int answ;
if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
answ = 0;
} else {
answ = mptcp_inq_hint(sk);
if (answ &&
(sk->sk_state == TCP_CLOSE ||
(sk->sk_shutdown & RCV_SHUTDOWN)))
answ--;
}
return answ;
}
To eliminate this deadlock, I had to remove the mptcp_data_lock() from
mptcp_inq_hint() and replace it with READ_ONCE(). I'm not sure if this
is problematic:
@@ static unsigned int mptcp_inq_hint(struct sock *sk)
const struct mptcp_sock *msk = mptcp_sk(sk);
u64 hint_val;
- /* Avoid races vs ack_seq updates. */
- mptcp_data_lock(sk);
- hint_val = msk->ack_seq - msk->copied_seq;
- mptcp_data_unlock(sk);
+ hint_val = READ_ONCE(msk->ack_seq) - READ_ONCE(msk->copied_seq);
if (hint_val >= INT_MAX)
return INT_MAX;
Additionally, mptcp_read_done() also needs similar modifications to
those made in __mptcp_read_sock():
@@ static void mptcp_read_done(struct sock *sk, size_t len)
mptcp_eat_recv_skb(sk, skb);
}
- mptcp_rcv_space_adjust(msk, len - left);
-
/* Clean up data we have read: This will do ACK frames. */
- if (left != len)
- mptcp_cleanup_rbuf(msk, len - left);
+ if (left != len) {
+ msk->read_copied = len - left;
+ set_bit(MPTCP_WORK_READ_COMPLETE, &msk->flags);
+ mptcp_schedule_work(sk);
+ }
}
Currently, the implementation of mptcp_read_done() is as follows:
static void mptcp_read_done(struct sock *sk, size_t len)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct sk_buff *skb;
size_t left;
u32 offset;
msk_owned_by_me(msk);
if (sk->sk_state == TCP_LISTEN)
return;
left = len;
while (left && (skb = mptcp_recv_skb(sk, &offset)) != NULL) {
int used;
used = min_t(size_t, skb->len - offset, left);
msk->bytes_consumed += used;
msk->copied_seq += used;
left -= used;
if (skb->len > offset + used)
break;
mptcp_eat_recv_skb(sk, skb);
}
/* Clean up data we have read: This will do ACK frames. */
if (left != len) {
msk->read_copied = len - left;
set_bit(MPTCP_WORK_READ_COMPLETE, &msk->flags);
mptcp_schedule_work(sk);
}
}
Another issue, unrelated to this patch, is that I have defined a
.read_done interface for TLS, with tcp_read_done() and
mptcp_read_done() corresponding to TCP and MPTCP, respectively. This
.read_done interface is very similar to .read_sock, and .read_sock is a
generic interface in struct proto_ops. I'm wondering whether we could
add a .read_done interface to struct proto_ops, so that in TLS we could
call the protocol-specific .read_done via sk->sk_socket->ops-
>read_done(). I'm not sure if this is a good idea or whether upstream
would accept it. I'd like to hear your opinion.
Thank you very much.
-Geliang
> ---
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index ca644ec53eed..b09e267f5d46 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2986,6 +2986,15 @@ static void mptcp_do_fastclose(struct sock
> *sk)
> }
> }
>
> +static void mptcp_read_complete(struct sock *sk)
> +{
> + struct mptcp_sock *msk = mptcp_sk(sk);
> +
> + mptcp_cleanup_rbuf(msk, msk->read_copied);
> + mptcp_rcv_space_adjust(msk, msk->read_copied);
> + msk->read_copied = 0;
> +}
> +
> static void mptcp_worker(struct work_struct *work)
> {
> struct mptcp_sock *msk = container_of(work, struct
> mptcp_sock, work);
> @@ -3026,6 +3035,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);
> @@ -4381,9 +4393,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 = MPTCP_SKB_CB(skb)->offset;
> if (offset < skb->len) {
> @@ -4395,10 +4404,7 @@ static struct sk_buff *mptcp_recv_skb(struct
> sock *sk, u32 *off)
> return NULL;
> }
>
> -/*
> - * 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)
> {
> @@ -4441,11 +4447,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 the workers's release
> callback.
> + */
> 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 4a2d40cd7b13..89902a98d383 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -115,6 +115,7 @@
> #define MPTCP_WORK_RTX 1
> #define MPTCP_FALLBACK_DONE 2
> #define MPTCP_WORK_CLOSE_SUBFLOW 3
> +#define MPTCP_WORK_READ_COMPLETE 4
>
> /* MPTCP socket release cb flags */
> #define MPTCP_PUSH_PENDING 1
> @@ -305,6 +306,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;
> unsigned long flags;
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker
2026-07-30 0:57 ` Geliang Tang
@ 2026-07-30 15:18 ` Paolo Abeni
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Abeni @ 2026-07-30 15:18 UTC (permalink / raw)
To: Geliang Tang, mptcp; +Cc: Geliang Tang
On 7/30/26 2:57 AM, Geliang Tang wrote:
> On Wed, 2026-07-29 at 10:55 +0200, Paolo Abeni wrote:
>> On 7/27/26 1:29 PM, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> When MPTCP carries TLS, the data path runs under mptcp_data_lock().
>>> Reaching sk->sk_data_ready(sk) synchronously ends up at
>>> tls_strp_check_rcv() -> mptcp_recv_skb() -> mptcp_move_skbs(),
>>> which
>>> calls mptcp_data_lock() on the same sk and recurses on
>>> sk_lock.slock.
>>
>> Is mptcp_move_skbs() really needed in mptcp_recv_skb()? Anyway it
>> looks
>> like that is not the only constraint: AFAICS, before the
>> mptcp_recv_skb()
>> calls, the TLS code would call __mptcp_read_sock() which in turns
>> calls
>> mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() that requires
>> holding
>> the msk socket lock in process context, while the mptcp/TLS caller is
>> in
>> BH scope.
>>
>>> Fix this by deferring sk->sk_data_ready(sk) to mptcp_worker() via a
>>> new MPTCP_WORK_DATA_READY bit, re-using the existing
>>> mptcp_schedule_work()/mptcp_cancel_work() infrastructure. The
>>> wakeup
>>> bit is consumed after the SOCK_DEAD && TCP_CLOSE destroy branch, so
>>> a
>>> socket that reaches the destroy path drops the pending wakeup
>>> rather
>>> than running it post-free.
>>
>> I think that unconditionally adding the work latency for non-TLS
>> application
>> is a no-go.
>>
>> Instead I *think* that the constraints in __mptcp_read_sock() could
>> be relaxed
>> with something alike the following (completely untested):
>
> Thank you for your patch. It is very useful, but when running TLS
> tests, it deadlocks with the mptcp_data_lock in mptcp_inq_hint().
This looks like an unrelated problem.
AFAICS tls calls tcp_in() under the sk socket lock, and the helper is
implemented accordingly (in fact is separated from tcp_inq_hint()). Even
the mptcp helper must use the same assumption. No need to call
additional locking, nor (AFAICS) ONCE annoation, as `ack_seq` and
`copied_seq` should be modified only under the msk socket lock.
> Currently, the implementation of mptcp_read_done() is as follows:
>
> static void mptcp_read_done(struct sock *sk, size_t len)
> {
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct sk_buff *skb;
> size_t left;
> u32 offset;
>
> msk_owned_by_me(msk);
>
> if (sk->sk_state == TCP_LISTEN)
> return;
>
> left = len;
> while (left && (skb = mptcp_recv_skb(sk, &offset)) != NULL) {
> int used;
>
> used = min_t(size_t, skb->len - offset, left);
> msk->bytes_consumed += used;
> msk->copied_seq += used;
> left -= used;
>
> if (skb->len > offset + used)
> break;
>
> mptcp_eat_recv_skb(sk, skb);
> }
>
> /* Clean up data we have read: This will do ACK frames. */
> if (left != len) {
> msk->read_copied = len - left;
Possibly both here and in __mptcp_read_sock() should be:
msk->read_copied += len - left
(note the '+=' operator instead of '=')
/P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage
2026-07-27 11:29 [PATCH mptcp-next 0/6] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (5 preceding siblings ...)
2026-07-27 11:29 ` [PATCH mptcp-next 6/6] mptcp: defer sk_data_ready to the worker Geliang Tang
@ 2026-07-27 12:37 ` MPTCP CI
6 siblings, 0 replies; 15+ messages in thread
From: MPTCP CI @ 2026-07-27 12:37 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
Hi Geliang,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/30263550148
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/08f9af979c9b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1135094
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] 15+ messages in thread