thr3ads.net - Virtualization - [PATCH net-next 3/5] virtio_net: Add page pool fragmentation support [May 2023]

If this information is useful, please help other people find it:
Share via:

Liang Chen

2023-May-26 05:46 UTC

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

To further enhance performance, implement page pool fragmentation
support and introduce a module parameter to enable or disable it.

In single-core vm testing environments, there is an additional performance
gain observed in the normal path compared to the one packet per page
approach.
  Upstream codebase: 47.5 Gbits/sec
  Upstream codebase with page pool: 50.2 Gbits/sec
  Upstream codebase with page pool fragmentation support: 52.3 Gbits/sec

There is also some performance gain for XDP cpumap.
  Upstream codebase: 1.38 Gbits/sec
  Upstream codebase with page pool: 9.74 Gbits/sec
  Upstream codebase with page pool fragmentation: 10.3 Gbits/sec

Signed-off-by: Liang Chen <liangchen.linux at gmail.com>
---
 drivers/net/virtio_net.c | 72 ++++++++++++++++++++++++++++++----------
 1 file changed, 55 insertions(+), 17 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 99c0ca0c1781..ac40b8c66c59 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -32,7 +32,9 @@ module_param(gso, bool, 0444);
 module_param(napi_tx, bool, 0644);
 
 static bool page_pool_enabled;
+static bool page_pool_frag;
 module_param(page_pool_enabled, bool, 0400);
+module_param(page_pool_frag, bool, 0400);
 
 /* FIXME: MTU in config. */
 #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
@@ -909,23 +911,32 @@ static struct page *xdp_linearize_page(struct
receive_queue *rq,
 				       struct page *p,
 				       int offset,
 				       int page_off,
-				       unsigned int *len)
+				       unsigned int *len,
+					   unsigned int *pp_frag_offset)
 {
 	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 	struct page *page;
+	unsigned int pp_frag_offset_val;
 
 	if (page_off + *len + tailroom > PAGE_SIZE)
 		return NULL;
 
 	if (rq->page_pool)
-		page = page_pool_dev_alloc_pages(rq->page_pool);
+		if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG)
+			page = page_pool_dev_alloc_frag(rq->page_pool, pp_frag_offset,
+							PAGE_SIZE);
+		else
+			page = page_pool_dev_alloc_pages(rq->page_pool);
 	else
 		page = alloc_page(GFP_ATOMIC);
 
 	if (!page)
 		return NULL;
 
