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 --- v2: - 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. pw-bot: cr 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 #include #include +#include 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; }