xdp: introduce bulking for page_pool tx return path

PATCH Superseded Lorenzo Bianconi 3 participants 12 messages Nov 10, 2020 – Nov 11, 2020

Subsystems (8)

Networking Network drivers BPF Mellanox mlx5 core +4 more subsystems

lore

Share & tools

lore

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #0 lore raw

v5 cover xdp: introduce bulking for page_pool tx return path

Changes since v4:
- fix comments
- introduce xdp_frame_bulk_init utility routine
- compiler annotations for I-cache code layout
- move rcu_read_lock outside fast-path
- mlx5 xdp bulking code optimization

Changes since v3:
- align DEV_MAP_BULK_SIZE to XDP_BULK_QUEUE_SIZE
- refactor page_pool_put_page_bulk to avoid code duplication

Changes since v2:
- move mvneta changes in a dedicated patch

Changes since v1:
- improve comments
- rework xdp_return_frame_bulk routine logic
- move count and xa fields at the beginning of xdp_frame_bulk struct
- invert logic in page_pool_put_page_bulk for loop

Lorenzo Bianconi (5):
net: xdp: introduce bulking for xdp tx return path
net: page_pool: add bulk support for ptr_ring
net: mvneta: add xdp tx return bulking support
net: mvpp2: add xdp tx return bulking support
net: mlx5: add xdp tx return bulking support

 drivers/net/ethernet/marvell/mvneta.c         | 10 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 10 ++-
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  | 22 ++++--
 include/net/page_pool.h                       | 26 +++++++
 include/net/xdp.h                             | 17 ++++-
 net/core/page_pool.c                          | 69 ++++++++++++++++---
 net/core/xdp.c                                | 54 +++++++++++++++
 7 files changed, 191 insertions(+), 17 deletions(-)

Signature · 1 line

2.26.2

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #1 lore raw

PATCH v5 1/5 net: xdp: introduce bulking for xdp tx return path

XDP bulk APIs introduce a defer/flush mechanism to return pages belonging to the same xdp_mem_allocator object (identified via the mem.id field) in bulk to optimize I-cache and D-cache since xdp_return_frame is usually run inside the driver NAPI tx completion loop.
The bulk queue size is set to 16 to be aligned to how XDP_REDIRECT bulking works. The bulk is flushed when it is full or when mem.id changes.
xdp_frame_bulk is usually stored/allocated on the function call-stack to avoid locking penalties.
Current implementation considers only page_pool memory model.

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 include/net/xdp.h | 17 +++++++++++++-
 net/core/xdp.c    | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)

include/net/xdp.h +16 -1

index 3814fb631d52..7d48b2ae217a 100644
@@ -104,6 +104,18 @@ struct xdp_frame {
 	struct net_device *dev_rx; /* used by cpumap */
 };
 
+#define XDP_BULK_QUEUE_SIZE	16
+struct xdp_frame_bulk {
+	int count;
+	void *xa;
+	void *q[XDP_BULK_QUEUE_SIZE];
+};
+
+static __always_inline void xdp_frame_bulk_init(struct xdp_frame_bulk *bq)
+{
+	/* bq->count will be zero'ed when bq->xa gets updated */
+	bq->xa = NULL;
+}
 
 static inline struct skb_shared_info *
 xdp_get_shared_info_from_frame(struct xdp_frame *frame)

@@ -194,6 +206,9 @@ struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
 void xdp_return_frame(struct xdp_frame *xdpf);
 void xdp_return_frame_rx_napi(struct xdp_frame *xdpf);
 void xdp_return_buff(struct xdp_buff *xdp);
+void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq);
+void xdp_return_frame_bulk(struct xdp_frame *xdpf,
+			   struct xdp_frame_bulk *bq);
 
 /* When sending xdp_frame into the network stack, then there is no
  * return point callback, which is needed to release e.g. DMA-mapping

@@ -245,6 +260,6 @@ bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
 void xdp_attachment_setup(struct xdp_attachment_info *info,
 			  struct netdev_bpf *bpf);
 
-#define DEV_MAP_BULK_SIZE 16
+#define DEV_MAP_BULK_SIZE XDP_BULK_QUEUE_SIZE
 
 #endif /* __LINUX_NET_XDP_H__ */

