netdev - RE: [PATCH V2 net] net: stmmac: fix MAC WoL unwork if PHY doesn't support WoL

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: <DB8PR04MB6795A418BE733407FB4B7AF6E6419@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Tue, 27 Apr 2021 02:44:43 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Andrew Lunn <andrew@...n.ch>,
        "Jisheng.Zhang@...aptics.com" <Jisheng.Zhang@...aptics.com>
CC:     "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH V2 net] net: stmmac: fix MAC WoL unwork if PHY doesn't
 support WoL


> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: 2021年4月26日 21:05
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: peppe.cavallaro@...com; alexandre.torgue@...com;
> joabreu@...opsys.com; davem@...emloft.net; kuba@...nel.org;
> f.fainelli@...il.com; Jisheng.Zhang@...aptics.com; netdev@...r.kernel.org;
> dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH V2 net] net: stmmac: fix MAC WoL unwork if PHY doesn't
> support WoL
> 
> > +	if (wol->wolopts & WAKE_PHY) {
> > +		int ret = phylink_ethtool_set_wol(priv->phylink, wol);
> 
> This is wrong. No PHY actually implements WAKE_PHY.
> 
> What PHYs do implement is WAKE_MAGIC, WAKE_MAGICSEC, WAKE_UCAST,
> and WAKE_BCAST. So there is a clear overlap with what the MAC can do.
> 
> So you need to decide which is going to provide each of these wakeups, the
> MAC or the PHY, and make sure only one does the actual implementation.

Hi Andrew,

Thanks for your review:-), PHY wakeup have not test at my side, need @Jisheng.Zhang@...aptics.com have a test if possible.

According to your comments, I did a quick draft, and have not test yet, could you please review the logic to see if I understand you correctly? Thanks.

--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -647,18 +647,7 @@ static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
        struct stmmac_priv *priv = netdev_priv(dev);
-       u32 support = WAKE_MAGIC | WAKE_UCAST;
-
-       if (!device_can_wakeup(priv->device))
-               return -EOPNOTSUPP;
-
-       if (!priv->plat->pmt) {
-               int ret = phylink_ethtool_set_wol(priv->phylink, wol);
-
-               if (!ret)
-                       device_set_wakeup_enable(priv->device, !!wol->wolopts);
-               return ret;
-       }
+       u32 support = WAKE_MAGIC | WAKE_UCAST | WAKE_MAGICSECURE |  WAKE_BCAST;

        /* By default almost all GMAC devices support the WoL via
         * magic frame but we can disable it if the HW capability
@@ -669,13 +658,24 @@ static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
        if (wol->wolopts & ~support)
                return -EINVAL;

-       if (wol->wolopts) {
-               pr_info("stmmac: wakeup enable\n");
-               device_set_wakeup_enable(priv->device, 1);
-               enable_irq_wake(priv->wol_irq);
-       } else {
+       if (!wol->wolopts) {
+               device_set_wakeup_capable(priv->device, 0);
                device_set_wakeup_enable(priv->device, 0);
                disable_irq_wake(priv->wol_irq);
+       } else {
+               if (priv->plat->pmt && ((wol->wolopts & WAKE_MAGIC) || (wol->wolopts & WAKE_UCAST))) {
+                       pr_info("stmmac: mac wakeup enable\n");
+                       enable_irq_wake(priv->wol_irq);
+               } else {
+                       int ret = phylink_ethtool_set_wol(priv->phylink, wol);
+
+                       if (!ret)
+                               pr_info("stmmac: phy wakeup enable\n");
+                       else
+                               return ret;
+               }
+               device_set_wakeup_capable(priv->device, 1);
+               device_set_wakeup_enable(priv->device, 1);
        }

        mutex_lock(&priv->lock);


Best Regards,
Joakim Zhang
> 	Andrew