linux-kernel - Re: [PATCH net-next 3/4] tun: use napi_build_skb in __tun_build

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5AE43BD6-FC65-4A75-BD59-9CD858D1F6B2@nutanix.com>
Date: Thu, 8 May 2025 03:08:57 +0000
From: Jon Kohler <jon@...anix.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC: "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net"
	<daniel@...earbox.net>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "hawk@...nel.org" <hawk@...nel.org>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org"
	<bpf@...r.kernel.org>,
        "aleksander.lobakin@...el.com"
	<aleksander.lobakin@...el.com>,
        Jason Wang <jasowang@...hat.com>, Andrew Lunn
	<andrew+netdev@...n.ch>,
        Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
	<pabeni@...hat.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 3/4] tun: use napi_build_skb in __tun_build_skb



> On May 7, 2025, at 4:50 PM, Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> Jon Kohler wrote:
>> Use napi_build_skb for small payload SKBs that end up using the
>> tun_build_skb path.
>> 
>> Signed-off-by: Jon Kohler <jon@...anix.com>
>> ---
>> drivers/net/tun.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index f7f7490e78dc..7b13d4bf5374 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1538,7 +1538,11 @@ static struct sk_buff *__tun_build_skb(struct tun_file *tfile,
>>       int buflen, int len, int pad,
>>       int metasize)
>> {
>> - struct sk_buff *skb = build_skb(buf, buflen);
>> + struct sk_buff *skb;
>> +
>> + local_bh_disable();
>> + skb = napi_build_skb(buf, buflen);
>> + local_bh_enable();
> 
> The goal of this whole series seems to be to use the percpu skb cache
> for bulk alloc.

Yes

> 
> As all these helpers' prefix indicates, they are meant to be used with
> NAPI. Not sure using them on a tun write() datapath is deemed
> acceptable. Or even correct. Perhaps the infrastructure authors have
> an opinion.

@Alexsander: thoughts on this one? Following in the footsteps of
cpu_map_kthread_run, that appears to be its own things, non-NAPI,
and "bpf: cpumap: switch to napi_skb_cache_get_bulk()” simply wrapped
that whole area with local_bh_disable

> 
> From commit 795bb1c00dd3 ("net: bulk free infrastructure for NAPI
> context, use napi_consume_skb") it does appear that technically all
> that is needed is to be called in softirq context.

I saw the same thing, it appears that it isn’t NAPI constrained (like say
napi_alloc_skb, which takes napi arg), but rather softirq and that’s that.

My initial read on all of this was that the napi_ prefix was the original
intent and it morphed out of that over time (to just be softirq protected).
Always happy to learn more if I’ve misread the situation