Re: [PATCH bpf-next V2 3/7] net: xdp: Add kfuncs to store hw metadata in xdp_buff

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

> 
> On 02/07/2025 16.58, Jesper Dangaard Brouer wrote:
> > From: Lorenzo Bianconi<lorenzo@kernel.org>
> > 
> > Introduce the following kfuncs to store hw metadata provided by the NIC
> > into the xdp_buff struct:
> > 
> > - rx-hash: bpf_xdp_store_rx_hash
> > - rx-vlan: bpf_xdp_store_rx_vlan
> > - rx-hw-ts: bpf_xdp_store_rx_ts
> > 
> > Signed-off-by: Lorenzo Bianconi<lorenzo@kernel.org>
> > Signed-off-by: Jesper Dangaard Brouer<hawk@kernel.org>
> > ---
> >   include/net/xdp.h |    5 +++++
> >   net/core/xdp.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> > 
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index bd3110fc7ef8..1ffba57714ea 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -963,12 +963,57 @@ __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx,
> [...]
> > +__bpf_kfunc int bpf_xdp_store_rx_ts(struct xdp_md *ctx, u64 ts)
> > +{
> > +	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
> > +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > +	struct skb_shared_hwtstamps *shwt = &sinfo->hwtstamps;
> > +
> > +	shwt->hwtstamp = ts;
> 
> Here we are storing into the SKB shared_info struct.  This is located at
> the SKB data tail.  Thus, this will very likely cause a cache-miss.
> 
> What about storing it into xdp->rx_meta and then starting a prefetch for
> shared_info?  (and updating patch-4 that moved it into SKB)

ack, I am fine with it. I can address it in v3.

Regards,
Lorenzo

> 
> (Reviewers should be aware that writing into the xdp_frame headroom
> (xdp->rx_meta) likely isn't a cache-miss, because all drivers does a
> prefetchw for this memory prior to running BPF-prog).
> 
> 
> > +	xdp->flags |= XDP_FLAGS_META_RX_TS;
> > +
> > +	return 0;
> > +}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]