[PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[Sagi Maimon]

axienet_dma_err_handler() is the driver's only reset path, and the only
two places that schedule it are axienet_tx_irq() and axienet_rx_irq(),
both on the XAXIDMA_IRQ_ERROR_MASK branch.  Every route into recovery
therefore depends on a DMA interrupt being delivered.

If a completion interrupt is lost the queue stops making progress with
descriptors unreclaimed, and nothing ever schedules the reset: the error
branch cannot run because no interrupt arrives, and NAPI is not polled
because it is only scheduled from those same handlers.  The interface
stays wedged until the driver is unloaded.

Add an .ndo_tx_timeout handler so the netdev watchdog provides a route
into the existing reset path that does not depend on the interrupt that
was lost.  The handler only queues the work; axienet_dma_err_handler()
then performs the reset in process context, as it already does for the
error-interrupt case.

.ndo_tx_timeout is added to axienet_netdev_ops alone.  On the dmaengine
path lp->dma_err_task is never initialised - INIT_WORK() for it lives in
axienet_init_legacy_dma() - so scheduling it there would be a bug, and
watchdog_timeo is set in the same branch for that reason.

Tested on a Xilinx AXI Ethernet MAC by clearing PCI_MSIX_FLAGS_ENABLE
underneath a running interface to drop completion interrupts: without
this patch the TX queue stalls permanently, with it the watchdog fires
and the interface resumes passing traffic.

Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 782f903d318f..b5927e979c34 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -55,6 +55,10 @@
 #define DMA_NUM_APP_WORDS		5
 #define LEN_APP				4
 #define RX_BUF_NUM_DEFAULT		128
+/* Well above any legitimate TX completion delay, including the worst case
+ * allowed by the DMA interrupt coalescing settings.
+ */
+#define AXIENET_TX_TIMEOUT		(5 * HZ)
 
 /* Must be shorter than length of ethtool_drvinfo.driver field to fit */
 #define DRIVER_NAME		"xaxienet"
@@ -1884,6 +1888,30 @@ axienet_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	} while (read_seqcount_retry(&lp->hw_stats_seqcount, start));
 }
 
+/**
+ * axienet_tx_timeout - Driver TX timeout callback
+ * @ndev:	Pointer to net_device structure
+ * @txqueue:	Index of the transmit queue that stalled
+ *
+ * Called by the netdev watchdog when a transmit queue has made no progress for
+ * @ndev->watchdog_timeo.  axienet_dma_err_handler() is the driver's only reset
+ * path, and it is otherwise scheduled solely from axienet_tx_irq() and
+ * axienet_rx_irq() - so a completion interrupt that is never delivered leaves
+ * the queue stopped with descriptors unreclaimed and no way back short of
+ * unloading the driver.  Schedule the reset from here as well, so a lost
+ * interrupt is recoverable.
+ *
+ * This runs from a timer, so it only queues the work; the reset itself happens
+ * in process context in axienet_dma_err_handler().
+ */
+static void axienet_tx_timeout(struct net_device *ndev, unsigned int txqueue)
+{
+	struct axienet_local *lp = netdev_priv(ndev);
+
+	netdev_err(ndev, "TX queue %u stalled, resetting DMA\n", txqueue);
+	schedule_work(&lp->dma_err_task);
+}
+
 static const struct net_device_ops axienet_netdev_ops = {
 	.ndo_open = axienet_open,
 	.ndo_stop = axienet_stop,
@@ -1894,6 +1922,7 @@ static const struct net_device_ops axienet_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr,
 	.ndo_eth_ioctl = axienet_ioctl,
 	.ndo_set_rx_mode = axienet_set_multicast_list,
+	.ndo_tx_timeout = axienet_tx_timeout,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = axienet_poll_controller,
 #endif
@@ -3054,6 +3083,10 @@ static int axienet_probe(struct platform_device *pdev)
 	} else {
 		ndev->netdev_ops = &axienet_netdev_ops;
 		ndev->ethtool_ops = &axienet_ethtool_ops;
+		/* netdev_watchdog_up() only arms the TX watchdog when
+		 * .ndo_tx_timeout is set, which is the legacy DMA path alone.
+		 */
+		ndev->watchdog_timeo = AXIENET_TX_TIMEOUT;
 	}
 	/* Check for Ethernet core IRQ (optional) */
 	if (lp->eth_irq < 0)
-- 
2.47.0

Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[Andrew Lunn]

