git: a550e8280b98 - main - iflib: Require sustained demand for TX watchdog
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 02 Sep 2026 17:40:00 UTC
The branch main has been updated by kbowling:
URL: https://cgit.FreeBSD.org/src/commit/?id=a550e8280b9881c8c207b842f51065f155b84951
commit a550e8280b9881c8c207b842f51065f155b84951
Author: Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-27 02:00:34 +0000
Commit: Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-02 17:36:20 +0000
iflib: Require sustained demand for TX watchdog
The restored watchdog arms when the outstanding descriptor count
grows, but then continues counting based only on the queue remaining
frozen. A single growth sample can therefore leave a quiet, nearly
empty queue armed until the watchdog resets the interface. Lockless
sampling of the queue counters can also manufacture the initial growth
sample.
This matches watchdog reports from I354 queues with 979 or 980 of
1022 usable descriptors still available. Neither queue was under
transmit backpressure when the reset flapped its link.
Keep the watchdog armed only while the outstanding count continues
to grow, the software ring is stalled, or the hardware ring is at
iflib's backpressure threshold. The last condition preserves hang
detection with simple-TX, which bypasses the software ring. A busy hang
still reaches the verdict while a frozen but quiet tail disarms. Retain
the final driver completion peek so a missed completion interrupt
schedules the queue task instead of resetting it.
Validated on an 82580 with one and four queue sets in the default
mp_ring and simple-TX modes. Sustained traffic and repeated burst/idle
cycles produced no false resets. Sixteen-flow runs exercised all four
queues in both modes. Clearing TCTL.EN under load in each configuration
filled the rings; the reset counter advanced once per injection, reset
restored TCTL and the link, and traffic recovered.
Tested by: glebius
Reviewed by: iflib (gallatin), manpages (ziaee)
Fixes: 69c3e0de01c1 ("iflib: restore TX watchdog functionality")
MFC after: 6 days (after 69c3e0de01c1)
Sponsored by: BBOX.io
---
share/man/man4/iflib.4 | 14 +++++++-----
sys/net/iflib.c | 60 ++++++++++++++++++++++++++++----------------------
2 files changed, 43 insertions(+), 31 deletions(-)
diff --git a/share/man/man4/iflib.4 b/share/man/man4/iflib.4
index 4cf08602fd04..fb8bb37413c7 100644
--- a/share/man/man4/iflib.4
+++ b/share/man/man4/iflib.4
@@ -1,4 +1,4 @@
-.Dd August 8, 2026
+.Dd August 26, 2026
.Dt IFLIB 4
.Os
.Sh NAME
@@ -131,10 +131,14 @@ Setting this to a non-zero value will disable the use of this feature.
.It Va net.iflib.tx_watchdog_periods
Number of consecutive
.Va net.iflib.timer_default
-intervals for which a transmit queue must stay frozen,
-that is, hold descriptors the hardware has not reported as completed
-while none are reclaimed,
-before iflib asks the hardware whether it has completions pending.
+intervals for which a transmit queue must remain frozen under demand before
+.Nm
+asks the hardware whether it has completions pending.
+A queue is frozen when it holds descriptors the hardware has not reported as
+completed while none are reclaimed.
+Demand persists while the outstanding descriptor count grows, the software
+queue is stalled, or the hardware descriptor ring is at its backpressure
+threshold.
If it reports none, the interface is reset.
Setting this to zero disables the check.
.El
diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index 37f1218bbb17..34d53d648a7c 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -481,6 +481,9 @@ get_inuse(int size, qidx_t cidx, qidx_t pidx, uint8_t gen)
#define TXQ_AVAIL(txq) ((txq->ift_size - txq->ift_pad) -\
get_inuse(txq->ift_size, txq->ift_cidx, txq->ift_pidx, txq->ift_gen))
+#define MAX_TX_DESC(ctx) MAX((ctx)->ifc_softc_ctx.isc_tx_tso_segments_max, \
+ (ctx)->ifc_softc_ctx.isc_tx_nsegments)
+
#define IDXDIFF(head, tail, wrap) \
((head) >= (tail) ? (head) - (tail) : (wrap) - (tail) + (head))
@@ -596,19 +599,19 @@ static int iflib_timer_default = 1000;
SYSCTL_INT(_net_iflib, OID_AUTO, timer_default, CTLFLAG_RW,
&iflib_timer_default, 0, "number of ticks between iflib_timer calls");
/*
- * Consecutive timer periods a TX queue must stay frozen - see
- * iflib_timer(), which defines that state - before the hardware is
- * asked whether it has completions pending. Four periods is roughly
- * two seconds with the default timer interval: a healthy queue on
- * hardware that coalesces completion reports (e.g. 8254x,
- * TXDCTL.WTHRESH) stays frozen for at most two (measured on 82541PI),
- * a wedged one until it is reset.
+ * Consecutive timer periods a TX queue must stay frozen while demand
+ * persists - see iflib_timer(), which defines those states - before the
+ * hardware is asked whether it has completions pending. Four periods is
+ * roughly two seconds with the default timer interval: a healthy queue on
+ * hardware that coalesces completion reports (e.g. 8254x, TXDCTL.WTHRESH)
+ * stays frozen for at most two (measured on 82541PI), a wedged one until it
+ * is reset.
*/
static int iflib_tx_watchdog_periods = 4;
SYSCTL_INT(_net_iflib, OID_AUTO, tx_watchdog_periods, CTLFLAG_RWTUN,
&iflib_tx_watchdog_periods, 0,
- "consecutive frozen timer periods before a TX queue is checked for "
- "a hang (0 disables the check)");
+ "consecutive frozen timer periods under demand before a TX queue is "
+ "checked for a hang (0 disables the check)");
#if IFLIB_DEBUG_COUNTERS
@@ -2447,8 +2450,8 @@ iflib_timer(void *arg)
* delays the verdict by one timer period.
*/
if (this_tick - txq->ift_last_timer_tick >= iflib_timer_default) {
- qidx_t outstanding;
- bool frozen;
+ qidx_t in_use, outstanding;
+ bool demand, frozen;
txq->ift_last_timer_tick = this_tick;
IFDI_TIMER(ctx, txq->ift_id);
@@ -2462,7 +2465,8 @@ iflib_timer(void *arg)
* reported and must not count (ift_rs_pending
* over-counts it by one per packet).
*/
- outstanding = txq->ift_in_use -
+ in_use = txq->ift_in_use;
+ outstanding = in_use -
(qidx_t)(txq->ift_processed - txq->ift_cleaned);
/*
@@ -2472,12 +2476,17 @@ iflib_timer(void *arg)
* up, with no pause frames and no pending doorbell
* (the laggard check below rings it).
*
- * Being frozen is not a fault - the hardware may
- * defer marking descriptors as completed
- * indefinitely, and 8254x hardware does so for a
- * quiet queue - therefore the check arms only when a
- * frozen queue also takes on new work, and acts only
- * once it has stayed frozen for
+ * Being frozen is not a fault - the hardware may defer
+ * marking descriptors as completed indefinitely, and
+ * 8254x hardware does so for a quiet queue. Continue
+ * arming only while demand persists: the outstanding
+ * count grows, the software ring is stalled, or the
+ * hardware ring has reached iflib's backpressure
+ * threshold. The last condition covers simple-TX, which
+ * does not use the software ring. This also prevents one
+ * mixed lockless counter sample from arming a quiet queue
+ * until the verdict. Act only once it has stayed frozen
+ * under demand for
* net.iflib.tx_watchdog_periods consecutive periods.
*/
frozen = outstanding > txq->ift_rs_pending &&
@@ -2485,10 +2494,12 @@ iflib_timer(void *arg)
txq->ift_db_pending == 0 &&
sctx->isc_pause_frames == 0 &&
ctx->ifc_link_state == LINK_STATE_UP;
- if (!frozen)
+ demand = outstanding > txq->ift_outstanding_prev ||
+ ifmp_ring_is_stalled(txq->ift_br) ||
+ in_use + MAX_TX_DESC(ctx) >= txq->ift_size - txq->ift_pad;
+ if (!frozen || !demand)
txq->ift_wdog_armed = 0;
- else if (txq->ift_wdog_armed > 0 ||
- outstanding > txq->ift_outstanding_prev) {
+ else {
if (txq->ift_wdog_armed < UINT16_MAX)
txq->ift_wdog_armed++;
}
@@ -2497,8 +2508,8 @@ iflib_timer(void *arg)
* Frozen long enough: ask the hardware. Completions
* ready but unharvested for this long mean the
* completion interrupt went missing - kick the
- * queue's task. Nothing ready, although the queue
- * kept taking on work, means it is hung.
+ * queue's task. Nothing ready while demand persisted
+ * means it is hung.
*/
if (iflib_tx_watchdog_periods > 0 &&
txq->ift_wdog_armed >= iflib_tx_watchdog_periods) {
@@ -3192,9 +3203,6 @@ txq_max_rs_deferred(iflib_txq_t txq)
#define QIDX(ctx, m) ((((m)->m_pkthdr.flowid & ctx->ifc_softc_ctx.isc_rss_table_mask) % NTXQSETS(ctx)) + FIRST_QSET(ctx))
#define DESC_RECLAIMABLE(q) ((int)((q)->ift_processed - (q)->ift_cleaned - (q)->ift_ctx->ifc_softc_ctx.isc_tx_nsegments))
-#define MAX_TX_DESC(ctx) MAX((ctx)->ifc_softc_ctx.isc_tx_tso_segments_max, \
- (ctx)->ifc_softc_ctx.isc_tx_nsegments)
-
static inline bool
iflib_txd_db_check(iflib_txq_t txq, int ring)
{