Re: [RFC 0/1] ethdev: per-packet Tx timestamp slot management

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 18 Aug 2026 00:54:14 +0530
Rajesh Kumar <rajesh3.kumar@intel.com> wrote:

> The current DPDK ethdev time synchronization framework is architected
> around a single, shared hardware latch. The existing API,
> `rte_eth_timesync_read_tx_timestamp()`, assumes a serialization model
> where only one TX timestamp is outstanding at any given time.
> 
> This model creates severe limitations for modern high-throughput network
> interface cards (NICs). When multiple packets requiring precise
> transmit timestamps are sent concurrently, the shared latch becomes a
> race-condition bottleneck. It makes timestamp retrieval unreliable and
> drops accuracy. Furthermore, Poll Mode Drivers (PMDs) backed by
> hardware that supports independent, per-packet timestamping slots have
> no way to expose this capability to the user.
> 
> To solve this, this RFC introduces a formal slot-based lifecycle API
> for per-packet Tx timestamp management. The API decouples timestamp
> tracking from the global latch model, enabling true asynchronous,
> parallel hardware timestamping.


Lots of reasonable AI feedback to the design.

Review of the RFC. Design issues first since that's what they're asking for, then code defects.

Design

    No capability discovery or exhaustion semantics. Nothing reports how many slots exist, whether they're per-port or per-queue, and slot_alloc() doesn't document what it returns when slots run out (-ENOSPC? -EAGAIN?). That's the first thing an application hits. Needs a rte_eth_dev_info field or query, and a defined out-of-slots errno.
    Queue asymmetry: alloc() takes tx_queue_id but read() and release() don't. Either slot_id is port-global (then why does alloc need the queue?) or it's per-queue (then read/release are ambiguous). Pick one and document it. Also tx_queue_id is never validated against nb_tx_queues in the ethdev layer.
    Interaction with the existing mechanism is undefined. Does the app still set RTE_MBUF_F_TX_IEEE1588_TMST? Can the legacy latch API and the slot API coexist on one port? PMDs today key tx timestamping off that flag; the RFC needs to say what supersedes what.
    Fast-path cost contradicts the stated motivation. The cover letter argues high-throughput concurrent timestamping, but the lifecycle is three dev_ops indirect calls plus a dynfield write per packet, all through the slow path. Fine for PTP rates; if the claim is more than that, alloc/release want burst variants or the intended rate should be stated.
    cycles_ns is self-contradictory: is it raw counter cycles or nanoseconds from the free-running clock? If cycles, drop the _ns and expose the frequency; if ns, call it raw_ns or free_ns. Also this struct switches to int64 ns while every other timesync call uses struct timespec; probably the right move but justify it in the cover letter.
    PMDs can't consume the dynfield as written. The offset and flag are static in rte_ethdev.c and not exposed to drivers. A PMD has to re-lookup by name, and the dynflag name only exists as a string concat inside the .c file, so drivers would hardcode "..._flag". Define the flag name macro in the header and provide a lookup helper, following the RTE_MBUF_DYNFIELD_TIMESTAMP_NAME pattern.
    Naming: rte_eth_timesync_tx_timestamp_stamp_mbuf stutters. ..._tx_slot_set_mbuf or similar.

Defects

    Silent dynflag failure in rte_eth_timesync_tx_slot_dynfield_register(). If both rte_mbuf_dynflag_register() and the lookup fail, rte_eth_timesync_tx_slot_dynflag stays 0, the function returns 0, and stamp_mbuf() ORs 0 into ol_flags and reports success. The PMD never sees the request. Must return error. Worse, the early return on offset >= 0 means the flag is never retried on subsequent calls, so one transient failure is permanent.
    Likely doesn't compile as posted: the diff adds no includes, but uses struct rte_mbuf_dynfield, rte_mbuf_dynflag_register() (needs rte_mbuf_dyn.h) and alignof (needs stdalign.h pre-C23). Check whether rte_ethdev.c already pulls those in; I don't believe it does.
    stamp_mbuf() takes port_id and ignores it, documented as "reserved for future PMD use". Either validate it or drop it; a parameter whose semantics arrive later is an API trap. Dropping it also removes the false implication that the call is port-scoped.

Nits

    stamp_mbuf() doxygen deviates from the file's param style, omits the -EINVAL return the code actually produces, and contains an em-dash. The dual_domain_timestamp struct fields lack doxygen comments.
    v1 needs rel_notes and prog_guide (ptp section) updates; RFC is fine without.