Message-Id: <9F5C55F0-84EC-48C2-94E2-7729A569C8CA@exactco.de>
Date: Tue, 2 Dec 2025 00:26:06 +0100
From: René Rebe <rene@...ctco.de>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: netdev@...r.kernel.org,
 nic_swsd@...ltek.com
Subject: Re: [PATCH] r8169: fix RTL8117 Wake-on-Lan in DASH mode

Hi,

> On 1. Dec 2025, at 22:12, Heiner Kallweit <hkallweit1@...il.com> wrote:
> 
> On 12/1/2025 9:31 PM, René Rebe wrote:
>> Hi
>> 
>>> On 1. Dec 2025, at 21:15, Heiner Kallweit <hkallweit1@...il.com> wrote:
>>> 
>>> On 12/1/2025 8:17 PM, René Rebe wrote:
>>>> Wake-on-Lan does currently not work in DASH mode, e.g. the ASUS Pro WS
>>>> X570-ACE with RTL8168fp/RTL8117.
>>>> 
>>>> Fix by not returning early in rtl_prepare_power_down when dash_enabled.
>>> Good
>>> 
>>>> While this fixes WOL, it still kills the OOB RTL8117 remote management
>>>> BMC connection. Fix by not calling rtl8168_driver_stop if wol is enabled.
>>>> 
>>> You mean remote management whilst system is powered down and waiting
>>> for a WoL packet? Note that link speed is reduced to a minimum then,
>>> and DMA is disabled. Who would drive the MAC?
>>> Realtek doesn't provide any chip documentation, therefore it's hard to
>>> say what is expected from the MAC driver in DASH case.
>> 
>> This RTL8117 has a 250 or 400 MHz MIPS cpu inside that runs
>> a out-of-band linux kernel. Pretty sketchy low-quality setup unfortunately:
>> 
>> https://www.youtube.com/watch?v=YqEa8Gd1c2I&t=1695s
>>> 
>>>> While at it, enable wake on magic packet by default, like most other
>>>> Linux drivers do.
>>>> 
>>> It's by intent that WoL is disabled per default. Most users don't use WoL
>>> and would suffer from higher power consumption if system is suspended
>>> or powered down.
>> 
>> It was just a suggestion, I can use ethtool, it is the only driver that does
>> not have it on by default in all the systems I have.
>> 
>>> Which benefit would you see if WoL would be enabled by default
>>> (in DASH and non-DASH case)?
>> 
>> So it just works when pro-sumers want to wake it up, not the most
>> important detail of the patch.
>> 
>>>> Signed-off-by: René Rebe <rene@...ctco.de>
>>> 
>>> Your patch apparently is meant to be a fix. Therefore please add Fixes
>>> tag and address to net tree.
>>> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.rst
>>> And please add all netdev maintainers when re-submitting.
>>> scripts/get_maintainer.pl provides all needed info.
>> 
>> Yes, I realized after sending. The only Fixes: would be the original
>> change adding the DASH support I assume?
>> 
>> Any opinion re not stopping DASH on if down? IMHO taking a
>> link down should not break the remote management connection.
>> 
> I have no clue how the OOB BMC interacts with MAC/PHY, and I have no
> hw supporting DASH to test. So not really a basis for an opinion.
> However: DASH has been existing on Realtek hw for at least 15 yrs,
> and I'm not aware of any complaint related to what you mention.
> So it doesn't seem to be a common use case.

Well the Asus Control Center Express is so bad and barely working
it does not surprise me nobody is using it. We reversed the protocol
and wrote some script and hacked VNC client to make it useful for us.

The Asus GPL compliance code dump for the MIPS Linux BMC system
has some rtl8168_oob or so BMC side driver for it to learn more details.

Maybe I should backup a fork of it on my GitHub to archive it.

Given the BMC should be reachable, would be acceptable if I work
out a patch to not take the phy down and always keep it up even
for if down and module unload when in “dash” mode?

> There are different generations of DASH in RTL8168DP, RTL8168EP,
> RTL8117, variants of RTL8125, RTL8127 etc. Having said that,
> there's a certain chance of a regression, even if the patch works
> correctly on your system. Therefore I'd prefer to handle any additional
> changes in separate patches, to facilitate bisecting in case of a
> regression.

Of course, will sent separate patches for each topic.

What about defaulting to wol by magic like most other linux
drivers? IMHO the drivers should all be have similar and not
all have some other defaults. In theory it could be made
a tree-wide kconfig if users and distros care enough what the
global default should be.

	René

>> I probably would need to single step thru the driver init to find out
>> what reset stops the out of band traffic there, too.
>> 
>> René
>> 
>>>> ---
>>>> 
>>>> There is still another issue that should be fixed: the dirver init
>>>> kills the OOB BMC connection until if up, too. We also should probaly
>>>> not even conditionalize rtl8168_driver_stop on wol_enabled as the BMC
>>>> should always be accessible. IMHO even on module unload.
>>>> 
>>>> ---
>>>> drivers/net/ethernet/realtek/r8169_main.c | 9 +++++----
>>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>> index 853aabedb128..e2f9b9027fe2 100644
>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>> @@ -2669,9 +2669,6 @@ static void rtl_wol_enable_rx(struct rtl8169_private *tp)
>>>> 
>>>> static void rtl_prepare_power_down(struct rtl8169_private *tp)
>>>> {
>>>> - if (tp->dash_enabled)
>>>> - return;
>>>> -
>>>> if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
>>>>   tp->mac_version == RTL_GIGA_MAC_VER_33)
>>>> rtl_ephy_write(tp, 0x19, 0xff64);
>>>> @@ -4807,7 +4804,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
>>>> rtl_disable_exit_l1(tp);
>>>> rtl_prepare_power_down(tp);
>>>> 
>>>> - if (tp->dash_type != RTL_DASH_NONE)
>>>> + if (tp->dash_type != RTL_DASH_NONE && !tp->saved_wolopts)
>>>> rtl8168_driver_stop(tp);
>>>> }
>>>> 
>>>> @@ -5406,6 +5403,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>> tp->pci_dev = pdev;
>>>> tp->supports_gmii = ent->driver_data == RTL_CFG_NO_GBIT ? 0 : 1;
>>>> tp->ocp_base = OCP_STD_PHY_BASE;
>>>> + tp->saved_wolopts = WAKE_MAGIC;
>>>> 
>>>> raw_spin_lock_init(&tp->mac_ocp_lock);
>>>> mutex_init(&tp->led_lock);
>>>> @@ -5565,6 +5563,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>> if (rc)
>>>> return rc;
>>>> 
>>>> + if (tp->saved_wolopts)
>>>> + __rtl8169_set_wol(tp, tp->saved_wolopts);
>>>> +
>>>> rc = register_netdev(dev);
>>>> if (rc)
>>>> return rc;
>>> 
>> 
> 

-- 
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Powered by blists - more mailing lists