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

[Matthieu Baerts <matttbe@kernel.org>]

Hi Paolo,

Thank you for having checked these comments.

On 09/09/2026 17:50, Paolo Abeni wrote:
> On 9/9/26 4:49 PM, netdev-bot+sashiko@kernel.org 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.

I agree with you.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.