mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt
@ 2026-09-15  6:55 Sagi Maimon
  2026-09-15 16:15 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Sagi Maimon @ 2026-09-15  6:55 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Michal Simek, linux-arm-kernel, linux-kernel,
	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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt
  2026-09-15  6:55 [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt Sagi Maimon
@ 2026-09-15 16:15 ` Andrew Lunn
  2026-09-16  3:56   ` Sagi Maimon
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2026-09-15 16:15 UTC (permalink / raw)
  To: Sagi Maimon
  Cc: Radhey Shyam Pandey, netdev, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michal Simek,
	linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt
  2026-09-15 16:15 ` Andrew Lunn
@ 2026-09-16  3:56   ` Sagi Maimon
  0 siblings, 0 replies; 3+ messages in thread
From: Sagi Maimon @ 2026-09-16  3:56 UTC (permalink / raw)
  To: andrew
  Cc: Radhey Shyam Pandey, netdev, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michal Simek,
	linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-16  3:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  6:55 [PATCH net-next] net: axienet: add a TX timeout handler to recover a lost DMA interrupt Sagi Maimon
2026-09-15 16:15 ` Andrew Lunn
2026-09-16  3:56   ` Sagi Maimon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®