Re: [PATCH RFC v4 0/6] virtio-net: add receive timestamp support

[Steffen Trumtrar <s.trumtrar@pengutronix.de>]

Hi Willem,

On 2026-11-01 at 09:57 -05, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> Steffen Trumtrar wrote:
> > This series picks up Willem de Bruijn's work on a virtio-net receive
> > timestamp. The device puts the time of arrival (TAI, ns) into the
> > virtio-net header so the guest can use it for PTP and RTT estimation
> > without taking the timestamp itself after the host-to-guest latency.
> > 
> > The series is an RFC. I am mostly interested in feedback on the
> > approach and on the wire format:
> 
> At a high level +1 from me.

Thanks!

> 
> Some comments inline.
> > 
> >  - The timestamp is a new field at the end of virtio_net_common_hdr
> >    after outer_nh_offset and ipsec, offset 32 (see virtio spec),
> >    negotiated with VIRTIO_NET_F_RX_TSTAMP (bit 49). Header size now 40
> >    bytes.
> >    Willem noted on v3 that virtio_net_common_hdr was not meant to be
> >    extended this way; I kept it as the simplest approach, but a
> >    separate struct for the timestamp header is an option.
> >  - Timestamping is enabled at runtime with SIOCSHWTSTAMP; all
> >    supported rx filters behave the same for now.
> >  - XDP programs can read the timestamp with
> >    bpf_xdp_metadata_rx_timestamp().
> >  - vhost-net passes the feature through; the timestamp itself is
> >    written by tun. The tun patch is only meant as a reference
> >    implementation for the device side.
> >  - The timestamp is in TAI.
> 
> Is this a requirement? It is common for PHCs to be synced to TAI,
> but not 100% the case. Does virtio need to specify this.
>

No, I don't think it has to be TAI. TAI is only what the tun reference
implementation does (ktime_get_clocktai_ns()). What the guest really
needs is to know which clock the timestamp comes from, so it can compare
it with something.

One idea for the spec change: don't fix the time base in virtio-net at
all, only say the timestamp is in nanoseconds of some device clock, and
let the device tell the driver which clock that is. virtio_rtc already
has clock ids with a type (UTC, TAI, monotonic, ...), and the Linux
driver registers a PHC for each of them (with VIRTIO_RTC_PTP).
virtio-net could carry the virtio_rtc clock id in its config space, and
the driver reports the index of that PHC in get_ts_info. That would also
fix the phc_index == -1 problem you pointed out in 2/6.

What I don't like about it: virtio-net then depends on a second device
being there, and with more than one virtio_rtc device the clock id alone
is not enough to find the right one. Without virtio_rtc the driver would
not offer hardware timestamps at all.

The alternative is a PHC in virtio-net itself (gettime over the control
vq), which is a lot more spec for one timestamp field.

Do you think the virtio_rtc way is reasonable, or is there something
simpler I'm missing?

> >    The driver reports it as
> >    SOF_TIMESTAMPING_RAW_HARDWARE without a PTP clock (phc_index -1),
> >    so userspace cannot tell which clock it refers to and may compare
> >    it against CLOCK_REALTIME (currently off by 37s). Options I see:
> >    convert to the guest's system time in the driver (correct only if
> >    host and guest agree on the TAI offset), expose a PHC so the
> >    time base is explicit, or keep the raw TAI value and leave the
> >    conversion to userspace. Which one is preferred?
> >  - Like the existing rx_hash, the XDP timestamp callback locates the
> >    header relative to xdp->data, so it must be called before
> >    bpf_xdp_adjust_head().
> > 
> > A virtio spec change is needed as well and not yet proposed.
> > 
> > Testing: the feature needs a QEMU that knows VIRTIO_NET_F_RX_TSTAMP and
> > sets the matching vnet header size; see
> > https://github.com/strumtrar/qemu/tree/v11.0.1/virtio-rx-stamps
> > 
> > Performance: I measured the receive rate with 64 byte UDP packets in
> > a nested QEMU/KVM setup with vhost-net. The variance between runs is
> > larger than any difference between the variants (without the series,
> > series without the feature, feature negotiated, feature enabled), so
> > I cannot show a measurable cost or the absence of one. Numbers from
> > bare metal will follow with a non-RFC version.
> 
> This does not seem performance sensitive to me. And it is opt-in.
> IMHO no need for extensive testing for performance regressions.
> 

Performance measurements where requested in some of the previous
versions because of the increased header size. But now, as this is
really opt-in, I'll drop that part from the cover letter.


Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |