Re: [PATCH v6 mptcp-next 11/11] mptcp: leverage the backlog for RX packet processing

[Mat Martineau <martineau@kernel.org>]

On Thu, 23 Oct 2025, Matthieu Baerts wrote:

> Hi Paolo, Mat,
>
> On 23/10/2025 17:11, Paolo Abeni wrote:
>>
>>
>> On 10/22/25 4:31 PM, Paolo Abeni wrote:
>>> When the msk socket is owned or the msk receive buffer is full,
>>> move the incoming skbs in a msk level backlog list. This avoid
>>> traversing the joined subflows and acquiring the subflow level
>>> socket lock at reception time, improving the RX performances.
>>>
>>> When processing the backlog, use the fwd alloc memory borrowed from
>>> the incoming subflow. skbs exceeding the msk receive space are
>>> not dropped; instead they are kept into the backlog until the receive
>>> buffer is freed. Dropping packets already acked at the TCP level is
>>> explicitly discouraged by the RFC and would corrupt the data stream
>>> for fallback sockets.
>>>
>>> Move the conditional reschedule in release_cb() to take action only
>>> after the first loop iteration, to avoid rescheduling just before
>>> releasing the lock.
>>>
>>> Special care is needed to avoid adding skbs to the backlog of a closed
>>> msk and to avoid leaving dangling references into the backlog
>>> at subflow closing time.
>
> (...)
>
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index 5a1d8f9e0fb0ec..0aae17ab77edb2 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>
> (...)
>
>>> -static bool __mptcp_move_skbs(struct sock *sk)
>>> +static bool mptcp_can_spool_backlog(struct sock *sk, u32 moved,
>>> +				    struct list_head *skbs)
>>>  {
>>> -	struct mptcp_subflow_context *subflow;
>>>  	struct mptcp_sock *msk = mptcp_sk(sk);
>>> -	bool ret = false;
>>>
>>> -	if (list_empty(&msk->conn_list))
>>> +	if (list_empty(&msk->backlog_list))
>>>  		return false;
>>>
>>> -	subflow = list_first_entry(&msk->conn_list,
>>> -				   struct mptcp_subflow_context, node);
>>> -	for (;;) {
>>> -		struct sock *ssk;
>>> -		bool slowpath;
>>> +	/* Borrowed mem could be zero only in the unlikely event that the bl
>>> +	 * is full
>>> +	 */
>>> +	if (likely(msk->borrowed_mem)) {
>>> +		sk_forward_alloc_add(sk, msk->borrowed_mem);
>>> +		msk->borrowed_mem = 0;
>>> +		sk->sk_reserved_mem = msk->backlog_len;
>>
>> With the above I intended to prevent the fwd memory handling from
>> releasing backlog_len bytes. Re-reading the relevant code, it does not
>> allow that (experimentation confirmed), see:
>>
>> https://elixir.bootlin.com/linux/v6.18-rc2/source/include/net/sock.h#L1593
>>
>> and:
>>
>> https://elixir.bootlin.com/linux/v6.18-rc2/source/include/net/sock.h#L1580
>>
>> This will need some more care. Also patch 2 will require some
>> significant rework.
>
> Thank you for looking at this complex part, and for having spot that!
>
>> @Mat, @Matttbe: could you please consider merging patches 1,3-9?
>>
>> I think they should be pretty uncontroversial, would make the series
>> more manegeable for future iterations (and would alleviate my
>> frustration to make this thing work correctly).
>
> It makes sense, fine by me. I will wait for Mat's review before applying
> them (patch 1 is for 'net' I suppose).
>

Applying 1,3-9 to our tree(s) makes sense to me. Maybe patch 5 to -net 
too? (just sent email about that before I saw this message).

Matthieu do you want me to reply to each so the RvB tag is in patchwork, 
or does this suffice for patches 1 & 3-9:

Reviewed-by: Mat Martineau <martineau@kernel.org>

- Mat