From 1b390de4ca4d77299d51ba18786883a03b9c2323 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Tue, 11 Aug 2026 21:26:55 +0700 Subject: [PATCH] docs(stats): clarify server receive-window semantics An audit of the server-reported receive window found no defect in the statistics pipeline: the bitmap is decoded raw, stored raw, never interpreted, and never feeds the directional loss estimates, which use the cumulative server receive count. The public rustdoc was silent on all of that, leaving consumers of PacketCounts free to read a window of 0x1 as a 63-packet loss burst. Document the field's contract, name the count as the only server input to LossStats, and cover the window-only case with a regression test. Co-Authored-By: Claude Opus 5 --- crates/irtt-client/src/event.rs | 5 ++++- crates/irtt-stats/src/lib.rs | 14 ++++++++++++- crates/irtt-stats/src/loss.rs | 5 +++++ crates/irtt-stats/tests/common/mod.rs | 16 ++++++++++++++ crates/irtt-stats/tests/stats.rs | 30 ++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 3 deletions(-) diff --git a/crates/irtt-client/src/event.rs b/crates/irtt-client/src/event.rs index 7d2dd13..3f58cbf 100644 --- a/crates/irtt-client/src/event.rs +++ b/crates/irtt-client/src/event.rs @@ -361,7 +361,10 @@ pub struct OneWayDelaySample { pub struct ReceivedStatsSample { /// Server-reported count of received packets, when available. pub count: Option, - /// Server-reported receive window value, when available. + /// Raw server-reported receive window bitmap, when available. + /// + /// The bitmap is preserved exactly as it appeared on the wire; the client + /// does not interpret its bits. pub window: Option, } diff --git a/crates/irtt-stats/src/lib.rs b/crates/irtt-stats/src/lib.rs index e721dc8..b89809a 100644 --- a/crates/irtt-stats/src/lib.rs +++ b/crates/irtt-stats/src/lib.rs @@ -229,8 +229,20 @@ pub struct PacketCounts { /// exposed separately. pub bytes_received: u64, /// Highest cumulative server-reported packets received, when available. + /// + /// This is the only server-reported input to the directional loss estimates + /// in [`LossStats`]. pub server_packets_received: Option, - /// Server-reported receive window, when available. + /// Raw server-reported receive window from the most recently processed + /// reply that carried one, when available. + /// + /// The value is stored exactly as the server reported it and is not + /// interpreted by this crate. It is a bounded bitmap of recent server + /// receive history rather than a fixed 64-packet loss mask, so a value of + /// `0x1` means no earlier history is represented and not that the previous + /// 63 packets were lost. Replies are aggregated in local arrival order, so + /// a late reply can leave an older window here than an already-processed + /// reply carried. pub server_received_window: Option, } diff --git a/crates/irtt-stats/src/loss.rs b/crates/irtt-stats/src/loss.rs index a705f24..579aeaa 100644 --- a/crates/irtt-stats/src/loss.rs +++ b/crates/irtt-stats/src/loss.rs @@ -6,6 +6,11 @@ use crate::PacketCounts; /// Percentage fields are percentages from `0.0` to `100.0` in normal cases. /// Directional upstream and downstream loss can be negative when /// server-reported counts exceed local expectations. +/// +/// The directional fields are derived from +/// [`PacketCounts::server_packets_received`] and are `None` when the server +/// reported no receive count. [`PacketCounts::server_received_window`] is never +/// used to derive loss. pub struct LossStats { /// Locally inferred total lost packets. pub lost_packets: u64, diff --git a/crates/irtt-stats/tests/common/mod.rs b/crates/irtt-stats/tests/common/mod.rs index bb60b2d..731ea71 100644 --- a/crates/irtt-stats/tests/common/mod.rs +++ b/crates/irtt-stats/tests/common/mod.rs @@ -84,6 +84,22 @@ fn reply_with_rtt(seq: u32, raw_ms: u64, rtt: RttSample) -> ClientEvent { } } +/// Builds a normal reply carrying exactly the supplied server received stats. +pub fn reply_with_received_stats( + seq: u32, + raw_ms: u64, + count: Option, + window: Option, +) -> ClientEvent { + let mut event = unadjusted_reply(seq, raw_ms); + let ClientEvent::EchoReply { received_stats, .. } = &mut event else { + unreachable!(); + }; + *received_stats = + (count.is_some() || window.is_some()).then_some(ReceivedStatsSample { count, window }); + event +} + pub fn unadjusted_late_reply(seq: u32, raw_ms: u64) -> ClientEvent { late_reply_from(unadjusted_reply(seq, raw_ms)) } diff --git a/crates/irtt-stats/tests/stats.rs b/crates/irtt-stats/tests/stats.rs index dd487f5..a4a92ed 100644 --- a/crates/irtt-stats/tests/stats.rs +++ b/crates/irtt-stats/tests/stats.rs @@ -2,7 +2,9 @@ mod common; use std::time::{Duration, Instant, UNIX_EPOCH}; -use common::{adjusted_reply, sent, ts, unadjusted_late_reply, unadjusted_reply}; +use common::{ + adjusted_reply, reply_with_received_stats, sent, ts, unadjusted_late_reply, unadjusted_reply, +}; use irtt_client::{ ClientEvent, ClientTimestamp, OneWayDelaySample, PacketMeta, RttSample, SignedDuration, }; @@ -378,6 +380,32 @@ fn empty_and_all_lost_edges_are_defined() { assert_eq!(all_lost.loss.packet_loss_percent, 100.0); } +#[test] +fn receive_window_is_stored_raw_and_never_drives_directional_loss() { + // Window-only negotiation: the server reports a bitmap but no receive count. + let mut collector = StatsCollector::new(StatsConfig::finite()); + collector.process(&sent(0, ts(0))); + collector.process(&reply_with_received_stats(0, 10, None, Some(0x1))); + + // `0x1` represents no earlier history. It must be preserved verbatim and + // must not be read as 63 lost packets or stand in for a receive count. + let snapshot = collector.snapshot(); + assert_eq!(snapshot.packets.server_received_window, Some(0x1)); + assert_eq!(snapshot.packets.server_packets_received, None); + assert_eq!(snapshot.loss.upstream_loss_packets, None); + assert_eq!(snapshot.loss.downstream_loss_packets, None); + assert_eq!(snapshot.loss.upstream_loss_percent, 0.0); + assert_eq!(snapshot.loss.downstream_loss_percent, 0.0); + + // An advancing bitmap is stored verbatim across the full 64-bit range. + collector.process(&sent(1, ts(10))); + collector.process(&reply_with_received_stats(1, 10, None, Some(u64::MAX))); + assert_eq!( + collector.snapshot().packets.server_received_window, + Some(u64::MAX) + ); +} + #[test] fn directional_loss_uses_server_received_count_when_available() { let mut collector = StatsCollector::new(StatsConfig::finite()); -- 2.54.0