veth: Fix TXQ stall race condition and add recovery

The changes introduced in commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops") have been found to cause a race condition in production environments.

Under specific circumstances, observed exclusively on ARM64 (aarch64) systems with Ampere Altra Max CPUs, a transmit queue (TXQ) can become permanently stalled. This happens when the race condition leads to the TXQ entering the QUEUE_STATE_DRV_XOFF state without a corresponding queue wake-up, preventing the attached qdisc from dequeueing packets and causing the network link to halt.

As a first step towards resolving this issue, this patch introduces a failsafe mechanism. It enables the net device watchdog by setting a timeout value and implements the .ndo_tx_timeout callback.

If a TXQ stalls, the watchdog will trigger the veth_tx_timeout() function, which logs a warning and calls netif_tx_wake_queue() to unstall the queue and allow traffic to resume.

The log message will look like this:

veth42: NETDEV WATCHDOG: CPU: 34: transmit queue 0 timed out 5393 ms
veth42: veth backpressure stalled(n:1) TXQ(0) re-enable

This provides a necessary recovery mechanism while the underlying race condition is investigated further. Subsequent patches will address the root cause and add more robust state handling.

Fixes dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")

Reviewed-by Toke Høiland-Jørgensen <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

drivers/net/veth.c +5 -0

+	netif_tx_wake_queue(txq);
+}
+
 static int veth_open(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);

