[PATCH net v2 0/4] net: axienet: fix TX queue handling around a DMA error reset

[PATCH net v2 0/4] net: axienet: fix TX queue handling around a DMA error reset

[Sagi Maimon]

axienet_dma_err_handler() resets the DMA engine together with the MAC
and rebuilds the TX ring, but it does not coordinate with the transmit
path, and it does not leave the MAC configured the way it found it.

v1 was a single patch that woke the queue at the end of the handler.
The Sashiko review showed that this wake could be lost to a concurrent
transmit and could undo the stop that suspend installs, and it pointed
out two older problems: the handler races axienet_start_xmit() while it
tears down the ring, and the reset loses the negotiated link speed and
pause settings.

This version:

1/4 starts the TX queue in axienet_open().  Nothing does today, so a
    queue stopped at close stays stopped.  2/4 depends on it.
2/4 quiesces the TX path in axienet_stop() before the ring is freed,
    which the suspend path otherwise leaves racing.
3/4 quiesces the TX queue across the whole error reset and wakes it at
    the end, unless the interface is stopping or detached.
4/4 has phylink reprogram the link settings after the reset.

v1 also called netif_trans_update() and said this stopped the watchdog
from refiring.  No .ndo_tx_timeout is installed in this tree, so the
call and the claim are both gone.

Tested on an AXI Ethernet MAC behind a PCIe endpoint with the series
applied: traffic passes, and resumes after each of ten down/up cycles
and five module reloads, all made with traffic running, with nothing
logged.  Suspend, a down/up cycle with the queue stopped by a full ring,
and the DMA error path were not exercised.

v1 omitted the Assisted-by: tag; v2 adds it.

v1: https://lore.kernel.org/netdev/20260917121306.23499-1-maimon.sagi@gmail.com/

Sagi Maimon (4):
  net: axienet: start the TX queue in axienet_open()
  net: axienet: quiesce the TX path before axienet_stop() tears it down
  net: axienet: quiesce the TX queue across a DMA error reset
  net: axienet: reprogram the link settings after a DMA error reset

 .../net/ethernet/xilinx/xilinx_axienet_main.c | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)


base-commit: 879e280b8486d4612ad1aa050d6fada2dd80cf1c
-- 
2.47.0

[PATCH net v2 1/4] net: axienet: start the TX queue in axienet_open()

[Sagi Maimon]

axienet_open() never starts the transmit queue, and nothing else does
either: __dev_open() and dev_activate() leave the driver's queue state
alone.

axienet_start_xmit() stops the queue with netif_stop_queue() when the TX
ring fills, and axienet_tx_poll() wakes it again as completions free
descriptors.  If the interface is brought down while the queue is
stopped, __QUEUE_STATE_DRV_XOFF survives into the next axienet_open().
The ring is reinitialised empty, so no TX completion ever arrives to run
the wake in axienet_tx_poll(), and the interface cannot transmit until
the driver is reloaded.  The resume path is unaffected only because
netif_device_attach() wakes the queues.

Start the queue at the end of a successful axienet_open(), as most
drivers do.

Tested on an AXI Ethernet MAC behind a PCIe endpoint: traffic passes,
and resumes after each of ten down/up cycles and five module reloads,
all made with traffic running.  A queue left stopped across a down/up
cycle, with the ring full, was not reproduced.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 782f903d318f..fb26d2e39fac 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1700,6 +1700,11 @@ static int axienet_open(struct net_device *ndev)
 			goto err_phy;
 	}
 
+	/* Nothing else clears a stop left over from before the last close:
+	 * the ring is empty, so no TX completion will wake the queue.
+	 */
+	netif_start_queue(ndev);
+
 	return 0;
 
 err_free_eth_irq:
-- 
2.47.0

[PATCH net v2 2/4] net: axienet: quiesce the TX path before axienet_stop() tears it down

[Sagi Maimon]

On the legacy DMA path axienet_stop() stops the DMA engine and frees the
TX descriptor ring with axienet_dma_bd_release(), but never stops the
transmit queue or waits for a transmit already in progress.