On Tue, Sep 15, 2026 at 09:55:34AM +0300, Sagi Maimon wrote:
> axienet_dma_err_handler() is the driver's only reset path, and the only
> two places that schedule it are axienet_tx_irq() and axienet_rx_irq(),
> both on the XAXIDMA_IRQ_ERROR_MASK branch.  Every route into recovery
> therefore depends on a DMA interrupt being delivered.
> 
> If a completion interrupt is lost the queue stops making progress with
> descriptors unreclaimed, and nothing ever schedules the reset: the error
> branch cannot run because no interrupt arrives, and NAPI is not polled
> because it is only scheduled from those same handlers.  The interface
> stays wedged until the driver is unloaded.
> 
> Add an .ndo_tx_timeout handler so the netdev watchdog provides a route
> into the existing reset path that does not depend on the interrupt that
> was lost.  The handler only queues the work; axienet_dma_err_handler()
> then performs the reset in process context, as it already does for the
> error-interrupt case.
> 
> .ndo_tx_timeout is added to axienet_netdev_ops alone.  On the dmaengine
> path lp->dma_err_task is never initialised - INIT_WORK() for it lives in
> axienet_init_legacy_dma() - so scheduling it there would be a bug, and
> watchdog_timeo is set in the same branch for that reason.
> 
> Tested on a Xilinx AXI Ethernet MAC by clearing PCI_MSIX_FLAGS_ENABLE
> underneath a running interface to drop completion interrupts: without
> this patch the TX queue stalls permanently, with it the watchdog fires
> and the interface resumes passing traffic.

Have you seen the hardware wedge without faking it by clearing
PCI_MSIX_FLAGS_ENABLE?

	Andrew

Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[Sagi Maimon]

On Tue, Sep 15, 2026 at 7:15 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Sep 15, 2026 at 09:55:34AM +0300, Sagi Maimon wrote:
> > axienet_dma_err_handler() is the driver's only reset path, and the only
> > two places that schedule it are axienet_tx_irq() and axienet_rx_irq(),
> > both on the XAXIDMA_IRQ_ERROR_MASK branch.  Every route into recovery
> > therefore depends on a DMA interrupt being delivered.
> >
> > If a completion interrupt is lost the queue stops making progress with
> > descriptors unreclaimed, and nothing ever schedules the reset: the error
> > branch cannot run because no interrupt arrives, and NAPI is not polled
> > because it is only scheduled from those same handlers.  The interface
> > stays wedged until the driver is unloaded.
> >
> > Add an .ndo_tx_timeout handler so the netdev watchdog provides a route
> > into the existing reset path that does not depend on the interrupt that
> > was lost.  The handler only queues the work; axienet_dma_err_handler()
> > then performs the reset in process context, as it already does for the
> > error-interrupt case.
> >
> > .ndo_tx_timeout is added to axienet_netdev_ops alone.  On the dmaengine
> > path lp->dma_err_task is never initialised - INIT_WORK() for it lives in
> > axienet_init_legacy_dma() - so scheduling it there would be a bug, and
> > watchdog_timeo is set in the same branch for that reason.
> >
> > Tested on a Xilinx AXI Ethernet MAC by clearing PCI_MSIX_FLAGS_ENABLE
> > underneath a running interface to drop completion interrupts: without
> > this patch the TX queue stalls permanently, with it the watchdog fires
> > and the interface resumes passing traffic.
>
> Have you seen the hardware wedge without faking it by clearing
> PCI_MSIX_FLAGS_ENABLE?
>
>         Andrew

No, I haven't seen a naturally occurring lost completion interrupt.
The scenario was reproduced via fault injection by disabling MSI-X delivery.

I do see occasional hangs on my development platform,
but those are caused by an unrelated FPGA bug that clears BAR0 and
PCI_COMMAND at runtime.
In that case the watchdog fires and recovery is attempted, but reset
cannot succeed because the MMIO window is no longer accessible.
I don't consider that relevant evidence for this patch.

My rationale is primarily structural.
axienet_dma_err_handler() is the driver's only reset path,and it is
currently reachable only from axienet_tx_irq() and axienet_rx_irq()
on the XAXIDMA_IRQ_ERROR_MASK path.
If a completion interrupt is never delivered, NAPI is never scheduled,
descriptors are never reclaimed, and there is no recovery path back
into the driver.

The watchdog provides an independent route into the existing reset
logic that does not itself depend on the interrupt that went missing.

