Skip to content

UC reliability over TB4/USB4: hybrid completion, retransmit budget, measured loss classes (stacked on #75) - #77

Open
afrog33k wants to merge 9 commits into
hellas-ai:mainfrom
afrog33k:uc-reliability-series
Open

afrog33k wants to merge 9 commits into
hellas-ai:mainfrom
afrog33k:uc-reliability-series

Conversation

@afrog33k

Copy link
Copy Markdown

UC reliability over TB4/USB4: hybrid completion, retransmit budget, and the measured loss classes

Stacked on #75 — commits 36a47f1/2b65af3 (the NHI throttle + its compat probe) are #75's; review those there. This PR's own content is the 6 commits after them. Builds clean on both ends of our leg: Fedora 6.18 (stock NHI tree) and Asahi 7.1.13-usb4gpu (Apple NHI tree).

The problem

Two hosts — Apple M2 Ultra, Asahi 7.1.13-usb4gpu (Apple NHI) and AMD Strix Halo, Fedora 6.18-rc7 (stock PCI NHI) — joined by one 20 Gb/s TB4 cable. After the throttle fix, every sustained UC run froze permanently: the sender's last send completed, the receiver had posted exactly one fewer reply, no RNR / duplicate / error anywhere, empty dmesg on both ends.

What the counters said (module debugfs, per-QP)

loss class measured rate symptom
stranded RX completion (frame DMA'd, wire-ACKed, IRQ never signalled) ~1/280 frames under load permanent freeze, clean counters
true wire loss ~1/1M frames recoverable by retransmit, costs ~5.09 s
orphan fragment (tail before vanished head) ~1/2M messages previously fatal

The design

  • Hybrid UC completion (tbv-uc-local-completion): the WC fires at local TX drain (UC semantics) but the send context stays pending until the peer ACK; a reap retransmits drained-but-unacked sends. UC sends are solicited unconditionally and the receiver ACKs at message completion, so an ACK is a real delivery receipt.
  • tbv-rx-supp-poll: the driver's own supplemental RX poll (1 ms delay / 16 ms window) reaps frames whose completion interrupt was lost. It existed but was hard-disabled out of an Apple-backend reorder concern; now enabled native-only behind a module param (rx_supp_poll=-1). One 1M-iteration run logged 18,334 rescues — the stranded-completion class is two orders of magnitude more common than previously estimated.
  • Sendq slot leak: every UC send leaked a slot, releasing at DRAIN (E4) instead of ACK — a depth-1 QP (ibv_uc_pingpong's normal shape) dies at max_send_wr with zero counter evidence.
  • UC RNR budget: non-retryable sends froze in rnr_waiting forever; they now retry under TBV_UC_RNR_MAX_RETRIES=7 (module policy — UC retry_cnt=0 must not mean "zero wire retries"). This intersects Default data QPs to infinite RNR retries #71 (infinite RNR retries as default): we'd rather keep the budget explicit and bounded; happy to converge either way.
  • Watchdog vs retransmit race: the receiver's 5.00 s active-msg watchdog errored 90 ms before the sender's 5.09 s retransmit was due, and its error-ACK killed the sender's QP too. UC rx timeout now out-waits the sender's full budget (×8).
  • Orphan fragments buffer through the reorder machinery instead of erroring the connection.
  • rc-reorder + hot-poll: per-frame completion was the bandwidth ceiling; reordered completion polling gives 3.4× at 256 KiB and takes the 2 B floor from 22 to 14 µs.

Everything new sits behind module params and is gated to the native backend; the Apple compatibility backend is untouched.

Measured acceptance (both ends checksummed)

measurement result
1M UC pingpong, 4096 B fragmented rc=0 both ends, 96.19 µs/iter, 681 Mbit/s, 0 retransmits, 0 errors
64 B × 10M runs converged, floor 57.64 µs/iter
llama.cpp RPC over RDMA 3/3 runs, warm floor 0.693 s
16 MiB byte-verified echo identical, 1.049 Gbit/s (per-frame-CPU-bound — documented as the open gap)
MLX ring all_sum, both GPUs CORRECT

Limitations, stated

Bandwidth at large messages is per-frame-completion-CPU-bound (bigger native frames / striping next). A true wire loss still costs the full retransmit stall — a selective fragment NAK would shrink 5.09 s to ~200 µs. Wire budgets are module policy, not negotiated. Validated on exactly one leg topology (Apple NHI ↔ stock NHI, Asahi 7.1.13 ↔ Fedora 6.18).

Ronald Adonyo and others added 8 commits September 11, 2026 19:35
On an Apple NHI the interrupt throttle register (0xd004c) is not the one
tb_ring_throttling() programs (0x38c00); the kernel's own ring activation
writes it from ring->interval_nsec and skips the write when that field is
zero. This module never set the field, so on Apple hosts the throttle was
silently skipped and the register kept its firmware default -- measurable as
a latency floor that does not move with message size (2 B through 4 KB all
~65.3 us on a USB4 link).

Set ring->interval_nsec before activation for every ring on an Apple host
(including native-backend rings toward a Linux peer: the NHI is Apple's
regardless of the peer), log the before/after value, and guard the field
access behind a Makefile compile probe so stock kernels still build.

Co-Authored-By: Claude Code <noreply@anthropic.com>
…build

Hydra build 12180 failed with

    path.c:1328:31: error: 'struct tb_ring' has no member named 'interval_nsec'

on linux 6.18.37. The throttle code is already guarded by
TBV_HAVE_RING_INTERVAL_NSEC, so the guard was not missing -- the probe that
sets it reached the wrong verdict, in the one direction a probe must never
reach it.

Two defects combined:

1. It looked only under $(KDIR)/include. The nixpkgs kernel -- and therefore
   every Hydra builder -- is packaged as a SPLIT build/source tree: KDIR is
   .../lib/modules/6.18.37/build, and the real headers live in the sibling
   .../lib/modules/6.18.37/source/include/linux/. The log shows both paths
   in the same compile line. So the header was simply never found.

2. It then failed OPEN. The old form was

       ifneq ($(shell grep -c interval_nsec $(TBV_TB_HDR) 2>/dev/null),0)

   and grep on a missing file prints nothing at all, so $(shell ...) expanded
   to the empty string, "" != "0" was true, and the flag was defined. The
   probe asserted the member was present precisely because it had been unable
   to look -- a capability probe whose failure mode is to claim the
   capability.

Either defect alone is survivable. Together they turn "I cannot find your
kernel headers" into "your kernel definitely has this member", which is how a
correctly guarded file still fails to compile.

The probe now searches $(KDIR)/include, $(KDIR)/source/include and
$(KDIR)/../source/include, and uses grep -l, which prints a filename on a
match and nothing otherwise -- so an absent or unreadable header can only ever
yield "absent". The pattern matches a member declaration rather than the bare
string, so prose in a comment cannot enable the code either.

Encoded as a check rather than a comment, because a comment would not have
caught this. tools/ci/kernel-compat-probe-test.sh drives the probe against
seven synthetic kernel trees through a new kernel/Makefile compat-report
target, and flake.nix runs it as checks.kernel-compat-probe. It is cheap: no
kernel, no compiler, ~1 second.

Test results, against the probe as it stands after this change:

    ok  split tree, member absent (the Hydra case)     -> no
    ok  split tree, member present                     -> yes
    ok  nested source/ packaging, member present       -> yes
    ok  flat tree, member absent (stock kernel)        -> no
    ok  flat tree, member present (Asahi USB4)         -> yes
    ok  header mentions it only in a comment           -> no
    ok  no header at all (must fail CLOSED)            -> no

The matrix earned its place immediately: the first version of this fix looked
for $(KDIR)/source/include and passed every case except "split tree, member
present", because build/ and source/ are siblings, not nested. A comment
would have shipped that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016rZMfxUZ4TdHnxd4ZjS5At
…s/ for the diff and the lane spec for the measured evidence.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit 18b1873)
Applied patch tbv-fedora-ring-diagnostics.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit a76bb0c)
Applied patch tbv-uc-local-completion.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit efad96c)
Applied patch tbv-rx-supp-poll.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit 1b2a165)
The fedora port carried two hand-edits beyond the lane patches: an extra
TBV_TB_HDR interval_nsec compile probe in the Makefile, and the
linux/io.h + version.h + ring_diag.h includes with the Linux 6.18 PCI NHI
diagnostic block in debugfs.c. Captured verbatim so this repo is byte-identical
to what actually builds and runs on both nodes.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit 5d72ecc)
tbv-rc-reorder.patch: the receiver's reorder timeout equalled the sender's
retry budget exactly (zero margin, both ends quantizing on their own reap
tick), so the receiver's cleanup raced the sender's retransmit, dropped the
message, and killed both QPs with an ACK_ERROR. Head-only silent expiry +
two extra budget units of margin + a poison-guard history entry.
256 KiB: 107.7 -> 371.1 MiB/s (inflight=8), all drop counters zero.

