Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

[Stephen Hemminger]

Begin forwarded message:

Date: Fri, 22 Feb 2013 08:41:04 -0800
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd


https://bugzilla.kernel.org/show_bug.cgi?id=54231





--- Comment #1 from Tomi Orava <tomimo@ncircle.nullnet.fi>  2013-02-22 16:40:49 ---
Although the r8169 has been working just fine on 3.4.31 for the past 5 days, it
seems that I missed the second DMA Burst setting in the previous patch that
should get fixed as well:

--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -77,6 +77,7 @@
 static const int multicast_filter_limit = 32;

 #define MAX_READ_REQUEST_SHIFT    12
+#define TX_DMA_BURST_512        5       /* Maximum PCI burst, limited to 512
*/
 #define TX_DMA_BURST    7    /* Maximum PCI burst, '7' is unlimited */
 #define InterFrameGap    0x03    /* 3 means InterFrameGap = the shortest one
*/

@@ -4406,8 +4407,14 @@ static void rtl_set_rx_tx_config_registers(struct
rtl8169_private *tp)
     void __iomem *ioaddr = tp->mmio_addr;

     /* Set DMA burst size and Interframe Gap Time */
-    RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
-        (InterFrameGap << TxInterFrameGapShift));
+
+    if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
+        RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
+            (InterFrameGap << TxInterFrameGapShift));
+    } else {
+        RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
+            (InterFrameGap << TxInterFrameGapShift));
+    }
 }

 static void rtl_hw_start(struct net_device *dev)
@@ -5148,8 +5155,13 @@ static void rtl_hw_start_8168(struct net_device *dev)

     rtl_set_rx_mode(dev);

-    RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
-        (InterFrameGap << TxInterFrameGapShift));
+    if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
+        RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
+            (InterFrameGap << TxInterFrameGapShift));
+    } else {
+        RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
+            (InterFrameGap << TxInterFrameGapShift));
+    }

     RTL_R8(IntrMask);

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Re: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

[Francois Romieu]

Stephen Hemminger <stephen@networkplumber.org> :
[...]
> https://bugzilla.kernel.org/show_bug.cgi?id=54231
> 
> --- Comment #1 from Tomi Orava <tomimo@ncircle.nullnet.fi>  2013-02-22 16:40:49 ---
> Although the r8169 has been working just fine on 3.4.31 for the past 5 days, it
> seems that I missed the second DMA Burst setting in the previous patch that
> should get fixed as well:
[...]
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4406,8 +4407,14 @@ static void rtl_set_rx_tx_config_registers(struct
> rtl8169_private *tp)
>      void __iomem *ioaddr = tp->mmio_addr;
> 
>      /* Set DMA burst size and Interframe Gap Time */
> -    RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> -        (InterFrameGap << TxInterFrameGapShift));
> +
> +    if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
> +        RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
> +            (InterFrameGap << TxInterFrameGapShift));
> +    } else {
> +        RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> +            (InterFrameGap << TxInterFrameGapShift));
> +    }
>  }

This one should not be used in the (RTL_CFG_1) 8168 / RTL_GIGA_MAC_VER_11
path.

Tomi, what does lspci say about your 8168b device ?

>  static void rtl_hw_start(struct net_device *dev)
> @@ -5148,8 +5155,13 @@ static void rtl_hw_start_8168(struct net_device *dev)
> 
>      rtl_set_rx_mode(dev);
> 
> -    RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> -        (InterFrameGap << TxInterFrameGapShift));
> +    if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
> +        RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
> +            (InterFrameGap << TxInterFrameGapShift));
> +    } else {
> +        RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> +            (InterFrameGap << TxInterFrameGapShift));
> +    }

	RTL_W32(TxConfig, (InterFrameGap << TxInterFrameGapShift) |
		(((tp->mac_version == RTL_GIGA_MAC_VER_11) ?
		 TX_DMA_BURST_512 : TX_DMA_BURST) << TxDMAShift));

-- 
Ueimor

Re: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

[Tomi Orava]

On 02/23/2013 01:09 AM, Francois Romieu wrote:
> Stephen Hemminger <stephen@networkplumber.org> :
> [...]
>> https://bugzilla.kernel.org/show_bug.cgi?id=54231
> This one should not be used in the (RTL_CFG_1) 8168 / RTL_GIGA_MAC_VER_11
> path.

I started re-testing after your comment and figured out that the
real problem seems to lie somehow with jumbo frames. Ie. the DMA burst
changes do not actually prevent the hangs at all in my case. The
catch here, that I missed previously, was that for some interesting
reason the NIC will fail in a couple of minutes with a suitable traffic
if the jumbo frames (mtu 4000) have been enabled from the start.
However, if I enable the jumbo frames manually after the system
has already started up, there are no stability issues related to network.

