Messages in this thread | | | Date | Thu, 1 Jun 2023 09:22:50 -0700 | Subject | Re: [PATCH net-next] ethtool: ioctl: improve error checking for set_wol | From | Justin Chen <> |
| |
On 6/1/2023 8:55 AM, Simon Horman wrote: > + Daniil Tatianin <d-tatianin@yandex-team.ru>, Andrew Lunn <andrew@lunn.ch> > as per ./scripts/get_maintainer.pl --git-min-percent 25 net/ethtool/ioctl.c > > On Wed, May 31, 2023 at 01:53:49PM -0700, Justin Chen wrote: >> The netlink version of set_wol checks for not supported wolopts and avoids >> setting wol when the correct wolopt is already set. If we do the same with >> the ioctl version then we can remove these checks from the driver layer. > > Hi Justin, > > Are you planning follow-up patches for the driver layer? >
I was planning to for the Broadcom drivers since those I can test. But I could do it across the board if that is preferred.
>> Signed-off-by: Justin Chen <justin.chen@broadcom.com> >> --- >> net/ethtool/ioctl.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c >> index 6bb778e10461..80f456f83db0 100644 >> --- a/net/ethtool/ioctl.c >> +++ b/net/ethtool/ioctl.c >> @@ -1436,15 +1436,25 @@ 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; >> + struct ethtool_wolinfo wol, cur_wol; >> int ret; >> >> - if (!dev->ethtool_ops->set_wol) >> + if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol) >> return -EOPNOTSUPP; > > Are there cases where (in-tree) drivers provide set_wol byt not get_wol? > If so, does this break their set_wol support? >
My original thought was to match netlink set wol behavior. So drivers that do that won't work with netlink set_wol right now. I'll skim around to see if any drivers do this. But I would reckon this should be a driver fix.
Thanks, Justin
>> >> + memset(&cur_wol, 0, sizeof(struct ethtool_wolinfo)); >> + cur_wol.cmd = ETHTOOL_GWOL; >> + dev->ethtool_ops->get_wol(dev, &cur_wol); >> + >> if (copy_from_user(&wol, useraddr, sizeof(wol))) >> return -EFAULT; >> >> + if (wol.wolopts & ~cur_wol.supported) >> + return -EOPNOTSUPP; >> + >> + if (wol.wolopts == cur_wol.wolopts) >> + return 0; >> + >> ret = dev->ethtool_ops->set_wol(dev, &wol); >> if (ret) >> return ret; [unhandled content-type:application/pkcs7-signature]
| |