net/core/xdp.c +59 -0

65 lines not on this page · raw message

Signature · 1 line

2.26.2

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #2 lore raw

PATCH v5 2/5 net: page_pool: add bulk support for ptr_ring

In reply to Lorenzo Bianconi

Introduce the capability to batch page_pool ptr_ring refill since it is usually run inside the driver NAPI tx completion loop.

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 include/net/page_pool.h | 26 ++++++++++++++++
 net/core/page_pool.c    | 69 +++++++++++++++++++++++++++++++++++------
 net/core/xdp.c          |  9 ++----
 3 files changed, 87 insertions(+), 17 deletions(-)

include/net/page_pool.h +26 -0

index 81d7773f96cd..b5b195305346 100644
@@ -152,6 +152,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params);
 void page_pool_destroy(struct page_pool *pool);
 void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *));
 void page_pool_release_page(struct page_pool *pool, struct page *page);
+void page_pool_put_page_bulk(struct page_pool *pool, void **data,
+			     int count);
 #else
 static inline void page_pool_destroy(struct page_pool *pool)
 {

@@ -165,6 +167,11 @@ static inline void page_pool_release_page(struct page_pool *pool,
 					  struct page *page)
 {
 }
+
+static inline void page_pool_put_page_bulk(struct page_pool *pool, void **data,
+					   int count)
+{
+}
 #endif
 
 void page_pool_put_page(struct page_pool *pool, struct page *page,

@@ -215,4 +222,23 @@ static inline void page_pool_nid_changed(struct page_pool *pool, int new_nid)
 	if (unlikely(pool->p.nid != new_nid))
 		page_pool_update_nid(pool, new_nid);
 }
+
+static inline void page_pool_ring_lock(struct page_pool *pool)
+	__acquires(&pool->ring.producer_lock)
+{
+	if (in_serving_softirq())
+		spin_lock(&pool->ring.producer_lock);
+	else
+		spin_lock_bh(&pool->ring.producer_lock);
+}
+
+static inline void page_pool_ring_unlock(struct page_pool *pool)
+	__releases(&pool->ring.producer_lock)
+{
+	if (in_serving_softirq())
+		spin_unlock(&pool->ring.producer_lock);
+	else
+		spin_unlock_bh(&pool->ring.producer_lock);
+}
+
 #endif /* _NET_PAGE_POOL_H */

net/core/page_pool.c +59 ~~-10~~

97 lines not on this page · raw message

net/core/xdp.c +2 -7

index bbaee7fdd44f..3d330ebda893 100644
@@ -393,16 +393,11 @@ EXPORT_SYMBOL_GPL(xdp_return_frame_rx_napi);
 void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq)
 {
 	struct xdp_mem_allocator *xa = bq->xa;
-	int i;
 
-	if (unlikely(!xa))
+	if (unlikely(!xa || !bq->count))
 		return;
 
-	for (i = 0; i < bq->count; i++) {
-		struct page *page = virt_to_head_page(bq->q[i]);
-
-		page_pool_put_full_page(xa->page_pool, page, false);
-	}
+	page_pool_put_page_bulk(xa->page_pool, bq->q, bq->count);
 	/* bq->xa is not cleared to save lookup, if mem.id same in next bulk */
 	bq->count = 0;
 }

Signature · 1 line

2.26.2

John Fastabend

maintainer

2020-11-11 09:30 UTC #3 lore raw

RE: [PATCH v5 net-nex 2/5] net: page_pool: add bulk support for ptr_ring

Quoted 12 lines · Lorenzo Bianconi

