[PATCH net] net: don't require the hwtstamp NDOs when a PHY provides timestamping

Nicolai Buchwitz <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Removing the legacy ioctl fallback made both hwtstamp NDOs mandatory. A
device that only timestamps in its PHY implements neither, so
SIOCSHWTSTAMP fails with EOPNOTSUPP before anything looks at the PHY and
PTP stops working there.

The check only ever picked the legacy path. That path is gone, so drop it
and test where the NDOs are actually called.

SIOCGHWTSTAMP is new here, not restored. The old path went through
phy_mii_ioctl(), which only handled SIOCSHWTSTAMP.

Such a device now returns -ENODEV while absent instead of -EOPNOTSUPP,
like the ones that do implement the NDOs.

Fixes: 5062245a5a7f ("net: remove legacy way to get/set HW timestamp config")
Signed-off-by: Nicolai Buchwitz <[email protected]>
---
This is an alternative to James' patch, which adds -EOPNOTSUPP stubs to
bcmgenet:

  https://lore.kernel.org/netdev/[email protected]/

A quick grep finds a few dozen more drivers pointing ndo_eth_ioctl at
phylib without either NDO. So this is better fixed in the core than by
adding the same stubs everywhere.

Left the HWTSTAMP_SOURCE_NETDEV branch alone on purpose, because hwprov
only gets installed by ethnl_set_tsconfig(), which already wants both NDOs.

Tested on a Raspberry Pi CM4 (BCM54213PE, PHC from bcm-phy-ptp) with
bcmgenet unmodified, get and set with tx_type 1 and rx_filter 12:

  before 5062245a5a7f   get EOPNOTSUPP   set ok
  without this patch    get EOPNOTSUPP   set EOPNOTSUPP
  with this patch       get ok           set ok

tsconfig keeps its own copy of the check, but that one is older than
5062245a5a7f and never worked for these devices, so IMHO this is an
extra patch for net-next.

 net/core/dev_ioctl.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index a320e264eaaf..164643140a52 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -276,19 +276,18 @@ int dev_get_hwtstamp_phylib(struct net_device *dev,
 	if (phy_is_default_hwtstamp(dev->phydev))
 		return phy_hwtstamp_get(dev->phydev, cfg);
 
+	if (!dev->netdev_ops->ndo_hwtstamp_get)
+		return -EOPNOTSUPP;
+
 	return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
 }
 
 static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 {
-	const struct net_device_ops *ops = dev->netdev_ops;
 	struct kernel_hwtstamp_config kernel_cfg = {};
 	struct hwtstamp_config cfg;
 	int err;
 
-	if (!ops->ndo_hwtstamp_get)
-		return -EOPNOTSUPP;
-
 	if (!netif_device_present(dev))
 		return -ENODEV;
 
@@ -359,12 +358,18 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 	cfg->source = phy_ts ? HWTSTAMP_SOURCE_PHYLIB : HWTSTAMP_SOURCE_NETDEV;
 
 	if (phy_ts && dev->see_all_hwtstamp_requests) {
+		if (!ops->ndo_hwtstamp_get)
+			return -EOPNOTSUPP;
+
 		err = ops->ndo_hwtstamp_get(dev, &old_cfg);
 		if (err)
 			return err;
 	}
 
 	if (!phy_ts || dev->see_all_hwtstamp_requests) {
+		if (!ops->ndo_hwtstamp_set)
+			return -EOPNOTSUPP;
+
 		err = ops->ndo_hwtstamp_set(dev, cfg, extack);
 		if (err) {
 			if (extack->_msg)
@@ -390,7 +395,6 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 
 static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 {
-	const struct net_device_ops *ops = dev->netdev_ops;
 	struct kernel_hwtstamp_config kernel_cfg = {};
 	struct netlink_ext_ack extack = {};
 	struct hwtstamp_config cfg;
@@ -413,9 +417,6 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 		return err;
 	}
 
-	if (!ops->ndo_hwtstamp_set)
-		return -EOPNOTSUPP;
-
 	if (!netif_device_present(dev))
 		return -ENODEV;
 
@@ -441,15 +442,11 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 int generic_hwtstamp_get_lower(struct net_device *dev,
 			       struct kernel_hwtstamp_config *kernel_cfg)
 {
-	const struct net_device_ops *ops = dev->netdev_ops;
 	int err;
 
 	if (!netif_device_present(dev))
 		return -ENODEV;
 
-	if (!ops->ndo_hwtstamp_get)
-		return -EOPNOTSUPP;
-
 	netdev_lock_ops(dev);
 	err = dev_get_hwtstamp_phylib(dev, kernel_cfg);
 	netdev_unlock_ops(dev);
@@ -462,15 +459,11 @@ int generic_hwtstamp_set_lower(struct net_device *dev,
 			       struct kernel_hwtstamp_config *kernel_cfg,
 			       struct netlink_ext_ack *extack)
 {
-	const struct net_device_ops *ops = dev->netdev_ops;
 	int err;
 
 	if (!netif_device_present(dev))
 		return -ENODEV;
 
-	if (!ops->ndo_hwtstamp_set)
-		return -EOPNOTSUPP;
-
 	netdev_lock_ops(dev);
 	err = dev_set_hwtstamp_phylib(dev, kernel_cfg, extack);
 	netdev_unlock_ops(dev);

base-commit: c9151088f1674fd29ff26a20f5fc687acf53a2f0
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.