On 28/10/2019 16:33, Jesper Dangaard Brouer wrote:

On Mon, 28 Oct 2019 13:59:21 +0000
Charles McLachlan [off-list ref] wrote:

quoted

+	/* No support yet for XDP metadata */
+	xdp_set_data_meta_invalid(&xdp);
+	xdp.data_end = xdp.data + rx_buf->len;
+	xdp.rxq = &rx_queue->xdp_rxq_info;

You can optimize this and only assign xdp_rxq_info once per NAPI.  E.g.
if you "allocate" struct xdp_buff on the callers stack, and pass it in
as a pointer.

Sadly, this function is about 5 levels deep from the NAPI loop, so I'm not sure 
that swapping one setting of a stack based pointer with many extra passings of 
a pointer will buy us anything. 

quoted

+	default:
+		bpf_warn_invalid_xdp_action(xdp_act);
+		/* Fall through */
+	case XDP_ABORTED:

You are missing a tracepoint to catch ABORTED, e.g:
  trace_xdp_exception(netdev, xdp_prog, xdp_act);

quoted

+		efx_free_rx_buffers(rx_queue, rx_buf, 1);
+		break;

You can do a /* Fall through */ to case XDP_DROP.

but not if I put the trace_xdp_exception in as well. I think we're always going 
to need two efx_free_rx_buffers calls.

quoted

+	case XDP_DROP:
+		efx_free_rx_buffers(rx_queue, rx_buf, 1);
+		break;
+	}
+
+	return xdp_act == XDP_PASS;
+}