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.
Fixes: 69c3e0de01c1 ("iflib: restore TX watchdog functionality")
MFC after: (directly after 69c3e0de01c1)Details
Details
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
My build is still running on the old hardware where I can test this.
Quick check, if I get it right:
- this solves the issue where demand is false on the next period.
- this does not solve an issue where ~1 packet per period is send while more than one is in the queue (no idea if this is behavior we can see in the real world): growth every period → demand true every period → armed reaches 4 → still resets.
| sys/net/iflib.c | ||
|---|---|---|
| 2527 | in_use + MAX_TX_DESC >= size - pad ⇔ avail <= MAX_TX_DESC | |
Comment Actions
I don't see any gap here. 1 pps is sustained demand, if an RS descriptor makes no progress for four periods the watchdog should investigate. The existing peek mentioned in the commit message provides progress if the hardware completed work but lost an interrupt.
| sys/net/iflib.c | ||
|---|---|---|
| 2527 | TXQ_AVAIL() re-reads concurrent values without a lock, so we are deliberately working against a single in_use snapshot. | |
| share/man/man4/iflib.4 | ||
|---|---|---|
| 135 | ||
Comment Actions
The patchset is now running on the host which needs the watchdog to trigger from time to time. It needs a while to run into the issue.
Comment Actions
Any idea how long this will take to trigger? I'd really like this patch to be pushed, as my test machines are all delivering spurious WD resets and need this patch merged (and a follow-on of my own to fix the world for ktls as well)
Comment Actions
If this fixes issues for some people, go ahead. The watchdog was broken for years and nobody complained. If this causes some issues on my system which triggered my investigation in the first place, we can fix the fix.