* [PATCH mptcp-net v24 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-10 23:32 ` netdev-bot+sashiko
2026-11-09 9:24 ` [PATCH mptcp-next v24 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
` (7 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
From: Geliang Tang <tanggeliang@kylinos.cn>
Fix mptcp_inq_hint() to compute available bytes using mptcp_data_avail()
(bytes_received - bytes_consumed) instead of ack_seq minus the head skb's
map_seq.
On passive TFO connections, the TFO skb has map_seq set to -skb->len.
Without the removed __mptcp_fastopen_gen_msk_ackseq() to adjust the TFO
skb's map_seq, mptcp_inq_hint() computes "ack_seq - map_seq = iasn + len",
almost always >= INT_MAX, causing SIOCINQ/FIONREAD and TCP_CM_INQ to
report INT_MAX.
Using mptcp_data_avail() avoids this by tracking bytes directly. This
change also affects non-TFO connections after DATA_FIN: with N bytes
unread, SIOCINQ now reports N instead of N+1, aligning with TCP's
SIOCINQ behavior. The EOF hint of 1 is preserved for the empty-queue
case when the socket is closed or shutdown.
Fixes: f03afb3aeb9d ("mptcp: drop __mptcp_fastopen_gen_msk_ackseq()")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
net/mptcp/protocol.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 02a4f479d9d8..fc4ffa326317 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2402,22 +2402,16 @@ 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;
+ u64 hint_val = mptcp_data_avail(msk);
- 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;
- 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,
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH mptcp-net v24 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
2026-11-09 9:24 ` [PATCH mptcp-net v24 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
@ 2026-11-10 23:32 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-11-10 23:32 UTC (permalink / raw)
To: geliang; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] In net/mptcp/protocol.c, mptcp_inq_hint() now returns
mptcp_data_avail() (bytes_received - bytes_consumed) for every caller.
- [Low] The commit message names only SIOCINQ/FIONREAD and TCP_CM_INQ as
hit by the bogus INT_MAX hint.
- [Low] Author and sign-off addresses differ.
--
mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
This commit changes mptcp_inq_hint() to compute the available bytes with
mptcp_data_avail() (bytes_received - bytes_consumed) instead of ack_seq
minus the head skb's map_seq. This fixes a bogus INT_MAX hint on passive
TFO connections, where the TFO skb has map_seq set to -skb->len.
> On passive TFO connections, the TFO skb has map_seq set to -skb->len.
> Without the removed __mptcp_fastopen_gen_msk_ackseq() to adjust the TFO
> skb's map_seq, mptcp_inq_hint() computes "ack_seq - map_seq = iasn + len",
> almost always >= INT_MAX, causing SIOCINQ/FIONREAD and TCP_CM_INQ to
> report INT_MAX.
[Severity: Low]
Should this list also mention receive buffer autotuning? Another caller of
mptcp_inq_hint(), mptcp_rcv_space_adjust() in net/mptcp/protocol.c, got
the same bogus value:
net/mptcp/protocol.c:mptcp_rcv_space_adjust() {
...
copied = msk->rcvq_space.copied;
copied -= mptcp_inq_hint(sk);
if (copied <= msk->rcvq_space.space)
goto new_measure;
...
}
On a passive TFO socket, after the remote key arrives, the TFO skb at the
head of the receive queue has map_seq = -skb->len. That value is set in
mptcp_fastopen_subflow_synack_set_params(). The old hint was clamped to
INT_MAX, so copied went negative, the code jumped to new_measure, and
mptcp_rcvbuf_grow() was never called.
A partial read only advances map_seq by count in __mptcp_recvmsg_mskq().
So the hint stayed wrong, and autotuning stayed off until the whole TFO
skb had been read.
This patch fixes that path as well. Could the commit message say so? It
would help stable backporters judge the fix.
> Fixes: f03afb3aeb9d ("mptcp: drop __mptcp_fastopen_gen_msk_ackseq()")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
[Severity: Low]
This isn't a bug, but the author address (geliang@kernel.org) does not
match the only Signed-off-by address (tanggeliang@kylinos.cn).
checkpatch.pl reports this as FROM_SIGN_OFF_MISMATCH.
Could the author address be updated, or a matching Signed-off-by be added?
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..fc4ffa3263176 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2402,22 +2402,16 @@ 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;
> + u64 hint_val = mptcp_data_avail(msk);
[Severity: Low]
mptcp_inq_hint() feeds more than SIOCINQ. It also supplies the TCP_CM_INQ
cmsg in mptcp_recvmsg():
unsigned int inq = mptcp_inq_hint(sk);
put_cmsg(msg, SOL_TCP, TCP_CM_INQ, sizeof(inq), &inq);
Does this change move TCP_CM_INQ away from TCP after a DATA_FIN?
bytes_received never counts the DATA_FIN. mptcp_check_data_fin() only
bumps ack_seq:
WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
So with N bytes still unread after a DATA_FIN, the TCP_CM_INQ value
reported by mptcp_recvmsg() drops from N+1 to N.
Plain TCP fills TCP_CM_INQ from tcp_inq_hint() in net/ipv4/tcp.c. That
value still includes the FIN:
inq = rcv_nxt - copied_seq;
...
if (inq == 0 && sock_flag(sk, SOCK_DONE))
inq = 1;
So TCP reports N+1 here, which is also what MPTCP reported before this
patch.
The commit message calls the N+1 to N change "aligning with TCP's SIOCINQ
behavior". That holds for SIOCINQ, because tcp_inq() subtracts 1 when
SOCK_DONE is set. For TCP_CM_INQ it is a user-visible change away from
TCP, and the message does not mention it. The same code is still in place
at the end of the series.
Should the commit message cover the TCP_CM_INQ change? Or should the cmsg
path keep reporting N+1?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794215614.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH mptcp-next v24 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-net v24 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-10 23:32 ` netdev-bot+sashiko
2026-11-09 9:24 ` [PATCH mptcp-net v24 3/8] mptcp: fix fallback dummy mapping anchor Geliang Tang
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
From: Geliang Tang <tanggeliang@kylinos.cn>
Add a SIOCINQ check in mptcp_connect's server mode when the MPTFO socket
option is enabled to detect TFO SIOCINQ overflow regressions.
When a TFO connection is accepted, the test polls MPTCP_INFO until the
MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED flag is set (using poll_timeout,
default 10s), ensuring the third ACK has been processed. It then checks
if SIOCINQ returns INT_MAX, which would indicate the regression where
mptcp_inq_hint() incorrectly returns INT_MAX for passive TFO connections.
Add SOL_MPTCP fallback definition for compatibility with older libc
headers. Handle ENOPROTOOPT and EOPNOTSUPP errors from getsockopt() to
skip the wait for TCP sockets or MPTCP sockets that fell back to TCP.
Print a warning if the remote key does not arrive within the timeout, but
continue with the SIOCINQ check rather than failing the test.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../selftests/net/mptcp/mptcp_connect.c | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 178d98d91fea..da35a5ce1541 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -34,6 +34,7 @@
#include <linux/time_types.h>
#include <linux/sockios.h>
#include <linux/compiler.h>
+#include <linux/mptcp.h>
extern int optind;
@@ -43,6 +44,9 @@ extern int optind;
#ifndef TCP_ULP
#define TCP_ULP 31
#endif
+#ifndef SOL_MPTCP
+#define SOL_MPTCP 284
+#endif
static int poll_timeout = 10 * 1000;
static bool listen_mode;
@@ -1176,6 +1180,28 @@ static void maybe_close(int fd)
close(fd);
}
+static int wait_for_remote_key(int fd)
+{
+ struct mptcp_info info;
+ socklen_t olen;
+ int elapsed = 0;
+
+ while (elapsed < poll_timeout || poll_timeout < 0) {
+ olen = sizeof(info);
+ if (getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &olen) < 0) {
+ /* Not an MPTCP socket (TCP or fallback), skip wait */
+ if (errno == ENOPROTOOPT || errno == EOPNOTSUPP)
+ return 0;
+ return -1;
+ }
+ if (info.mptcpi_flags & MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED)
+ return 0;
+ usleep(10000);
+ elapsed += 10;
+ }
+ return 1;
+}
+
int main_loop_s(int listensock)
{
struct sockaddr_storage ss;
@@ -1207,6 +1233,27 @@ int main_loop_s(int listensock)
check_sockaddr(pf, &ss, salen);
check_getpeername(remotesock, &ss, salen);
+ if (cfg_sockopt_types.mptfo) {
+ int ret = wait_for_remote_key(remotesock);
+ int avail;
+
+ if (ret < 0) {
+ perror("TFO: getsockopt(MPTCP_INFO)");
+ close(remotesock);
+ return 1;
+ }
+ if (ret > 0)
+ fprintf(stderr,
+ "TFO: timeout waiting for remote key\n");
+
+ if (ioctl(remotesock, SIOCINQ, &avail) == 0 &&
+ avail == INT_MAX) {
+ fprintf(stderr, "TFO SIOCINQ overflow\n");
+ close(remotesock);
+ return 1;
+ }
+ }
+
if (cfg_input) {
fd = open(cfg_input, O_RDONLY);
if (fd < 0)
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH mptcp-next v24 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow
2026-11-09 9:24 ` [PATCH mptcp-next v24 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
@ 2026-11-10 23:32 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-11-10 23:32 UTC (permalink / raw)
To: geliang; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] wait_for_remote_key() in
tools/testing/selftests/net/mptcp/mptcp_connect.c can loop forever.
--
selftests: mptcp: connect: check TFO SIOCINQ overflow
This adds a SIOCINQ check to the mptcp_connect server mode when the
MPTFO socket option is enabled. After accept(), the server polls
MPTCP_INFO until MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED is set (bounded by
poll_timeout), then fails the test if SIOCINQ reports INT_MAX.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..da35a5ce15419 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
[ ... ]
> @@ -1176,6 +1180,28 @@ static void maybe_close(int fd)
> close(fd);
> }
>
> +static int wait_for_remote_key(int fd)
> +{
> + struct mptcp_info info;
> + socklen_t olen;
> + int elapsed = 0;
> +
> + while (elapsed < poll_timeout || poll_timeout < 0) {
> + olen = sizeof(info);
> + if (getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &olen) < 0) {
> + /* Not an MPTCP socket (TCP or fallback), skip wait */
> + if (errno == ENOPROTOOPT || errno == EOPNOTSUPP)
> + return 0;
> + return -1;
> + }
> + if (info.mptcpi_flags & MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED)
> + return 0;
> + usleep(10000);
> + elapsed += 10;
> + }
> + return 1;
> +}
[Severity: Low]
Can wait_for_remote_key() loop forever when poll_timeout is negative?
main() sets poll_timeout to -1 for any -t value <= 0:
main() {
...
case 't':
poll_timeout = atoi(optarg) * 1000;
if (poll_timeout <= 0)
poll_timeout = -1;
...
}
With poll_timeout == -1 the while condition is always true. The loop
then only exits if REMOTE_KEY_RECEIVED shows up or getsockopt() fails.
Nothing in the loop checks whether the connection is still alive (no
poll() for POLLHUP/POLLERR, no SO_ERROR, no MPTCP_INFO_FLAG_FALLBACK
check).
For a passive TFO socket, accept() returns at SYN time, before the MPC
third ACK. Suppose that subflow gets reset, or the third ACK never
arrives, for example because the client exits early and its kernel
answers the SYN-ACK with an RST. The msk is not marked as fallback in
that case, so mptcp_getsockopt() keeps reaching
mptcp_getsockopt_sol_mptcp() and MPTCP_INFO keeps succeeding.
REMOTE_KEY_RECEIVED stays clear, because msk->can_ack is only set in
subflow_set_remote_key():
WRITE_ONCE(msk->can_ack, true);
and that code needs the MPC ACK to be processed first.
Without this patch, the same dead connection would reach poll(-1) in
copyfd_io(), which wakes on HUP/ERR and returns. With this patch, the
server just keeps sleeping 10ms per iteration.
With a finite timeout (the in-tree scripts pass -t 30), the same case
stalls the server for the whole timeout before copyfd_io() sees the
error.
Could the loop also check for a dead socket, e.g. with a short poll()
for POLLHUP/POLLERR, or by stopping once the subflow is closed?
> +
> int main_loop_s(int listensock)
> {
> struct sockaddr_storage ss;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794215614.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH mptcp-net v24 3/8] mptcp: fix fallback dummy mapping anchor
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-net v24 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-next v24 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-next v24 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
From: Geliang Tang <tanggeliang@kylinos.cn>
Commit 2834f8edd74d ("mptcp: cleanup fallback dummy mapping generation")
anchored map_seq at the DSN of skb->seq but left map_subflow_seq
anchored at copied_seq. This caused mptcp_subflow_get_mapped_dsn() to
return the DSN at skb->seq instead of copied_seq.
When TCP queues a partially overlapping segment (seq < rcv_nxt), the skb
starts before copied_seq, giving offset > 0. mptcp_init_skb() then sets
map_seq to the wrong DSN, causing __mptcp_move_skb() to skip data or
drop the skb, leaving a hole in the stream.
Fix by anchoring map_subflow_seq at TCP_SKB_CB(skb)->seq, ensuring
mptcp_subflow_get_mapped_dsn() returns the correct DSN at copied_seq.
Fixes: 2834f8edd74d ("mptcp: cleanup fallback dummy mapping generation")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
net/mptcp/subflow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 00677fb3efcd..6f07544bfe40 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1496,7 +1496,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] 16+ messages in thread* [PATCH mptcp-next v24 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (2 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-net v24 3/8] mptcp: fix fallback dummy mapping anchor Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-next v24 5/8] mptcp: drop the cant_coalesce CB field Geliang Tang
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
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 fc4ffa326317..71f5b0209192 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] 16+ messages in thread* [PATCH mptcp-next v24 5/8] mptcp: drop the cant_coalesce CB field
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (3 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-next v24 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-09 9:24 ` [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field Geliang Tang
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
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 __mptcp_sync_rcv_sequence() to update the TFO skb
map_seq and end_seq using the IASN stored in msk->tfo_iasn, and call it
from __mptcp_move_skb() before any new skb lands into the msk.
When the 3rd ack arrives while the msk is owned by user space, defer the
sync via MPTCP_SYNC_SEQ in cb_flags. subflow_set_remote_key() stores
tfo_iasn and either calls __mptcp_sync_rcv_sequence() directly (when the
socket is not locked) or sets MPTCP_SYNC_SEQ for deferred processing.
mptcp_move_skbs() and mptcp_release_cb() check MPTCP_SYNC_SEQ at each
iteration before spooling the backlog, using __test_and_clear_bit() since
the cb_flags are protected by the msk data lock. A rcvd_dummy_seq flag
tracks whether the TFO skb still carries the dummy mapping and needs
synchronization.
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 | 39 ++++++++++++++++++++++++++++++++++++---
net/mptcp/protocol.h | 5 ++++-
net/mptcp/subflow.c | 8 ++++++++
4 files changed, 52 insertions(+), 6 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 71f5b0209192..ce0f9c2e6fd1 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,24 @@ 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;
+
+ 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->tfo_iasn - 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)
{
u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
@@ -446,6 +462,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:
@@ -2375,7 +2397,13 @@ static bool mptcp_move_skbs(struct sock *sk)
u32 moved = 0;
mptcp_data_lock(sk);
- while (mptcp_can_spool_backlog(sk, &skbs)) {
+ while (1) {
+ if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
+ __mptcp_sync_rcv_sequence(sk);
+
+ if (!mptcp_can_spool_backlog(sk, &skbs))
+ break;
+
mptcp_data_unlock(sk);
enqueued |= __mptcp_move_skbs(sk, &skbs, &moved);
@@ -3674,6 +3702,8 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->bytes_retrans = 0;
msk->rcvspace_init = 0;
msk->fastclosing = 0;
+ msk->tfo_iasn = 0;
+ msk->rcvd_dummy_seq = false;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
@@ -3876,6 +3906,9 @@ static void mptcp_release_cb(struct sock *sk)
struct list_head join_list, skbs;
bool spool_bl;
+ if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags))
+ __mptcp_sync_rcv_sequence(sk);
+
spool_bl = mptcp_can_spool_backlog(sk, &skbs);
if (!flags && !spool_bl)
break;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3be60da12924..b4e49d719200 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]))
@@ -325,6 +325,7 @@ struct mptcp_sock {
u32 last_ack_recv;
unsigned long timer_ival;
u32 token;
+ u64 tfo_iasn;
unsigned long flags;
unsigned long cb_flags;
bool recovery; /* closing subflow write queue reinjected */
@@ -336,6 +337,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 +1195,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 6f07544bfe40..f9dfb6662e3c 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -519,6 +519,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
*/
@@ -537,6 +539,12 @@ 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);
+ msk->tfo_iasn = 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] 16+ messages in thread* [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (4 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-next v24 5/8] mptcp: drop the cant_coalesce CB field Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-09 21:18 ` sashiko-bot
2026-11-10 23:32 ` netdev-bot+sashiko
2026-11-09 9:24 ` [PATCH mptcp-next v24 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
` (2 subsequent siblings)
8 siblings, 2 replies; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
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 introduced `copied_seq` field is always accessed under the msk
socket lock, delegating the synchronization with IASN to mptcp_move_skbs()
and 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.
In mptcp_init_skb(), map_seq is computed as mptcp_subflow_get_mapped_dsn()
minus offset. A mapping boundary check using before() for proper 32-bit
modular arithmetic detects when the skb spans multiple DSS mappings
(offset > 0 and skb starts before the current mapping). In that case,
the consumed prefix bytes belong to an old mapping and must be removed
before setting map_seq. The function now returns bool to indicate success.
When removing the prefix, we handle linear and paged data separately.
For offset within linear data where offset <= skb_headlen, use skb_pull()
which is safe and doesn't modify truesize. For offset in paged data where
offset > skb_headlen, use pskb_pull() which may expand the skb head. To
avoid truesize corruption in pskb_expand_head(), we first orphan the skb
to disconnect it from the socket, perform the pull, recompute truesize
manually, then reattach with skb_set_owner_r(). This ensures proper memory
accounting.
When mptcp_init_skb() fails, the caller increments seq to skip the
problematic skb, records the failure via MPTCP_MIB_RCVPRUNED, and jumps
to cleanup to ensure tp->copied_seq and more_data_avail are properly
updated. This maintains TCP/MPTCP state consistency, and MPTCP-level
retransmission will fill any resulting gaps.
When the skb stays within a single mapping and offset > 0, map_seq is
rewound to cover the already-consumed prefix. A new MPTCP_SKB_CB_F_OFFSET
flag marks such skbs. __mptcp_try_coalesce() refuses to coalesce when the
from skb carries this flag, preventing an unvalidated prefix from being
merged into a validated buffer. mptcp_data_queue_ofo() skips the
rb_replace_node() replacement when the new skb covers an existing OFO
entry and carries the flag, so that previously validated data is not
overwritten by an unvalidated prefix; the new skb is inserted after the
existing entry instead, and the prefix is naturally skipped when the OFO
queue is later processed.
Overall this simplifies a bit the __mptcp_recvmsg_mskq() and the
__mptcp_move_skb() code and will also make possible the next patch.
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 | 122 ++++++++++++++++++++++++++++---------------
net/mptcp/protocol.h | 5 +-
net/mptcp/subflow.c | 6 ++-
4 files changed, 94 insertions(+), 48 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index e18b31028e7e..48298bfe85a4 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -45,10 +45,12 @@ 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)->flags = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
mptcp_data_lock(sk);
@@ -56,6 +58,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;
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 ce0f9c2e6fd1..30933dd9f0b1 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,7 +168,7 @@ 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 ||
+ MPTCP_SKB_CB(from)->flags & MPTCP_SKB_CB_F_OFFSET ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
return false;
@@ -375,6 +375,11 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
* | skb1 |
* continue traversing
*/
+ } else if (MPTCP_SKB_CB(skb)->flags &
+ MPTCP_SKB_CB_F_OFFSET) {
+ /* skb covers skb1 but has an unvalidated
+ * prefix; keep skb1, continue traversing.
+ */
} else {
/* skb's seq == skb1's seq and skb covers skb1.
* Replace skb1 with skb.
@@ -415,25 +420,54 @@ 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 bool 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;
+ /* Detect mapping boundary: skb spans multiple mappings */
+ if (unlikely(offset > 0 &&
+ before(TCP_SKB_CB(skb)->seq - subflow->ssn_offset,
+ subflow->map_subflow_seq))) {
+ if (offset >= skb->len) {
+ sk_eat_skb(ssk, skb);
+ return false;
+ }
+
+ if (offset > skb_headlen(skb)) {
+ skb_orphan(skb);
+ if (!pskb_pull(skb, offset)) {
+ sk_eat_skb(ssk, skb);
+ return false;
+ }
+ skb->truesize = SKB_TRUESIZE(skb_end_offset(skb)) +
+ skb->data_len;
+ skb_set_owner_r(skb, ssk);
+ } else {
+ if (!skb_pull(skb, offset)) {
+ sk_eat_skb(ssk, skb);
+ return false;
+ }
+ }
+ offset = 0;
+ }
+
/* the skb map_seq accounts for the skb 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)->flags = offset ? MPTCP_SKB_CB_F_OFFSET : 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
__skb_unlink(skb, &ssk->sk_receive_queue);
skb_ext_reset(skb);
skb_dst_drop(skb);
+
+ return true;
}
void __mptcp_sync_rcv_sequence(struct sock *sk)
@@ -494,10 +528,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;
}
@@ -855,7 +885,12 @@ 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);
+ if (!mptcp_init_skb(ssk, skb, offset)) {
+ MPTCP_INC_STATS(sock_net(sk),
+ MPTCP_MIB_RCVPRUNED);
+ seq += len;
+ goto out;
+ }
if (own_msk) {
mptcp_subflow_lend_fwdmem(subflow, skb);
@@ -878,6 +913,7 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
}
}
+out:
WRITE_ONCE(tp->copied_seq, seq);
more_data_avail = mptcp_subflow_data_available(ssk);
@@ -918,12 +954,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;
@@ -2207,33 +2240,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);
@@ -2252,14 +2276,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 {
@@ -2398,8 +2420,10 @@ static bool mptcp_move_skbs(struct sock *sk)
mptcp_data_lock(sk);
while (1) {
- 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 += msk->tfo_iasn;
__mptcp_sync_rcv_sequence(sk);
+ }
if (!mptcp_can_spool_backlog(sk, &skbs))
break;
@@ -2439,6 +2463,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;
@@ -2457,6 +2482,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;
@@ -2466,7 +2496,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)
@@ -2476,8 +2506,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()
@@ -2520,6 +2553,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);
@@ -3696,6 +3733,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;
@@ -3906,8 +3944,10 @@ static void mptcp_release_cb(struct sock *sk)
struct list_head join_list, skbs;
bool spool_bl;
- 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 += msk->tfo_iasn;
__mptcp_sync_rcv_sequence(sk);
+ }
spool_bl = mptcp_can_spool_backlog(sk, &skbs);
if (!flags && !spool_bl)
@@ -4628,7 +4668,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 +4710,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 b4e49d719200..fd3326518ee5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -128,10 +128,12 @@
#define MPTCP_SYNC_SNDBUF 7
#define MPTCP_SYNC_SEQ 8
+#define MPTCP_SKB_CB_F_OFFSET 0x01
+
struct mptcp_skb_cb {
u64 map_seq;
u64 end_seq;
- u32 offset;
+ u16 flags;
u8 has_rxtstamp;
};
@@ -311,6 +313,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;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index f9dfb6662e3c..83beaa54b634 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -541,10 +541,12 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
msk->tfo_iasn = 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,
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field
2026-11-09 9:24 ` [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field Geliang Tang
@ 2026-11-09 21:18 ` sashiko-bot
2026-11-10 23:32 ` netdev-bot+sashiko
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-09 21:18 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] mptcp: sk_forward_alloc corruption in mptcp_init_skb() on paged
pull
- [Medium] mptcp: rewound map_seq in mptcp_init_skb() exposes
unvalidated prefix
--
commit 53ea4c3d9a0507effa8276bb08b2e103414e8c2d
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 and handles already-consumed skb prefixes in
mptcp_init_skb().
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index ce0f9c2e6fd1a..30933dd9f0b1d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -375,6 +375,11 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
> * | skb1 |
> * continue traversing
> */
> + } else if (MPTCP_SKB_CB(skb)->flags &
> + MPTCP_SKB_CB_F_OFFSET) {
> + /* skb covers skb1 but has an unvalidated
> + * prefix; keep skb1, continue traversing.
> + */
> } else {
> /* skb's seq == skb1's seq and skb covers skb1.
> * Replace skb1 with skb.
> @@ -415,25 +420,54 @@ 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 bool 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;
>
> + /* Detect mapping boundary: skb spans multiple mappings */
> + if (unlikely(offset > 0 &&
> + before(TCP_SKB_CB(skb)->seq - subflow->ssn_offset,
> + subflow->map_subflow_seq))) {
> + if (offset >= skb->len) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> +
> + if (offset > skb_headlen(skb)) {
> + skb_orphan(skb);
> + if (!pskb_pull(skb, offset)) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> + skb->truesize = SKB_TRUESIZE(skb_end_offset(skb)) +
> + skb->data_len;
> + skb_set_owner_r(skb, ssk);
> + } else {
> + if (!skb_pull(skb, offset)) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> + }
> + offset = 0;
> + }
[Severity: High]
Does orphaning and re-charging skb here corrupt ssk->sk_forward_alloc and
subflow->lent_mem_frag?
skb_orphan(skb) invokes sock_rfree() -> sk_mem_uncharge(ssk, orig_truesize),
which calls sk_mem_reclaim() and returns whole pages from
ssk->sk_forward_alloc to tcp_memory_allocated. skb_set_owner_r(skb, ssk)
then calls sk_mem_charge(ssk, skb->truesize) without sk_rmem_schedule(),
subtracting the new truesize from ssk->sk_forward_alloc without allocating
backing pages, and altering skb->truesize breaks the PAGE_SIZE alignment
that mptcp_subflow_lend_fwdmem() and __mptcp_close_ssk() rely on. When the
subflow closes, ssk->sk_forward_alloc is left negative (triggering
WARN_ON_ONCE(sk->sk_forward_alloc) in inet_sock_destruct()) while msk
reclaims the transferred pages a second time.
Note that pskb_expand_head() only adjusts skb->truesize when !skb->sk or
skb->destructor == sock_edemux, so calling pskb_pull() directly while skb is
still owned by ssk avoids changing skb->truesize or touching
ssk->sk_forward_alloc.
> +
> /* the skb map_seq accounts for the skb 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)->flags = offset ? MPTCP_SKB_CB_F_OFFSET : 0;
[Severity: Medium]
Can rewinding MPTCP_SKB_CB(skb)->map_seq by offset expose the unvalidated
prefix to the receive and out-of-order queues?
When an incoming subflow segment overlaps tp->copied_seq within the current
DSS mapping, validate_data_csum() starts at tp->copied_seq +
subflow->map_csum_len and does not validate the first offset bytes of skb.
Because mptcp_init_skb() rewinds map_seq to include that prefix and only
records a single bit in MPTCP_SKB_CB_F_OFFSET:
In mptcp_data_queue_ofo(), if the already-validated range in
msk->out_of_order_queue spans more than one skb (for example skb1a covering
[1000, 1050) and skb1b covering [1050, 1100)) and a segment arrives with
mapped_dsn 1100 and offset 100, traversal skips replacing skb1a via the
MPTCP_SKB_CB_F_OFFSET check, moves right to skb1b, sees seq (1000) <
skb1b->map_seq (1050), inserts skb to the left of skb1b, and then drops the
checksum-validated skb1b in merge_right, replacing [1050, 1100) with skb's
unvalidated prefix. Similarly, mptcp_prune_ofo_queue() compares the rewound
in_skb->map_seq against the validated ooo_last_skb and can prune it, and in
__mptcp_move_skb(), if the earlier segment that advanced tp->copied_seq was
dropped at the msk level due to backlog or rmem limits before advancing
msk->ack_seq, the unvalidated prefix is accepted as in-sequence data.
Pulling offset bytes in mptcp_init_skb() whenever offset > 0 (not only
across mapping boundaries) would strip the unvalidated prefix before map_seq
is set.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794215614.git.tanggeliang@kylinos.cn?part=6
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field
2026-11-09 9:24 ` [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field Geliang Tang
2026-11-09 21:18 ` sashiko-bot
@ 2026-11-10 23:32 ` netdev-bot+sashiko
1 sibling, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-11-10 23:32 UTC (permalink / raw)
To: geliang; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.
Critical: 0 · High: 2 · Medium: 2 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The F_OFFSET guard does not fully protect validated data from an
unvalidated prefix.
- [High] The orphan/re-own sequence in mptcp_init_skb() unbalances
subflow forward-alloc accounting.
- [Medium] When offset > skb_headlen(skb), mptcp_init_skb() orphans the
skb, calls pskb_pull(skb, offset), and then sets skb->truesize =…
- [Medium] msk->copied_seq counts only bytes copied to user space and
never takes the DATA_FIN sequence slot.
- [Low] To drop a prefix that ends in the paged area, mptcp_init_skb()
calls pskb_pull(skb, offset).
--
mptcp: remove CB offset field
This patch replaces the per-skb CB offset field with a new msk-level
copied_seq, which tracks bytes copied to user space the way TCP does.
mptcp_init_skb() now rewinds map_seq by the already consumed offset and
marks such skbs with MPTCP_SKB_CB_F_OFFSET. When the skb spans a DSS
mapping boundary, it trims the prefix instead.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index ce0f9c2e6fd1a..30933dd9f0b1d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -375,6 +375,11 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
> * | skb1 |
> * continue traversing
> */
> + } else if (MPTCP_SKB_CB(skb)->flags &
> + MPTCP_SKB_CB_F_OFFSET) {
> + /* skb covers skb1 but has an unvalidated
> + * prefix; keep skb1, continue traversing.
> + */
[Severity: High]
Is validated data still protected from the unvalidated prefix on the
other paths through mptcp_data_queue_ofo()?
mptcp_init_skb() now rewinds map_seq by offset. A TCP partial-overlap
retransmission S therefore also claims the prefix
[TCP_SKB_CB(skb)->seq, tp->copied_seq) in DSN space.
validate_data_csum() never checksums those bytes, because it starts at:
seq = tcp_sk(ssk)->copied_seq + subflow->map_csum_len;
This branch only skips the equal-seq rb_replace_node(). After it, the
traversal goes right and S is inserted. The merge_right loop then drops
any later entry covered by S, and it does not check the flag:
merge_right:
while ((skb1 = skb_rb_next(skb)) != NULL) {
if (before64(end_seq, MPTCP_SKB_CB(skb1)->end_seq))
break;
rb_erase(&skb1->rbnode, &msk->out_of_order_queue);
mptcp_drop(sk, skb1);
Here is an example. A1 [d, d+m) and A2 [d+m, d+a) come from the same
mapping. The (limit >> 3) check leaves them uncoalesced, which is easy
to hit with 64KB GRO skbs and the default rcvbuf. S covers [d, d+a+n)
and has F_OFFSET set:
mptcp_data_queue_ofo(S)
takes the F_OFFSET branch at A1, goes right
goes left of A2, inserts S
merge_right: skb_rb_next(S) == A2, drops the validated A2
__mptcp_ofo_queue() then queues A1 and S without coalescing. recvmsg
reads DSN [d+m, d+a) from S's unvalidated prefix. The same thing
happens when S is inserted to the left of skb1 (seq before skb1's
map_seq).
The in-sequence path in __mptcp_move_skb() also ignores the flag:
if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
__mptcp_move_skbs() runs __mptcp_move_skb() on every backlog skb before
it calls __mptcp_ofo_queue(). So ack_seq can reach S's rewound map_seq
while an earlier copy is still in the OFO queue. A2 being dropped under
rmem pressure, followed by S taking the partial path, has the same
effect.
Before this patch, S's map_seq started at the first new byte, so the
prefix was never delivered. The commit message says previously
validated data "is not overwritten by an unvalidated prefix".
With DSS checksums enabled, could a middlebox that rewrites only the
overlapping part of a retransmission go undetected here? Would it work
better to trim the prefix, or to stop F_OFFSET skbs from covering
existing data on every path?
The later commit "mptcp: sync mptcp skb cb layout with tcp one" keeps
the same logic.
> } else {
> /* skb's seq == skb1's seq and skb covers skb1.
> * Replace skb1 with skb.
[ ... ]
> @@ -415,25 +420,54 @@ 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 bool 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;
>
> + /* Detect mapping boundary: skb spans multiple mappings */
> + if (unlikely(offset > 0 &&
> + before(TCP_SKB_CB(skb)->seq - subflow->ssn_offset,
> + subflow->map_subflow_seq))) {
> + if (offset >= skb->len) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> +
> + if (offset > skb_headlen(skb)) {
> + skb_orphan(skb);
> + if (!pskb_pull(skb, offset)) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
[Severity: Low]
Is pskb_pull() the right tool for discarding a prefix that ends in the
paged area?
pskb_pull() goes through __pskb_pull_tail(skb, offset - headlen). That
can call pskb_expand_head(skb, 0, eat + 128, GFP_ATOMIC) and then copy
every prefix byte into the new head with skb_copy_bits(). __skb_pull()
then throws those bytes away.
For a large GRO skb, this is a high-order GFP_ATOMIC allocation in
softirq context, made only to hold data that is never read. The dead
bytes also stay in the enlarged head until the skb is freed.
__pskb_pull_tail() also fails for unreadable (devmem) frags. On any
failure, sk_eat_skb() drops the whole skb, including valid bytes after
the prefix that TCP has already acked. Recovery then depends on
MPTCP-level retransmission.
Would a frag-trimming head trim like __pskb_trim_head(), which needs
no allocation and copies nothing, fit better here?
> + skb->truesize = SKB_TRUESIZE(skb_end_offset(skb)) +
> + skb->data_len;
[Severity: Medium]
Does this truesize computation under-account the remaining frags?
This branch only runs when the prefix reaches into the paged area, so
page frags or frag_list members usually remain after the pull.
skb->data_len counts only their payload, not the memory they pin. Some
examples: 2-4KB buffers holding about 1.5KB, high-order GRO pages, and
the sk_buff and head overhead of each frag_list member.
skb_condense() rebuilds truesize from the head size only after all
frags are gone, because fragments do not record their own truesize:
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
The commit message says:
To avoid truesize corruption in pskb_expand_head(), we first orphan
the skb to disconnect it from the socket, perform the pull,
recompute truesize manually, then reattach with skb_set_owner_r().
This ensures proper memory accounting.
Is that accurate? pskb_expand_head() only changes truesize for
orphaned skbs:
if (!skb->sk || skb->destructor == sock_edemux)
skb->truesize += size - osize;
An owned skb would keep its truesize, and the charge/uncharge pair
would stay balanced. The orphan seems to be what makes truesize change
in the first place.
The understated truesize then feeds skb_set_owner_r(ssk), lend/borrow,
backlog_len, the msk rmem/rcvbuf checks, the protocol memory_allocated
counter and memcg. A remote peer can reach this path: it sends a
segment, then an overlapping retransmission whose new DSS mapping
starts at the old end.
> + skb_set_owner_r(skb, ssk);
[Severity: High]
Can this orphan and re-own sequence drive ssk->sk_forward_alloc
negative?
The skb is in the TCP receive queue with sock_rfree as its destructor.
TCP already scheduled its truesize T_old:
mptcp_init_skb()
skb_orphan(skb)
sock_rfree()
sk_mem_uncharge(ssk, T_old)
sk_mem_reclaim() <- returns whole pages of ssk forward alloc
pskb_pull()
skb_set_owner_r(skb, ssk)
sk_mem_charge(ssk, T_new) <- only sk_forward_alloc_add(sk, -size)
Nothing calls sk_rmem_schedule() for T_new, so ssk->sk_forward_alloc
ends up around -T_new. mptcp_subflow_lend_fwdmem() and
mptcp_borrow_fwdmem() then credit T_new to the msk forward alloc. The
protocol counter no longer covers that memory, and the msk later
reclaims and uncharges those pages a second time.
The peer can close the subflow right away, with a FIN or RST after the
crafted segment. The ssk is then destroyed with a negative forward
alloc, which trips this check in inet_sock_destruct():
WARN_ON_ONCE(sk->sk_forward_alloc);
It also leaves tcp_memory_allocated and the memcg socket counters
permanently too low. The peer can repeat this across connections.
Since pskb_expand_head() leaves truesize alone for owned skbs, is the
orphan needed at all? The later commits in the series keep this code
unchanged.
> + } else {
> + if (!skb_pull(skb, offset)) {
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> + }
> + offset = 0;
> + }
[ ... ]
> @@ -2207,33 +2240,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;
[Severity: Medium]
What keeps msk->copied_seq in the same DSN space as map_seq once a
DATA_FIN has been processed?
copied_seq only advances by the bytes copied to user space, but
mptcp_check_data_fin() also consumes a sequence slot:
WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
The old code anchored the per-skb offset and map_seq to ack_seq in the
partial-packet and OFO paths, and this patch removes that. Data at or
past rcv_data_fin_seq is not rejected by __mptcp_move_skb(), by
__mptcp_add_backlog(), or on the subflow path.
Suppose a peer sends a mapping that covers the DATA_FIN slot.
subflow_check_data_avail() discards the bytes below ack_seq through
mptcp_subflow_discard_data(). mptcp_init_skb() then rewinds map_seq to
fin_seq - k and sets F_OFFSET. After that:
__mptcp_move_skb()
partial path: copy_len = end_seq - ack_seq (bytes_received += len - k - 1)
__mptcp_recvmsg_mskq()
offset = copied_seq - map_seq = k (bytes_consumed += len - k)
bytes_consumed now exceeds bytes_received by 1, so the u64
mptcp_data_avail() underflows. The effects:
- mptcp_inq_hint() reports INT_MAX.
- mptcp_epollin_ready() is always true.
- mptcp_close() sees unread data and does a fastclose.
- The byte at the FIN slot reaches the application.
Now suppose the data instead has map_seq == fin_seq + 1 == ack_seq. It
is queued in sequence, and offset here becomes (u64)-1. The skb is
skipped as already peeked and never freed, so it stays charged in
sk_receive_queue until close. On the read_sock/splice path,
mptcp_recv_skb() silently eats it instead.
Only a misbehaving peer that sends data past its own DATA_FIN can
trigger this. Should copied_seq account for the DATA_FIN slot? The
later commits in the series still don't adjust it.
> u32 data_len = skb->len - offset;
> u32 count;
> int err;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794215614.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH mptcp-next v24 7/8] mptcp: sync mptcp skb cb layout with tcp one
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (5 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-next v24 6/8] mptcp: remove CB offset field Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-10 23:32 ` netdev-bot+sashiko
2026-11-09 9:24 ` [PATCH mptcp-next v24 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
2026-11-09 11:21 ` [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
8 siblings, 1 reply; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
From: Paolo Abeni <pabeni@redhat.com>
The MPTCP protocol uses a significantly different CB layout WRT TCP, as it
includes different information and uses 64 bits for the sequence numbers.
As the msk-level rcvbuf buffer size is limited by the core socket code to
INT_MAX, and after validating the incoming skb vs the current receive
window, we can safely use 32 bits for MPTCP-level sequence numbers. This
allows updating the MPTCP CB layout so that key fields with corresponding
TCP-level data (map_seq/seq, end_seq/end_seq, flags/tcp_flags) use the
same offsets inside the CB itself.
The CB layout changes map_seq and end_seq from u64 to u32, adds a new
map_seq64 field to track the full 64-bit DSN, and aligns the existing flags
field with TCP's tcp_flags offset. 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 a build time check to ensure the CB layout matches TCP's layout for the
three shared fields: map_seq, end_seq, and flags.
map_seq64 is computed as mptcp_subflow_get_mapped_dsn() in mptcp_init_skb()
minus offset, and map_seq is the lower 32 bits.
The receive path now uses 32-bit before()/after() helpers for most sequence
comparisons, while map_seq64 is used for window validation and coalescing
checks. __mptcp_try_coalesce() gains a map_seq64 contiguity check to ensure
skbs are truly adjacent in the 64-bit DSN space.
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 | 5 ++-
net/mptcp/protocol.c | 93 +++++++++++++++++++++++++++-----------------
net/mptcp/protocol.h | 6 ++-
3 files changed, 64 insertions(+), 40 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 48298bfe85a4..bd52e9528b29 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -48,7 +48,8 @@ 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;
@@ -58,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;
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 30933dd9f0b1..683ffa8f6c7a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,12 +168,14 @@ 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 ||
MPTCP_SKB_CB(from)->flags & MPTCP_SKB_CB_F_OFFSET ||
((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;
@@ -255,8 +257,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;
@@ -302,16 +304,23 @@ 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);
+ if (after64(READ_ONCE(msk->ack_seq), max_seq))
+ max_seq = READ_ONCE(msk->ack_seq);
- 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",
@@ -346,7 +355,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;
@@ -358,18 +367,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 |
@@ -405,7 +414,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);
@@ -454,10 +463,11 @@ static bool 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 = offset ? MPTCP_SKB_CB_F_OFFSET : 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
@@ -484,13 +494,14 @@ void __mptcp_sync_rcv_sequence(struct sock *sk)
if (!skb)
return;
- MPTCP_SKB_CB(skb)->map_seq = msk->tfo_iasn - skb->len;
+ MPTCP_SKB_CB(skb)->map_seq64 = msk->tfo_iasn - 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;
@@ -502,7 +513,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)) {
@@ -520,14 +531,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;
}
@@ -928,39 +939,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;
@@ -2249,7 +2258,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;
@@ -4668,7 +4677,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;
@@ -4918,11 +4927,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 fd3326518ee5..8c868bcbafb8 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -131,10 +131,12 @@
#define MPTCP_SKB_CB_F_OFFSET 0x01
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] 16+ messages in thread* Re: [PATCH mptcp-next v24 7/8] mptcp: sync mptcp skb cb layout with tcp one
2026-11-09 9:24 ` [PATCH mptcp-next v24 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-11-10 23:32 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-11-10 23:32 UTC (permalink / raw)
To: geliang; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message never says why the MPTCP CB layout must match
TCP's.
- [Low] The patch puts mptcp_skb_cb.flags at the same offset and width as
tcp_skb_cb.tcp_flags (u16 at offset 12) and adds a BUILD_BUG_ON for it.
--
mptcp: sync mptcp skb cb layout with tcp one
This commit shrinks the MPTCP skb cb map_seq and end_seq fields to 32
bits so that map_seq, end_seq and flags share offsets with tcp_skb_cb's
seq, end_seq and tcp_flags. It adds map_seq64 to hold the full 64-bit
DSN, and the receive path now uses 32-bit before()/after() comparisons,
with map_seq64 used for window admission and coalescing checks. A
build-time check in mptcp_proto_init() enforces the shared offsets.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 30933dd9f0b1d..683ffa8f6c7a2 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -4918,11 +4927,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);
> +
[Severity: Low]
The commit message says:
This allows updating the MPTCP CB layout so that key fields with
corresponding TCP-level data (map_seq/seq, end_seq/end_seq,
flags/tcp_flags) use the same offsets inside the CB itself.
It doesn't say why these fields need to share offsets. Could the
changelog say which code is expected to read msk-level skbs through
TCP_SKB_CB()?
Nothing in this patch does that. Nothing up to the end of the series
("mptcp: defer read_sock cleanup to mptcp_worker") does either. Every
TCP_SKB_CB() user in net/mptcp/ works on subflow skbs before
mptcp_init_skb() rewrites the cb.
The only hint is the "upcoming TLS-over-MPTCP" mention in the last patch
of the series. net/tls/ only reads TCP_SKB_CB(skb)->seq, so why do flags
and tcp_flags also need to alias?
The subject says "sync mptcp skb cb layout with tcp one", but only three
fields line up. has_rxtstamp at offset 14 sits on tcp_skb_cb's sacked.
map_seq64 at offset 16 covers the txstamp_ack/eor/has_rxtstamp bitfield
byte and ack_seq. Would a narrower subject be more accurate?
[ ... ]
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index fd3326518ee5c..8c868bcbafb8c 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -131,10 +131,12 @@
> #define MPTCP_SKB_CB_F_OFFSET 0x01
>
> 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;
> };
[Severity: Low]
MPTCP_SKB_CB_F_OFFSET is 0x01, which is the same bit as TCPHDR_FIN:
include/net/tcp.h:
#define TCPHDR_FIN BIT(0)
Now that flags sits on top of tcp_flags, mptcp_init_skb() writes a value
that TCP would read as FIN:
net/mptcp/protocol.c:mptcp_init_skb() {
...
MPTCP_SKB_CB(skb)->flags = offset ? MPTCP_SKB_CB_F_OFFSET : 0;
...
}
These skbs keep the flag when they land in the msk OFO queue or in
sk_receive_queue. Suppose a TCP-style reader later handles msk-level
skbs with a check like the one in tcp_recv_skb()/tcp_read_sock():
if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
Would it treat these skbs as FIN, then account seq wrongly or stop
reading early?
CHK_CB_FIELD(flags, tcp_flags) in mptcp_proto_init() checks only offsets
and sizes, not whether the bits mean the same thing.
Could F_OFFSET use a value that doesn't collide with the TCPHDR_* bits?
Alternatively, could the code document that tcp_flags must not be read
on msk skbs?
There is a similar overlap with map_seq64. At offset 16 it covers the
txstamp_ack/eor/has_rxtstamp bitfield byte of tcp_skb_cb. On
little-endian systems those bits would come from the low byte of the
peer-chosen DSN.
No in-tree code reads TCP_SKB_CB() on msk-level skbs today (net/tls only
reads ->seq), so this only matters for future users of the shared
layout.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794215614.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH mptcp-next v24 8/8] mptcp: defer read_sock cleanup to mptcp_worker
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (6 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-next v24 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-11-09 9:24 ` Geliang Tang
2026-11-10 23:32 ` netdev-bot+sashiko
2026-11-09 11:21 ` [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
8 siblings, 1 reply; 16+ messages in thread
From: Geliang Tang @ 2026-11-09 9:24 UTC (permalink / raw)
To: mptcp
From: Geliang Tang <tanggeliang@kylinos.cn>
This patch prepares __mptcp_read_sock() for future MPTCP TLS support,
where it may be reached from BH context. In that case, calling
mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() is not safe as both
expect process context.
Note: The BH call path does not exist in the current tree. This is
groundwork for the upcoming TLS-over-MPTCP series. Currently, the only
callers are mptcp_splice_read() and svc_tcp_recvfrom(), both of which
hold lock_sock() and run in process context.
Conditionally defer cleanup to mptcp_worker(): when the caller doesn't
own the socket lock (BH context), accumulate consumed bytes in
msk->read_copied and signal the worker through MPTCP_WORK_READ_COMPLETE.
When the caller owns the lock (process context), perform inline cleanup
to avoid delaying window updates and autotuning.
The existing backlog flush in mptcp_recv_skb() is now gated on
sock_owned_by_user(). This ensures process context callers flush the
backlog before processing skbs, while future BH context callers skip it.
The reason is that BH callers would already hold the msk spinlock, so
mptcp_data_lock() inside mptcp_move_skbs() would self-deadlock.
Initialize msk->read_copied in __mptcp_init_sock() and reset it in
mptcp_disconnect().
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
net/mptcp/protocol.c | 38 +++++++++++++++++++++++++++++++++-----
net/mptcp/protocol.h | 2 ++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 683ffa8f6c7a..3f81a55e6e00 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3186,6 +3186,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;
@@ -3261,6 +3276,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);
@@ -3304,6 +3322,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);
@@ -3751,6 +3770,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
msk->fastclosing = 0;
msk->tfo_iasn = 0;
msk->rcvd_dummy_seq = false;
+ msk->read_copied = 0;
mptcp_init_rtt_est(msk);
/* for fallback's sake */
@@ -4673,7 +4693,7 @@ static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off)
struct sk_buff *skb;
u32 offset;
- if (!list_empty(&msk->backlog_list))
+ if (sock_owned_by_user(sk) && !list_empty(&msk->backlog_list))
mptcp_move_skbs(sk);
while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
@@ -4690,6 +4710,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)
@@ -4731,11 +4752,18 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
if (noack)
goto out;
- mptcp_rcv_space_adjust(msk, copied);
-
if (copied > 0) {
- mptcp_recv_skb(sk, &offset);
- mptcp_cleanup_rbuf(msk, copied);
+ if (sock_owned_by_user(sk)) {
+ /* Process context: inline cleanup */
+ mptcp_rcv_space_adjust(msk, copied);
+ mptcp_recv_skb(sk, &offset);
+ mptcp_cleanup_rbuf(msk, copied);
+ } else {
+ /* BH context: defer to worker */
+ 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 8c868bcbafb8..fb927bc3bcdc 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
@@ -328,6 +329,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;
u64 tfo_iasn;
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH mptcp-next v24 8/8] mptcp: defer read_sock cleanup to mptcp_worker
2026-11-09 9:24 ` [PATCH mptcp-next v24 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-11-10 23:32 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-11-10 23:32 UTC (permalink / raw)
To: geliang; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The patch adds a deferred-cleanup path that nothing in the
current tree can reach.
- [Low] The Signed-off-by address does not match the author.
--
mptcp: defer read_sock cleanup to mptcp_worker
This patch prepares __mptcp_read_sock() for a future BH context caller
(TLS over MPTCP). When the socket lock isn't owned, it defers
mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() to mptcp_worker() using
a new msk->read_copied counter and the MPTCP_WORK_READ_COMPLETE flag. It
also gates the backlog flush in mptcp_recv_skb() on sock_owned_by_user().
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
[Severity: Low]
This isn't a bug, but the Signed-off-by address doesn't match the commit
author:
Author: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
checkpatch.pl reports this as FROM_SIGN_OFF_MISMATCH. Could the From:
address be changed, or a Signed-off-by with the matching address be added?
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 683ffa8f6c7a2..3f81a55e6e00a 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -4731,11 +4752,18 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
> if (noack)
> goto out;
>
> - mptcp_rcv_space_adjust(msk, copied);
> -
> if (copied > 0) {
> - mptcp_recv_skb(sk, &offset);
> - mptcp_cleanup_rbuf(msk, copied);
> + if (sock_owned_by_user(sk)) {
> + /* Process context: inline cleanup */
> + mptcp_rcv_space_adjust(msk, copied);
> + mptcp_recv_skb(sk, &offset);
> + mptcp_cleanup_rbuf(msk, copied);
> + } else {
> + /* BH context: defer to worker */
> + msk->read_copied += copied;
> + set_bit(MPTCP_WORK_READ_COMPLETE, &msk->flags);
> + mptcp_schedule_work(sk);
> + }
[Severity: Low]
This isn't a bug, but can anything in the current tree reach this else
branch? It only runs when sock_owned_by_user(sk) is false.
Every ->read_sock caller on an MPTCP socket takes the socket lock first:
mptcp_splice_read()
lock_sock(sk);
__mptcp_splice_read()->mptcp_read_sock()->__mptcp_read_sock()
net/sunrpc/svcsock.c:svc_tcp_recvfrom() {
...
lock_sock(sock->sk);
len = sock->ops->read_sock(sock->sk, &desc, svc_tcp_recv_actor);
...
}
strparser can't be attached to an MPTCP socket either. KCM requires
csk->sk_protocol == IPPROTO_TCP, and sockmap requires sk_is_tcp().
The commit message says the same thing:
"The BH call path does not exist in the current tree. This is
groundwork for the upcoming TLS-over-MPTCP series."
That means MPTCP_WORK_READ_COMPLETE is never set and msk->read_copied stays
zero. As a result, mptcp_read_complete() is never called from
mptcp_worker().
Without the TLS caller, it is hard to judge whether the deferred path is
complete. For example, it skips the trailing mptcp_recv_skb() call that
eats fully consumed skbs. It also delays window updates and autotuning
until the worker runs. No test covers this path either.
Would it be better to post this patch in the same series as the
TLS-over-MPTCP changes that first use it?
> }
> out:
> return copied;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794215614.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage
2026-11-09 9:24 [PATCH mptcp-next v24 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (7 preceding siblings ...)
2026-11-09 9:24 ` [PATCH mptcp-next v24 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-11-09 11:21 ` MPTCP CI
8 siblings, 0 replies; 16+ messages in thread
From: MPTCP CI @ 2026-11-09 11:21 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! ✅
- Perf: Unstable: 3 failed test(s): error:sched-mem-pressure(2) validation:sched-baseline validation:sched-weak-secondary ⚠️
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/42596050890
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/5138147a8616
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1205871
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] 16+ messages in thread