[PATCH net v2] r8169: avoid unsolicited interrupts

[PATCH net v2] r8169: avoid unsolicited interrupts

[Heiner Kallweit]

It was reported that after resume from suspend a PCI error is logged
and connectivity is broken. Error message is:
PCI error (cmd = 0x0407, status_errs = 0x0000)
The message seems to be a red herring as none of the error bits is set,
and the PCI command register value also is normal. Exception handling
for a PCI error includes a chip reset what apparently brakes connectivity
here. The interrupt status bit triggering the PCI error handling isn't
actually used on PCIe chip versions, so it's not clear why this bit is
set by the chip. Fix this by ignoring this bit on PCIe chip versions.

Fixes: 0e4851502f84 ("r8169: merge with version 8.001.00 of Realtek's r8168 driver")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219388
Tested-by: Atlas Yu <atlas.yu@canonical.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- choose alternative approach because v1 brakes connection under heavy load
---
 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 4166f1ab8..79e7b223b 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4749,7 +4749,9 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	if ((status & 0xffff) == 0xffff || !(status & tp->irq_mask))
 		return IRQ_NONE;
 
-	if (unlikely(status & SYSErr)) {
+	/* At least RTL8168fp may unexpectedly set the SYSErr bit */
+	if (unlikely(status & SYSErr &&
+	    tp->mac_version <= RTL_GIGA_MAC_VER_06)) {
 		rtl8169_pcierr_interrupt(tp->dev);
 		goto out;
 	}
-- 
2.47.0

Re: [PATCH net v2] r8169: avoid unsolicited interrupts

[Simon Horman]

On Fri, Oct 18, 2024 at 11:08:16AM +0200, Heiner Kallweit wrote:
> It was reported that after resume from suspend a PCI error is logged
> and connectivity is broken. Error message is:
> PCI error (cmd = 0x0407, status_errs = 0x0000)
> The message seems to be a red herring as none of the error bits is set,
> and the PCI command register value also is normal. Exception handling
> for a PCI error includes a chip reset what apparently brakes connectivity
> here. The interrupt status bit triggering the PCI error handling isn't
> actually used on PCIe chip versions, so it's not clear why this bit is
> set by the chip. Fix this by ignoring this bit on PCIe chip versions.
> 
> Fixes: 0e4851502f84 ("r8169: merge with version 8.001.00 of Realtek's r8168 driver")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219388
> Tested-by: Atlas Yu <atlas.yu@canonical.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2] r8169: avoid unsolicited interrupts

[Francois Romieu]

Heiner Kallweit <hkallweit1@gmail.com> :
|...]
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 4166f1ab8..79e7b223b 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4749,7 +4749,9 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
>  	if ((status & 0xffff) == 0xffff || !(status & tp->irq_mask))
>  		return IRQ_NONE;
>  
> -	if (unlikely(status & SYSErr)) {
> +	/* At least RTL8168fp may unexpectedly set the SYSErr bit */
> +	if (unlikely(status & SYSErr &&
> +	    tp->mac_version <= RTL_GIGA_MAC_VER_06)) {
>  		rtl8169_pcierr_interrupt(tp->dev);
>  		goto out;
>  	}

Some paleolithic references:
a27993f3d9daca0dffa26577a83822db99c952e2
d03902b8864d7814c938f67befade5a3bba68708

First occurences were related to 32/64 bits PCI systems somewhere around
2004~2006 (IBM PPC, who cares now ?).

It also appeared sometimes when the receive frame length check misbehaved.

It mostly was a plain PCI devices behavior. I am not too sure about 8168b
but I don't remember anything above (including) 8168c needing it.

-- 
Ueimor

Re: [PATCH net v2] r8169: avoid unsolicited interrupts

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 18 Oct 2024 11:08:16 +0200 you wrote:
> It was reported that after resume from suspend a PCI error is logged
> and connectivity is broken. Error message is:
> PCI error (cmd = 0x0407, status_errs = 0x0000)
> The message seems to be a red herring as none of the error bits is set,
> and the PCI command register value also is normal. Exception handling
> for a PCI error includes a chip reset what apparently brakes connectivity
> here. The interrupt status bit triggering the PCI error handling isn't
> actually used on PCIe chip versions, so it's not clear why this bit is
> set by the chip. Fix this by ignoring this bit on PCIe chip versions.
> 
> [...]

Here is the summary with links:
  - [net,v2] r8169: avoid unsolicited interrupts
    https://git.kernel.org/netdev/net/c/10ce0db78700

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html