Messages in this thread | | | Date | Thu, 1 Jun 2023 11:27:40 -0700 | Subject | Re: [PATCH net-next] ethtool: ioctl: improve error checking for set_wol | From | Justin Chen <> |
| |
On 6/1/23 9:22 AM, Justin Chen wrote: > > > 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 >
I see a driver at drivers/net/phy/microchip.c. But this is a phy driver set_wol hook.
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]
| |