On Fri, 15 May 2026 23:19:29 +0000
Mark Blasko <[email protected]> wrote:
> This patch series introduces support for GVE hardware timestamping on DQO
> queues. To support concurrent access, a mutex lock is introduced to protect
> admin queue operations. A mechanism is then added to periodically synchronize
> the NIC clock via AdminQ, and support is introduced for the read_clock ethdev
> operation. Finally, the RX datapath is updated to reconstruct full 64-bit
> timestamps from the 32-bit values in DQO descriptors.
>
> ---
> v2:
> - Patch 1: Dropped ROBUST mutex attribute.
> - Patch 3: Added adminq timestamp counter reset to gve_adminq_alloc.
> - Patch 4:
> - Removed redundant void* casts.
> - Handled alarm reschedule failures by marking timestamp stale.
> - Added transient error logging on memzone allocation failure.
> - Patch 5: Scoped read_clock ethdev operation strictly to DQO queues.
> - Patch 6:
> - Scoped timestamp offload capability advertisement strictly to
> DQO queues.
> - Predicated capability advertisement directly on memzone
> allocation.
> - Initialized mbuf_timestamp_offset to -1.
> - Added blank line separating release notes.
> ---
>
> Mark Blasko (6):
> net/gve: add thread safety to admin queue
> net/gve: add device option support for HW timestamps
> net/gve: add AdminQ command for NIC timestamps
> net/gve: add periodic NIC clock synchronization
> net/gve: support read clock ethdev op
> net/gve: reconstruct HW timestamps from DQO
>
> .mailmap | 1 +
> doc/guides/nics/features/gve.ini | 1 +
> doc/guides/nics/gve.rst | 20 ++++
> doc/guides/rel_notes/release_26_07.rst | 4 +
> drivers/net/gve/base/gve_adminq.c | 128 +++++++++++++++++----
> drivers/net/gve/base/gve_adminq.h | 29 +++++
> drivers/net/gve/base/gve_desc_dqo.h | 8 +-
> drivers/net/gve/gve_ethdev.c | 149 ++++++++++++++++++++++++-
> drivers/net/gve/gve_ethdev.h | 39 +++++++
> drivers/net/gve/gve_rx_dqo.c | 26 +++++
> 10 files changed, 383 insertions(+), 22 deletions(-)
>
Looks good, once again AI followup still found some things you need to address.
Patch 5: net/gve: support read clock ethdev op
Error: gve_read_clock is defined as static but never assigned to
any eth_dev_ops table. In v1 the patch added
".read_clock = gve_read_clock" to both gve_eth_dev_ops and
gve_eth_dev_ops_dqo. The v2 changelog notes "Scoped read_clock
ethdev operation strictly to DQO queues," which should have left
the assignment in gve_eth_dev_ops_dqo and removed it from
gve_eth_dev_ops. Instead both assignments were dropped, leaving
the function unreferenced. DPDK CI builds with -Dwerror=true, so
-Wunused-function will fail the build, and the read_clock feature
is unreachable at runtime in any case.
Restore the assignment in gve_eth_dev_ops_dqo:
static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
...
.reta_query = gve_rss_reta_query,
.read_clock = gve_read_clock,
};
Warning: (unchanged from v1) gve_read_clock and the periodic
gve_read_nic_clock alarm callback both issue
GVE_ADMINQ_REPORT_NIC_TIMESTAMP into the single shared DMA buffer
priv->nic_ts_report, then read it after gve_adminq_execute_cmd
has released adminq_lock. If gve_read_clock is preempted between
gve_adminq_report_nic_timestamp returning and the be64_to_cpu
read, the alarm callback can memset() and reissue its own
command, so the user thread will read either zero or another
command's response. The simplest fix is for gve_read_clock to
return the cached priv->last_read_nic_timestamp instead of
issuing a fresh adminq command - the 250ms periodic sync keeps
it fresh enough for .read_clock semantics. Once the dev_op
registration is restored this race becomes reachable.