On the dev_close() path this is covered by the core:
dev_deactivate_many() has already quiesced the qdisc and waited for
in-flight transmits with synchronize_net().  axienet_suspend() instead
calls axienet_stop() directly.  Its netif_device_detach() only sets
__QUEUE_STATE_DRV_XOFF, without taking the transmit lock, so an
axienet_start_xmit() that was already running can still be writing a
descriptor into lp->tx_bd_v, or kicking XAXIDMA_TX_TDESC, while the
engine is reset and the ring is freed underneath it.

Call netif_tx_disable() once TX NAPI is disabled and the error work has
been flushed.  It takes each queue's transmit lock, so it waits for any
transmit in progress, and nothing can wake the queue afterwards: the
error work returns early once lp->stopping is set, and axienet_tx_poll()
can no longer run.

The dmaengine path is left as it is.  There the completion callback can
wake the queue until the channel has been terminated, so it would need a
different ordering.

Tested on an AXI Ethernet MAC behind a PCIe endpoint: traffic passes,
and after each of ten down/up cycles and five module reloads, all made
with traffic running and each running this path, traffic resumes and
nothing is logged.  Suspend was not exercised.

Fixes: a3de357b087e ("net: axiemac: add PM callbacks to support suspend/resume")
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index fb26d2e39fac..6d448d0b523d 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1739,6 +1739,14 @@ static int axienet_stop(struct net_device *ndev)
 
 		napi_disable(&lp->napi_tx);
 		napi_disable(&lp->napi_rx);
+
+		/* Nothing can wake the queue now: the error work returns early
+		 * once lp->stopping is set, and TX NAPI is disabled.  Stop it and
+		 * wait out any transmit in progress before the ring goes away.
+		 * dev_close() has already done this, but axienet_suspend() calls
+		 * us directly.
+		 */
+		netif_tx_disable(ndev);
 	}
 
 	cancel_work_sync(&lp->rx_dim.work);
-- 
2.47.0

[PATCH net v2 3/4] net: axienet: quiesce the TX queue across a DMA error reset

[Sagi Maimon]

axienet_dma_err_handler() resets the DMA engine, frees every TX
descriptor's skb and mapping, and rewinds lp->tx_bd_ci and
lp->tx_bd_tail to 0.  It has two problems with the transmit path.

First, nothing excludes axienet_start_xmit() while it does so.
napi_disable() only stops axienet_tx_poll(), and the handler takes no
transmit lock.  A transmit running concurrently can publish an skb into
a descriptor that the handler then frees, and dereference it afterwards
in netdev_sent_queue(), or program a descriptor whose mapping the
handler has just released and kick XAXIDMA_TX_TDESC with a tail pointer
the handler is about to rewind.

Second, the handler never restarts the queue.  If the ring was full when
the error hit, axienet_start_xmit() had stopped the queue with
netif_stop_queue(), and that __QUEUE_STATE_DRV_XOFF survives the reset:
netdev_reset_queue() clears only __QUEUE_STATE_STACK_XOFF, and nothing
at all without CONFIG_BQL.  The wake in axienet_tx_poll() is reached
only when axienet_free_tx_chain() reclaims packets, which cannot happen
once the handler has cleared every status word, so the interface stops
transmitting until it is brought down and up again.

Quiesce the transmit path with netif_tx_disable() once TX NAPI is
disabled, so that no transmit is in progress or can start while the ring
is torn down, and wake the queue once the reset is complete.  Because
the handler now owns the queue state for its whole duration, the wake
cannot be lost to a concurrent netif_stop_queue().

Skip the wake if the interface is being stopped or the device has been
detached for suspend, or it would undo the stop that
netif_device_detach() installed; axienet_stop() and axienet_open() own
the queue state then.  A detach racing with the check is covered by
axienet_stop() quiescing the queue again before it tears anything down.

Both problems were reported by the Sashiko AI review bot.

