* [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
@ 2026-03-10 18:31 Vishwanath Seshagiri
2026-03-13 7:51 ` Jason Wang
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Vishwanath Seshagiri @ 2026-03-10 18:31 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang
Cc: Xuan Zhuo, Eugenio Pérez, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Wei,
Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 32960 bytes --]
Use page_pool for RX buffer allocation in mergeable and small buffer
modes to enable page recycling and avoid repeated page allocator calls.
skb_mark_for_recycle() enables page reuse in the network stack.
Big packets mode is unchanged because it uses page->private for linked
list chaining of multiple pages per buffer, which conflicts with
page_pool's internal use of page->private.
Implement conditional DMA premapping using virtqueue_dma_dev():
- When non-NULL (vhost, virtio-pci): use PP_FLAG_DMA_MAP with page_pool
handling DMA mapping, submit via virtqueue_add_inbuf_premapped()
- When NULL (VDUSE, direct physical): page_pool handles allocation only,
submit via virtqueue_add_inbuf_ctx()
This preserves the DMA premapping optimization from commit 31f3cd4e5756b
("virtio-net: rq submits premapped per-buffer") while adding page_pool
support as a prerequisite for future zero-copy features (devmem TCP,
io_uring ZCRX).
Page pools are created in probe and destroyed in remove (not open/close),
following existing driver behavior where RX buffers remain in virtqueues
across interface state changes.
Signed-off-by: Vishwanath Seshagiri <vishs@meta.com>
---
Changes in v11:
- add_recvbuf_small: encode alloc_len and xdp_headroom in ctx via
mergeable_len_to_ctx() so receive_small() recovers the actual buflen
via mergeable_ctx_to_truesize() (Michael S. Tsirkin)
- receive_small_build_skb, receive_small_xdp: accept buflen parameter
instead of recomputing it, to use the actual allocation size
- v10:
https://lore.kernel.org/virtualization/9752a952-195d-4da3-bc7a-5a4a1f2fd2ca@meta.com/
Changes in v10:
- add_recvbuf_small: use alloc_len to avoid clobbering len; v9 feedback
was about truesize under-accounting, not variable naming — misunderstood
the comment in v9
- v9:
https://lore.kernel.org/virtualization/20260302041005.1627210-1-vishs@meta.com/
Changes in v9:
- Fix virtnet_skb_append_frag() for XSK callers (Michael S. Tsirkin)
- v8:
https://lore.kernel.org/virtualization/e824c5a3-cfe0-4d11-958f-c3ec82d11d37@meta.com/
Changes in v8:
- Remove virtnet_no_page_pool() helper, replace with direct !rq->page_pool
checks or inlined conditions (Xuan Zhuo)
- Extract virtnet_rq_submit() helper to consolidate DMA/non-DMA buffer
submission in add_recvbuf_small() and add_recvbuf_mergeable()
- Add skb_mark_for_recycle(nskb) for overflow frag_list skbs in
virtnet_skb_append_frag() to ensure page_pool pages are returned to
the pool instead of freed via put_page()
- Rebase on net-next (kzalloc_objs API)
- v7:
https://lore.kernel.org/virtualization/20260210014305.3236342-1-vishs@meta.com/
Changes in v7:
- Replace virtnet_put_page() helper with direct page_pool_put_page()
calls (Xuan Zhuo)
- Add virtnet_no_page_pool() helper to consolidate big_packets mode check
(Michael S. Tsirkin)
- Add DMA sync_for_cpu for subsequent buffers in xdp_linearize_page() when
use_page_pool_dma is set (Michael S. Tsirkin)
- Remove unused pp_params.dev assignment in non-DMA path
- Add page pool recreation in virtnet_restore_up() for freeze/restore support (Chris Mason's
Review Prompt)
- v6:
https://lore.kernel.org/virtualization/20260208175410.1910001-1-vishs@meta.com/
Changes in v6:
- Drop page_pool_frag_offset_add() helper and switch to page_pool_alloc_va();
page_pool_alloc_netmem() already handles internal fragmentation internally
(Jakub Kicinski)
- v5:
https://lore.kernel.org/virtualization/20260206002715.1885869-1-vishs@meta.com/
Benchmark results:
Configuration: pktgen TX -> tap -> vhost-net | virtio-net RX -> XDP_DROP
Small packets (64 bytes, mrg_rxbuf=off):
1Q: 853,493 -> 868,923 pps (+1.8%)
2Q: 1,655,793 -> 1,696,707 pps (+2.5%)
4Q: 3,143,375 -> 3,302,511 pps (+5.1%)
8Q: 6,082,590 -> 6,156,894 pps (+1.2%)
Mergeable RX (64 bytes):
1Q: 766,168 -> 814,493 pps (+6.3%)
2Q: 1,384,871 -> 1,670,639 pps (+20.6%)
4Q: 2,773,081 -> 3,080,574 pps (+11.1%)
8Q: 5,600,615 -> 6,043,891 pps (+7.9%)
Mergeable RX (1500 bytes):
1Q: 741,579 -> 785,442 pps (+5.9%)
2Q: 1,310,043 -> 1,534,554 pps (+17.1%)
4Q: 2,748,700 -> 2,890,582 pps (+5.2%)
8Q: 5,348,589 -> 5,618,664 pps (+5.0%)
drivers/net/Kconfig | 1 +
drivers/net/virtio_net.c | 497 ++++++++++++++++++++-------------------
2 files changed, 251 insertions(+), 247 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 17108c359216..b2fd90466bab 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -452,6 +452,7 @@ config VIRTIO_NET
depends on VIRTIO
select NET_FAILOVER
select DIMLIB
+ select PAGE_POOL
help
This is the virtual network driver for virtio. It can be used with
QEMU based VMMs (like KVM or Xen). Say Y or M.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 72d6a9c6a5a2..a85d75a7f539 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,7 @@
#include <net/netdev_rx_queue.h>
#include <net/netdev_queues.h>
#include <net/xdp_sock_drv.h>
+#include <net/page_pool/helpers.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -290,14 +291,6 @@ struct virtnet_interrupt_coalesce {
u32 max_usecs;
};
-/* The dma information of pages allocated at a time. */
-struct virtnet_rq_dma {
- dma_addr_t addr;
- u32 ref;
- u16 len;
- u16 need_sync;
-};
-
/* Internal representation of a send virtqueue */
struct send_queue {
/* Virtqueue associated with this send _queue */
@@ -356,8 +349,10 @@ struct receive_queue {
/* Average packet length for mergeable receive buffers. */
struct ewma_pkt_len mrg_avg_pkt_len;
- /* Page frag for packet buffer allocation. */
- struct page_frag alloc_frag;
+ struct page_pool *page_pool;
+
+ /* True if page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
+ bool use_page_pool_dma;
/* RX: fragments + linear part + virtio header */
struct scatterlist sg[MAX_SKB_FRAGS + 2];
@@ -370,9 +365,6 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
- /* Record the last dma info to free after new pages is allocated. */
- struct virtnet_rq_dma *last_dma;
-
struct xsk_buff_pool *xsk_pool;
/* xdp rxq used by xsk */
@@ -521,11 +513,14 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
struct virtnet_rq_stats *stats);
static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
struct sk_buff *skb, u8 flags);
-static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
+static struct sk_buff *virtnet_skb_append_frag(struct receive_queue *rq,
+ struct sk_buff *head_skb,
struct sk_buff *curr_skb,
struct page *page, void *buf,
int len, int truesize);
static void virtnet_xsk_completed(struct send_queue *sq, int num);
+static void free_unused_bufs(struct virtnet_info *vi);
+static void virtnet_del_vqs(struct virtnet_info *vi);
enum virtnet_xmit_type {
VIRTNET_XMIT_TYPE_SKB,
@@ -709,12 +704,10 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
static void virtnet_rq_free_buf(struct virtnet_info *vi,
struct receive_queue *rq, void *buf)
{
- if (vi->mergeable_rx_bufs)
- put_page(virt_to_head_page(buf));
- else if (vi->big_packets)
+ if (!rq->page_pool)
give_pages(rq, buf);
else
- put_page(virt_to_head_page(buf));
+ page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
}
static void enable_rx_mode_work(struct virtnet_info *vi)
@@ -876,10 +869,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
skb = virtnet_build_skb(buf, truesize, p - buf, len);
if (unlikely(!skb))
return NULL;
+ /* Big packets mode chains pages via page->private, which is
+ * incompatible with the way page_pool uses page->private.
+ * Currently, big packets mode doesn't use page pools.
+ */
+ if (!rq->page_pool) {
+ page = (struct page *)page->private;
+ if (page)
+ give_pages(rq, page);
+ }
- page = (struct page *)page->private;
- if (page)
- give_pages(rq, page);
goto ok;
}
@@ -925,133 +924,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr = skb_vnet_common_hdr(skb);
memcpy(hdr, hdr_p, hdr_len);
if (page_to_free)
- put_page(page_to_free);
+ page_pool_put_page(rq->page_pool, page_to_free, -1, true);
return skb;
}
-static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)
-{
- struct virtnet_info *vi = rq->vq->vdev->priv;
- struct page *page = virt_to_head_page(buf);
- struct virtnet_rq_dma *dma;
- void *head;
- int offset;
-
- BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
-
- head = page_address(page);
-
- dma = head;
-
- --dma->ref;
-
- if (dma->need_sync && len) {
- offset = buf - (head + sizeof(*dma));
-
- virtqueue_map_sync_single_range_for_cpu(rq->vq, dma->addr,
- offset, len,
- DMA_FROM_DEVICE);
- }
-
- if (dma->ref)
- return;
-
- virtqueue_unmap_single_attrs(rq->vq, dma->addr, dma->len,
- DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
- put_page(page);
-}
-
static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
{
- struct virtnet_info *vi = rq->vq->vdev->priv;
- void *buf;
-
- BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
+ BUG_ON(!rq->page_pool);
- buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
- if (buf)
- virtnet_rq_unmap(rq, buf, *len);
-
- return buf;
-}
-
-static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
-{
- struct virtnet_info *vi = rq->vq->vdev->priv;
- struct virtnet_rq_dma *dma;
- dma_addr_t addr;
- u32 offset;
- void *head;
-
- BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
-
- head = page_address(rq->alloc_frag.page);
-
- offset = buf - head;
-
- dma = head;
-
- addr = dma->addr - sizeof(*dma) + offset;
-
- sg_init_table(rq->sg, 1);
- sg_fill_dma(rq->sg, addr, len);
-}
-
-static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
-{
- struct page_frag *alloc_frag = &rq->alloc_frag;
- struct virtnet_info *vi = rq->vq->vdev->priv;
- struct virtnet_rq_dma *dma;
- void *buf, *head;
- dma_addr_t addr;
-
- BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
-
- head = page_address(alloc_frag->page);
-
- dma = head;
-
- /* new pages */
- if (!alloc_frag->offset) {
- if (rq->last_dma) {
- /* Now, the new page is allocated, the last dma
- * will not be used. So the dma can be unmapped
- * if the ref is 0.
- */
- virtnet_rq_unmap(rq, rq->last_dma, 0);
- rq->last_dma = NULL;
- }
-
- dma->len = alloc_frag->size - sizeof(*dma);
-
- addr = virtqueue_map_single_attrs(rq->vq, dma + 1,
- dma->len, DMA_FROM_DEVICE, 0);
- if (virtqueue_map_mapping_error(rq->vq, addr))
- return NULL;
-
- dma->addr = addr;
- dma->need_sync = virtqueue_map_need_sync(rq->vq, addr);
-
- /* Add a reference to dma to prevent the entire dma from
- * being released during error handling. This reference
- * will be freed after the pages are no longer used.
- */
- get_page(alloc_frag->page);
- dma->ref = 1;
- alloc_frag->offset = sizeof(*dma);
-
- rq->last_dma = dma;
- }
-
- ++dma->ref;
-
- buf = head + alloc_frag->offset;
-
- get_page(alloc_frag->page);
- alloc_frag->offset += size;
-
- return buf;
+ return virtqueue_get_buf_ctx(rq->vq, len, ctx);
}
static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
@@ -1067,9 +949,6 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
return;
}
- if (!vi->big_packets || vi->mergeable_rx_bufs)
- virtnet_rq_unmap(rq, buf, 0);
-
virtnet_rq_free_buf(vi, rq, buf);
}
@@ -1335,7 +1214,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
truesize = len;
- curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
+ curr_skb = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
buf, len, truesize);
if (!curr_skb) {
put_page(page);
@@ -1771,7 +1650,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
return ret;
}
-static void put_xdp_frags(struct xdp_buff *xdp)
+static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
{
struct skb_shared_info *shinfo;
struct page *xdp_page;
@@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
shinfo = xdp_get_shared_info_from_buff(xdp);
for (i = 0; i < shinfo->nr_frags; i++) {
xdp_page = skb_frag_page(&shinfo->frags[i]);
- put_page(xdp_page);
+ page_pool_put_page(rq->page_pool, xdp_page, -1, true);
}
}
}
@@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
if (page_off + *len + tailroom > PAGE_SIZE)
return NULL;
- page = alloc_page(GFP_ATOMIC);
+ page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
if (!page)
return NULL;
@@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
p = virt_to_head_page(buf);
off = buf - page_address(p);
+ if (rq->use_page_pool_dma)
+ page_pool_dma_sync_for_cpu(rq->page_pool, p,
+ off, buflen);
+
if (check_mergeable_len(dev, ctx, buflen)) {
- put_page(p);
+ page_pool_put_page(rq->page_pool, p, -1, true);
goto err_buf;
}
@@ -1905,38 +1788,36 @@ static struct page *xdp_linearize_page(struct net_device *dev,
* is sending packet larger than the MTU.
*/
if ((page_off + buflen + tailroom) > PAGE_SIZE) {
- put_page(p);
+ page_pool_put_page(rq->page_pool, p, -1, true);
goto err_buf;
}
memcpy(page_address(page) + page_off,
page_address(p) + off, buflen);
page_off += buflen;
- put_page(p);
+ page_pool_put_page(rq->page_pool, p, -1, true);
}
/* Headroom does not contribute to packet length */
*len = page_off - XDP_PACKET_HEADROOM;
return page;
err_buf:
- __free_pages(page, 0);
+ page_pool_put_page(rq->page_pool, page, -1, true);
return NULL;
}
static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
unsigned int xdp_headroom,
void *buf,
- unsigned int len)
+ unsigned int len,
+ unsigned int buflen)
{
unsigned int header_offset;
unsigned int headroom;
- unsigned int buflen;
struct sk_buff *skb;
header_offset = VIRTNET_RX_PAD + xdp_headroom;
headroom = vi->hdr_len + header_offset;
- buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
skb = virtnet_build_skb(buf, buflen, headroom, len);
if (unlikely(!skb))
@@ -1955,6 +1836,7 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
void *buf,
unsigned int xdp_headroom,
unsigned int len,
+ unsigned int buflen,
unsigned int *xdp_xmit,
struct virtnet_rq_stats *stats)
{
@@ -1963,7 +1845,6 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
struct virtio_net_hdr_mrg_rxbuf *hdr = buf + header_offset;
struct page *page = virt_to_head_page(buf);
struct page *xdp_page;
- unsigned int buflen;
struct xdp_buff xdp;
struct sk_buff *skb;
unsigned int metasize = 0;
@@ -1976,9 +1857,6 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
if (unlikely(hdr->hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
goto err_xdp;
- buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
-
if (unlikely(xdp_headroom < virtnet_get_headroom(vi))) {
int offset = buf - page_address(page) + header_offset;
unsigned int tlen = len + vi->hdr_len;
@@ -1996,7 +1874,7 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
goto err_xdp;
buf = page_address(xdp_page);
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
page = xdp_page;
}
@@ -2028,13 +1906,15 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
if (metasize)
skb_metadata_set(skb, metasize);
+ skb_mark_for_recycle(skb);
+
return skb;
err_xdp:
u64_stats_inc(&stats->xdp_drops);
err:
u64_stats_inc(&stats->drops);
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
xdp_xmit:
return NULL;
}
@@ -2047,7 +1927,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
unsigned int *xdp_xmit,
struct virtnet_rq_stats *stats)
{
- unsigned int xdp_headroom = (unsigned long)ctx;
+ unsigned int xdp_headroom = mergeable_ctx_to_headroom(ctx);
+ unsigned int buflen = mergeable_ctx_to_truesize(ctx);
struct page *page = virt_to_head_page(buf);
struct sk_buff *skb;
@@ -2056,6 +1937,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
*/
buf -= VIRTNET_RX_PAD + xdp_headroom;
+ if (rq->use_page_pool_dma) {
+ int offset = buf - page_address(page) +
+ VIRTNET_RX_PAD + xdp_headroom;
+
+ page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
+ }
+
len -= vi->hdr_len;
u64_stats_add(&stats->bytes, len);
@@ -2073,21 +1961,23 @@ static struct sk_buff *receive_small(struct net_device *dev,
xdp_prog = rcu_dereference(rq->xdp_prog);
if (xdp_prog) {
skb = receive_small_xdp(dev, vi, rq, xdp_prog, buf,
- xdp_headroom, len, xdp_xmit,
- stats);
+ xdp_headroom, len, buflen,
+ xdp_xmit, stats);
rcu_read_unlock();
return skb;
}
rcu_read_unlock();
}
- skb = receive_small_build_skb(vi, xdp_headroom, buf, len);
- if (likely(skb))
+ skb = receive_small_build_skb(vi, xdp_headroom, buf, len, buflen);
+ if (likely(skb)) {
+ skb_mark_for_recycle(skb);
return skb;
+ }
err:
u64_stats_inc(&stats->drops);
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
return NULL;
}
@@ -2142,7 +2032,7 @@ static void mergeable_buf_free(struct receive_queue *rq, int num_buf,
}
u64_stats_add(&stats->bytes, len);
page = virt_to_head_page(buf);
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
}
}
@@ -2252,8 +2142,12 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
page = virt_to_head_page(buf);
offset = buf - page_address(page);
+ if (rq->use_page_pool_dma)
+ page_pool_dma_sync_for_cpu(rq->page_pool, page,
+ offset, len);
+
if (check_mergeable_len(dev, ctx, len)) {
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
goto err;
}
@@ -2272,7 +2166,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
return 0;
err:
- put_xdp_frags(xdp);
+ put_xdp_frags(rq, xdp);
return -EINVAL;
}
@@ -2337,7 +2231,7 @@ static void *mergeable_xdp_get_buf(struct virtnet_info *vi,
if (*len + xdp_room > PAGE_SIZE)
return NULL;
- xdp_page = alloc_page(GFP_ATOMIC);
+ xdp_page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
if (!xdp_page)
return NULL;
@@ -2347,7 +2241,7 @@ static void *mergeable_xdp_get_buf(struct virtnet_info *vi,
*frame_sz = PAGE_SIZE;
- put_page(*page);
+ page_pool_put_page(rq->page_pool, *page, -1, true);
*page = xdp_page;
@@ -2393,6 +2287,8 @@ static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
head_skb = build_skb_from_xdp_buff(dev, vi, &xdp, xdp_frags_truesz);
if (unlikely(!head_skb))
break;
+
+ skb_mark_for_recycle(head_skb);
return head_skb;
case XDP_TX:
@@ -2403,10 +2299,10 @@ static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
break;
}
- put_xdp_frags(&xdp);
+ put_xdp_frags(rq, &xdp);
err_xdp:
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
mergeable_buf_free(rq, num_buf, dev, stats);
u64_stats_inc(&stats->xdp_drops);
@@ -2414,7 +2310,8 @@ static struct sk_buff *receive_mergeable_xdp(struct net_device *dev,
return NULL;
}
-static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
+static struct sk_buff *virtnet_skb_append_frag(struct receive_queue *rq,
+ struct sk_buff *head_skb,
struct sk_buff *curr_skb,
struct page *page, void *buf,
int len, int truesize)
@@ -2429,6 +2326,9 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
if (unlikely(!nskb))
return NULL;
+ if (head_skb->pp_recycle)
+ skb_mark_for_recycle(nskb);
+
if (curr_skb == head_skb)
skb_shinfo(curr_skb)->frag_list = nskb;
else
@@ -2446,7 +2346,10 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
offset = buf - page_address(page);
if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
- put_page(page);
+ if (head_skb->pp_recycle)
+ page_pool_put_page(rq->page_pool, page, -1, true);
+ else
+ put_page(page);
skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
len, truesize);
} else {
@@ -2475,6 +2378,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
head_skb = NULL;
+
+ if (rq->use_page_pool_dma)
+ page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
+
u64_stats_add(&stats->bytes, len - vi->hdr_len);
if (check_mergeable_len(dev, ctx, len))
@@ -2499,6 +2406,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
if (unlikely(!curr_skb))
goto err_skb;
+
+ skb_mark_for_recycle(head_skb);
while (--num_buf) {
buf = virtnet_rq_get_buf(rq, &len, &ctx);
if (unlikely(!buf)) {
@@ -2513,11 +2422,17 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
u64_stats_add(&stats->bytes, len);
page = virt_to_head_page(buf);
+ if (rq->use_page_pool_dma) {
+ offset = buf - page_address(page);
+ page_pool_dma_sync_for_cpu(rq->page_pool, page,
+ offset, len);
+ }
+
if (check_mergeable_len(dev, ctx, len))
goto err_skb;
truesize = mergeable_ctx_to_truesize(ctx);
- curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
+ curr_skb = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
buf, len, truesize);
if (!curr_skb)
goto err_skb;
@@ -2527,7 +2442,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
return head_skb;
err_skb:
- put_page(page);
+ page_pool_put_page(rq->page_pool, page, -1, true);
mergeable_buf_free(rq, num_buf, dev, stats);
err_buf:
@@ -2658,40 +2573,54 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
virtnet_receive_done(vi, rq, skb, flags);
}
-/* Unlike mergeable buffers, all buffers are allocated to the
- * same size, except for the headroom. For this reason we do
- * not need to use mergeable_len_to_ctx here - it is enough
- * to store the headroom as the context ignoring the truesize.
+static int virtnet_rq_submit(struct receive_queue *rq, char *buf,
+ int len, void *ctx, gfp_t gfp)
+{
+ if (rq->use_page_pool_dma) {
+ struct page *page = virt_to_head_page(buf);
+ dma_addr_t addr = page_pool_get_dma_addr(page) +
+ (buf - (char *)page_address(page));
+
+ sg_init_table(rq->sg, 1);
+ sg_fill_dma(rq->sg, addr, len);
+ return virtqueue_add_inbuf_premapped(rq->vq, rq->sg, 1,
+ buf, ctx, gfp);
+ }
+
+ sg_init_one(rq->sg, buf, len);
+ return virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
+}
+
+/* With page_pool, the actual allocation may exceed the requested size
+ * when the remaining page fragment can't fit another buffer. Encode
+ * the actual allocation size in ctx so build_skb() gets the correct
+ * buflen for truesize accounting.
*/
static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
gfp_t gfp)
{
- char *buf;
unsigned int xdp_headroom = virtnet_get_headroom(vi);
- void *ctx = (void *)(unsigned long)xdp_headroom;
- int len = vi->hdr_len + VIRTNET_RX_PAD + GOOD_PACKET_LEN + xdp_headroom;
+ unsigned int len = vi->hdr_len + VIRTNET_RX_PAD + GOOD_PACKET_LEN + xdp_headroom;
+ unsigned int alloc_len;
+ char *buf;
+ void *ctx;
int err;
len = SKB_DATA_ALIGN(len) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- if (unlikely(!skb_page_frag_refill(len, &rq->alloc_frag, gfp)))
- return -ENOMEM;
-
- buf = virtnet_rq_alloc(rq, len, gfp);
+ alloc_len = len;
+ buf = page_pool_alloc_va(rq->page_pool, &alloc_len, gfp);
if (unlikely(!buf))
return -ENOMEM;
buf += VIRTNET_RX_PAD + xdp_headroom;
- virtnet_rq_init_one_sg(rq, buf, vi->hdr_len + GOOD_PACKET_LEN);
-
- err = virtqueue_add_inbuf_premapped(rq->vq, rq->sg, 1, buf, ctx, gfp);
- if (err < 0) {
- virtnet_rq_unmap(rq, buf, 0);
- put_page(virt_to_head_page(buf));
- }
+ ctx = mergeable_len_to_ctx(alloc_len, xdp_headroom);
+ err = virtnet_rq_submit(rq, buf, vi->hdr_len + GOOD_PACKET_LEN, ctx, gfp);
+ if (err < 0)
+ page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
return err;
}
@@ -2764,13 +2693,12 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
static int add_recvbuf_mergeable(struct virtnet_info *vi,
struct receive_queue *rq, gfp_t gfp)
{
- struct page_frag *alloc_frag = &rq->alloc_frag;
unsigned int headroom = virtnet_get_headroom(vi);
unsigned int tailroom = headroom ? sizeof(struct skb_shared_info) : 0;
unsigned int room = SKB_DATA_ALIGN(headroom + tailroom);
- unsigned int len, hole;
- void *ctx;
+ unsigned int len, alloc_len;
char *buf;
+ void *ctx;
int err;
/* Extra tailroom is needed to satisfy XDP's assumption. This
@@ -2779,39 +2707,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
*/
len = get_mergeable_buf_len(rq, &rq->mrg_avg_pkt_len, room);
- if (unlikely(!skb_page_frag_refill(len + room, alloc_frag, gfp)))
- return -ENOMEM;
-
- if (!alloc_frag->offset && len + room + sizeof(struct virtnet_rq_dma) > alloc_frag->size)
- len -= sizeof(struct virtnet_rq_dma);
-
- buf = virtnet_rq_alloc(rq, len + room, gfp);
+ alloc_len = len + room;
+ buf = page_pool_alloc_va(rq->page_pool, &alloc_len, gfp);
if (unlikely(!buf))
return -ENOMEM;
buf += headroom; /* advance address leaving hole at front of pkt */
- hole = alloc_frag->size - alloc_frag->offset;
- if (hole < len + room) {
- /* To avoid internal fragmentation, if there is very likely not
- * enough space for another buffer, add the remaining space to
- * the current buffer.
- * XDP core assumes that frame_size of xdp_buff and the length
- * of the frag are PAGE_SIZE, so we disable the hole mechanism.
- */
- if (!headroom)
- len += hole;
- alloc_frag->offset += hole;
- }
- virtnet_rq_init_one_sg(rq, buf, len);
+ if (!headroom)
+ len = alloc_len - room;
ctx = mergeable_len_to_ctx(len + room, headroom);
- err = virtqueue_add_inbuf_premapped(rq->vq, rq->sg, 1, buf, ctx, gfp);
- if (err < 0) {
- virtnet_rq_unmap(rq, buf, 0);
- put_page(virt_to_head_page(buf));
- }
+ err = virtnet_rq_submit(rq, buf, len, ctx, gfp);
+
+ if (err < 0)
+ page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
return err;
}
@@ -2963,7 +2874,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
int packets = 0;
void *buf;
- if (!vi->big_packets || vi->mergeable_rx_bufs) {
+ if (rq->page_pool) {
void *ctx;
while (packets < budget &&
(buf = virtnet_rq_get_buf(rq, &len, &ctx))) {
@@ -3128,7 +3039,10 @@ static int virtnet_enable_queue_pair(struct virtnet_info *vi, int qp_index)
return err;
err = xdp_rxq_info_reg_mem_model(&vi->rq[qp_index].xdp_rxq,
- MEM_TYPE_PAGE_SHARED, NULL);
+ vi->rq[qp_index].page_pool ?
+ MEM_TYPE_PAGE_POOL :
+ MEM_TYPE_PAGE_SHARED,
+ vi->rq[qp_index].page_pool);
if (err < 0)
goto err_xdp_reg_mem_model;
@@ -3168,6 +3082,82 @@ static void virtnet_update_settings(struct virtnet_info *vi)
vi->duplex = duplex;
}
+static int virtnet_create_page_pools(struct virtnet_info *vi)
+{
+ int i, err;
+
+ if (vi->big_packets && !vi->mergeable_rx_bufs)
+ return 0;
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ struct receive_queue *rq = &vi->rq[i];
+ struct page_pool_params pp_params = { 0 };
+ struct device *dma_dev;
+
+ if (rq->page_pool)
+ continue;
+
+ if (rq->xsk_pool)
+ continue;
+
+ pp_params.order = 0;
+ pp_params.pool_size = virtqueue_get_vring_size(rq->vq);
+ pp_params.nid = dev_to_node(vi->vdev->dev.parent);
+ pp_params.netdev = vi->dev;
+ pp_params.napi = &rq->napi;
+
+ /* Use page_pool DMA mapping if backend supports DMA API.
+ * DMA_SYNC_DEV is needed for non-coherent archs on recycle.
+ */
+ dma_dev = virtqueue_dma_dev(rq->vq);
+ if (dma_dev) {
+ pp_params.dev = dma_dev;
+ pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
+ pp_params.dma_dir = DMA_FROM_DEVICE;
+ pp_params.max_len = PAGE_SIZE;
+ pp_params.offset = 0;
+ rq->use_page_pool_dma = true;
+ } else {
+ /* No DMA API (e.g., VDUSE): page_pool for allocation only. */
+ pp_params.flags = 0;
+ rq->use_page_pool_dma = false;
+ }
+
+ rq->page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(rq->page_pool)) {
+ err = PTR_ERR(rq->page_pool);
+ rq->page_pool = NULL;
+ goto err_cleanup;
+ }
+ }
+ return 0;
+
+err_cleanup:
+ while (--i >= 0) {
+ struct receive_queue *rq = &vi->rq[i];
+
+ if (rq->page_pool) {
+ page_pool_destroy(rq->page_pool);
+ rq->page_pool = NULL;
+ }
+ }
+ return err;
+}
+
+static void virtnet_destroy_page_pools(struct virtnet_info *vi)
+{
+ int i;
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ struct receive_queue *rq = &vi->rq[i];
+
+ if (rq->page_pool) {
+ page_pool_destroy(rq->page_pool);
+ rq->page_pool = NULL;
+ }
+ }
+}
+
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -5715,6 +5705,10 @@ static int virtnet_restore_up(struct virtio_device *vdev)
if (err)
return err;
+ err = virtnet_create_page_pools(vi);
+ if (err)
+ goto err_del_vqs;
+
virtio_device_ready(vdev);
enable_rx_mode_work(vi);
@@ -5724,12 +5718,24 @@ static int virtnet_restore_up(struct virtio_device *vdev)
err = virtnet_open(vi->dev);
rtnl_unlock();
if (err)
- return err;
+ goto err_destroy_pools;
}
netif_tx_lock_bh(vi->dev);
netif_device_attach(vi->dev);
netif_tx_unlock_bh(vi->dev);
+ return 0;
+
+err_destroy_pools:
+ virtio_reset_device(vdev);
+ free_unused_bufs(vi);
+ virtnet_destroy_page_pools(vi);
+ virtnet_del_vqs(vi);
+ return err;
+
+err_del_vqs:
+ virtio_reset_device(vdev);
+ virtnet_del_vqs(vi);
return err;
}
@@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
/* In big_packets mode, xdp cannot work, so there is no need to
* initialize xsk of rq.
*/
- if (vi->big_packets && !vi->mergeable_rx_bufs)
+ if (!vi->rq[qid].page_pool)
return -ENOENT;
if (qid >= vi->curr_queue_pairs)
@@ -6287,17 +6293,6 @@ static void free_receive_bufs(struct virtnet_info *vi)
rtnl_unlock();
}
-static void free_receive_page_frags(struct virtnet_info *vi)
-{
- int i;
- for (i = 0; i < vi->max_queue_pairs; i++)
- if (vi->rq[i].alloc_frag.page) {
- if (vi->rq[i].last_dma)
- virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
- put_page(vi->rq[i].alloc_frag.page);
- }
-}
-
static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf)
{
struct virtnet_info *vi = vq->vdev->priv;
@@ -6401,7 +6396,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
vqs_info = kzalloc_objs(*vqs_info, total_vqs);
if (!vqs_info)
goto err_vqs_info;
- if (!vi->big_packets || vi->mergeable_rx_bufs) {
+ if (vi->mergeable_rx_bufs || !vi->big_packets) {
ctx = kzalloc_objs(*ctx, total_vqs);
if (!ctx)
goto err_ctx;
@@ -6441,10 +6436,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);
vi->sq[i].vq = vqs[txq2vq(i)];
}
-
/* run here: ret == 0. */
-
err_find:
kfree(ctx);
err_ctx:
@@ -6945,6 +6938,14 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free;
}
+ /* Create page pools for receive queues.
+ * Page pools are created at probe time so they can be used
+ * with premapped DMA addresses throughout the device lifetime.
+ */
+ err = virtnet_create_page_pools(vi);
+ if (err)
+ goto free_irq_moder;
+
#ifdef CONFIG_SYSFS
if (vi->mergeable_rx_bufs)
dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
@@ -6958,7 +6959,7 @@ static int virtnet_probe(struct virtio_device *vdev)
vi->failover = net_failover_create(vi->dev);
if (IS_ERR(vi->failover)) {
err = PTR_ERR(vi->failover);
- goto free_vqs;
+ goto free_page_pools;
}
}
@@ -7075,9 +7076,11 @@ static int virtnet_probe(struct virtio_device *vdev)
unregister_netdev(dev);
free_failover:
net_failover_destroy(vi->failover);
-free_vqs:
+free_page_pools:
+ virtnet_destroy_page_pools(vi);
+free_irq_moder:
+ virtnet_free_irq_moder(vi);
virtio_reset_device(vdev);
- free_receive_page_frags(vi);
virtnet_del_vqs(vi);
free:
free_netdev(dev);
@@ -7102,7 +7105,7 @@ static void remove_vq_common(struct virtnet_info *vi)
free_receive_bufs(vi);
- free_receive_page_frags(vi);
+ virtnet_destroy_page_pools(vi);
virtnet_del_vqs(vi);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-10 18:31 [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation Vishwanath Seshagiri
@ 2026-03-13 7:51 ` Jason Wang
2026-03-13 9:26 ` Vishwanath Seshagiri
2026-03-13 16:50 ` Vishwanath Seshagiri
2026-03-16 9:56 ` Michael S. Tsirkin
` (2 subsequent siblings)
3 siblings, 2 replies; 21+ messages in thread
From: Jason Wang @ 2026-03-13 7:51 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Wed, Mar 11, 2026 at 2:31 AM Vishwanath Seshagiri <vishs@meta.com> wrote:
>
> Use page_pool for RX buffer allocation in mergeable and small buffer
> modes to enable page recycling and avoid repeated page allocator calls.
> skb_mark_for_recycle() enables page reuse in the network stack.
>
> Big packets mode is unchanged because it uses page->private for linked
> list chaining of multiple pages per buffer, which conflicts with
> page_pool's internal use of page->private.
>
> Implement conditional DMA premapping using virtqueue_dma_dev():
> - When non-NULL (vhost, virtio-pci): use PP_FLAG_DMA_MAP with page_pool
> handling DMA mapping, submit via virtqueue_add_inbuf_premapped()
> - When NULL (VDUSE, direct physical): page_pool handles allocation only,
> submit via virtqueue_add_inbuf_ctx()
>
> This preserves the DMA premapping optimization from commit 31f3cd4e5756b
> ("virtio-net: rq submits premapped per-buffer") while adding page_pool
> support as a prerequisite for future zero-copy features (devmem TCP,
> io_uring ZCRX).
>
> Page pools are created in probe and destroyed in remove (not open/close),
> following existing driver behavior where RX buffers remain in virtqueues
> across interface state changes.
>
> Signed-off-by: Vishwanath Seshagiri <vishs@meta.com>
> ---
> Changes in v11:
> - add_recvbuf_small: encode alloc_len and xdp_headroom in ctx via
> mergeable_len_to_ctx() so receive_small() recovers the actual buflen
> via mergeable_ctx_to_truesize() (Michael S. Tsirkin)
> - receive_small_build_skb, receive_small_xdp: accept buflen parameter
> instead of recomputing it, to use the actual allocation size
> - v10:
> https://lore.kernel.org/virtualization/9752a952-195d-4da3-bc7a-5a4a1f2fd2ca@meta.com/
>
> Changes in v10:
> - add_recvbuf_small: use alloc_len to avoid clobbering len; v9 feedback
> was about truesize under-accounting, not variable naming — misunderstood
> the comment in v9
> - v9:
> https://lore.kernel.org/virtualization/20260302041005.1627210-1-vishs@meta.com/
>
> Changes in v9:
> - Fix virtnet_skb_append_frag() for XSK callers (Michael S. Tsirkin)
> - v8:
> https://lore.kernel.org/virtualization/e824c5a3-cfe0-4d11-958f-c3ec82d11d37@meta.com/
>
> Changes in v8:
> - Remove virtnet_no_page_pool() helper, replace with direct !rq->page_pool
> checks or inlined conditions (Xuan Zhuo)
> - Extract virtnet_rq_submit() helper to consolidate DMA/non-DMA buffer
> submission in add_recvbuf_small() and add_recvbuf_mergeable()
> - Add skb_mark_for_recycle(nskb) for overflow frag_list skbs in
> virtnet_skb_append_frag() to ensure page_pool pages are returned to
> the pool instead of freed via put_page()
> - Rebase on net-next (kzalloc_objs API)
> - v7:
> https://lore.kernel.org/virtualization/20260210014305.3236342-1-vishs@meta.com/
>
> Changes in v7:
> - Replace virtnet_put_page() helper with direct page_pool_put_page()
> calls (Xuan Zhuo)
> - Add virtnet_no_page_pool() helper to consolidate big_packets mode check
> (Michael S. Tsirkin)
> - Add DMA sync_for_cpu for subsequent buffers in xdp_linearize_page() when
> use_page_pool_dma is set (Michael S. Tsirkin)
> - Remove unused pp_params.dev assignment in non-DMA path
> - Add page pool recreation in virtnet_restore_up() for freeze/restore support (Chris Mason's
> Review Prompt)
> - v6:
> https://lore.kernel.org/virtualization/20260208175410.1910001-1-vishs@meta.com/
>
> Changes in v6:
> - Drop page_pool_frag_offset_add() helper and switch to page_pool_alloc_va();
> page_pool_alloc_netmem() already handles internal fragmentation internally
> (Jakub Kicinski)
> - v5:
> https://lore.kernel.org/virtualization/20260206002715.1885869-1-vishs@meta.com/
>
> Benchmark results:
>
> Configuration: pktgen TX -> tap -> vhost-net | virtio-net RX -> XDP_DROP
>
> Small packets (64 bytes, mrg_rxbuf=off):
> 1Q: 853,493 -> 868,923 pps (+1.8%)
> 2Q: 1,655,793 -> 1,696,707 pps (+2.5%)
> 4Q: 3,143,375 -> 3,302,511 pps (+5.1%)
> 8Q: 6,082,590 -> 6,156,894 pps (+1.2%)
>
> Mergeable RX (64 bytes):
> 1Q: 766,168 -> 814,493 pps (+6.3%)
> 2Q: 1,384,871 -> 1,670,639 pps (+20.6%)
> 4Q: 2,773,081 -> 3,080,574 pps (+11.1%)
> 8Q: 5,600,615 -> 6,043,891 pps (+7.9%)
>
> Mergeable RX (1500 bytes):
> 1Q: 741,579 -> 785,442 pps (+5.9%)
> 2Q: 1,310,043 -> 1,534,554 pps (+17.1%)
> 4Q: 2,748,700 -> 2,890,582 pps (+5.2%)
> 8Q: 5,348,589 -> 5,618,664 pps (+5.0%)
>
> drivers/net/Kconfig | 1 +
> drivers/net/virtio_net.c | 497 ++++++++++++++++++++-------------------
> 2 files changed, 251 insertions(+), 247 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 17108c359216..b2fd90466bab 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -452,6 +452,7 @@ config VIRTIO_NET
> depends on VIRTIO
> select NET_FAILOVER
> select DIMLIB
> + select PAGE_POOL
> help
> This is the virtual network driver for virtio. It can be used with
> QEMU based VMMs (like KVM or Xen). Say Y or M.
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 72d6a9c6a5a2..a85d75a7f539 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -26,6 +26,7 @@
> #include <net/netdev_rx_queue.h>
> #include <net/netdev_queues.h>
> #include <net/xdp_sock_drv.h>
> +#include <net/page_pool/helpers.h>
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -290,14 +291,6 @@ struct virtnet_interrupt_coalesce {
> u32 max_usecs;
> };
>
> -/* The dma information of pages allocated at a time. */
> -struct virtnet_rq_dma {
> - dma_addr_t addr;
> - u32 ref;
> - u16 len;
> - u16 need_sync;
> -};
> -
> /* Internal representation of a send virtqueue */
> struct send_queue {
> /* Virtqueue associated with this send _queue */
> @@ -356,8 +349,10 @@ struct receive_queue {
> /* Average packet length for mergeable receive buffers. */
> struct ewma_pkt_len mrg_avg_pkt_len;
>
> - /* Page frag for packet buffer allocation. */
> - struct page_frag alloc_frag;
> + struct page_pool *page_pool;
> +
> + /* True if page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
> + bool use_page_pool_dma;
>
> /* RX: fragments + linear part + virtio header */
> struct scatterlist sg[MAX_SKB_FRAGS + 2];
> @@ -370,9 +365,6 @@ struct receive_queue {
>
> struct xdp_rxq_info xdp_rxq;
>
> - /* Record the last dma info to free after new pages is allocated. */
> - struct virtnet_rq_dma *last_dma;
> -
> struct xsk_buff_pool *xsk_pool;
>
> /* xdp rxq used by xsk */
> @@ -521,11 +513,14 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> struct virtnet_rq_stats *stats);
> static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
> struct sk_buff *skb, u8 flags);
> -static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
> +static struct sk_buff *virtnet_skb_append_frag(struct receive_queue *rq,
> + struct sk_buff *head_skb,
> struct sk_buff *curr_skb,
> struct page *page, void *buf,
> int len, int truesize);
> static void virtnet_xsk_completed(struct send_queue *sq, int num);
> +static void free_unused_bufs(struct virtnet_info *vi);
> +static void virtnet_del_vqs(struct virtnet_info *vi);
>
> enum virtnet_xmit_type {
> VIRTNET_XMIT_TYPE_SKB,
> @@ -709,12 +704,10 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
> static void virtnet_rq_free_buf(struct virtnet_info *vi,
> struct receive_queue *rq, void *buf)
> {
> - if (vi->mergeable_rx_bufs)
> - put_page(virt_to_head_page(buf));
> - else if (vi->big_packets)
> + if (!rq->page_pool)
> give_pages(rq, buf);
> else
> - put_page(virt_to_head_page(buf));
> + page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
> }
>
> static void enable_rx_mode_work(struct virtnet_info *vi)
> @@ -876,10 +869,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> skb = virtnet_build_skb(buf, truesize, p - buf, len);
> if (unlikely(!skb))
> return NULL;
> + /* Big packets mode chains pages via page->private, which is
> + * incompatible with the way page_pool uses page->private.
> + * Currently, big packets mode doesn't use page pools.
> + */
> + if (!rq->page_pool) {
> + page = (struct page *)page->private;
> + if (page)
> + give_pages(rq, page);
> + }
>
> - page = (struct page *)page->private;
> - if (page)
> - give_pages(rq, page);
> goto ok;
> }
>
> @@ -925,133 +924,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> hdr = skb_vnet_common_hdr(skb);
> memcpy(hdr, hdr_p, hdr_len);
> if (page_to_free)
> - put_page(page_to_free);
> + page_pool_put_page(rq->page_pool, page_to_free, -1, true);
>
> return skb;
> }
>
> -static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)
> -{
> - struct virtnet_info *vi = rq->vq->vdev->priv;
> - struct page *page = virt_to_head_page(buf);
> - struct virtnet_rq_dma *dma;
> - void *head;
> - int offset;
> -
> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> -
> - head = page_address(page);
> -
> - dma = head;
> -
> - --dma->ref;
> -
> - if (dma->need_sync && len) {
> - offset = buf - (head + sizeof(*dma));
> -
> - virtqueue_map_sync_single_range_for_cpu(rq->vq, dma->addr,
> - offset, len,
> - DMA_FROM_DEVICE);
> - }
> -
> - if (dma->ref)
> - return;
> -
> - virtqueue_unmap_single_attrs(rq->vq, dma->addr, dma->len,
> - DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
> - put_page(page);
> -}
> -
> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> {
> - struct virtnet_info *vi = rq->vq->vdev->priv;
> - void *buf;
> -
> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> + BUG_ON(!rq->page_pool);
>
> - buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> - if (buf)
> - virtnet_rq_unmap(rq, buf, *len);
> -
> - return buf;
> -}
> -
> -static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
> -{
> - struct virtnet_info *vi = rq->vq->vdev->priv;
> - struct virtnet_rq_dma *dma;
> - dma_addr_t addr;
> - u32 offset;
> - void *head;
> -
> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> -
> - head = page_address(rq->alloc_frag.page);
> -
> - offset = buf - head;
> -
> - dma = head;
> -
> - addr = dma->addr - sizeof(*dma) + offset;
> -
> - sg_init_table(rq->sg, 1);
> - sg_fill_dma(rq->sg, addr, len);
> -}
> -
> -static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
> -{
> - struct page_frag *alloc_frag = &rq->alloc_frag;
> - struct virtnet_info *vi = rq->vq->vdev->priv;
> - struct virtnet_rq_dma *dma;
> - void *buf, *head;
> - dma_addr_t addr;
> -
> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> -
> - head = page_address(alloc_frag->page);
> -
> - dma = head;
> -
> - /* new pages */
> - if (!alloc_frag->offset) {
> - if (rq->last_dma) {
> - /* Now, the new page is allocated, the last dma
> - * will not be used. So the dma can be unmapped
> - * if the ref is 0.
> - */
> - virtnet_rq_unmap(rq, rq->last_dma, 0);
> - rq->last_dma = NULL;
> - }
> -
> - dma->len = alloc_frag->size - sizeof(*dma);
> -
> - addr = virtqueue_map_single_attrs(rq->vq, dma + 1,
> - dma->len, DMA_FROM_DEVICE, 0);
> - if (virtqueue_map_mapping_error(rq->vq, addr))
> - return NULL;
> -
> - dma->addr = addr;
> - dma->need_sync = virtqueue_map_need_sync(rq->vq, addr);
> -
> - /* Add a reference to dma to prevent the entire dma from
> - * being released during error handling. This reference
> - * will be freed after the pages are no longer used.
> - */
> - get_page(alloc_frag->page);
> - dma->ref = 1;
> - alloc_frag->offset = sizeof(*dma);
> -
> - rq->last_dma = dma;
> - }
> -
> - ++dma->ref;
> -
> - buf = head + alloc_frag->offset;
> -
> - get_page(alloc_frag->page);
> - alloc_frag->offset += size;
> -
> - return buf;
> + return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> }
>
> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> @@ -1067,9 +949,6 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> return;
> }
>
> - if (!vi->big_packets || vi->mergeable_rx_bufs)
> - virtnet_rq_unmap(rq, buf, 0);
> -
> virtnet_rq_free_buf(vi, rq, buf);
> }
>
> @@ -1335,7 +1214,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
>
> truesize = len;
>
> - curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
> + curr_skb = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
> buf, len, truesize);
> if (!curr_skb) {
> put_page(page);
> @@ -1771,7 +1650,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> return ret;
> }
>
> -static void put_xdp_frags(struct xdp_buff *xdp)
> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
> {
> struct skb_shared_info *shinfo;
> struct page *xdp_page;
> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
> shinfo = xdp_get_shared_info_from_buff(xdp);
> for (i = 0; i < shinfo->nr_frags; i++) {
> xdp_page = skb_frag_page(&shinfo->frags[i]);
> - put_page(xdp_page);
> + page_pool_put_page(rq->page_pool, xdp_page, -1, true);
> }
> }
> }
> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> if (page_off + *len + tailroom > PAGE_SIZE)
> return NULL;
>
> - page = alloc_page(GFP_ATOMIC);
> + page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
> if (!page)
> return NULL;
>
> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> p = virt_to_head_page(buf);
> off = buf - page_address(p);
>
> + if (rq->use_page_pool_dma)
> + page_pool_dma_sync_for_cpu(rq->page_pool, p,
> + off, buflen);
Intresting, I think we need a patch for -stable to sync for cpu as
well (and probably the XDP_TX path).
> +
> if (check_mergeable_len(dev, ctx, buflen)) {
> - put_page(p);
> + page_pool_put_page(rq->page_pool, p, -1, true);
> goto err_buf;
> }
>
> @@ -1905,38 +1788,36 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> * is sending packet larger than the MTU.
> */
> if ((page_off + buflen + tailroom) > PAGE_SIZE) {
> - put_page(p);
> + page_pool_put_page(rq->page_pool, p, -1, true);
> goto err_buf;
> }
>
> memcpy(page_address(page) + page_off,
> page_address(p) + off, buflen);
> page_off += buflen;
> - put_page(p);
> + page_pool_put_page(rq->page_pool, p, -1, true);
> }
>
> /* Headroom does not contribute to packet length */
> *len = page_off - XDP_PACKET_HEADROOM;
> return page;
> err_buf:
> - __free_pages(page, 0);
> + page_pool_put_page(rq->page_pool, page, -1, true);
> return NULL;
> }
>
> static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
> unsigned int xdp_headroom,
> void *buf,
> - unsigned int len)
> + unsigned int len,
> + unsigned int buflen)
> {
> unsigned int header_offset;
> unsigned int headroom;
> - unsigned int buflen;
> struct sk_buff *skb;
>
> header_offset = VIRTNET_RX_PAD + xdp_headroom;
> headroom = vi->hdr_len + header_offset;
> - buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
> - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>
Any reason for remvoing this?
The rest looks fine.
Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-13 7:51 ` Jason Wang
@ 2026-03-13 9:26 ` Vishwanath Seshagiri
2026-03-16 7:41 ` Jason Wang
2026-03-13 16:50 ` Vishwanath Seshagiri
1 sibling, 1 reply; 21+ messages in thread
From: Vishwanath Seshagiri @ 2026-03-13 9:26 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On 3/13/26 1:21 PM, Jason Wang wrote:
> On Wed, Mar 11, 2026 at 2:31 AM Vishwanath Seshagiri <vishs@meta.com> wrote:
>>
>> Use page_pool for RX buffer allocation in mergeable and small buffer
>> modes to enable page recycling and avoid repeated page allocator calls.
>> skb_mark_for_recycle() enables page reuse in the network stack.
>>
>> Big packets mode is unchanged because it uses page->private for linked
>> list chaining of multiple pages per buffer, which conflicts with
>> page_pool's internal use of page->private.
>>
>> Implement conditional DMA premapping using virtqueue_dma_dev():
>> - When non-NULL (vhost, virtio-pci): use PP_FLAG_DMA_MAP with page_pool
>> handling DMA mapping, submit via virtqueue_add_inbuf_premapped()
>> - When NULL (VDUSE, direct physical): page_pool handles allocation only,
>> submit via virtqueue_add_inbuf_ctx()
>>
>> This preserves the DMA premapping optimization from commit 31f3cd4e5756b
>> ("virtio-net: rq submits premapped per-buffer") while adding page_pool
>> support as a prerequisite for future zero-copy features (devmem TCP,
>> io_uring ZCRX).
>>
>> Page pools are created in probe and destroyed in remove (not open/close),
>> following existing driver behavior where RX buffers remain in virtqueues
>> across interface state changes.
>>
>> Signed-off-by: Vishwanath Seshagiri <vishs@meta.com>
>> ---
>> Changes in v11:
>> - add_recvbuf_small: encode alloc_len and xdp_headroom in ctx via
>> mergeable_len_to_ctx() so receive_small() recovers the actual buflen
>> via mergeable_ctx_to_truesize() (Michael S. Tsirkin)
>> - receive_small_build_skb, receive_small_xdp: accept buflen parameter
>> instead of recomputing it, to use the actual allocation size
>> - v10:
>> https://lore.kernel.org/virtualization/9752a952-195d-4da3-bc7a-5a4a1f2fd2ca@meta.com/
>>
>> Changes in v10:
>> - add_recvbuf_small: use alloc_len to avoid clobbering len; v9 feedback
>> was about truesize under-accounting, not variable naming — misunderstood
>> the comment in v9
>> - v9:
>> https://lore.kernel.org/virtualization/20260302041005.1627210-1-vishs@meta.com/
>>
>> Changes in v9:
>> - Fix virtnet_skb_append_frag() for XSK callers (Michael S. Tsirkin)
>> - v8:
>> https://lore.kernel.org/virtualization/e824c5a3-cfe0-4d11-958f-c3ec82d11d37@meta.com/
>>
>> Changes in v8:
>> - Remove virtnet_no_page_pool() helper, replace with direct !rq->page_pool
>> checks or inlined conditions (Xuan Zhuo)
>> - Extract virtnet_rq_submit() helper to consolidate DMA/non-DMA buffer
>> submission in add_recvbuf_small() and add_recvbuf_mergeable()
>> - Add skb_mark_for_recycle(nskb) for overflow frag_list skbs in
>> virtnet_skb_append_frag() to ensure page_pool pages are returned to
>> the pool instead of freed via put_page()
>> - Rebase on net-next (kzalloc_objs API)
>> - v7:
>> https://lore.kernel.org/virtualization/20260210014305.3236342-1-vishs@meta.com/
>>
>> Changes in v7:
>> - Replace virtnet_put_page() helper with direct page_pool_put_page()
>> calls (Xuan Zhuo)
>> - Add virtnet_no_page_pool() helper to consolidate big_packets mode check
>> (Michael S. Tsirkin)
>> - Add DMA sync_for_cpu for subsequent buffers in xdp_linearize_page() when
>> use_page_pool_dma is set (Michael S. Tsirkin)
>> - Remove unused pp_params.dev assignment in non-DMA path
>> - Add page pool recreation in virtnet_restore_up() for freeze/restore support (Chris Mason's
>> Review Prompt)
>> - v6:
>> https://lore.kernel.org/virtualization/20260208175410.1910001-1-vishs@meta.com/
>>
>> Changes in v6:
>> - Drop page_pool_frag_offset_add() helper and switch to page_pool_alloc_va();
>> page_pool_alloc_netmem() already handles internal fragmentation internally
>> (Jakub Kicinski)
>> - v5:
>> https://lore.kernel.org/virtualization/20260206002715.1885869-1-vishs@meta.com/
>>
>> Benchmark results:
>>
>> Configuration: pktgen TX -> tap -> vhost-net | virtio-net RX -> XDP_DROP
>>
>> Small packets (64 bytes, mrg_rxbuf=off):
>> 1Q: 853,493 -> 868,923 pps (+1.8%)
>> 2Q: 1,655,793 -> 1,696,707 pps (+2.5%)
>> 4Q: 3,143,375 -> 3,302,511 pps (+5.1%)
>> 8Q: 6,082,590 -> 6,156,894 pps (+1.2%)
>>
>> Mergeable RX (64 bytes):
>> 1Q: 766,168 -> 814,493 pps (+6.3%)
>> 2Q: 1,384,871 -> 1,670,639 pps (+20.6%)
>> 4Q: 2,773,081 -> 3,080,574 pps (+11.1%)
>> 8Q: 5,600,615 -> 6,043,891 pps (+7.9%)
>>
>> Mergeable RX (1500 bytes):
>> 1Q: 741,579 -> 785,442 pps (+5.9%)
>> 2Q: 1,310,043 -> 1,534,554 pps (+17.1%)
>> 4Q: 2,748,700 -> 2,890,582 pps (+5.2%)
>> 8Q: 5,348,589 -> 5,618,664 pps (+5.0%)
>>
>> drivers/net/Kconfig | 1 +
>> drivers/net/virtio_net.c | 497 ++++++++++++++++++++-------------------
>> 2 files changed, 251 insertions(+), 247 deletions(-)
>>
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 17108c359216..b2fd90466bab 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -452,6 +452,7 @@ config VIRTIO_NET
>> depends on VIRTIO
>> select NET_FAILOVER
>> select DIMLIB
>> + select PAGE_POOL
>> help
>> This is the virtual network driver for virtio. It can be used with
>> QEMU based VMMs (like KVM or Xen). Say Y or M.
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 72d6a9c6a5a2..a85d75a7f539 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -26,6 +26,7 @@
>> #include <net/netdev_rx_queue.h>
>> #include <net/netdev_queues.h>
>> #include <net/xdp_sock_drv.h>
>> +#include <net/page_pool/helpers.h>
>>
>> static int napi_weight = NAPI_POLL_WEIGHT;
>> module_param(napi_weight, int, 0444);
>> @@ -290,14 +291,6 @@ struct virtnet_interrupt_coalesce {
>> u32 max_usecs;
>> };
>>
>> -/* The dma information of pages allocated at a time. */
>> -struct virtnet_rq_dma {
>> - dma_addr_t addr;
>> - u32 ref;
>> - u16 len;
>> - u16 need_sync;
>> -};
>> -
>> /* Internal representation of a send virtqueue */
>> struct send_queue {
>> /* Virtqueue associated with this send _queue */
>> @@ -356,8 +349,10 @@ struct receive_queue {
>> /* Average packet length for mergeable receive buffers. */
>> struct ewma_pkt_len mrg_avg_pkt_len;
>>
>> - /* Page frag for packet buffer allocation. */
>> - struct page_frag alloc_frag;
>> + struct page_pool *page_pool;
>> +
>> + /* True if page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
>> + bool use_page_pool_dma;
>>
>> /* RX: fragments + linear part + virtio header */
>> struct scatterlist sg[MAX_SKB_FRAGS + 2];
>> @@ -370,9 +365,6 @@ struct receive_queue {
>>
>> struct xdp_rxq_info xdp_rxq;
>>
>> - /* Record the last dma info to free after new pages is allocated. */
>> - struct virtnet_rq_dma *last_dma;
>> -
>> struct xsk_buff_pool *xsk_pool;
>>
>> /* xdp rxq used by xsk */
>> @@ -521,11 +513,14 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
>> struct virtnet_rq_stats *stats);
>> static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
>> struct sk_buff *skb, u8 flags);
>> -static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
>> +static struct sk_buff *virtnet_skb_append_frag(struct receive_queue *rq,
>> + struct sk_buff *head_skb,
>> struct sk_buff *curr_skb,
>> struct page *page, void *buf,
>> int len, int truesize);
>> static void virtnet_xsk_completed(struct send_queue *sq, int num);
>> +static void free_unused_bufs(struct virtnet_info *vi);
>> +static void virtnet_del_vqs(struct virtnet_info *vi);
>>
>> enum virtnet_xmit_type {
>> VIRTNET_XMIT_TYPE_SKB,
>> @@ -709,12 +704,10 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
>> static void virtnet_rq_free_buf(struct virtnet_info *vi,
>> struct receive_queue *rq, void *buf)
>> {
>> - if (vi->mergeable_rx_bufs)
>> - put_page(virt_to_head_page(buf));
>> - else if (vi->big_packets)
>> + if (!rq->page_pool)
>> give_pages(rq, buf);
>> else
>> - put_page(virt_to_head_page(buf));
>> + page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
>> }
>>
>> static void enable_rx_mode_work(struct virtnet_info *vi)
>> @@ -876,10 +869,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> skb = virtnet_build_skb(buf, truesize, p - buf, len);
>> if (unlikely(!skb))
>> return NULL;
>> + /* Big packets mode chains pages via page->private, which is
>> + * incompatible with the way page_pool uses page->private.
>> + * Currently, big packets mode doesn't use page pools.
>> + */
>> + if (!rq->page_pool) {
>> + page = (struct page *)page->private;
>> + if (page)
>> + give_pages(rq, page);
>> + }
>>
>> - page = (struct page *)page->private;
>> - if (page)
>> - give_pages(rq, page);
>> goto ok;
>> }
>>
>> @@ -925,133 +924,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> hdr = skb_vnet_common_hdr(skb);
>> memcpy(hdr, hdr_p, hdr_len);
>> if (page_to_free)
>> - put_page(page_to_free);
>> + page_pool_put_page(rq->page_pool, page_to_free, -1, true);
>>
>> return skb;
>> }
>>
>> -static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)
>> -{
>> - struct virtnet_info *vi = rq->vq->vdev->priv;
>> - struct page *page = virt_to_head_page(buf);
>> - struct virtnet_rq_dma *dma;
>> - void *head;
>> - int offset;
>> -
>> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
>> -
>> - head = page_address(page);
>> -
>> - dma = head;
>> -
>> - --dma->ref;
>> -
>> - if (dma->need_sync && len) {
>> - offset = buf - (head + sizeof(*dma));
>> -
>> - virtqueue_map_sync_single_range_for_cpu(rq->vq, dma->addr,
>> - offset, len,
>> - DMA_FROM_DEVICE);
>> - }
>> -
>> - if (dma->ref)
>> - return;
>> -
>> - virtqueue_unmap_single_attrs(rq->vq, dma->addr, dma->len,
>> - DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
>> - put_page(page);
>> -}
>> -
>> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
>> {
>> - struct virtnet_info *vi = rq->vq->vdev->priv;
>> - void *buf;
>> -
>> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
>> + BUG_ON(!rq->page_pool);
>>
>> - buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> - if (buf)
>> - virtnet_rq_unmap(rq, buf, *len);
>> -
>> - return buf;
>> -}
>> -
>> -static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
>> -{
>> - struct virtnet_info *vi = rq->vq->vdev->priv;
>> - struct virtnet_rq_dma *dma;
>> - dma_addr_t addr;
>> - u32 offset;
>> - void *head;
>> -
>> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
>> -
>> - head = page_address(rq->alloc_frag.page);
>> -
>> - offset = buf - head;
>> -
>> - dma = head;
>> -
>> - addr = dma->addr - sizeof(*dma) + offset;
>> -
>> - sg_init_table(rq->sg, 1);
>> - sg_fill_dma(rq->sg, addr, len);
>> -}
>> -
>> -static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
>> -{
>> - struct page_frag *alloc_frag = &rq->alloc_frag;
>> - struct virtnet_info *vi = rq->vq->vdev->priv;
>> - struct virtnet_rq_dma *dma;
>> - void *buf, *head;
>> - dma_addr_t addr;
>> -
>> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
>> -
>> - head = page_address(alloc_frag->page);
>> -
>> - dma = head;
>> -
>> - /* new pages */
>> - if (!alloc_frag->offset) {
>> - if (rq->last_dma) {
>> - /* Now, the new page is allocated, the last dma
>> - * will not be used. So the dma can be unmapped
>> - * if the ref is 0.
>> - */
>> - virtnet_rq_unmap(rq, rq->last_dma, 0);
>> - rq->last_dma = NULL;
>> - }
>> -
>> - dma->len = alloc_frag->size - sizeof(*dma);
>> -
>> - addr = virtqueue_map_single_attrs(rq->vq, dma + 1,
>> - dma->len, DMA_FROM_DEVICE, 0);
>> - if (virtqueue_map_mapping_error(rq->vq, addr))
>> - return NULL;
>> -
>> - dma->addr = addr;
>> - dma->need_sync = virtqueue_map_need_sync(rq->vq, addr);
>> -
>> - /* Add a reference to dma to prevent the entire dma from
>> - * being released during error handling. This reference
>> - * will be freed after the pages are no longer used.
>> - */
>> - get_page(alloc_frag->page);
>> - dma->ref = 1;
>> - alloc_frag->offset = sizeof(*dma);
>> -
>> - rq->last_dma = dma;
>> - }
>> -
>> - ++dma->ref;
>> -
>> - buf = head + alloc_frag->offset;
>> -
>> - get_page(alloc_frag->page);
>> - alloc_frag->offset += size;
>> -
>> - return buf;
>> + return virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> }
>>
>> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>> @@ -1067,9 +949,6 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>> return;
>> }
>>
>> - if (!vi->big_packets || vi->mergeable_rx_bufs)
>> - virtnet_rq_unmap(rq, buf, 0);
>> -
>> virtnet_rq_free_buf(vi, rq, buf);
>> }
>>
>> @@ -1335,7 +1214,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
>>
>> truesize = len;
>>
>> - curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
>> + curr_skb = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
>> buf, len, truesize);
>> if (!curr_skb) {
>> put_page(page);
>> @@ -1771,7 +1650,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>> return ret;
>> }
>>
>> -static void put_xdp_frags(struct xdp_buff *xdp)
>> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
>> {
>> struct skb_shared_info *shinfo;
>> struct page *xdp_page;
>> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
>> shinfo = xdp_get_shared_info_from_buff(xdp);
>> for (i = 0; i < shinfo->nr_frags; i++) {
>> xdp_page = skb_frag_page(&shinfo->frags[i]);
>> - put_page(xdp_page);
>> + page_pool_put_page(rq->page_pool, xdp_page, -1, true);
>> }
>> }
>> }
>> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
>> if (page_off + *len + tailroom > PAGE_SIZE)
>> return NULL;
>>
>> - page = alloc_page(GFP_ATOMIC);
>> + page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
>> if (!page)
>> return NULL;
>>
>> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
>> p = virt_to_head_page(buf);
>> off = buf - page_address(p);
>>
>> + if (rq->use_page_pool_dma)
>> + page_pool_dma_sync_for_cpu(rq->page_pool, p,
>> + off, buflen);
>
> Intresting, I think we need a patch for -stable to sync for cpu as
> well (and probably the XDP_TX path).
>
>
>> +
>> if (check_mergeable_len(dev, ctx, buflen)) {
>> - put_page(p);
>> + page_pool_put_page(rq->page_pool, p, -1, true);
>> goto err_buf;
>> }
>>
>> @@ -1905,38 +1788,36 @@ static struct page *xdp_linearize_page(struct net_device *dev,
>> * is sending packet larger than the MTU.
>> */
>> if ((page_off + buflen + tailroom) > PAGE_SIZE) {
>> - put_page(p);
>> + page_pool_put_page(rq->page_pool, p, -1, true);
>> goto err_buf;
>> }
>>
>> memcpy(page_address(page) + page_off,
>> page_address(p) + off, buflen);
>> page_off += buflen;
>> - put_page(p);
>> + page_pool_put_page(rq->page_pool, p, -1, true);
>> }
>>
>> /* Headroom does not contribute to packet length */
>> *len = page_off - XDP_PACKET_HEADROOM;
>> return page;
>> err_buf:
>> - __free_pages(page, 0);
>> + page_pool_put_page(rq->page_pool, page, -1, true);
>> return NULL;
>> }
>>
>> static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
>> unsigned int xdp_headroom,
>> void *buf,
>> - unsigned int len)
>> + unsigned int len,
>> + unsigned int buflen)
>> {
>> unsigned int header_offset;
>> unsigned int headroom;
>> - unsigned int buflen;
>> struct sk_buff *skb;
>>
>> header_offset = VIRTNET_RX_PAD + xdp_headroom;
>> headroom = vi->hdr_len + header_offset;
>> - buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
>> - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>>
>
> Any reason for remvoing this?
page_pool_alloc_va() can return a larger allocation than requested as it
appends remaining fragment space to avoid truesize underestimation
(comment in page_pool_alloc_netmem() in helpers.h). The old hardcoded
computation would always produce the requested ~512 bytes, ignoring any
extra space page_pool gave us, so build_skb() would set skb->truesize
too low. To pass the real size through: add_recvbuf_small() encodes
alloc_len and xdp_headroom into ctx via mergeable_len_to_ctx(alloc_len,
xdp_headroom). On the receive side, receive_small() extracts it with
mergeable_ctx_to_truesize(ctx) and passes it as the buflen parameter to
receive_small_build_skb().
>
> The rest looks fine.
>
> Thanks
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-13 7:51 ` Jason Wang
2026-03-13 9:26 ` Vishwanath Seshagiri
@ 2026-03-13 16:50 ` Vishwanath Seshagiri
2026-03-16 7:35 ` Jason Wang
1 sibling, 1 reply; 21+ messages in thread
From: Vishwanath Seshagiri @ 2026-03-13 16:50 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On 3/13/26 1:21 PM, Jason Wang wrote:
>> -static void put_xdp_frags(struct xdp_buff *xdp)
>> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
>> {
>> struct skb_shared_info *shinfo;
>> struct page *xdp_page;
>> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
>> shinfo = xdp_get_shared_info_from_buff(xdp);
>> for (i = 0; i < shinfo->nr_frags; i++) {
>> xdp_page = skb_frag_page(&shinfo->frags[i]);
>> - put_page(xdp_page);
>> + page_pool_put_page(rq->page_pool, xdp_page, -1, true);
>> }
>> }
>> }
>> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
>> if (page_off + *len + tailroom > PAGE_SIZE)
>> return NULL;
>>
>> - page = alloc_page(GFP_ATOMIC);
>> + page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
>> if (!page)
>> return NULL;
>>
>> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
>> p = virt_to_head_page(buf);
>> off = buf - page_address(p);
>>
>> + if (rq->use_page_pool_dma)
>> + page_pool_dma_sync_for_cpu(rq->page_pool, p,
>> + off, buflen);
>
> Intresting, I think we need a patch for -stable to sync for cpu as
> well (and probably the XDP_TX path).
In the old code, I believed the sync was handled implicitly inside
virtnet_rq_get_buf() -> virtnet_rq_unmap() before data was accessed.
Could you point me at the specific path you're concerned about for
-stable? I want to make sure I understand the issue correctly before
sending a fix.
>
>
>> +
>> if (check_mergeable_len(dev, ctx, buflen)) {
>> - put_page(p);
>> + page_pool_put_page(rq->page_pool, p, -1, true);
>> goto err_buf;
>> }
Sorry for missing that comment in the earlier email
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-13 16:50 ` Vishwanath Seshagiri
@ 2026-03-16 7:35 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2026-03-16 7:35 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Sat, Mar 14, 2026 at 12:51 AM Vishwanath Seshagiri <vishs@meta.com> wrote:
>
>
>
> On 3/13/26 1:21 PM, Jason Wang wrote:
> >> -static void put_xdp_frags(struct xdp_buff *xdp)
> >> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
> >> {
> >> struct skb_shared_info *shinfo;
> >> struct page *xdp_page;
> >> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
> >> shinfo = xdp_get_shared_info_from_buff(xdp);
> >> for (i = 0; i < shinfo->nr_frags; i++) {
> >> xdp_page = skb_frag_page(&shinfo->frags[i]);
> >> - put_page(xdp_page);
> >> + page_pool_put_page(rq->page_pool, xdp_page, -1, true);
> >> }
> >> }
> >> }
> >> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >> if (page_off + *len + tailroom > PAGE_SIZE)
> >> return NULL;
> >>
> >> - page = alloc_page(GFP_ATOMIC);
> >> + page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
> >> if (!page)
> >> return NULL;
> >>
> >> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >> p = virt_to_head_page(buf);
> >> off = buf - page_address(p);
> >>
> >> + if (rq->use_page_pool_dma)
> >> + page_pool_dma_sync_for_cpu(rq->page_pool, p,
> >> + off, buflen);
> >
> > Intresting, I think we need a patch for -stable to sync for cpu as
> > well (and probably the XDP_TX path).
>
> In the old code, I believed the sync was handled implicitly inside
> virtnet_rq_get_buf() -> virtnet_rq_unmap() before data was accessed.
> Could you point me at the specific path you're concerned about for
> -stable? I want to make sure I understand the issue correctly before
> sending a fix.
You're right; the code seems fine.
(Note that, when big_packets is true but small is used, we will go
virtqueue_get_buf(), so the unamp is done via deatch_buf().
>
> >
> >
> >> +
> >> if (check_mergeable_len(dev, ctx, buflen)) {
> >> - put_page(p);
> >> + page_pool_put_page(rq->page_pool, p, -1, true);
> >> goto err_buf;
> >> }
>
> Sorry for missing that comment in the earlier email
No worries.
Thanks
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-13 9:26 ` Vishwanath Seshagiri
@ 2026-03-16 7:41 ` Jason Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2026-03-16 7:41 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Fri, Mar 13, 2026 at 5:27 PM Vishwanath Seshagiri <vishs@meta.com> wrote:
>
> On 3/13/26 1:21 PM, Jason Wang wrote:
> > On Wed, Mar 11, 2026 at 2:31 AM Vishwanath Seshagiri <vishs@meta.com> wrote:
> >>
> >> Use page_pool for RX buffer allocation in mergeable and small buffer
> >> modes to enable page recycling and avoid repeated page allocator calls.
> >> skb_mark_for_recycle() enables page reuse in the network stack.
> >>
> >> Big packets mode is unchanged because it uses page->private for linked
> >> list chaining of multiple pages per buffer, which conflicts with
> >> page_pool's internal use of page->private.
> >>
> >> Implement conditional DMA premapping using virtqueue_dma_dev():
> >> - When non-NULL (vhost, virtio-pci): use PP_FLAG_DMA_MAP with page_pool
> >> handling DMA mapping, submit via virtqueue_add_inbuf_premapped()
> >> - When NULL (VDUSE, direct physical): page_pool handles allocation only,
> >> submit via virtqueue_add_inbuf_ctx()
> >>
> >> This preserves the DMA premapping optimization from commit 31f3cd4e5756b
> >> ("virtio-net: rq submits premapped per-buffer") while adding page_pool
> >> support as a prerequisite for future zero-copy features (devmem TCP,
> >> io_uring ZCRX).
> >>
> >> Page pools are created in probe and destroyed in remove (not open/close),
> >> following existing driver behavior where RX buffers remain in virtqueues
> >> across interface state changes.
> >>
> >> Signed-off-by: Vishwanath Seshagiri <vishs@meta.com>
> >> ---
> >> Changes in v11:
> >> - add_recvbuf_small: encode alloc_len and xdp_headroom in ctx via
> >> mergeable_len_to_ctx() so receive_small() recovers the actual buflen
> >> via mergeable_ctx_to_truesize() (Michael S. Tsirkin)
> >> - receive_small_build_skb, receive_small_xdp: accept buflen parameter
> >> instead of recomputing it, to use the actual allocation size
> >> - v10:
> >> https://lore.kernel.org/virtualization/9752a952-195d-4da3-bc7a-5a4a1f2fd2ca@meta.com/
> >>
> >> Changes in v10:
> >> - add_recvbuf_small: use alloc_len to avoid clobbering len; v9 feedback
> >> was about truesize under-accounting, not variable naming — misunderstood
> >> the comment in v9
> >> - v9:
> >> https://lore.kernel.org/virtualization/20260302041005.1627210-1-vishs@meta.com/
> >>
> >> Changes in v9:
> >> - Fix virtnet_skb_append_frag() for XSK callers (Michael S. Tsirkin)
> >> - v8:
> >> https://lore.kernel.org/virtualization/e824c5a3-cfe0-4d11-958f-c3ec82d11d37@meta.com/
> >>
> >> Changes in v8:
> >> - Remove virtnet_no_page_pool() helper, replace with direct !rq->page_pool
> >> checks or inlined conditions (Xuan Zhuo)
> >> - Extract virtnet_rq_submit() helper to consolidate DMA/non-DMA buffer
> >> submission in add_recvbuf_small() and add_recvbuf_mergeable()
> >> - Add skb_mark_for_recycle(nskb) for overflow frag_list skbs in
> >> virtnet_skb_append_frag() to ensure page_pool pages are returned to
> >> the pool instead of freed via put_page()
> >> - Rebase on net-next (kzalloc_objs API)
> >> - v7:
> >> https://lore.kernel.org/virtualization/20260210014305.3236342-1-vishs@meta.com/
> >>
> >> Changes in v7:
> >> - Replace virtnet_put_page() helper with direct page_pool_put_page()
> >> calls (Xuan Zhuo)
> >> - Add virtnet_no_page_pool() helper to consolidate big_packets mode check
> >> (Michael S. Tsirkin)
> >> - Add DMA sync_for_cpu for subsequent buffers in xdp_linearize_page() when
> >> use_page_pool_dma is set (Michael S. Tsirkin)
> >> - Remove unused pp_params.dev assignment in non-DMA path
> >> - Add page pool recreation in virtnet_restore_up() for freeze/restore support (Chris Mason's
> >> Review Prompt)
> >> - v6:
> >> https://lore.kernel.org/virtualization/20260208175410.1910001-1-vishs@meta.com/
> >>
> >> Changes in v6:
> >> - Drop page_pool_frag_offset_add() helper and switch to page_pool_alloc_va();
> >> page_pool_alloc_netmem() already handles internal fragmentation internally
> >> (Jakub Kicinski)
> >> - v5:
> >> https://lore.kernel.org/virtualization/20260206002715.1885869-1-vishs@meta.com/
> >>
> >> Benchmark results:
> >>
> >> Configuration: pktgen TX -> tap -> vhost-net | virtio-net RX -> XDP_DROP
> >>
> >> Small packets (64 bytes, mrg_rxbuf=off):
> >> 1Q: 853,493 -> 868,923 pps (+1.8%)
> >> 2Q: 1,655,793 -> 1,696,707 pps (+2.5%)
> >> 4Q: 3,143,375 -> 3,302,511 pps (+5.1%)
> >> 8Q: 6,082,590 -> 6,156,894 pps (+1.2%)
> >>
> >> Mergeable RX (64 bytes):
> >> 1Q: 766,168 -> 814,493 pps (+6.3%)
> >> 2Q: 1,384,871 -> 1,670,639 pps (+20.6%)
> >> 4Q: 2,773,081 -> 3,080,574 pps (+11.1%)
> >> 8Q: 5,600,615 -> 6,043,891 pps (+7.9%)
> >>
> >> Mergeable RX (1500 bytes):
> >> 1Q: 741,579 -> 785,442 pps (+5.9%)
> >> 2Q: 1,310,043 -> 1,534,554 pps (+17.1%)
> >> 4Q: 2,748,700 -> 2,890,582 pps (+5.2%)
> >> 8Q: 5,348,589 -> 5,618,664 pps (+5.0%)
> >>
> >> drivers/net/Kconfig | 1 +
> >> drivers/net/virtio_net.c | 497 ++++++++++++++++++++-------------------
> >> 2 files changed, 251 insertions(+), 247 deletions(-)
> >>
> >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> >> index 17108c359216..b2fd90466bab 100644
> >> --- a/drivers/net/Kconfig
> >> +++ b/drivers/net/Kconfig
> >> @@ -452,6 +452,7 @@ config VIRTIO_NET
> >> depends on VIRTIO
> >> select NET_FAILOVER
> >> select DIMLIB
> >> + select PAGE_POOL
> >> help
> >> This is the virtual network driver for virtio. It can be used with
> >> QEMU based VMMs (like KVM or Xen). Say Y or M.
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index 72d6a9c6a5a2..a85d75a7f539 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -26,6 +26,7 @@
> >> #include <net/netdev_rx_queue.h>
> >> #include <net/netdev_queues.h>
> >> #include <net/xdp_sock_drv.h>
> >> +#include <net/page_pool/helpers.h>
> >>
> >> static int napi_weight = NAPI_POLL_WEIGHT;
> >> module_param(napi_weight, int, 0444);
> >> @@ -290,14 +291,6 @@ struct virtnet_interrupt_coalesce {
> >> u32 max_usecs;
> >> };
> >>
> >> -/* The dma information of pages allocated at a time. */
> >> -struct virtnet_rq_dma {
> >> - dma_addr_t addr;
> >> - u32 ref;
> >> - u16 len;
> >> - u16 need_sync;
> >> -};
> >> -
> >> /* Internal representation of a send virtqueue */
> >> struct send_queue {
> >> /* Virtqueue associated with this send _queue */
> >> @@ -356,8 +349,10 @@ struct receive_queue {
> >> /* Average packet length for mergeable receive buffers. */
> >> struct ewma_pkt_len mrg_avg_pkt_len;
> >>
> >> - /* Page frag for packet buffer allocation. */
> >> - struct page_frag alloc_frag;
> >> + struct page_pool *page_pool;
> >> +
> >> + /* True if page_pool handles DMA mapping via PP_FLAG_DMA_MAP */
> >> + bool use_page_pool_dma;
> >>
> >> /* RX: fragments + linear part + virtio header */
> >> struct scatterlist sg[MAX_SKB_FRAGS + 2];
> >> @@ -370,9 +365,6 @@ struct receive_queue {
> >>
> >> struct xdp_rxq_info xdp_rxq;
> >>
> >> - /* Record the last dma info to free after new pages is allocated. */
> >> - struct virtnet_rq_dma *last_dma;
> >> -
> >> struct xsk_buff_pool *xsk_pool;
> >>
> >> /* xdp rxq used by xsk */
> >> @@ -521,11 +513,14 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> >> struct virtnet_rq_stats *stats);
> >> static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
> >> struct sk_buff *skb, u8 flags);
> >> -static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
> >> +static struct sk_buff *virtnet_skb_append_frag(struct receive_queue *rq,
> >> + struct sk_buff *head_skb,
> >> struct sk_buff *curr_skb,
> >> struct page *page, void *buf,
> >> int len, int truesize);
> >> static void virtnet_xsk_completed(struct send_queue *sq, int num);
> >> +static void free_unused_bufs(struct virtnet_info *vi);
> >> +static void virtnet_del_vqs(struct virtnet_info *vi);
> >>
> >> enum virtnet_xmit_type {
> >> VIRTNET_XMIT_TYPE_SKB,
> >> @@ -709,12 +704,10 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
> >> static void virtnet_rq_free_buf(struct virtnet_info *vi,
> >> struct receive_queue *rq, void *buf)
> >> {
> >> - if (vi->mergeable_rx_bufs)
> >> - put_page(virt_to_head_page(buf));
> >> - else if (vi->big_packets)
> >> + if (!rq->page_pool)
> >> give_pages(rq, buf);
> >> else
> >> - put_page(virt_to_head_page(buf));
> >> + page_pool_put_page(rq->page_pool, virt_to_head_page(buf), -1, false);
> >> }
> >>
> >> static void enable_rx_mode_work(struct virtnet_info *vi)
> >> @@ -876,10 +869,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >> skb = virtnet_build_skb(buf, truesize, p - buf, len);
> >> if (unlikely(!skb))
> >> return NULL;
> >> + /* Big packets mode chains pages via page->private, which is
> >> + * incompatible with the way page_pool uses page->private.
> >> + * Currently, big packets mode doesn't use page pools.
> >> + */
> >> + if (!rq->page_pool) {
> >> + page = (struct page *)page->private;
> >> + if (page)
> >> + give_pages(rq, page);
> >> + }
> >>
> >> - page = (struct page *)page->private;
> >> - if (page)
> >> - give_pages(rq, page);
> >> goto ok;
> >> }
> >>
> >> @@ -925,133 +924,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >> hdr = skb_vnet_common_hdr(skb);
> >> memcpy(hdr, hdr_p, hdr_len);
> >> if (page_to_free)
> >> - put_page(page_to_free);
> >> + page_pool_put_page(rq->page_pool, page_to_free, -1, true);
> >>
> >> return skb;
> >> }
> >>
> >> -static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)
> >> -{
> >> - struct virtnet_info *vi = rq->vq->vdev->priv;
> >> - struct page *page = virt_to_head_page(buf);
> >> - struct virtnet_rq_dma *dma;
> >> - void *head;
> >> - int offset;
> >> -
> >> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> >> -
> >> - head = page_address(page);
> >> -
> >> - dma = head;
> >> -
> >> - --dma->ref;
> >> -
> >> - if (dma->need_sync && len) {
> >> - offset = buf - (head + sizeof(*dma));
> >> -
> >> - virtqueue_map_sync_single_range_for_cpu(rq->vq, dma->addr,
> >> - offset, len,
> >> - DMA_FROM_DEVICE);
> >> - }
> >> -
> >> - if (dma->ref)
> >> - return;
> >> -
> >> - virtqueue_unmap_single_attrs(rq->vq, dma->addr, dma->len,
> >> - DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
> >> - put_page(page);
> >> -}
> >> -
> >> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> >> {
> >> - struct virtnet_info *vi = rq->vq->vdev->priv;
> >> - void *buf;
> >> -
> >> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> >> + BUG_ON(!rq->page_pool);
> >>
> >> - buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> >> - if (buf)
> >> - virtnet_rq_unmap(rq, buf, *len);
> >> -
> >> - return buf;
> >> -}
> >> -
> >> -static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
> >> -{
> >> - struct virtnet_info *vi = rq->vq->vdev->priv;
> >> - struct virtnet_rq_dma *dma;
> >> - dma_addr_t addr;
> >> - u32 offset;
> >> - void *head;
> >> -
> >> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> >> -
> >> - head = page_address(rq->alloc_frag.page);
> >> -
> >> - offset = buf - head;
> >> -
> >> - dma = head;
> >> -
> >> - addr = dma->addr - sizeof(*dma) + offset;
> >> -
> >> - sg_init_table(rq->sg, 1);
> >> - sg_fill_dma(rq->sg, addr, len);
> >> -}
> >> -
> >> -static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
> >> -{
> >> - struct page_frag *alloc_frag = &rq->alloc_frag;
> >> - struct virtnet_info *vi = rq->vq->vdev->priv;
> >> - struct virtnet_rq_dma *dma;
> >> - void *buf, *head;
> >> - dma_addr_t addr;
> >> -
> >> - BUG_ON(vi->big_packets && !vi->mergeable_rx_bufs);
> >> -
> >> - head = page_address(alloc_frag->page);
> >> -
> >> - dma = head;
> >> -
> >> - /* new pages */
> >> - if (!alloc_frag->offset) {
> >> - if (rq->last_dma) {
> >> - /* Now, the new page is allocated, the last dma
> >> - * will not be used. So the dma can be unmapped
> >> - * if the ref is 0.
> >> - */
> >> - virtnet_rq_unmap(rq, rq->last_dma, 0);
> >> - rq->last_dma = NULL;
> >> - }
> >> -
> >> - dma->len = alloc_frag->size - sizeof(*dma);
> >> -
> >> - addr = virtqueue_map_single_attrs(rq->vq, dma + 1,
> >> - dma->len, DMA_FROM_DEVICE, 0);
> >> - if (virtqueue_map_mapping_error(rq->vq, addr))
> >> - return NULL;
> >> -
> >> - dma->addr = addr;
> >> - dma->need_sync = virtqueue_map_need_sync(rq->vq, addr);
> >> -
> >> - /* Add a reference to dma to prevent the entire dma from
> >> - * being released during error handling. This reference
> >> - * will be freed after the pages are no longer used.
> >> - */
> >> - get_page(alloc_frag->page);
> >> - dma->ref = 1;
> >> - alloc_frag->offset = sizeof(*dma);
> >> -
> >> - rq->last_dma = dma;
> >> - }
> >> -
> >> - ++dma->ref;
> >> -
> >> - buf = head + alloc_frag->offset;
> >> -
> >> - get_page(alloc_frag->page);
> >> - alloc_frag->offset += size;
> >> -
> >> - return buf;
> >> + return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> >> }
> >>
> >> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> >> @@ -1067,9 +949,6 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> >> return;
> >> }
> >>
> >> - if (!vi->big_packets || vi->mergeable_rx_bufs)
> >> - virtnet_rq_unmap(rq, buf, 0);
> >> -
> >> virtnet_rq_free_buf(vi, rq, buf);
> >> }
> >>
> >> @@ -1335,7 +1214,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
> >>
> >> truesize = len;
> >>
> >> - curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
> >> + curr_skb = virtnet_skb_append_frag(rq, head_skb, curr_skb, page,
> >> buf, len, truesize);
> >> if (!curr_skb) {
> >> put_page(page);
> >> @@ -1771,7 +1650,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> >> return ret;
> >> }
> >>
> >> -static void put_xdp_frags(struct xdp_buff *xdp)
> >> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
> >> {
> >> struct skb_shared_info *shinfo;
> >> struct page *xdp_page;
> >> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
> >> shinfo = xdp_get_shared_info_from_buff(xdp);
> >> for (i = 0; i < shinfo->nr_frags; i++) {
> >> xdp_page = skb_frag_page(&shinfo->frags[i]);
> >> - put_page(xdp_page);
> >> + page_pool_put_page(rq->page_pool, xdp_page, -1, true);
> >> }
> >> }
> >> }
> >> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >> if (page_off + *len + tailroom > PAGE_SIZE)
> >> return NULL;
> >>
> >> - page = alloc_page(GFP_ATOMIC);
> >> + page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
> >> if (!page)
> >> return NULL;
> >>
> >> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >> p = virt_to_head_page(buf);
> >> off = buf - page_address(p);
> >>
> >> + if (rq->use_page_pool_dma)
> >> + page_pool_dma_sync_for_cpu(rq->page_pool, p,
> >> + off, buflen);
> >
> > Intresting, I think we need a patch for -stable to sync for cpu as
> > well (and probably the XDP_TX path).
> >
> >
> >> +
> >> if (check_mergeable_len(dev, ctx, buflen)) {
> >> - put_page(p);
> >> + page_pool_put_page(rq->page_pool, p, -1, true);
> >> goto err_buf;
> >> }
> >>
> >> @@ -1905,38 +1788,36 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >> * is sending packet larger than the MTU.
> >> */
> >> if ((page_off + buflen + tailroom) > PAGE_SIZE) {
> >> - put_page(p);
> >> + page_pool_put_page(rq->page_pool, p, -1, true);
> >> goto err_buf;
> >> }
> >>
> >> memcpy(page_address(page) + page_off,
> >> page_address(p) + off, buflen);
> >> page_off += buflen;
> >> - put_page(p);
> >> + page_pool_put_page(rq->page_pool, p, -1, true);
> >> }
> >>
> >> /* Headroom does not contribute to packet length */
> >> *len = page_off - XDP_PACKET_HEADROOM;
> >> return page;
> >> err_buf:
> >> - __free_pages(page, 0);
> >> + page_pool_put_page(rq->page_pool, page, -1, true);
> >> return NULL;
> >> }
> >>
> >> static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
> >> unsigned int xdp_headroom,
> >> void *buf,
> >> - unsigned int len)
> >> + unsigned int len,
> >> + unsigned int buflen)
> >> {
> >> unsigned int header_offset;
> >> unsigned int headroom;
> >> - unsigned int buflen;
> >> struct sk_buff *skb;
> >>
> >> header_offset = VIRTNET_RX_PAD + xdp_headroom;
> >> headroom = vi->hdr_len + header_offset;
> >> - buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
> >> - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> >>
> >
> > Any reason for remvoing this?
>
> page_pool_alloc_va() can return a larger allocation than requested as it
> appends remaining fragment space to avoid truesize underestimation
> (comment in page_pool_alloc_netmem() in helpers.h). The old hardcoded
> computation would always produce the requested ~512 bytes, ignoring any
> extra space page_pool gave us, so build_skb() would set skb->truesize
> too low. To pass the real size through: add_recvbuf_small() encodes
> alloc_len and xdp_headroom into ctx via mergeable_len_to_ctx(alloc_len,
> xdp_headroom). On the receive side, receive_small() extracts it with
> mergeable_ctx_to_truesize(ctx) and passes it as the buflen parameter to
> receive_small_build_skb().
>
Right.
So
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-10 18:31 [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation Vishwanath Seshagiri
2026-03-13 7:51 ` Jason Wang
@ 2026-03-16 9:56 ` Michael S. Tsirkin
2026-03-16 10:43 ` Michael S. Tsirkin
2026-03-17 2:30 ` patchwork-bot+netdevbpf
2026-03-23 15:01 ` Omar Elghoul
3 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-16 9:56 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Tue, Mar 10, 2026 at 11:31:04AM -0700, Vishwanath Seshagiri wrote:
> @@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
> /* In big_packets mode, xdp cannot work, so there is no need to
> * initialize xsk of rq.
> */
> - if (vi->big_packets && !vi->mergeable_rx_bufs)
> + if (!vi->rq[qid].page_pool)
> return -ENOENT;
>
> if (qid >= vi->curr_queue_pairs)
It seems that a qid that exceeds curr_queue_pairs would previously get
-EINVAL and now gets -ENOENT. Maybe reorder the checks:
if (qid >= vi->curr_queue_pairs)
return -EINVAL;
/* In big_packets mode, xdp cannot work, so there is no need to
* initialize xsk of rq.
*/
if (!vi->rq[qid].page_pool)
return -ENOENT;
Alternatively I think we can completely drop this chunk: we already seem
to init page_pull at all times except for big packets mode, so the
current code is fine I think.
--
MST
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-16 9:56 ` Michael S. Tsirkin
@ 2026-03-16 10:43 ` Michael S. Tsirkin
2026-03-16 11:57 ` Vishwanath Seshagiri
0 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-16 10:43 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Mon, Mar 16, 2026 at 05:56:20AM -0400, Michael S. Tsirkin wrote:
> On Tue, Mar 10, 2026 at 11:31:04AM -0700, Vishwanath Seshagiri wrote:
> > @@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
> > /* In big_packets mode, xdp cannot work, so there is no need to
> > * initialize xsk of rq.
> > */
> > - if (vi->big_packets && !vi->mergeable_rx_bufs)
> > + if (!vi->rq[qid].page_pool)
> > return -ENOENT;
> >
> > if (qid >= vi->curr_queue_pairs)
>
>
>
> It seems that a qid that exceeds curr_queue_pairs would previously get
> -EINVAL and now gets -ENOENT.
Or maybe this if (qid >= vi->curr_queue_pairs) is dead code?
I looked at it some more and I can't find a path where this
triggers.
> Maybe reorder the checks:
>
> if (qid >= vi->curr_queue_pairs)
> return -EINVAL;
>
> /* In big_packets mode, xdp cannot work, so there is no need to
> * initialize xsk of rq.
> */
> if (!vi->rq[qid].page_pool)
> return -ENOENT;
>
>
> Alternatively I think we can completely drop this chunk: we already seem
> to init page_pull at all times except for big packets mode, so the
> current code is fine I think.
>
>
> --
> MST
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-16 10:43 ` Michael S. Tsirkin
@ 2026-03-16 11:57 ` Vishwanath Seshagiri
2026-03-16 12:04 ` Michael S. Tsirkin
0 siblings, 1 reply; 21+ messages in thread
From: Vishwanath Seshagiri @ 2026-03-16 11:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On 3/16/26 4:13 PM, Michael S. Tsirkin wrote:
> On Mon, Mar 16, 2026 at 05:56:20AM -0400, Michael S. Tsirkin wrote:
>> On Tue, Mar 10, 2026 at 11:31:04AM -0700, Vishwanath Seshagiri wrote:
>>> @@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
>>> /* In big_packets mode, xdp cannot work, so there is no need to
>>> * initialize xsk of rq.
>>> */
>>> - if (vi->big_packets && !vi->mergeable_rx_bufs)
>>> + if (!vi->rq[qid].page_pool)
>>> return -ENOENT;
>>>
>>> if (qid >= vi->curr_queue_pairs)
>>
>>
>>
>> It seems that a qid that exceeds curr_queue_pairs would previously get
>> -EINVAL and now gets -ENOENT.
>
> Or maybe this if (qid >= vi->curr_queue_pairs) is dead code?
> I looked at it some more and I can't find a path where this
> triggers.
>
>> Maybe reorder the checks:
>>
>> if (qid >= vi->curr_queue_pairs)
>> return -EINVAL;
>>
>> /* In big_packets mode, xdp cannot work, so there is no need to
>> * initialize xsk of rq.
>> */
>> if (!vi->rq[qid].page_pool)
>> return -ENOENT;
>>
>>
>> Alternatively I think we can completely drop this chunk: we already seem
>> to init page_pull at all times except for big packets mode, so the
>> current code is fine I think.
Yes, I agree qid >= curr_queue_pairs appears to be dead code.
xsk_reg_pool_at_qid() in the XSK core already validates
queue_id < max(real_num_rx_queues, real_num_tx_queues) before
ndo_bpf is called, and real_num_rx_queues == curr_queue_pairs is an
invariant in virtio_net. Both paths hold rtnl_lock so no race is
possible. That said, I'll adopt your suggested reorder for v12 to ensure
vi->rq[qid] isn't accessed before a bounds check, even if the check
is currently redundant.
>>
>>
>> --
>> MST
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-16 11:57 ` Vishwanath Seshagiri
@ 2026-03-16 12:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-16 12:04 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Wei, Matteo Croce, Ilias Apalodimas, netdev, virtualization,
linux-kernel, kernel-team
On Mon, Mar 16, 2026 at 05:27:57PM +0530, Vishwanath Seshagiri wrote:
> On 3/16/26 4:13 PM, Michael S. Tsirkin wrote:
> > On Mon, Mar 16, 2026 at 05:56:20AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Mar 10, 2026 at 11:31:04AM -0700, Vishwanath Seshagiri wrote:
> > > > @@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
> > > > /* In big_packets mode, xdp cannot work, so there is no need to
> > > > * initialize xsk of rq.
> > > > */
> > > > - if (vi->big_packets && !vi->mergeable_rx_bufs)
> > > > + if (!vi->rq[qid].page_pool)
> > > > return -ENOENT;
> > > > if (qid >= vi->curr_queue_pairs)
> > >
> > >
> > >
> > > It seems that a qid that exceeds curr_queue_pairs would previously get
> > > -EINVAL and now gets -ENOENT.
> >
> > Or maybe this if (qid >= vi->curr_queue_pairs) is dead code?
> > I looked at it some more and I can't find a path where this
> > triggers.
> >
> > > Maybe reorder the checks:
> > >
> > > if (qid >= vi->curr_queue_pairs)
> > > return -EINVAL;
> > >
> > > /* In big_packets mode, xdp cannot work, so there is no need to
> > > * initialize xsk of rq.
> > > */
> > > if (!vi->rq[qid].page_pool)
> > > return -ENOENT;
> > >
> > >
> > > Alternatively I think we can completely drop this chunk: we already seem
> > > to init page_pull at all times except for big packets mode, so the
> > > current code is fine I think.
>
> Yes, I agree qid >= curr_queue_pairs appears to be dead code.
> xsk_reg_pool_at_qid() in the XSK core already validates
> queue_id < max(real_num_rx_queues, real_num_tx_queues) before
> ndo_bpf is called, and real_num_rx_queues == curr_queue_pairs is an
> invariant in virtio_net. Both paths hold rtnl_lock so no race is
> possible. That said, I'll adopt your suggested reorder for v12 to ensure
> vi->rq[qid] isn't accessed before a bounds check, even if the check
> is currently redundant.
I think we can just take this as-is, and do more cleanups on top.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > >
> > >
> > > --
> > > MST
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-10 18:31 [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation Vishwanath Seshagiri
2026-03-13 7:51 ` Jason Wang
2026-03-16 9:56 ` Michael S. Tsirkin
@ 2026-03-17 2:30 ` patchwork-bot+netdevbpf
2026-03-23 15:01 ` Omar Elghoul
3 siblings, 0 replies; 21+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-03-17 2:30 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, dw, technoboy85, ilias.apalodimas, netdev,
virtualization, linux-kernel, kernel-team
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 10 Mar 2026 11:31:04 -0700 you wrote:
> Use page_pool for RX buffer allocation in mergeable and small buffer
> modes to enable page recycling and avoid repeated page allocator calls.
> skb_mark_for_recycle() enables page reuse in the network stack.
>
> Big packets mode is unchanged because it uses page->private for linked
> list chaining of multiple pages per buffer, which conflicts with
> page_pool's internal use of page->private.
>
> [...]
Here is the summary with links:
- [net-next,v11] virtio_net: add page_pool support for buffer allocation
https://git.kernel.org/netdev/net-next/c/24fbd3967f3f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-10 18:31 [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation Vishwanath Seshagiri
` (2 preceding siblings ...)
2026-03-17 2:30 ` patchwork-bot+netdevbpf
@ 2026-03-23 15:01 ` Omar Elghoul
2026-03-23 15:52 ` Michael S. Tsirkin
3 siblings, 1 reply; 21+ messages in thread
From: Omar Elghoul @ 2026-03-23 15:01 UTC (permalink / raw)
To: vishs
Cc: andrew+netdev, davem, dw, edumazet, eperezma, ilias.apalodimas,
jasowang, kernel-team, kuba, linux-kernel, mst, netdev, pabeni,
technoboy85, virtualization, xuanzhuo, oelghoul
Hi,
I've been testing linux-next (tags later than 03/17) and hit new issues in
virtio-net on s390x. I bisected the issue, and I found this patch to be the
first buggy commit.
The issue seems to only be reproducible when running in Secure Execution.
Tested in a KVM guest, the virtio-net performance appears greatly reduced,
and the dmesg output shows many instances of the following error messages.
Partial relevant logs
=====================
[ 49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
[ 74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
[ 403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
[ 403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
[ 403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
[ 403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
[ 403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
Steps to reproduce
==================
1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
enabled and a virtio-net-ccw device attached.
2. Run dmesg. The error message is usually already present at boot time,
but if not, it can be reproduced by creating any network traffic.
(*) This patch was not tested in a non-KVM hypervisor environment.
I've further confirmed that reverting this patch onto its parent commit
resolves the issue. Please let me know if you'd like me to test a fix or if
you would need more information.
Thanks in advance.
Best,
Omar
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 15:01 ` Omar Elghoul
@ 2026-03-23 15:52 ` Michael S. Tsirkin
2026-03-23 16:54 ` Omar Elghoul
2026-03-23 16:58 ` Michael S. Tsirkin
0 siblings, 2 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-23 15:52 UTC (permalink / raw)
To: Omar Elghoul
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
> Hi,
>
> I've been testing linux-next (tags later than 03/17) and hit new issues in
> virtio-net on s390x. I bisected the issue, and I found this patch to be the
> first buggy commit.
>
> The issue seems to only be reproducible when running in Secure Execution.
> Tested in a KVM guest, the virtio-net performance appears greatly reduced,
> and the dmesg output shows many instances of the following error messages.
>
> Partial relevant logs
> =====================
> [ 49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
> [ 74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
> [ 403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
> [ 403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> [ 403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
> [ 403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
> [ 403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
>
> Steps to reproduce
> ==================
> 1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
> enabled and a virtio-net-ccw device attached.
> 2. Run dmesg. The error message is usually already present at boot time,
> but if not, it can be reproduced by creating any network traffic.
>
> (*) This patch was not tested in a non-KVM hypervisor environment.
>
> I've further confirmed that reverting this patch onto its parent commit
> resolves the issue. Please let me know if you'd like me to test a fix or if
> you would need more information.
>
> Thanks in advance.
>
> Best,
> Omar
Well... I am not sure how I missed it. Obvious in hindsight:
static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
void *buf, unsigned int len, void **ctx,
unsigned int *xdp_xmit,
struct virtnet_rq_stats *stats)
{
struct net_device *dev = vi->dev;
struct sk_buff *skb;
u8 flags;
if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
pr_debug("%s: short packet %i\n", dev->name, len);
DEV_STATS_INC(dev, rx_length_errors);
virtnet_rq_free_buf(vi, rq, buf);
return;
}
/* About the flags below:
* 1. Save the flags early, as the XDP program might overwrite them.
* These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
* stay valid after XDP processing.
* 2. XDP doesn't work with partially checksummed packets (refer to
* virtnet_xdp_set()), so packets marked as
* VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
*/
if (vi->mergeable_rx_bufs) {
flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
stats);
} else if (vi->big_packets) {
void *p = page_address((struct page *)buf);
flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
skb = receive_big(dev, vi, rq, buf, len, stats);
} else {
flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
}
So we are reading the header, before dma sync, which is within
receive_mergeable and friends:
static struct sk_buff *receive_mergeable(struct net_device *dev,
struct virtnet_info *vi,
struct receive_queue *rq,
void *buf,
void *ctx,
unsigned int len,
unsigned int *xdp_xmit,
struct virtnet_rq_stats *stats)
{
struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
struct page *page = virt_to_head_page(buf);
int offset = buf - page_address(page);
struct sk_buff *head_skb, *curr_skb;
unsigned int truesize = mergeable_ctx_to_truesize(ctx);
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
head_skb = NULL;
if (rq->use_page_pool_dma)
page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
Just as a test, the below should fix it (compiled only), but the real
fix is more complex since we need to be careful to avoid expensive syncing
twice.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 97035b49bae7..57b4f5954bed 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
{
+ void *buf;
+
BUG_ON(!rq->page_pool);
- return virtqueue_get_buf_ctx(rq->vq, len, ctx);
+ buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
+ if (buf && rq->use_page_pool_dma && *len) {
+ struct page *page = virt_to_head_page(buf);
+ int offset = buf - page_address(page);
+
+ page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
+ }
+
+ return buf;
}
static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
--
MST
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 15:52 ` Michael S. Tsirkin
@ 2026-03-23 16:54 ` Omar Elghoul
2026-03-23 17:10 ` Michael S. Tsirkin
2026-03-23 16:58 ` Michael S. Tsirkin
1 sibling, 1 reply; 21+ messages in thread
From: Omar Elghoul @ 2026-03-23 16:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On 3/23/26 11:52 AM, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
>> [...]
> Well... I am not sure how I missed it. Obvious in hindsight:
>
> static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> void *buf, unsigned int len, void **ctx,
> unsigned int *xdp_xmit,
> struct virtnet_rq_stats *stats)
> {
> struct net_device *dev = vi->dev;
> struct sk_buff *skb;
> u8 flags;
>
> if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> DEV_STATS_INC(dev, rx_length_errors);
> virtnet_rq_free_buf(vi, rq, buf);
> return;
> }
>
> /* About the flags below:
> * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> * stay valid after XDP processing.
> * 2. XDP doesn't work with partially checksummed packets (refer to
> * virtnet_xdp_set()), so packets marked as
> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
> */
>
> if (vi->mergeable_rx_bufs) {
> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
> stats);
> } else if (vi->big_packets) {
> void *p = page_address((struct page *)buf);
>
> flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> skb = receive_big(dev, vi, rq, buf, len, stats);
> } else {
> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
> }
>
>
> So we are reading the header, before dma sync, which is within
> receive_mergeable and friends:
Thank you for your analysis and explanation.
>
> static struct sk_buff *receive_mergeable(struct net_device *dev,
> struct virtnet_info *vi,
> struct receive_queue *rq,
> void *buf,
> void *ctx,
> unsigned int len,
> unsigned int *xdp_xmit,
> struct virtnet_rq_stats *stats)
> {
> struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> struct page *page = virt_to_head_page(buf);
> int offset = buf - page_address(page);
> struct sk_buff *head_skb, *curr_skb;
> unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>
> head_skb = NULL;
>
> if (rq->use_page_pool_dma)
> page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>
>
>
> Just as a test, the below should fix it (compiled only), but the real
> fix is more complex since we need to be careful to avoid expensive syncing
> twice.
I applied your patch and tested it on my system. With this change, I
could not reproduce the same error anymore. I would be happy to test a
proper fix once you have one.
>
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..57b4f5954bed 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>
> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> {
> + void *buf;
> +
> BUG_ON(!rq->page_pool);
>
> - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> + if (buf && rq->use_page_pool_dma && *len) {
> + struct page *page = virt_to_head_page(buf);
> + int offset = buf - page_address(page);
> +
> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
> + }
> +
> + return buf;
> }
>
> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 15:52 ` Michael S. Tsirkin
2026-03-23 16:54 ` Omar Elghoul
@ 2026-03-23 16:58 ` Michael S. Tsirkin
2026-03-23 17:09 ` Omar Elghoul
` (2 more replies)
1 sibling, 3 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-23 16:58 UTC (permalink / raw)
To: Omar Elghoul
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
> > Hi,
> >
> > I've been testing linux-next (tags later than 03/17) and hit new issues in
> > virtio-net on s390x. I bisected the issue, and I found this patch to be the
> > first buggy commit.
> >
> > The issue seems to only be reproducible when running in Secure Execution.
> > Tested in a KVM guest, the virtio-net performance appears greatly reduced,
> > and the dmesg output shows many instances of the following error messages.
> >
> > Partial relevant logs
> > =====================
> > [ 49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
> > [ 74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
> > [ 403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
> > [ 403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> > [ 403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
> > [ 403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
> > [ 403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> >
> > Steps to reproduce
> > ==================
> > 1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
> > enabled and a virtio-net-ccw device attached.
> > 2. Run dmesg. The error message is usually already present at boot time,
> > but if not, it can be reproduced by creating any network traffic.
> >
> > (*) This patch was not tested in a non-KVM hypervisor environment.
> >
> > I've further confirmed that reverting this patch onto its parent commit
> > resolves the issue. Please let me know if you'd like me to test a fix or if
> > you would need more information.
> >
> > Thanks in advance.
> >
> > Best,
> > Omar
>
> Well... I am not sure how I missed it. Obvious in hindsight:
>
> static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> void *buf, unsigned int len, void **ctx,
> unsigned int *xdp_xmit,
> struct virtnet_rq_stats *stats)
> {
> struct net_device *dev = vi->dev;
> struct sk_buff *skb;
> u8 flags;
>
> if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> DEV_STATS_INC(dev, rx_length_errors);
> virtnet_rq_free_buf(vi, rq, buf);
> return;
> }
>
> /* About the flags below:
> * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> * stay valid after XDP processing.
> * 2. XDP doesn't work with partially checksummed packets (refer to
> * virtnet_xdp_set()), so packets marked as
> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
> */
>
> if (vi->mergeable_rx_bufs) {
> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
> stats);
> } else if (vi->big_packets) {
> void *p = page_address((struct page *)buf);
>
> flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> skb = receive_big(dev, vi, rq, buf, len, stats);
> } else {
> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
> }
>
>
> So we are reading the header, before dma sync, which is within
> receive_mergeable and friends:
>
> static struct sk_buff *receive_mergeable(struct net_device *dev,
> struct virtnet_info *vi,
> struct receive_queue *rq,
> void *buf,
> void *ctx,
> unsigned int len,
> unsigned int *xdp_xmit,
> struct virtnet_rq_stats *stats)
> {
> struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> struct page *page = virt_to_head_page(buf);
> int offset = buf - page_address(page);
> struct sk_buff *head_skb, *curr_skb;
> unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>
> head_skb = NULL;
>
> if (rq->use_page_pool_dma)
> page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>
>
>
> Just as a test, the below should fix it (compiled only), but the real
> fix is more complex since we need to be careful to avoid expensive syncing
> twice.
>
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..57b4f5954bed 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>
> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> {
> + void *buf;
> +
> BUG_ON(!rq->page_pool);
>
> - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> + if (buf && rq->use_page_pool_dma && *len) {
> + struct page *page = virt_to_head_page(buf);
> + int offset = buf - page_address(page);
> +
> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
> + }
> +
> + return buf;
> }
>
> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>
>
>
>
> --
> MST
or maybe like this:
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 97035b49bae7..835f52651006 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
*/
buf -= VIRTNET_RX_PAD + xdp_headroom;
- if (rq->use_page_pool_dma) {
- int offset = buf - page_address(page) +
- VIRTNET_RX_PAD + xdp_headroom;
-
- page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
- }
-
len -= vi->hdr_len;
u64_stats_add(&stats->bytes, len);
@@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
head_skb = NULL;
- if (rq->use_page_pool_dma)
- page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
-
u64_stats_add(&stats->bytes, len - vi->hdr_len);
if (check_mergeable_len(dev, ctx, len))
@@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
return;
}
+ if (rq->use_page_pool_dma) {
+ struct page *page = virt_to_head_page(buf);
+ int offset = buf - page_address(page);
+
+ page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
+ }
+
/* About the flags below:
* 1. Save the flags early, as the XDP program might overwrite them.
* These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 16:58 ` Michael S. Tsirkin
@ 2026-03-23 17:09 ` Omar Elghoul
2026-03-23 17:50 ` Vishwanath Seshagiri
2026-03-24 0:34 ` Jason Wang
2026-03-24 8:20 ` Aithal, Srikanth
2 siblings, 1 reply; 21+ messages in thread
From: Omar Elghoul @ 2026-03-23 17:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On 3/23/26 12:58 PM, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
>> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
>>> [...]
>> Well... I am not sure how I missed it. Obvious in hindsight:
>>
>> static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>> void *buf, unsigned int len, void **ctx,
>> unsigned int *xdp_xmit,
>> struct virtnet_rq_stats *stats)
>> {
>> struct net_device *dev = vi->dev;
>> struct sk_buff *skb;
>> u8 flags;
>>
>> if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>> pr_debug("%s: short packet %i\n", dev->name, len);
>> DEV_STATS_INC(dev, rx_length_errors);
>> virtnet_rq_free_buf(vi, rq, buf);
>> return;
>> }
>>
>> /* About the flags below:
>> * 1. Save the flags early, as the XDP program might overwrite them.
>> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>> * stay valid after XDP processing.
>> * 2. XDP doesn't work with partially checksummed packets (refer to
>> * virtnet_xdp_set()), so packets marked as
>> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
>> */
>>
>> if (vi->mergeable_rx_bufs) {
>> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
>> skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
>> stats);
>> } else if (vi->big_packets) {
>> void *p = page_address((struct page *)buf);
>>
>> flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
>> skb = receive_big(dev, vi, rq, buf, len, stats);
>> } else {
>> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
>> skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
>> }
>>
>>
>> So we are reading the header, before dma sync, which is within
>> receive_mergeable and friends:
>>
>> static struct sk_buff *receive_mergeable(struct net_device *dev,
>> struct virtnet_info *vi,
>> struct receive_queue *rq,
>> void *buf,
>> void *ctx,
>> unsigned int len,
>> unsigned int *xdp_xmit,
>> struct virtnet_rq_stats *stats)
>> {
>> struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
>> int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
>> struct page *page = virt_to_head_page(buf);
>> int offset = buf - page_address(page);
>> struct sk_buff *head_skb, *curr_skb;
>> unsigned int truesize = mergeable_ctx_to_truesize(ctx);
>> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>>
>> head_skb = NULL;
>>
>> if (rq->use_page_pool_dma)
>> page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>>
>>
>>
>> Just as a test, the below should fix it (compiled only), but the real
>> fix is more complex since we need to be careful to avoid expensive syncing
>> twice.
>>
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 97035b49bae7..57b4f5954bed 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>>
>> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
>> {
>> + void *buf;
>> +
>> BUG_ON(!rq->page_pool);
>>
>> - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> + if (buf && rq->use_page_pool_dma && *len) {
>> + struct page *page = virt_to_head_page(buf);
>> + int offset = buf - page_address(page);
>> +
>> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
>> + }
>> +
>> + return buf;
>> }
>>
>> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>>
>>
>>
>>
>> --
>> MST
> or maybe like this:
Both of these patches resolve the issue on my end.
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..835f52651006 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
> */
> buf -= VIRTNET_RX_PAD + xdp_headroom;
>
> - if (rq->use_page_pool_dma) {
> - int offset = buf - page_address(page) +
> - VIRTNET_RX_PAD + xdp_headroom;
> -
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> - }
> -
> len -= vi->hdr_len;
> u64_stats_add(&stats->bytes, len);
>
> @@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>
> head_skb = NULL;
>
> - if (rq->use_page_pool_dma)
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> -
> u64_stats_add(&stats->bytes, len - vi->hdr_len);
>
> if (check_mergeable_len(dev, ctx, len))
> @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
> }
>
> + if (rq->use_page_pool_dma) {
> + struct page *page = virt_to_head_page(buf);
> + int offset = buf - page_address(page);
> +
> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> + }
> +
> /* About the flags below:
> * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 16:54 ` Omar Elghoul
@ 2026-03-23 17:10 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-23 17:10 UTC (permalink / raw)
To: Omar Elghoul
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On Mon, Mar 23, 2026 at 12:54:03PM -0400, Omar Elghoul wrote:
> On 3/23/26 11:52 AM, Michael S. Tsirkin wrote:
> > On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
> > > [...]
> > Well... I am not sure how I missed it. Obvious in hindsight:
> >
> > static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> > void *buf, unsigned int len, void **ctx,
> > unsigned int *xdp_xmit,
> > struct virtnet_rq_stats *stats)
> > {
> > struct net_device *dev = vi->dev;
> > struct sk_buff *skb;
> > u8 flags;
> > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > DEV_STATS_INC(dev, rx_length_errors);
> > virtnet_rq_free_buf(vi, rq, buf);
> > return;
> > }
> > /* About the flags below:
> > * 1. Save the flags early, as the XDP program might overwrite them.
> > * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> > * stay valid after XDP processing.
> > * 2. XDP doesn't work with partially checksummed packets (refer to
> > * virtnet_xdp_set()), so packets marked as
> > * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
> > */
> > if (vi->mergeable_rx_bufs) {
> > flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> > skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
> > stats);
> > } else if (vi->big_packets) {
> > void *p = page_address((struct page *)buf);
> > flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> > skb = receive_big(dev, vi, rq, buf, len, stats);
> > } else {
> > flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> > skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
> > }
> >
> >
> > So we are reading the header, before dma sync, which is within
> > receive_mergeable and friends:
> Thank you for your analysis and explanation.
> >
> > static struct sk_buff *receive_mergeable(struct net_device *dev,
> > struct virtnet_info *vi,
> > struct receive_queue *rq,
> > void *buf,
> > void *ctx,
> > unsigned int len,
> > unsigned int *xdp_xmit,
> > struct virtnet_rq_stats *stats)
> > {
> > struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> > int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> > struct page *page = virt_to_head_page(buf);
> > int offset = buf - page_address(page);
> > struct sk_buff *head_skb, *curr_skb;
> > unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> > unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> > head_skb = NULL;
> > if (rq->use_page_pool_dma)
> > page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> >
> >
> > Just as a test, the below should fix it (compiled only), but the real
> > fix is more complex since we need to be careful to avoid expensive syncing
> > twice.
>
> I applied your patch and tested it on my system. With this change, I could
> not reproduce the same error anymore. I would be happy to test a proper fix
> once you have one.
>
> >
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 97035b49bae7..57b4f5954bed 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> > {
> > + void *buf;
> > +
> > BUG_ON(!rq->page_pool);
> > - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > + if (buf && rq->use_page_pool_dma && *len) {
> > + struct page *page = virt_to_head_page(buf);
> > + int offset = buf - page_address(page);
> > +
> > + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
> > + }
> > +
> > + return buf;
> > }
> > static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> >
> >
> >
> >
just sent
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 17:09 ` Omar Elghoul
@ 2026-03-23 17:50 ` Vishwanath Seshagiri
2026-03-23 23:37 ` Michael S. Tsirkin
0 siblings, 1 reply; 21+ messages in thread
From: Vishwanath Seshagiri @ 2026-03-23 17:50 UTC (permalink / raw)
To: Omar Elghoul, Michael S. Tsirkin
Cc: andrew+netdev, davem, dw, edumazet, eperezma, ilias.apalodimas,
jasowang, kernel-team, kuba, linux-kernel, netdev, pabeni,
technoboy85, virtualization, xuanzhuo
On 3/23/26 10:39 PM, Omar Elghoul wrote:
> On 3/23/26 12:58 PM, Michael S. Tsirkin wrote:
>
>> On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
>>> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
>>>> [...]
>>> Well... I am not sure how I missed it. Obvious in hindsight:
>>>
>>> static void receive_buf(struct virtnet_info *vi, struct receive_queue
>>> *rq,
>>> void *buf, unsigned int len, void **ctx,
>>> unsigned int *xdp_xmit,
>>> struct virtnet_rq_stats *stats)
>>> {
>>> struct net_device *dev = vi->dev;
>>> struct sk_buff *skb;
>>> u8 flags;
>>> if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>>> pr_debug("%s: short packet %i\n", dev->name, len);
>>> DEV_STATS_INC(dev, rx_length_errors);
>>> virtnet_rq_free_buf(vi, rq, buf);
>>> return;
>>> }
>>> /* About the flags below:
>>> * 1. Save the flags early, as the XDP program might
>>> overwrite them.
>>> * These flags ensure packets marked as
>>> VIRTIO_NET_HDR_F_DATA_VALID
>>> * stay valid after XDP processing.
>>> * 2. XDP doesn't work with partially checksummed packets
>>> (refer to
>>> * virtnet_xdp_set()), so packets marked as
>>> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP
>>> processing.
>>> */
>>> if (vi->mergeable_rx_bufs) {
>>> flags = ((struct virtio_net_common_hdr *)buf)-
>>> >hdr.flags;
>>> skb = receive_mergeable(dev, vi, rq, buf, ctx, len,
>>> xdp_xmit,
>>> stats);
>>> } else if (vi->big_packets) {
>>> void *p = page_address((struct page *)buf);
>>> flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
>>> skb = receive_big(dev, vi, rq, buf, len, stats);
>>> } else {
>>> flags = ((struct virtio_net_common_hdr *)buf)-
>>> >hdr.flags;
>>> skb = receive_small(dev, vi, rq, buf, ctx, len,
>>> xdp_xmit, stats);
>>> }
>>>
>>>
>>> So we are reading the header, before dma sync, which is within
>>> receive_mergeable and friends:
>>>
>>> static struct sk_buff *receive_mergeable(struct net_device *dev,
>>> struct virtnet_info *vi,
>>> struct receive_queue *rq,
>>> void *buf,
>>> void *ctx,
>>> unsigned int len,
>>> unsigned int *xdp_xmit,
>>> struct virtnet_rq_stats
>>> *stats)
>>> {
>>> struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
>>> int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
>>> struct page *page = virt_to_head_page(buf);
>>> int offset = buf - page_address(page);
>>> struct sk_buff *head_skb, *curr_skb;
>>> unsigned int truesize = mergeable_ctx_to_truesize(ctx);
>>> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>>> head_skb = NULL;
>>> if (rq->use_page_pool_dma)
>>> page_pool_dma_sync_for_cpu(rq->page_pool, page,
>>> offset, len);
>>>
>>>
>>> Just as a test, the below should fix it (compiled only), but the real
>>> fix is more complex since we need to be careful to avoid expensive
>>> syncing
>>> twice.
>>>
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 97035b49bae7..57b4f5954bed 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct
>>> virtnet_info *vi,
>>> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len,
>>> void **ctx)
>>> {
>>> + void *buf;
>>> +
>>> BUG_ON(!rq->page_pool);
>>> - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
>>> + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
>>> + if (buf && rq->use_page_pool_dma && *len) {
>>> + struct page *page = virt_to_head_page(buf);
>>> + int offset = buf - page_address(page);
>>> +
>>> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
>>> + }
>>> +
>>> + return buf;
>>> }
>>> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>>>
>>>
>>>
>>>
>>> --
>>> MST
>> or maybe like this:
> Both of these patches resolve the issue on my end.
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 97035b49bae7..835f52651006 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct
>> net_device *dev,
>> */
>> buf -= VIRTNET_RX_PAD + xdp_headroom;
>> - if (rq->use_page_pool_dma) {
>> - int offset = buf - page_address(page) +
>> - VIRTNET_RX_PAD + xdp_headroom;
>> -
>> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>> - }
>> -
>> len -= vi->hdr_len;
>> u64_stats_add(&stats->bytes, len);
>> @@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct
>> net_device *dev,
>> head_skb = NULL;
>> - if (rq->use_page_pool_dma)
>> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>> -
>> u64_stats_add(&stats->bytes, len - vi->hdr_len);
>> if (check_mergeable_len(dev, ctx, len))
>> @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info
>> *vi, struct receive_queue *rq,
>> return;
>> }
>> + if (rq->use_page_pool_dma) {
>> + struct page *page = virt_to_head_page(buf);
>> + int offset = buf - page_address(page);
>> +
>> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>> + }
>> +
>> /* About the flags below:
>> * 1. Save the flags early, as the XDP program might overwrite
>> them.
>> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>>
Hello Omar. Thank you for the bisect, apologies for breaking the
functionality. Thank you Michael for the patch, I will be careful next
time about DMA sync issues. Looks like the bug is fixed, is there
anything else needed from my end?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 17:50 ` Vishwanath Seshagiri
@ 2026-03-23 23:37 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2026-03-23 23:37 UTC (permalink / raw)
To: Vishwanath Seshagiri
Cc: Omar Elghoul, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On Mon, Mar 23, 2026 at 01:50:29PM -0400, Vishwanath Seshagiri wrote:
>
>
> On 3/23/26 10:39 PM, Omar Elghoul wrote:
> > On 3/23/26 12:58 PM, Michael S. Tsirkin wrote:
> >
> > > On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
> > > > > [...]
> > > > Well... I am not sure how I missed it. Obvious in hindsight:
> > > >
> > > > static void receive_buf(struct virtnet_info *vi, struct
> > > > receive_queue *rq,
> > > > void *buf, unsigned int len, void **ctx,
> > > > unsigned int *xdp_xmit,
> > > > struct virtnet_rq_stats *stats)
> > > > {
> > > > struct net_device *dev = vi->dev;
> > > > struct sk_buff *skb;
> > > > u8 flags;
> > > > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > > > pr_debug("%s: short packet %i\n", dev->name, len);
> > > > DEV_STATS_INC(dev, rx_length_errors);
> > > > virtnet_rq_free_buf(vi, rq, buf);
> > > > return;
> > > > }
> > > > /* About the flags below:
> > > > * 1. Save the flags early, as the XDP program might
> > > > overwrite them.
> > > > * These flags ensure packets marked as
> > > > VIRTIO_NET_HDR_F_DATA_VALID
> > > > * stay valid after XDP processing.
> > > > * 2. XDP doesn't work with partially checksummed
> > > > packets (refer to
> > > > * virtnet_xdp_set()), so packets marked as
> > > > * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP
> > > > processing.
> > > > */
> > > > if (vi->mergeable_rx_bufs) {
> > > > flags = ((struct virtio_net_common_hdr *)buf)-
> > > > >hdr.flags;
> > > > skb = receive_mergeable(dev, vi, rq, buf, ctx,
> > > > len, xdp_xmit,
> > > > stats);
> > > > } else if (vi->big_packets) {
> > > > void *p = page_address((struct page *)buf);
> > > > flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> > > > skb = receive_big(dev, vi, rq, buf, len, stats);
> > > > } else {
> > > > flags = ((struct virtio_net_common_hdr *)buf)-
> > > > >hdr.flags;
> > > > skb = receive_small(dev, vi, rq, buf, ctx, len,
> > > > xdp_xmit, stats);
> > > > }
> > > >
> > > >
> > > > So we are reading the header, before dma sync, which is within
> > > > receive_mergeable and friends:
> > > >
> > > > static struct sk_buff *receive_mergeable(struct net_device *dev,
> > > > struct virtnet_info *vi,
> > > > struct receive_queue *rq,
> > > > void *buf,
> > > > void *ctx,
> > > > unsigned int len,
> > > > unsigned int *xdp_xmit,
> > > > struct
> > > > virtnet_rq_stats *stats)
> > > > {
> > > > struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> > > > int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> > > > struct page *page = virt_to_head_page(buf);
> > > > int offset = buf - page_address(page);
> > > > struct sk_buff *head_skb, *curr_skb;
> > > > unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> > > > unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> > > > head_skb = NULL;
> > > > if (rq->use_page_pool_dma)
> > > > page_pool_dma_sync_for_cpu(rq->page_pool, page,
> > > > offset, len);
> > > >
> > > >
> > > > Just as a test, the below should fix it (compiled only), but the real
> > > > fix is more complex since we need to be careful to avoid
> > > > expensive syncing
> > > > twice.
> > > >
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 97035b49bae7..57b4f5954bed 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct
> > > > virtnet_info *vi,
> > > > static void *virtnet_rq_get_buf(struct receive_queue *rq, u32
> > > > *len, void **ctx)
> > > > {
> > > > + void *buf;
> > > > +
> > > > BUG_ON(!rq->page_pool);
> > > > - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > > + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > > > + if (buf && rq->use_page_pool_dma && *len) {
> > > > + struct page *page = virt_to_head_page(buf);
> > > > + int offset = buf - page_address(page);
> > > > +
> > > > + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
> > > > + }
> > > > +
> > > > + return buf;
> > > > }
> > > > static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> > > >
> > > >
> > > >
> > > >
> > > > --
> > > > MST
> > > or maybe like this:
> > Both of these patches resolve the issue on my end.
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 97035b49bae7..835f52651006 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct
> > > net_device *dev,
> > > */
> > > buf -= VIRTNET_RX_PAD + xdp_headroom;
> > > - if (rq->use_page_pool_dma) {
> > > - int offset = buf - page_address(page) +
> > > - VIRTNET_RX_PAD + xdp_headroom;
> > > -
> > > - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> > > - }
> > > -
> > > len -= vi->hdr_len;
> > > u64_stats_add(&stats->bytes, len);
> > > @@ -2398,9 +2391,6 @@ static struct sk_buff
> > > *receive_mergeable(struct net_device *dev,
> > > head_skb = NULL;
> > > - if (rq->use_page_pool_dma)
> > > - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> > > -
> > > u64_stats_add(&stats->bytes, len - vi->hdr_len);
> > > if (check_mergeable_len(dev, ctx, len))
> > > @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info
> > > *vi, struct receive_queue *rq,
> > > return;
> > > }
> > > + if (rq->use_page_pool_dma) {
> > > + struct page *page = virt_to_head_page(buf);
> > > + int offset = buf - page_address(page);
> > > +
> > > + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> > > + }
> > > +
> > > /* About the flags below:
> > > * 1. Save the flags early, as the XDP program might overwrite
> > > them.
> > > * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> > >
>
> Hello Omar. Thank you for the bisect, apologies for breaking the
> functionality. Thank you Michael for the patch, I will be careful next
> time about DMA sync issues. Looks like the bug is fixed, is there
> anything else needed from my end?
>
Would be great to have you test the patch on your end and report
the performace. Thanks!
--
MST
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 16:58 ` Michael S. Tsirkin
2026-03-23 17:09 ` Omar Elghoul
@ 2026-03-24 0:34 ` Jason Wang
2026-03-24 8:20 ` Aithal, Srikanth
2 siblings, 0 replies; 21+ messages in thread
From: Jason Wang @ 2026-03-24 0:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Omar Elghoul, vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, kernel-team, kuba, linux-kernel, netdev, pabeni,
technoboy85, virtualization, xuanzhuo
On Tue, Mar 24, 2026 at 12:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
> > > Hi,
> > >
> > > I've been testing linux-next (tags later than 03/17) and hit new issues in
> > > virtio-net on s390x. I bisected the issue, and I found this patch to be the
> > > first buggy commit.
> > >
> > > The issue seems to only be reproducible when running in Secure Execution.
> > > Tested in a KVM guest, the virtio-net performance appears greatly reduced,
> > > and the dmesg output shows many instances of the following error messages.
> > >
> > > Partial relevant logs
> > > =====================
> > > [ 49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
> > > [ 74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
> > > [ 403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
> > > [ 403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> > > [ 403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
> > > [ 403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
> > > [ 403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> > >
> > > Steps to reproduce
> > > ==================
> > > 1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
> > > enabled and a virtio-net-ccw device attached.
> > > 2. Run dmesg. The error message is usually already present at boot time,
> > > but if not, it can be reproduced by creating any network traffic.
> > >
> > > (*) This patch was not tested in a non-KVM hypervisor environment.
> > >
> > > I've further confirmed that reverting this patch onto its parent commit
> > > resolves the issue. Please let me know if you'd like me to test a fix or if
> > > you would need more information.
> > >
> > > Thanks in advance.
> > >
> > > Best,
> > > Omar
> >
> > Well... I am not sure how I missed it. Obvious in hindsight:
> >
> > static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> > void *buf, unsigned int len, void **ctx,
> > unsigned int *xdp_xmit,
> > struct virtnet_rq_stats *stats)
> > {
> > struct net_device *dev = vi->dev;
> > struct sk_buff *skb;
> > u8 flags;
> >
> > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > DEV_STATS_INC(dev, rx_length_errors);
> > virtnet_rq_free_buf(vi, rq, buf);
> > return;
> > }
> >
> > /* About the flags below:
> > * 1. Save the flags early, as the XDP program might overwrite them.
> > * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> > * stay valid after XDP processing.
> > * 2. XDP doesn't work with partially checksummed packets (refer to
> > * virtnet_xdp_set()), so packets marked as
> > * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
> > */
> >
> > if (vi->mergeable_rx_bufs) {
> > flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> > skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
> > stats);
> > } else if (vi->big_packets) {
> > void *p = page_address((struct page *)buf);
> >
> > flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> > skb = receive_big(dev, vi, rq, buf, len, stats);
> > } else {
> > flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> > skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
> > }
> >
> >
> > So we are reading the header, before dma sync, which is within
> > receive_mergeable and friends:
> >
> > static struct sk_buff *receive_mergeable(struct net_device *dev,
> > struct virtnet_info *vi,
> > struct receive_queue *rq,
> > void *buf,
> > void *ctx,
> > unsigned int len,
> > unsigned int *xdp_xmit,
> > struct virtnet_rq_stats *stats)
> > {
> > struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> > int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> > struct page *page = virt_to_head_page(buf);
> > int offset = buf - page_address(page);
> > struct sk_buff *head_skb, *curr_skb;
> > unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> > unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> >
> > head_skb = NULL;
> >
> > if (rq->use_page_pool_dma)
> > page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> >
> >
> >
> > Just as a test, the below should fix it (compiled only), but the real
> > fix is more complex since we need to be careful to avoid expensive syncing
> > twice.
> >
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 97035b49bae7..57b4f5954bed 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >
> > static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> > {
> > + void *buf;
> > +
> > BUG_ON(!rq->page_pool);
> >
> > - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > + if (buf && rq->use_page_pool_dma && *len) {
> > + struct page *page = virt_to_head_page(buf);
> > + int offset = buf - page_address(page);
> > +
> > + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
> > + }
> > +
> > + return buf;
> > }
> >
> > static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> >
> >
> >
> >
> > --
> > MST
>
> or maybe like this:
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..835f52651006 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
> */
> buf -= VIRTNET_RX_PAD + xdp_headroom;
>
> - if (rq->use_page_pool_dma) {
> - int offset = buf - page_address(page) +
> - VIRTNET_RX_PAD + xdp_headroom;
> -
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> - }
> -
> len -= vi->hdr_len;
> u64_stats_add(&stats->bytes, len);
>
> @@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>
> head_skb = NULL;
>
> - if (rq->use_page_pool_dma)
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> -
> u64_stats_add(&stats->bytes, len - vi->hdr_len);
>
> if (check_mergeable_len(dev, ctx, len))
> @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
> }
>
> + if (rq->use_page_pool_dma) {
> + struct page *page = virt_to_head_page(buf);
> + int offset = buf - page_address(page);
> +
> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> + }
> +
> /* About the flags below:
> * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>
This looks better.
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
2026-03-23 16:58 ` Michael S. Tsirkin
2026-03-23 17:09 ` Omar Elghoul
2026-03-24 0:34 ` Jason Wang
@ 2026-03-24 8:20 ` Aithal, Srikanth
2 siblings, 0 replies; 21+ messages in thread
From: Aithal, Srikanth @ 2026-03-24 8:20 UTC (permalink / raw)
To: Michael S. Tsirkin, Omar Elghoul
Cc: vishs, andrew+netdev, davem, dw, edumazet, eperezma,
ilias.apalodimas, jasowang, kernel-team, kuba, linux-kernel,
netdev, pabeni, technoboy85, virtualization, xuanzhuo
On 3/23/2026 10:28 PM, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
>> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
>>> Hi,
>>>
>>> I've been testing linux-next (tags later than 03/17) and hit new issues in
>>> virtio-net on s390x. I bisected the issue, and I found this patch to be the
>>> first buggy commit.
>>>
>>> The issue seems to only be reproducible when running in Secure Execution.
>>> Tested in a KVM guest, the virtio-net performance appears greatly reduced,
>>> and the dmesg output shows many instances of the following error messages.
>>>
>>> Partial relevant logs
>>> =====================
>>> [ 49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
>>> [ 74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
>>> [ 403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
>>> [ 403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
>>> [ 403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
>>> [ 403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
>>> [ 403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
>>>
>>> Steps to reproduce
>>> ==================
>>> 1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
>>> enabled and a virtio-net-ccw device attached.
>>> 2. Run dmesg. The error message is usually already present at boot time,
>>> but if not, it can be reproduced by creating any network traffic.
>>>
>>> (*) This patch was not tested in a non-KVM hypervisor environment.
>>>
>>> I've further confirmed that reverting this patch onto its parent commit
>>> resolves the issue. Please let me know if you'd like me to test a fix or if
>>> you would need more information.
>>>
>>> Thanks in advance.
>>>
>>> Best,
>>> Omar
>>
>> Well... I am not sure how I missed it. Obvious in hindsight:
>>
>> static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>> void *buf, unsigned int len, void **ctx,
>> unsigned int *xdp_xmit,
>> struct virtnet_rq_stats *stats)
>> {
>> struct net_device *dev = vi->dev;
>> struct sk_buff *skb;
>> u8 flags;
>>
>> if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>> pr_debug("%s: short packet %i\n", dev->name, len);
>> DEV_STATS_INC(dev, rx_length_errors);
>> virtnet_rq_free_buf(vi, rq, buf);
>> return;
>> }
>>
>> /* About the flags below:
>> * 1. Save the flags early, as the XDP program might overwrite them.
>> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>> * stay valid after XDP processing.
>> * 2. XDP doesn't work with partially checksummed packets (refer to
>> * virtnet_xdp_set()), so packets marked as
>> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
>> */
>>
>> if (vi->mergeable_rx_bufs) {
>> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
>> skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
>> stats);
>> } else if (vi->big_packets) {
>> void *p = page_address((struct page *)buf);
>>
>> flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
>> skb = receive_big(dev, vi, rq, buf, len, stats);
>> } else {
>> flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
>> skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
>> }
>>
>>
>> So we are reading the header, before dma sync, which is within
>> receive_mergeable and friends:
>>
>> static struct sk_buff *receive_mergeable(struct net_device *dev,
>> struct virtnet_info *vi,
>> struct receive_queue *rq,
>> void *buf,
>> void *ctx,
>> unsigned int len,
>> unsigned int *xdp_xmit,
>> struct virtnet_rq_stats *stats)
>> {
>> struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
>> int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
>> struct page *page = virt_to_head_page(buf);
>> int offset = buf - page_address(page);
>> struct sk_buff *head_skb, *curr_skb;
>> unsigned int truesize = mergeable_ctx_to_truesize(ctx);
>> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>>
>> head_skb = NULL;
>>
>> if (rq->use_page_pool_dma)
>> page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
>>
>>
>>
>> Just as a test, the below should fix it (compiled only), but the real
>> fix is more complex since we need to be careful to avoid expensive syncing
>> twice.
>>
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 97035b49bae7..57b4f5954bed 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>>
>> static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
>> {
>> + void *buf;
>> +
>> BUG_ON(!rq->page_pool);
>>
>> - return virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
>> + if (buf && rq->use_page_pool_dma && *len) {
>> + struct page *page = virt_to_head_page(buf);
>> + int offset = buf - page_address(page);
>> +
>> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len);
>> + }
>> +
>> + return buf;
>> }
>>
>> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
>>
>>
>>
>>
>> --
>> MST
>
> or maybe like this:
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..835f52651006 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
> */
> buf -= VIRTNET_RX_PAD + xdp_headroom;
>
> - if (rq->use_page_pool_dma) {
> - int offset = buf - page_address(page) +
> - VIRTNET_RX_PAD + xdp_headroom;
> -
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> - }
> -
> len -= vi->hdr_len;
> u64_stats_add(&stats->bytes, len);
>
> @@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>
> head_skb = NULL;
>
> - if (rq->use_page_pool_dma)
> - page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> -
> u64_stats_add(&stats->bytes, len - vi->hdr_len);
>
> if (check_mergeable_len(dev, ctx, len))
> @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
> }
>
> + if (rq->use_page_pool_dma) {
> + struct page *page = virt_to_head_page(buf);
> + int offset = buf - page_address(page);
> +
> + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> + }
> +
> /* About the flags below:
> * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
>
I see the same virtio_net regression introduced by commit 24fbd3967f3f
(“virtio_net: add page_pool support for buffer allocation”) on AMD EPYC
(KVM/QEMU secure guests with virtio-net), in addition to s390x as in
Omar Elghoul’s report [1].
The failure mode matches: broken guest networking until that commit is
reverted or worked around.
I built and tested both of Michael Tsirkin’s proposed fixes in this
thread [2] [3]; each resolves the issue on my AMD EPYC setup.
[1]
https://lore.kernel.org/all/20260323150136.14452-1-oelghoul@linux.ibm.com/
[2]
https://lore.kernel.org/all/20260323114313-mutt-send-email-mst@kernel.org/
[3]
https://lore.kernel.org/all/20260323122728-mutt-send-email-mst@kernel.org/#t
Tested-by: Srikanth Aithal <sraithal@amd.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2026-03-24 8:20 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-10 18:31 [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation Vishwanath Seshagiri
2026-03-13 7:51 ` Jason Wang
2026-03-13 9:26 ` Vishwanath Seshagiri
2026-03-16 7:41 ` Jason Wang
2026-03-13 16:50 ` Vishwanath Seshagiri
2026-03-16 7:35 ` Jason Wang
2026-03-16 9:56 ` Michael S. Tsirkin
2026-03-16 10:43 ` Michael S. Tsirkin
2026-03-16 11:57 ` Vishwanath Seshagiri
2026-03-16 12:04 ` Michael S. Tsirkin
2026-03-17 2:30 ` patchwork-bot+netdevbpf
2026-03-23 15:01 ` Omar Elghoul
2026-03-23 15:52 ` Michael S. Tsirkin
2026-03-23 16:54 ` Omar Elghoul
2026-03-23 17:10 ` Michael S. Tsirkin
2026-03-23 16:58 ` Michael S. Tsirkin
2026-03-23 17:09 ` Omar Elghoul
2026-03-23 17:50 ` Vishwanath Seshagiri
2026-03-23 23:37 ` Michael S. Tsirkin
2026-03-24 0:34 ` Jason Wang
2026-03-24 8:20 ` Aithal, Srikanth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox