[PATCH net] r8169: enable the RxOverflow interrupt again

[PATCH net] r8169: enable the RxOverflow interrupt again

[Jure Rebernik]

Commit 6994520a3328 ("r8169: disable interrupt source RxOverflow")
removed RxOverflow (RxDescUnavail in the vendor driver) from the
interrupt mask on the grounds that NAPI is scheduled anyway when the
chip runs out of RX descriptors. On RTL8125B that does not hold.

When the RTL8125B runs out of RX descriptors it sets RxOverflow and
stops receiving. It does not start again when the driver returns
descriptors to it, only when RxOverflow is cleared in IntrStatus.
rtl8169_interrupt() clears only the bits it read on entry. If the ring
fills while NAPI runs with the interrupt mask cleared, RxOverflow is
set after that read and stays set: NAPI empties the ring, unmasks the
interrupts and completes, the stopped receiver raises no RxOK, and
RxOverflow is no longer an interrupt source. Reception stays stopped,
and every frame is counted in rx_missed, until another event such as
TxOK makes the handler clear all status bits.

This was observed on a machine in real use: an RTL8125B (XID 641) on
the PCIe bus of an arm64 Qualcomm QCS8550, running v7.2.6. With UDP
at 800 Mbit/s inbound and nothing sent by the host, reception stopped
for seconds in 13 of 20 30 s runs, losing 5% to 31% of the datagrams,
and a single transmitted packet restarted it. To confirm the cause,
a debugfs dump added to a test build of the driver showed, during each
stop, IntrMask 0x2f, IntrStatus 0x50, NAPI idle and all 256 RX
descriptors owned by the NIC. Writing only RxOverflow to IntrStatus
from user space, through the sysfs PCI resource mapping and with
nothing transmitted, restarted reception in 17 of 17 stops.
Reproducing the stall itself needs no kernel modification.

With this change backported to v7.2.6, 0 of 20 runs stopped and the
worst run lost 0.03%, on two RTL8125B boards. It was build-tested,
not run, on this tree, where the single-vector path RTL8125B uses
is the same as in v7.2. The multi-vector mapping used by RTL8127 is
not changed and was not tested.

Restore the mask of the chips that had RxOverflow before that commit.
Only RTL8125B was tested.

The investigation was done with an AI coding assistant (Claude Code):
it wrote the HIL test scripts, the debugfs instrumentation and the
register tools used above, proposed this change and drafted this
changelog. I reviewed the change and the evidence, and the test
results above come from runs on our hardware.

Fixes: 6994520a3328 ("r8169: disable interrupt source RxOverflow")
Cc: stable@vger.kernel.org
Signed-off-by: Jure Rebernik <jure.rebernik@luxonis.com>
---

Notes:
    This restores RxOverflow for every chip that had it before 6994520a3328,
    but I could only test RTL8125B. If you prefer, I can send a v2 that sets
    it for the RTL8125 family only (rtl_is_8125()).

 drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index cd4ebc654dd0..62709160d354 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -6086,7 +6086,9 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
 		tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
 
 		if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
-			tp->irq_mask |= SYSErr | RxFIFOOver;
+			tp->irq_mask |= SYSErr | RxOverflow | RxFIFOOver;
+		else
+			tp->irq_mask |= RxOverflow;
 	}
 }
 
-- 
2.39.5

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

[Jacob Keller]

On 11/1/2026 2:27 AM, Jure Rebernik wrote:
> Commit 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> removed RxOverflow (RxDescUnavail in the vendor driver) from the
> interrupt mask on the grounds that NAPI is scheduled anyway when the
> chip runs out of RX descriptors. On RTL8125B that does not hold.
> 

This commit says it was "Tested on RTL8168h."

> When the RTL8125B runs out of RX descriptors it sets RxOverflow and
> stops receiving. It does not start again when the driver returns
> descriptors to it, only when RxOverflow is cleared in IntrStatus.
> rtl8169_interrupt() clears only the bits it read on entry. If the ring
> fills while NAPI runs with the interrupt mask cleared, RxOverflow is
> set after that read and stays set: NAPI empties the ring, unmasks the
> interrupts and completes, the stopped receiver raises no RxOK, and
> RxOverflow is no longer an interrupt source. Reception stays stopped,
> and every frame is counted in rx_missed, until another event such as
> TxOK makes the handler clear all status bits.
> 
> This was observed on a machine in real use: an RTL8125B (XID 641) on
> the PCIe bus of an arm64 Qualcomm QCS8550, running v7.2.6. With UDP
> at 800 Mbit/s inbound and nothing sent by the host, reception stopped
> for seconds in 13 of 20 30 s runs, losing 5% to 31% of the datagrams,
> and a single transmitted packet restarted it. To confirm the cause,
> a debugfs dump added to a test build of the driver showed, during each
> stop, IntrMask 0x2f, IntrStatus 0x50, NAPI idle and all 256 RX
> descriptors owned by the NIC. Writing only RxOverflow to IntrStatus
> from user space, through the sysfs PCI resource mapping and with
> nothing transmitted, restarted reception in 17 of 17 stops.
> Reproducing the stall itself needs no kernel modification.
> 
> With this change backported to v7.2.6, 0 of 20 runs stopped and the
> worst run lost 0.03%, on two RTL8125B boards. It was build-tested,
> not run, on this tree, where the single-vector path RTL8125B uses
> is the same as in v7.2. The multi-vector mapping used by RTL8127 is
> not changed and was not tested.
> 
> Restore the mask of the chips that had RxOverflow before that commit.
> Only RTL8125B was tested.
> 
> The investigation was done with an AI coding assistant (Claude Code):
> it wrote the HIL test scripts, the debugfs instrumentation and the
> register tools used above, proposed this change and drafted this
> changelog. I reviewed the change and the evidence, and the test
> results above come from runs on our hardware.
> 
> Fixes: 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jure Rebernik <jure.rebernik@luxonis.com>
> ---
> 
> Notes:
>     This restores RxOverflow for every chip that had it before 6994520a3328,
>     but I could only test RTL8125B. If you prefer, I can send a v2 that sets
>     it for the RTL8125 family only (rtl_is_8125()).
> 

