[PATCH] r8169: fix packet truncation after S4 resume on RTL8168H/RTL8111H

lilinmao posted 1 patch 2 months, 1 week ago
drivers/net/ethernet/realtek/r8169_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] r8169: fix packet truncation after S4 resume on RTL8168H/RTL8111H
Posted by lilinmao 2 months, 1 week ago
From: Linmao Li <lilinmao@kylinos.cn>

After resume from S4 (hibernate), RTL8168H/RTL8111H truncates incoming
packets. Packet captures show messages like "IP truncated-ip - 146 bytes
missing!".

The issue is caused by RxConfig not being properly re-initialized after
resume. Re-initializing the RxConfig register before the chip
re-initialization sequence avoids the truncation and restores correct
packet reception.

This follows the same pattern as commit ef9da46ddef0 ("r8169: fix data
corruption issue on RTL8402").

Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9c601f271c02..4b0ac73565ea 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4994,8 +4994,9 @@ static int rtl8169_resume(struct device *device)
 	if (!device_may_wakeup(tp_to_dev(tp)))
 		clk_prepare_enable(tp->clk);
 
-	/* Reportedly at least Asus X453MA truncates packets otherwise */
-	if (tp->mac_version == RTL_GIGA_MAC_VER_37)
+	/* Some chip versions may truncate packets without this initialization */
+	if (tp->mac_version == RTL_GIGA_MAC_VER_37 ||
+	    tp->mac_version == RTL_GIGA_MAC_VER_46)
 		rtl_init_rxcfg(tp);
 
 	return rtl8169_runtime_resume(device);
-- 
2.25.1
Re: [PATCH] r8169: fix packet truncation after S4 resume on RTL8168H/RTL8111H
Posted by Paolo Abeni 2 months, 1 week ago
On 10/6/25 5:49 AM, lilinmao wrote:
> From: Linmao Li <lilinmao@kylinos.cn>

From: tag is not needed when the submitting email address matches the SoB

> 
> After resume from S4 (hibernate), RTL8168H/RTL8111H truncates incoming
> packets. Packet captures show messages like "IP truncated-ip - 146 bytes
> missing!".
> 
> The issue is caused by RxConfig not being properly re-initialized after
> resume. Re-initializing the RxConfig register before the chip
> re-initialization sequence avoids the truncation and restores correct
> packet reception.
> 
> This follows the same pattern as commit ef9da46ddef0 ("r8169: fix data
> corruption issue on RTL8402").
> 
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

Please send a new version including the fixes tag as asked by Jacob.
While at it please also add the missing 'net' prefix inside the subject.

You can retain Jacob's ack.

Thanks,

Paolo
Re: [PATCH] r8169: fix packet truncation after S4 resume on RTL8168H/RTL8111H
Posted by Jacob Keller 2 months, 1 week ago

On 10/5/2025 8:49 PM, lilinmao wrote:
> From: Linmao Li <lilinmao@kylinos.cn>
> 
> After resume from S4 (hibernate), RTL8168H/RTL8111H truncates incoming
> packets. Packet captures show messages like "IP truncated-ip - 146 bytes
> missing!".
> 
> The issue is caused by RxConfig not being properly re-initialized after
> resume. Re-initializing the RxConfig register before the chip
> re-initialization sequence avoids the truncation and restores correct
> packet reception.
> 
> This follows the same pattern as commit ef9da46ddef0 ("r8169: fix data
> corruption issue on RTL8402").
> 
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> 

You forgot to tag the subject prefix with 'net', but its quite obvious
this should go through the net fixes tree.

---
>  drivers/net/ethernet/realtek/r8169_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 9c601f271c02..4b0ac73565ea 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4994,8 +4994,9 @@ static int rtl8169_resume(struct device *device)
>  	if (!device_may_wakeup(tp_to_dev(tp)))
>  		clk_prepare_enable(tp->clk);
>  
> -	/* Reportedly at least Asus X453MA truncates packets otherwise */
> -	if (tp->mac_version == RTL_GIGA_MAC_VER_37)
> +	/* Some chip versions may truncate packets without this initialization */
> +	if (tp->mac_version == RTL_GIGA_MAC_VER_37 ||
> +	    tp->mac_version == RTL_GIGA_MAC_VER_46)
>  		rtl_init_rxcfg(tp);


Part of me wonders if there is a problem with just calling
rtl_init_rxcfg() here unconditionally.

Its contents are here:

> 
> static void rtl_init_rxcfg(struct rtl8169_private *tp)
> {
>         switch (tp->mac_version) {
>         case RTL_GIGA_MAC_VER_02 ... RTL_GIGA_MAC_VER_06:
>         case RTL_GIGA_MAC_VER_10 ... RTL_GIGA_MAC_VER_17:
>                 RTL_W32(tp, RxConfig, RX_FIFO_THRESH | RX_DMA_BURST);
>                 break;
>         case RTL_GIGA_MAC_VER_18 ... RTL_GIGA_MAC_VER_24:
>         case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_36:
>         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_52:
>                 RTL_W32(tp, RxConfig, RX128_INT_EN | RX_MULTI_EN | RX_DMA_BURST | RX_EARLY_OFF);
>                 break;
>         case RTL_GIGA_MAC_VER_61:
>                 RTL_W32(tp, RxConfig, RX_FETCH_DFLT_8125 | RX_DMA_BURST);
>                 break;
>         case RTL_GIGA_MAC_VER_63 ... RTL_GIGA_MAC_VER_LAST:
>                 RTL_W32(tp, RxConfig, RX_FETCH_DFLT_8125 | RX_DMA_BURST |
>                         RX_PAUSE_SLOT_ON);
>                 break;
>         default:
>                 RTL_W32(tp, RxConfig, RX128_INT_EN | RX_DMA_BURST);
>                 break;
>         }
> }


So based on version, we're going to do a different write, depending on
which hardware.

Without knowing the hardware, I can't tell if there could be side
effects from this write that are a problem on certain revisions... But
if there aren't, it seems better to call rtl_init_rxcfg unconditionally
just to ensure that the register is properly initialized. It could
potentially prevent finding the same issue on another revision in the
future.

Either way, this is obviously a fix for the given revision so I don't
see a reason to hold that up:

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


>  	return rtl8169_runtime_resume(device);