On 2026-11-01 at 10:12 -05, Willem de Bruijn <[email protected]> wrote:
> Steffen Trumtrar wrote: > > For the skb, the timestamp is read from the header before the XDP > > program runs, so XDP_PASS keeps it even if the program changes the > > packet. XDP therefore works with rx timestamping enabled. > > > > Add xmo_rx_timestamp so XDP programs can read the timestamp with > > bpf_xdp_metadata_rx_timestamp(). It returns -ENODATA if timestamping > > is not enabled or the device did not timestamp the packet. Install the > > metadata ops also if only VIRTIO_NET_F_RX_TSTAMP is negotiated; > > rx_hash returns -ENODATA then. > > > > Like virtnet_xdp_rx_hash(), the callback finds the header relative to > > xdp->data, so it has to be called before bpf_xdp_adjust_head(). > > > > Signed-off-by: Steffen Trumtrar <[email protected]> > > Assisted-by: Claude:claude-opus-5-5 > > --- > > drivers/net/virtio_net.c | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index 23af10c613b48..e79b65653fad7 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -6848,8 +6848,30 @@ static int virtnet_xdp_rx_hash(const struct xdp_md > > *_ctx, u32 *hash, > > return 0; > > } > > > > +static int virtnet_xdp_rx_timestamp(const struct xdp_md *_ctx, u64 > > *timestamp) > > +{ > > + const struct xdp_buff *xdp = (void *)_ctx; > > + const struct virtio_net_common_hdr *hdr; > > + struct virtnet_info *vi; > > + u64 tstamp; > > + > > + vi = netdev_priv(xdp->rxq->dev); > > + if (!READ_ONCE(vi->hwts_rx_en)) > > + return -ENODATA; > > + > > + hdr = (const struct virtio_net_common_hdr *)(xdp->data - vi->hdr_len); > > Reading `hdr->tstamp` in place from `xdp->data - vi->hdr_len` violates > the XDP > metadata area contract (`Documentation/networking/xdp-rx-metadata.rst`: > drivers > must copy device-reserved metadata out of `[data_hard_start, data)` before > presenting the frame to XDP) and breaks standard XDP metadata consumers: > - `hdr->tstamp` sits at `[xdp->data - 8, xdp->data)`. Standard XDP > programs—such > as `tools/testing/selftests/bpf/progs/xdp_hw_metadata.c`—call > `bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta))` first, > initialize > fields in `meta` (e.g. `meta->hint_valid = 0` at `[xdp->data - 4, > xdp->data)`), > and only then call `bpf_xdp_metadata_rx_timestamp(ctx, > &meta->rx_timestamp)`. > Initializing `meta` overwrites `hdr->tstamp` (and `hdr->hash_v1_hdr`) > before > this kfunc runs. > - Calling `bpf_xdp_adjust_head()` before reading metadata also shifts > `xdp->data`, > reading arbitrary bytes at `xdp->data - vi->hdr_len`. > > Wrap `struct xdp_buff` in a driver struct on the stack (`container_of`, > and in > `struct xdp_buff_xsk`'s `cb` verified with `XSK_CHECK_PRIV_TYPE()` for > AF_XDP, > matching `gve`/`igc`/`ice`/`mlx5`) and stash `tstamp` (already extracted > in > `receive_buf()` and `virtnet_receive_xsk_buf()` before XDP runs) and the > hash > fields before `bpf_prog_run_xdp()`. Agreed, Sashiko complained about the same thing. I'll stash the timestamp in a wrapper around xdp_buff before the program runs. 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 |