I was thinking that it might make sense to only remove RxOverflow from
RTL8168h, the one that the original change was tested on. However,  the
original change is not motivated as a bug fix, and I think given we
*know* it is harmful to have this bit clear on RTL8125B, I think I agree
with this implementation that sets it for all device types.

If people disagree, then I think v2 should just clear RxOverflow only
for RTL8168h rather than only keeping it on RTL8125 since thats the only
one claimed to be tested in the original commit. To me, it seems
entirely plausible that RTL8168h also has this same behavior and
requires the flag but simply wasted tested with this same use case.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index cd4ebc654dd0..62709160d354 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -6086,7 +6086,9 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
>  		tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
>  
>  		if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
> -			tp->irq_mask |= SYSErr | RxFIFOOver;
> +			tp->irq_mask |= SYSErr | RxOverflow | RxFIFOOver;
> +		else
> +			tp->irq_mask |= RxOverflow;
>  	}
>  }
>  

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

[Jure Rebernik]

On Wed, Nov 04, 2026 at 02:49:53PM -0800, Jacob Keller wrote:
> If people disagree, then I think v2 should just clear RxOverflow only
> for RTL8168h rather than only keeping it on RTL8125 since thats the only
> one claimed to be tested in the original commit. To me, it seems
> entirely plausible that RTL8168h also has this same behavior and
> requires the flag but simply wasted tested with this same use case.
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks for the review, Jacob.

Adding Sunil Goutham to Cc, who reviewed 6994520a3328, the commit this
patch fixes.

-- Jure Rebernik

On Wed, Nov 4, 2026 at 11:50 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
> On 11/1/2026 2:27 AM, Jure Rebernik wrote:
> > Commit 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> > removed RxOverflow (RxDescUnavail in the vendor driver) from the
> > interrupt mask on the grounds that NAPI is scheduled anyway when the
> > chip runs out of RX descriptors. On RTL8125B that does not hold.
> >
>
> This commit says it was "Tested on RTL8168h."
>
> > When the RTL8125B runs out of RX descriptors it sets RxOverflow and
> > stops receiving. It does not start again when the driver returns
> > descriptors to it, only when RxOverflow is cleared in IntrStatus.
> > rtl8169_interrupt() clears only the bits it read on entry. If the ring
> > fills while NAPI runs with the interrupt mask cleared, RxOverflow is
> > set after that read and stays set: NAPI empties the ring, unmasks the
> > interrupts and completes, the stopped receiver raises no RxOK, and
> > RxOverflow is no longer an interrupt source. Reception stays stopped,
> > and every frame is counted in rx_missed, until another event such as
> > TxOK makes the handler clear all status bits.
> >
> > This was observed on a machine in real use: an RTL8125B (XID 641) on
> > the PCIe bus of an arm64 Qualcomm QCS8550, running v7.2.6. With UDP
> > at 800 Mbit/s inbound and nothing sent by the host, reception stopped
> > for seconds in 13 of 20 30 s runs, losing 5% to 31% of the datagrams,
> > and a single transmitted packet restarted it. To confirm the cause,
> > a debugfs dump added to a test build of the driver showed, during each
> > stop, IntrMask 0x2f, IntrStatus 0x50, NAPI idle and all 256 RX
> > descriptors owned by the NIC. Writing only RxOverflow to IntrStatus
> > from user space, through the sysfs PCI resource mapping and with
> > nothing transmitted, restarted reception in 17 of 17 stops.
> > Reproducing the stall itself needs no kernel modification.
> >
> > With this change backported to v7.2.6, 0 of 20 runs stopped and the
> > worst run lost 0.03%, on two RTL8125B boards. It was build-tested,
> > not run, on this tree, where the single-vector path RTL8125B uses
> > is the same as in v7.2. The multi-vector mapping used by RTL8127 is
> > not changed and was not tested.
> >
> > Restore the mask of the chips that had RxOverflow before that commit.
> > Only RTL8125B was tested.
> >
> > The investigation was done with an AI coding assistant (Claude Code):
> > it wrote the HIL test scripts, the debugfs instrumentation and the
> > register tools used above, proposed this change and drafted this
> > changelog. I reviewed the change and the evidence, and the test
> > results above come from runs on our hardware.
> >
> > Fixes: 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jure Rebernik <jure.rebernik@luxonis.com>
> > ---
> >
> > Notes:
> >     This restores RxOverflow for every chip that had it before 6994520a3328,
> >     but I could only test RTL8125B. If you prefer, I can send a v2 that sets
> >     it for the RTL8125 family only (rtl_is_8125()).
> >
>
> I was thinking that it might make sense to only remove RxOverflow from
> RTL8168h, the one that the original change was tested on. However,  the
> original change is not motivated as a bug fix, and I think given we
> *know* it is harmful to have this bit clear on RTL8125B, I think I agree
> with this implementation that sets it for all device types.
>
> If people disagree, then I think v2 should just clear RxOverflow only
> for RTL8168h rather than only keeping it on RTL8125 since thats the only
> one claimed to be tested in the original commit. To me, it seems
> entirely plausible that RTL8168h also has this same behavior and
> requires the flag but simply wasted tested with this same use case.
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>
> >  drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> > index cd4ebc654dd0..62709160d354 100644
> > --- a/drivers/net/ethernet/realtek/r8169_main.c
> > +++ b/drivers/net/ethernet/realtek/r8169_main.c
> > @@ -6086,7 +6086,9 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
> >               tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
> >
> >               if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
> > -                     tp->irq_mask |= SYSErr | RxFIFOOver;
> > +                     tp->irq_mask |= SYSErr | RxOverflow | RxFIFOOver;
> > +             else
> > +                     tp->irq_mask |= RxOverflow;
> >       }
> >  }
> >
>

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

