Re: R8169: Network lockups in 4.18.{8,9,10} (and 4.19 dev)

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
To: Chris Clayton <chris2553@googlemail.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Azat Khuzhin <a3at.mail@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Realtek linux nic maintainers <nic_swsd@realtek.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: R8169: Network lockups in 4.18.{8,9,10} (and 4.19 dev)
Date: Thu, 11 Oct 2018 14:23:36 +0200	[thread overview]
Message-ID: <6f3f206c-7226-7182-ec81-05547cf39239@maciej.szmigiero.name> (raw)
In-Reply-To: <d14f04cd-329d-cea9-14e1-d90ad629df74@googlemail.com>

On 11.10.2018 10:24, Chris Clayton wrote:
> On 11/10/2018 01:12, Maciej S. Szmigiero wrote:
>> On 11.10.2018 00:49, Chris Clayton wrote:
>>>> Now, knowing the "right" value you can experiment with what rtl_init_rxcfg()
>>>> writes (under the "default:" label for your NIC model).
>>>>
>>>
>>> This might be more interesting. Through a combination of viewing the output from pr_notice() and the output from
>>> "ethtool -d", I can see RxConfig with the following values
>>>
>>> 	During boot:	0x00028700
>>> 	Before suspend:	0x0002870e
>>> 	During resume:	0x00024000
>>> 	Post resume:	0x0002870e
>>>
>>> As I did with 4.18.10 early on in the process, I removed the call to rtl_init_rxcfg() from rtl_hw_start() and rebuilt,
>>> installed and rebooted. Now I see the following values:
>>>
>>> 	During boot:	0x00028700
>>> 	Before suspend:	0x0002870e
>>> 	During resume:	0x00024000
>>> 	Post resume:	0x0002400e
>>>
>>
>> Now we can finally see some difference...
>> Besides missing RX128_INT_EN (bit 15 or 0x8000) and RX_DMA_BURST
>> (bits 8-10 or 0x700) - that rtl_init_rxcfg() would normally set so this
>> is kind of expected - one can see that the working configuration
>> post-resume has bit 14 (or 0x4000) set, too.
>>
>> This bit is described in the driver as RX_MULTI_EN ("8111c only") and is
>> set by rtl_init_rxcfg() for example for RTL_GIGA_MAC_VER_35.
>>
>> RTL_GIGA_MAC_VER_35 is described in the driver as being in the same
>> family as your RTL_GIGA_MAC_VER_38, so can you please try the following
>> change:
>> --- r8169.c
>> +++ r8169.c
>> @@ -4271,6 +4271,7 @@ static void rtl_init_rxcfg(struct rtl816
>>  	case RTL_GIGA_MAC_VER_18 ... RTL_GIGA_MAC_VER_24:
>>  	case RTL_GIGA_MAC_VER_34:
>>  	case RTL_GIGA_MAC_VER_35:
>> +	case RTL_GIGA_MAC_VER_38:
>>  		RTL_W32(tp, RxConfig, RX128_INT_EN | RX_MULTI_EN | RX_DMA_BURST);
>>  		break;
>>  	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_51:
>>
>> This will add RX_MULTI_EN also for your chip model (you need to add back
>> the call to rtl_init_rxcfg() to rtl_hw_start(), naturally).
>>
> 
> That's done the trick. With the above change applied, my network runs running fine after a suspend/resume cycle and the
> ping times are back in the 14-15ms range.

Nice!

I will submit a patch, it would be great if you could test it and then
add a "Tested-by:" tag.

> Chris

Maciej