From: Ziwei Xiao <ziweixiao@google.com>

This patch series add the support of Rx HW timestamping, which sends
adminq commands periodically to the device for clock synchronization with
the nic.

Changes:
v4:
  - release the ptp in the error path of gve_init_clock (Jakub Kicinski)
  - add two more reserved fields in gve_nic_ts_report, anticipating
    upcoming use, to align size expectations with the device from the
    start (team internal review, Shachar Raindel)
v3:
  - change the last_read to be u64 on patch 6/8 (Vadim Fedorenko)
  - update the title and commit message of patch 7/8 to show it's adding
    support for ndo functions instead of ioctls (Jakub Kicinski)
  - Utilize extack for error logging instead of dev_err (Jakub Kicinski)
v2:
  - add initial PTP device support to utilize the ptp's aux_work to
    schedule sending adminq commands periodically (Jakub Kicinski,
    Vadim Fedorenko)
  - add adminq lock patch into this patch series instead of sending out
    to net since it's only needed to resolve the conflicts between the
    upcoming PTP aux_work and the queue creation/destruction adminq
    commands (Jakub Kicinski)
  - add the missing READ_ONCE (Joe Damato)

Harshitha Ramamurthy (1):
  gve: Add initial PTP device support

John Fraker (5):
  gve: Add device option for nic clock synchronization
  gve: Add adminq command to report nic timestamp
  gve: Add rx hardware timestamp expansion
  gve: Implement ndo_hwtstamp_get/set for RX timestamping
  gve: Advertise support for rx hardware timestamping

Kevin Yang (1):
  gve: Add support to query the nic clock

Ziwei Xiao (1):
  gve: Add adminq lock for queues creation and destruction

 drivers/net/ethernet/google/Kconfig           |   1 +
 drivers/net/ethernet/google/gve/Makefile      |   4 +-
 drivers/net/ethernet/google/gve/gve.h         |  29 ++++
 drivers/net/ethernet/google/gve/gve_adminq.c  |  98 +++++++++++--
 drivers/net/ethernet/google/gve/gve_adminq.h  |  28 ++++
 .../net/ethernet/google/gve/gve_desc_dqo.h    |   3 +-
 drivers/net/ethernet/google/gve/gve_ethtool.c |  23 ++-
 drivers/net/ethernet/google/gve/gve_main.c    |  47 ++++++
 drivers/net/ethernet/google/gve/gve_ptp.c     | 137 ++++++++++++++++++
 drivers/net/ethernet/google/gve/gve_rx_dqo.c  |  26 ++++
 10 files changed, 380 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/ethernet/google/gve/gve_ptp.c

-- 
2.50.0.rc0.604.gd4ff7b7c86-goog

On Mon,  9 Jun 2025 18:40:21 +0000 Harshitha Ramamurthy wrote:
> This patch series add the support of Rx HW timestamping, which sends
> adminq commands periodically to the device for clock synchronization with
> the nic.

IIUC:
 - the driver will only register the PHC if timestamping is enabled
   (and unregister it when disabled),
 - there is no way to read the PHC from user space other than via
   packet timestamps,
 - the ethtool API does not report which PHC is associated with the
   NIC, presumably because the PHC is useless to the user space.

Do I understand that correctly? It's pretty unusual. Why not let user
read the clock?

Why unregister the PHC? I understand that you may want to cancel 
the quite aggressive timestamp refresh work, but why kill the whole
clock... Perhaps ptp_cancel_worker_sync() didn't exist when you wrote
this code?

On Tue, Jun 10, 2025 at 6:23 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon,  9 Jun 2025 18:40:21 +0000 Harshitha Ramamurthy wrote:
> > This patch series add the support of Rx HW timestamping, which sends
> > adminq commands periodically to the device for clock synchronization with
> > the nic.
>
> IIUC:
>  - the driver will only register the PHC if timestamping is enabled
>    (and unregister it when disabled),
I checked around the other drivers and it looked like they would
register the PHC when initializing the driver and keep it alive until
destroying the driver. I can change it to be that way on V5.
>  - there is no way to read the PHC from user space other than via
>    packet timestamps,
The ability to read the PHC from user space will be added in the
future patch series when adding the actual PTP support. For this
patch, it's adding the initial ptp to utilize the ptp_schedule_worker
to read the NIC clock as suggested in the previous review comments.
>  - the ethtool API does not report which PHC is associated with the
>    NIC, presumably because the PHC is useless to the user space.
>
Thanks for pointing it out. I can add the phc_index info into the
gve_get_ts_info on V5.
> Do I understand that correctly? It's pretty unusual. Why not let user
> read the clock?
>
> Why unregister the PHC? I understand that you may want to cancel
> the quite aggressive timestamp refresh work, but why kill the whole
> clock... Perhaps ptp_cancel_worker_sync() didn't exist when you wrote
> this code?
Will utilize ptp_cancel_worker_sync instead of unregistering the PHC
every time on V5.