Re: [PATCH net-next v7 1/5] net: move ARFS rmap management to core

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 10 Feb 2025 08:04:43 -0700 Ahmed Zaki wrote:
> On 2025-02-06 7:29 p.m., Jakub Kicinski wrote:
> 
> > Speaking of which, why do the auto-removal in napi_disable()
> > rather than netif_napi_del() ? We don't reinstall on napi_enable()
> > and doing a disable() + enable() is fairly common during driver
> > reconfig.
> >   
> 
> The patch does not re-install the notifiers in napi_add either, they are 
> installed in set_irq() :
> 
> napi_add_config()  -> napi_set_irq()  -> napi_enable()
> 
> so napi_disable or napi_del seemed both OK to me.
> 
> However, I moved notifier auto-removal to npi_del() and did some testing 
> on ice but it seems the driver does not delete napi on "ip link down" 
> and that generates warnings on free_irq(). It only disables the napis.
> 
> So is this a bug? Do we need to ask drivers to disable __and__ delete 
> napis before freeing the IRQs?
> 
> If not, then we have to keep notifier aut-removal in napi_diasable().

If the driver releases the IRQ but keeps the NAPI instance I would have
expected it to call:

	napi_set_irq(napi, -1);

before freeing the IRQ. Otherwise the NAPI instance will "point" to 
a freed IRQ.