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.
