| Message ID | 20230303164248.499286-3-kory.maincent@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Up until now, there was no way to let the user select the layer at which time stamping occurs. The stack assumed that PHY time stamping is always preferred, but some MAC/PHY combinations were buggy. | expand |
Hi Köry,
I love your patch! Perhaps something to improve:
[auto build test WARNING on v6.2]
[also build test WARNING on next-20230303]
[cannot apply to net/master net-next/master horms-ipvs/master linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-ethtool-Refactor-identical-get_ts_info-implementations/20230304-004527
patch link: https://lore.kernel.org/r/20230303164248.499286-3-kory.maincent%40bootlin.com
patch subject: [PATCH v2 2/4] net: Expose available time stamping layers to user space.
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230304/202303040133.slT4slaW-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/90d54e1c6ed12a0b55c868e7808d93f61dad3534
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review K-ry-Maincent/net-ethtool-Refactor-identical-get_ts_info-implementations/20230304-004527
git checkout 90d54e1c6ed12a0b55c868e7808d93f61dad3534
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash net/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303040133.slT4slaW-lkp@intel.com/
All warnings (new ones prefixed by >>):
net/core/net-sysfs.c: In function 'available_timestamping_providers_show':
>> net/core/net-sysfs.c:627:35: warning: variable 'ops' set but not used [-Wunused-but-set-variable]
627 | const struct ethtool_ops *ops;
| ^~~
net/core/net-sysfs.c: In function 'current_timestamping_provider_show':
net/core/net-sysfs.c:657:35: warning: variable 'ops' set but not used [-Wunused-but-set-variable]
657 | const struct ethtool_ops *ops;
| ^~~
vim +/ops +627 net/core/net-sysfs.c
622
623 static ssize_t available_timestamping_providers_show(struct device *dev,
624 struct device_attribute *attr,
625 char *buf)
626 {
> 627 const struct ethtool_ops *ops;
628 struct net_device *netdev;
629 struct phy_device *phydev;
630 int ret = 0;
631
632 netdev = to_net_dev(dev);
633 phydev = netdev->phydev;
634 ops = netdev->ethtool_ops;
635
636 if (!rtnl_trylock())
637 return restart_syscall();
638
639 ret += sprintf(buf, "%s\n", "mac");
640 buf += 4;
641
642 if (phy_has_tsinfo(phydev)) {
643 ret += sprintf(buf, "%s\n", "phy");
644 buf += 4;
645 }
646
647 rtnl_unlock();
648
649 return ret;
650 }
651 static DEVICE_ATTR_RO(available_timestamping_providers);
652
On Fri, 3 Mar 2023 17:42:39 +0100 Köry Maincent wrote: > Time stamping on network packets may happen either in the MAC or in > the PHY, but not both. In preparation for making the choice > selectable, expose both the current and available layers via sysfs. Ethtool, please, no sysfs.
Köry Maincent wrote: > From: Richard Cochran <richardcochran@gmail.com> > > Time stamping on network packets may happen either in the MAC or in > the PHY, but not both. In preparation for making the choice > selectable, expose both the current and available layers via sysfs. > > In accordance with the kernel implementation as it stands, the current > layer will always read as "phy" when a PHY time stamping device is > present. Future patches will allow changing the current layer > administratively. > > Signed-off-by: Richard Cochran <richardcochran@gmail.com> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- > > Notes: > Changes in v2: > - Move the introduction of selected_timestamping_layer variable in next > patch. > > .../ABI/testing/sysfs-class-net-timestamping | 17 ++++++ > net/core/net-sysfs.c | 60 +++++++++++++++++++ > 2 files changed, 77 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-net-timestamping > > diff --git a/Documentation/ABI/testing/sysfs-class-net-timestamping b/Documentation/ABI/testing/sysfs-class-net-timestamping > new file mode 100644 > index 000000000000..529c3a6eb607 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-net-timestamping > @@ -0,0 +1,17 @@ > +What: /sys/class/net/<iface>/available_timestamping_providers > +Date: January 2022 > +Contact: Richard Cochran <richardcochran@gmail.com> > +Description: > + Enumerates the available providers for SO_TIMESTAMPING. > + The possible values are: > + - "mac" The MAC provides time stamping. > + - "phy" The PHY or MII device provides time stamping. > + > +What: /sys/class/net/<iface>/current_timestamping_provider > +Date: January 2022 > +Contact: Richard Cochran <richardcochran@gmail.com> > +Description: > + Show the current SO_TIMESTAMPING provider. > + The possible values are: > + - "mac" The MAC provides time stamping. > + - "phy" The PHY or MII device provides time stamping. > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index 8409d41405df..26095634fb31 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -620,6 +620,64 @@ static ssize_t threaded_store(struct device *dev, > } > static DEVICE_ATTR_RW(threaded); > > +static ssize_t available_timestamping_providers_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + const struct ethtool_ops *ops; > + struct net_device *netdev; > + struct phy_device *phydev; > + int ret = 0; > + > + netdev = to_net_dev(dev); > + phydev = netdev->phydev; > + ops = netdev->ethtool_ops; > + > + if (!rtnl_trylock()) > + return restart_syscall(); > + > + ret += sprintf(buf, "%s\n", "mac"); > + buf += 4; > + Should advertising mac be subject to having ops->get_ts_info? > + if (phy_has_tsinfo(phydev)) { > + ret += sprintf(buf, "%s\n", "phy"); > + buf += 4; > + } > + > + rtnl_unlock(); > + > + return ret; > +} > +static DEVICE_ATTR_RO(available_timestamping_providers);
diff --git a/Documentation/ABI/testing/sysfs-class-net-timestamping b/Documentation/ABI/testing/sysfs-class-net-timestamping new file mode 100644 index 000000000000..529c3a6eb607 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-net-timestamping @@ -0,0 +1,17 @@ +What: /sys/class/net/<iface>/available_timestamping_providers +Date: January 2022 +Contact: Richard Cochran <richardcochran@gmail.com> +Description: + Enumerates the available providers for SO_TIMESTAMPING. + The possible values are: + - "mac" The MAC provides time stamping. + - "phy" The PHY or MII device provides time stamping. + +What: /sys/class/net/<iface>/current_timestamping_provider +Date: January 2022 +Contact: Richard Cochran <richardcochran@gmail.com> +Description: + Show the current SO_TIMESTAMPING provider. + The possible values are: + - "mac" The MAC provides time stamping. + - "phy" The PHY or MII device provides time stamping. diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 8409d41405df..26095634fb31 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -620,6 +620,64 @@ static ssize_t threaded_store(struct device *dev, } static DEVICE_ATTR_RW(threaded); +static ssize_t available_timestamping_providers_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + const struct ethtool_ops *ops; + struct net_device *netdev; + struct phy_device *phydev; + int ret = 0; + + netdev = to_net_dev(dev); + phydev = netdev->phydev; + ops = netdev->ethtool_ops; + + if (!rtnl_trylock()) + return restart_syscall(); + + ret += sprintf(buf, "%s\n", "mac"); + buf += 4; + + if (phy_has_tsinfo(phydev)) { + ret += sprintf(buf, "%s\n", "phy"); + buf += 4; + } + + rtnl_unlock(); + + return ret; +} +static DEVICE_ATTR_RO(available_timestamping_providers); + +static ssize_t current_timestamping_provider_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + const struct ethtool_ops *ops; + struct net_device *netdev; + struct phy_device *phydev; + int ret; + + netdev = to_net_dev(dev); + phydev = netdev->phydev; + ops = netdev->ethtool_ops; + + if (!rtnl_trylock()) + return restart_syscall(); + + if (phy_has_tsinfo(phydev)) { + ret = sprintf(buf, "%s\n", "phy"); + } else { + ret = sprintf(buf, "%s\n", "mac"); + } + + rtnl_unlock(); + + return ret; +} +static DEVICE_ATTR_RO(current_timestamping_provider); + static struct attribute *net_class_attrs[] __ro_after_init = { &dev_attr_netdev_group.attr, &dev_attr_type.attr, @@ -653,6 +711,8 @@ static struct attribute *net_class_attrs[] __ro_after_init = { &dev_attr_carrier_up_count.attr, &dev_attr_carrier_down_count.attr, &dev_attr_threaded.attr, + &dev_attr_available_timestamping_providers.attr, + &dev_attr_current_timestamping_provider.attr, NULL, }; ATTRIBUTE_GROUPS(net_class);