Merge branch 'ethtool-runtime-pm'

Heiner Kallweit says:

====================
ethtool: runtime-resume netdev parent before ethtool 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
ethtool ops in kernel.

This patch series covers the typical case that the netdev parent is power-
managed, e.g. a PCI device. Not sure whether cases exist where the netdev
itself is power-managed. If yes then we may need an extension for this.
But the series as-is at least shouldn't cause problems in that case.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2021-08-03 12:58:22 +01:00
commit 2dbf4c2e7e
3 changed files with 56 additions and 22 deletions

View file

@ -24,6 +24,7 @@
#include <linux/rtnetlink.h>
#include <linux/sched/signal.h>
#include <linux/net.h>
#include <linux/pm_runtime.h>
#include <net/devlink.h>
#include <net/xdp_sock_drv.h>
#include <net/flow_offload.h>
@ -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(&ethcmd, 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;
}

View file

@ -2,6 +2,7 @@
#include <net/sock.h>
#include <linux/ethtool_netlink.h>
#include <linux/pm_runtime.h>
#include "netlink.h"
static struct genl_family ethtool_genl_family;
@ -29,6 +30,44 @@ const struct nla_policy ethnl_header_policy_stats[] = {
ETHTOOL_FLAGS_STATS),
};
int ethnl_ops_begin(struct net_device *dev)
{
int ret;
if (!dev)
return 0;
if (dev->dev.parent)
pm_runtime_get_sync(dev->dev.parent);
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);
}
/**
* ethnl_parse_header_dev_get() - parse request header
* @req_info: structure to put results into
@ -101,12 +140,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;

View file

@ -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