netdev - Re: [PATCH v3 bpf-next 2/2] net: xdp: introduce xdp_prepare

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201216095240.43867406@carbon>
Date:   Wed, 16 Dec 2020 09:52:40 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        ast@...nel.org, daniel@...earbox.net, alexander.duyck@...il.com,
        saeed@...nel.org, brouer@...hat.com
Subject: Re: [PATCH v3 bpf-next 2/2] net: xdp: introduce xdp_prepare_buff
 utility routine

On Tue, 15 Dec 2020 14:47:10 +0100
Lorenzo Bianconi <lorenzo.bianconi@...hat.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!

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