tbv-hot-poll.patch: rx_supp_poll/tx_progress_poll mode 2, delay-0 re-arm on
the module's WQ_HIGHPRI workqueue, armed from TX and RX, bypassing the
kernel's normal-priority kworker delivery hop. 2 B latency typical
21-22 -> 13.7-14.3 us, min 7.5, max 23-25 (tail gone), stdev ~2.
1M UC pingpong 96.19 -> 32.46 us/iter.

kernel/{ibdev,path}.c synced to the exact deployed build on both ends.

Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit 161c5f8)
@hellasbot

hellasbot commented Sep 19, 2026 •

Copy link
Copy Markdown

Hydra: passed

Head 536c4a64e47c · Evaluation #166198 · Hydra jobset

All 22 builds passed.

The native-credit patch changed TBV_NATIVE_DATA_CREDIT_BATCH 32->256 in
proto/native_data.h, but that header is wire contract: upstream's
checks.proto-smoke pins the start-credit invariant against it, and a stock
peer computing 32-frame batches against our 256 could starve start credits
on large messages. The batch is policy, not frame layout, so it now lives in
a module param (tbv_native_credit_batch, default 32 = stock-compatible);
path.c computes the return threshold and start-credit requirement from the
param with the same formulas, and the header stays upstream-identical.

Fleet keeps the measured 256 (of +28-51% at 64-256 KiB) via modprobe.d.
Smokes: proto/reliability/identity rc=0; .ko builds on Fedora 6.18 and
Asahi 7.1.13-usb4gpu.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants