netdev - Re: WoL broken in r8169.c since kernel 4.19

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f40f95a-f595-45f5-5641-9eb4837da71e@gmail.com>
Date:   Tue, 29 Jan 2019 20:01:14 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Marc Haber <mh+netdev@...schlus.de>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: WoL broken in r8169.c since kernel 4.19

Hi Marc,

the change to replace __rtl8169_set_wol(tp, 0) doesn't seem to be the right commit
because it was included in 4.18 already. And if you read the commit description you'll
see that it was replaced because it caused issues with certain boards. Having said that
it's not an option for us.

Still I'm struggling to see where the relevant difference between 4.18 and 4.19 is.
Especially as 4.19 and also later versions work perfectly fine here.

Can you in addition apply the following (again it may not apply cleanly) and provide
the results for 4.18 and 4.19?

And from today's run, can you provide the full dmesg output? I'd like to check
why the message was written on resume only.

Heiner

---
 drivers/net/ethernet/realtek/r8169.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index bd26d3f2e..e9c37f10c 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1414,6 +1414,8 @@ static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	wol->supported = WAKE_ANY;
 	wol->wolopts = tp->saved_wolopts;
 	rtl_unlock_work(tp);
+
+	pr_info("get_wol: 0x%08x\n", wol->wolopts);
 }
 
 static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
@@ -1491,6 +1493,8 @@ static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct device *d = tp_to_dev(tp);
 
+	pr_info("set_wol: 0x%08x\n", wol->wolopts);
+
 	if (wol->wolopts & ~WAKE_ANY)
 		return -EINVAL;
 
-- 
2.20.1




On 29.01.2019 16:35, Marc Haber wrote:
> Hi,
> 
> after having a good night's sleep over that, it's obviously a merge
> commit which cannot easily be reverted. How would I continue after
> identifying a merge commit as the culprit?
> 
> On Tue, Jan 29, 2019 at 08:32:53AM +0100, Marc Haber wrote:
>> According to bisect, the first bad commit is
>> 19725496da5602b401eae389736ab00d1817e264
>>
>> commit 19725496da5602b401eae389736ab00d1817e264
>> Merge: aea5f654e6b7 9981b4fb8684
> 
> git diff aea5f654e6b7..19725496da5602b401eae389736ab00d1817e264,
> filtered for r8169 looks manageable:
> 
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -7396,8 +7396,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struc
>                 return rc;
>         }
>  
> -       /* override BIOS settings, use userspace tools to enable WOL */
> -       __rtl8169_set_wol(tp, 0);
> +       tp->saved_wolopts = __rtl8169_get_wol(tp);
>  
>         mutex_init(&tp->wk.mutex);
>         u64_stats_init(&tp->rx_stats.syncp);
> 
> but the other one seems unmanageably big:
> 
> [18/5009]mh@fan:~/linux/git/linux (master % u=) $ git diff 9981b4fb8684..19725496da5602b401eae389736ab00d1817e264 -- drivers/net/ethernet/realtek/r8169.c | diffstat
>  r8169.c |  815 ++++++++++++++++++----------------------------------------------
>  1 file changed, 234 insertions(+), 581 deletions(-)
> [19/5009]mh@fan:~/linux/git/linux (master % u=) $ 
> 
> -------
> But, indeed, adding the call to __rtl8169_set_wol(tp, 0) fixes the issue
> for me and the machine now wakes up from StR on a magic packet without
> having to go through strange ethtool motions.
> -------
> 
> Would that code change be suitable for the official kernel cod?
> 
> Greetings
> Marc
>