add page_pool support for page recycling in veth driver

PATCH Superseded Lorenzo Bianconi 2 participants 5 messages Apr 20, 2023 – Apr 22, 2023

Subsystems (3)

Networking Network drivers BPF

lore

Share & tools

lore

Lorenzo Bianconi

author

2023-04-20 11:20 UTC #0 lore raw

cover add page_pool support for page recycling in veth driver

Introduce page_pool support in veth driver in order to recycle pages in veth_convert_skb_to_xdp_buff routine and avoid reallocating the skb through the page allocator when we run a xdp program on the device and we receive skbs from the stack.

Lorenzo Bianconi (2):
net: veth: add page_pool for page recycling
net: veth: add page_pool stats

 drivers/net/Kconfig |  2 ++
 drivers/net/veth.c  | 74 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 68 insertions(+), 8 deletions(-)

Signature · 1 line

2.40.0

Lorenzo Bianconi

author

2023-04-20 11:20 UTC #1 lore raw

PATCH 1/2 net: veth: add page_pool for page recycling

Introduce page_pool support in veth driver in order to recycle pages in veth_convert_skb_to_xdp_buff routine and avoid reallocating the skb through the page allocator.
The patch has been sending tcp traffic to a veth pair where the remote peer is running a simple xdp program just returing xdp_pass:

veth upstream codebase:
MTU 1500B: ~ 8Gbps
MTU 8000B: ~ 13.9Gbps

veth upstream codebase + pp support:
MTU 1500B: ~ 9.2Gbps
MTU 8000B: ~ 16.2Gbps

Tested-by Maryam Tahhan <[email protected]>

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

---
 drivers/net/Kconfig |  1 +
 drivers/net/veth.c  | 54 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 50 insertions(+), 5 deletions(-)

drivers/net/Kconfig +1 -0

index c34bd432da27..368c6f5b327e 100644
@@ -402,6 +402,7 @@ config TUN_VNET_CROSS_LE
 
 config VETH
 	tristate "Virtual ethernet pair device"
+	select PAGE_POOL
 	help
 	  This device is a local ethernet tunnel. Devices are created in pairs.
 	  When one end receives the packet it appears on its pair and vice

drivers/net/veth.c +49 -5

131 lines not on this page · raw message

Signature · 1 line

2.40.0

Jakub Kicinski

maintainer

2023-04-22 03:22 UTC #2 lore raw

Re: [PATCH net-next 1/2] net: veth: add page_pool for page recycling

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 732

Quoted 6 lines · On Thu, 20 Apr 2023 13:16:21 +0200 Lorenzo Bianconi wrote:

drivers/net/veth.c +1 -1

@@ -727,17 +729,21 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 			goto drop;
 
 		/* Allocate skb head */
-		page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
+		if (rq->page_pool)

There's some condition under which we can get to XDP enabled but no pool?

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 739

Quoted 8 lines

drivers/net/veth.c +2 -1

+			page = page_pool_dev_alloc_pages(rq->page_pool);
 		if (!page)
 			goto drop;
 
 		nskb = build_skb(page_address(page), PAGE_SIZE);
 		if (!nskb) {
-			put_page(page);
+			page_pool_put_full_page(rq->page_pool, page, false);

You can recycle direct, AFAIU the basic rule of thumb is that it's always safe to recycle direct from the context which allocates from the pool.

Quoted 12 lines

drivers/net/veth.c +3 -0

 			goto drop;
 		}
 
 		skb_reserve(nskb, VETH_XDP_HEADROOM);
+		skb_copy_header(nskb, skb);
+		skb_mark_for_recycle(nskb);
+
 		size = min_t(u32, skb->len, max_head_size);
 		if (skb_copy_bits(skb, 0, nskb->data, size)) {
 			consume_skb(nskb);

@@ -745,16 +751,17 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 		}

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 760

Quoted 10 lines

drivers/net/veth.c +1 -1

 		skb_put(nskb, size);
 
-		skb_copy_header(nskb, skb);
 		head_off = skb_headroom(nskb) - skb_headroom(skb);
 		skb_headers_offset_update(nskb, head_off);
 
 		/* Allocate paged area of new skb */
 		off = size;
 		len = skb->len - off;
+		page = NULL;

Why do you clear the page pointer?

Signature · 1 line

pw-bot: cr

Lorenzo Bianconi

author

2023-04-22 18:49 UTC #3 lore raw

Re: [PATCH net-next 1/2] net: veth: add page_pool for page recycling

Quoted 10 lines · On Thu, 20 Apr 2023 13:16:21 +0200 Lorenzo Bianconi wrote:

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 732
Quoted 6 lines
drivers/net/veth.c +1 -1
@@ -727,17 +729,21 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 			goto drop;
 
 		/* Allocate skb head */
-		page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
+		if (rq->page_pool)
There's some condition under which we can get to XDP enabled but no pool?

ack, since we destroy the page_pool after disabling the rq napi we can get rid of 'if (rq->page_pool)' checks here.

Quoted 5 lines

drivers/net/veth.c +1 -0

+			page = page_pool_dev_alloc_pages(rq->page_pool);
 		if (!page)
 			goto drop;

