[PATCH v2 2/2] net-sysfs: Ensure begin/complete are called in speed_show() and duplex_show()

From: Kai-Heng Feng
Date: Wed Feb 05 2020 - 03:17:07 EST


Device like igb gets runtime suspended when there's no link partner. We
can't get correct speed under that state:
$ cat /sys/class/net/enp3s0/speed
1000

In addition to that, an error can also be spotted in dmesg:
[ 385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost

It's because the igb device doesn't get runtime resumed before calling
get_link_ksettings().

So let's use a new helper to call begin() and complete() like what
dev_ethtool() does, to runtime resume/suspend or power up/down the
device properly.

Once this fix is in place, igb can show the speed correctly without link
partner:
$ cat /sys/class/net/enp3s0/speed
-1

Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
---
v2:
- Add a new helper with begin/complete and use it in net-sysfs.

include/linux/ethtool.h | 4 ++++
net/core/net-sysfs.c | 4 ++--
net/ethtool/ioctl.c | 33 ++++++++++++++++++++++++++++++++-
3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 95991e4300bf..785ec1921417 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -160,6 +160,10 @@ extern int
__ethtool_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *link_ksettings);

+extern int
+__ethtool_get_link_ksettings_full(struct net_device *dev,
+ struct ethtool_link_ksettings *link_ksettings);
+
/**
* ethtool_intersect_link_masks - Given two link masks, AND them together
* @dst: first mask and where result is stored
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 4c826b8bf9b1..a199e15a080f 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -201,7 +201,7 @@ static ssize_t speed_show(struct device *dev,
if (netif_running(netdev)) {
struct ethtool_link_ksettings cmd;

- if (!__ethtool_get_link_ksettings(netdev, &cmd))
+ if (!__ethtool_get_link_ksettings_full(netdev, &cmd))
ret = sprintf(buf, fmt_dec, cmd.base.speed);
}
rtnl_unlock();
@@ -221,7 +221,7 @@ static ssize_t duplex_show(struct device *dev,
if (netif_running(netdev)) {
struct ethtool_link_ksettings cmd;

- if (!__ethtool_get_link_ksettings(netdev, &cmd)) {
+ if (!__ethtool_get_link_ksettings_full(netdev, &cmd)) {
const char *duplex;

switch (cmd.base.duplex) {
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index b987052d91ef..faeba247c1fb 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -420,7 +420,9 @@ struct ethtool_link_usettings {
} link_modes;
};

-/* Internal kernel helper to query a device ethtool_link_settings. */
+/* Internal kernel helper to query a device ethtool_link_settings. To be called
+ * inside begin/complete block.
+ */
int __ethtool_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *link_ksettings)
{
@@ -434,6 +436,35 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
}
EXPORT_SYMBOL(__ethtool_get_link_ksettings);

+/* Internal kernel helper to query a device ethtool_link_settings. To be called
+ * outside of begin/complete block.
+ */
+int __ethtool_get_link_ksettings_full(struct net_device *dev,
+ struct ethtool_link_ksettings *link_ksettings)
+{
+ int rc;
+
+ ASSERT_RTNL();
+
+ if (!dev->ethtool_ops->get_link_ksettings)
+ return -EOPNOTSUPP;
+
+ if (dev->ethtool_ops->begin) {
+ rc = dev->ethtool_ops->begin(dev);
+ if (rc < 0)
+ return rc;
+ }
+
+ memset(link_ksettings, 0, sizeof(*link_ksettings));
+ rc = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
+
+ if (dev->ethtool_ops->complete)
+ dev->ethtool_ops->complete(dev);
+
+ return rc;
+}
+EXPORT_SYMBOL(__ethtool_get_link_ksettings_full);
+
/* convert ethtool_link_usettings in user space to a kernel internal
* ethtool_link_ksettings. return 0 on success, errno on error.
*/
--
2.17.1