Re: [PATCH net v2] tun: bound the NAPI receive queue

[Antonio Quartulli <antonio@mandelbit.com>]

Hi Willem,

thanks a lot for the review. See below:

On 30/10/2026 22:52, Willem de Bruijn wrote:
> Antonio Quartulli wrote:
[...]>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 770b8fe085c3..1ace0c1f22f2 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -60,6 +60,7 @@
>>   #include <linux/rcupdate.h>
>>   #include <net/net_namespace.h>
>>   #include <net/netns/generic.h>
>> +#include <net/hotdata.h>
>>   #include <net/rtnetlink.h>
>>   #include <net/sock.h>
>>   #include <net/xdp.h>
> 
> Keep alphebetical order

ACK, will fix in v3.

> 
> 
>> @@ -1856,6 +1857,17 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>>   			zerocopy = true;
>>   	}
>>   
>> +	/* In NAPI mode packets are queued here and processed by the NAPI
>> +	 * poll, possibly in a kthread or ksoftirqd. Bound that queue like
>> +	 * the per-CPU backlog, and drop before allocating and copying.
>> +	 */
>> +	if (tfile->napi_enabled && !frags && !msg_control &&
> 
> Why disable if frags? That is still using NAPI scheduling.

Right, the exclusion doesn't help much.
In frags mode tun_get_user() processes the packet synchronously
with napi_gro_frags() and never queues to sk_write_queue, so the
check could only fire if something else filled that queue.
tun_xdp_one() can, on the same tfile, therefore I'll drop the
exclusion and also bounds tun_xdp_one() in v3.

> 
> Same for msg_control. tun_chr_write_iter calls with NULL msg_control.
> 
>> +	    unlikely(skb_queue_len_lockless(&tfile->sk.sk_write_queue) >=
>> +		     READ_ONCE(net_hotdata.max_backlog))) {
> 
> Nit: consider > instead of >=, same as enqueue_to_backlog
> 

ACK

>> +		dev_core_stats_rx_dropped_inc(tun->dev);
>> +		return total_len;
>> +	}
> 
> also best effort: more robust would require retesting once holding
> the lock.

I see.
In v3 I'll keep the lockless check before allocating and copying, and
recheck under the queue lock before __skb_queue_tail(), in both
tun_get_user() and tun_xdp_one().
Similarly to what enqueue_to_backlog() does.


Regards,

> 
>> +
>>   	if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
>>   		/* For the packet that is not easy to be processed
>>   		 * (e.g gso or jumbo packet), we will do it at after
>> -- 
>> 2.56.0
>>
> 

-- 
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com