netdev - Re: [RFC net-next 1/2] page_pool: allow caching from safely localized NAPI

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230403101219.59a83043@kernel.org>
Date:   Mon, 3 Apr 2023 10:12:19 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, hawk@...nel.org
Subject: Re: [RFC net-next 1/2] page_pool: allow caching from safely
 localized NAPI

On Mon, 3 Apr 2023 18:30:55 +0300 Ilias Apalodimas wrote:
> > Meaning in page_pool_return_skb_page() or all the way from
> > napi_consume_skb()? The former does indeed sounds like a good idea!  
> 
> page_pool_return_skb_page() (and maybe page_pool_put_full_page()).
> FWIW we completely agree on napi_consume_skb().  We are trying to keep
> page_pool and the net layer as disjoint as possible.  The only point
> we 'pollute' networking code is the recycle bit checking and we'd
> prefer keeping it like that

Ack, OTOH plumbing thru the budget argument within netdev code should
not be a major refactoring. So maybe I should do that after all.

Otherwise we have two different conditions - netdev only recycles skbs
based on the NAPI budget != 0, but page pool will assume that
in_softirq() && !in_hardirq() is always safe.

The latter is safe, I think, unless someone adds a print half way thru
the cache update... but then it's also safe in NAPI skb recycling,
so napi_consume_skb() should stop taking the budget and just look
at preempt flags...

To make the correctness obvious, for now, I think I will refactor 
the netdev code to pass a "in NAPI poll" bool to
page_pool_return_skb_page(), and add a WARN_ON(!softirq || hardirq).

Let's see how the code ends up looking, I'll send it as RFCv2 rather
than PATCH to make it clear I'm not sure it's okay with you :)