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

[Daniel Borkmann]

On 12/15/20 2:47 PM, Lorenzo Bianconi wrote:
[...]

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 329397c60d84..61d3f5f8b7f3 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -866,10 +866,8 @@ static u32 xennet_run_xdp(struct netfront_queue *queue, 
struct page *pdata,

xdp_init_buff(xdp, XEN_PAGE_SIZE - XDP_PACKET_HEADROOM,

                      &queue->xdp_rxq);
-       xdp->data_hard_start = page_address(pdata);
-       xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
+       xdp_prepare_buff(xdp, page_address(pdata), XDP_PACKET_HEADROOM, len);
        xdp_set_data_meta_invalid(xdp);
-       xdp->data_end = xdp->data + len;

act = bpf_prog_run_xdp(prog, xdp);

        switch (act) {
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 3fb3a9aa1b71..66d8a4b317a3 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -83,6 +83,18 @@ xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct 
xdp_rxq_info *rxq)
        xdp->rxq = rxq;
  }

+static inline void

nit: maybe __always_inline

+xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start,
+                int headroom, int data_len)
+{
+       unsigned char *data = hard_start + headroom;
+
+       xdp->data_hard_start = hard_start;
+       xdp->data = data;
+       xdp->data_end = data + data_len;
+       xdp->data_meta = data;
+}
+
  /* Reserve memory area at end-of data area.
   *

For the drivers with xdp_set_data_meta_invalid(), we're basically setting 
xdp->data_meta
twice unless compiler is smart enough to optimize the first one away (did you 
double check?).
Given this is supposed to be a cleanup, why not integrate this logic as well so 
the
xdp_set_data_meta_invalid() doesn't get extra treatment?

Thanks,
Daniel