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

[Matthieu Baerts <matttbe@kernel.org>]

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).

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