Re: [PATCH net] net: ethtool: let tsconfig reach a PHY-only timestamp provider

[Nicolai Buchwitz]

On 23.9.2026 12:10, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 2 (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).
>
> - [High] TSCONFIG_SET now reports success (bare ACK, ret 0) with no
>   ETHTOOL_MSG_TSCONFIG_SET_REPLY on devices that have no hardware…
> - [Medium] With ethnl_set_tsconfig_validate() gone, 
> ethnl_set_tsconfig()
>   is reachable on netdevs that implement neither ndo_hwtstamp_get nor…
> - [Medium] net: ethtool: TSCONFIG_SET provider switch fails on devices
>   lacking ndo_hwtstamp_set
>
> Pre-existing issues:
> - [Medium] A struct hwtstamp_provider installed into dev->hwprov by
>   TSCONFIG_SET is never freed when the netdevice is unregistered/freed…
>
> --
>
> net: ethtool: let tsconfig reach a PHY-only timestamp provider
>
> This drops the ndo_hwtstamp_get check in tsconfig_prepare_data() and 
> the
> whole ethnl_set_tsconfig_validate() gate so that TSCONFIG_GET/SET can 
> be
> served by a PHY on a MAC that implements no hwtstamp NDO. A guard is 
> added
> in dev_get_hwtstamp_phylib() before calling ndo_hwtstamp_get.
>
> Related discussion for the ioctl side:
> https://lore.kernel.org/netdev/20260918095540.34286-1-nb@xxxxxxxxxxx/
>
> > diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> > index 164643140a523..f6029f60c1dc9 100644
> > --- a/net/core/dev_ioctl.c
> > +++ b/net/core/dev_ioctl.c
> > @@ -267,7 +267,8 @@ int dev_get_hwtstamp_phylib(struct net_device 
> > *dev,
> >  		    hwprov->phydev)
> >  			return phy_hwtstamp_get(hwprov->phydev, cfg);
> >
> > -		if (hwprov->source == HWTSTAMP_SOURCE_NETDEV)
> > +		if (hwprov->source == HWTSTAMP_SOURCE_NETDEV &&
> > +		    dev->netdev_ops->ndo_hwtstamp_get)
> >  			return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
> >
> >  		return -EOPNOTSUPP;
>
> [Severity: Medium]
> This guard makes the read side safe, but can the install side now latch 
> a
> HWTSTAMP_SOURCE_NETDEV provider on a device that has no hwtstamp NDOs 
> at
> all, with no way back?

Ack, I will guard the install path in v2.

> [...]

> [Severity: Medium]
> With ethnl_set_tsconfig_validate() gone, a TSCONFIG_SET that selects
> the PHY provider now reaches ethnl_set_tsconfig() on a MAC without
> hwtstamp NDOs. Does the provider switch itself ever succeed there
> unless the PHY happens to be the default timestamper?

Will add a check in v2.

> [...]

> [Severity: High]
> Does removing this gate turn TSCONFIG_SET into a silent success on 
> devices
> that have no timestamp provider at all?

Ack, this is an ungly regression and would be the opposite of what 
e8ab231782e9
had fixed. I will narrow down the gate.

> [...]

> [Severity: Medium]
> This isn't a bug introduced by this patch, but the dev->hwprov lifetime
> looks unbalanced and dropping .set_validate widens the set of devices 
> that
> can reach the install path.

Agree on the issue, but would probably fix this in a separate patch.

> [...]

pw-bot: cr