-	memcpy(page_address(page) + page_off, page_address(p) + offset, *len);
+	pp_frag_offset_val = pp_frag_offset ? *pp_frag_offset : 0;
+
+	memcpy(page_address(page) + page_off + pp_frag_offset_val,
+	       page_address(p) + offset, *len);
 	page_off += *len;
 
 	while (--*num_buf) {
@@ -948,7 +959,7 @@ static struct page *xdp_linearize_page(struct receive_queue
*rq,
 			goto err_buf;
 		}
 
-		memcpy(page_address(page) + page_off,
+		memcpy(page_address(page) + page_off + pp_frag_offset_val,
 		       page_address(p) + off, buflen);
 		page_off += buflen;
 		virtnet_put_page(rq, p);
@@ -1029,7 +1040,7 @@ static struct sk_buff *receive_small_xdp(struct net_device
*dev,
 			SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 		xdp_page = xdp_linearize_page(rq, &num_buf, page,
 					      offset, header_offset,
-					      &tlen);
+					      &tlen, NULL);
 		if (!xdp_page)
 			goto err_xdp;
 
@@ -1323,6 +1334,7 @@ static void *mergeable_xdp_get_buf(struct virtnet_info
*vi,
 	unsigned int headroom = mergeable_ctx_to_headroom(ctx);
 	struct page *xdp_page;
 	unsigned int xdp_room;
+	unsigned int page_frag_offset = 0;
 
 	/* Transient failure which in theory could occur if
 	 * in-flight packets from before XDP was enabled reach
@@ -1356,7 +1368,8 @@ static void *mergeable_xdp_get_buf(struct virtnet_info
*vi,
 		xdp_page = xdp_linearize_page(rq, num_buf,
 					      *page, offset,
 					      VIRTIO_XDP_HEADROOM,
-					      len);
+					      len,
+						  &page_frag_offset);
 		if (!xdp_page)
 			return NULL;
 	} else {
@@ -1366,14 +1379,19 @@ static void *mergeable_xdp_get_buf(struct virtnet_info
*vi,
 			return NULL;
 
 		if (rq->page_pool)
-			xdp_page = page_pool_dev_alloc_pages(rq->page_pool);
+			if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG)
+				xdp_page = page_pool_dev_alloc_frag(rq->page_pool,
+								    &page_frag_offset, PAGE_SIZE);
+			else
+				xdp_page = page_pool_dev_alloc_pages(rq->page_pool);
 		else
 			xdp_page = alloc_page(GFP_ATOMIC);
+
 		if (!xdp_page)
 			return NULL;
 
-		memcpy(page_address(xdp_page) + VIRTIO_XDP_HEADROOM,
-		       page_address(*page) + offset, *len);
+		memcpy(page_address(xdp_page) + VIRTIO_XDP_HEADROOM +
+				page_frag_offset, page_address(*page) + offset, *len);
 	}
 
 	*frame_sz = PAGE_SIZE;
@@ -1382,7 +1400,7 @@ static void *mergeable_xdp_get_buf(struct virtnet_info
*vi,
 
 	*page = xdp_page;
 
-	return page_address(*page) + VIRTIO_XDP_HEADROOM;
+	return page_address(*page) + VIRTIO_XDP_HEADROOM + page_frag_offset;
 }
 
 static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
@@ -1762,6 +1780,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
 	void *ctx;
 	int err;
 	unsigned int len, hole;
+	unsigned int pp_frag_offset;
 
 	/* Extra tailroom is needed to satisfy XDP's assumption. This
 	 * means rx frags coalescing won't work, but consider we've
@@ -1769,13 +1788,29 @@ static int add_recvbuf_mergeable(struct virtnet_info
*vi,
 	 */
 	len = get_mergeable_buf_len(rq, &rq->mrg_avg_pkt_len, room);
 	if (rq->page_pool) {
-		struct page *page;
+		if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG) {
+			if (unlikely(!page_pool_dev_alloc_frag(rq->page_pool,
+							       &pp_frag_offset, len + room)))
+				return -ENOMEM;
+			buf = (char *)page_address(rq->page_pool->frag_page) +
+				pp_frag_offset;
+			buf += headroom; /* advance address leaving hole at front of pkt */
+			hole = (PAGE_SIZE << rq->page_pool->p.order)
+				- rq->page_pool->frag_offset;
+			if (hole < len + room) {
+				if (!headroom)
+					len += hole;
+				rq->page_pool->frag_offset += hole;
+			}
+		} else {
+			struct page *page;
 
-		page = page_pool_dev_alloc_pages(rq->page_pool);
-		if (unlikely(!page))
-			return -ENOMEM;
-		buf = (char *)page_address(page);
-		buf += headroom; /* advance address leaving hole at front of pkt */
+			page = page_pool_dev_alloc_pages(rq->page_pool);
+			if (unlikely(!page))
+				return -ENOMEM;
+			buf = (char *)page_address(page);
+			buf += headroom; /* advance address leaving hole at front of pkt */
+		}
 	} else {
 		if (unlikely(!skb_page_frag_refill(len + room, alloc_frag, gfp)))
 			return -ENOMEM;
@@ -3800,13 +3835,16 @@ static void virtnet_alloc_page_pool(struct receive_queue
*rq)
 	struct virtio_device *vdev = rq->vq->vdev;
 
 	struct page_pool_params pp_params = {
-		.order = 0,
+		.order = page_pool_frag ? SKB_FRAG_PAGE_ORDER : 0,
 		.pool_size = rq->vq->num_max,
 		.nid = dev_to_node(vdev->dev.parent),
 		.dev = vdev->dev.parent,
 		.offset = 0,
 	};
 
+	if (page_pool_frag)
+		pp_params.flags |= PP_FLAG_PAGE_FRAG;
+
 	rq->page_pool = page_pool_create(&pp_params);
 	if (IS_ERR(rq->page_pool)) {
 		dev_warn(&vdev->dev, "page pool creation failed: %ld\n",
-- 
2.31.1

kernel test robot

2023-May-26 17:44 UTC

head link

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

Hi Liang,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:   
https://github.com/intel-lab-lkp/linux/commits/Liang-Chen/virtio_net-Add-page_pool-support-to-improve-performance/20230526-135805
base:   net-next/main
patch link:   
https://lore.kernel.org/r/20230526054621.18371-3-liangchen.linux%40gmail.com
patch subject: [PATCH net-next 3/5] virtio_net: Add page pool fragmentation
support
config: x86_64-defconfig
(https://download.01.org/0day-ci/archive/20230527/202305270116.TJ31IjNL-lkp at
intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        #
https://github.com/intel-lab-lkp/linux/commit/dda0469e059354b61192e1d25b77c57351346282
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review
Liang-Chen/virtio_net-Add-page_pool-support-to-improve-performance/20230526-135805
        git checkout dda0469e059354b61192e1d25b77c57351346282
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp at intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305270116.TJ31IjNL-lkp at
intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `virtnet_find_vqs':
   virtio_net.c:(.text+0x901fd2): undefined reference to `page_pool_create'
   ld: vmlinux.o: in function `add_recvbuf_mergeable.isra.0':
   virtio_net.c:(.text+0x905662): undefined reference to
`page_pool_alloc_pages'>> ld: virtio_net.c:(.text+0x905715): undefined reference to
`page_pool_alloc_frag'   ld: vmlinux.o: in function `xdp_linearize_page':
   virtio_net.c:(.text+0x906c50): undefined reference to
`page_pool_alloc_pages'
   ld: virtio_net.c:(.text+0x906e33): undefined reference to
`page_pool_alloc_frag'
   ld: vmlinux.o: in function
`mergeable_xdp_get_buf.isra.0':>> virtio_net.c:(.text+0x90740e): undefined reference to
`page_pool_alloc_frag'
>> ld: virtio_net.c:(.text+0x90750b): undefined reference to
`page_pool_alloc_pages'
-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Michael S. Tsirkin

2023-May-28 06:25 UTC

head link

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

On Fri, May 26, 2023 at 01:46:19PM +0800, Liang Chen
wrote:> To further enhance performance, implement page pool fragmentation
> support and introduce a module parameter to enable or disable it.
> 
> In single-core vm testing environments, there is an additional performance
> gain observed in the normal path compared to the one packet per page
> approach.
>   Upstream codebase: 47.5 Gbits/sec
>   Upstream codebase with page pool: 50.2 Gbits/sec
>   Upstream codebase with page pool fragmentation support: 52.3 Gbits/sec
> 
> There is also some performance gain for XDP cpumap.
>   Upstream codebase: 1.38 Gbits/sec
>   Upstream codebase with page pool: 9.74 Gbits/sec
>   Upstream codebase with page pool fragmentation: 10.3 Gbits/sec
> 
> Signed-off-by: Liang Chen <liangchen.linux at gmail.com>
I think it's called fragmenting not fragmentation.

> ---
>  drivers/net/virtio_net.c | 72 ++++++++++++++++++++++++++++++----------
>  1 file changed, 55 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 99c0ca0c1781..ac40b8c66c59 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -32,7 +32,9 @@ module_param(gso, bool, 0444);
>  module_param(napi_tx, bool, 0644);
>  
>  static bool page_pool_enabled;
> +static bool page_pool_frag;
>  module_param(page_pool_enabled, bool, 0400);
> +module_param(page_pool_frag, bool, 0400);
>  
>  /* FIXME: MTU in config. */
>  #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
So here again same questions.

-when is this a net perf gain when does it have no effect?
-can be on by default
- can we get rid of the extra modes?

> @@ -909,23 +911,32 @@ static struct page *xdp_linearize_page(struct
receive_queue *rq,
>  				       struct page *p,
>  				       int offset,
>  				       int page_off,
> -				       unsigned int *len)
> +				       unsigned int *len,
> +					   unsigned int *pp_frag_offset)
>  {
>  	int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>  	struct page *page;
> +	unsigned int pp_frag_offset_val;
>  
>  	if (page_off + *len + tailroom > PAGE_SIZE)
>  		return NULL;
>  
>  	if (rq->page_pool)
> -		page = page_pool_dev_alloc_pages(rq->page_pool);
> +		if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG)
> +			page = page_pool_dev_alloc_frag(rq->page_pool, pp_frag_offset,
> +							PAGE_SIZE);
> +		else
> +			page = page_pool_dev_alloc_pages(rq->page_pool);
>  	else
>  		page = alloc_page(GFP_ATOMIC);
>  
>  	if (!page)
>  		return NULL;
>  
> -	memcpy(page_address(page) + page_off, page_address(p) + offset, *len);
> +	pp_frag_offset_val = pp_frag_offset ? *pp_frag_offset : 0;
> +
> +	memcpy(page_address(page) + page_off + pp_frag_offset_val,
> +	       page_address(p) + offset, *len);
>  	page_off += *len;
>  
>  	while (--*num_buf) {
> @@ -948,7 +959,7 @@ static struct page *xdp_linearize_page(struct
receive_queue *rq,
>  			goto err_buf;
>  		}
>  
> -		memcpy(page_address(page) + page_off,
> +		memcpy(page_address(page) + page_off + pp_frag_offset_val,
>  		       page_address(p) + off, buflen);
>  		page_off += buflen;
>  		virtnet_put_page(rq, p);
> @@ -1029,7 +1040,7 @@ static struct sk_buff *receive_small_xdp(struct
net_device *dev,
>  			SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>  		xdp_page = xdp_linearize_page(rq, &num_buf, page,
>  					      offset, header_offset,
> -					      &tlen);
> +					      &tlen, NULL);
>  		if (!xdp_page)
>  			goto err_xdp;
>  
> @@ -1323,6 +1334,7 @@ static void *mergeable_xdp_get_buf(struct
virtnet_info *vi,
>  	unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>  	struct page *xdp_page;
>  	unsigned int xdp_room;
> +	unsigned int page_frag_offset = 0;
>  
>  	/* Transient failure which in theory could occur if
>  	 * in-flight packets from before XDP was enabled reach
> @@ -1356,7 +1368,8 @@ static void *mergeable_xdp_get_buf(struct
virtnet_info *vi,
>  		xdp_page = xdp_linearize_page(rq, num_buf,
>  					      *page, offset,
>  					      VIRTIO_XDP_HEADROOM,
> -					      len);
> +					      len,
> +						  &page_frag_offset);
>  		if (!xdp_page)
>  			return NULL;
>  	} else {
> @@ -1366,14 +1379,19 @@ static void *mergeable_xdp_get_buf(struct
virtnet_info *vi,
>  			return NULL;
>  
>  		if (rq->page_pool)
> -			xdp_page = page_pool_dev_alloc_pages(rq->page_pool);
> +			if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG)
> +				xdp_page = page_pool_dev_alloc_frag(rq->page_pool,
> +								    &page_frag_offset, PAGE_SIZE);
> +			else
> +				xdp_page = page_pool_dev_alloc_pages(rq->page_pool);
>  		else
>  			xdp_page = alloc_page(GFP_ATOMIC);
> +
>  		if (!xdp_page)
>  			return NULL;
>  
> -		memcpy(page_address(xdp_page) + VIRTIO_XDP_HEADROOM,
> -		       page_address(*page) + offset, *len);
> +		memcpy(page_address(xdp_page) + VIRTIO_XDP_HEADROOM +
> +				page_frag_offset, page_address(*page) + offset, *len);
>  	}
>  
>  	*frame_sz = PAGE_SIZE;
> @@ -1382,7 +1400,7 @@ static void *mergeable_xdp_get_buf(struct
virtnet_info *vi,
>  
>  	*page = xdp_page;
>  
> -	return page_address(*page) + VIRTIO_XDP_HEADROOM;
> +	return page_address(*page) + VIRTIO_XDP_HEADROOM + page_frag_offset;
>  }
>  
>  static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
> @@ -1762,6 +1780,7 @@ static int add_recvbuf_mergeable(struct virtnet_info
*vi,
>  	void *ctx;
>  	int err;
>  	unsigned int len, hole;
> +	unsigned int pp_frag_offset;
>  
>  	/* Extra tailroom is needed to satisfy XDP's assumption. This
>  	 * means rx frags coalescing won't work, but consider we've
> @@ -1769,13 +1788,29 @@ static int add_recvbuf_mergeable(struct
virtnet_info *vi,
>  	 */
>  	len = get_mergeable_buf_len(rq, &rq->mrg_avg_pkt_len, room);
>  	if (rq->page_pool) {
> -		struct page *page;
> +		if (rq->page_pool->p.flags & PP_FLAG_PAGE_FRAG) {
> +			if (unlikely(!page_pool_dev_alloc_frag(rq->page_pool,
> +							       &pp_frag_offset, len + room)))
> +				return -ENOMEM;
> +			buf = (char *)page_address(rq->page_pool->frag_page) +
> +				pp_frag_offset;
> +			buf += headroom; /* advance address leaving hole at front of pkt */
> +			hole = (PAGE_SIZE << rq->page_pool->p.order)
> +				- rq->page_pool->frag_offset;
> +			if (hole < len + room) {
> +				if (!headroom)
> +					len += hole;
> +				rq->page_pool->frag_offset += hole;
> +			}
> +		} else {
> +			struct page *page;
>  
> -		page = page_pool_dev_alloc_pages(rq->page_pool);
> -		if (unlikely(!page))
> -			return -ENOMEM;
> -		buf = (char *)page_address(page);
> -		buf += headroom; /* advance address leaving hole at front of pkt */
> +			page = page_pool_dev_alloc_pages(rq->page_pool);
> +			if (unlikely(!page))
> +				return -ENOMEM;
> +			buf = (char *)page_address(page);
> +			buf += headroom; /* advance address leaving hole at front of pkt */
> +		}
>  	} else {
>  		if (unlikely(!skb_page_frag_refill(len + room, alloc_frag, gfp)))
>  			return -ENOMEM;
> @@ -3800,13 +3835,16 @@ static void virtnet_alloc_page_pool(struct
receive_queue *rq)
>  	struct virtio_device *vdev = rq->vq->vdev;
>  
>  	struct page_pool_params pp_params = {
> -		.order = 0,
> +		.order = page_pool_frag ? SKB_FRAG_PAGE_ORDER : 0,
>  		.pool_size = rq->vq->num_max,
>  		.nid = dev_to_node(vdev->dev.parent),
>  		.dev = vdev->dev.parent,
>  		.offset = 0,
>  	};
>  
> +	if (page_pool_frag)
> +		pp_params.flags |= PP_FLAG_PAGE_FRAG;
> +
>  	rq->page_pool = page_pool_create(&pp_params);
>  	if (IS_ERR(rq->page_pool)) {
>  		dev_warn(&vdev->dev, "page pool creation failed:
%ld\n",
> -- 
> 2.31.1

Virtualization - May 2023 - [PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support

[PATCH net-next 3/5] virtio_net: Add page pool fragmentation support