Re: [PATCH] net: enetc: fix sirq-storm by clearing IDR registers

[Zefir Kurtisi <zefir.kurtisi@gmail.com>]

On 2/27/26 12:57, Vladimir Oltean wrote:
> On Fri, Feb 27, 2026 at 12:28:17PM +0100, Zefir Kurtisi wrote:
>> While I was debugging the issue I extended enetc with features that still
>> might be useful; the first change was to selectively process only the
>> pending BDs instead of looping over all available, saving a significant
>> number of unneeded calls to enetc_clean_rx/tx_ring(); the second adds a
>> fail-safe mechanism for potentially other issues taking the same code path
>> based on the fact that now a BD is only processed when its IDR is active,
>> which means there must be BDs available to process.
> 
> We'll perform our own investigation of what is happening when
> transmitting short frames. It will take some time.
> 
> But I'm not sure I understand what you mean about processing a BD ring
> when its interrupt is active. NAPI (Documentation/networking/napi.rst)
> works on the basic premise that a single hardirq is sufficient to process
> a very large batch of frames, and keeping hardirqs enabled is detrimential
> to performance. Instead, NAPI (re-)schedules softirqs until there are no
> further frames to process, and only then re-enables the hardirq.
> 
> Perhaps I didn't understand very well what you mean, but it sounds like
> you want to circumvent NAPI, essentially. When enetc_poll() is called,
> you seem to assume it's the first time it's been called after enetc_msix()
> has called napi_schedule(). But it's not. It can also reschedule itself,
> when the work done is equal to the NAPI budget (complete==false), with
> the hardirq _still_ masked.
> 
> The correct NAPI behaviour _is_ for the hardirq to be masked for a very
> long while, when subject to continuous streams of traffic.

The change proposed does in no way change how and when interrupts are 
re-enabled or NAPI is re-scheduled. What it does is:

assume there are completed RX frames, but no completed TX frames:
SIRXIDR=0x0001, SITXIDR=0x0000

enetc_poll today does:
* enetc_clean_tx_ring(0) -> nop
  * read TB0CIR and find out it did not progress
* enetc_clean_tx_ring(2) -> nop
* enetc_clean_tx_ring(4) -> nop
* enetc_clean_tx_ring(6) -> nop
* enetc_clean_rx_ring(0)

Proposal instead: as long as SITXIDR==0x0000, limit to
* enetc_clean_rx_ring(0)

IRQ remains masked until NAPI is completed, behaviour is same as before. 
If there was an RX burst and NAPI is re-scheduled multiple times and 
during that time a TX BD completes, the SITXIDR would get the related 
bit set and TX BD would be processed in enetc_poll - all before the 
interrupt is re-armed.


But if the improvement is not obvious, it maybe does not justify adding 
the change - in the end it spares some function calls and register 
reads, which might not be that significant.

Cheers,
Zefir