Subject: Re: [PATCH v13 bpf-next 05/18] net: xdp: add xdp_update_skb_shared_info utility routine

From: Lorenzo Bianconi <[email-masked]>
Date: Tue, 7 Sep 2021 15:44:44 +0200

[...]
> 
> Do we need to clear gso_type here as it is shared/union with xdp_frags_size
> ?
> (see below ... it is already cleared before call)
> 
> 
> > +}
> > +
> >   /* Avoids inlining WARN macro in fast-path */
> >   void xdp_warn(const char *msg, const char *func, const int line);
> >   #define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__)
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index cc92ccb38432..504be3ce3ca9 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -531,8 +531,20 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
> >   					   struct sk_buff *skb,
> >   					   struct net_device *dev)
> >   {
> > +	unsigned int frag_size, frag_tsize;
> >   	unsigned int headroom, frame_size;
> >   	void *hard_start;
> > +	u8 nr_frags;
> > +
> > +	/* xdp multi-buff frame */
> > +	if (unlikely(xdp_frame_is_mb(xdpf))) {
> > +		struct skb_shared_info *sinfo;
> > +
> > +		sinfo = xdp_get_shared_info_from_frame(xdpf);
> > +		frag_tsize = sinfo->xdp_frags_tsize;
> > +		frag_size = sinfo->xdp_frags_size;
> > +		nr_frags = sinfo->nr_frags;
> > +	}
> >   	/* Part of headroom was reserved to xdpf */
> >   	headroom = sizeof(*xdpf) + xdpf->headroom;
> > @@ -552,6 +564,11 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
> >   	if (xdpf->metasize)
> >   		skb_metadata_set(skb, xdpf->metasize);
> > +	if (unlikely(xdp_frame_is_mb(xdpf)))
> > +		xdp_update_skb_shared_info(skb, nr_frags,
> > +					   frag_size, frag_tsize,
> > +					   xdp_frame_is_frag_pfmemalloc(xdpf));
> > +
> 
> There is a build_skb_around() call before this call, which via
> __build_skb_around() will clear top part of skb_shared_info.
> (Thus, clearing gso_type not needed ... see above)

yes, this is why I need save sinfo->nr_frags, sinfo->xdp_frags_size, ...

Regards,
Lorenzo

> 
> >   	/* Essential SKB info: protocol and skb->dev */
> >   	skb->protocol = eth_type_trans(skb, dev);
> 
> 
> Acked-by: Jesper Dangaard Brouer <[email-masked]>
>