Re: [PATCH RFC v4 5/6] virtio-net: add XDP rx timestamp metadata

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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 <s.trumtrar@pengutronix.de>
> 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()`.