Re: [PATCH net v2 15/15] mptcp: avoid pruning for OoW data

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Incomplete realization of the patch's stated principle ("do it
  only when we expect to accept the skb triggering the cleanup"): after…

--

mptcp: avoid pruning for OoW data

This moves the mptcp_try_rmem_schedule() call in mptcp_data_queue_ofo()
so that it runs after the out-of-window check instead of before it, which
avoids running the destructive OoO queue pruning for skbs that are going
to be dropped as out of window.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 4309fca6b119..0098e2830931 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -313,12 +313,6 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>  	u64 seq, end_seq, max_seq;
>  	struct sk_buff *skb1;
>  
> -	if (!mptcp_try_rmem_schedule(sk, skb)) {
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		mptcp_drop(sk, skb);
> -		return;
> -	}
> -
>  	seq = MPTCP_SKB_CB(skb)->map_seq;
>  	end_seq = MPTCP_SKB_CB(skb)->end_seq;
>  	max_seq = atomic64_read(&msk->rcv_wnd_sent);
> @@ -335,6 +329,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>  		return;
>  	}
>  
> +	if (!mptcp_try_rmem_schedule(sk, skb)) {
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +		mptcp_drop(sk, skb);
> +		return;
> +	}
> +

[Severity: Medium]
The commit message says pruning should be done "only when we expect
to accept the skb triggering the cleanup".  Does the new placement
fully match that, given the duplicate-data check still happens later in
the rbtree walk of mptcp_data_queue_ofo()?

Consider an OoO queue holding A (map_seq 1000, end_seq 2000), B (2000,
3000), C (3000, 4000) with ooo_last_skb == C, and an in-window skb
(map_seq 1200, end_seq 1500) arriving while sk_rmem_alloc exceeds
sk_rcvbuf.  mptcp_prune_ofo_queue() walks back from ooo_last_skb:

net/mptcp/protocol.c:mptcp_prune_ofo_queue() {
	...
		/* 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))
			break;

		pruned = true;
		prev = rb_prev(node);
		rb_erase(node, &msk->out_of_order_queue);
		mptcp_drop(sk, skb);
	...
}

after64(1200, 3000) and after64(1200, 2000) are both false, so C and
then B are erased and freed; the loop only breaks at A.

mptcp_can_ingest() then succeeds, and the tree walk reaches:

			if (!after64(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;
			}

so the incoming skb is discarded as duplicate after two already queued
segments were destroyed for it.  Since neither the rbtree walk nor
rb_link_node()/rb_insert_color() allocates memory, would it work to move
the mptcp_try_rmem_schedule() call below the duplicate and coalesce
classification instead of only below the out-of-window test?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org