Introduce the capability to batch page_pool ptr_ring refill since it is usually run inside the driver NAPI tx completion loop.
Suggested-by Jesper Dangaard Brouer <[email protected]>
Co-developed-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 include/net/page_pool.h | 26 ++++++++++++++++
 net/core/page_pool.c    | 69 +++++++++++++++++++++++++++++++++++------
 net/core/xdp.c          |  9 ++----
 3 files changed, 87 insertions(+), 17 deletions(-)

[...]

Quoted 13 lines

net/core/page_pool.c +13 -0

+/* Caller must not use data area after call, as this function overwrites it */
+void page_pool_put_page_bulk(struct page_pool *pool, void **data,
+			     int count)
+{
+	int i, bulk_len = 0, pa_len = 0;
+
+	for (i = 0; i < count; i++) {
+		struct page *page = virt_to_head_page(data[i]);
+
+		page = __page_pool_put_page(pool, page, -1, false);
+		/* Approved for bulk recycling in ptr_ring cache */
+		if (page)
+			data[bulk_len++] = page;

net/core/page_pool.c void page_pool_put_page(struct page_pool *pool, struct page *page, · line 445

Quoted 10 lines

net/core/page_pool.c +10 -0

+	}
+
+	if (unlikely(!bulk_len))
+		return;
+
+	/* Bulk producer into ptr_ring page_pool cache */
+	page_pool_ring_lock(pool);
+	for (i = 0; i < bulk_len; i++) {
+		if (__ptr_ring_produce(&pool->ring, data[i]))
+			data[pa_len++] = data[i];

How about bailing out on the first error? bulk_len should be less than 16 right, so should we really keep retying hoping ring->size changes?

Quoted 4 lines

net/core/page_pool.c +4 -0

+	}
+	page_pool_ring_unlock(pool);
+
+	if (likely(!pa_len))

net/core/page_pool.c void page_pool_put_page(struct page_pool *pool, struct page *page, · line 459

Quoted 10 lines

net/core/page_pool.c +10 -0

+		return;
+
+	/* ptr_ring cache full, free pages outside producer lock since
+	 * put_page() with refcnt == 1 can be an expensive operation
+	 */
+	for (i = 0; i < pa_len; i++)
+		page_pool_return_page(pool, data[i]);
+}
+EXPORT_SYMBOL(page_pool_put_page_bulk);
+

Otherwise LGTM.

Lorenzo Bianconi

author

2020-11-11 10:43 UTC #4 lore raw

Re: [PATCH v5 net-nex 2/5] net: page_pool: add bulk support for ptr_ring

Quoted 34 lines

Quoted 12 lines · Lorenzo Bianconi
Introduce the capability to batch page_pool ptr_ring refill since it is usually run inside the driver NAPI tx completion loop.
Suggested-by Jesper Dangaard Brouer <[email protected]>
Co-developed-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 include/net/page_pool.h | 26 ++++++++++++++++
 net/core/page_pool.c    | 69 +++++++++++++++++++++++++++++++++++------
 net/core/xdp.c          |  9 ++----
 3 files changed, 87 insertions(+), 17 deletions(-)
[...]
Quoted 18 lines
net/core/page_pool.c +18 -0
+/* Caller must not use data area after call, as this function overwrites it */
+void page_pool_put_page_bulk(struct page_pool *pool, void **data,
+			     int count)
+{
+	int i, bulk_len = 0, pa_len = 0;
+
+	for (i = 0; i < count; i++) {
+		struct page *page = virt_to_head_page(data[i]);
+
+		page = __page_pool_put_page(pool, page, -1, false);
+		/* Approved for bulk recycling in ptr_ring cache */
+		if (page)
+			data[bulk_len++] = page;
+	}
+
+	if (unlikely(!bulk_len))
+		return;
+

Quoted 8 lines

net/core/page_pool.c void page_pool_put_page(struct page_pool *pool, struct page *page, · line 445
Quoted 5 lines
net/core/page_pool.c +5 -0
+	/* Bulk producer into ptr_ring page_pool cache */
+	page_pool_ring_lock(pool);
+	for (i = 0; i < bulk_len; i++) {
+		if (__ptr_ring_produce(&pool->ring, data[i]))
+			data[pa_len++] = data[i];
How about bailing out on the first error? bulk_len should be less than 16 right, so should we really keep retying hoping ring->size changes?

do you mean doing something like:

	page_pool_ring_lock(pool);
	if (__ptr_ring_full(&pool->ring)) {
		pa_len = bulk_len;
		page_pool_ring_unlock(pool);
		goto out;
	}
	...
out:
	for (i = 0; i < pa_len; i++) {
		...
	}

I do not know if it is better or not since the consumer can run in parallel. @Jesper/Ilias: any idea?

Regards,
Lorenzo

Quoted 17 lines

Quoted 5 lines
net/core/page_pool.c +4 -0
+	}
+	page_pool_ring_unlock(pool);
+
+	if (likely(!pa_len))
net/core/page_pool.c void page_pool_put_page(struct page_pool *pool, struct page *page, · line 459
Quoted 10 lines
net/core/page_pool.c +10 -0
+		return;
+
+	/* ptr_ring cache full, free pages outside producer lock since
+	 * put_page() with refcnt == 1 can be an expensive operation
+	 */
+	for (i = 0; i < pa_len; i++)
+		page_pool_return_page(pool, data[i]);
+}
+EXPORT_SYMBOL(page_pool_put_page_bulk);
+
Otherwise LGTM.

Jesper Dangaard Brouer

2020-11-11 13:00 UTC #5 lore raw

Re: [PATCH v5 net-nex 2/5] net: page_pool: add bulk support for ptr_ring

Quoted 45 lines · On Wed, 11 Nov 2020 11:43:31 +0100 Lorenzo Bianconi <[email protected]> wrote:

Quoted 34 lines
Quoted 12 lines · Lorenzo Bianconi
Introduce the capability to batch page_pool ptr_ring refill since it is usually run inside the driver NAPI tx completion loop.
Suggested-by Jesper Dangaard Brouer <[email protected]>
Co-developed-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 include/net/page_pool.h | 26 ++++++++++++++++
 net/core/page_pool.c    | 69 +++++++++++++++++++++++++++++++++++------
 net/core/xdp.c          |  9 ++----
 3 files changed, 87 insertions(+), 17 deletions(-)  
[...]
Quoted 18 lines
(quoted diff) +18 -0
+/* Caller must not use data area after call, as this function overwrites it */
+void page_pool_put_page_bulk(struct page_pool *pool, void **data,
+			     int count)
+{
+	int i, bulk_len = 0, pa_len = 0;
+
+	for (i = 0; i < count; i++) {
+		struct page *page = virt_to_head_page(data[i]);
+
+		page = __page_pool_put_page(pool, page, -1, false);
+		/* Approved for bulk recycling in ptr_ring cache */
+		if (page)
+			data[bulk_len++] = page;
+	}
+
+	if (unlikely(!bulk_len))
+		return;
+
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5 -0
+	/* Bulk producer into ptr_ring page_pool cache */
+	page_pool_ring_lock(pool);
+	for (i = 0; i < bulk_len; i++) {
+		if (__ptr_ring_produce(&pool->ring, data[i]))
+			data[pa_len++] = data[i];  
How about bailing out on the first error? bulk_len should be less than 16 right, so should we really keep retying hoping ring->size changes?
do you mean doing something like:

Quoted 11 lines

	page_pool_ring_lock(pool);
	if (__ptr_ring_full(&pool->ring)) {
		pa_len = bulk_len;
		page_pool_ring_unlock(pool);
		goto out;
	}
	...
out:
	for (i = 0; i < pa_len; i++) {
		...
	}

I think this is the change John is looking for:

net/core/page_pool.c +7 -6

index a06606f07df0..3093fe4e1cd7 100644
@@ -424,7 +424,7 @@ EXPORT_SYMBOL(page_pool_put_page);
 void page_pool_put_page_bulk(struct page_pool *pool, void **data,
                             int count)
 {
-       int i, bulk_len = 0, pa_len = 0;
+       int i, bulk_len = 0;
        bool order0 = (pool->p.order == 0);
 
        for (i = 0; i < count; i++) {

@@ -448,17 +448,18 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data,
        page_pool_ring_lock(pool);
        for (i = 0; i < bulk_len; i++) {
                if (__ptr_ring_produce(&pool->ring, data[i]))
-                       data[pa_len++] = data[i];
+                       break; /* ring_full */
        }
        page_pool_ring_unlock(pool);
 
-       if (likely(!pa_len))
+       /* Hopefully all pages was return into ptr_ring */
+       if (likely(i == bulk_len))
                return;
 
-       /* ptr_ring cache full, free pages outside producer lock since
-        * put_page() with refcnt == 1 can be an expensive operation
+       /* ptr_ring cache full, free remaining pages outside producer lock
+        * since put_page() with refcnt == 1 can be an expensive operation
         */
-       for (i = 0; i < pa_len; i++)
+       for (; i < bulk_len; i++)
                page_pool_return_page(pool, data[i]);
 }
 EXPORT_SYMBOL(page_pool_put_page_bulk);

Quoted 2 lines

I do not know if it is better or not since the consumer can run in parallel. @Jesper/Ilias: any idea?

Currently it is not very likely that the consumer runs in parallel, but is it possible. (As you experienced on your testlab with mlx5, the DMA-TX completion did run on another CPU, but I asked you to reconfigure this to have it run on same CPU, as it is suboptimal). When we (finally) support this memory type for SKBs it will be more normal to happen.

But, John is right, for ptr_ring we should exit as soon the first "produce" fails. This is because I know how ptr_ring works internally. The "consumer" will free slots in chunks of 16 slots, so it is not very likely that a slot opens up.

Quoted 17 lines

Quoted 5 lines

(quoted diff) +4 -0

+	}
+	page_pool_ring_unlock(pool);
+
+	if (likely(!pa_len))

Review

Quoted 10 lines

(quoted diff) +10 -0

+		return;
+
+	/* ptr_ring cache full, free pages outside producer lock since
+	 * put_page() with refcnt == 1 can be an expensive operation
+	 */
+	for (i = 0; i < pa_len; i++)
+		page_pool_return_page(pool, data[i]);
+}
+EXPORT_SYMBOL(page_pool_put_page_bulk);
+

Otherwise LGTM.

Signature · 4 lines

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

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #6 lore raw

PATCH v5 3/5 net: mvneta: add xdp tx return bulking support

In reply to Lorenzo Bianconi

Convert mvneta driver to xdp_return_frame_bulk APIs.

XDP_REDIRECT (upstream codepath): 275Kpps XDP_REDIRECT (upstream codepath + bulking APIs): 284Kpps

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/marvell/mvneta.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

drivers/net/ethernet/marvell/mvneta.c +9 -1

index 54b0bf574c05..183530ed4d1d 100644
@@ -1834,8 +1834,13 @@ static void mvneta_txq_bufs_free(struct mvneta_port *pp,
 				 struct netdev_queue *nq, bool napi)
 {
 	unsigned int bytes_compl = 0, pkts_compl = 0;
+	struct xdp_frame_bulk bq;
 	int i;
 
+	xdp_frame_bulk_init(&bq);
+
+	rcu_read_lock(); /* need for xdp_return_frame_bulk */
+
 	for (i = 0; i < num; i++) {
 		struct mvneta_tx_buf *buf = &txq->buf[txq->txq_get_index];
 		struct mvneta_tx_desc *tx_desc = txq->descs +

@@ -1857,9 +1862,12 @@ static void mvneta_txq_bufs_free(struct mvneta_port *pp,
 			if (napi && buf->type == MVNETA_TYPE_XDP_TX)
 				xdp_return_frame_rx_napi(buf->xdpf);
 			else
-				xdp_return_frame(buf->xdpf);
+				xdp_return_frame_bulk(buf->xdpf, &bq);
 		}
 	}
+	xdp_flush_frame_bulk(&bq);
+
+	rcu_read_unlock();
 
 	netdev_tx_completed_queue(nq, pkts_compl, bytes_compl);
 }

Signature · 1 line

2.26.2

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #7 lore raw

PATCH v5 4/5 net: mvpp2: add xdp tx return bulking support

In reply to Lorenzo Bianconi

Convert mvpp2 driver to xdp_return_frame_bulk APIs.

XDP_REDIRECT (upstream codepath): 1.79Mpps XDP_REDIRECT (upstream codepath + bulking APIs): 1.93Mpps

Tested-by Matteo Croce <[email protected]>

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +9 -1

index f6616c8933ca..3069e192d773 100644
@@ -2440,8 +2440,13 @@ static void mvpp2_txq_bufs_free(struct mvpp2_port *port,
 				struct mvpp2_tx_queue *txq,
 				struct mvpp2_txq_pcpu *txq_pcpu, int num)
 {
+	struct xdp_frame_bulk bq;
 	int i;
 
+	xdp_frame_bulk_init(&bq);
+
+	rcu_read_lock(); /* need for xdp_return_frame_bulk */
+
 	for (i = 0; i < num; i++) {
 		struct mvpp2_txq_pcpu_buf *tx_buf =
 			txq_pcpu->buffs + txq_pcpu->txq_get_index;

@@ -2454,10 +2459,13 @@ static void mvpp2_txq_bufs_free(struct mvpp2_port *port,
 			dev_kfree_skb_any(tx_buf->skb);
 		else if (tx_buf->type == MVPP2_TYPE_XDP_TX ||
 			 tx_buf->type == MVPP2_TYPE_XDP_NDO)
-			xdp_return_frame(tx_buf->xdpf);
+			xdp_return_frame_bulk(tx_buf->xdpf, &bq);
 
 		mvpp2_txq_inc_get(txq_pcpu);
 	}
+	xdp_flush_frame_bulk(&bq);
+
+	rcu_read_unlock();
 }
 
 static inline struct mvpp2_rx_queue *mvpp2_get_rx_queue(struct mvpp2_port *port,

Signature · 1 line

2.26.2

Lorenzo Bianconi

author

2020-11-10 15:38 UTC #8 lore raw

PATCH v5 5/5 net: mlx5: add xdp tx return bulking support

In reply to Lorenzo Bianconi

Convert mlx5 driver to xdp_return_frame_bulk APIs.

XDP_REDIRECT (upstream codepath): 8.9Mpps XDP_REDIRECT (upstream codepath + bulking APIs): 10.2Mpps

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  | 22 +++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c +18 -4

index ae90d533a350..2e3e78b0f333 100644
@@ -366,7 +366,8 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
 static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
 				  struct mlx5e_xdp_wqe_info *wi,
 				  u32 *xsk_frames,
-				  bool recycle)
+				  bool recycle,
+				  struct xdp_frame_bulk *bq)
 {
 	struct mlx5e_xdp_info_fifo *xdpi_fifo = &sq->db.xdpi_fifo;
 	u16 i;

@@ -379,7 +380,7 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
 			/* XDP_TX from the XSK RQ and XDP_REDIRECT */
 			dma_unmap_single(sq->pdev, xdpi.frame.dma_addr,
 					 xdpi.frame.xdpf->len, DMA_TO_DEVICE);
-			xdp_return_frame(xdpi.frame.xdpf);
+			xdp_return_frame_bulk(xdpi.frame.xdpf, bq);
 			break;
 		case MLX5E_XDP_XMIT_MODE_PAGE:
 			/* XDP_TX from the regular RQ */

@@ -397,12 +398,15 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
 
 bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq)
 {
+	struct xdp_frame_bulk bq;
 	struct mlx5e_xdpsq *sq;
 	struct mlx5_cqe64 *cqe;
 	u32 xsk_frames = 0;
 	u16 sqcc;
 	int i;
 
+	xdp_frame_bulk_init(&bq);
+
 	sq = container_of(cq, struct mlx5e_xdpsq, cq);
 
 	if (unlikely(!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state)))

@@ -434,7 +438,7 @@ bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq)
 
 			sqcc += wi->num_wqebbs;
 
-			mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, true);
+			mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, true, &bq);
 		} while (!last_wqe);
 
 		if (unlikely(get_cqe_opcode(cqe) != MLX5_CQE_REQ)) {

@@ -447,6 +451,8 @@ bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq)
 		}
 	} while ((++i < MLX5E_TX_CQ_POLL_BUDGET) && (cqe = mlx5_cqwq_get_cqe(&cq->wq)));
 