I'm happy to reword the commit message to make it clear that the
validation was performed via fault injection rather than from an
observed field failure.
If you feel that scenario alone does not justify the additional code,
I can drop the patch.

Thanks,
Sagi

Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

- [High] The new .ndo_tx_timeout handler (axienet_tx_timeout(),
  drivers/net/ethernet/xilinx/xilinx_axienet_main.c:1907-1913) only…

Pre-existing issues:
- [High] axienet_dma_err_handler()
  (drivers/net/ethernet/xilinx/xilinx_axienet_main.c:2727-2810) tears…

--

net: axienet: add a TX timeout handler to recover a lost DMA interrupt

This adds an .ndo_tx_timeout handler to axienet_netdev_ops that queues
lp->dma_err_task, so the netdev watchdog can reach the existing DMA reset
path without depending on a DMA interrupt.  watchdog_timeo is set to 5 * HZ
in the legacy DMA branch of axienet_probe().

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 782f903d318f3..b5927e979c341 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c

[ ... ]

> @@ -1884,6 +1888,30 @@ axienet_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>  	} while (read_seqcount_retry(&lp->hw_stats_seqcount, start));
>  }
>  
> +/**
> + * axienet_tx_timeout - Driver TX timeout callback
> + * @ndev:	Pointer to net_device structure
> + * @txqueue:	Index of the transmit queue that stalled
> + *
> + * Called by the netdev watchdog when a transmit queue has made no progress for
> + * @ndev->watchdog_timeo.  axienet_dma_err_handler() is the driver's only reset
> + * path, and it is otherwise scheduled solely from axienet_tx_irq() and
> + * axienet_rx_irq() - so a completion interrupt that is never delivered leaves
> + * the queue stopped with descriptors unreclaimed and no way back short of
> + * unloading the driver.  Schedule the reset from here as well, so a lost
> + * interrupt is recoverable.
> + *
> + * This runs from a timer, so it only queues the work; the reset itself happens
> + * in process context in axienet_dma_err_handler().
> + */
> +static void axienet_tx_timeout(struct net_device *ndev, unsigned int txqueue)
> +{
> +	struct axienet_local *lp = netdev_priv(ndev);
> +
> +	netdev_err(ndev, "TX queue %u stalled, resetting DMA\n", txqueue);
> +	schedule_work(&lp->dma_err_task);
> +}

[Severity: High]
Does the queue actually get restarted after this reset?

dev_watchdog() only calls ndo_tx_timeout for a queue that is already
stopped:

net/sched/sch_generic.c:dev_watchdog() {
	...
			if (!netif_xmit_stopped(txq))
				continue;
	...
			dev->netdev_ops->ndo_tx_timeout(dev, i);
}

In the scenario the changelog describes (lost TX completion, descriptors
unreclaimed) the stop bit is __QUEUE_STATE_DRV_XOFF, set here:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit() {
	...
	if (axienet_check_tx_bd_space(lp, MAX_SKB_FRAGS + 1)) {
		netif_stop_queue(ndev);
	...
}

axienet_dma_err_handler() then resets DMA, zeroes every TX BD, sets
lp->tx_bd_ci = lp->tx_bd_tail = 0, re-enables NAPI and returns.  The only
queue-state clearing it does is netdev_reset_queue(), which touches
STACK_XOFF only, and nothing at all without CONFIG_BQL:

include/linux/netdevice.h:netdev_tx_reset_queue() {
#ifdef CONFIG_BQL
	clear_bit(__QUEUE_STATE_STACK_XOFF, &q->state);
	dql_reset(&q->dql);
#endif
}

There is no netif_wake_queue()/netif_tx_wake_all_queues() and no
netif_trans_update() anywhere in axienet_dma_err_handler().  The remaining
wake site in axienet_tx_poll() is gated on packets:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_tx_poll() {
	packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false,
					&size, budget);

	if (packets) {
	...
		if (!axienet_check_tx_bd_space(lp, MAX_SKB_FRAGS + 1))
			netif_wake_queue(ndev);
	}
}

Can axienet_free_tx_chain() ever return non-zero after the handler cleared
cur_p->status for every descriptor?  And the wake in axienet_start_xmit()
cannot run while the queue is stopped.

