ixgbe: use xdp_buff to simplify call path

Alex, is this what you are thinking w.r.t. pushing the xdp_buff through the ixgbe skb build routines. It seems to look OK but only lightly tested for now.

I hvaen't decided if I want to push it into the first set of patches or as a 4th patch.

Signed-off-by John Fastabend <[email protected]>

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   95 +++++++++++--------------
 1 file changed, 41 insertions(+), 54 deletions(-)

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +3 -4

index 49f7eb6..db89588 100644
@@ -2114,23 +2114,26 @@ static void ixgbe_put_rx_buffer(struct ixgbe_ring *rx_ring,
 
 static struct sk_buff *ixgbe_construct_skb(struct ixgbe_ring *rx_ring,
                                           struct ixgbe_rx_buffer *rx_buffer,
-                                          union ixgbe_adv_rx_desc *rx_desc,
-                                          unsigned int headroom,
-                                          unsigned int size)
+                                          struct xdp_buff *xdp,
+                                          union ixgbe_adv_rx_desc *rx_desc)
 {
-       void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
+       unsigned int size = xdp->data_end - xdp->data;
 #if (PAGE_SIZE < 8192)
        unsigned int truesize = ixgbe_rx_pg_size(rx_ring) / 2;

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +2 -3

        /* prefetch first cache line of first page */
-       prefetch(va);
+       prefetch(xdp->data);
 #if L1_CACHE_BYTES < 128
-       prefetch(va + L1_CACHE_BYTES);
+       prefetch(xdp->data + L1_CACHE_BYTES);
 #endif
 
        /* allocate a skb to store the frags */

@@ -2138,14 +2141,12 @@ static struct sk_buff *ixgbe_construct_skb(struct ixgbe_ring *rx_ring,
        if (unlikely(!skb))
                return NULL;
 
-       off_page = IXGBE_SKB_PAD - headroom;

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +29 -43

 #if (PAGE_SIZE < 8192)
                rx_buffer->page_offset ^= truesize;

@@ -2153,7 +2154,7 @@ static struct sk_buff *ixgbe_construct_skb(struct ixgbe_ring *rx_ring,
                rx_buffer->page_offset += truesize;
 #endif
        } else {
-               memcpy(__skb_put(skb, size), va - off_page,
+               memcpy(__skb_put(skb, size), xdp->data,
                       ALIGN(size, sizeof(long)));
                rx_buffer->pagecnt_bias++;
        }

@@ -2163,11 +2164,9 @@ static struct sk_buff *ixgbe_construct_skb(struct ixgbe_ring *rx_ring,
 
 static struct sk_buff *ixgbe_build_skb(struct ixgbe_ring *rx_ring,
                                       struct ixgbe_rx_buffer *rx_buffer,
-                                      union ixgbe_adv_rx_desc *rx_desc,
-                                      unsigned int headroom,
-                                      unsigned int size)
+                                      struct xdp_buff *xdp,
+                                      union ixgbe_adv_rx_desc *rx_desc)
 {
-       void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
 #if (PAGE_SIZE < 8192)
        unsigned int truesize = ixgbe_rx_pg_size(rx_ring) / 2;
 #else

@@ -2177,19 +2176,19 @@ static struct sk_buff *ixgbe_build_skb(struct ixgbe_ring *rx_ring,
        struct sk_buff *skb;
 
        /* prefetch first cache line of first page */
-       prefetch(va);
+       prefetch(xdp->data);
 #if L1_CACHE_BYTES < 128
-       prefetch(va + L1_CACHE_BYTES);
+       prefetch(xdp->data + L1_CACHE_BYTES);
 #endif
 
        /* build an skb to around the page buffer */
-       skb = build_skb(va - IXGBE_SKB_PAD, truesize);
+       skb = build_skb(xdp->data_hard_start, truesize);
        if (unlikely(!skb))
                return NULL;
 
        /* update pointers within the skb to store the data */
-       skb_reserve(skb, headroom);
-       __skb_put(skb, size);
+       skb_reserve(skb, xdp->data - xdp->data_hard_start);
+       __skb_put(skb, xdp->data_end - xdp->data);
 
        /* record DMA address if this is the start of a chain of buffers */
        if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))

@@ -2214,14 +2213,10 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,
 
 static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
                                     struct ixgbe_ring  *rx_ring,
-                                    struct ixgbe_rx_buffer *rx_buffer,
-                                    unsigned int *headroom,
-                                    unsigned int *size)
+                                    struct xdp_buff *xdp)
 {
        int result = IXGBE_XDP_PASS;
        struct bpf_prog *xdp_prog;
-       struct xdp_buff xdp;
-       void *addr;
        u32 act;
 
        rcu_read_lock();

@@ -2230,30 +2225,12 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
        if (!xdp_prog)
                goto xdp_out;
 
-       addr = page_address(rx_buffer->page) + rx_buffer->page_offset;
-       xdp.data_hard_start = addr - *headroom;
-       xdp.data = addr;
-       xdp.data_end = addr + *size;
-
-       act = bpf_prog_run_xdp(xdp_prog, &xdp);
+       act = bpf_prog_run_xdp(xdp_prog, xdp);
        switch (act) {
        case XDP_PASS:
-               *headroom = xdp.data - xdp.data_hard_start;
-               *size = xdp.data_end - xdp.data;
                return IXGBE_XDP_PASS;
        case XDP_TX:
-               result = ixgbe_xmit_xdp_ring(adapter, &xdp);
-               if (result == IXGBE_XDP_TX) {
-#if (PAGE_SIZE < 8192)
-                       rx_buffer->page_offset ^= ixgbe_rx_pg_size(rx_ring) / 2;
-#else
-                       rx_buffer->page_offset += ring_uses_build_skb(rx_ring) ?
-                                       SKB_DATA_ALIGN(IXGBE_SKB_PAD + size) :
-                                       SKB_DATA_ALIGN(size);
-#endif
-               } else {
-                       rx_buffer->pagecnt_bias++; /* give page back */
-               }
+               result = ixgbe_xmit_xdp_ring(adapter, xdp);
                break;
        default:
                bpf_warn_invalid_xdp_action(act);

@@ -2261,7 +2238,6 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
                trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
                /* fallthrough -- handle aborts by dropping packet */
        case XDP_DROP:
-               rx_buffer->pagecnt_bias++; /* give page back */
                result = IXGBE_XDP_CONSUMED;
                break;
        }

@@ -2296,10 +2272,10 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
        u16 cleaned_count = ixgbe_desc_unused(rx_ring);
 
        while (likely(total_rx_packets < budget)) {
-               unsigned int headroom = ixgbe_rx_offset(rx_ring);
                union ixgbe_adv_rx_desc *rx_desc;
                struct ixgbe_rx_buffer *rx_buffer;
                struct sk_buff *skb;
+               struct xdp_buff xdp;
                unsigned int size;
 
                /* return some buffers to hardware, one at a time is too slow */

@@ -2320,20 +2296,31 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
                dma_rmb();
 
                rx_buffer = ixgbe_get_rx_buffer(rx_ring, rx_desc, &skb, size);
+               xdp.data = page_address(rx_buffer->page) + rx_buffer->page_offset;
+               xdp.data_hard_start = xdp.data - ixgbe_rx_offset(rx_ring);
+               xdp.data_end = xdp.data + size;
 
-               skb = ixgbe_run_xdp(adapter, rx_ring, rx_buffer,
-                                   &headroom, &size);
+               skb = ixgbe_run_xdp(adapter, rx_ring, &xdp);
                if (IS_ERR(skb)) { /* XDP consumed buffer */
+                       if (PTR_ERR(skb) == -IXGBE_XDP_TX) {
+#if (PAGE_SIZE < 8192)
+                               rx_buffer->page_offset ^= ixgbe_rx_pg_size(rx_ring) / 2;
+#else
+                               rx_buffer->page_offset += ring_uses_build_skb(rx_ring) ?
+                                               SKB_DATA_ALIGN(IXGBE_SKB_PAD + size) :
+                                               SKB_DATA_ALIGN(size);
+#endif
+                       } else {
+                               rx_buffer->pagecnt_bias++; /* give page back */
+                       }
                        total_rx_packets++;
                        total_rx_bytes += size;
                } else if (skb) {
                        ixgbe_add_rx_frag(rx_ring, rx_buffer, skb, size);
                } else if (ring_uses_build_skb(rx_ring)) {
-                       skb = ixgbe_build_skb(rx_ring, rx_buffer,
-                                             rx_desc, headroom, size);
+                       skb = ixgbe_build_skb(rx_ring, rx_buffer, &xdp, rx_desc);
                } else {
-                       skb = ixgbe_construct_skb(rx_ring, rx_buffer,
-                                                 rx_desc, headroom, size);
+                       skb = ixgbe_construct_skb(rx_ring, rx_buffer, &xdp, rx_desc);
                }
 
                /* exit if we failed to retrieve a buffer */

Quoted 4 lines · John Fastabend

Alex, is this what you are thinking w.r.t. pushing the xdp_buff through the ixgbe skb build routines. It seems to look OK but only lightly tested for now.

Quoted 2 lines

I hvaen't decided if I want to push it into the first set of patches or as a 4th patch.

Yeah, this is more or less what I was thinking. Basically we just end up pulling most of the rx_buffer_info structure onto the stack and then process it there. If we folded it into the other patches it might actually make them a bit smaller since this patch is having to go through and unto a number of things those patches are doing.