fix(nesdoctor): bloat was measured against the median, and could grade a bad line A

Found by running the published one-liner, which is the only reason it was
found: `up=34Mbps rtt=188ms rttload=181ms bloat=+0ms grade=A` on a connection
that measured +115 ms and grade F three hours earlier.

The idle baseline was the median of twelve handshakes to one anycast address.
On the development connection those twelve came back **bimodal**:

    [56, 56, 57, 59, 60, 176, 177, 179, 179, 179, 182, 368]
    min 56   p50 177   max 368   spread 312 ms on an *idle* link

Two points of presence answering. The median therefore lands wherever the split
happens to fall, and when it lands high the loaded median comes in *below* it,
the difference goes negative, `.max(0.0)` clamps it to zero, and the headline
number reports grade A.

That is the one error direction that cannot be tolerated here. A tool whose
whole pitch is a number nobody else shows you has no business saying "your line
is fine" about a line that is not.

Bloat is now measured against the **minimum**. Queueing is delay above the
floor the path can achieve, so the floor is the baseline -- which is also how
every bufferbloat test does it. Twenty samples rather than twelve.

The distance verdict deliberately keeps the **median**, because it asks a
different question. Bloat asks how much queueing is added, so its baseline is
the best case. HOST-READY-LOCAL asks what a player will actually see, so it
takes the typical case: on a link that is bimodal between 56 ms and 180 ms, the
floor would call it near when half of all connections are not.

Both are now reported, and the gap between them is itself the finding -- a
floor of 55 ms against a typical of 180 ms says the route is the problem, which
no single number could have said.

Verified on the same connection: floor 55, typical 180, loaded 95, **+39 ms,
grade C**, verdict HOST-NET. Defensible, and no longer flattering.

Version to 0.1.1. Submissions carry it, so any row with `v=0.1.0` has a grade
that cannot be trusted.
This commit is contained in:
Wanjohi
2026-09-02 13:34:00 +03:00
parent 166c1e9c24
commit f0e65b9738
6 changed files with 62 additions and 14 deletions

2
Cargo.lock generated
View File

