netdev - Re: [PATCH net-next v5 1/6] net: move ARFS rmap management to core

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: <20250114173805.23d254a9@kernel.org>
Date: Tue, 14 Jan 2025 17:38:05 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Ahmed Zaki <ahmed.zaki@...el.com>
Cc: <netdev@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>,
 <andrew+netdev@...n.ch>, <edumazet@...gle.com>, <horms@...nel.org>,
 <pabeni@...hat.com>, <davem@...emloft.net>, <michael.chan@...adcom.com>,
 <tariqt@...dia.com>, <anthony.l.nguyen@...el.com>,
 <przemyslaw.kitszel@...el.com>, <jdamato@...tly.com>, <shayd@...dia.com>,
 <akpm@...ux-foundation.org>, <shayagr@...zon.com>,
 <kalesh-anakkur.purayil@...adcom.com>, <pavan.chebbi@...adcom.com>,
 <yury.norov@...il.com>, <darinzon@...zon.com>
Subject: Re: [PATCH net-next v5 1/6] net: move ARFS rmap management to core

On Tue, 14 Jan 2025 18:00:30 -0700 Ahmed Zaki wrote:
> > Similarly netif_napi_set_irq() may get called with -1 to clear
> > the IRQ number, which you currently treat at a real IRQ id, AFAICT.  
> 
> correct there is no handling for irq = -1. So netif_napi_set_irq() needs 
> to add the irq to the rmap only if it is > 0.
> 
> I need to clarify expectation of netif_napi_set_irq() because I only see 
> it called with irq = -1 in napi_add_weight. But you say it can be called 
> with irq = -1 to "clear" the IRQ.

I _think_ that's what Amritha had in mind. For queue <> NAPI linking
similarly we are expected to call the same helper with a NULL param.

> Does this mean that, if irq = -1, we need to "delete" the irq from rmap 
> if a valid irq already existed (which means this can happen as an 
> alternative to napi_del()/napi_diable())? or just skip adding irq to 
> rmap is enough?

I'm afraid we need both. Most drivers today simply never clear the IRQ,
they will just delete the NAPI and kfree() its memory. So we need to
"catch" NAPIs with IRQs assigned getting deleted and clean up the IRQ.

In the future some drivers may explicitly call the set with -1,
especially now that the IRQ has more implications than just getting
reported via netlink. We need to support that, too.

And for a good measure we should also throw in a warning if a driver
tries to set the IRQ but the IRQ is already set in the NAPI (not -1).