Skip to content

Add optional bandwidth-aware mode for urltest - #4398

Open
zydo wants to merge 221 commits into
SagerNet:testingfrom
zydo:testing
Open

Add optional bandwidth-aware mode for urltest#4398
zydo wants to merge 221 commits into
SagerNet:testingfrom
zydo:testing

Conversation

@zydo

@zydo zydo commented Aug 7, 2026

Copy link
Copy Markdown

Closes #4397.

Problem

urltest ranks outbounds by a single scalar: time to response headers from a HEAD. That is a good proxy for reachability and round-trip latency, but carries no information about sustained throughput. On a congested or shaped path the two decouple — the probe completes before the connection leaves slow start, so it never observes that throughput has collapsed, and the slower outbound is ranked first. No value of tolerance can express "prefer the outbound that actually moves data", because every stored sample is latency.

Approach

An opt-in bandwidth_test block, disabled by default. When disabled the probe path is byte-for-byte what it is today: a HEAD with no body transfer.

When enabled, each outbound is additionally probed with a bounded GET:

  • TTFB is recorded with the existing timer semantics, so delay keeps its current meaning
  • the body is read into one reusable 32 KiB buffer until max_bytes, then the request context is cancelled so the remainder is torn down rather than drained
  • throughput is bytes / transfer_time, excluding TTFB
  • both metrics land on URLTestHistory and surface through the Clash API

Selection gains two strategies alongside the unchanged default: throughput, and throughput_with_latency_floor (discard outbounds above the floor, then rank survivors by throughput).

Design notes

Bounded, not a benchmark. A cap in the low hundreds of KiB measures while the flow is still near slow start, so the absolute number understates a fast path. That is acceptable for ranking — the ratio between a shaped and unshaped path is already large at that scale — and the docs say so explicitly rather than presenting it as a speed test.

Memory is independent of the cap. max_bytes bounds bytes read, not bytes retained; the discard loop uses a single pooled buffer. This is the binding constraint on iOS, where the network extension runs under a hard jetsam limit (#3976).

Cost control. Bandwidth probing runs on its own cadence (5× the latency interval by default), with concurrency 2 rather than the latency sweep's fixed 10, and inherits the group's existing idle suspension and pause.Manager handling — no probing while the group is unused or the device is asleep.

Non-2xx is rejected. A rate-limited endpoint returns a small error page fast, which would otherwise score as excellent throughput (cf. #4189). Note this makes the two probes asymmetric: the latency probe accepts any status, this one requires 2xx.

Failure decays rather than evicting. A failed bandwidth probe records a zero sample and never touches the latency history, so it cannot deselect an outbound the latency probe still considers reachable. Median smoothing over the last N samples absorbs a single transient failure while letting sustained failure decay an outbound out of contention — this is what keeps the group from oscillating and repeatedly firing interruptGroup.Interrupt.

Storage merges instead of replacing. StoreURLTestDelay / StoreURLTestBandwidth update one metric while preserving the other, copy-on-write under the lock since readers hold the pointer from LoadURLTestHistory. The four existing latency call sites (Clash API ×2, daemon ×2) were switched over so a manually triggered delay test no longer discards throughput.

Deviation from the issue

The issue argued for no default url, failing closed. This PR ships one: https://speed.cloudflare.com/__down?bytes=<max_bytes>, mirroring how the latency probe defaults to generate_204. The byte count is derived from max_bytes so the response is exactly as large as the probe reads, and the docs recommend substituting your own endpoint. Happy to revert to fail-closed if you prefer the original reasoning.

Config uses plain numbers (max_bytes: 262144, throughput_tolerance: 25) rather than the issue's "256KiB" / "25%" strings, to avoid introducing two new marshalable option types.

Testing

  • protocol/group: option validation, median smoothing, relative hysteresis, sliding sample window
  • common/urltest: the byte cap, non-2xx rejection, empty body, throughput arithmetic, and the storage merge semantics
  • test/urltest_test.go: end-to-end through real shadowsocks proxies. urltest had no integration coverage at all, so this adds the baseline too — latency ranking, dropping a failed outbound, tolerance holding the incumbent, and URLTest reporting delays — before the bandwidth cases.

The headline test encodes the issue's own table: two paths where one answers fast then crawls and the other is slow to first byte but moves data. Same config, only strategy differs — latency picks the first, throughput picks the second. It cannot pass for the wrong reason: with no throughput sample, selection falls back to latency and picks the first.

Ran the integration suite 10× consecutively with no failures. goleak (already wired into that package) covers the second ticker's cleanup.

Two things found along the way

minDelay == 0 is a sentinel collision. In latency Select, minDelay == 0 means "no incumbent yet", but a probe over a fast path genuinely measures 0 ms — so a healthy outbound reads as unset and a slower one overrides it. Reproducible over loopback; the integration tests keep every backend above a millisecond to work around it. Pre-existing and independent of this change — happy to file separately or fix here, whichever you prefer.

An untouched group probes exactly once. The retry ticker is only created by Touch(), so a group that is never dialed probes at PostStart and never again; a single transient failure leaves it permanently unselected. Harmless in production since traffic touches the group, but worth knowing.

Open question

TCP and UDP are selected separately, but there is one probe, dialed over TCP, so UDP selection is ranked by a TCP-measured sample. Defensible for a QUIC outbound sharing the path, less so otherwise. Latency has the same property today, so this is not a regression — but I am happy to restrict throughput ranking to TCP if you would rather be conservative.

Commits

  1. Repair test module dependency graph — the test/ module could not build at all: gvisor was pinned to a 2025 revision while sing-tun and tailscale had moved to versions requiring the current one. Unrelated to the feature, kept separate; verified by moving the new test file aside and reproducing on a clean tree.
  2. Add optional bandwidth-aware mode for urltest — the feature, tests, and docs (en + zh, plus regenerated docs/schema.json).

@ma1exe

ma1exe commented Aug 7, 2026

Copy link
Copy Markdown

Please, add this feature.

zydo added 2 commits August 8, 2026 01:57
The test module pinned gvisor to a 2025 revision while sing-tun and
tailscale had moved on to versions requiring the current one, so the
module failed to build before any test could run.
The latency probe measures time to response headers, which carries no
information about sustained throughput. On a congested or shaped path
the two decouple: a small probe completes before the connection leaves
slow start, so it never observes that throughput has collapsed, and
urltest ranks the slower outbound first.

Add an opt-in bandwidth_test block that probes each outbound with a
bounded GET, reading at most max_bytes into a single reusable buffer and
cancelling at the cap rather than draining the remainder. Effective
throughput is recorded alongside the existing delay and exposed through
the Clash API, and two new strategies rank on it: throughput, and
throughput_with_latency_floor.

Disabled by default. When disabled the probe path is unchanged and no
response body is ever transferred. Bandwidth probing runs on its own
cadence and inherits the group's idle suspension and pause handling.

Also adds urltest integration coverage, which did not previously exist.

Closes SagerNet#4397
@nekohasekai
nekohasekai force-pushed the testing branch 4 times, most recently from 9b20d01 to 426c5fa Compare August 9, 2026 06:58
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.

Feature request: Optional bandwidth-aware mode for URLTest

6 participants