+	xdp_flush_frame_bulk(&bq);
+
 	if (xsk_frames)
 		xsk_tx_completed(sq->xsk_pool, xsk_frames);

@@ -463,8 +469,13 @@ bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq)
 
 void mlx5e_free_xdpsq_descs(struct mlx5e_xdpsq *sq)
 {
+	struct xdp_frame_bulk bq;
 	u32 xsk_frames = 0;
 
+	xdp_frame_bulk_init(&bq);
+
+	rcu_read_lock(); /* need for xdp_return_frame_bulk */
+
 	while (sq->cc != sq->pc) {
 		struct mlx5e_xdp_wqe_info *wi;
 		u16 ci;

@@ -474,9 +485,12 @@ void mlx5e_free_xdpsq_descs(struct mlx5e_xdpsq *sq)
 
 		sq->cc += wi->num_wqebbs;
 
-		mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, false);
+		mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, false, &bq);
 	}
 
+	xdp_flush_frame_bulk(&bq);
+	rcu_read_unlock();
+
 	if (xsk_frames)
 		xsk_tx_completed(sq->xsk_pool, xsk_frames);
 }

Signature · 1 line

2.26.2

Jesper Dangaard Brouer

Tested-by on v5 5/5

2020-11-10 16:19 UTC #9 lore raw

