Review the Cursor-authored io_uring Phase B: flip default to poll, harden uring, cleanups #16

Merged

vxfemboy merged 2 commits from chore/cursor-review-followups into main

2026-07-02 20:38:50 +00:00

vxfemboy commented

2026-07-02 17:04:24 +00:00

(Migrated from github.com)

What & why

You asked me to review the Cursor-co-authored commits, clean up, and actually run the benchmarks. This PR is the result. The headline: the io_uring Phase B driver (#7) was the default on every modern Linux host, but it's slower and less robust than the epoll fallback it defaults over.

Benchmarks (re-run on this host, not trusting the committed numbers)

metric	default `PollDriver`	opt-in io_uring
tunnel RTT (north-star)	~0.48 ms	~0.63 ms	uring ~33% worse — 4/4 runs, both orderings
clean-link iperf3	~305–319 Mbit/s	~306–356	tied

io_uring delivers no measurable benefit and regresses the one metric it exists to improve. Mechanism: no SQPOLL (so it pays the same two syscalls epoll does, plus provided-buffer bookkeeping), and its GSO batching is a no-op (MAX_GSO_SEGMENTS_PER_SEND = 1).

Changes

1. Flip the default to PollDriver (tunnel.rs). io_uring is now opt-in via YIP_USE_URING=1 (was: default-on, escape with YIP_FORCE_POLL). CI still gates both drivers in netns-tunnel-test.

2. Harden the opt-in uring path — two robustness gaps a code review found, both departures from poll.rs's contract:

EINTR on the blocking ring wait (submit_and_wait) propagated and killed the tunnel; poll.rs's epoll loop retries it. Added an EINTR-retrying wrapper over both wait sites.
Send-completion errors were dropped for every errno, silently swallowing fatal errors (ENETUNREACH/ECONNREFUSED/EBADF) forever. Now mirrors poll.rs exactly via a per-slot SendKind: TUN writes always drop; UDP sends drop on transient buffer pressure (EAGAIN/EWOULDBLOCK/ENOBUFS) but propagate fatal errors.

3. Cleanups + honest docs: removed a stray committed task-7-report.md; re-measured & re-framed the bench README A/B section; README/CHANGELOG no longer present io_uring as the active latency mechanism; documented the MAX_GSO_SEGMENTS_PER_SEND=1 GSO no-op as the first thing to fix before uring could earn the default back.

Not done here (deliberately)

Re-enabling GSO batching — raising the cap risks re-breaking arq_recovers_bulk_loss (coalesced bursts were why it was capped to 1). Left as a documented follow-up; it needs its own re-benchmark.
The per-packet heap-copy and shared-256-slot findings are documented but out of scope for this PR.

Verification

cargo build --workspace + clippy --all-targets -D warnings + unit tests: green.
All three netns tunnel tests (ping, ping-under-loss, ARQ integrity) pass under both drivers.
A/B RTT confirms the default (no env) now takes the 0.38 ms poll path.

🤖 Generated with Claude Code

## What & why You asked me to review the Cursor-co-authored commits, clean up, and actually run the benchmarks. This PR is the result. The headline: **the io_uring Phase B driver (#7) was the default on every modern Linux host, but it's slower *and* less robust than the epoll fallback it defaults over.** ### Benchmarks (re-run on this host, not trusting the committed numbers) | metric | default `PollDriver` | opt-in io_uring | | |---|---|---|---| | **tunnel RTT** (north-star) | **~0.48 ms** | ~0.63 ms | uring ~33% worse — 4/4 runs, both orderings | | clean-link iperf3 | ~305–319 Mbit/s | ~306–356 | tied | io_uring delivers no measurable benefit and regresses the one metric it exists to improve. Mechanism: no SQPOLL (so it pays the same two syscalls epoll does, plus provided-buffer bookkeeping), and its GSO batching is a no-op (`MAX_GSO_SEGMENTS_PER_SEND = 1`). ## Changes **1. Flip the default to `PollDriver`** (`tunnel.rs`). io_uring is now opt-in via `YIP_USE_URING=1` (was: default-on, escape with `YIP_FORCE_POLL`). CI still gates **both** drivers in `netns-tunnel-test`. **2. Harden the opt-in uring path** — two robustness gaps a code review found, both departures from `poll.rs`'s contract: - **`EINTR` on the blocking ring wait** (`submit_and_wait`) propagated and killed the tunnel; `poll.rs`'s epoll loop retries it. Added an `EINTR`-retrying wrapper over both wait sites. - **Send-completion errors were dropped for *every* errno**, silently swallowing fatal errors (`ENETUNREACH`/`ECONNREFUSED`/`EBADF`) forever. Now mirrors `poll.rs` exactly via a per-slot `SendKind`: TUN writes always drop; UDP sends drop on transient buffer pressure (`EAGAIN`/`EWOULDBLOCK`/`ENOBUFS`) but propagate fatal errors. **3. Cleanups + honest docs**: removed a stray committed `task-7-report.md`; re-measured & re-framed the bench README A/B section; README/CHANGELOG no longer present io_uring as the *active* latency mechanism; documented the `MAX_GSO_SEGMENTS_PER_SEND=1` GSO no-op as the first thing to fix before uring could earn the default back. ## Not done here (deliberately) - **Re-enabling GSO batching** — raising the cap risks re-breaking `arq_recovers_bulk_loss` (coalesced bursts were why it was capped to 1). Left as a documented follow-up; it needs its own re-benchmark. - The per-packet heap-copy and shared-256-slot findings are documented but out of scope for this PR. ## Verification - `cargo build --workspace` + `clippy --all-targets -D warnings` + unit tests: green. - All three netns tunnel tests (ping, ping-under-loss, ARQ integrity) pass under **both** drivers. - A/B RTT confirms the default (no env) now takes the 0.38 ms poll path. 🤖 Generated with [Claude Code](https://claude.com/claude-code)