[PATCH v2] mptcp: do not drop partial packets

Shardul Bankar posted 1 patch 1 week, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260422143931.43281-1-shardul.b@mpiricsoftware.com
net/mptcp/protocol.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
[PATCH v2] mptcp: do not drop partial packets
Posted by Shardul Bankar 1 week, 5 days ago
When a packet arrives with map_seq < ack_seq < end_seq, the beginning
of the packet has already been acknowledged but the end contains new
data.  Currently the entire packet is dropped as "old data," forcing
the sender to retransmit.

Instead, skip the already-acked bytes by adjusting the skb offset and
enqueue only the new portion.  Update bytes_received and ack_seq to
reflect the new data consumed.

A previous attempt at this fix (commit 1d2ce718811a ("mptcp: do not
drop partial packets"), reverted in commit bf39160c4218 ("Revert
"mptcp: do not drop partial packets"")) also added a zero-window
check and changed rcv_wnd_sent initialization, which caused test
regressions.  This version addresses only the partial packet handling
without modifying receive window accounting.

Fixes: ab174ad8ef76 ("mptcp: move ooo skbs into msk out of order queue.")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/600
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
v2: Drop the mptcp_try_coalesce() attempt for partial packets, since
    non-zero offset always prevents coalescing (Paolo).

 net/mptcp/protocol.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 614c3f583ca0..73ec6563ab2b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -397,12 +397,24 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 		return false;
 	}
 
-	/* old data, keep it simple and drop the whole pkt, sender
-	 * will retransmit as needed, if needed.
+	/* Completely old data? */
+	if (!after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
+		mptcp_drop(sk, skb);
+		return false;
+	}
+
+	/* Partial packet: map_seq < ack_seq < end_seq.
+	 * Skip the already-acked bytes and enqueue the new data.
 	 */
-	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
-	mptcp_drop(sk, skb);
-	return false;
+	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;
+	msk->bytes_received += copy_len;
+	WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
+
+	skb_set_owner_r(skb, sk);
+	__skb_queue_tail(&sk->sk_receive_queue, skb);
+	return true;
 }
 
 static void mptcp_stop_rtx_timer(struct sock *sk)
-- 
2.34.1
Re: [PATCH v2] mptcp: do not drop partial packets
Posted by Simon Horman 1 week, 3 days ago
From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
mptcp: do not drop partial packets

This commit modifies the handling of out-of-order packets where
map_seq < ack_seq < end_seq. Instead of dropping the entire packet,
it skips the already-acknowledged bytes and enqueues the remainder.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/600

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fbffd3a43fe86..4e9fba692a701 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -397,12 +397,24 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)

[ ... ]

> +	/* Partial packet: map_seq < ack_seq < end_seq.
> +	 * Skip the already-acked bytes and enqueue the new data.
> +	 */
> +	copy_len = MPTCP_SKB_CB(skb)->end_seq - msk->ack_seq;
> +	MPTCP_SKB_CB(skb)->offset += msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
> +	msk->bytes_received += copy_len;
> +	WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);

Should MPTCP_SKB_CB(skb)->map_seq also be advanced here to reflect the newly
adjusted offset?

If map_seq remains stale after skipping the already-acked bytes, does this
cause mptcp_inq_hint() to overestimate the amount of unread data? 

Looking at the hint calculation, it calculates the unread bytes using
READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq for the first
enqueued SKB.

If inq_hint is artificially large, could this corrupt the ioctl(FIONREAD)
and TCP_INQ cmsg values returned to userspace?

Furthermore, could this suppress receive window auto-tuning? In
mptcp_rcv_space_adjust(), an artificially large inq_hint might cause
the calculation copied -= mptcp_inq_hint(sk) to become negative, which
would stall window growth.

> +
> +	skb_set_owner_r(skb, sk);
> +	__skb_queue_tail(&sk->sk_receive_queue, skb);
> +	return true;
>  }
Re: [PATCH v2] mptcp: do not drop partial packets
Posted by Matthieu Baerts 1 week, 5 days ago
Hi Shardul,

On 22/04/2026 16:39, Shardul Bankar wrote:
> When a packet arrives with map_seq < ack_seq < end_seq, the beginning
> of the packet has already been acknowledged but the end contains new
> data.  Currently the entire packet is dropped as "old data," forcing
> the sender to retransmit.
> 
> Instead, skip the already-acked bytes by adjusting the skb offset and
> enqueue only the new portion.  Update bytes_received and ack_seq to
> reflect the new data consumed.

Thank you for the v2.

I didn't review it (yet), but just to let you know that there are some
rules on Netdev [1] (that usually also applied on MPTCP side), and an
important one is:

 - don't repost your patches within one 24h period

Each version generates a lot of emails that are sent and need to be
triaged. With the high volume, it is then harder for us to follow, plus
a lot of shared resources are used, etc.