Tested on an AXI Ethernet MAC behind a PCIe endpoint: traffic passes,
including across ten down/up cycles made with traffic running, with this
series applied.  The DMA error path itself was not exercised.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 6d448d0b523d..f16dbfc7dc93 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -2724,6 +2724,11 @@ static void axienet_dma_err_handler(struct work_struct *work)
 	napi_disable(&lp->napi_tx);
 	napi_disable(&lp->napi_rx);
 
+	/* With TX NAPI disabled nothing else can wake the queue.  Stop it and
+	 * wait out any transmit in progress, so the ring can be torn down.
+	 */
+	netif_tx_disable(ndev);
+
 	axienet_setoptions(ndev, lp->options &
 			   ~(XAE_OPTION_TXEN | XAE_OPTION_RXEN));
 
@@ -2791,6 +2796,13 @@ static void axienet_dma_err_handler(struct work_struct *work)
 	napi_enable(&lp->napi_rx);
 	napi_enable(&lp->napi_tx);
 	axienet_setoptions(ndev, lp->options);
+
+	/* Leave the queue stopped if the interface is going down or the
+	 * device was detached for suspend: axienet_stop() and axienet_open()
+	 * own the queue state then.
+	 */
+	if (!READ_ONCE(lp->stopping) && netif_device_present(ndev))
+		netif_wake_queue(ndev);
 }
 
 /**
-- 
2.47.0

[PATCH net v2 4/4] net: axienet: reprogram the link settings after a DMA error reset

[Sagi Maimon]

axienet_dma_err_handler() resets the DMA engine, which resets the AXI
Ethernet core with it.  The handler then restores RCW1, the interrupt
enable mask, the MAC address, the multicast filter and lp->options, but
not the link speed field of XAE_EMMC_OFFSET, and it writes XAE_FCC with
only XAE_FCC_FCRX_MASK, discarding whatever pause configuration had been
negotiated.

axienet_mac_link_up() is the only code that programs the link speed and
the negotiated pause bits, and phylink calls it only when the link state
changes.  Nothing about the reset is visible to phylink, so it is never
called again: the MAC keeps its reset-default link speed while software
still believes the negotiated one is in effect, and on a 10 or 100 Mb/s
link frames are clocked at the wrong rate until an unrelated link flap
happens to rerun axienet_mac_link_up().  axienet_open() avoids this only
because it runs phylink_start() after axienet_device_reset().

Tell phylink the link was lost with phylink_mac_change(), so it takes
the link down and back up and calls axienet_mac_link_up() with the
current settings, under its own locking.  Do it after the final
axienet_setoptions(), which also writes XAE_FCC, so the negotiated pause
settings are the ones left in place - the same order as in
axienet_open().  Skip it under the same conditions as the queue wake:
axienet_stop() stops phylink, and a resume restarts it.

The link going down and up is visible, which seems right: the MAC has
been reset.  A frame may still leave at the reset-default speed in the
short window before phylink runs; the reset has just dropped everything
that was in flight anyway.

This was reported by the Sashiko AI review bot.

Tested on an AXI Ethernet MAC behind a PCIe endpoint: traffic passes,
including across ten down/up cycles made with traffic running, with this
series applied.  The DMA error path itself was not exercised.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index f16dbfc7dc93..d53f7dc7bd22 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -2801,8 +2801,15 @@ static void axienet_dma_err_handler(struct work_struct *work)
 	 * device was detached for suspend: axienet_stop() and axienet_open()
 	 * own the queue state then.
 	 */
-	if (!READ_ONCE(lp->stopping) && netif_device_present(ndev))
+	if (!READ_ONCE(lp->stopping) && netif_device_present(ndev)) {
+		/* The reset also cleared the link speed and pause settings,
+		 * which only axienet_mac_link_up() programs.  Have phylink take
+		 * the link down and up again so that it is called.  This must
+		 * follow the axienet_setoptions() above, which writes XAE_FCC.
+		 */
+		phylink_mac_change(lp->phylink, false);
 		netif_wake_queue(ndev);
+	}
 }
 
 /**
-- 
2.47.0