I ran my tests again with unmodified 3.7.9 kernel and got the
same results as with the 3.4.31 version.

> 
> Tomi, what does lspci say about your 8168b device ?

The NIC information is:

03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI
Express Gigabit Ethernet controller (rev 01)

03:00.0 0200: 10ec:8168 (rev 01)
    Subsystem: 1043:8385
    Flags: bus master, fast devsel, latency 0, IRQ 40
    I/O ports at e800 [size=256]
    Memory at febff000 (64-bit, non-prefetchable) [size=4K]
    Expansion ROM at febc0000 [disabled] [size=128K]
    Capabilities: [40] Power Management version 2
    Capabilities: [48] Vital Product Data
    Capabilities: [50] MSI: Enable+ Count=1/2 Maskable- 64bit+
    Capabilities: [60] Express Endpoint, MSI 00
    Capabilities: [84] Vendor Specific Information: Len=4c <?>
    Kernel driver in use: r8169

Tomi

Re: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

[Francois Romieu]

Tomi Orava <tomimo@ncircle.nullnet.fi> :
> On 02/23/2013 01:09 AM, Francois Romieu wrote:
[...]
> I started re-testing after your comment and figured out that the
> real problem seems to lie somehow with jumbo frames. Ie. the DMA burst
> changes do not actually prevent the hangs at all in my case. The
> catch here, that I missed previously, was that for some interesting
> reason the NIC will fail in a couple of minutes with a suitable traffic
> if the jumbo frames (mtu 4000) have been enabled from the start.
> However, if I enable the jumbo frames manually after the system
> has already started up, there are no stability issues related to network.

If you mean that 'ip link set dev ... mtu 4000; ip link set dev ... up'
fails whereas 'ip link set dev ... up; ip link set dev ... mtu 4000'
works, the patch below is a candidate:

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 8900398..af99498 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4766,7 +4766,7 @@ static void rtl_hw_start_8168bb(struct rtl8169_private *tp)
 	RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) & ~R8168_CPCMD_QUIRK_MASK);
 
 	rtl_tx_performance_tweak(pdev,
-		(0x5 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
+		(0x2 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
 }
 
 static void rtl_hw_start_8168bef(struct rtl8169_private *tp)
---

[...]
> > Tomi, what does lspci say about your 8168b device ?
> 
> The NIC information is:
> 
> 03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI
> Express Gigabit Ethernet controller (rev 01)
> 
> 03:00.0 0200: 10ec:8168 (rev 01)

Ok, so the first hunk of the patch that Stephen forwarded would not make
any difference for your chipset.

-- 
Ueimor

Re: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd

[Tomi Orava]

On 02/25/2013 12:04 AM, Francois Romieu wrote:
> Tomi Orava <tomimo@ncircle.nullnet.fi> :
>> On 02/23/2013 01:09 AM, Francois Romieu wrote:
> [...]
>> I started re-testing after your comment and figured out that the
>> real problem seems to lie somehow with jumbo frames. Ie. the DMA burst
>> changes do not actually prevent the hangs at all in my case. The
>> catch here, that I missed previously, was that for some interesting
>> reason the NIC will fail in a couple of minutes with a suitable traffic
>> if the jumbo frames (mtu 4000) have been enabled from the start.
>> However, if I enable the jumbo frames manually after the system
>> has already started up, there are no stability issues related to network.
> 
> If you mean that 'ip link set dev ... mtu 4000; ip link set dev ... up'
> fails whereas 'ip link set dev ... up; ip link set dev ... mtu 4000'
> works, the patch below is a candidate:

Yes, this fits exactly my problem. I tested this change on 3.7.9 and
on 3.4.33 and I had no network hangs anymore.

Thanks!

Tomi

> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 8900398..af99498 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4766,7 +4766,7 @@ static void rtl_hw_start_8168bb(struct rtl8169_private *tp)
>  	RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) & ~R8168_CPCMD_QUIRK_MASK);
>  
>  	rtl_tx_performance_tweak(pdev,
> -		(0x5 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
> +		(0x2 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
>  }
>  
>  static void rtl_hw_start_8168bef(struct rtl8169_private *tp)
> ---
> 
> [...]
>>> Tomi, what does lspci say about your 8168b device ?
>>
>> The NIC information is:
>>
>> 03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI
>> Express Gigabit Ethernet controller (rev 01)
>>
>> 03:00.0 0200: 10ec:8168 (rev 01)
> 
> Ok, so the first hunk of the patch that Stephen forwarded would not make
> any difference for your chipset.
>