fix(net): give inline fast-path flows sender-side retransmission

[AprilNEA]

Closes #486 — the residual gap from #451: both inline owners (arcbox-net-inject's readv thread, direct_rx's tokio reader) had window gating but no retransmission, so a single frame dropped past guest eth0 wedged the flow forever. The inline_conn.rs premise "the inject path is lossless — no gap can form" only holds to eth0; #451 established the guest-internal bridge → veth → container-netns backlog drops under burst, and for container flows the TCP endpoint sits behind that hop. Blocks #262 (promote all flows to inline) and matters for #250 (HV default).

Design (as assessed in #486)

The zero-copy read (socket → guest descriptor buffer, payload never host-side) is fundamentally at odds with retransmission, so:

• Tee-on-send: each owner copies every sent payload into a per-flow retransmission ring shared with the bridge's FastPathConn — (base seq, unACKed bytes, FIN seq once sent), std types only (Arc<Mutex<(u32, VecDeque<u8>, Option<u32>)>>) so splicetcp and arcbox-net-inject share it without depending on each other, the same pattern as the existing shared seq atomics. Bounded by the 256 KiB honored-window budget the owners already respect.
• Bridge-side recovery: poll_fast_path (which the shared datapath loop already drives for inline flows — fix(net): add a zero-window persist probe to the download fast path #477 relies on this) drains the ring's ACKed prefix as guest_acked advances and re-emits everything past the ack point on triple dup-ACK (already counted by the intercept for inline flows, previously never consumed) or RTO (same INITIAL→MAX backoff as the polled path), including a recorded lost FIN.
• Retransmissions travel the ordinary polled frame path (guest_tx) — correct and rare; the zero-copy hot loop is untouched except the tee memcpy (noise at single-digit Gbps next to the syscall).
• The stale "lossless" comment is corrected.

Tests

• Bridge side (captured inline conn, simulated owner): RTO re-emit from the ring, triple-dup-ACK fast retransmit without RTO wait, ACK draining the ring + no spurious retransmit, lost-FIN re-emit that stops once ACKed past.
• Owner side: tee assertions folded into the existing send/EOF tests of both owners — including the multi-descriptor readv span (500 B across 3 descriptors) and FIN-position recording.

splicetcp 94 + arcbox-net-inject 9 + arcbox-net 82 all green; fmt + clippy clean; arcbox-vmm compiles.

HV acceptance (2026-07-21/22, via the new ARCBOX_E2E_BACKEND override)

The #450 network-workload suite on the HV backend: 7/8 checks pass — including every datapath scenario this retransmission exists for (burst single/multi, churn, parallel_large_downloads — the burst-loss shape that wedged inline flows before the fix — uploads, DNS). The single failure, docker_build_network, reproduces identically on VZ and on an origin/master control build: a pre-existing master regression (tracked as #490, stalls in the buildkit/API zone before any egress), not caused by this PR. VZ shows the same 7/8 — no regression on the default backend.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8127eca498

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[pullfrog]

✅ No new issues found.

Reviewed changes — adds sender-side loss recovery to the two inline fast-path owners, which previously had window gating but no retransmission, so a frame dropped past guest eth0 wedged an inline flow forever; a follow-up commit adds an e2e backend override to enable the HV acceptance run.

• Shared retransmission ring RetxRing — Arc<Mutex<(base_seq, VecDeque<u8>, Option<fin_seq>)>>, defined as a std-only tuple in both splicetcp::direct_rx and arcbox-net-inject::inline_conn so the two crates share it without depending on each other; the cross-crate assignment in inline_sink.rs compile-guards any divergence.
• Tee-on-send in both owners — inject.rs tees the multi-descriptor readv span (payload offset on descriptor 0) and direct_rx.rs tees the socket read; each records the FIN position at EOF.
• Bridge-side recovery in poll_fast_path — drains the ACKed prefix as guest_acked advances and re-emits from the ring (plus a lost FIN) on triple-dup-ACK or RTO, mirroring the existing non-inline retransmit_buf path; retransmits travel the ordinary polled guest_tx path, not the zero-copy inject path.
• Corrected stale "lossless" comment on InlineConn::send_budget; added tests on both owner paths and four bridge-side recovery scenarios.
• ARCBOX_E2E_BACKEND scenario override (tests/e2e/src/scenario.rs) — lets run_vz_scenario_with_log rerun any scenario against HV (default vz), and records the actual backend in RunMetrics, enabling the #486 acceptance suite on the HV datapath.

I verified the load-bearing correctness questions directly against the code:

• The inject.rs tee raw-pointer read is in-bounds — per_desc_len[i] is the payload length, so TOTAL_HDR_LEN + take never exceeds desc_lens[0].
• The ring stays bounded by HONORED_WINDOW_CAP — both owners gate reads on send_budget, so only unACKed bytes accumulate.
• FastPathConn retransmit fields are initialized at promote and the intercept updates the shared atomics and dup-ACK counter for inline flows, so the re-emit block behaves identically to the well-tested non-inline path (including the "parked" RTO clock while nothing is in flight).

ℹ️ #486 acceptance (HV network-workload run under real loss) not yet executed

The retransmission logic is well covered by unit tests on both owner paths and four bridge-side recovery scenarios (RTO re-emit, triple-dup-ACK fast retransmit, ACK draining without spurious retransmit, lost-FIN re-emit). The new ARCBOX_E2E_BACKEND=hv override supplies the mechanism to run the network-workload suite on the real HV datapath — the very condition that motivates the fix — but the PR does not yet include a recorded result from that run.

• The author notes this: full acceptance per #486 remains follow-up work needing its own session.
• Nothing here blocks merge; this is a note so the follow-up validation is tracked rather than lost.

Technical details

# #486 acceptance run pending (mechanism now present)

## Affected sites
- `tests/e2e/src/scenario.rs:56` — `ARCBOX_E2E_BACKEND` override enables HV runs.
- PR description — "Full acceptance per #486 (network-workload suite on the HV backend) remains follow-up work."

## Required outcome
- An inline-promoted flow is exercised through the network-workload suite on HV under induced burst loss (bridge → veth → container netns backlog), confirming a flow that would previously have wedged now recovers and completes.

## Suggested approach (optional)
- Run the network-workload scenario with `ARCBOX_E2E_BACKEND=hv` and confirm throughput recovers after loss; VZ is the oracle, so compare HV against VZ per the backend split.

## Open questions for the human (optional)
- Should merge gate on that HV run, or is landing the unit-tested logic now (with the acceptance run as a tracked follow-up) acceptable given #262/#250 depend on it?

ℹ️ Nitpicks

• tests/e2e/src/scenario.rs — run_vz_scenario_with_log can now run any backend, so the _vz in the name is mildly misleading; the inline comment already acknowledges "default vz, the name notwithstanding", so a rename is optional.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[greptile-apps]

Greptile Summary

This PR adds sender-side recovery for dropped inline TCP frames. The main changes are:

• Tee inline payloads and FIN positions into a shared retransmission ring.
• Retransmit unacknowledged data after duplicate ACKs or timeout.
• Reserve RX batch capacity for recovery traffic.
• Add focused recovery tests and an HV E2E backend override.
• Recover from corrupt Docker image cache archives.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex Logs

What T-Rex did

• The current-head proof was reviewed to confirm the exact command run, the working directory, an exit code of 0, and verbose per-test results.
• The before/after logs were examined to verify they share the inline-tcp-retransmission-20260721T165850Z stem.
• It was verified that no macOS-only injector or hypervisor paths were attempted.
• Two log artifacts were provided to support reviewer inspection.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
common/splicetcp/src/direct_rx.rs	Adds the shared retransmission ring and records async inline payloads and FIN positions.
common/splicetcp/src/tcp_bridge/fast_path.rs	Drains acknowledged ring data and retransmits outstanding payloads or FINs.
common/splicetcp/src/tcp_bridge/mod.rs	Stores the optional retransmission ring on inline fast-path connections.
common/splicetcp/src/tcp_bridge/tests.rs	Adds coverage for timeout, duplicate-ACK, ACK-drain, FIN, and sequence-clamping behavior.
virt/arcbox-net-inject/src/inject.rs	Tees descriptor payloads into the ring and reserves batch space for recovery frames.
virt/arcbox-net-inject/src/inline_conn.rs	Defines the shared ring shape and adds it to inline connection state.
virt/arcbox-vmm/src/vmm/darwin_hv/inline_sink.rs	Passes the retransmission ring into the HV inline connection owner.
tests/e2e/src/docker.rs	Discards corrupt image caches and validates newly saved archives.
tests/e2e/src/scenario.rs	Allows E2E scenarios to select HV while keeping VZ as the default.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Owner as Inline owner
participant Ring as Retransmission ring
participant Guest as Guest TCP path
participant Bridge as TCP bridge

Owner->>Owner: Advance send sequence
Owner->>Ring: Tee payload or record FIN
Owner->>Guest: Publish inline frame
Guest-->>Bridge: Send ACK
Bridge->>Ring: Drain acknowledged prefix
alt Duplicate ACK threshold or timeout
    Bridge->>Ring: Read outstanding data
    Bridge->>Guest: Re-emit payload or FIN
end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Owner as Inline owner
participant Ring as Retransmission ring
participant Guest as Guest TCP path
participant Bridge as TCP bridge

Owner->>Owner: Advance send sequence
Owner->>Ring: Tee payload or record FIN
Owner->>Guest: Publish inline frame
Guest-->>Bridge: Send ACK
Bridge->>Ring: Drain acknowledged prefix
alt Duplicate ACK threshold or timeout
    Bridge->>Ring: Read outstanding data
    Bridge->>Guest: Re-emit payload or FIN
end

Loading

_{Reviews (4): Last reviewed commit: "test(net): assert window compliance by s..." | Re-trigger Greptile}

[pullfrog]

✅ No new issues found.

Reviewed changes — this run covers the hardening commit landed since the prior pullfrog review, which resolves the two Codex findings (P1 retransmit starvation, P2 teeing ahead of SND.NXT) with matching tests.

• Reserved channel-drain budget in the inject thread — poll_inline_conns now stops at inline_cap = BATCH_SIZE - CHANNEL_RESERVE (192 of 256), while the phase-3 channel drain still runs to full BATCH_SIZE; since guest_tx carries these flows' own ACKs and retransmissions, a continuously readable conn can no longer starve a stalled sibling's recovery frames.
• Tee now follows the sequence advance in both owners — direct_rx tees each chunk after our_seq.fetch_add inside the send loop, and inject.rs moves the our_seq advance ahead of the tee block, so the shared ring can only run short of SND.NXT, never ahead (a racing bridge poll retransmits short at worst, repaired by the next RTO).
• Bridge clamps re-emission to in-flight bytes — poll_fast_path re-emits ring[offset..].take(in_flight), a belt-and-suspenders guard so a transiently-ahead ring can never retransmit past sent (which the intercept would reject as beyond our_seq).
• Added regression test inline_retransmit_never_exceeds_sent (100 teed / 40 committed → exactly 40 retransmitted), plus owner-side tee/FIN-position assertions folded into existing tests.

I verified the load-bearing questions directly against the code:

• The reserve holds: batch is shared across phases and phase 3 drains up to BATCH_SIZE (line 128), so the 64-slot headroom is genuinely available; worst-case single-readv overshoot (num_used ≤ 16) still leaves ~49 reserved slots.
• The clamp is sound: inside the !nothing_in_flight branch in_flight = sent − acked is in (0, 0x8000_0000), so .take(in_flight as usize) bounds retransmission to the committed sequence space.
• The tee reordering means the production ring runs short, never ahead — the clamp and the new test defend the inverse transient as documented insurance, not a live code path.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes — this run covers the single commit landed since the prior pullfrog review: a test-only hardening of the e2e image-cache helper. No production code changed.

• Cache self-heal in ensure_image (tests/e2e/src/docker.rs) — a failed docker load from the cached tar no longer ?-propagates and wedges every subsequent run; it now warns, remove_files the corrupt tar, and falls through to a fresh pull.
• Post-save archive validation — after docker save, the tar is verified with tar -tf and discarded on failure, catching a truncated docker save (HV, #256) at write time instead of on the next run's load. The return Ok(()) is retained because the image is already loaded in the daemon from the pull, so discarding the cache only prevents future poisoning.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

CI failed on the Run clippy step: clippy::map_unwrap_or at tests/e2e/src/docker.rs:145, in code this PR added (the tar-validation cache guard). Fixed by replacing listing.map(|o| o.status.success()).unwrap_or(false) with listing.is_ok_and(|o| o.status.success()) — the exact clippy-suggested rewrite. Committed and pushed to fix/inline-fast-path-retransmission.

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes — this run covers the two test-only commits landed since the prior pullfrog review (cacb6761): a clippy fix and a flakiness fix to the guest-window test. No production code changed.

• is_ok_and clippy fix in ensure_image (tests/e2e/src/docker.rs) — the post-save tar validation now reads !listing.is_ok_and(|o| o.status.success()), which correctly treats both a failed tar spawn and a non-zero listing as a bad cache. Behaviorally equivalent to the prior form, clippy-clean.
• Coverage-based window assertion in poll_fast_path_respects_guest_window (common/splicetcp/src/tcp_bridge/tests.rs) — the drain helper now takes a base and measures the highest sequence-space coverage emitted past it (covered.max(seq + payload - base)) instead of summing byte totals. This fixes a real flake: on a slow machine the 200ms RTO can fire mid-drain and retransmit in-window bytes, which the old byte-sum double-counted and could push past the <= 65535 window assertion. Coverage is the correct window metric and retransmissions don't inflate it.

I verified the coverage arithmetic is safe: the flow's data starts at seq 1 (confirmed by the ACK field 1 + first) and the ring base advances to 1 + first on the guest ACK before the second drain, so no emitted frame ever sits below base — wrapping_sub(base) stays small and cannot produce a spurious near-u32::MAX coverage value.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}