If DRV_XOFF survives the reset, and txq->trans_start is never refreshed,
doesn't dev_watchdog()'s time_after(jiffies, trans_start + watchdog_timeo)
stay true, so the timeout path re-fires every 5 seconds, re-running
napi_disable() / axienet_dma_stop() / __axienet_device_reset() /
axienet_dma_start() and printing netdev_crit + netdev_err indefinitely
while the interface still cannot transmit?

The driver's other reset path does refresh the timestamp:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_device_reset() {
	...
	netif_trans_update(ndev);
}

Would adding netif_trans_update() plus a wake of the TX queue at the tail
of axienet_dma_err_handler() be needed for the recovery claimed in the
changelog ("the watchdog fires and the interface resumes passing traffic")
to hold for the DRV_XOFF case rather than only for a BQL-only stop?

[Severity: High]
This is a pre-existing issue and not introduced by this patch, but the new
trigger makes it much easier to reach: axienet_dma_err_handler() tears down
the TX ring with no exclusion against axienet_start_xmit().

The worker calls napi_disable() (which only excludes axienet_tx_poll()),
axienet_dma_stop(), then re-enables transmission for a BQL-only stop before
it starts freeing descriptors:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_dma_err_handler() {
	axienet_dma_stop(lp);
	netdev_reset_queue(ndev);	/* clears __QUEUE_STATE_STACK_XOFF */

	for (i = 0; i < lp->tx_bd_num; i++) {
		cur_p = &lp->tx_bd_v[i];
		if (cur_p->cntrl) {
			dma_addr_t addr = desc_get_phys_addr(lp, cur_p);

			dma_unmap_single(lp->dev, addr, ..., DMA_TO_DEVICE);
		}
		if (cur_p->skb)
			dev_kfree_skb_irq(cur_p->skb);
	...
	lp->tx_bd_ci = 0;
	lp->tx_bd_tail = 0;

	axienet_dma_start(lp);
}

It never takes the per-queue _xmit_lock and never calls
netif_tx_disable()/netif_device_detach(), and the core unfreezes the queues
as soon as ndo_tx_timeout() returns, long before the work runs:

net/sched/sch_generic.c:dev_watchdog() {
			netif_freeze_queues(dev);
			dev->netdev_ops->ndo_tx_timeout(dev, i);
			netif_unfreeze_queues(dev);
}

So while the teardown loop is running, can a transmit on another CPU enter

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit() {
	...
	WRITE_ONCE(lp->tx_bd_tail, new_tail_ptr);
	netdev_sent_queue(ndev, skb->len);

	/* Start the transfer */
	axienet_dma_out_addr(lp, XAXIDMA_TX_TDESC_OFFSET, tail_p);
}

and write cur_p->cntrl/phys/skb into a descriptor the worker is about to
unmap and clear?  The outcomes look like an skb freed by the worker and
then re-referenced, a DMA mapping released while a fresh buffer is being
programmed into the same descriptor, and lp->tx_bd_tail/tx_bd_ci rewound to
0 out of step with the hardware tail pointer written by axienet_start_xmit().

Since this new .ndo_tx_timeout makes the reset a routine event (and, per the
comment above, potentially every 5 seconds), should the reset path be made
to exclude the xmit path, for example with netif_tx_disable() around the
descriptor teardown?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915065534.102249-1-maimon.sagi%40gmail.com

Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt

[Sagi Maimon]

On Thu, Sep 17, 2026 at 9:58 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0
>
> - [High] The new .ndo_tx_timeout handler (axienet_tx_timeout(),
>   drivers/net/ethernet/xilinx/xilinx_axienet_main.c:1907-1913) only…
>
> Pre-existing issues:
> - [High] axienet_dma_err_handler()
>   (drivers/net/ethernet/xilinx/xilinx_axienet_main.c:2727-2810) tears…
>
> --
>
> net: axienet: add a TX timeout handler to recover a lost DMA interrupt
>
> This adds an .ndo_tx_timeout handler to axienet_netdev_ops that queues
> lp->dma_err_task, so the netdev watchdog can reach the existing DMA reset
> path without depending on a DMA interrupt.  watchdog_timeo is set to 5 * HZ
> in the legacy DMA branch of axienet_probe().
>
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > index 782f903d318f3..b5927e979c341 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>
> [ ... ]
>
> > @@ -1884,6 +1888,30 @@ axienet_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
> >       } while (read_seqcount_retry(&lp->hw_stats_seqcount, start));
> >  }
> >
> > +/**
> > + * axienet_tx_timeout - Driver TX timeout callback
> > + * @ndev:    Pointer to net_device structure
> > + * @txqueue: Index of the transmit queue that stalled
> > + *
> > + * Called by the netdev watchdog when a transmit queue has made no progress for
> > + * @ndev->watchdog_timeo.  axienet_dma_err_handler() is the driver's only reset
> > + * path, and it is otherwise scheduled solely from axienet_tx_irq() and
> > + * axienet_rx_irq() - so a completion interrupt that is never delivered leaves
> > + * the queue stopped with descriptors unreclaimed and no way back short of
> > + * unloading the driver.  Schedule the reset from here as well, so a lost
> > + * interrupt is recoverable.
> > + *
> > + * This runs from a timer, so it only queues the work; the reset itself happens
> > + * in process context in axienet_dma_err_handler().
> > + */
> > +static void axienet_tx_timeout(struct net_device *ndev, unsigned int txqueue)
> > +{
> > +     struct axienet_local *lp = netdev_priv(ndev);
> > +
> > +     netdev_err(ndev, "TX queue %u stalled, resetting DMA\n", txqueue);
> > +     schedule_work(&lp->dma_err_task);
> > +}
>
> [Severity: High]
> Does the queue actually get restarted after this reset?
>

