Re: [PATCH net] r8169: enable the RxOverflow interrupt again

[Jure Rebernik]

On Fri, 6 Nov 2026 13:18:28 -0800 Jakub Kicinski wrote:
> Not really following the explanation, TBH.
> In general "ring empty" IRQs are discouraged because the host is likely
> overloaded, so having the NIC force the NAPI is not helpful. But sounds
> like you say there's some sort of a race?

Yes, sorry, the description was not clear. The interrupt is not there to
force NAPI. On the RTL8125B the receiver stays stopped after it runs out
of descriptors until RxOverflow is cleared in IntrStatus, even after
NAPI has emptied the ring. The sequence, as I understand it:

 1. The ring fills. The chip completes the last frames (RxOK), then sets
    RxOverflow and stops receiving.
 2. rtl8169_interrupt() read IntrStatus before RxOverflow was set, so it
    clears only RxOK.
 3. NAPI empties the ring and unmasks the interrupts.
 4. The stopped receiver raises no new RxOK, and RxOverflow is not an
    interrupt source, so nothing clears it. Reception stays stopped
    until a TX completion makes the handler clear all status bits.

What I measured is the stopped state (IntrMask 0x2f, IntrStatus 0x50,
NAPI idle, all descriptors owned by the NIC), and that writing only
RxOverflow to IntrStatus restarts reception with nothing transmitted.
The order of steps 1 and 2 is my inference.

So the fix only needs RxOverflow cleared once the ring is empty. I see
two ways, both tested on an RTL8125B with v7.2.6:

a) v1 with your suggestion, RxOverflow in the unconditional init of
   tp->irq_mask, so the handler clears it.

b) No "ring empty" interrupt. Clear RxOverflow in rtl8169_poll() right
   before unmasking:

        if (work_done < budget && napi_complete_done(napi, work_done)) {
                rtl_ack_events(tp, RxOverflow);
                rtl_irq_enable(tp);
        }

For each, a test build switched between the old and the new behaviour
on the same boot, in blocks of 5 to 10 runs of 30 s with UDP at
800 Mbit/s inbound, about 2.14 million datagrams per run. Totals over
all runs of each kind:

              runs that lost    datagrams     datagrams
              >1000 datagrams   sent, total   lost, total
   (a) old    5 of 76           162865429      411511 (0.25%)
       new    0 of 76           162861980         489 (0.0003%)
   (b) old    9 of 75           160722174     1041366 (0.65%)
       new    0 of 75           160721463         846 (0.0005%)

The small loss left with (a) and (b), 489 and 846 datagrams in total and
at most 61 and 184 in a single run, matches the NIC's rx_missed counter
over the same runs, 492 and 846. So the NIC dropped those frames, while
the RX ring was briefly full. I used the upstream ring size (256
entries) on purpose for these tests. Our tree also raises the ring to
1024 entries (not part of this patch), and with that and the v1 change
the same test lost no datagrams in 20 runs on two boards.

Which one would you prefer? I'll send v2 accordingly.

> LLMs point out that you're adding the same bit on both sides so just
> add it in the unconditional init of tp->irq_mask above

Agreed, v2 of (a) does that.

-- Jure Rebernik