Re: [PATCH v5 net-nex 5/5] net: mlx5: add xdp tx return bulking support

Quoted 8 lines · On Tue, 10 Nov 2020 16:38:00 +0100 Lorenzo Bianconi <[email protected]> wrote:

Convert mlx5 driver to xdp_return_frame_bulk APIs.
XDP_REDIRECT (upstream codepath): 8.9Mpps XDP_REDIRECT (upstream codepath + bulking APIs): 10.2Mpps
Co-developed-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

I did most of my testing on this driver:

Tested-by Jesper Dangaard Brouer <[email protected]>

Signature · 4 lines

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

Jesper Dangaard Brouer

Acked-by on the v5 series

2020-11-10 15:51 UTC #10 lore raw

In reply to Lorenzo Bianconi

Quoted 6 lines · On Tue, 10 Nov 2020 16:37:55 +0100 Lorenzo Bianconi <[email protected]> wrote:

XDP bulk APIs introduce a defer/flush mechanism to return pages belonging to the same xdp_mem_allocator object (identified via the mem.id field) in bulk to optimize I-cache and D-cache since xdp_return_frame is usually run inside the driver NAPI tx completion loop.
Convert mvneta, mvpp2 and mlx5 drivers to xdp_return_frame_bulk APIs.

Series

Acked-by Jesper Dangaard Brouer <[email protected]>