You are right that it does not, for the DRV_XOFF case.  Thank you - the
analysis is correct and I had not covered that path.

For what it is worth, the hardware run behind the changelog did recover,
and the logs say why it did not expose this.  Sampling once a second
across the wedge (MSI-X disabled, TX completions stopped):

  T+6   TXirq=24542  tx_pkts=1767  tx_timeout=0  backlog=131264b/92p
  T+7   TXirq=24542  tx_pkts=1767  tx_timeout=1  backlog=131306b/93p
  T+8   TXirq=24542  tx_pkts=1767  tx_timeout=1  backlog=0b/0p
  ...
  T+22  TXirq=24542  tx_pkts=1767  tx_timeout=1  backlog=0b/0p

The qdisc backlog drains to zero on the sample after the reset while
tx_packets stays frozen, so the queue was woken, and tx_timeout stays at
1 for the remaining 15 seconds, so the watchdog did not re-fire.  With
CONFIG_BQL=y the dql limit stopped the queue via STACK_XOFF before the
128-descriptor ring filled, and netdev_reset_queue() clears exactly that
bit.  "TX ring unexpectedly full" never appeared in either run, which is
consistent with DRV_XOFF never being set.

So the recovery was only ever demonstrated for the BQL stop, exactly as
you suggest, and the DRV_XOFF path is untested.

That gap is pre-existing and independent of this patch -
axienet_dma_err_handler() has never woken the queue, and the
error-interrupt path into it has the same problem - so I have sent it
separately rather than folding it in:

  net: axienet: restart the TX queue after a DMA error reset

That one is a missing call and I am comfortable sending it reasoned from
the code, but this patch is a different matter: its changelog claims the
interface resumes passing traffic, and that is a behavioural claim I
should demonstrate rather than assert.  So I will repost the TX timeout
patch once I can test the DRV_XOFF case on hardware, about a month from
now.

