Skip to content

Commit

Permalink
Merge branch 'ethtool-runtime-pm'
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
David S. Miller committed Aug 3, 2021
2 parents c32325b + d43c65b commit 2dbf4c2
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 22 deletions.
18 changes: 15 additions & 3 deletions net/ethtool/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
45 changes: 39 additions & 6 deletions net/ethtool/netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 2 additions & 13 deletions net/ethtool/netlink.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2dbf4c2

Please sign in to comment.