diff --git a/Cargo.lock b/Cargo.lock index 4d2180f9..9cb552db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2462,7 +2462,7 @@ dependencies = [ [[package]] name = "nesdoctor" -version = "0.1.0" +version = "0.1.1" dependencies = [ "anyhow", "clap", diff --git a/apps/nesdoctor/Cargo.toml b/apps/nesdoctor/Cargo.toml index df0fdacc..aaa0ff92 100644 --- a/apps/nesdoctor/Cargo.toml +++ b/apps/nesdoctor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nesdoctor" -version = "0.1.0" +version = "0.1.1" edition.workspace = true license.workspace = true repository.workspace = true diff --git a/apps/nesdoctor/README.md b/apps/nesdoctor/README.md index 7a256391..6a9b0ed5 100644 --- a/apps/nesdoctor/README.md +++ b/apps/nesdoctor/README.md @@ -62,12 +62,18 @@ their connection adds when it is busy**, and for anything interactive that is th figure that decides it: ``` - upstream 28 Mbps - latency, idle 179 ms - latency, loaded 198 ms - added under load +19 ms grade B + upstream 39 Mbps + latency, idle floor 55 ms + latency, idle typ. 180 ms + 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 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. diff --git a/apps/nesdoctor/src/main.rs b/apps/nesdoctor/src/main.rs index f4cfaecd..2a31cec7 100644 --- a/apps/nesdoctor/src/main.rs +++ b/apps/nesdoctor/src/main.rs @@ -353,7 +353,8 @@ fn print_net(n: &net::NetReport) { .unwrap_or_else(|| "—".into()) }; 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) { println!( " \x1b[2m That is the round trip to the *nearest* major network, so it is a\x1b[0m" diff --git a/apps/nesdoctor/src/net.rs b/apps/nesdoctor/src/net.rs index 0b11f064..dffc7511 100644 --- a/apps/nesdoctor/src/net.rs +++ b/apps/nesdoctor/src/net.rs @@ -55,7 +55,12 @@ const CHUNK: usize = 1 << 20; // 1 MiB per write #[derive(Debug, Serialize)] 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, + /// 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, pub loaded_rtt_ms: Option, pub loaded_rtt_p95_ms: Option, /// Loaded minus idle: the queue, in milliseconds. @@ -77,6 +82,7 @@ impl NetReport { fn unavailable(note: &str) -> Self { Self { idle_rtt_ms: None, + idle_rtt_p50_ms: None, loaded_rtt_ms: None, loaded_rtt_p95_ms: None, bloat_ms: None, @@ -116,8 +122,14 @@ pub fn run() -> NetReport { }; // --- 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( "no TCP handshake completed to 1.1.1.1:443 — a firewall may block it, so the \ latency half could not run", @@ -160,7 +172,19 @@ pub fn run() -> NetReport { let loaded_p50 = percentile(&loaded, 0.50); 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) { (None, _) => "upstream could not be measured (the upload sink was unreachable), so the \ @@ -175,7 +199,8 @@ pub fn run() -> 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_p95_ms: loaded_p95, bloat_ms: bloat, @@ -204,6 +229,7 @@ fn sample_rtt(addr: SocketAddr, n: usize, gap: Duration) -> Vec { out } +/// `p` of 0.0 gives the minimum, which is what the bloat baseline uses. fn percentile(v: &[f64], p: f64) -> Option { if v.is_empty() { return None; diff --git a/apps/nesdoctor/src/report.rs b/apps/nesdoctor/src/report.rs index 1f645f3d..9012bd7b 100644 --- a/apps/nesdoctor/src/report.rs +++ b/apps/nesdoctor/src/report.rs @@ -119,7 +119,17 @@ pub fn verdict(sys: &SysInfo, host: &HostReport, net: &NetReport) -> Verdict { (Some(up), Some(bloat)) => { if up < MIN_UP_MBPS || bloat > MAX_BLOAT_MS { 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 } else { Verdict::HostReady @@ -185,8 +195,10 @@ pub fn summary_line(f_: &Full) -> String { match (net.upstream_mbps, net.bloat_ms, net.grade) { (Some(up), Some(b), Some(g)) => f.push(format!( - "up={up:.0}Mbps rtt={}ms bloat=+{b:.0}ms grade={g}", - net.idle_rtt_ms.map_or("?".into(), |r| format!("{r:.0}")) + "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_p50_ms + .map_or("?".into(), |r| format!("{r:.0}")) )), _ => 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 { 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 { put("rttload", format!("{v:.0}")); }