Quoted 6 lines

Changes since v4:
- fix comments
- introduce xdp_frame_bulk_init utility routine
- compiler annotations for I-cache code layout
- move rcu_read_lock outside fast-path
- mlx5 xdp bulking code optimization

I've done a lot of these changes, and benchmarked them on mlx5, details in[1].

[1] https://github.com/xdp-project/xdp-project/blob/master/areas/mem/xdp_bulk_return01.org

Quoted 29 lines

Changes since v3:
- align DEV_MAP_BULK_SIZE to XDP_BULK_QUEUE_SIZE
- refactor page_pool_put_page_bulk to avoid code duplication
Changes since v2:
- move mvneta changes in a dedicated patch
Changes since v1:
- improve comments
- rework xdp_return_frame_bulk routine logic
- move count and xa fields at the beginning of xdp_frame_bulk struct
- invert logic in page_pool_put_page_bulk for loop
Lorenzo Bianconi (5):
net: xdp: introduce bulking for xdp tx return path
net: page_pool: add bulk support for ptr_ring
net: mvneta: add xdp tx return bulking support
net: mvpp2: add xdp tx return bulking support
net: mlx5: add xdp tx return bulking support
 drivers/net/ethernet/marvell/mvneta.c         | 10 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 10 ++-
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  | 22 ++++--
 include/net/page_pool.h                       | 26 +++++++
 include/net/xdp.h                             | 17 ++++-
 net/core/page_pool.c                          | 69 ++++++++++++++++---
 net/core/xdp.c                                | 54 +++++++++++++++
 7 files changed, 191 insertions(+), 17 deletions(-)

