[PATCH net v3] net: atlantic: stop TX again before freeing rings

[PATCH net v3] net: atlantic: stop TX again before freeing rings

[Hanson Wang]

aq_nic_stop() disables TX before it cancels the service work, frees
the link IRQ and disables NAPI. Those paths can wake the queues again.
ndo_set_features, ethtool ring changes and XDP setup call
aq_ndev_close() directly, without dev_deactivate(), so a re-woken
queue is still reached from ndo_start_xmit while aq_nic_deinit() frees
buff_ring. An IGMP transmit on a UGREEN DXP2800 then oopses in
aq_nic_map_skb() with buff_ring NULL and a non-zero sw_tail.

ndo_xdp_xmit can clear that stopped bit. aq_nic_xmit_xdpf() updates
the queue state, and a drained ring has enough free descriptors to
wake the subqueue. The device stays IFF_UP across this internal
close, so a redirected frame can wake the queue after it was stopped
and a later SKB transmit can hit the freed ring. aq_set_ringparam()
then frees the aq_vec that embeds those rings while aq_ring_tx[]
still points at them.

Close an admission gate before the first queue stop, and publish it
with smp_store_release() only after the rings are initialized.
ndo_xdp_xmit holds rcu_read_lock() across the ring lookup and the
transmit, and returns -ENETDOWN while the gate is closed.
synchronize_net() runs after NAPI is disabled and waits out
transmitters that already passed the gate, including those that woke
a queue. A second netif_tx_disable() then drains SKB transmit before
hw_stop() and the ring free.

Fixes: 3aec6412e007 ("aquantia: Fix Tx queue hangups")
Cc: stable@vger.kernel.org
Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>
---
v3:
- No code change. v2 was left in Changes Requested by mistake.
v2: https://lore.kernel.org/netdev/20261104062716.4096311-1-hanson.wang@ugreen.com/
- Gate ndo_xdp_xmit and synchronize_net() before the second
  netif_tx_disable(). A drained ring otherwise wakes the queue from
  aq_nic_xmit_xdpf() after that stop.
- The same gate keeps ndo_xdp_xmit out of aq_ring_tx[] while
  aq_set_ringparam() frees the aq_vec that embeds the rings.
v1: https://lore.kernel.org/netdev/20261031032308.2991494-1-hanson.wang@ugreen.com/

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 42e0fe9248f288fb052fc0372f8907e4b7ebf501..e791ed18f1cd69ff7d0ca6fb0efcd6396a541415 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -597,6 +597,8 @@ int aq_nic_start(struct aq_nic_s *self)
 
 		netdev_set_tc_queue(self->ndev, i, self->aq_vecs, offset);
 	}
+	/* Publish the initialized rings to external XDP transmitters. */
+	smp_store_release(&self->xdp_tx_enabled, true);
 	netif_tx_start_all_queues(self->ndev);
 
 err_exit:
@@ -1428,6 +1430,8 @@ int aq_nic_stop(struct aq_nic_s *self)
 {
 	unsigned int i = 0U;
 
+	WRITE_ONCE(self->xdp_tx_enabled, false);
+
 	netif_tx_disable(self->ndev);
 	netif_carrier_off(self->ndev);
 
@@ -1448,6 +1452,17 @@ int aq_nic_stop(struct aq_nic_s *self)
 
 	aq_ptp_ring_stop(self);
 
+	/* Drain external XDP transmitters that passed the admission gate.
+	 * They can wake TX queues, so wait before the final queue stop.
+	 */
+	synchronize_net();
+
+	/* Service work, IRQs, NAPI and external XDP can no longer wake
+	 * queues. Drain SKB transmit before aq_nic_deinit() frees rings.
+	 */
+	netif_tx_disable(self->ndev);
+	netif_carrier_off(self->ndev);
+
 	return self->aq_hw_ops->hw_stop(self->aq_hw);
 }
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
index ad33f8586532b859215cc996aeaf1d662babd97a..84ac097ba12405ee257759cfa2241ec8a5ffb37c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
@@ -136,6 +136,8 @@ struct aq_nic_s {
 	unsigned int packet_filter;
 	unsigned int power_state;
 	u8 port;
+	/* Admission gate for ndo_xdp_xmit, drained with synchronize_net(). */
+	bool xdp_tx_enabled;
 	const struct aq_hw_ops *aq_hw_ops;
 	const struct aq_fw_ops *aq_fw_ops;
 	struct aq_nic_cfg_s aq_nic_cfg;
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index e1193c6719d9d6e01ad95e3b80950b70a88d510f..6ff916fe25e424a2e0da44f11504e53e48005d47 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -19,6 +19,7 @@
 #include <linux/bpf_trace.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/rcupdate.h>
 
 static void aq_get_rxpages_xdp(struct aq_ring_buff_s *buff,
 			       struct xdp_buff *xdp)
@@ -422,6 +423,14 @@ int aq_xdp_xmit(struct net_device *dev, int num_frames,
 	struct aq_nic_cfg_s *aq_cfg;
 	struct aq_ring_s *ring;
 
+	/* Cover configuration and ring lookups as well as transmission. */
+	rcu_read_lock();
+	/* Pair with aq_nic_start() to observe initialized rings. */
+	if (unlikely(!smp_load_acquire(&aq_nic->xdp_tx_enabled))) {
+		rcu_read_unlock();
+		return -ENETDOWN;
+	}
+
 	aq_cfg = aq_nic_get_cfg(aq_nic);
 	vec = cpu % aq_cfg->vecs;
 	ring = aq_nic->aq_ring_tx[AQ_NIC_CFG_TCVEC2RING(aq_cfg, 0, vec)];
@@ -433,6 +442,8 @@ int aq_xdp_xmit(struct net_device *dev, int num_frames,
 			drop++;
 	}
 
+	rcu_read_unlock();
+
 	return num_frames - drop;
 }
 

Re: [PATCH net v3] net: atlantic: stop TX again before freeing rings

[Jakub Kicinski]

On Mon,  9 Nov 2026 15:19:35 +0800 Hanson Wang wrote:
> +	/* Drain external XDP transmitters that passed the admission gate.
> +	 * They can wake TX queues, so wait before the final queue stop.
> +	 */
> +	synchronize_net();

Is this really needed? presumably you removed a NAPI at some point which
implies a RCU sync.

> +	/* Service work, IRQs, NAPI and external XDP can no longer wake
> +	 * queues. Drain SKB transmit before aq_nic_deinit() frees rings.
> +	 */
> +	netif_tx_disable(self->ndev);
> +	netif_carrier_off(self->ndev);

Double-stopping is pretty hacky. Can you figure out a way of avoiding 
the race instead of stopping the queues twice?
-- 
pw-bot: cr