On Thu, Dec 17, 2020 at 10:16:06AM -0800, Saeed Mahameed wrote:
> On Wed, 2020-12-16 at 16:01 +0100, Lorenzo Bianconi wrote:
> > > On Tue, 15 Dec 2020 14:47:10 +0100
> > > Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> 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.
> > > >
>
> How is this related to this series? i suggest to keep this series clean
> of vendor specific unrelated optimizations, this must be done in a
> separate patchset.
Well, Lorenzo explicitly is touching the thing that I referred to, so I
just ask if he can optimize it while he's at it.
Of course I'm fine with addressing this by myself once -next opens :)
>
>
> > > > 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
> > >
>
