[PATCH] mptcp: do not drop partial packets

Shardul Bankar posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
net/mptcp/protocol.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
[PATCH] mptcp: do not drop partial packets
Posted by Shardul Bankar 1 month, 3 weeks 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>
---
 net/mptcp/protocol.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 614c3f583ca0..6858e6e283e3 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -397,12 +397,27 @@ 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);
+	tail = skb_peek_tail(&sk->sk_receive_queue);
+	if (tail && mptcp_try_coalesce(sk, tail, skb))
+		return true;
+
+	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] mptcp: do not drop partial packets
Posted by Paolo Abeni 1 month, 3 weeks ago
On 4/22/26 2:09 PM, Shardul Bankar wrote:
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 614c3f583ca0..6858e6e283e3 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -397,12 +397,27 @@ 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;

here MPTCP_SKB_CB(skb)->offset is always != 0 ...

> +	msk->bytes_received += copy_len;
> +	WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
> +	tail = skb_peek_tail(&sk->sk_receive_queue);
> +	if (tail && mptcp_try_coalesce(sk, tail, skb))

... so mptcp_try_coalesce() will always fail.

/P
Re: [PATCH] mptcp: do not drop partial packets
Posted by Shardul Bankar 1 month, 3 weeks ago
On Wed, 2026-04-22 at 15:51 +0200, Paolo Abeni wrote:
> On 4/22/26 2:09 PM, Shardul Bankar wrote:
> > +
> > +       /* 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;
> 
> here MPTCP_SKB_CB(skb)->offset is always != 0 ...
> 
> > +       msk->bytes_received += copy_len;
> > +       WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len);
> > +       tail = skb_peek_tail(&sk->sk_receive_queue);
> > +       if (tail && mptcp_try_coalesce(sk, tail, skb))
> 
> ... so mptcp_try_coalesce() will always fail.

Thanks, good catch. Fixed in v2.

Regards,
Shardul