@@ -2462,7 +2462,7 @@ dependencies = [
[[package]] [[package]]
name = "nesdoctor" name = "nesdoctor"
version = "0.1.0" version = "0.1.1"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"clap", "clap",

View File

@@ -1,6 +1,6 @@
[package] [package]
name = "nesdoctor" name = "nesdoctor"
version = "0.1.0" version = "0.1.1"
edition.workspace = true edition.workspace = true
license.workspace = true license.workspace = true
repository.workspace = true repository.workspace = true

View File

@@ -62,12 +62,18 @@ their connection adds when it is busy**, and for anything interactive that is th
figure that decides it: figure that decides it:
``` ```
upstream 28 Mbps upstream 39 Mbps
latency, idle 179 ms latency, idle floor 55 ms
latency, loaded 198 ms latency, idle typ. 180 ms
added under load +19 ms grade B latency, loaded 95 ms
added under load +39 ms grade C
``` ```
Two idle figures because they answer different questions. **Floor** is the best
the path can do, and queueing is everything above it — so that is what the
bloat number is measured against. **Typical** is what a connection actually
gets, and where the two differ this much, the route itself is the problem.
A 500 Mbps uplink that queues for 300 ms under load cannot carry a game. A A 500 Mbps uplink that queues for 300 ms under load cannot carry a game. A
25 Mbps one with `fq_codel` or CAKE can. If your grade is C or F it is almost 25 Mbps one with `fq_codel` or CAKE can. If your grade is C or F it is almost
always a router setting rather than a line you need to upgrade. always a router setting rather than a line you need to upgrade.

View File

@@ -353,7 +353,8 @@ fn print_net(n: &net::NetReport) {
.unwrap_or_else(|| "".into()) .unwrap_or_else(|| "".into())
}; };
println!(" upstream {}", f(n.upstream_mbps, " Mbps")); println!(" upstream {}", f(n.upstream_mbps, " Mbps"));
println!(" latency, idle {}", f(n.idle_rtt_ms, " ms")); println!(" latency, idle floor {}", f(n.idle_rtt_ms, " ms"));
println!(" latency, idle typ. {}", f(n.idle_rtt_p50_ms, " ms"));
if n.idle_rtt_ms.is_some_and(|r| r > 60.0) { if n.idle_rtt_ms.is_some_and(|r| r > 60.0) {
println!( println!(
" \x1b[2m That is the round trip to the *nearest* major network, so it is a\x1b[0m" " \x1b[2m That is the round trip to the *nearest* major network, so it is a\x1b[0m"

View File

@@ -55,7 +55,12 @@ const CHUNK: usize = 1 << 20; // 1 MiB per write
#[derive(Debug, Serialize)] #[derive(Debug, Serialize)]
pub struct NetReport { pub struct NetReport {
/// The **floor** of the idle distribution, which is the propagation delay
/// the path is capable of. Baseline for [`NetReport::bloat_ms`].
pub idle_rtt_ms: Option<f64>, pub idle_rtt_ms: Option<f64>,
/// The **typical** idle round trip. Reported separately because the two
/// answer different questions and, on a badly routed link, differ wildly.
pub idle_rtt_p50_ms: Option<f64>,
pub loaded_rtt_ms: Option<f64>, pub loaded_rtt_ms: Option<f64>,
pub loaded_rtt_p95_ms: Option<f64>, pub loaded_rtt_p95_ms: Option<f64>,
/// Loaded minus idle: the queue, in milliseconds. /// Loaded minus idle: the queue, in milliseconds.
@@ -77,6 +82,7 @@ impl NetReport {
fn unavailable(note: &str) -> Self { fn unavailable(note: &str) -> Self {
Self { Self {
idle_rtt_ms: None, idle_rtt_ms: None,
idle_rtt_p50_ms: None,
loaded_rtt_ms: None, loaded_rtt_ms: None,
loaded_rtt_p95_ms: None, loaded_rtt_p95_ms: None,
bloat_ms: None, bloat_ms: None,
@@ -116,8 +122,14 @@ pub fn run() -> NetReport {
}; };
// --- idle baseline --------------------------------------------------- // --- idle baseline ---------------------------------------------------
let idle = sample_rtt(addr, 12, Duration::from_millis(120)); //
let Some(idle_p50) = percentile(&idle, 0.50) else { // Twenty samples, not twelve. Measured on a Nairobi connection
// 2026-09-02, twelve idle handshakes to one anycast address came back
// **bimodal** -- `[56, 56, 57, 59, 60, 176, 177, 179, 179, 179, 182, 368]`,
// two different points of presence answering, 312 ms of spread on an *idle*
// link.
let idle = sample_rtt(addr, 20, Duration::from_millis(120));
let Some(idle_min) = percentile(&idle, 0.0) else {
return NetReport::unavailable( return NetReport::unavailable(
"no TCP handshake completed to 1.1.1.1:443 — a firewall may block it, so the \ "no TCP handshake completed to 1.1.1.1:443 — a firewall may block it, so the \
latency half could not run", latency half could not run",
@@ -160,7 +172,19 @@ pub fn run() -> NetReport {
let loaded_p50 = percentile(&loaded, 0.50); let loaded_p50 = percentile(&loaded, 0.50);
let loaded_p95 = percentile(&loaded, 0.95); let loaded_p95 = percentile(&loaded, 0.95);
let bloat = loaded_p50.map(|l| (l - idle_p50).max(0.0)); let idle_p50 = percentile(&idle, 0.50);
// Bloat is measured against the **minimum**, not the median.
//
// Queueing is delay *above the floor the path can do*, so the floor is the
// baseline; that is also how every bufferbloat test does it. Using the
// median was a real bug and it failed in the dangerous direction: on the
// bimodal link above, the idle median landed at 188 ms while the loaded
// median came back 181 ms, so the difference went negative, clamped to
// zero, and reported **grade A on a connection that measures +115 ms and
// grade F**. A tool whose headline number can say "fine" about a line that
// is not fine has no business being trusted with the rest.
let bloat = loaded_p50.map(|l| (l - idle_min).max(0.0));
let note = match (&upstream_mbps, &bloat) { let note = match (&upstream_mbps, &bloat) {
(None, _) => "upstream could not be measured (the upload sink was unreachable), so the \ (None, _) => "upstream could not be measured (the upload sink was unreachable), so the \
@@ -175,7 +199,8 @@ pub fn run() -> NetReport {
}; };
NetReport { NetReport {
idle_rtt_ms: Some(idle_p50), idle_rtt_ms: Some(idle_min),
idle_rtt_p50_ms: idle_p50,
loaded_rtt_ms: loaded_p50, loaded_rtt_ms: loaded_p50,
loaded_rtt_p95_ms: loaded_p95, loaded_rtt_p95_ms: loaded_p95,
bloat_ms: bloat, bloat_ms: bloat,
@@ -204,6 +229,7 @@ fn sample_rtt(addr: SocketAddr, n: usize, gap: Duration) -> Vec<f64> {
out out
} }
/// `p` of 0.0 gives the minimum, which is what the bloat baseline uses.
fn percentile(v: &[f64], p: f64) -> Option<f64> { fn percentile(v: &[f64], p: f64) -> Option<f64> {
if v.is_empty() { if v.is_empty() {
return None; return None;

View File

@@ -119,7 +119,17 @@ pub fn verdict(sys: &SysInfo, host: &HostReport, net: &NetReport) -> Verdict {
(Some(up), Some(bloat)) => { (Some(up), Some(bloat)) => {
if up < MIN_UP_MBPS || bloat > MAX_BLOAT_MS { if up < MIN_UP_MBPS || bloat > MAX_BLOAT_MS {
Verdict::HostBlockedByNetwork Verdict::HostBlockedByNetwork
} else if net.idle_rtt_ms.is_some_and(|r| r > FAR_RTT_MS) { // Deliberately the *median* and not the floor. Bloat asks "how much
// queueing is added", so its baseline is the best the path can do.
// This asks "what will a player actually see", so it takes the
// typical case -- and on a link whose idle latency is bimodal
// between 56 ms and 180 ms, the floor would call it near when half
// of all connections are not.
} else if net
.idle_rtt_p50_ms
.or(net.idle_rtt_ms)
.is_some_and(|r| r > FAR_RTT_MS)
{
Verdict::HostReadyLocalOnly Verdict::HostReadyLocalOnly
} else { } else {
Verdict::HostReady Verdict::HostReady
@@ -185,8 +195,10 @@ pub fn summary_line(f_: &Full) -> String {
match (net.upstream_mbps, net.bloat_ms, net.grade) { match (net.upstream_mbps, net.bloat_ms, net.grade) {
(Some(up), Some(b), Some(g)) => f.push(format!( (Some(up), Some(b), Some(g)) => f.push(format!(
"up={up:.0}Mbps rtt={}ms bloat=+{b:.0}ms grade={g}", "up={up:.0}Mbps rtt={}/{}ms bloat=+{b:.0}ms grade={g}",
net.idle_rtt_ms.map_or("?".into(), |r| format!("{r:.0}")) net.idle_rtt_ms.map_or("?".into(), |r| format!("{r:.0}")),
net.idle_rtt_p50_ms
.map_or("?".into(), |r| format!("{r:.0}"))
)), )),
_ => f.push("net=unmeasured".into()), _ => f.push("net=unmeasured".into()),
} }
@@ -442,6 +454,9 @@ pub fn submit_url(base: &str, f_: &Full) -> String {
if let Some(v) = net.idle_rtt_ms { if let Some(v) = net.idle_rtt_ms {
put("rtt", format!("{v:.0}")); put("rtt", format!("{v:.0}"));
} }
if let Some(v) = net.idle_rtt_p50_ms {
put("rttidle50", format!("{v:.0}"));
}
if let Some(v) = net.loaded_rtt_ms { if let Some(v) = net.loaded_rtt_ms {
put("rttload", format!("{v:.0}")); put("rttload", format!("{v:.0}"));
} }