Hi Michael, Kuba,
While troubleshooting virtual network issues in a production environment,
I once ran into a problem where the virtio_net receive queue became
permanently stuck.
The virtio_net RX path relies on backend interrupt notifications.
The backend(vhost_net/vhost-user) appends buffers to the used ring
and then notifies the guest, relying on the hypervisor(KVM) to inject
the interrupt. On receiving it, virtio_net schedules NAPI, drains the
used ring and refills descriptors.
If the hypervisor fails to inject the interrupt(e.g. a transient KVM
failure), the guest never schedules NAPI, so it neither consumes buffers
nor returns descriptors. The backend keeps appending until the ring is full,
then stops and, with no free descriptors left, also stops sending
notifications. Both sides now wait for the other, and the RX queue is
permanently stuck.
The root cause is the lost interrupt in the hypervisor, and the proper
fix belongs there. Nevertheless, I believe virtio_net needs a way to
observe and diagnose it. Today, once a queue hangs, there is no signal
to the operator that anything is wrong: the RX path has no equivalent of
the TX watchdog (ndo_tx_timeout). This series lets virtio_net detect a
stuck RX queue. It is detection only: the driver records the event and
logs a warning, leaving recovery to a follow-up if desired.
Implementation
--------------
Patch 1 exports virtqueue_get_last_used_idx(), a read-only accessor for
the last_used_idx. Together with the existing virtqueue_poll(), it lets
a driver ask whether the device has produced buffers that the driver has
not consumed yet ("backlog").
Patch 2 adds a per-device RX watchdog that scans all RX queues once per
second. A queue is considered stuck when, for rx_watchdog_timeo seconds
(default 5, 0 disables it), all of the following hold simultaneously:
- the queue has a non-zero backlog (used.idx != last_used_idx);
- the driver makes no consumption progress (last_used_idx unchanged);
- no new interrupt arrives (rq->calls unchanged).
On detection it logs a warning, rate-limited to once per timeout.
Patch 3 adds a per-queue rx_timeouts statistic, exposed via ethtool -S,
so the number of watchdog events can be observed per queue.
RFC
---
This is sent as an RFC to gather feedback on a few open points:
- Scope: this series only detects the stuck queue. Is it better to
keep detection and recovery separate, or should recovery (forcing a
NAPI poll to drain the queue) be included?
- Default: rx_watchdog_timeo defaults to 5 seconds (enabled). Should
the watchdog be enabled by default, or disabled unless explicitly
requested?
Parts of this series, including portions of this cover letter, were drafted
with AI assistance. I have carefully reviewed everything;
questions and comments are welcome.
Thanks!
Lange
Longjun Tang (3):
virtio: add virtqueue_get_last_used_idx() helper
virtio_net: introduce an RX queue watchdog for stuck detection
virtio_net: add rx_timeouts per-queue statistic
drivers/net/virtio_net.c | 95 ++++++++++++++++++++++++++++++++++++
drivers/virtio/virtio_ring.c | 18 ++++++-
include/linux/virtio.h | 2 +
3 files changed, 113 insertions(+), 2 deletions(-)
--
2.25.1
From: Longjun Tang <redacted>
The backend raises an interrupt after adding buffers to the used ring,
and the hypervisor injects it. If one is lost (e.g., a transient KVM
failure), NAPI will not be scheduled and the driver stops consuming
buffers. The ring eventually fills up, the backend stops notifying, and
the queue is left permanently stuck.
Add a per-device watchdog that scans the RX queues once per second and
detects a queue that has a non-zero backlog while making no consumption
progress and receiving no new interrupt for rx_watchdog_timeo seconds
(default 5, 0 disables it). On detection it logs a warning, rate-limited
to once per timeout.
Assisted-by: Kilo:deepseek-v4-pro
Signed-off-by: Longjun Tang <redacted>
---
drivers/net/virtio_net.c | 88 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
@@ -330,6 +334,11 @@ struct receive_queue {/* The number of rx notifications */u16calls;+/* RX watchdog state for stuck detection. */+u16watchdog_last_used_idx;+u16watchdog_calls;+unsignedlongwatchdog_jiffies;+/* Is dynamic interrupt moderation enabled? */booldim_enabled;
@@ -441,6 +450,9 @@ struct virtnet_info {/* Work struct for setting rx mode */structwork_structrx_mode_work;+/* RX watchdog timer for stuck detection */+structtimer_listrx_watchdog;+/* OK to queue work setting RX mode? */boolrx_mode_work_enabled;
@@ -3046,6 +3058,77 @@ static int virtnet_poll(struct napi_struct *napi, int budget)returnreceived;}+/*+*Thiswatchdogdetectsthatstate:aqueueisconsideredstalledwhen+*ithasanon-zerobacklog,makesnoconsumptionprogressandreceives+*nonewinterruptforrx_watchdog_timeoseconds.Ondetectionitlogs+*awarning.+*/+staticvoidvirtnet_rx_watchdog(structtimer_list*t)+{+structvirtnet_info*vi=timer_container_of(vi,t,rx_watchdog);+unsignedlongtimeout=rx_watchdog_timeo*HZ;+inti;++if(!rx_watchdog_timeo)+return;++for(i=0;i<vi->curr_queue_pairs;i++){+structreceive_queue*rq=&vi->rq[i];+u16last_used=virtqueue_get_last_used_idx(rq->vq);+u16calls=rq->calls;+boolbacklog=virtqueue_poll(rq->vq,last_used);++if(!backlog||last_used!=rq->watchdog_last_used_idx||+calls!=rq->watchdog_calls){+/* No pending data, or the queue made progress, or a+*newinterruptarrived:restartthewindow.+*/+rq->watchdog_last_used_idx=last_used;+rq->watchdog_calls=calls;+rq->watchdog_jiffies=jiffies;+continue;+}++if(time_after(jiffies,rq->watchdog_jiffies+timeout)){+unsignedintstall_ms=+jiffies_to_msecs(jiffies-rq->watchdog_jiffies);++netdev_warn(vi->dev,"RX queue %u stalled for %u ms\n",+i,stall_ms);++/* Rate-limit to one event per timeout. */+rq->watchdog_jiffies=jiffies;+}+}++mod_timer(&vi->rx_watchdog,jiffies+HZ);+}++staticvoidvirtnet_rx_watchdog_start(structvirtnet_info*vi)+{+inti;++if(!rx_watchdog_timeo)+return;++for(i=0;i<vi->curr_queue_pairs;i++){+structreceive_queue*rq=&vi->rq[i];++rq->watchdog_last_used_idx=+virtqueue_get_last_used_idx(rq->vq);+rq->watchdog_calls=rq->calls;+rq->watchdog_jiffies=jiffies;+}++mod_timer(&vi->rx_watchdog,jiffies+HZ);+}++staticvoidvirtnet_rx_watchdog_stop(structvirtnet_info*vi)+{+timer_delete_sync(&vi->rx_watchdog);+}+staticvoidvirtnet_disable_queue_pair(structvirtnet_info*vi,intqp_index){virtnet_napi_tx_disable(&vi->sq[qp_index]);
@@ -3209,6 +3292,8 @@ static int virtnet_open(struct net_device *dev)netif_carrier_on(dev);}+virtnet_rx_watchdog_start(vi);+return0;err_enable_qp:
@@ -3802,6 +3887,8 @@ static int virtnet_close(struct net_device *dev)structvirtnet_info*vi=netdev_priv(dev);inti;+virtnet_rx_watchdog_stop(vi);+/* Prevent the config change callback from changing carrier*afterclose*/
@@ -6869,6 +6956,7 @@ static int virtnet_probe(struct virtio_device *vdev)INIT_WORK(&vi->config_work,virtnet_config_changed_work);INIT_WORK(&vi->rx_mode_work,virtnet_rx_mode_work);+timer_setup(&vi->rx_watchdog,virtnet_rx_watchdog,0);if(virtio_has_feature(vdev,VIRTIO_NET_F_MRG_RXBUF)){vi->mergeable_rx_bufs=true;
From: Longjun Tang <redacted>
Export a read-only accessor for the last consumed used ring index.
It's useful for drivers that need to detect whether the device has
produced buffers that have not been consumed yet, by passing the
returned value to virtqueue_poll().
The split ring now writes the field with WRITE_ONCE() (the packed
ring already did) to document that the new reader can run concurrently
with the poll path.
Assisted-by: Kilo:deepseek-v4-pro
Signed-off-by: Longjun Tang <redacted>
---
drivers/virtio/virtio_ring.c | 18 ++++++++++++++++--
include/linux/virtio.h | 2 ++
2 files changed, 18 insertions(+), 2 deletions(-)
@@ -1001,7 +1001,7 @@ static void *virtqueue_get_buf_ctx_split(struct vring_virtqueue *vq,/* detach_buf_split clears data, so grab it now. */ret=vq->split.desc_state[i].data;detach_buf_split(vq,i,ctx);-vq->last_used_idx++;+WRITE_ONCE(vq->last_used_idx,vq->last_used_idx+1);/* If we expect an interrupt for the next entry, tell host*bywritingeventindexandflushoutthewritebefore*thereadinthenextget_bufcall.*/
@@ -1068,7 +1068,7 @@ static void *virtqueue_get_buf_ctx_split_in_order(struct vring_virtqueue *vq,ret=vq->split.desc_state[last_used].data;detach_buf_split_in_order(vq,last_used,ctx);-vq->last_used_idx++;+WRITE_ONCE(vq->last_used_idx,vq->last_used_idx+1);vq->last_used+=(vq->vq.num_free-num_free);/* If we expect an interrupt for the next entry, tell host*bywritingeventindexandflushoutthewritebefore
From: Longjun Tang <redacted>
Add an rx_timeouts field to the per-queue RX statistics, expose
it via ethtool and increment it when the RX watchdog fires,
so RX watchdog timeouts can be observed per queue.
Assisted-by: Kilo:deepseek-v4-pro
Signed-off-by: Longjun Tang <redacted>
---
drivers/net/virtio_net.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
From: Longjun Tang <redacted>
Export a read-only accessor for the last consumed used ring index.
It's useful for drivers that need to detect whether the device has
produced buffers that have not been consumed yet, by passing the
returned value to virtqueue_poll().
The split ring now writes the field with WRITE_ONCE() (the packed
ring already did) to document that the new reader can run concurrently
with the poll path.
Assisted-by: Kilo:deepseek-v4-pro
Signed-off-by: Longjun Tang <redacted>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing watchdog state initialization when dynamically increasing receive queues, leading to immediate false stall warnings.
--
commit c9ed39984b2f85816232c0d723aba63e3a109800
Author: Longjun Tang [off-list ref]
virtio_net: introduce an RX queue watchdog for stuck detection
This commit introduces a per-device watchdog to detect stalled RX queues in
virtio-net. The watchdog periodically scans queues and logs a warning if a
queue has a non-zero backlog but makes no progress and receives no interrupts.
@@ -3046,6 +3058,77 @@ static int virtnet_poll(struct napi_struct *napi, int budget) return received; }+/*+ * This watchdog detects that state: a queue is considered stalled when+ * it has a non-zero backlog, makes no consumption progress and receives+ * no new interrupt for rx_watchdog_timeo seconds. On detection it logs+ * a warning.+ */+static void virtnet_rx_watchdog(struct timer_list *t)+{+ struct virtnet_info *vi = timer_container_of(vi, t, rx_watchdog);+ unsigned long timeout = rx_watchdog_timeo * HZ;+ int i;++ if (!rx_watchdog_timeo)+ return;++ for (i = 0; i < vi->curr_queue_pairs; i++) {+ struct receive_queue *rq = &vi->rq[i];+ u16 last_used = virtqueue_get_last_used_idx(rq->vq);+ u16 calls = rq->calls;+ bool backlog = virtqueue_poll(rq->vq, last_used);++ if (!backlog || last_used != rq->watchdog_last_used_idx ||+ calls != rq->watchdog_calls) {+ /* No pending data, or the queue made progress, or a+ * new interrupt arrived: restart the window.+ */+ rq->watchdog_last_used_idx = last_used;+ rq->watchdog_calls = calls;+ rq->watchdog_jiffies = jiffies;+ continue;+ }++ if (time_after(jiffies, rq->watchdog_jiffies + timeout)) {
[Severity: Medium]
When receive queues are dynamically increased via ethtool, does this condition
falsely evaluate to true?
Looking at virtnet_set_queues(), it increases vi->curr_queue_pairs but doesn't
initialize watchdog_jiffies for the newly added queues:
virtnet_set_queues() {
...
vi->curr_queue_pairs = queue_pairs;
if (dev->flags & IFF_UP) {
local_bh_disable();
for (int i = 0; i < vi->curr_queue_pairs; ++i)
virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
local_bh_enable();
}
...
}
If the watchdog timer executes right after NAPI fills the new ring and a
packet is immediately received (making backlog true), the watchdog evaluates
the time_after() condition.
Since watchdog_jiffies remains 0 for the new queues, would this immediately
log a false positive "RX queue stalled" warning with a huge stall time?
+ unsigned int stall_ms =
+ jiffies_to_msecs(jiffies - rq->watchdog_jiffies);
+
+ netdev_warn(vi->dev, "RX queue %u stalled for %u ms\n",
+ i, stall_ms);
+
+ /* Rate-limit to one event per timeout. */
+ rq->watchdog_jiffies = jiffies;
+ }
+ }
+
+ mod_timer(&vi->rx_watchdog, jiffies + HZ);
+}
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent writers to per-queue u64_stats_sync from timer context and NAPI violate mutual exclusion.
--
commit 2e5fa229ff4c8c773848d0c1f36b14c2d2f2edf7
Author: Longjun Tang [off-list ref]
virtio_net: add rx_timeouts per-queue statistic
This commit adds an rx_timeouts field to the per-queue RX statistics.
It increments the counter when the RX watchdog fires and exposes it
via ethtool to track per-queue stall events.
[Severity: High]
Does this introduce a race condition with the NAPI poll loop?
The u64_stats_sync API requires mutually exclusive writers per syncp.
This timer callback (virtnet_rx_watchdog) executes in a global timer softirq
context on an arbitrary CPU and writes to rq->stats.syncp.
Concurrently, NAPI polling executes in the receive queue's bound softirq
context on another CPU and also writes to rq->stats.syncp. Since
rx_timeouts was added to virtnet_rq_stats_desc, NAPI dynamically iterates
over it during virtnet_receive():
drivers/net/virtio_net.c:virtnet_receive() {
...
u64_stats_add(item, u64_stats_read(src));
...
}
Concurrent writers corrupt the sequence counter on 32-bit systems, causing
readers (such as ndo_get_stats64 via ethtool) to spin in an infinite
retry loop.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260908092646.108865-1-lange_tang@163.com?part=3
From: Lange Tang <hidden> Date: 2026-09-16 08:16:05
At 2026-09-09 17:27:18, sashiko-bot@kernel.org wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent writers to per-queue u64_stats_sync from timer context and NAPI violate mutual exclusion.
--
commit 2e5fa229ff4c8c773848d0c1f36b14c2d2f2edf7
Author: Longjun Tang [off-list ref]
virtio_net: add rx_timeouts per-queue statistic
This commit adds an rx_timeouts field to the per-queue RX statistics.
It increments the counter when the RX watchdog fires and exposes it
via ethtool to track per-queue stall events.
[Severity: High]
Does this introduce a race condition with the NAPI poll loop?
The u64_stats_sync API requires mutually exclusive writers per syncp.
This timer callback (virtnet_rx_watchdog) executes in a global timer softirq
context on an arbitrary CPU and writes to rq->stats.syncp.
Concurrently, NAPI polling executes in the receive queue's bound softirq
context on another CPU and also writes to rq->stats.syncp. Since
rx_timeouts was added to virtnet_rq_stats_desc, NAPI dynamically iterates
over it during virtnet_receive():
drivers/net/virtio_net.c:virtnet_receive() {
...
u64_stats_add(item, u64_stats_read(src));
...
}
Concurrent writers corrupt the sequence counter on 32-bit systems, causing
readers (such as ndo_get_stats64 via ethtool) to spin in an infinite
retry loop.
This is a real bug. u64_stats_sync requires mutually exclusive writers, but the
watchdog timer runs on an arbitrary CPU while the queue's NAPI aggregates rq->stats under the same syncp.
I will fix this in the next revision by moving the rx_timeouts accounting write into the NAPI poll; the watchdog
will only detect the stall and call napi_schedule().
I would appreciate any feedback, especially on whether this direction is worth pursuing.
Thanks.
Longjun