* [PATCH mptcp-net v23 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 12:57 ` [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
` (7 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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. When
RCV_SHUTDOWN is set (DATA_FIN received), add 1 to include the FIN byte,
matching the previous ack_seq - map_seq behavior.
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, 7 insertions(+), 11 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 02a4f479d9d8..421df3af1ef9 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2402,22 +2402,18 @@ 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;
+ if (sk->sk_shutdown & RCV_SHUTDOWN)
+ hint_val++;
- return (unsigned int)hint_val;
- }
-
- if (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN))
+ if (!hint_val && sk->sk_state == TCP_CLOSE)
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] 18+ messages in thread* Re: [PATCH mptcp-net v23 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
2026-11-06 12:57 ` [PATCH mptcp-net v23 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
@ 2026-11-08 7:37 ` netdev-bot+sashiko
0 siblings, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, kuba
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
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).
- [Medium] In net/mptcp/protocol.c, mptcp_inq_hint() now adds 1 whenever
sk->sk_shutdown has RCV_SHUTDOWN set.
--
mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint
This patch changes mptcp_inq_hint() so it computes the available bytes
with mptcp_data_avail() rather than ack_seq minus the head skb's map_seq.
That avoids an INT_MAX result on passive TFO connections. When
RCV_SHUTDOWN is set, it adds 1 to count the FIN byte.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..421df3af1ef93 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2402,22 +2402,18 @@ 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;
> + if (sk->sk_shutdown & RCV_SHUTDOWN)
> + hint_val++;
[Severity: Medium]
Can this report one byte too many when RCV_SHUTDOWN is set but no
DATA_FIN was received?
The commit message says:
"When RCV_SHUTDOWN is set (DATA_FIN received), add 1 to include the FIN
byte, matching the previous ack_seq - map_seq behavior."
As far as I can tell, only mptcp_check_data_fin() sets RCV_SHUTDOWN and
also adds the FIN byte to ack_seq:
net/mptcp/protocol.c:mptcp_check_data_fin() {
...
WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
WRITE_ONCE(msk->rcv_data_fin, 0);
WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | RCV_SHUTDOWN);
...
}
A local shutdown(fd, SHUT_RD) goes through the .shutdown op in
mptcp_stream_ops, which is inet_shutdown():
net/ipv4/af_inet.c:inet_shutdown() {
...
WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | how);
if (sk->sk_prot->shutdown)
sk->sk_prot->shutdown(sk, how);
...
}
mptcp_shutdown() only handles SEND_SHUTDOWN. It does not purge
sk_receive_queue, and ack_seq stays the same.
A peer MP_FASTCLOSE has the same effect:
net/mptcp/protocol.c:mptcp_check_fastclose() {
...
mptcp_set_state(sk, TCP_CLOSE);
WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK);
...
}
Here too there is no DATA_FIN byte. The msk receive queue is only purged
later, in mptcp_destroy_common().
In both cases, with N unread bytes queued, wouldn't
mptcp_ioctl()->mptcp_inq_hint() for SIOCINQ/FIONREAD, and the TCP_CM_INQ
cmsg from mptcp_recvmsg(), now return N+1?
The old code returned exactly N here. __mptcp_recvmsg_mskq() advances
the head skb's map_seq on partial reads, so ack_seq minus the head
map_seq equals the unread byte count.
For comparison, tcp_inq_hint() counts the FIN only through rcv_nxt. It
does not add a byte after a local SHUT_RD.
Would it be better to add the extra byte only when a DATA_FIN was
actually counted in ack_seq, rather than whenever RCV_SHUTDOWN is set?
>
> - return (unsigned int)hint_val;
> - }
> -
> - if (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN))
> + if (!hint_val && sk->sk_state == TCP_CLOSE)
> return 1;
>
> - return 0;
> + return (unsigned int)hint_val;
> }
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-11-06 12:57 ` [PATCH mptcp-net v23 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-06 13:15 ` sashiko-bot
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 12:57 ` [PATCH mptcp-net v23 3/8] mptcp: fix fallback dummy mapping anchor Geliang Tang
` (6 subsequent siblings)
8 siblings, 2 replies; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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 (with 100ms timeout),
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 error from getsockopt() to skip the wait
for TCP sockets or MPTCP sockets that fell back to TCP.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../selftests/net/mptcp/mptcp_connect.c | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 178d98d91fea..3a72bae43b90 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 bool wait_for_remote_key(int fd)
+{
+ struct mptcp_info info;
+ int retries = 10;
+ socklen_t olen;
+
+ while (retries > 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)
+ return true;
+ return false;
+ }
+ if (info.mptcpi_flags & MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED)
+ return true;
+ usleep(10000);
+ retries--;
+ }
+ return false;
+}
+
int main_loop_s(int listensock)
{
struct sockaddr_storage ss;
@@ -1207,6 +1233,24 @@ int main_loop_s(int listensock)
check_sockaddr(pf, &ss, salen);
check_getpeername(remotesock, &ss, salen);
+ if (cfg_sockopt_types.mptfo) {
+ int avail;
+
+ if (!wait_for_remote_key(remotesock)) {
+ fprintf(stderr,
+ "TFO: timeout waiting for third ACK\n");
+ close(remotesock);
+ return 1;
+ }
+
+ 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] 18+ messages in thread* Re: [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow
2026-11-06 12:57 ` [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
@ 2026-11-06 13:15 ` sashiko-bot
2026-11-08 7:37 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-11-06 13:15 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] selftests: mptcp: unhandled EOPNOTSUPP in
wait_for_remote_key() for IPv4 TCP
--
commit 058a45159cca4e12fb9271f3ed25cdefa12d46b9
Author: Geliang Tang <geliang@kernel.org>
selftests: mptcp: connect: check TFO SIOCINQ overflow
This commit adds a SIOCINQ check in mptcp_connect's server mode when the
MPTFO socket option is enabled to detect TFO SIOCINQ overflow regressions.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..3a72bae43b906 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 bool wait_for_remote_key(int fd)
> +{
> + struct mptcp_info info;
> + int retries = 10;
> + socklen_t olen;
> +
> + while (retries > 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)
> + return true;
> + return false;
> + }
[Severity: Medium]
Should wait_for_remote_key() also check for EOPNOTSUPP here?
For AF_INET TCP sockets (and MPTCP sockets that fell back to TCP),
tcp_getsockopt() delegates non-SOL_TCP levels to ip_getsockopt() ->
do_ip_getsockopt(), which returns -EOPNOTSUPP instead of -ENOPROTOOPT
when level != SOL_IP. As a result, IPv4 TCP and TCP-fallback test cases
with -o MPTFO return false from wait_for_remote_key() and fail in
main_loop_s() with "TFO: timeout waiting for third ACK".
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793969127.git.tanggeliang@kylinos.cn?part=2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow
2026-11-06 12:57 ` [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
2026-11-06 13:15 ` sashiko-bot
@ 2026-11-08 7:37 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, kuba
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] wait_for_remote_key() in
tools/testing/selftests/net/mptcp/mptcp_connect.c only treats errno ==…
- [Low] The new TFO check in main_loop_s()
(tools/testing/selftests/net/mptcp/mptcp_connect.c) fails the whole…
--
selftests: mptcp: connect: check TFO SIOCINQ overflow
This adds a SIOCINQ check to mptcp_connect's server mode when the MPTFO
socket option is enabled. The server polls MPTCP_INFO until
MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED is set, then fails if SIOCINQ returns
INT_MAX. ENOPROTOOPT from getsockopt() is meant to skip the wait for TCP
sockets and for MPTCP sockets that fell back to TCP.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..3a72bae43b906 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 bool wait_for_remote_key(int fd)
> +{
> + struct mptcp_info info;
> + int retries = 10;
> + socklen_t olen;
> +
> + while (retries > 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)
> + return true;
> + return false;
[Severity: Medium]
Does this skip branch work for IPv4? On an IPv4 TCP socket, or an IPv4
MPTCP socket that fell back to TCP, getsockopt(SOL_MPTCP) seems to fail
with EOPNOTSUPP rather than ENOPROTOOPT.
The call goes tcp_getsockopt()->ip_getsockopt()->do_ip_getsockopt(), and
there:
net/ipv4/ip_sockglue.c:do_ip_getsockopt() {
...
if (level != SOL_IP)
return -EOPNOTSUPP;
...
}
The netfilter fallback in ip_getsockopt() only runs for -ENOPROTOOPT, so
EOPNOTSUPP reaches userspace unchanged. A fallen-back msk takes the same
path, because mptcp_getsockopt() forwards the call to
tcp_getsockopt(ssk, ...).
Only the IPv6 path returns ENOPROTOOPT:
net/ipv6/ipv6_sockglue.c:ipv6_getsockopt() {
...
if (level != SOL_IPV6)
return -ENOPROTOOPT;
...
}
So on IPv4 this helper returns false, and main_loop_s() prints "TFO:
timeout waiting for third ACK" and returns 1.
That looks like it would fail every IPv4 -o MPTFO transfer where the
server side is TCP or falls back to TCP. Examples are mptcp_connect.sh with
-t (the MPTCP TCP and TCP MPTCP transfers that run_tests_mptfo() starts
through run_tests_lo() for 10.0.1.1), -tt, or a manual
"mptcp_connect -l -s TCP -o MPTFO 0.0.0.0".
mptcp_sockopt.c already checks for EOPNOTSUPP after a SOL_MPTCP
getsockopt. Should EOPNOTSUPP be accepted here too, or should the wait be
skipped when the socket isn't MPTCP?
> + }
> + if (info.mptcpi_flags & MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED)
> + return true;
> + usleep(10000);
> + retries--;
> + }
> + return false;
> +}
> +
> int main_loop_s(int listensock)
> {
> struct sockaddr_storage ss;
> @@ -1207,6 +1233,24 @@ int main_loop_s(int listensock)
> check_sockaddr(pf, &ss, salen);
> check_getpeername(remotesock, &ss, salen);
>
> + if (cfg_sockopt_types.mptfo) {
> + int avail;
> +
> + if (!wait_for_remote_key(remotesock)) {
> + fprintf(stderr,
> + "TFO: timeout waiting for third ACK\n");
> + close(remotesock);
> + return 1;
> + }
[Severity: Low]
Should a missed remote key fail the whole transfer? Not seeing
MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED within the fixed 100ms (10 x
usleep(10000)) only means the SIOCINQ precondition isn't met yet. It
doesn't mean the SIOCINQ regression happened.
The rest of the tool uses poll_timeout = 10 * 1000 ms. The third ACK
depends on softirq processing in the peer namespace. On a slow or loaded
CI VM (debug, KASAN or lockdep kernels), could it arrive after 100ms and
make the MPTFO group flaky? Would skipping the SIOCINQ check, or a longer
wait, be safer here?
Also, wait_for_remote_key() returns the same false for a real timeout and
for any getsockopt() error. This message always reports a timeout, so
errno-based failures like the IPv4 EOPNOTSUPP case above show up as
timeouts and the real cause is lost.
> +
> + if (ioctl(remotesock, SIOCINQ, &avail) == 0 &&
> + avail == INT_MAX) {
> + fprintf(stderr, "TFO SIOCINQ overflow\n");
> + close(remotesock);
> + return 1;
> + }
> + }
> +
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH mptcp-net v23 3/8] mptcp: fix fallback dummy mapping anchor
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
2026-11-06 12:57 ` [PATCH mptcp-net v23 1/8] mptcp: fix TFO SIOCINQ overflow in mptcp_inq_hint Geliang Tang
2026-11-06 12:57 ` [PATCH mptcp-next v23 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 12:57 ` [PATCH mptcp-next v23 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
` (5 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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] 18+ messages in thread* Re: [PATCH mptcp-net v23 3/8] mptcp: fix fallback dummy mapping anchor
2026-11-06 12:57 ` [PATCH mptcp-net v23 3/8] mptcp: fix fallback dummy mapping anchor Geliang Tang
@ 2026-11-08 7:37 ` netdev-bot+sashiko
0 siblings, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, 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 From: and Signed-off-by: email addresses don't match in
commit "mptcp: fix fallback dummy mapping anchor".
--
mptcp: fix fallback dummy mapping anchor
The fallback dummy mapping in subflow_check_data_avail() now anchors
map_subflow_seq at TCP_SKB_CB(skb)->seq rather than at copied_seq. This
makes it match map_seq, which is already anchored at the DSN of skb->seq.
With both anchored there, mptcp_subflow_get_mapped_dsn() returns the
correct DSN at copied_seq when the queued skb partially overlaps
already-received data.
> Author: Geliang Tang <geliang@kernel.org>
[ ... ]
> Fixes: 2834f8edd74d ("mptcp: cleanup fallback dummy mapping generation")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
[Severity: Low]
This isn't a bug, but the author address and the only Signed-off-by
address are different:
Author: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
checkpatch.pl reports this as FROM_SIGN_OFF_MISMATCH.
Could you either add a "From: Geliang Tang <tanggeliang@kylinos.cn>" line
at the top of the body so the author matches the sign-off, or add a
Signed-off-by for the geliang@kernel.org address?
Other patches in the same series, such as "mptcp: drop the
mptcp_ooo_try_coalesce() helper" and "mptcp: remove CB offset field",
are signed off as Geliang Tang <geliang@kernel.org>. Should one identity
be used across the whole series?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH mptcp-next v23 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (2 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-net v23 3/8] mptcp: fix fallback dummy mapping anchor Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-06 12:57 ` [PATCH mptcp-next v23 5/8] mptcp: drop the cant_coalesce CB field Geliang Tang
` (4 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
It's used to save an additional comparison for in-order skbs, but is
also a barrier to remove CB offset. Remove the helper, let
__mptcp_try_coalesce() always perform the sequence check and remove
duplicate checks from the callers.
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 421df3af1ef9..4495a148b40c 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] 18+ messages in thread* [PATCH mptcp-next v23 5/8] mptcp: drop the cant_coalesce CB field
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (3 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-next v23 4/8] mptcp: drop the mptcp_ooo_try_coalesce() helper Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-06 12:57 ` [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field Geliang Tang
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
Such field is used to ensure in-sequence processing in case of fastopen.
Instead let's perform synchronization of the fastopen skb sequence
when the IASN becomes available with the 3rd ack.
When the cant_coalesce field has been introduced, commit f03afb3aeb9d
("mptcp: drop __mptcp_fastopen_gen_msk_ackseq()") noted that updating the
already queued skb for passive fastopen socket at 3rd ack time would be
difficult and race prone. The main point is that such update don't need
to be synchronously performed at 3rd ack time, but is sufficient to
perform it before the next segment is introduced into the msk.
To such extent, add __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 4495a148b40c..a6c4bad0a8b9 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);
@@ -3676,6 +3704,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 */
@@ -3878,6 +3908,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] 18+ messages in thread* [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (4 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-next v23 5/8] mptcp: drop the cant_coalesce CB field Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-06 13:41 ` sashiko-bot
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 12:57 ` [PATCH mptcp-next v23 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
` (2 subsequent siblings)
8 siblings, 2 replies; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
Instead, use a new msk-level field to track the bytes already consumed
inside each skb, carrying the amount of bytes already copied to
user-space, alike what TCP is already doing.
The newly 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 are removed via
pskb_pull() (to handle non-linear skbs safely) before setting map_seq.
The function now returns bool to indicate success. When pskb_pull() fails,
the caller updates seq and jumps to the cleanup path to ensure
tp->copied_seq and more_data_avail are properly updated.
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 | 8 ++--
net/mptcp/protocol.c | 98 +++++++++++++++++++++++++-------------------
net/mptcp/protocol.h | 2 +-
net/mptcp/subflow.c | 6 ++-
4 files changed, 66 insertions(+), 48 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index e18b31028e7e..78481c100e2a 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -45,10 +45,11 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
subflow->ssn_offset += skb->len;
has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
- /* Only the sequence delta is relevant */
- MPTCP_SKB_CB(skb)->map_seq = -skb->len;
+ /* The TFO segment data sits before the IASN; before receiving
+ * the remote key, IASN is assumed being 0.
+ */
+ MPTCP_SKB_CB(skb)->map_seq = -(u64)skb->len;
MPTCP_SKB_CB(skb)->end_seq = 0;
- MPTCP_SKB_CB(skb)->offset = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
mptcp_data_lock(sk);
@@ -56,6 +57,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 a6c4bad0a8b9..f75297c4d6b0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,7 +168,6 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
int limit = READ_ONCE(sk->sk_rcvbuf);
if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
- MPTCP_SKB_CB(from)->offset ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
return false;
@@ -415,25 +414,38 @@ 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 || !pskb_pull(skb, offset)) {
+ WARN_ON_ONCE(1);
+ 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)->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 +506,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 +863,10 @@ 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)) {
+ seq += len;
+ goto out;
+ }
if (own_msk) {
mptcp_subflow_lend_fwdmem(subflow, skb);
@@ -878,6 +889,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 +930,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 +2216,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 +2252,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 +2396,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;
@@ -2441,6 +2441,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
struct mptcp_sock *msk = mptcp_sk(sk);
struct scm_timestamping_internal tss;
int copied = 0, cmsg_flags = 0;
+ u64 peek_seq, *seq;
int target;
long timeo;
@@ -2459,6 +2460,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
len = min_t(size_t, len, INT_MAX);
target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);
+ seq = &msk->copied_seq;
+ if (flags & MSG_PEEK) {
+ peek_seq = msk->copied_seq;
+ seq = &peek_seq;
+ }
if (unlikely(msk->recvmsg_inq))
cmsg_flags = MPTCP_CMSG_INQ;
@@ -2468,7 +2474,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int err, bytes_read;
bytes_read = __mptcp_recvmsg_mskq(sk, msg, len - copied, flags,
- copied, &tss, &cmsg_flags,
+ seq, &tss, &cmsg_flags,
&last);
if (unlikely(bytes_read < 0)) {
if (!copied)
@@ -2478,8 +2484,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
copied += bytes_read;
- if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk))
+ if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk)) {
+ if (flags & MSG_PEEK)
+ peek_seq = msk->copied_seq + copied;
continue;
+ }
/* only the MPTCP socket status is relevant here. The exit
* conditions mirror closely tcp_recvmsg()
@@ -2522,6 +2531,10 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
copied = copied ? : err;
goto out_err;
}
+
+ /* Recompute peek offset after eventual seq resync. */
+ if (flags & MSG_PEEK)
+ peek_seq = msk->copied_seq + copied;
}
mptcp_cleanup_rbuf(msk, copied);
@@ -3698,6 +3711,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;
@@ -3908,8 +3922,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)
@@ -4630,7 +4646,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;
@@ -4672,11 +4688,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..7878e6ac6c0b 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -131,7 +131,6 @@
struct mptcp_skb_cb {
u64 map_seq;
u64 end_seq;
- u32 offset;
u8 has_rxtstamp;
};
@@ -311,6 +310,7 @@ struct mptcp_sock {
u64 rcv_data_fin_seq;
u64 bytes_retrans;
u64 bytes_consumed;
+ u64 copied_seq;
int snd_burst;
int old_wspace;
u64 recovery_snd_nxt; /* in recovery mode accept up to this seq;
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] 18+ messages in thread* Re: [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field
2026-11-06 12:57 ` [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field Geliang Tang
@ 2026-11-06 13:41 ` sashiko-bot
2026-11-08 7:37 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-11-06 13:41 UTC (permalink / raw)
To: Geliang Tang, mptcp
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] mptcp: truesize corruption after pskb_pull() in
mptcp_init_skb()
- [Medium] mptcp: unpulled skb prefix in mptcp_init_skb() rewinds
map_seq
--
commit 6d46cc996583689225db9776f596454406aeafdb
Author: Paolo Abeni <pabeni@redhat.com>
mptcp: remove CB offset field
This commit replaces the per-skb MPTCP_SKB_CB(skb)->offset field with an
msk-level copied_seq counter to track consumed receive data across skbs.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a6c4bad0a8b96..f75297c4d6b0d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -415,25 +414,38 @@ 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 || !pskb_pull(skb, offset)) {
> + WARN_ON_ONCE(1);
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> + offset = 0;
> + }
[Severity: High]
Can calling pskb_pull() here corrupt skb->truesize and socket memory
accounting when skb is nonlinear?
At this point skb is still owned by ssk with skb->destructor set to
sock_rfree. When offset exceeds skb_headlen(skb) and the linear head
lacks tailroom (or skb is cloned), __pskb_pull_tail() calls
pskb_expand_head(), which increases skb_end_offset(skb) without updating
skb->truesize because skb->sk is set:
net/core/skbuff.c:pskb_expand_head() {
...
if (!skb->sk || skb->destructor == sock_edemux)
skb->truesize += size - osize;
}
pskb_pull() then advances skb->data to skb->tail, leaving
skb_headlen(skb) == 0. If this skb is later coalesced as the from
buffer in __mptcp_try_coalesce() -> skb_try_coalesce(), the
skb_headlen(from) == 0 path computes:
delta = from->truesize - SKB_TRUESIZE(skb_end_offset(from));
Because skb_end_offset(from) grew while from->truesize did not, delta is
underestimated by the head expansion size and can be smaller than
from->len or negative, triggering WARN_ON_ONCE(delta < len) in
skb_try_coalesce() and corrupting to->truesize and sk_rmem_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;
[Severity: Medium]
When a TCP subflow queues a partially overlapping segment that starts
within the current DSS mapping, offset is positive while
before(TCP_SKB_CB(skb)->seq - subflow->ssn_offset,
subflow->map_subflow_seq) is false. Does rewinding map_seq by offset
without pulling the prefix expose unvalidated bytes to the MPTCP queues?
When data checksums are enabled, validate_data_csum() starts at
tcp_sk(ssk)->copied_seq + subflow->map_csum_len, so the already-consumed
prefix skb[0..offset) in an overlapping segment was never checked
against the DSS checksum. Rewinding map_seq to include that prefix
causes mptcp_data_queue_ofo() to treat the unpulled bytes as valid
payload covering earlier DSNs, replacing and dropping previously
validated out-of-order skbs:
net/mptcp/protocol.c:mptcp_data_queue_ofo() {
...
} else {
/* skb's seq == skb1's seq and skb covers skb1.
* Replace skb1 with skb.
*/
rb_replace_node(&skb1->rbnode, &skb->rbnode,
&msk->out_of_order_queue);
mptcp_drop(sk, skb1);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
goto merge_right;
}
}
When msk->ack_seq later advances to that mapping, userspace reads the
unvalidated prefix from skb instead of the validated bytes that were in
skb1.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793969127.git.tanggeliang@kylinos.cn?part=6
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field
2026-11-06 12:57 ` [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field Geliang Tang
2026-11-06 13:41 ` sashiko-bot
@ 2026-11-08 7:37 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, kuba
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] mptcp: truesize corruption after pskb_pull() in mptcp_init_skb()
- [Medium] mptcp_init_skb() (net/mptcp/protocol.c) gains a new mapping-
boundary branch.
--
mptcp: remove CB offset field
The patch drops the per-skb MPTCP_SKB_CB(skb)->offset field. Bytes
already consumed inside each skb are now tracked by a new msk-level
copied_seq field, as plain TCP does. When an skb spans multiple DSS
mappings, mptcp_init_skb() now strips the consumed prefix with
pskb_pull(), and it returns false if the pull fails.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a6c4bad0a8b96..f75297c4d6b0d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -415,25 +414,38 @@ 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 || !pskb_pull(skb, offset)) {
> + WARN_ON_ONCE(1);
> + sk_eat_skb(ssk, skb);
> + return false;
> + }
> + offset = 0;
> + }
[Severity: High]
Does this pskb_pull() leave skb->truesize out of sync with the head
size?
At this point the skb is still on ssk->sk_receive_queue and still owned
by the subflow, so skb->sk is ssk and skb->destructor is sock_rfree.
When offset exceeds skb_headlen(), pskb_pull() goes through
__pskb_pull_tail(). That calls pskb_expand_head() when tailroom is
short or the skb is cloned.
pskb_expand_head() only fixes up truesize for skbs that are orphaned:
net/core/skbuff.c:pskb_expand_head() {
...
if (!skb->sk || skb->destructor == sock_edemux)
skb->truesize += size - osize;
...
}
So skb_end_offset(skb) grows and skb->truesize stays the same.
pskb_pull() then advances skb->data by the whole offset. That leaves
skb_headlen(skb) == 0, with all remaining payload in frags.
Now suppose this skb later becomes the from argument of
skb_try_coalesce(). That can happen through __mptcp_try_coalesce() from
__mptcp_move_skb(), __mptcp_add_backlog(), mptcp_data_queue_ofo() or
__mptcp_ofo_queue(). The headlen == 0 branch there computes:
delta = from->truesize - SKB_TRUESIZE(skb_end_offset(from));
This delta is now short by the head expansion. It can fall below
from->len, or go negative, and trip WARN_ON_ONCE(delta < len).
The wrong delta is then added to to->truesize and charged through
mptcp_try_coalesce() into sk->sk_rmem_alloc and the msk forward memory.
On the backlog path it also feeds into the ssk rmem and
msk->backlog_len accounting. So the receive memory accounting drifts
for the rest of the connection's life. Even without coalescing, the
skb is under-charged when its truesize is moved to the msk.
As described for the other pskb_pull() concern, a peer can reach this
branch with a segment that partially overlaps already consumed data and
carries a DSS mapping starting in its middle. With GRO-merged or
coalesced paged skbs, this looks remotely triggerable.
How should this be fixed? One option is to uncharge or orphan the skb
from ssk before modifying it, and recompute truesize after the pull.
Another is to avoid reshaping the skb in this path at all. Which one
did you have in mind?
[Severity: Medium]
Can this pskb_pull() fail under normal runtime conditions and then WARN
and drop data?
When offset is larger than skb_headlen(), pskb_pull() goes through
__pskb_pull_tail(). That function returns NULL in two cases:
net/core/skbuff.c:__pskb_pull_tail() {
...
if (!skb_frags_readable(skb))
return NULL;
if (eat > 0 || skb_cloned(skb)) {
if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
GFP_ATOMIC))
return NULL;
}
...
}
The first is any skb with unreadable (devmem/netmem) frags. The second
is a GFP_ATOMIC failure when the skb has no tailroom or is cloned, for
example while a packet tap is active.
So an ordinary atomic allocation failure in softirq hits WARN_ON_ONCE(1).
On hosts with panic_on_warn, that becomes a crash.
Can a remote peer steer packets into this branch?
validate_mapping() in net/mptcp/subflow.c compares tp->copied_seq, not
the skb start, against the mapping:
u32 ssn = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
if (unlikely(before(ssn, subflow->map_subflow_seq))) {
Suppose [S, S+100) has already been consumed. The peer can then send a
partially overlapping segment [S+50, S+200) whose DSS mapping starts at
S+100. tcp_data_queue() queues that segment untrimmed, and
mptcp_init_skb() is then called with offset > 0 on an skb that starts
before map_subflow_seq.
Even when the pull succeeds, the receiver may have to reallocate a
(possibly high-order) linear head in softirq. It then copies up to a
GRO-merged skb's worth of paged data (around 64KB) into it, only to
throw that data away.
The previous code handled the same case without any allocation by
recording MPTCP_SKB_CB(skb)->offset.
> /* the skb map_seq accounts for the skb offset:
> * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
[ ... ]
> @@ -855,7 +863,10 @@ 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)) {
> + seq += len;
> + goto out;
> + }
On the failure path, is it intended that tp->copied_seq moves past the
skb that sk_eat_skb() just freed?
Those bytes have already been acked at the TCP level, but msk->ack_seq
never covers them. They are dropped without bumping any MIB counter. The
goto also skips the map_remaining < len DSS-corruption check below.
The MPTCP stream would then stall until MPTCP-level retransmission or
reinjection fills the hole.
The commit message only says:
When pskb_pull() fails,
the caller updates seq and jumps to the cleanup path to ensure
tp->copied_seq and more_data_avail are properly updated.
It does not mention the WARN or the dropped data.
>
> if (own_msk) {
> mptcp_subflow_lend_fwdmem(subflow, skb);
> @@ -878,6 +889,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);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH mptcp-next v23 7/8] mptcp: sync mptcp skb cb layout with tcp one
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (5 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-next v23 6/8] mptcp: remove CB offset field Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 12:57 ` [PATCH mptcp-next v23 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
2026-11-06 15:01 ` [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
8 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Geliang Tang
From: Paolo Abeni <pabeni@redhat.com>
The MPTCP protocol uses a significantly different CB layout WRT TCP, as it
includes different information and 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 fields with a corresponding
TCP-level data use the same area 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 adds a flags field.
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 shared fields.
In mptcp_init_skb(), map_seq64 is computed as mptcp_subflow_get_mapped_dsn()
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 | 6 ++-
net/mptcp/protocol.c | 94 +++++++++++++++++++++++++++-----------------
net/mptcp/protocol.h | 7 +++-
3 files changed, 67 insertions(+), 40 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 78481c100e2a..bd52e9528b29 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;
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 f75297c4d6b0..2a99b5ced3b0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -168,11 +168,13 @@ static bool __mptcp_try_coalesce(struct sock *sk, struct sk_buff *to,
int limit = READ_ONCE(sk->sk_rcvbuf);
if (MPTCP_SKB_CB(from)->map_seq != MPTCP_SKB_CB(to)->end_seq ||
+ MPTCP_SKB_CB(from)->map_seq64 != MPTCP_SKB_CB(to)->map_seq64 +
+ to->len ||
((to->len + from->len) > (limit >> 3)) ||
!skb_try_coalesce(to, from, fragstolen, delta))
return false;
- pr_debug("colesced seq %llx into %llx new len %d new end seq %llx\n",
+ pr_debug("colesced seq %x into %x new len %d new end seq %x\n",
MPTCP_SKB_CB(from)->map_seq, MPTCP_SKB_CB(to)->map_seq,
to->len, MPTCP_SKB_CB(from)->end_seq);
MPTCP_SKB_CB(to)->end_seq = MPTCP_SKB_CB(from)->end_seq;
@@ -254,8 +256,8 @@ static void mptcp_prune_ofo_queue(struct sock *sk,
struct sk_buff *skb = rb_to_skb(node);
/* Stop pruning if the incoming skb would land in OoO tail. */
- if (after64(MPTCP_SKB_CB(in_skb)->map_seq,
- MPTCP_SKB_CB(skb)->map_seq))
+ if (after(MPTCP_SKB_CB(in_skb)->map_seq,
+ MPTCP_SKB_CB(skb)->map_seq))
break;
pruned = true;
@@ -301,16 +303,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",
@@ -345,7 +354,7 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
}
/* Can avoid an rbtree lookup if we are adding skb after ooo_last_skb */
- if (!before64(seq, MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq)) {
+ if (!before(seq, MPTCP_SKB_CB(msk->ooo_last_skb)->end_seq)) {
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOQUEUETAIL);
parent = &msk->ooo_last_skb->rbnode;
p = &parent->rb_right;
@@ -357,18 +366,18 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
while (*p) {
parent = *p;
skb1 = rb_to_skb(parent);
- if (before64(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
+ if (before(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
p = &parent->rb_left;
continue;
}
- if (before64(seq, MPTCP_SKB_CB(skb1)->end_seq)) {
- if (!after64(end_seq, MPTCP_SKB_CB(skb1)->end_seq)) {
+ if (before(seq, MPTCP_SKB_CB(skb1)->end_seq)) {
+ if (!after((u32)end_seq, MPTCP_SKB_CB(skb1)->end_seq)) {
/* All the bits are present. Drop. */
mptcp_drop(sk, skb);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
return;
}
- if (after64(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
+ if (after(seq, MPTCP_SKB_CB(skb1)->map_seq)) {
/* partial overlap:
* | skb |
* | skb1 |
@@ -399,7 +408,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);
@@ -433,11 +442,13 @@ 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 = 0;
MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
__skb_unlink(skb, &ssk->sk_receive_queue);
@@ -462,13 +473,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;
@@ -480,7 +492,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)) {
@@ -498,14 +510,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;
}
@@ -904,39 +916,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;
@@ -2225,7 +2235,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;
@@ -4646,7 +4656,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;
@@ -4896,11 +4906,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 7878e6ac6c0b..fa54f70ab501 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -129,9 +129,12 @@
#define MPTCP_SYNC_SEQ 8
struct mptcp_skb_cb {
- u64 map_seq;
- u64 end_seq;
+ u32 map_seq;
+ u32 end_seq;
+ u32 unused;
+ u16 flags;
u8 has_rxtstamp;
+ u64 map_seq64;
};
#define MPTCP_SKB_CB(__skb) ((struct mptcp_skb_cb *)&((__skb)->cb[0]))
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH mptcp-next v23 7/8] mptcp: sync mptcp skb cb layout with tcp one
2026-11-06 12:57 ` [PATCH mptcp-next v23 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-11-08 7:37 ` netdev-bot+sashiko
0 siblings, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, 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 says the new layout makes "fields with a
corresponding TCP-level data use the same area inside the CB itself".
--
mptcp: sync mptcp skb cb layout with tcp one
This commit shrinks the MPTCP-level map_seq and end_seq CB fields to 32
bits. It also adds a map_seq64 field holding the full 64-bit DSN and a
flags field, and moves most receive path sequence comparisons to the
32-bit before()/after() helpers. Build-time checks are added so that some
MPTCP CB fields stay aligned with their tcp_skb_cb counterparts.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index f75297c4d6b0d..2a99b5ced3b0c 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -4896,11 +4906,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);
> +
[ ... ]
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 7878e6ac6c0b6..fa54f70ab501d 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -129,9 +129,12 @@
> #define MPTCP_SYNC_SEQ 8
>
> struct mptcp_skb_cb {
> - u64 map_seq;
> - u64 end_seq;
> + u32 map_seq;
> + u32 end_seq;
> + u32 unused;
> + u16 flags;
> u8 has_rxtstamp;
> + u64 map_seq64;
> };
[Severity: Low]
The commit message says this change allows:
"updating the MPTCP CB layout so that fields with a corresponding
TCP-level data use the same area inside the CB itself."
Is that accurate for has_rxtstamp? map_seq, end_seq and flags line up
with seq, end_seq and tcp_flags in struct tcp_skb_cb. has_rxtstamp does
not, although TCP has a field with the same name and meaning.
In struct mptcp_skb_cb, the u8 has_rxtstamp sits at byte 14. In struct
tcp_skb_cb, byte 14 is the sacked byte:
include/net/tcp.h:struct tcp_skb_cb {
...
__u16 tcp_flags; /* TCP header flags (tcp[12-13])*/
__u8 sacked; /* State flags for SACK. */
__u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
...
__u8 txstamp_ack:2, /* Record TX timestamp for ack? */
eor:1, /* Is skb MSG_EOR marked? */
has_rxtstamp:1, /* SKB has a RX timestamp */
unused:4;
__u32 ack_seq; /* Sequence number ACK'd */
...
}
TCP's has_rxtstamp bit is in byte 16. In the MPTCP struct, the new u64
map_seq64 covers bytes 16-23. It therefore overlaps both the
txstamp_ack/eor/has_rxtstamp byte and ack_seq.
The CHK_CB_FIELD() checks in mptcp_proto_init() only cover map_seq/seq,
end_seq/end_seq and flags/tcp_flags. offsetof() also can't be applied to
the has_rxtstamp bitfield, so nothing at build time would catch this.
As far as I can tell, no current path reads these bytes through
TCP_SKB_CB() on msk-level skbs:
- mptcp_init_skb() and mptcp_fastopen_subflow_synack_set_params() copy
TCP_SKB_CB(skb)->has_rxtstamp into a local before rewriting the CB.
- __mptcp_recvmsg_mskq() reads it via MPTCP_SKB_CB().
Suppose a TCP helper is later run on skbs in the msk receive or OFO
queue and reads TCP_SKB_CB(skb)->has_rxtstamp, ->eor or ->sacked.
Wouldn't it silently get bits of map_seq64 or of the MPTCP has_rxtstamp
byte?
Would it make sense to do one of these?
- Narrow the commit message, including the "shared fields" wording
about the build time check, to seq, end_seq and flags.
- Place has_rxtstamp and map_seq64 so they don't collide with the TCP
fields, and document the overlap.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH mptcp-next v23 8/8] mptcp: defer read_sock cleanup to mptcp_worker
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (6 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-next v23 7/8] mptcp: sync mptcp skb cb layout with tcp one Geliang Tang
@ 2026-11-06 12:57 ` Geliang Tang
2026-11-08 7:37 ` netdev-bot+sashiko
2026-11-06 15:01 ` [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage MPTCP CI
8 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2026-11-06 12:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang, Paolo Abeni
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.
Restore the backlog flush in mptcp_recv_skb() with a sock_owned_by_user()
check. This ensures process context callers flush the backlog before
processing skbs, while future BH context callers skip it to avoid
deadlock on mptcp_data_lock().
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 2a99b5ced3b0..98556d1ebace 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3165,6 +3165,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;
@@ -3240,6 +3255,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);
@@ -3283,6 +3301,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);
@@ -3730,6 +3749,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 */
@@ -4652,7 +4672,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) {
@@ -4669,6 +4689,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)
@@ -4710,11 +4731,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 fa54f70ab501..3580cf66f303 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -117,6 +117,7 @@
#define MPTCP_FALLBACK_DONE 2
#define MPTCP_WORK_CLOSE_SUBFLOW 3
#define MPTCP_RTX_ENABLED 4
+#define MPTCP_WORK_READ_COMPLETE 5
/* MPTCP socket release cb flags */
#define MPTCP_PUSH_PENDING 1
@@ -326,6 +327,7 @@ struct mptcp_sock {
u32 last_data_sent;
u32 last_data_recv;
u32 last_ack_recv;
+ int read_copied;
unsigned long timer_ival;
u32 token;
u64 tfo_iasn;
--
2.55.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH mptcp-next v23 8/8] mptcp: defer read_sock cleanup to mptcp_worker
2026-11-06 12:57 ` [PATCH mptcp-next v23 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-11-08 7:37 ` netdev-bot+sashiko
0 siblings, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:37 UTC (permalink / raw)
To: geliang; +Cc: mptcp, tanggeliang, pabeni, 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 says the patch will "Restore the backlog flush
in mptcp_recv_skb() with a sock_owned_by_user() check".
--
mptcp: defer read_sock cleanup to mptcp_worker
This patch prepares __mptcp_read_sock() for future callers that run in
BH context. When the msk is not owned by the user, it defers
mptcp_rcv_space_adjust() and mptcp_cleanup_rbuf() to mptcp_worker() via
msk->read_copied and MPTCP_WORK_READ_COMPLETE. It also makes the backlog
flush in mptcp_recv_skb() depend on sock_owned_by_user().
> Restore the backlog flush in mptcp_recv_skb() with a sock_owned_by_user()
> check. This ensures process context callers flush the backlog before
> processing skbs, while future BH context callers skip it to avoid
> deadlock on mptcp_data_lock().
[Severity: Low]
Is "Restore" the right word here? In the parent commit, mptcp_recv_skb()
in net/mptcp/protocol.c already flushed the backlog unconditionally:
if (!list_empty(&msk->backlog_list))
mptcp_move_skbs(sk);
That flush came in with commit 250d9766a984, and nothing earlier in the
series removed it. This patch only adds a condition to it:
if (sock_owned_by_user(sk) && !list_empty(&msk->backlog_list))
mptcp_move_skbs(sk);
So the existing flush is now skipped whenever the msk is not owned by the
user. The wording looks left over from an earlier revision in which the
flush was removed.
Could the message say instead that the existing flush is now gated on lock
ownership? It could also give the reason: BH callers would already hold
the msk spinlock, so mptcp_data_lock() inside mptcp_move_skbs() would
self-deadlock.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793969127.git.tanggeliang%40kylinos.cn
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage
2026-11-06 12:57 [PATCH mptcp-next v23 0/8] Reduce the differences between TCP and MPTCP for TLS usage Geliang Tang
` (7 preceding siblings ...)
2026-11-06 12:57 ` [PATCH mptcp-next v23 8/8] mptcp: defer read_sock cleanup to mptcp_worker Geliang Tang
@ 2026-11-06 15:01 ` MPTCP CI
8 siblings, 0 replies; 18+ messages in thread
From: MPTCP CI @ 2026-11-06 15:01 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: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/42224796804
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1e352901aecd
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1204169
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] 18+ messages in thread