One last thing, because this patch is not an urgent fix, do you mind
sending new versions only the to MPTCP ML: to a restricted number of
people for the first versions, there is enough traffic on Netdev.

[1] https://docs.kernel.org/process/maintainer-netdev.html

> A previous attempt at this fix (commit 1d2ce718811a ("mptcp: do not
> drop partial packets"), reverted in commit bf39160c4218 ("Revert

Note: these two commits should not be mentioned here, they have only
been applied to the MPTCP tree, but not upstreamed. Instead, please use
lore links, e.g.

https://lore.kernel.org/c9b426a4e163aa3c4fe8b80c79f1a610f47ae7d8.1763075056.git.pabeni@redhat.com

> "mptcp: do not drop partial packets"")) also added a zero-window
> check and changed rcv_wnd_sent initialization, which caused test
> regressions.  This version addresses only the partial packet handling
> without modifying receive window accounting.
> 
> Fixes: ab174ad8ef76 ("mptcp: move ooo skbs into msk out of order queue.")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/600
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>

Checkpatch is complaining that the author of the patch is not the same
as the one who sent the patch; You probably need to run this command to
fix it:

  git commit --amend --reset-author

(when the patch is sent, `git format-patch` will add a second From: tag)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH v2] mptcp: do not drop partial packets
Posted by Shardul Bankar 1 week, 4 days ago
On Wed, 2026-04-22 at 19:03 +0200, Matthieu Baerts wrote:
> Hi Shardul,
> 
> On 22/04/2026 16:39, Shardul Bankar wrote:
> > 
> 
> Thank you for the v2.
> 
> I didn't review it (yet), but just to let you know that there are
> some
> rules on Netdev [1] (that usually also applied on MPTCP side), and an
> important one is:
> 
>  - don't repost your patches within one 24h period
> 
> Each version generates a lot of emails that are sent and need to be
> triaged. With the high volume, it is then harder for us to follow,
> plus
> a lot of shared resources are used, etc.
> 
> One last thing, because this patch is not an urgent fix, do you mind
> sending new versions only the to MPTCP ML: to a restricted number of
> people for the first versions, there is enough traffic on Netdev.
> 
> [1] https://docs.kernel.org/process/maintainer-netdev.html
> 
> > A previous attempt at this fix (commit 1d2ce718811a ("mptcp: do not
> > drop partial packets"), reverted in commit bf39160c4218 ("Revert
> 
> Note: these two commits should not be mentioned here, they have only
> been applied to the MPTCP tree, but not upstreamed. Instead, please
> use
> lore links, e.g.
> 
> https://lore.kernel.org/c9b426a4e163aa3c4fe8b80c79f1a610f47ae7d8.1763075056.git.pabeni@redhat.com
> 
> > "mptcp: do not drop partial packets"")) also added a zero-window
> > check and changed rcv_wnd_sent initialization, which caused test
> > regressions.  This version addresses only the partial packet
> > handling
> > without modifying receive window accounting.
> > 
> > Fixes: ab174ad8ef76 ("mptcp: move ooo skbs into msk out of order
> > queue.")
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/600
> > Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> 
> Checkpatch is complaining that the author of the patch is not the
> same
> as the one who sent the patch; You probably need to run this command
> to
> fix it:
> 
>   git commit --amend --reset-author
> 
> (when the patch is sent, `git format-patch` will add a second From:
> tag)
> 
> Cheers,
> Matt

Hi Matthieu,

Thanks for the pointers. I'll wait for your code review and address the
process feedback (recipient list, commit message, email mismatch) along
with any code comments together in v3.

Regards,
Shardul
Re: [PATCH v2] mptcp: do not drop partial packets
Posted by Matthieu Baerts 1 week ago
Hi Shardul,

-Cc: netdev

On 23/04/2026 14:39, Shardul Bankar wrote:
> On Wed, 2026-04-22 at 19:03 +0200, Matthieu Baerts wrote:
>> Hi Shardul,
>>
>> On 22/04/2026 16:39, Shardul Bankar wrote:
>>>
>>
>> Thank you for the v2.
>>
>> I didn't review it (yet), but just to let you know that there are

(...)
> Thanks for the pointers. I'll wait for your code review and address the
> process feedback (recipient list, commit message, email mismatch) along
> with any code comments together in v3.
Just to avoid a deadlock here; don't hesitate to check Sashiko's
reviewed -- forwarded by Simon -- to see if it makes sense or not. If it
does, feel free to send a review. If it doesn't, please explain why.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH v2] mptcp: do not drop partial packets
Posted by MPTCP CI 1 week, 5 days ago
Hi Shardul,

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): Unstable: 3 failed test(s): packetdrill_dss packetdrill_mp_capable selftest_mptcp_connect_sendfile ⚠️ 
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/24785370107

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/8d6488282da0
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1084324


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)