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

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

[Nicolai Buchwitz]

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 <nb@tipi-net.de>
---
This is an alternative to James' patch, which adds -EOPNOTSUPP stubs to
bcmgenet:

  https://lore.kernel.org/netdev/20260918030149.80398-1-jjc@jclark.com/

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

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

[Kory Maincent]

On 9/18/26 11:55, Nicolai Buchwitz wrote:
> 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 <nb@tipi-net.de>

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Incomplete fix: the ethtool-netlink tsconfig path still
  rejects PHY-only devices, so the two documented-equivalent uAPIs now…

--

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

This moves the ndo_hwtstamp_get/ndo_hwtstamp_set presence checks out of
dev_get_hwtstamp()/dev_set_hwtstamp() and the generic_hwtstamp_*_lower()
helpers, and puts them where the callbacks are actually invoked, so a
device that only timestamps in its PHY can use SIOCGHWTSTAMP and
SIOCSHWTSTAMP again.

> 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)

[Severity: Medium]
With the checks relocated here, the ioctl entry points now reach
phy_hwtstamp_get()/phy_hwtstamp_set() for a device whose MAC implements
neither NDO but whose phydev satisfies phy_is_default_hwtstamp().  The
ethtool netlink peers of those ioctls keep the old unconditional gates,
and they run before these helpers are ever called.

net/ethtool/tsconfig.c:tsconfig_prepare_data() {
	...
	if (!dev->netdev_ops->ndo_hwtstamp_get)
		return -EOPNOTSUPP;

	ret = ethnl_ops_begin(dev);
	...
	ret = dev_get_hwtstamp_phylib(dev, &cfg);
	...
}

net/ethtool/tsconfig.c:ethnl_set_tsconfig_validate() {
	const struct net_device_ops *ops = req_base->dev->netdev_ops;

	if (!ops->ndo_hwtstamp_set || !ops->ndo_hwtstamp_get)
		return -EOPNOTSUPP;

	return 1;
}

Does this leave the two interfaces disagreeing on the same hardware?  On
the bcmgenet plus BCM54213PE setup named in the changelog,
SIOCGHWTSTAMP/SIOCSHWTSTAMP would succeed after this patch, while
ETHTOOL_MSG_TSCONFIG_GET and ETHTOOL_MSG_TSCONFIG_SET would still return
-EOPNOTSUPP, even though Documentation/networking/ethtool-netlink.rst
lists those netlink operations as the replacements for the two ioctls in
its ioctl-to-netlink mapping table.

The note about this ("tsconfig keeps its own copy of the check ... IMHO
this is an extra patch for net-next") sits below the --- separator, so it
does not end up in git history.  Could it be moved into the commit message
so that maintainers and stable backporters can see that PHY-only
timestamping devices are only fixed on the ioctl path?

One detail for whoever narrows the tsconfig gates later: the fix is not a
mechanical copy of this one.  The HWTSTAMP_SOURCE_NETDEV branch in
dev_get_hwtstamp_phylib() still calls the callback with no NULL test:

net/core/dev_ioctl.c:dev_get_hwtstamp_phylib() {
	...
	if (hwprov->source == HWTSTAMP_SOURCE_NETDEV)
		return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
	...
}

dev->hwprov is only ever assigned in ethnl_set_tsconfig(), which is
reached through ethnl_set_tsconfig_validate(), so that branch is safe
today precisely because of the check under discussion.  Should the two
sites be changed together?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918095540.34286-1-nb%40tipi-net.de

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

[Nicolai Buchwitz]

Hi (S|Cl)ashiko

On 21.9.2026 11:57, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let 
> patchwork
> know by including "pw-bot: cr" as a separate line at the end of your 
> reply
> (one such reply per series is enough).
> 
> - [Medium] Incomplete fix: the ethtool-netlink tsconfig path still
>   rejects PHY-only devices, so the two documented-equivalent uAPIs now…
> 
> --
> 
> [...]

> [Severity: Medium]
> With the checks relocated here, the ioctl entry points now reach
> phy_hwtstamp_get()/phy_hwtstamp_set() for a device whose MAC implements
> neither NDO but whose phydev satisfies phy_is_default_hwtstamp().  The
> ethtool netlink peers of those ioctls keep the old unconditional gates,
> and they run before these helpers are ever called.
> 
> net/ethtool/tsconfig.c:tsconfig_prepare_data() {
> 	...
> 	if (!dev->netdev_ops->ndo_hwtstamp_get)
> 		return -EOPNOTSUPP;
> 
> 	ret = ethnl_ops_begin(dev);
> 	...
> 	ret = dev_get_hwtstamp_phylib(dev, &cfg);
> 	...
> }
> 
> net/ethtool/tsconfig.c:ethnl_set_tsconfig_validate() {
> 	const struct net_device_ops *ops = req_base->dev->netdev_ops;
> 
> 	if (!ops->ndo_hwtstamp_set || !ops->ndo_hwtstamp_get)
> 		return -EOPNOTSUPP;
> 
> 	return 1;
> }
> 
> Does this leave the two interfaces disagreeing on the same hardware?  
> On

Yes, but they already disagreed before this patch (EOPNOTSUPP).

The ioctls are the regression and AFAIU TSCONFIG_GET/SET never worked
here. Both gates are part of 6e9e2eed4f39 (~ a year before 
5062245a5a7f).

> the bcmgenet plus BCM54213PE setup named in the changelog,
> SIOCGHWTSTAMP/SIOCSHWTSTAMP would succeed after this patch, while
> ETHTOOL_MSG_TSCONFIG_GET and ETHTOOL_MSG_TSCONFIG_SET would still 
> return
> -EOPNOTSUPP, even though Documentation/networking/ethtool-netlink.rst
> lists those netlink operations as the replacements for the two ioctls 
> in
> its ioctl-to-netlink mapping table.
> 
> The note about this ("tsconfig keeps its own copy of the check ... IMHO
> this is an extra patch for net-next") sits below the --- separator, so 
> it
> does not end up in git history.  Could it be moved into the commit 
> message
> so that maintainers and stable backporters can see that PHY-only
> timestamping devices are only fixed on the ioctl path?

That would suggest the regression is half fixed. It isn't, the tsconfig
gap is a separate and older bug. I placed it deliberately below ---.

> 
> One detail for whoever narrows the tsconfig gates later: the fix is not 
> a
> mechanical copy of this one.  The HWTSTAMP_SOURCE_NETDEV branch in
> dev_get_hwtstamp_phylib() still calls the callback with no NULL test:
> 
> net/core/dev_ioctl.c:dev_get_hwtstamp_phylib() {
> 	...
> 	if (hwprov->source == HWTSTAMP_SOURCE_NETDEV)
> 		return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
> 	...
> }
> 
> dev->hwprov is only ever assigned in ethnl_set_tsconfig(), which is
> reached through ethnl_set_tsconfig_validate(), so that branch is safe
> today precisely because of the check under discussion.  Should the two
> sites be changed together?

No, IMHO not here. It would change change behaviour that never worked.
Thus I would suggest fixing this with a follow-up to net-next.

> 
> [ ... ]

Thanks,
Nicolai