From f32a213765739f2a1db319346799f130a3d08820 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 1 Aug 2021 12:36:48 +0200 Subject: [PATCH 1/4] ethtool: runtime-resume netdev parent before ethtool ioctl ops If a network device is runtime-suspended then: - network device may be flagged as detached and all ethtool ops (even if not accessing the device) will fail because netif_device_present() returns false - ethtool ops may fail because device is not accessible (e.g. because being in D3 in case of a PCI device) It may not be desirable that userspace can't use even simple ethtool ops that not access the device if interface or link is down. To be more friendly to userspace let's ensure that device is runtime-resumed when executing the respective ethtool op in kernel. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- net/ethtool/ioctl.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index b0fa2b00ad43..81fa36a4c9c4 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -2692,7 +2693,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr) int rc; netdev_features_t old_features; - if (!dev || !netif_device_present(dev)) + if (!dev) return -ENODEV; if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd))) @@ -2748,10 +2749,18 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr) return -EPERM; } + if (dev->dev.parent) + pm_runtime_get_sync(dev->dev.parent); + + if (!netif_device_present(dev)) { + rc = -ENODEV; + goto out; + } + if (dev->ethtool_ops->begin) { rc = dev->ethtool_ops->begin(dev); - if (rc < 0) - return rc; + if (rc < 0) + goto out; } old_features = dev->features; @@ -2970,6 +2979,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr) if (old_features != dev->features) netdev_features_change(dev); +out: + if (dev->dev.parent) + pm_runtime_put(dev->dev.parent); return rc; } From c5ab51df03e2d7ec8e57904aaa2c4d03b607b2b5 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 1 Aug 2021 12:37:39 +0200 Subject: [PATCH 2/4] ethtool: move implementation of ethnl_ops_begin/complete to netlink.c In preparation of subsequent extensions to both functions move the implementations from netlink.h to netlink.c. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- net/ethtool/netlink.c | 14 ++++++++++++++ net/ethtool/netlink.h | 15 ++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 73e0f5b626bf..ac720d684789 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -29,6 +29,20 @@ const struct nla_policy ethnl_header_policy_stats[] = { ETHTOOL_FLAGS_STATS), }; +int ethnl_ops_begin(struct net_device *dev) +{ + if (dev && dev->ethtool_ops->begin) + return dev->ethtool_ops->begin(dev); + else + return 0; +} + +void ethnl_ops_complete(struct net_device *dev) +{ + if (dev && dev->ethtool_ops->complete) + dev->ethtool_ops->complete(dev); +} + /** * ethnl_parse_header_dev_get() - parse request header * @req_info: structure to put results into diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index 3fc395c86702..077aac3929a8 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -247,19 +247,8 @@ struct ethnl_reply_data { struct net_device *dev; }; -static inline int ethnl_ops_begin(struct net_device *dev) -{ - if (dev && dev->ethtool_ops->begin) - return dev->ethtool_ops->begin(dev); - else - return 0; -} - -static inline void ethnl_ops_complete(struct net_device *dev) -{ - if (dev && dev->ethtool_ops->complete) - dev->ethtool_ops->complete(dev); -} +int ethnl_ops_begin(struct net_device *dev); +void ethnl_ops_complete(struct net_device *dev); /** * struct ethnl_request_ops - unified handling of GET requests From 41107ac22fcf39c45afaf1a59e259e5e0059e31a Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 1 Aug 2021 12:40:05 +0200 Subject: [PATCH 3/4] ethtool: move netif_device_present check from ethnl_parse_header_dev_get to ethnl_ops_begin If device is runtime-suspended and not accessible then it may be flagged as not present. If checking whether device is present is done too early then we may bail out before we have the chance to runtime-resume the device. Therefore move this check to ethnl_ops_begin(). This is in preparation of a follow-up patch that tries to runtime-resume the device before executing ethtool ops. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- net/ethtool/netlink.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index ac720d684789..e628d17f595c 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -31,7 +31,13 @@ const struct nla_policy ethnl_header_policy_stats[] = { int ethnl_ops_begin(struct net_device *dev) { - if (dev && dev->ethtool_ops->begin) + if (!dev) + return 0; + + if (!netif_device_present(dev)) + return -ENODEV; + + if (dev->ethtool_ops->begin) return dev->ethtool_ops->begin(dev); else return 0; @@ -115,12 +121,6 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info, return -EINVAL; } - if (dev && !netif_device_present(dev)) { - dev_put(dev); - NL_SET_ERR_MSG(extack, "device not present"); - return -ENODEV; - } - req_info->dev = dev; req_info->flags = flags; return 0; From d43c65b05b848e0b2db1a6c78b02c189da3a95b5 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 1 Aug 2021 12:41:31 +0200 Subject: [PATCH 4/4] ethtool: runtime-resume netdev parent in ethnl_ops_begin If a network device is runtime-suspended then: - network device may be flagged as detached and all ethtool ops (even if not accessing the device) will fail because netif_device_present() returns false - ethtool ops may fail because device is not accessible (e.g. because being in D3 in case of a PCI device) It may not be desirable that userspace can't use even simple ethtool ops that not access the device if interface or link is down. To be more friendly to userspace let's ensure that device is runtime-resumed when executing the respective ethtool op in kernel. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- net/ethtool/netlink.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index e628d17f595c..417aaf9ca219 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -2,6 +2,7 @@ #include #include +#include #include "netlink.h" static struct genl_family ethtool_genl_family; @@ -31,22 +32,40 @@ const struct nla_policy ethnl_header_policy_stats[] = { int ethnl_ops_begin(struct net_device *dev) { + int ret; + if (!dev) return 0; - if (!netif_device_present(dev)) - return -ENODEV; + if (dev->dev.parent) + pm_runtime_get_sync(dev->dev.parent); - if (dev->ethtool_ops->begin) - return dev->ethtool_ops->begin(dev); - else - return 0; + if (!netif_device_present(dev)) { + ret = -ENODEV; + goto err; + } + + if (dev->ethtool_ops->begin) { + ret = dev->ethtool_ops->begin(dev); + if (ret) + goto err; + } + + return 0; +err: + if (dev->dev.parent) + pm_runtime_put(dev->dev.parent); + + return ret; } void ethnl_ops_complete(struct net_device *dev) { if (dev && dev->ethtool_ops->complete) dev->ethtool_ops->complete(dev); + + if (dev->dev.parent) + pm_runtime_put(dev->dev.parent); } /**