Re: [PATCH net-next 01/33] xdp: add frame size to xdp_buff

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Fri, 24 Apr 2020 16:00:53 +0200
Toke Høiland-Jørgensen <toke@redhat.com> wrote:

> Jesper Dangaard Brouer <brouer@redhat.com> writes:
> 
> > XDP have evolved to support several frame sizes, but xdp_buff was not
> > updated with this information. The frame size (frame_sz) member of
> > xdp_buff is introduced to know the real size of the memory the frame is
> > delivered in.
> >
> > When introducing this also make it clear that some tailroom is
> > reserved/required when creating SKBs using build_skb().
> >
> > It would also have been an option to introduce a pointer to
> > data_hard_end (with reserved offset). The advantage with frame_sz is
> > that (like rxq) drivers only need to setup/assign this value once per
> > NAPI cycle. Due to XDP-generic (and some drivers) it's not possible to
> > store frame_sz inside xdp_rxq_info, because it's varies per packet as it
> > can be based/depend on packet length.
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>  
> 
> With one possible nit below:
> 
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>

thx

> > ---
> >  include/net/xdp.h |   13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/include/net/xdp.h b/include/net/xdp.h
> > index 40c6d3398458..1ccf7df98bee 100644
> > --- a/include/net/xdp.h
> > +++ b/include/net/xdp.h
> > @@ -6,6 +6,8 @@
> >  #ifndef __LINUX_NET_XDP_H__
> >  #define __LINUX_NET_XDP_H__
> >  
> > +#include <linux/skbuff.h> /* skb_shared_info */
> > +
> >  /**
> >   * DOC: XDP RX-queue information
> >   *
> > @@ -70,8 +72,19 @@ struct xdp_buff {
> >  	void *data_hard_start;
> >  	unsigned long handle;
> >  	struct xdp_rxq_info *rxq;
> > +	u32 frame_sz; /* frame size to deduct data_hard_end/reserved tailroom*/  
> 
> I think maybe you want to s/deduct/deduce/ here?

Okay, queued for V2.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer