Skip to content

Commit 8adc2e6

Browse files
committed
net: macb: add TX stall watchdog as defence-in-depth safety net
Patches 1/3 and 2/3 address two candidate races that could lead to a TCOMP completion being missed on PCIe-attached macb instances. This patch adds a defence-in-depth safety net, in case a further race remains that we have not identified. The watchdog is a per-queue delayed_work that runs once per second. It snapshots queue->tx_tail; if the ring is non-empty (queue->tx_head != queue->tx_tail) and tx_tail has not advanced since the previous tick, it calls macb_tx_restart(). No new recovery logic is introduced. macb_tx_restart() already exists in this file, is correctly locked (tx_ptr_lock, bp->lock), and verifies that the hardware's TBQP is behind the driver's head index before re-asserting TSTART. On a healthy ring it is a no-op at the hardware level; the watchdog only supplies the missing trigger. On a healthy queue the per-tick cost is one spin_lock_irqsave() / spin_unlock_irqrestore() and one branch. The delayed_work is only scheduled between macb_open() and macb_close(), and is cancelled synchronously on close. Context for submission: on our 24-node Raspberry Pi 5 fleet, before this series, an out-of-band user-space watchdog (monitoring tx_packets from /sys/class/net/.../statistics and toggling the link down/up when it froze) was required to keep nodes usable. We include this kernel-side watchdog as a cleaner in-kernel equivalent for any residual stall that patches 1 and 2 do not cover. We are willing to drop this patch if the view is that 1 and 2 should stand alone. Link: cilium/cilium#43198 Link: https://bugs.launchpad.net/ubuntu/+source/linux-raspi/+bug/2133877 Signed-off-by: Lukasz Raczylo <lukasz@raczylo.com>
1 parent 1c36ab5 commit 8adc2e6

2 files changed

Lines changed: 64 additions & 0 deletions

File tree

drivers/net/ethernet/cadence/macb.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,6 +1294,11 @@ struct macb_queue {
12941294
struct work_struct tx_error_task;
12951295
bool txubr_pending;
12961296
bool tx_pending;
1297+
1298+
/* TX stall watchdog -- see macb_tx_stall_watchdog() in macb_main.c */
1299+
struct delayed_work tx_stall_watchdog_work;
1300+
unsigned int tx_stall_last_tail;
1301+
12971302
struct napi_struct napi_tx;
12981303

12991304
dma_addr_t rx_ring_dma;

drivers/net/ethernet/cadence/macb_main.c

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2030,6 +2030,59 @@ static int macb_tx_poll(struct napi_struct *napi, int budget)
20302030
return work_done;
20312031
}
20322032

2033+
#define MACB_TX_STALL_INTERVAL_MS 1000
2034+
2035+
/*
2036+
* TX stall watchdog.
2037+
*
2038+
* Defence-in-depth against lost TCOMP interrupts. macb already has a
2039+
* recovery chain (tx_pending -> txubr_pending -> macb_tx_restart())
2040+
* that fires on TCOMP; if TCOMP itself is lost the TX ring stalls
2041+
* silently until something else kicks TSTART. This watchdog runs
2042+
* once per second per queue, snapshots tx_tail, and calls
2043+
* macb_tx_restart() if the ring is non-empty and tx_tail has not
2044+
* advanced since the previous tick.
2045+
*
2046+
* macb_tx_restart() already checks the hardware's TBQP against the
2047+
* driver's head index before re-asserting TSTART, so on a healthy
2048+
* ring this is a no-op at the hardware level. The watchdog only
2049+
* adds the missing trigger.
2050+
*/
2051+
static void macb_tx_stall_watchdog(struct work_struct *work)
2052+
{
2053+
struct macb_queue *queue = container_of(to_delayed_work(work),
2054+
struct macb_queue,
2055+
tx_stall_watchdog_work);
2056+
struct macb *bp = queue->bp;
2057+
unsigned int cur_tail, cur_head;
2058+
bool stalled = false;
2059+
unsigned long flags;
2060+
2061+
if (!netif_running(bp->dev))
2062+
return;
2063+
2064+
spin_lock_irqsave(&queue->tx_ptr_lock, flags);
2065+
cur_tail = queue->tx_tail;
2066+
cur_head = queue->tx_head;
2067+
if (cur_head != cur_tail &&
2068+
cur_tail == queue->tx_stall_last_tail)
2069+
stalled = true;
2070+
else
2071+
queue->tx_stall_last_tail = cur_tail;
2072+
spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
2073+
2074+
if (stalled) {
2075+
netdev_warn_once(bp->dev,
2076+
"TX stall detected on queue %u (tail=%u head=%u); re-kicking TSTART\n",
2077+
(unsigned int)(queue - bp->queues),
2078+
cur_tail, cur_head);
2079+
macb_tx_restart(queue);
2080+
}
2081+
2082+
schedule_delayed_work(&queue->tx_stall_watchdog_work,
2083+
msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS));
2084+
}
2085+
20332086
static void macb_hresp_error_task(struct work_struct *work)
20342087
{
20352088
struct macb *bp = from_work(bp, work, hresp_err_bh_work);
@@ -3297,6 +3350,9 @@ static int macb_open(struct net_device *dev)
32973350
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
32983351
napi_enable(&queue->napi_rx);
32993352
napi_enable(&queue->napi_tx);
3353+
queue->tx_stall_last_tail = queue->tx_tail;
3354+
schedule_delayed_work(&queue->tx_stall_watchdog_work,
3355+
msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS));
33003356
}
33013357

33023358
macb_init_hw(bp);
@@ -3343,6 +3399,7 @@ static int macb_close(struct net_device *dev)
33433399
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
33443400
napi_disable(&queue->napi_rx);
33453401
napi_disable(&queue->napi_tx);
3402+
cancel_delayed_work_sync(&queue->tx_stall_watchdog_work);
33463403
netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
33473404
}
33483405

@@ -4941,6 +4998,8 @@ static int macb_init(struct platform_device *pdev)
49414998
}
49424999

49435000
INIT_WORK(&queue->tx_error_task, macb_tx_error_task);
5001+
INIT_DELAYED_WORK(&queue->tx_stall_watchdog_work,
5002+
macb_tx_stall_watchdog);
49445003
q++;
49455004
}
49465005

0 commit comments

Comments
 (0)