[PATCH 0/6] net/gve: add hardware timestamping support

[Mark Blasko]

Thanks for the review, and apologies for the accidental spam
with v1! We'll address most of the initial comments exactly as
suggested in v2, but have a few follow-ups:

> [PATCH 4/6] net/gve: add periodic NIC clock synchronization
>
> gve_setup_nic_timestamp() discards the gve_alloc_nic_ts_report() error.
> If the memzone allocation fails, priv->nic_timestamp_supported (set in
> 2/6) remains true, so dev_info_get() in 6/6 will advertise
> RTE_ETH_RX_OFFLOAD_TIMESTAMP and read_clock() in 5/6 will return -EIO
> on every call. Clear priv->nic_timestamp_supported on alloc failure so
> the capability is advertised honestly.

To avoid dishonest capability advertisements and -EIO loops while
preserving hardware capability tracking, we plan to leave
priv->nic_timestamp_supported untouched (as it represents immutable
hardware state). Instead, in v2 we'll predicate capability
advertisement in gve_dev_info_get() directly on memzone allocation
(priv->nic_ts_report_mz) and add an explicit error log advising a
device reset for transient recovery.

On Wed, May 13, 2026 at 7:41 AM Stephen Hemminger
<stephen at networkplumber.org> wrote:
> Patch 5: net/gve: support read clock ethdev op
>
> Warning: gve_read_clock and the periodic alarm callback
> gve_read_nic_clock share a single DMA buffer (priv->nic_ts_report)
> for the GVE_ADMINQ_REPORT_NIC_TIMESTAMP response. The adminq_lock
> introduced in patch 1 serializes the adminq command exchange, but
> is released inside gve_adminq_execute_cmd before either caller
> reads the buffer:
>
>     err = gve_adminq_report_nic_timestamp(priv,
>                                 priv->nic_ts_report_mz->iova);
>     /* lock has already been released here */
>     if (err != 0)
>         return err;
>     ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
>
> If the user thread (gve_read_clock) is preempted between returning
> from gve_adminq_report_nic_timestamp and the be64_to_cpu read, the
> alarm callback can run, memset() the buffer to zero, take the
> adminq lock, issue its own command, and either leave its own
> response in the buffer or be mid-DMA when the user thread reads.
> The user thread will then read zero or a different command's
> response.
>
> The simplest fix is to return the cached
> priv->last_read_nic_timestamp from gve_read_clock rather than
> issuing a fresh adminq command. The periodic sync runs every
> 250ms, so the cached value is recent; the .read_clock semantics
> do not require a brand new device read. Alternatively, use a
> separate per-caller DMA buffer or extend the critical section
> to cover the buffer read.

Before returning the cached timestamp as suggested, we wanted to
clarify the API contract:
- Is the fact that .read_clock does not require a fresh device read
  documented in the DPDK specification, or is this based on typical
  application use cases? If the latter, what are those typical use
  cases?