From: "Michael S. Tsirkin" <mst@redhat.com>
To: Liang Chen <liangchen.linux@gmail.com>
Cc: jasowang@redhat.com, virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
xuanzhuo@linux.alibaba.com, kuba@kernel.org, edumazet@google.com,
davem@davemloft.net, pabeni@redhat.com,
alexander.duyck@gmail.com
Subject: Re: [PATCH net-next 3/5] virtio_net: Add page pool fragmentation support
Date: Sun, 28 May 2023 02:25:21 -0400 [thread overview]
Message-ID: <20230528022057-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20230526054621.18371-3-liangchen.linux@gmail.com>
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@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
next prev parent reply other threads:[~2023-05-28 6:25 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-26 5:46 [PATCH net-next 1/5] virtio_net: Fix an unsafe reference to the page chain Liang Chen
2023-05-26 5:46 ` [PATCH net-next 2/5] virtio_net: Add page_pool support to improve performance Liang Chen
2023-05-26 6:50 ` Jason Wang
2023-05-27 12:35 ` Liang Chen
2023-05-28 6:40 ` Michael S. Tsirkin
2023-05-29 7:28 ` Liang Chen
2023-05-31 3:10 ` Xuan Zhuo
2023-06-07 9:11 ` Liang Chen
2023-06-07 9:33 ` Xuan Zhuo
2023-05-26 16:11 ` kernel test robot
2023-05-28 6:27 ` Michael S. Tsirkin
2023-05-29 7:28 ` Liang Chen
2023-05-28 6:20 ` Michael S. Tsirkin
2023-05-29 7:27 ` Liang Chen
2023-05-29 9:55 ` Michael S. Tsirkin
2023-05-30 1:19 ` Liang Chen
2023-06-07 9:08 ` Liang Chen
2023-06-07 9:35 ` Xuan Zhuo
2023-06-07 13:58 ` Liang Chen
2023-06-07 20:17 ` Michael S. Tsirkin
2023-06-08 0:38 ` Jason Wang
2023-06-08 3:54 ` Xuan Zhuo
2023-06-09 2:57 ` Liang Chen
2023-07-05 5:41 ` Liang Chen
2023-07-05 6:04 ` Jason Wang
2023-07-06 1:01 ` Liang Chen
2023-06-09 2:57 ` Liang Chen
2023-11-29 14:50 ` Zhu Yanjun
2023-11-29 14:59 ` Michael S. Tsirkin
2023-11-29 15:22 ` Zhu Yanjun
2023-11-29 15:29 ` Zhu Yanjun
2023-11-30 2:34 ` Xuan Zhuo
2023-11-30 5:30 ` Zhu Yanjun
2023-12-01 1:38 ` Xuan Zhuo
2023-12-04 5:24 ` Zhu Yanjun
2023-11-30 7:17 ` Zhu Yanjun
2023-05-26 5:46 ` [PATCH net-next 3/5] virtio_net: Add page pool fragmentation support Liang Chen
2023-05-26 8:29 ` Horatiu Vultur
2023-05-27 12:36 ` Liang Chen
2023-05-26 17:44 ` kernel test robot
2023-05-28 6:25 ` Michael S. Tsirkin [this message]
2023-05-29 7:29 ` Liang Chen
2023-05-29 1:33 ` Yunsheng Lin
2023-05-29 7:30 ` Liang Chen
2023-05-26 5:46 ` [PATCH net-next 4/5] virtio_ring: Introduce DMA pre-handler Liang Chen
2023-05-26 6:57 ` Jason Wang
2023-05-26 5:46 ` [PATCH net-next 5/5] virtio_net: Implement " Liang Chen
2023-05-26 7:06 ` Jason Wang
2023-05-27 12:35 ` Liang Chen
2023-05-26 17:34 ` kernel test robot
2023-05-26 6:38 ` [PATCH net-next 1/5] virtio_net: Fix an unsafe reference to the page chain Jason Wang
2023-05-27 12:33 ` Liang Chen
2023-05-28 6:29 ` Michael S. Tsirkin
2023-05-29 7:25 ` Liang Chen
2023-05-28 6:16 ` Michael S. Tsirkin
2023-05-29 7:25 ` Liang Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230528022057-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=alexander.duyck@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jasowang@redhat.com \
--cc=kuba@kernel.org \
--cc=liangchen.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).