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

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:

quoted

quoted

On Tue, 15 Dec 2020 14:47:10 +0100
Lorenzo Bianconi [off-list ref] wrote:

quoted

[...]

quoted

quoted

 	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-

quoted

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 :)

quoted

quoted

quoted

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

quoted

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