Messages in this thread | | | Subject | Re: [PATCH v3 2/2] net-sysfs: Ensure begin/complete are called in speed_show() and duplex_show() | From | Kai-Heng Feng <> | Date | Thu, 20 Feb 2020 13:45:16 +0800 |
| |
> On Feb 7, 2020, at 18:10, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote: > > 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 > > -1 here means SPEED_UNKNOWN, which is the correct value when igb is > runtime suspended. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
A gentle ping...
Kai-Heng
> --- > v3: > - Specify -1 means SPEED_UNKNOWN. > 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 >
| |