Signature · 4 lines

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

Lorenzo Bianconi

author

2020-11-10 15:56 UTC #11 lore raw

Quoted 21 lines

Quoted 6 lines · On Tue, 10 Nov 2020 16:37:55 +0100 Lorenzo Bianconi <[email protected]> wrote:
XDP bulk APIs introduce a defer/flush mechanism to return pages belonging to the same xdp_mem_allocator object (identified via the mem.id field) in bulk to optimize I-cache and D-cache since xdp_return_frame is usually run inside the driver NAPI tx completion loop.
Convert mvneta, mvpp2 and mlx5 drivers to xdp_return_frame_bulk APIs.
Series
Acked-by Jesper Dangaard Brouer <[email protected]>
Quoted 6 lines
Changes since v4:
- fix comments
- introduce xdp_frame_bulk_init utility routine
- compiler annotations for I-cache code layout
- move rcu_read_lock outside fast-path
- mlx5 xdp bulking code optimization

Quoted 3 lines

I've done a lot of these changes, and benchmarked them on mlx5, details in[1].
[1] https://github.com/xdp-project/xdp-project/blob/master/areas/mem/xdp_bulk_return01.org

ops sorry, I forgot to add it to the cover-letter.

Regards,
Lorenzo

Quoted 39 lines

Quoted 30 lines
Changes since v3:
- align DEV_MAP_BULK_SIZE to XDP_BULK_QUEUE_SIZE
- refactor page_pool_put_page_bulk to avoid code duplication
Changes since v2:
- move mvneta changes in a dedicated patch
Changes since v1:
- improve comments
- rework xdp_return_frame_bulk routine logic
- move count and xa fields at the beginning of xdp_frame_bulk struct
- invert logic in page_pool_put_page_bulk for loop
Lorenzo Bianconi (5):
net: xdp: introduce bulking for xdp tx return path
net: page_pool: add bulk support for ptr_ring
net: mvneta: add xdp tx return bulking support
net: mvpp2: add xdp tx return bulking support
net: mlx5: add xdp tx return bulking support
 drivers/net/ethernet/marvell/mvneta.c         | 10 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 10 ++-
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  | 22 ++++--
 include/net/page_pool.h                       | 26 +++++++
 include/net/xdp.h                             | 17 ++++-
 net/core/page_pool.c                          | 69 ++++++++++++++++---
 net/core/xdp.c                                | 54 +++++++++++++++
 7 files changed, 191 insertions(+), 17 deletions(-)
Signature · 4 lines
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer