[PATCH net] r8169: disable default rx interrupt coalescing on RTL8168

[PATCH net] r8169: disable default rx interrupt coalescing on RTL8168

[Heiner Kallweit]

It was reported that re-introducing ASPM, in combination with RX
interrupt coalescing, results in significantly increased packet
latency, see [0]. Disabling ASPM or RX interrupt coalescing fixes
the issue. Therefore change the driver's default to disable RX
interrupt coalescing. Users still have the option to enable RX
coalescing via ethtool.

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925496

Fixes: a99790bf5c7f ("r8169: Reinstate ASPM Support")
Reported-by: Mike Crowe <mac@mcrowe.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c9ee1c8eb..8a8a4f70d 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5460,7 +5460,7 @@ static void rtl_hw_start_8168(struct rtl8169_private *tp)
 	tp->cp_cmd |= PktCntrDisable | INTT_1;
 	RTL_W16(tp, CPlusCmd, tp->cp_cmd);
 
-	RTL_W16(tp, IntrMitigate, 0x5151);
+	RTL_W16(tp, IntrMitigate, 0x5100);
 
 	/* Work around for RxFIFO overflow. */
 	if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
-- 
2.21.0

Re: [PATCH net] r8169: disable default rx interrupt coalescing on RTL8168

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 30 Mar 2019 17:13:24 +0100

> It was reported that re-introducing ASPM, in combination with RX
> interrupt coalescing, results in significantly increased packet
> latency, see [0]. Disabling ASPM or RX interrupt coalescing fixes
> the issue. Therefore change the driver's default to disable RX
> interrupt coalescing. Users still have the option to enable RX
> coalescing via ethtool.
> 
> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925496
> 
> Fixes: a99790bf5c7f ("r8169: Reinstate ASPM Support")
> Reported-by: Mike Crowe <mac@mcrowe.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied and queued up for -stable.

Frankly, I think we were better off with ASPM disabled.  But I know
a lot of people find that unacceptable.

This chip has some many quirks/warts when ASPM is enabled, look how
long it took us to A) get ASPM enabled reasonably and B) discover
this bug.

Re: [PATCH net] r8169: disable default rx interrupt coalescing on RTL8168

[Heiner Kallweit]

On 30.03.2019 19:06, David Miller wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Sat, 30 Mar 2019 17:13:24 +0100
> 
>> It was reported that re-introducing ASPM, in combination with RX
>> interrupt coalescing, results in significantly increased packet
>> latency, see [0]. Disabling ASPM or RX interrupt coalescing fixes
>> the issue. Therefore change the driver's default to disable RX
>> interrupt coalescing. Users still have the option to enable RX
>> coalescing via ethtool.
>>
>> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925496
>>
>> Fixes: a99790bf5c7f ("r8169: Reinstate ASPM Support")
>> Reported-by: Mike Crowe <mac@mcrowe.com>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Applied and queued up for -stable.
> 
> Frankly, I think we were better off with ASPM disabled.  But I know
> a lot of people find that unacceptable.
> 
Yes, I can hear the notebook owners scream already, and complain about
reduced battery runtime. It would help to have an opt-in option for
ASPM. But a module parameter for it is discouraged, and I see no other
acceptable way to control ASPM on the chip side. I could misuse some
ethtool callback or add a new tunable. But controlling a PCI feature
through ethtool doesn't sound too nice. Maybe I could add a sysfs
attribute.

> This chip has some many quirks/warts when ASPM is enabled, look how
> long it took us to A) get ASPM enabled reasonably and B) discover
> this bug.
> 

Re: [PATCH net] r8169: disable default rx interrupt coalescing on RTL8168

[Mike Crowe]

On Saturday 30 March 2019 at 17:13:24 +0100, Heiner Kallweit wrote:
> It was reported that re-introducing ASPM, in combination with RX
> interrupt coalescing, results in significantly increased packet
> latency, see [0]. Disabling ASPM or RX interrupt coalescing fixes
> the issue. Therefore change the driver's default to disable RX
> interrupt coalescing. Users still have the option to enable RX
> coalescing via ethtool.
> 
> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925496
> 
> Fixes: a99790bf5c7f ("r8169: Reinstate ASPM Support")
> Reported-by: Mike Crowe <mac@mcrowe.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index c9ee1c8eb..8a8a4f70d 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -5460,7 +5460,7 @@ static void rtl_hw_start_8168(struct rtl8169_private *tp)
>  	tp->cp_cmd |= PktCntrDisable | INTT_1;
>  	RTL_W16(tp, CPlusCmd, tp->cp_cmd);
>  
> -	RTL_W16(tp, IntrMitigate, 0x5151);
> +	RTL_W16(tp, IntrMitigate, 0x5100);
>  
>  	/* Work around for RxFIFO overflow. */
>  	if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
> -- 
> 2.21.0

Tested-by: Mike Crowe <mac@mcrowe.com>

I configured my ASUS BIOS to reenable "EPA Power Saving Mode" and tried
running the current state of linux-5.0.y with the above patch applied on
top.

As expected, packet latency was good immediately after booting. Running
"ethtool -c" showed rx-usecs=0, rx-frames=1. Also as expected, manually
increasing these to rx-usecs=200, rx-frames=4 caused the latency problem to
return.

Thanks for finding a way to address this problem without causing both users
of ageing motherboards and modern notebooks to scream.

Mike.