Re: [RFC PATCH] net: stmmac: Fix the problem about interrupt storm

[Romain Gantois <romain.gantois@bootlin.com>]

Hello,

On dimanche 3 novembre 2024 20:00:54 heure normale d’Europe centrale Avi 
Fishman wrote:
> Hi all,
> 
...
> >  Yes. It could also happen between the dev_open() and
> > 
> > clear_bit(STMMAC_DOWN) calls.
> > Although we did not reproduce this scenario, it should have happened
> > if we had increased
> > the number of test samples. In addition, I found that other people had
> > similar problems before.
> > The link is:
> > https://lore.kernel.org/all/20210208140820.10410-11-Sergey.Semin@baikalele
> > ctronics.ru/> 
> > > Moreover, it seems strange to me that stmmac_interrupt()
> > > unconditionnally
> > > ignores interrupts when the driver is in STMMAC_DOWN state. This seems
> > > like
> > > dangerous behaviour, since it could cause IRQ storm issues whenever
> > > something in the driver sets this state. I'm not too familiar with the
> > > interrupt handling in this driver, but maybe stmmac_interrupt() could
> > > clear interrupts unconditionnally in the STMMAC_DOWN state?
> > 
> > Clear interrupts unconditionally in the STMMAC_DOWN state directly
> > certainly won't cause this problem.
> > This may be too rough, maybe this design has other considerations.
> 
> But then after the dev_open() you might miss interrupt, no?

Indeed, but in any case, unconditionally returning from an IRQ handler without 
clearing any interrupt flags seems like very strange behavior to me.

Disabling and reenabling interrupts as you suggested does seem like a
good solution for this particular scenario, but it doesn't solve the more
general issue of the dangerous way stmmac_interrupt handles this.

Maybe the setting and clearing of this STMMAC_DOWN bit should
be wrapped in some kind of handler which also disables all interrupts?

Best Regards

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com