Skip to content

fix(quake): skip latency emulation on WSL2 - #279

Open
huklaa wants to merge 4 commits into
circlefin:mainfrom
huklaa:fix-quake-wsl2-latency
Open

fix(quake): skip latency emulation on WSL2#279
huklaa wants to merge 4 commits into
circlefin:mainfrom
huklaa:fix-quake-wsl2-latency

Conversation

@huklaa

@huklaa huklaa commented Aug 21, 2026

Copy link
Copy Markdown

Fixes #31
Quake-generated latency setup scripts currently fail on WSL2 because the required tc qdisc kernel modules are unavailable. Detect WSL2 via /proc/sys/kernel/osrelease and skip latency emulation there so localdev can continue starting without manual script edits.
Normal Linux environments keep the existing tc/netem behavior unchanged.

@osr21

osr21 commented Aug 21, 2026

Copy link
Copy Markdown

The approach here is sound, and one detail worth stating explicitly for reviewers: the generated script runs inside the containers, and on Docker Desktop / WSL2 backends the container shares the WSL2 utility VM's kernel — so /proc/sys/kernel/osrelease inside the container also reports microsoft-standard-WSL2. In-container detection is therefore reliable, which isn't obvious at first glance.

Three observations from reviewing the diff (including the latest commit bfc0bfa):

  1. Trailing whitespace introduced in bfc0bfa — the "add trailing newline" commit fixes the missing EOF newline in crates/quake/src/latency.rs, but the added line before Ok(node_regions) contains trailing spaces (and doubles the blank line). cargo fmt strips both, so this will trip a cargo fmt --check gate. A quick cargo fmt pass on the branch would clean it up.

  2. The pattern also matches WSL1grep -qi microsoft matches WSL1 (4.4.0-…-Microsoft) as well as WSL2 (…-microsoft-standard-WSL2). That's arguably desirable since WSL1 lacks netem too, but the echoed message says "WSL2 detected". Either generalize the message to "WSL detected" or pin the pattern to microsoft-standard if the intent is WSL2-only.

  3. Consider a capability probe (or an override) instead of platform detection — Microsoft's stock WSL2 kernel omits CONFIG_NET_SCH_NETEM, but a common documented workaround is running a custom-built kernel with netem enabled; those users would now get latency emulation silently disabled even though their kernel supports it. A probe skips only when netem is actually unavailable:

    if ! tc qdisc add dev lo root netem delay 1ms 2>/dev/null; then
      echo "netem unavailable on this kernel; skipping latency emulation."
      exit 0
    fi
    tc qdisc del dev lo root 2>/dev/null || true

    This also future-proofs against Microsoft adding netem to the stock kernel. Alternatively, keep the osrelease check but honor an escape hatch (e.g. QUAKE_FORCE_LATENCY=1). Worth considering because the skip silently changes simulation semantics — inter-region latencies collapse to ~0, so consensus timing measured on WSL2 won't be comparable to Linux runs; the probe at least keeps emulation on for kernels that can do it.

None of these block the fix — it unblocks localdev on WSL2 exactly as #31 asks.

@osr21

osr21 commented Aug 21, 2026

Copy link
Copy Markdown

Re-checked the branch at head bcc00b9 — both flagged items are resolved:

  • The trailing-whitespace line before Ok(node_regions) is gone (245cbaf), and the two unrelated edits that commit briefly introduced (the /// Sydney, Australia doc comment and the /*us-w-2 */ matrix column alignment) were restored in bcc00b9. The file now diffs against main with only the intended guard block plus the EOF newline.

  • Pinning the pattern to microsoft-standard is the right call, and it doesn't regress WSL1 users either: WSL1 has no Linux kernel of its own, so Docker Desktop on a WSL1-only setup falls back to the Hyper-V/linuxkit VM — whose kernel ships netem — meaning containers never actually execute under a WSL1 osrelease string. The WSL2-only match is exactly the set of environments where the qdisc modules are missing.

The capability-probe/override idea from my earlier comment remains optional future-proofing (custom WSL2 kernels with CONFIG_NET_SCH_NETEM); as it stands the PR does what #31 needs. LGTM.

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.

WSL2: latency_setup.sh fails due to missing tc (traffic control) kernel module

2 participants