Quoted 8 lines

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 739
Quoted 4 lines
drivers/net/veth.c +1 -1
 		nskb = build_skb(page_address(page), PAGE_SIZE);
 		if (!nskb) {
-			put_page(page);
+			page_pool_put_full_page(rq->page_pool, page, false);
You can recycle direct, AFAIU the basic rule of thumb is that it's always safe to recycle direct from the context which allocates from the pool.

ack, I will fix it.

Quoted 19 lines

drivers/net/veth.c +3 -1

 			goto drop;
 		}
 
 		skb_reserve(nskb, VETH_XDP_HEADROOM);
+		skb_copy_header(nskb, skb);
+		skb_mark_for_recycle(nskb);
+
 		size = min_t(u32, skb->len, max_head_size);
 		if (skb_copy_bits(skb, 0, nskb->data, size)) {
 			consume_skb(nskb);

@@ -745,16 +751,17 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
 		}
 		skb_put(nskb, size);
 
-		skb_copy_header(nskb, skb);
 		head_off = skb_headroom(nskb) - skb_headroom(skb);
 		skb_headers_offset_update(nskb, head_off);

Quoted 6 lines

drivers/net/veth.c static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq, · line 760
Quoted 4 lines
drivers/net/veth.c +1 -0
 		/* Allocate paged area of new skb */
 		off = size;
 		len = skb->len - off;
+		page = NULL;
Why do you clear the page pointer?

since we do not need 'if (rq->page_pool)' checks anymore we can get rid of it.

Regards,
Lorenzo

Quoted 3 lines

Signature · 2 lines
-- 
pw-bot: cr

Lorenzo Bianconi

author

2023-04-20 11:20 UTC #4 lore raw

PATCH 2/2 net: veth: add page_pool stats

In reply to Lorenzo Bianconi

Introduce page_pool stats support to report info about local page_pool through ethtool

Tested-by Maryam Tahhan <[email protected]>

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

---
 drivers/net/Kconfig |  1 +
 drivers/net/veth.c  | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

drivers/net/Kconfig +1 -0

index 368c6f5b327e..d0a1ed216d15 100644
@@ -403,6 +403,7 @@ config TUN_VNET_CROSS_LE
 config VETH
 	tristate "Virtual ethernet pair device"
 	select PAGE_POOL
+	select PAGE_POOL_STATS
 	help
 	  This device is a local ethernet tunnel. Devices are created in pairs.
 	  When one end receives the packet it appears on its pair and vice

drivers/net/veth.c +17 -3

index 141b7745ba43..4e08a4633d25 100644
@@ -157,6 +157,8 @@ static void veth_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 			for (j = 0; j < VETH_TQ_STATS_LEN; j++)
 				ethtool_sprintf(&p, "tx_queue_%u_%.18s",
 						i, veth_tq_stats_desc[j].desc);
+
+		page_pool_ethtool_stats_get_strings(p);
 		break;
 	}
 }

@@ -167,7 +169,8 @@ static int veth_get_sset_count(struct net_device *dev, int sset)
 	case ETH_SS_STATS:
 		return ARRAY_SIZE(ethtool_stats_keys) +
 		       VETH_RQ_STATS_LEN * dev->real_num_rx_queues +
-		       VETH_TQ_STATS_LEN * dev->real_num_tx_queues;
+		       VETH_TQ_STATS_LEN * dev->real_num_tx_queues +
+		       page_pool_ethtool_stats_get_count();
 	default:
 		return -EOPNOTSUPP;
 	}

@@ -178,7 +181,8 @@ static void veth_get_ethtool_stats(struct net_device *dev,
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
 	struct net_device *peer = rtnl_dereference(priv->peer);
-	int i, j, idx;
+	struct page_pool_stats pp_stats = {};
+	int i, j, idx, pp_idx;
 
 	data[0] = peer ? peer->ifindex : 0;
 	idx = 1;

@@ -197,9 +201,10 @@ static void veth_get_ethtool_stats(struct net_device *dev,
 		} while (u64_stats_fetch_retry(&rq_stats->syncp, start));
 		idx += VETH_RQ_STATS_LEN;
 	}
+	pp_idx = idx;
 
 	if (!peer)
-		return;
+		goto page_pool_stats;
 
 	rcv_priv = netdev_priv(peer);
 	for (i = 0; i < peer->real_num_rx_queues; i++) {

@@ -216,7 +221,16 @@ static void veth_get_ethtool_stats(struct net_device *dev,
 				data[tx_idx + j] += *(u64 *)(base + offset);
 			}
 		} while (u64_stats_fetch_retry(&rq_stats->syncp, start));
+		pp_idx = tx_idx + VETH_TQ_STATS_LEN;
+	}
+
+page_pool_stats:
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		if (!priv->rq[i].page_pool)
+			continue;
+		page_pool_get_stats(priv->rq[i].page_pool, &pp_stats);
 	}
+	page_pool_ethtool_stats_get(&data[pp_idx], &pp_stats);
 }
 
 static void veth_get_channels(struct net_device *dev,

Signature · 1 line

2.40.0