[v7,net-next,00/10] Introduce ndo_hwtstamp_get() and ndo_hwtstamp_set()

  Based on previous RFCs from Maxim Georgiev:
https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/

this series attempts to introduce new API for the hardware timestamping
control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).

I don't have any board with phylib hardware timestamping, so I would
appreciate testing (especially on lan966x, the most intricate
conversion). I was, however, able to test netdev level timestamping,
because I also have some more unsubmitted conversions in progress:

https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7

I hope that the concerns expressed in the comments of previous series
were addressed, and that Köry Maincent's series:
https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
can make progress in parallel with the conversion of the rest of drivers.

Maxim Georgiev (5):
  net: add NDOs for configuring hardware timestamping
  net: add hwtstamping helpers for stackable net devices
  net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
  net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
  net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()

Vladimir Oltean (5):
  net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  net: fec: delete fec_ptp_disable_hwts()
  net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
    converted drivers

 drivers/net/bonding/bond_main.c               | 105 ++++++----
 drivers/net/ethernet/freescale/fec.h          |   6 +-
 drivers/net/ethernet/freescale/fec_main.c     |  41 ++--
 drivers/net/ethernet/freescale/fec_ptp.c      |  43 ++--
 .../ethernet/microchip/lan966x/lan966x_main.c |  61 ++++--
 .../ethernet/microchip/lan966x/lan966x_main.h |  12 +-
 .../ethernet/microchip/lan966x/lan966x_ptp.c  |  34 ++--
 .../ethernet/microchip/sparx5/sparx5_main.h   |   9 +-
 .../ethernet/microchip/sparx5/sparx5_netdev.c |  35 +++-
 .../ethernet/microchip/sparx5/sparx5_ptp.c    |  24 ++-
 drivers/net/macvlan.c                         |  34 ++--
 include/linux/net_tstamp.h                    |  28 +++
 include/linux/netdevice.h                     |  25 +++
 net/8021q/vlan_dev.c                          |  27 ++-
 net/core/dev_ioctl.c                          | 183 +++++++++++++++++-
 15 files changed, 480 insertions(+), 187 deletions(-)
  

On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
> 
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> 
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
> 
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
> 
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
> can make progress in parallel with the conversion of the rest of drivers.
> 

This series looks good to me, nice cleanup and reducing some boiler
plate code is excellent.

I'd like to convert the Intel drivers too, but I am not sure when I can
commit to doing that as I have a lot on my plate presently.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Maxim Georgiev (5):
>   net: add NDOs for configuring hardware timestamping
>   net: add hwtstamping helpers for stackable net devices
>   net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> 
> Vladimir Oltean (5):
>   net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: fec: delete fec_ptp_disable_hwts()
>   net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
>     converted drivers
> 
>  drivers/net/bonding/bond_main.c               | 105 ++++++----
>  drivers/net/ethernet/freescale/fec.h          |   6 +-
>  drivers/net/ethernet/freescale/fec_main.c     |  41 ++--
>  drivers/net/ethernet/freescale/fec_ptp.c      |  43 ++--
>  .../ethernet/microchip/lan966x/lan966x_main.c |  61 ++++--
>  .../ethernet/microchip/lan966x/lan966x_main.h |  12 +-
>  .../ethernet/microchip/lan966x/lan966x_ptp.c  |  34 ++--
>  .../ethernet/microchip/sparx5/sparx5_main.h   |   9 +-
>  .../ethernet/microchip/sparx5/sparx5_netdev.c |  35 +++-
>  .../ethernet/microchip/sparx5/sparx5_ptp.c    |  24 ++-
>  drivers/net/macvlan.c                         |  34 ++--
>  include/linux/net_tstamp.h                    |  28 +++
>  include/linux/netdevice.h                     |  25 +++
>  net/8021q/vlan_dev.c                          |  27 ++-
>  net/core/dev_ioctl.c                          | 183 +++++++++++++++++-
>  15 files changed, 480 insertions(+), 187 deletions(-)
>
  

On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote:
> On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
> > Based on previous RFCs from Maxim Georgiev:
> > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
> > 
> > this series attempts to introduce new API for the hardware timestamping
> > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> > 
> > I don't have any board with phylib hardware timestamping, so I would
> > appreciate testing (especially on lan966x, the most intricate
> > conversion). I was, however, able to test netdev level timestamping,
> > because I also have some more unsubmitted conversions in progress:
> > 
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
> > 
> > I hope that the concerns expressed in the comments of previous series
> > were addressed, and that Köry Maincent's series:
> > https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
> > can make progress in parallel with the conversion of the rest of drivers.
> > 
> 
> This series looks good to me, nice cleanup and reducing some boiler
> plate code is excellent.
> 
> I'd like to convert the Intel drivers too, but I am not sure when I can
> commit to doing that as I have a lot on my plate presently.
> 
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks for the review. The conversion of Intel drivers is in the Github
link I had posted.
  

On 7/13/2023 3:33 PM, Vladimir Oltean wrote:
> On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote:
>> On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
>>> Based on previous RFCs from Maxim Georgiev:
>>> https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
>>>
>>> this series attempts to introduce new API for the hardware timestamping
>>> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>>>
>>> I don't have any board with phylib hardware timestamping, so I would
>>> appreciate testing (especially on lan966x, the most intricate
>>> conversion). I was, however, able to test netdev level timestamping,
>>> because I also have some more unsubmitted conversions in progress:
>>>
>>> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>>>
>>> I hope that the concerns expressed in the comments of previous series
>>> were addressed, and that Köry Maincent's series:
>>> https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
>>> can make progress in parallel with the conversion of the rest of drivers.
>>>
>>
>> This series looks good to me, nice cleanup and reducing some boiler
>> plate code is excellent.
>>
>> I'd like to convert the Intel drivers too, but I am not sure when I can
>> commit to doing that as I have a lot on my plate presently.
>>
>> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> 
> Thanks for the review. The conversion of Intel drivers is in the Github
> link I had posted.

Oh nice!
  

The 07/13/2023 15:18, Vladimir Oltean wrote:

Hi Vladimir,

> 
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
> 
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> 
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
> 
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7

I have tested this patch series on lan966x. In both cases, when there is
a PHY that supports HW timestamping and when the isn't a PHY that
supports HW timestamping. In both cases it behaves as expected. Thanks!
Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
> can make progress in parallel with the conversion of the rest of drivers.
> 
> Maxim Georgiev (5):
>   net: add NDOs for configuring hardware timestamping
>   net: add hwtstamping helpers for stackable net devices
>   net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> 
> Vladimir Oltean (5):
>   net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: fec: delete fec_ptp_disable_hwts()
>   net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
>     converted drivers
> 
>  drivers/net/bonding/bond_main.c               | 105 ++++++----
>  drivers/net/ethernet/freescale/fec.h          |   6 +-
>  drivers/net/ethernet/freescale/fec_main.c     |  41 ++--
>  drivers/net/ethernet/freescale/fec_ptp.c      |  43 ++--
>  .../ethernet/microchip/lan966x/lan966x_main.c |  61 ++++--
>  .../ethernet/microchip/lan966x/lan966x_main.h |  12 +-
>  .../ethernet/microchip/lan966x/lan966x_ptp.c  |  34 ++--
>  .../ethernet/microchip/sparx5/sparx5_main.h   |   9 +-
>  .../ethernet/microchip/sparx5/sparx5_netdev.c |  35 +++-
>  .../ethernet/microchip/sparx5/sparx5_ptp.c    |  24 ++-
>  drivers/net/macvlan.c                         |  34 ++--
>  include/linux/net_tstamp.h                    |  28 +++
>  include/linux/netdevice.h                     |  25 +++
>  net/8021q/vlan_dev.c                          |  27 ++-
>  net/core/dev_ioctl.c                          | 183 +++++++++++++++++-
>  15 files changed, 480 insertions(+), 187 deletions(-)
> 
> --
> 2.34.1
>
  

On Thu, Jul 13, 2023 at 6:19 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
>
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
>
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/
> can make progress in parallel with the conversion of the rest of drivers.
>
> Maxim Georgiev (5):
>   net: add NDOs for configuring hardware timestamping
>   net: add hwtstamping helpers for stackable net devices
>   net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>   net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>
> Vladimir Oltean (5):
>   net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: fec: delete fec_ptp_disable_hwts()
>   net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
>   net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
>     converted drivers
>
>  drivers/net/bonding/bond_main.c               | 105 ++++++----
>  drivers/net/ethernet/freescale/fec.h          |   6 +-
>  drivers/net/ethernet/freescale/fec_main.c     |  41 ++--
>  drivers/net/ethernet/freescale/fec_ptp.c      |  43 ++--
>  .../ethernet/microchip/lan966x/lan966x_main.c |  61 ++++--
>  .../ethernet/microchip/lan966x/lan966x_main.h |  12 +-
>  .../ethernet/microchip/lan966x/lan966x_ptp.c  |  34 ++--
>  .../ethernet/microchip/sparx5/sparx5_main.h   |   9 +-
>  .../ethernet/microchip/sparx5/sparx5_netdev.c |  35 +++-
>  .../ethernet/microchip/sparx5/sparx5_ptp.c    |  24 ++-
>  drivers/net/macvlan.c                         |  34 ++--
>  include/linux/net_tstamp.h                    |  28 +++
>  include/linux/netdevice.h                     |  25 +++
>  net/8021q/vlan_dev.c                          |  27 ++-
>  net/core/dev_ioctl.c                          | 183 +++++++++++++++++-
>  15 files changed, 480 insertions(+), 187 deletions(-)
>
> --
> 2.34.1
>

Vladimir, thank you for taking over and improving this patch stack!

I see you dropped the netdevsim patch:
https://www.spinics.net/lists/netdev/msg901378.html
Do you believe it's not useful any more since the rest of the
patches in the stack were tested through other means?
  

Hi Maxim,

On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote:
> Vladimir, thank you for taking over and improving this patch stack!
> 
> I see you dropped the netdevsim patch:
> https://www.spinics.net/lists/netdev/msg901378.html
> Do you believe it's not useful any more since the rest of the
> patches in the stack were tested through other means?

I just didn't consider that adding mock hardware timestamping support to
netdevsim was necessary or useful, considering the number of other driver
conversions that will have to be submitted. Just an extra, avoidable effort
for me.
  

On Fri, Jul 14, 2023 at 10:00:59AM +0200, Horatiu Vultur wrote:
> The 07/13/2023 15:18, Vladimir Oltean wrote:
> 
> Hi Vladimir,
> 
> > 
> > Based on previous RFCs from Maxim Georgiev:
> > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/
> > 
> > this series attempts to introduce new API for the hardware timestamping
> > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> > 
> > I don't have any board with phylib hardware timestamping, so I would
> > appreciate testing (especially on lan966x, the most intricate
> > conversion). I was, however, able to test netdev level timestamping,
> > because I also have some more unsubmitted conversions in progress:
> > 
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
> 
> I have tested this patch series on lan966x. In both cases, when there is
> a PHY that supports HW timestamping and when the isn't a PHY that
> supports HW timestamping. In both cases it behaves as expected. Thanks!
> Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com>

Thanks for testing! I'll apply this tag to patches:

net: add NDOs for configuring hardware timestamping
net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from converted drivers
  

On 7/17/2023 4:25 AM, Vladimir Oltean wrote:
> Hi Maxim,
> 
> On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote:
>> Vladimir, thank you for taking over and improving this patch stack!
>>
>> I see you dropped the netdevsim patch:
>> https://www.spinics.net/lists/netdev/msg901378.html
>> Do you believe it's not useful any more since the rest of the
>> patches in the stack were tested through other means?
> 
> I just didn't consider that adding mock hardware timestamping support to
> netdevsim was necessary or useful, considering the number of other driver
> conversions that will have to be submitted. Just an extra, avoidable effort
> for me.

FWIW I think its unnecessary as well.

I read through the implementation and noticed that it also used the
.get_ts_info callback by directly reporting whatever type and filter was
set via SIOCSHWTSTAMP, rather than reporting some device capability.

Obviously as a mock device there is no real capability, and that was
likely done for testing purposes. However, it would still leave the
kernel with an implementation that does not follow the expected rules
for these ioctls.

For a mock device thats not really an issue. However, I'd prefer to
avoid such in the kernel so that its not available for copying when
someone without such knowledge comes along to write a new driver.
  

On Mon, Jul 17, 2023 at 01:23:02PM -0700, Jacob Keller wrote:

> For a mock device thats not really an issue. However, I'd prefer to
> avoid such in the kernel so that its not available for copying when
> someone without such knowledge comes along to write a new driver.

+1