[Jakub Kicinski]

On Sun,  1 Nov 2026 11:27:20 +0100 Jure Rebernik wrote:
> Commit 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> removed RxOverflow (RxDescUnavail in the vendor driver) from the
> interrupt mask on the grounds that NAPI is scheduled anyway when the
> chip runs out of RX descriptors. On RTL8125B that does not hold.
> 
> When the RTL8125B runs out of RX descriptors it sets RxOverflow and
> stops receiving. It does not start again when the driver returns
> descriptors to it, only when RxOverflow is cleared in IntrStatus.
> rtl8169_interrupt() clears only the bits it read on entry. If the ring
> fills while NAPI runs with the interrupt mask cleared, RxOverflow is
> set after that read and stays set: NAPI empties the ring, unmasks the
> interrupts and completes, the stopped receiver raises no RxOK, and
> RxOverflow is no longer an interrupt source. Reception stays stopped,
> and every frame is counted in rx_missed, until another event such as
> TxOK makes the handler clear all status bits.

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?

> This was observed on a machine in real use: an RTL8125B (XID 641) on
> the PCIe bus of an arm64 Qualcomm QCS8550, running v7.2.6. With UDP
> at 800 Mbit/s inbound and nothing sent by the host, reception stopped
> for seconds in 13 of 20 30 s runs, losing 5% to 31% of the datagrams,
> and a single transmitted packet restarted it. To confirm the cause,
> a debugfs dump added to a test build of the driver showed, during each
> stop, IntrMask 0x2f, IntrStatus 0x50, NAPI idle and all 256 RX
> descriptors owned by the NIC. Writing only RxOverflow to IntrStatus
> from user space, through the sysfs PCI resource mapping and with
> nothing transmitted, restarted reception in 17 of 17 stops.
> Reproducing the stall itself needs no kernel modification.
> 
> With this change backported to v7.2.6, 0 of 20 runs stopped and the
> worst run lost 0.03%, on two RTL8125B boards. It was build-tested,
> not run, on this tree, where the single-vector path RTL8125B uses
> is the same as in v7.2. The multi-vector mapping used by RTL8127 is
> not changed and was not tested.
> 
> Restore the mask of the chips that had RxOverflow before that commit.
> Only RTL8125B was tested.
> 
> The investigation was done with an AI coding assistant (Claude Code):
> it wrote the HIL test scripts, the debugfs instrumentation and the
> register tools used above, proposed this change and drafted this
> changelog. I reviewed the change and the evidence, and the test
> results above come from runs on our hardware.
> 
> Fixes: 6994520a3328 ("r8169: disable interrupt source RxOverflow")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jure Rebernik <jure.rebernik@luxonis.com>
> ---
> 
> Notes:
>     This restores RxOverflow for every chip that had it before 6994520a3328,
>     but I could only test RTL8125B. If you prefer, I can send a v2 that sets
>     it for the RTL8125 family only (rtl_is_8125()).
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index cd4ebc654dd0..62709160d354 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -6086,7 +6086,9 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
>  		tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
>  
>  		if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
> -			tp->irq_mask |= SYSErr | RxFIFOOver;
> +			tp->irq_mask |= SYSErr | RxOverflow | RxFIFOOver;
> +		else
> +			tp->irq_mask |= RxOverflow;

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
-- 
pw-bot: cr

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