Re: [PATCH net-next RFC] Add NDOs for hardware timestamp get/set

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 1 Apr 2023 22:12:15 +0300 Vladimir Oltean wrote:
> Actually, and here is the problem, DSA will want to see the timestamping
> request with the new code path too, not just with the legacy one.
> But, in this form, the dsa_ndo_eth_ioctl() -> dsa_master_ioctl() code
> path wants to do one of two things: it either denies the configuration,
> or passes it further, unchanged, to the master's netdev_ops->ndo_eth_ioctl().
> 
> By being written around the legacy ndo_eth_ioctl(), dsa_ndo_eth_ioctl()
> places a requirement which conflicts with any attempt to convert any
> kernel driver to the new API, because basically any net device can serve
> as a DSA master, and simply put, DSA wants to see timestamping requests
> to the DSA master, old or new API.
> 
> The only "useful" piece of logic from dsa_master_ioctl() is to deny the
> hwtstamp_set operation in some cases, so it's clear that it's useless
> for dsa_master_ioctl() to have to call the master's netdev_ops->ndo_eth_ioctl()
> when dev_eth_ioctl() already would have done it anyway.

So the current patch can only convert drivers which can't be a DSA
master :( (realistically any big iron NIC for example)
It should be relatively easy to plumb both the ifr and the in-kernel
config thru all the DSA APIs and have it call the right helper, too,
tho? SMOC?

> I can make dsa_ndo_eth_ioctl() disappear and replace it with a netdev
> notifier as per this patch:
> https://lore.kernel.org/netdev/20220317225035.3475538-1-vladimir.oltean@nxp.com/
> 
> My understanding of Jakub's objection is that the scope of the
> NETDEV_ETH_IOCTL is too wide, and as such, it would need to change to
> something like NETDEV_HWTSTAMP_SET. I can make that change if that is
> the only objection, and resubmit that as preparation work for the
> ndo_hwtstamp_set() effort.

My objection to the IOCTL is that there's a lot of boilerplate that 
the drivers have to copy and that it makes it harder to do meaningful
work in the core.