Re: [PATCH v3 bpf-next 2/2] net: xdp: introduce xdp_prepare_buff utility routine

[Lorenzo Bianconi <lorenzo.bianconi@redhat.com>]

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

> On Tue, 15 Dec 2020 14:47:10 +0100
> Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
> 
> > [...]
> > > >  	xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp);
> > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > > > index 4dbbbd49c389..fcd1ca3343fb 100644
> > > > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > > > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > > > @@ -2393,12 +2393,12 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
> > > >  
> > > >  		/* retrieve a buffer from the ring */
> > > >  		if (!skb) {
> > > > -			xdp.data = page_address(rx_buffer->page) +
> > > > -				   rx_buffer->page_offset;
> > > > -			xdp.data_meta = xdp.data;
> > > > -			xdp.data_hard_start = xdp.data -
> > > > -					      i40e_rx_offset(rx_ring);
> > > > -			xdp.data_end = xdp.data + size;
> > > > +			unsigned int offset = i40e_rx_offset(rx_ring);  
> > > 
> > > I now see that we could call the i40e_rx_offset() once per napi, so can
> > > you pull this variable out and have it initialized a single time? Applies
> > > to other intel drivers as well.  
> >
> > ack, fine. I will fix in v4.
> 
> Be careful with the Intel drivers.  They have two modes (at compile
> time) depending on PAGE_SIZE in system.  In one of the modes (default
> one) you can place init of xdp.frame_sz outside the NAPI loop and init a
> single time.  In the other mode you cannot, and it becomes dynamic per
> packet.  Intel review this carefully, please!

ack. Actully I kept the xdp.frame_sz configuration in the NAPI loop but
an Intel review will be nice.

Regards,
Lorenzo

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

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