netdev - Re: [PATCH net-next 1/3] net: skb: plumb napi state thru skb freeing paths

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: <20230412212059.1b7a3364@kernel.org>
Date:   Wed, 12 Apr 2023 21:20:59 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jesper Dangaard Brouer <jbrouer@...hat.com>
Cc:     davem@...emloft.net, brouer@...hat.com, netdev@...r.kernel.org,
        edumazet@...gle.com, pabeni@...hat.com, hawk@...nel.org,
        ilias.apalodimas@...aro.org, linyunsheng@...wei.com,
        Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [PATCH net-next 1/3] net: skb: plumb napi state thru skb
 freeing paths

On Wed, 12 Apr 2023 11:29:08 +0200 Jesper Dangaard Brouer wrote:
> On 11/04/2023 22.17, Jakub Kicinski wrote:
> > We maintain a NAPI-local cache of skbs which is fed by napi_consume_skb().
> > Going forward we will also try to cache head and data pages.
> > Plumb the "are we in a normal NAPI context" information thru
> > deeper into the freeing path, up to skb_release_data() and
> > skb_free_head()/skb_pp_recycle().
> > 
> > Use "bool in_normal_napi" rather than bare "int budget",  
> 
> The code was changed to "napi_safe", the desc should reflect this ;-)

Ah, sed would have been a better idea after all :)

> > the further we get from NAPI the more confusing the budget
> > argument may seem (particularly whether 0 or MAX is the
> > correct value to pass in when not in NAPI).  
> 
> I do like the code cleanup.
> It is worth explaining/mentioning that where budget==0 comes from?
> 
> (Cc. Alex, please correct me.)
> My understanding is that this is caused by netconsole/netpoll (see
> net/core/netpoll.c func poll_one_napi()), which is a kernel (net)console
> debugging facility sending UDP packets via using only TX side of
> napi_poll.  Thus, we are really trying to protect against doing these
> recycle tricks for when netpoll/netconcole is running (which I guess
> makes sense as we are likely printing/sending an OOPS msg over UDP).

Yup, that's correct. I'll add a sentence or two to that effect.