| Message ID | 1686605822-34544-1-git-send-email-justin.chen@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2bddad9ec65a925da4ab4f73a9377d80540ca67b |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] ethtool: ioctl: account for sopass diff in set_wol | expand |
On Mon, Jun 12, 2023 at 02:37:00PM -0700, Justin Chen wrote: > sopass won't be set if wolopt doesn't change. This means the following > will fail to set the correct sopass. > ethtool -s eth0 wol s sopass 11:22:33:44:55:66 > ethtool -s eth0 wol s sopass 22:44:55:66:77:88 > > Make sure we call into the driver layer set_wol if sopass is different. > > Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol") > Signed-off-by: Justin Chen <justin.chen@broadcom.com> > --- > > Note: Tagged "Fixes" patch has not hit rc yet. > > net/ethtool/ioctl.c | 3 ++- Hi Justin Does the netlink version get this correct? And just for my own curiosity, why are you so interested in the ioctl version, which is deprecated and not used by modern versions of ethtool? Andrew
On 6/12/23 3:28 PM, Andrew Lunn wrote: > On Mon, Jun 12, 2023 at 02:37:00PM -0700, Justin Chen wrote: >> sopass won't be set if wolopt doesn't change. This means the following >> will fail to set the correct sopass. >> ethtool -s eth0 wol s sopass 11:22:33:44:55:66 >> ethtool -s eth0 wol s sopass 22:44:55:66:77:88 >> >> Make sure we call into the driver layer set_wol if sopass is different. >> >> Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol") >> Signed-off-by: Justin Chen <justin.chen@broadcom.com> >> --- >> >> Note: Tagged "Fixes" patch has not hit rc yet. >> >> net/ethtool/ioctl.c | 3 ++- > > Hi Justin > > Does the netlink version get this correct? > Yup netlink gets this correct > And just for my own curiosity, why are you so interested in the ioctl > version, which is deprecated and not used by modern versions of > ethtool? > No particular reason. I was working on another driver and was planning to remove the set_wol error checking in the driver layer when I realized netlink set_wol was already doing it. Then on further inspection I noticed the ioctl version doesn't do the same thing. Justin > Andrew
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 12 Jun 2023 14:37:00 -0700 you wrote: > sopass won't be set if wolopt doesn't change. This means the following > will fail to set the correct sopass. > ethtool -s eth0 wol s sopass 11:22:33:44:55:66 > ethtool -s eth0 wol s sopass 22:44:55:66:77:88 > > Make sure we call into the driver layer set_wol if sopass is different. > > [...] Here is the summary with links: - [net-next] ethtool: ioctl: account for sopass diff in set_wol https://git.kernel.org/netdev/net-next/c/2bddad9ec65a You are awesome, thank you!
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 37b582225854..4a51e0ec295c 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -1452,7 +1452,8 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr) if (wol.wolopts & ~cur_wol.supported) return -EINVAL; - if (wol.wolopts == cur_wol.wolopts) + if (wol.wolopts == cur_wol.wolopts && + !memcmp(wol.sopass, cur_wol.sopass, sizeof(wol.sopass))) return 0; ret = dev->ethtool_ops->set_wol(dev, &wol);
sopass won't be set if wolopt doesn't change. This means the following will fail to set the correct sopass. ethtool -s eth0 wol s sopass 11:22:33:44:55:66 ethtool -s eth0 wol s sopass 22:44:55:66:77:88 Make sure we call into the driver layer set_wol if sopass is different. Fixes: 55b24334c0f2 ("ethtool: ioctl: improve error checking for set_wol") Signed-off-by: Justin Chen <justin.chen@broadcom.com> --- Note: Tagged "Fixes" patch has not hit rc yet. net/ethtool/ioctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)