[PATCH net] virtio_net: skip Rx refill when polled by netpoll

[PATCH net] virtio_net: skip Rx refill when polled by netpoll

[Breno Leitao]

We are seeing pp_ref_count underflow warnings on virtio_net hosts with
heavy netconsole traffic, followed by general protection faults in
napi_consume_skb() on frag addresses made of packet data, when
skb_defer_free_flush() frees the corrupted skbs:

  WARNING: ./include/net/page_pool/helpers.h:297 at page_pool_alloc_frag_netmem+0x1d9/0x230, CPU#540: swapper/540/0
  Call Trace:
   <IRQ>
   try_fill_recv+0x399/0x940
   virtnet_poll+0x538/0xc10
   net_rx_action+0x21d/0x610
   irq_exit_rcu+0xf9/0x3a0
   common_interrupt+0x85/0x90
   </IRQ>

The theory is that virtnet_poll() refills the Rx ring when netpoll
calls it. netpoll polls NAPI with a zero budget to reclaim Tx buffers,
on whatever CPU is transmitting, but virtnet_receive() still refills
it.

Since commit 24fbd3967f3f ("virtio_net: add page_pool support for buffer
allocation") that refill allocates from the page pool, which must not be
used with a zero budget (see Documentation/networking/napi.rst).

A reproducer on the net tree supports the theory: an 8-CPU arm64 VM with
QEMU user networking, a netconsole target on the virtio_net device and
three TCP receivers on other CPUs checking a known byte pattern. A
printk flood through netconsole corrupted all three streams within about
three minutes, after which recvmsg() failed with -EFAULT and the same
warning fired. A test kthread calling netpoll_poll_dev() in a loop, as
__netpoll_send_skb() does while the Tx queue is busy, corrupts the
streams within seconds.

Only reclaim Tx when the budget is zero

Fixes: 24fbd3967f3f ("virtio_net: add page_pool support for buffer allocation")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/virtio_net.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dd2c41ef6fbe72..9db5ec875c141f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3037,6 +3037,9 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
 
 	virtnet_poll_cleantx(rq, budget);
 
+	if (unlikely(!budget))
+		return 0;
+
 	received = virtnet_receive(rq, budget, &xdp_xmit);
 	rq->packets_in_napi += received;
 

---
base-commit: 1d5e2c57e9ffc134e14b3f837c8d7c53b508bd22
change-id: 20261109-virtio-net-netpoll-pp-170f0dc4d6e0

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH net] virtio_net: skip Rx refill when polled by netpoll

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch is incomplete: zero-budget netpoll polling can still
  call page pool APIs, which the commit message itself says must not…
- [Low] The commit message calls the root cause 'the theory' and
  describes two reproducers: a netconsole printk flood that corrupts TCP…

--

virtio_net: skip Rx refill when polled by netpoll

This commit makes virtnet_poll() return right after Tx reclaim when it is
called with a zero budget, which is how netpoll calls it. The goal is to
keep the Rx refill path from allocating from the page pool outside a real
NAPI poll. It targets pp_ref_count underflows and skb corruption seen with
heavy netconsole traffic since page_pool support was added.

> A reproducer on the net tree supports the theory: an 8-CPU arm64 VM with
> QEMU user networking, a netconsole target on the virtio_net device and
> three TCP receivers on other CPUs checking a known byte pattern. A
> printk flood through netconsole corrupted all three streams within about
> three minutes, after which recvmsg() failed with -EFAULT and the same
> warning fired. A test kthread calling netpoll_poll_dev() in a loop, as
> __netpoll_send_skb() does while the Tx queue is busy, corrupts the
> streams within seconds.
>
> Only reclaim Tx when the budget is zero

[Severity: Low]
This isn't a bug, but the message calls the root cause a theory and
describes two reproducers. It never says whether they stop failing with
this patch applied.

Could the message say whether the pp_ref_count underflow, the
napi_consume_skb() faults and the stream corruption no longer reproduce
with this change?

Also, the last sentence "Only reclaim Tx when the budget is zero" is
missing a trailing period.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index dd2c41ef6fbe7..9db5ec875c141 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3037,6 +3037,9 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>  
>  	virtnet_poll_cleantx(rq, budget);
>  
> +	if (unlikely(!budget))
> +		return 0;
> +

[Severity: High]
Does this keep all page pool calls out of the zero-budget path? The early
return comes after virtnet_poll_cleantx(rq, budget), and that still runs
when netpoll calls virtnet_poll() with budget == 0.

When XDP falls back to the shared, locked Tx mode, virtnet_xdp_set() does:

	if (curr_qp + xdp_qp > vi->max_queue_pairs) {
		...
		xdp_qp = 0;
	}

In that mode is_xdp_raw_buffer_queue() is false for every queue. XDP_TX and
redirected frames then land on the same sq that virtnet_poll_cleantx()
reclaims.

The XDP case in __free_old_xmit() does not look at in_napi:

		case VIRTNET_XMIT_TYPE_XDP:
			frame = ptr;
			...
			xdp_return_frame(frame);
			break;

Since 24fbd3967f3f the rxq is registered as MEM_TYPE_PAGE_POOL, so the
call chain becomes:

poll_napi()->poll_one_napi()->virtnet_poll(napi, 0)
  virtnet_poll_cleantx(rq, 0)
    free_old_xmit(sq, txq, false)
      __free_old_xmit()
        xdp_return_frame()
          page_pool_put_unrefed_netmem()

In the usual netconsole case, netpoll runs in process context with IRQs
disabled by __netpoll_send_skb(). page_pool_napi_local() returns false
there. The page then goes through page_pool_recycle_in_ring() and
page_pool_producer_lock(), which call spin_lock_bh() and spin_unlock_bh()
with IRQs off.

Won't that trip lockdep_assert_irqs_enabled() in __local_bh_enable_ip()?

If a softirq is pending, softirqs would also run nested inside netpoll
with IRQs re-enabled. For example, virtnet_tx_wake_queue() on an earlier
queue in the same poll_napi() loop raises NET_TX_SOFTIRQ.

At that point netpoll holds napi->poll_owner and the sq txq lock. Could a
nested napi_poll() of the same rq spin forever in netpoll_poll_lock()?
Could a nested XDP_TX to the same txq spin in __netif_tx_lock()?

Netpoll can also run from a hardirq that interrupted net_rx_action() on
the CPU that owns the source rq's NAPI. For netconsole this happens during
panic. In that case page_pool_napi_local() returns true. The frame is then
pushed into the lockless pool->alloc.cache while the interrupted NAPI may
be using it.

Isn't that the same alloc-cache and pp_ref_count corruption this patch is
trying to fix?

Documentation/networking/napi.rst, which the commit message cites, says no
XDP or page pool APIs may be called when the budget is 0. Should
VIRTNET_XMIT_TYPE_XDP reclaim also be skipped or deferred when budget is 0?

>  	received = virtnet_receive(rq, budget, &xdp_xmit);
>  	rq->packets_in_napi += received;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-virtio-net-netpoll-pp-v1-1-3f004b406d71%40debian.org