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

[Paolo Abeni]

On 9/9/26 4:49 PM, netdev-bot+sashiko@xxxxxxxxxx wrote:
> 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()?

The change suggested here would add quite a bit of complexity for little
gain, and would diverge MPTCP behavior from TCP. We learned the lesson
the hard way we need very good reasons to do the latter. I'm pretty sure
we are better off with the proposed code.

/P