On Sun, May 28, 2023 at 2:25 PM Michael S. Tsirkin [off-list ref] wrote:

On Fri, May 26, 2023 at 01:46:19PM +0800, Liang Chen wrote:

quoted

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 <redacted>

I think it's called fragmenting not fragmentation.

Sure, thanks!

quoted

---
 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?

Yeah, now I believe it makes sense to enable it by default to avoid
the extra modes. Thanks.

quoted

@@ -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