@@ -1711,6 +1723,7 @@ static const struct net_device_ops veth_netdev_ops = {
 	.ndo_bpf		= veth_xdp,
 	.ndo_xdp_xmit		= veth_ndo_xdp_xmit,
 	.ndo_get_peer_dev	= veth_peer_dev,
+	.ndo_tx_timeout		= veth_tx_timeout,
 };
 
 static const struct xdp_metadata_ops veth_xdp_metadata_ops = {

@@ -1749,6 +1762,7 @@ static void veth_setup(struct net_device *dev)
 	dev->priv_destructor = veth_dev_free;
 	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
 	dev->max_mtu = ETH_MAX_MTU;
+	dev->watchdog_timeo = msecs_to_jiffies(5000);
 
 	dev->hw_features = VETH_FEATURES;
 	dev->hw_enc_features = VETH_FEATURES;

Quoted 28 lines · On Wed, 05 Nov 2025 18:28:12 +0100 Jesper Dangaard Brouer wrote:

The changes introduced in commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops") have been found to cause a race condition in production environments.

Under specific circumstances, observed exclusively on ARM64 (aarch64) systems with Ampere Altra Max CPUs, a transmit queue (TXQ) can become permanently stalled. This happens when the race condition leads to the TXQ entering the QUEUE_STATE_DRV_XOFF state without a corresponding queue wake-up, preventing the attached qdisc from dequeueing packets and causing the network link to halt.

As a first step towards resolving this issue, this patch introduces a failsafe mechanism. It enables the net device watchdog by setting a timeout value and implements the .ndo_tx_timeout callback.

If a TXQ stalls, the watchdog will trigger the veth_tx_timeout() function, which logs a warning and calls netif_tx_wake_queue() to unstall the queue and allow traffic to resume.

The log message will look like this:

veth42: NETDEV WATCHDOG: CPU: 34: transmit queue 0 timed out 5393 ms
veth42: veth backpressure stalled(n:1) TXQ(0) re-enable

This provides a necessary recovery mechanism while the underlying race condition is investigated further. Subsequent patches will address the root cause and add more robust state handling.

Quoted 3 lines

Fixes dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")

Reviewed-by Toke Høiland-Jørgensen <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

I think this belongs in net-next.. Fail safe is not really a bug fix. I'm slightly worried we're missing a corner case and will cause timeouts to get printed for someone's config.

drivers/net/veth.c static int veth_set_channels(struct net_device *dev, · line 1383

Quoted 6 lines

drivers/net/veth.c +6 -0

+static void veth_tx_timeout(struct net_device *dev, unsigned int txqueue)
+{
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, txqueue);
+
+	netdev_err(dev, "veth backpressure stalled(n:%ld) TXQ(%u) re-enable\n",
+		   atomic_long_read(&txq->trans_timeout), txqueue);

If you think the trans_timeout is useful, let's add it to the message core prints? And then we can make this msg just veth specific, I don't think we should be repeating what core already printed.

Quoted 3 lines

I think this belongs in net-next.. Fail safe is not really a bug fix. I'm slightly worried we're missing a corner case and will cause timeouts to get printed for someone's config.

This is a recovery fix. If the race condition fix isn't 100% then this patch will allow veth to recover. Thus, to me it makes sense to group these two patches together.

I'm more worried that we we're missing a corner case that we cannot recover from. Than triggering timeouts to get printed, for a config where NAPI consumer veth_poll() takes more that 5 seconds to run (budget max 64 packets this needs to consume packets at a rate less than 12.8 pps). It might be good to get some warnings if the system is operating this slow.

Also remember this is not the default config that most people use. The code is only activated if attaching a qdisc to veth, which isn't default. Plus, NAPI mode need to be activated, where in normal NAPI mode the producer and consumer usually runs on the same CPU, which makes it impossible to overflow the ptr_ring. The veth backpressure is primarily needed when running with threaded-NAPI, where it is natural that producer and consumer runs on different CPUs. In our production setup the consumer is always slower than the producer (as the product inside the namespace have installed too many nftables rules).

Quoted 10 lines

drivers/net/veth.c static int veth_set_channels(struct net_device *dev, · line 1383

Quoted 6 lines

drivers/net/veth.c +6 -0

+static void veth_tx_timeout(struct net_device *dev, unsigned int txqueue)
+{
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, txqueue);
+
+	netdev_err(dev, "veth backpressure stalled(n:%ld) TXQ(%u) re-enable\n",
+		   atomic_long_read(&txq->trans_timeout), txqueue);  

If you think the trans_timeout is useful, let's add it to the message core prints? And then we can make this msg just veth specific, I don't think we should be repeating what core already printed.

The trans_timeout is a counter for how many times this TXQ have seen a timeout. It is practical as it directly tell us if this a frequent event (without having to search log files for similar events).

It does make sense to add this to the core message ("NETDEV WATCHDOG") with the same argument. For physical NICs these logs are present in production. Looking at logs through Kibana (right now) and it would make my life easier to see the number of times the individual queues have experienced timeouts. The logs naturally gets spaced in time by the timeout, making it harder to tell the even frequency. Such a patch would naturally go though net-next.

Do you still want me to remove the frequency counter from this message? By the same argument it is practical for me to have as a single log line when troubleshooting this in practice.

BTW, I've already backported this watchdog patch to prod kernel (without race fix) and I'll try to reproduce the race in staging/lab on some ARM64 servers. If I reproduce it will be practical to have this counter.

Quoted 15 lines · On Fri, 7 Nov 2025 14:42:58 +0100 Jesper Dangaard Brouer wrote:

Quoted 3 lines

I think this belongs in net-next.. Fail safe is not really a bug fix. I'm slightly worried we're missing a corner case and will cause timeouts to get printed for someone's config.

This is a recovery fix. If the race condition fix isn't 100% then this patch will allow veth to recover. Thus, to me it makes sense to group these two patches together.

I'm more worried that we we're missing a corner case that we cannot recover from. Than triggering timeouts to get printed, for a config where NAPI consumer veth_poll() takes more that 5 seconds to run (budget max 64 packets this needs to consume packets at a rate less than 12.8 pps). It might be good to get some warnings if the system is operating this slow.

Quoted 9 lines

Also remember this is not the default config that most people use. The code is only activated if attaching a qdisc to veth, which isn't default. Plus, NAPI mode need to be activated, where in normal NAPI mode the producer and consumer usually runs on the same CPU, which makes it impossible to overflow the ptr_ring. The veth backpressure is primarily needed when running with threaded-NAPI, where it is natural that producer and consumer runs on different CPUs. In our production setup the consumer is always slower than the producer (as the product inside the namespace have installed too many nftables rules).

I understand all of this, but IMO the fix is in patch 2. This is a resiliency improvement, not a fix.

Commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops") introduced a race condition that can lead to a permanently stalled TXQ. This was observed in production on ARM64 systems (Ampere Altra Max).

The race occurs in veth_xmit(). The producer observes a full ptr_ring and stops the queue (netif_tx_stop_queue()). The subsequent conditional logic, intended to re-wake the queue if the consumer had just emptied it (if (__ptr_ring_empty(...)) netif_tx_wake_queue()), can fail. This leads to a "lost wakeup" where the TXQ remains stopped (QUEUE_STATE_DRV_XOFF) and traffic halts.

This failure is caused by an incorrect use of the __ptr_ring_empty() API from the producer side. As noted in kernel comments, this check is not guaranteed to be correct if a consumer is operating on another CPU. The empty test is based on ptr_ring->consumer_head, making it reliable only for the consumer. Using this check from the producer side is fundamentally racy.

This patch fixes the race by adopting the more robust logic from an earlier version V4 of the patchset, which always flushed the peer:

(1) In veth_xmit(), the racy conditional wake-up logic and its memory barrier are removed. Instead, after stopping the queue, we unconditionally call __veth_xdp_flush(rq). This guarantees that the NAPI consumer is scheduled, making it solely responsible for re-waking the TXQ.
This handles the race where veth_poll() consumes all packets and completes
NAPI *before* veth_xmit() on the producer side has called netif_tx_stop_queue. The __veth_xdp_flush(rq) will observe rx_notify_masked is false and schedule NAPI.

(2) On the consumer side, the logic for waking the peer TXQ is moved out of veth_xdp_rcv() and placed at the end of the veth_poll() function. This placement is part of fixing the race, as the netif_tx_queue_stopped() check must occur after rx_notify_masked is potentially set to false during NAPI completion.
This handles the race where veth_poll() consumes all packets, but haven't
finished (rx_notify_masked is still true). The producer veth_xmit() stops the TXQ and __veth_xdp_flush(rq) will observe rx_notify_masked is true, meaning not starting NAPI. Then veth_poll() change rx_notify_masked to false and stops NAPI. Before exiting veth_poll() will observe TXQ is stopped and wake it up.

Fixes dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")

Signed-off-by Jesper Dangaard Brouer <[email protected]>