> dev_watchdog() only calls ndo_tx_timeout for a queue that is already
> stopped:
>
> net/sched/sch_generic.c:dev_watchdog() {
>         ...
>                         if (!netif_xmit_stopped(txq))
>                                 continue;
>         ...
>                         dev->netdev_ops->ndo_tx_timeout(dev, i);
> }
>
> In the scenario the changelog describes (lost TX completion, descriptors
> unreclaimed) the stop bit is __QUEUE_STATE_DRV_XOFF, set here:
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit() {
>         ...
>         if (axienet_check_tx_bd_space(lp, MAX_SKB_FRAGS + 1)) {
>                 netif_stop_queue(ndev);
>         ...
> }
>
> axienet_dma_err_handler() then resets DMA, zeroes every TX BD, sets
> lp->tx_bd_ci = lp->tx_bd_tail = 0, re-enables NAPI and returns.  The only
> queue-state clearing it does is netdev_reset_queue(), which touches
> STACK_XOFF only, and nothing at all without CONFIG_BQL:
>
> include/linux/netdevice.h:netdev_tx_reset_queue() {
> #ifdef CONFIG_BQL
>         clear_bit(__QUEUE_STATE_STACK_XOFF, &q->state);
>         dql_reset(&q->dql);
> #endif
> }
>
> There is no netif_wake_queue()/netif_tx_wake_all_queues() and no
> netif_trans_update() anywhere in axienet_dma_err_handler().  The remaining
> wake site in axienet_tx_poll() is gated on packets:
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_tx_poll() {
>         packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false,
>                                         &size, budget);
>
>         if (packets) {
>         ...
>                 if (!axienet_check_tx_bd_space(lp, MAX_SKB_FRAGS + 1))
>                         netif_wake_queue(ndev);
>         }
> }
>
> Can axienet_free_tx_chain() ever return non-zero after the handler cleared
> cur_p->status for every descriptor?  And the wake in axienet_start_xmit()
> cannot run while the queue is stopped.
>
> If DRV_XOFF survives the reset, and txq->trans_start is never refreshed,
> doesn't dev_watchdog()'s time_after(jiffies, trans_start + watchdog_timeo)
> stay true, so the timeout path re-fires every 5 seconds, re-running
> napi_disable() / axienet_dma_stop() / __axienet_device_reset() /
> axienet_dma_start() and printing netdev_crit + netdev_err indefinitely
> while the interface still cannot transmit?
>
> The driver's other reset path does refresh the timestamp:
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_device_reset() {
>         ...
>         netif_trans_update(ndev);
> }
>
> Would adding netif_trans_update() plus a wake of the TX queue at the tail
> of axienet_dma_err_handler() be needed for the recovery claimed in the
> changelog ("the watchdog fires and the interface resumes passing traffic")
> to hold for the DRV_XOFF case rather than only for a BQL-only stop?
>
> [Severity: High]
> This is a pre-existing issue and not introduced by this patch, but the new
> trigger makes it much easier to reach: axienet_dma_err_handler() tears down
> the TX ring with no exclusion against axienet_start_xmit().
>
> The worker calls napi_disable() (which only excludes axienet_tx_poll()),
> axienet_dma_stop(), then re-enables transmission for a BQL-only stop before
> it starts freeing descriptors:
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_dma_err_handler() {
>         axienet_dma_stop(lp);
>         netdev_reset_queue(ndev);       /* clears __QUEUE_STATE_STACK_XOFF */
>
>         for (i = 0; i < lp->tx_bd_num; i++) {
>                 cur_p = &lp->tx_bd_v[i];
>                 if (cur_p->cntrl) {
>                         dma_addr_t addr = desc_get_phys_addr(lp, cur_p);
>
>                         dma_unmap_single(lp->dev, addr, ..., DMA_TO_DEVICE);
>                 }
>                 if (cur_p->skb)
>                         dev_kfree_skb_irq(cur_p->skb);
>         ...
>         lp->tx_bd_ci = 0;
>         lp->tx_bd_tail = 0;
>
>         axienet_dma_start(lp);
> }
>
> It never takes the per-queue _xmit_lock and never calls
> netif_tx_disable()/netif_device_detach(), and the core unfreezes the queues
> as soon as ndo_tx_timeout() returns, long before the work runs:
>
> net/sched/sch_generic.c:dev_watchdog() {
>                         netif_freeze_queues(dev);
>                         dev->netdev_ops->ndo_tx_timeout(dev, i);
>                         netif_unfreeze_queues(dev);
> }
>
> So while the teardown loop is running, can a transmit on another CPU enter
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit() {
>         ...
>         WRITE_ONCE(lp->tx_bd_tail, new_tail_ptr);
>         netdev_sent_queue(ndev, skb->len);
>
>         /* Start the transfer */
>         axienet_dma_out_addr(lp, XAXIDMA_TX_TDESC_OFFSET, tail_p);
> }
>
> and write cur_p->cntrl/phys/skb into a descriptor the worker is about to
> unmap and clear?  The outcomes look like an skb freed by the worker and
> then re-referenced, a DMA mapping released while a fresh buffer is being
> programmed into the same descriptor, and lp->tx_bd_tail/tx_bd_ci rewound to
> 0 out of step with the hardware tail pointer written by axienet_start_xmit().
>
> Since this new .ndo_tx_timeout makes the reset a routine event (and, per the
> comment above, potentially every 5 seconds), should the reset path be made
> to exclude the xmit path, for example with netif_tx_disable() around the
> descriptor teardown?

Agreed, and I think that is a real bug today rather than only with this
patch.  It is a bigger change to a path I cannot currently exercise, so
I would prefer to send it as its own patch once I have the board back.

>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915065534.102249-1-maimon.sagi%40gmail.com