| Message ID | 6a574380-06f6-4bce-9705-e2b554359337@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: fix WoL handling when suspending the PHY | expand |
On 09/24/2018 12:58 PM, Heiner Kallweit wrote: > Add flag wol_enabled to struct net_device indicating whether > Wake-on-LAN is enabled. As first user phy_suspend() will use it to > decide whether PHY can be suspended or not. > > Fixes: f1e911d5d0df ("r8169: add basic phylib support") > Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop") > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Thanks Heiner!
(I wrote my comment to v1 because I overlooked there is a v2 already; duplicating it here.) On Mon, Sep 24, 2018 at 09:58:59PM +0200, Heiner Kallweit wrote: > Add flag wol_enabled to struct net_device indicating whether > Wake-on-LAN is enabled. As first user phy_suspend() will use it to > decide whether PHY can be suspended or not. > > Fixes: f1e911d5d0df ("r8169: add basic phylib support") > Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop") > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > include/linux/netdevice.h | 3 +++ > net/core/ethtool.c | 9 ++++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 1cbbf77a6..f5f1f1450 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1756,6 +1756,8 @@ enum netdev_priv_flags { > * switch driver and used to set the phys state of the > * switch port. > * > + * @wol_enabled: Wake-on-LAN is enabled > + * > * FIXME: cleanup struct net_device such that network protocol info > * moves out. > */ > @@ -2039,6 +2041,7 @@ struct net_device { > struct lock_class_key *qdisc_tx_busylock; > struct lock_class_key *qdisc_running_key; > bool proto_down; > + unsigned wol_enabled:1; > }; > #define to_net_dev(d) container_of(d, struct net_device, dev) > As there is no bitfield yet, this would add 4 bytes to struct net_device. How about using a bit in net_device::priv_flags like IFF_RXFH_CONFIGURED in ethtool_set_rxfh_indir() and ethtool_set_rxfh()? Michal Kubecek
On 25.09.2018 10:28, Michal Kubecek wrote: > (I wrote my comment to v1 because I overlooked there is a v2 already; > duplicating it here.) > > On Mon, Sep 24, 2018 at 09:58:59PM +0200, Heiner Kallweit wrote: >> Add flag wol_enabled to struct net_device indicating whether >> Wake-on-LAN is enabled. As first user phy_suspend() will use it to >> decide whether PHY can be suspended or not. >> >> Fixes: f1e911d5d0df ("r8169: add basic phylib support") >> Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop") >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> include/linux/netdevice.h | 3 +++ >> net/core/ethtool.c | 9 ++++++++- >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index 1cbbf77a6..f5f1f1450 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -1756,6 +1756,8 @@ enum netdev_priv_flags { >> * switch driver and used to set the phys state of the >> * switch port. >> * >> + * @wol_enabled: Wake-on-LAN is enabled >> + * >> * FIXME: cleanup struct net_device such that network protocol info >> * moves out. >> */ >> @@ -2039,6 +2041,7 @@ struct net_device { >> struct lock_class_key *qdisc_tx_busylock; >> struct lock_class_key *qdisc_running_key; >> bool proto_down; >> + unsigned wol_enabled:1; >> }; >> #define to_net_dev(d) container_of(d, struct net_device, dev) >> > > As there is no bitfield yet, this would add 4 bytes to struct net_device. > How about using a bit in net_device::priv_flags like IFF_RXFH_CONFIGURED > in ethtool_set_rxfh_indir() and ethtool_set_rxfh()? > Indeed alternatively we could add a private flag and the related getter/setter. Regarding the size argument: We have few bool members in struct net_device (uc_promisc, dismantle, needs_free_netdev, proto_down) which most likely can be transparently converted to bitfield members. Then a wol_enabled bitfield member would be no overhead. I don't have a strong preference and would leave it up to David. > Michal Kubecek > Heiner
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 1cbbf77a6..f5f1f1450 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1756,6 +1756,8 @@ enum netdev_priv_flags { * switch driver and used to set the phys state of the * switch port. * + * @wol_enabled: Wake-on-LAN is enabled + * * FIXME: cleanup struct net_device such that network protocol info * moves out. */ @@ -2039,6 +2041,7 @@ struct net_device { struct lock_class_key *qdisc_tx_busylock; struct lock_class_key *qdisc_running_key; bool proto_down; + unsigned wol_enabled:1; }; #define to_net_dev(d) container_of(d, struct net_device, dev) diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 9d4e56d97..86f765d42 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -1395,6 +1395,7 @@ static int ethtool_get_wol(struct net_device *dev, char __user *useraddr) static int ethtool_set_wol(struct net_device *dev, char __user *useraddr) { struct ethtool_wolinfo wol; + int ret; if (!dev->ethtool_ops->set_wol) return -EOPNOTSUPP; @@ -1402,7 +1403,13 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr) if (copy_from_user(&wol, useraddr, sizeof(wol))) return -EFAULT; - return dev->ethtool_ops->set_wol(dev, &wol); + ret = dev->ethtool_ops->set_wol(dev, &wol); + if (ret) + return ret; + + dev->wol_enabled = !!wol.wolopts; + + return 0; } static int ethtool_get_eee(struct net_device *dev, char __user *useraddr)
Add flag wol_enabled to struct net_device indicating whether Wake-on-LAN is enabled. As first user phy_suspend() will use it to decide whether PHY can be suspended or not. Fixes: f1e911d5d0df ("r8169: add basic phylib support") Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop") Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- include/linux/netdevice.h | 3 +++ net/core/ethtool.c | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-)