From c4bf8959c14dc8b60fad54fdc1b76e991a40d523 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 28 Oct 2026 23:06:59 +0100 Subject: [PATCH 1/4] tcp: an ACK is timed from its newest segment's departure, at the clock's resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With timestamps negotiated, every RTT sample came from the echoed TSval, and TSval ticks once a millisecond, the finest RFC 7323 §5.4 allows. On the T14's 2 ms LAN path that quantum is half the round trip and a quarter of HyStart++'s 4 ms threshold. Without timestamps there was one timed segment per round trip. Each segment of new data now records when it left, up to 64 at once, and an ACK is timed from the newest segment it newly acknowledges (RFC 8985 §6.1), at the stack's own clock. That is also the segment the receiver did not hold waiting for a second, so a delayed ACK inflates the sample less than an echo of the earliest. Any retransmission clears the record (Karn, RFC 6298 §3): the ACK that repairs a hole covers what was held behind it. Such an ACK, and one whose segment was sent while the record was full, falls back to the echoed timestamp, which also times a retransmission (RFC 7323 §4.1). Every sample is weighted by RFC 7323 Appendix G's ExpectedSamples, with and without timestamps. An echo of a TSval never sent is still counted, and the ACK is still timed by its own departure. s_rt_008 now reads the newest segment's 15 ms where the single timer read the first one's 20 ms. s_rt_011 reads its own 30 ms sample. s_rt_017 holds Karn's rule after a fast retransmission, and s_rt_018 a 1.5 ms sample that an echo would read as 1 ms. Without timestamps the single Karn timer gave one sample a round trip, so a round never reached HyStart++'s N_RTT_SAMPLE of 8 and its delay exit was inert. Departures are timed with and without timestamps, so that case now samples once an ACK too (s_rt_019). An expiry also clears the record: a late ACK would time the original across the expiry and undo its backed-off timer (s_pl_002). Closes issues/tcp-samples-rtt-at-a-millisecond.md. Co-Authored-By: Claude Opus 5.5 --- issues/tcp-samples-rtt-at-a-millisecond.md | 36 ------------- system/netstack/shard/tcp/src/conn.rs | 53 +++++++++++-------- system/netstack/shard/tcp/tests/common/mod.rs | 8 +++ system/netstack/shard/tcp/tests/rtt.rs | 47 ++++++++++++++-- 4 files changed, 84 insertions(+), 60 deletions(-) delete mode 100644 issues/tcp-samples-rtt-at-a-millisecond.md diff --git a/issues/tcp-samples-rtt-at-a-millisecond.md b/issues/tcp-samples-rtt-at-a-millisecond.md deleted file mode 100644 index 1b260165be0..00000000000 --- a/issues/tcp-samples-rtt-at-a-millisecond.md +++ /dev/null @@ -1,36 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-28 ---- - -# [tcp] samples the round trip at a millisecond, and without timestamps once a round trip - -With timestamps negotiated, [tcp] takes every RTT sample from the echoed -TSval (`Ts::echo_rtt`, `system/netstack/shard/tcp/src/conn.rs`), and TSval -ticks once a millisecond (`tsval`, `src/lib.rs`). That tick is the finest RFC -7323 §5.4 permits ("1 ms to 1 sec per tick"), so the tick is not what to -change. But a LAN round trip of 1-3 ms reads as 1, 2 or 3 ms. - -- On the T14's wired path, srtt read 4-7 ms at cwnd stops and the base round - trip is about 2 ms. HyStart++'s delay exit (RFC 9406, #1102) compares round - minima against a threshold of at least 4 ms, so a 1 ms quantum is a quarter - of it. -- RFC 6298's timer adds G = 1 ms (`rtt.rs`), so RTO is unaffected below its - 200 ms floor. - -**Without timestamps, HyStart++'s delay exit is inert.** [tcp] then times one -segment per round trip (Karn's single timer, `timing` in `conn.rs`). A round -never reaches RFC 9406's `N_RTT_SAMPLE` of 8, so the initial slow start grows -with L = 8 until a loss. L = 8 is §4.3's value inside HyStart++, whose delay -exit bounds the overshoot, and §4.3 recommends 8 samples a round. ToyOS offers -timestamps in every SYN, so this needs a peer that declines them. - -Nothing has measured an effect. On a 2 ms path a sample can be off by up to -half the round trip. - -**Exit**: samples at the stack's clock resolution, one an ACK, with -timestamps and without. One way is RFC 8985 §6.1's per-segment transmission -time, keyed by sequence and read on the ACK that newly acknowledges a segment -never retransmitted. Timestamps would then only disambiguate retransmissions -and drive PAWS. Host-test the sample against a known delay below 1 ms. diff --git a/system/netstack/shard/tcp/src/conn.rs b/system/netstack/shard/tcp/src/conn.rs index 7bc214618a8..9f9239fef1f 100644 --- a/system/netstack/shard/tcp/src/conn.rs +++ b/system/netstack/shard/tcp/src/conn.rs @@ -3,6 +3,7 @@ //! each transmit opportunity builds. use alloc::boxed::Box; +use alloc::collections::VecDeque; use core::num::NonZeroU16; use core::time::Duration; @@ -348,7 +349,10 @@ pub struct Sync { pub rtx_timer: Option, rto_pending: bool, timeout_rtx: bool, - timing: Option<(Seq, Instant)>, + /// When each segment of new data in flight left, by its end, while none has been resent since + /// it did (RFC 6298 §3, Karn): the samples an ACK gives at the clock's own resolution (RFC 8985 + /// §6.1). At most [`TIMED`]; a segment sent while it is full is not timed. + departures: VecDeque<(Seq, Instant)>, /// RFC 8985 §7: when the loss probe is due, whether it is, the probe in flight (SND.NXT once /// it left and whether it was a retransmission), and whether an RTT sample came since the last. probe_at: Option, @@ -403,6 +407,10 @@ impl SendLimit { } } +/// The departures a connection times at once: one stretch ACK's worth after another for a window +/// of hundreds of segments, at a fixed cost per connection. +const TIMED: usize = 64; + fn us32(n: usize) -> u32 { u32::try_from(n).unwrap_or(u32::MAX) } @@ -449,7 +457,7 @@ impl Sync { rtx_timer: None, rto_pending: false, timeout_rtx: false, - timing: None, + departures: VecDeque::new(), probe_at: None, probe_due: false, probe: None, @@ -831,24 +839,25 @@ impl Sync { } } + /// The ACK's RTT sample: from the departure of the newest segment it newly acknowledges, else + /// from the timestamp it echoes, which also times a retransmission (RFC 7323 §4.1). Either is + /// weighted by RFC 7323 Appendix G's ExpectedSamples. fn sample(&mut self, seg: &In<'_>, ack: Seq, flight: u32, ctx: &mut Ctx<'_>) -> Option { let now = ctx.now; - let (rtt, weight) = match self.ts { - Some(ts) => { - let t = seg.options.timestamps()?; - let Some(rtt) = ts.echo_rtt(t.echo, now) else { - ctx.log.count(Counter::TsEcrInvalid); - return None; - }; - (rtt, flight.div_ceil(self.smss().saturating_mul(2).max(1)).max(1)) - } - None => { - let (_, at) = self.timing.filter(|&(end, _)| ack.at_or_after(end))?; - self.timing = None; - (now.since(at), 1) + let echoed = self.ts.zip(seg.options.timestamps()).and_then(|(ts, t)| { + let rtt = ts.echo_rtt(t.echo, now); + if rtt.is_none() { + ctx.log.count(Counter::TsEcrInvalid); } - }; - self.rtt.sample(rtt, weight); + rtt + }); + let mut departed = None; + while let Some(&(_, at)) = self.departures.front().filter(|&&(end, _)| ack.at_or_after(end)) { + self.departures.pop_front(); + departed = Some(at); + } + let rtt = departed.map(|at| now.since(at)).or(echoed)?; + self.rtt.sample(rtt, flight.div_ceil(self.smss().saturating_mul(2).max(1)).max(1)); self.sampled = true; Some(rtt) } @@ -1148,7 +1157,8 @@ impl Sync { } self.rto_pending = true; self.rtt.back_off(); - self.timing = None; + // An expiry puts every departure in doubt, whether or not its retransmission has left. + self.departures.clear(); (self.probe_at, self.probe_due, self.probe) = (None, false, None); let repeat = core::mem::replace(&mut self.timeout_rtx, true); self.cc.on_timeout(self.tx.flight(), repeat); @@ -1306,12 +1316,13 @@ impl Sync { } if resent { ctx.log.add(Counter::RetransmitBytes, u64::from(old.earlier(end).since(start))); - self.timing = None; + // The ACK that repairs a hole also covers what was sent before it, held behind the hole. + self.departures.clear(); if start == self.tx.una { self.rto_pending = false; } - } else if end.after(old) && self.timing.is_none() && self.ts.is_none() { - self.timing = Some((end, now)); + } else if end.after(old) && self.departures.len() < TIMED { + self.departures.push_back((end, now)); } self.arm(now); if len > 0 && !resent { diff --git a/system/netstack/shard/tcp/tests/common/mod.rs b/system/netstack/shard/tcp/tests/common/mod.rs index 1215f1e14f4..2df47c311fb 100644 --- a/system/netstack/shard/tcp/tests/common/mod.rs +++ b/system/netstack/shard/tcp/tests/common/mod.rs @@ -578,6 +578,14 @@ impl H { outs } + /// Delivers `s` `us` microseconds past `t`, with no timer due before it. + pub fn input_past(&mut self, t: i64, us: u64, s: S) -> Vec { + let at = Instant::from_nanos(self.instant(t).nanos() + us * 1_000); + assert!(at >= self.now && self.tcp.next_deadline().is_none_or(|d| d > at)); + (self.t, self.now) = (t, at); + self.deliver(s) + } + /// Delivers `s` at `t` from the fixture's peer (or `s.from`), then offers an opportunity. pub fn input(&mut self, t: i64, s: S) -> Vec { let mut outs = self.at(t); diff --git a/system/netstack/shard/tcp/tests/rtt.rs b/system/netstack/shard/tcp/tests/rtt.rs index 4093801542c..e4e1c270c2b 100644 --- a/system/netstack/shard/tcp/tests/rtt.rs +++ b/system/netstack/shard/tcp/tests/rtt.rs @@ -42,15 +42,17 @@ fn s_rt_007_karn() { assert_eq!((info.srtt, info.rttvar, info.rto), (Some(ms(10)), Some(std::time::Duration::from_micros(3_750)), ms(200))); } +/// RFC 8985 §6.1: an ACK is timed from the newest segment it newly acknowledges, which the +/// receiver did not hold waiting for a second. #[test] -fn s_rt_008_one_segment_timed() { +fn s_rt_008_the_newest_segment_acknowledged_is_timed() { let mut h = fixture_e(); let (_, outs) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); nothing(&outs); h.send(0, 1460); h.send(5, 1460); h.input(20, seg(5001).ack(3921)); - assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(11_250)), "one 20 ms sample"); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(10_625)), "one 15 ms sample"); } #[test] @@ -72,13 +74,14 @@ fn s_rt_010_a_retransmission_echoed() { assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(12_500)), "a 30 ms sample"); } +/// An echo of a TSval never sent is counted; the ACK is still timed by its segment's departure. #[test] fn s_rt_011_an_echo_from_the_future() { let mut h = fixture_ef(); h.send(0, 100); h.input(30, seg(5001).ack(1101).wnd(512).ts(50_030, 5000)); let info = h.info(); - assert_eq!(info.srtt, Some(ms(10))); + assert_eq!(info.srtt, Some(std::time::Duration::from_micros(12_500))); assert_eq!(info.snd_una.get(), 1101); assert_eq!(h.count(Counter::TsEcrInvalid), 1); } @@ -121,3 +124,41 @@ fn s_rt_016_no_timer_when_all_is_acknowledged() { h.input(10, seg(5001).ack(1101)); assert_eq!(h.info().rtx_timer, None); } + +/// A sample is the departure's own time, finer than the timestamp clock's millisecond: 1.5 ms +/// reads as 1.5 ms where an echoed TSval would say 1 or 2. +#[test] +fn s_rt_018_a_sample_below_the_timestamp_tick() { + let mut h = fixture_ef(); + h.send(0, 100); + h.input_past(1, 500, seg(5001).ack(1101).wnd(512).ts(50_001, 1000)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(8_937))); +} + +/// Karn (RFC 6298 §3): after a fast retransmission, the departures before it time nothing: the +/// ACK that repairs the hole covers segments held behind it. +#[test] +fn s_rt_017_a_retransmission_forgets_the_departures_before_it() { + let mut h = fixture_e(); + assert_eq!(h.send(0, 5840).len(), 4); + for t in 11..=13 { + h.input(t, seg(5001).ack(1001)); + } + assert_eq!(h.count(Counter::FastRecovery), 1); + h.input(20, seg(5001).ack(6841)); + assert_eq!(h.info().srtt, Some(ms(10)), "a segment held behind the hole times nothing"); +} + +/// Without timestamps too, every ACK that newly acknowledges a timed segment is a sample, not one +/// a round trip: HyStart++ can collect its N_RTT_SAMPLE in a round. +#[test] +fn s_rt_019_each_ack_is_a_sample_without_timestamps() { + let mut h = fixture_e(); + let (_, _) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + h.send(0, 1460); + h.send(0, 1460); + h.input(20, seg(5001).ack(2461)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(11_250)), "a 20 ms sample"); + h.input(28, seg(5001).ack(3921)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(13_343)), "and a 28 ms one"); +} From f1a230e0046febd27a0c0ec8f63d35f7d6235abe Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 31 Oct 2026 17:02:11 +0100 Subject: [PATCH 2/4] tcp: the CSS round counter is named for what it counts `tcp.hystart.css.rounds` counted rounds that entered CSS at least once, not RFC 9406's CSS_ROUNDS spent in CSS: rounds 2-5 without a new entry do not count. It is now `tcp.hystart.css.entry-rounds`. The send_limit test that still named the dropped acknowledged-bytes count is renamed for what it holds. Both were #1102's review r2 suggestions. Co-Authored-By: Claude Opus 5.5 --- system/netstack/shard/tcp/src/conn.rs | 2 +- system/netstack/shard/tcp/src/counters.rs | 2 +- system/netstack/shard/tcp/tests/send_limit.rs | 2 +- tests/guest/osbench/src/sys/toyos.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/system/netstack/shard/tcp/src/conn.rs b/system/netstack/shard/tcp/src/conn.rs index 9f9239fef1f..37dad65040b 100644 --- a/system/netstack/shard/tcp/src/conn.rs +++ b/system/netstack/shard/tcp/src/conn.rs @@ -785,7 +785,7 @@ impl Sync { Some(SlowStart::Conservative { first }) => { ctx.log.count(Counter::HyStartCss); if first { - ctx.log.count(Counter::HyStartCssRounds); + ctx.log.count(Counter::HyStartCssEntryRounds); } } Some(SlowStart::Ended) => { diff --git a/system/netstack/shard/tcp/src/counters.rs b/system/netstack/shard/tcp/src/counters.rs index 247016f73b8..f3c3636810d 100644 --- a/system/netstack/shard/tcp/src/counters.rs +++ b/system/netstack/shard/tcp/src/counters.rs @@ -80,7 +80,7 @@ toyos_net_wire::counters! { AcksNotLimited = "tcp.acks.not-limited"; CwndGrownBytes = "tcp.cwnd.grown.bytes"; HyStartCss = "tcp.hystart.css"; - HyStartCssRounds = "tcp.hystart.css.rounds"; + HyStartCssEntryRounds = "tcp.hystart.css.entry-rounds"; HyStartResumed = "tcp.hystart.resumed"; HyStartExits = "tcp.hystart.exits"; HyStartExitCwndBytes = "tcp.hystart.exit.cwnd.bytes"; diff --git a/system/netstack/shard/tcp/tests/send_limit.rs b/system/netstack/shard/tcp/tests/send_limit.rs index dcd4ce49233..90c88310d53 100644 --- a/system/netstack/shard/tcp/tests/send_limit.rs +++ b/system/netstack/shard/tcp/tests/send_limit.rs @@ -98,7 +98,7 @@ fn a_wait_that_ends_in_a_timeout_is_congestions() { /// Each ACK outside recovery is counted by whether cwnd ended the opportunity before it, with /// what it grew cwnd by. #[test] -fn each_ack_is_counted_with_what_it_acknowledged_and_grew() { +fn each_ack_is_counted_by_its_limit_with_what_it_grew() { let mut h = fixture_e(); assert_eq!(h.send(0, 16_060).len(), 10, "cwnd stops the sender with a segment waiting"); assert_eq!(h.input(10, seg(5001).ack(3921)).len(), 1, "the user's data ends the next"); diff --git a/tests/guest/osbench/src/sys/toyos.rs b/tests/guest/osbench/src/sys/toyos.rs index 207ed14dc7f..e3715270c90 100644 --- a/tests/guest/osbench/src/sys/toyos.rs +++ b/tests/guest/osbench/src/sys/toyos.rs @@ -419,7 +419,7 @@ pub fn net_counters() -> Result, String> { "net.tcp.acks.not-limited", "net.tcp.cwnd.grown.bytes", "net.tcp.hystart.css", - "net.tcp.hystart.css.rounds", + "net.tcp.hystart.css.entry-rounds", "net.tcp.hystart.resumed", "net.tcp.hystart.exits", "net.tcp.hystart.exit.cwnd.bytes", From 15fdb0e9b9099e810f3ce03422462a4449f8749c Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 31 Oct 2026 18:21:51 +0100 Subject: [PATCH 3/4] review r1: every segment in flight is timed, and each sample names its source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The departure record held at most 64 segments. On the T14's path about 230-255 are in flight, so roughly three ACKs in four still took the 1 ms echo. An ACK that crossed from the last recorded segment into unrecorded data was also timed from the older entry, which overstated the round trip. The record now holds runs, not segments, and records every send. A send at the same instant as the last joins its run. A send shorter than SMSS also joins a run shorter than SMSS, and is timed from the run's start. So the record is bounded by the flight, at most two runs per SMSS plus one, with no cap that drops a send. An ACK is timed from the newest run it reaches, a partly acknowledged one included. The echo now times only data that a resend or an expiry took out of the record. - `tcp.rtt.departure` and `tcp.rtt.echo` count each sample's source. osbench reads them, so metal can show the share. - s_rt_020 (the review's case): with more than 64 segments in flight, an ACK of only the newest, 0.5 ms after an ACK of the first 64, is timed from its departure, and no sample is an echo. It fails under the old rule. - s_rt_021: a full segment after small sends starts its own run. - The expiry's clear now gives RFC 6298 §5's reason. A backed-off RTO stands until new data is sent and acknowledged. - rtt.rs tests are in order, and s_rt_019 checks set_options' outputs. Co-Authored-By: Claude Opus 5.5 --- system/netstack/shard/tcp/src/conn.rs | 55 +++++++++++++----- system/netstack/shard/tcp/src/counters.rs | 2 + system/netstack/shard/tcp/tests/rtt.rs | 70 +++++++++++++++++++---- tests/guest/osbench/src/sys/toyos.rs | 4 +- 4 files changed, 104 insertions(+), 27 deletions(-) diff --git a/system/netstack/shard/tcp/src/conn.rs b/system/netstack/shard/tcp/src/conn.rs index 37dad65040b..15964defcf6 100644 --- a/system/netstack/shard/tcp/src/conn.rs +++ b/system/netstack/shard/tcp/src/conn.rs @@ -349,10 +349,12 @@ pub struct Sync { pub rtx_timer: Option, rto_pending: bool, timeout_rtx: bool, - /// When each segment of new data in flight left, by its end, while none has been resent since - /// it did (RFC 6298 §3, Karn): the samples an ACK gives at the clock's own resolution (RFC 8985 - /// §6.1). At most [`TIMED`]; a segment sent while it is full is not timed. - departures: VecDeque<(Seq, Instant)>, + /// When the new data in flight left, while none of it has been resent since (RFC 6298 §3, + /// Karn): the samples an ACK gives at the clock's own resolution (RFC 8985 §6.1). Every send + /// is timed. A send at the same instant as the last joins its run, and so does one shorter than + /// SMSS after a run shorter than SMSS. The record then holds at most two runs for each SMSS in + /// flight and one more, and a joined small send is timed from its run's start, never later. + departures: VecDeque, /// RFC 8985 §7: when the loss probe is due, whether it is, the probe in flight (SND.NXT once /// it left and whether it was a retransmission), and whether an RTT sample came since the last. probe_at: Option, @@ -407,9 +409,13 @@ impl SendLimit { } } -/// The departures a connection times at once: one stretch ACK's worth after another for a window -/// of hundreds of segments, at a fixed cost per connection. -const TIMED: usize = 64; +/// `[start, end)` of new data, which left at `at`. +#[derive(Clone, Copy, Debug)] +struct Departure { + start: Seq, + end: Seq, + at: Instant, +} fn us32(n: usize) -> u32 { u32::try_from(n).unwrap_or(u32::MAX) @@ -839,8 +845,9 @@ impl Sync { } } - /// The ACK's RTT sample: from the departure of the newest segment it newly acknowledges, else - /// from the timestamp it echoes, which also times a retransmission (RFC 7323 §4.1). Either is + /// The ACK's RTT sample: from the departure of the newest segment it newly acknowledges, else, + /// for data a resend or an expiry took out of the record, from the timestamp it echoes, which + /// also times a retransmission (RFC 7323 §4.1). Every ACK of new data is a sample, so each is /// weighted by RFC 7323 Appendix G's ExpectedSamples. fn sample(&mut self, seg: &In<'_>, ack: Seq, flight: u32, ctx: &mut Ctx<'_>) -> Option { let now = ctx.now; @@ -852,11 +859,24 @@ impl Sync { rtt }); let mut departed = None; - while let Some(&(_, at)) = self.departures.front().filter(|&&(end, _)| ack.at_or_after(end)) { + while let Some(run) = self.departures.front().copied().filter(|run| ack.after(run.start)) { + departed = Some(run.at); + if ack.before(run.end) { + break; + } self.departures.pop_front(); - departed = Some(at); } - let rtt = departed.map(|at| now.since(at)).or(echoed)?; + let rtt = match (departed, echoed) { + (Some(at), _) => { + ctx.log.count(Counter::RttFromDeparture); + now.since(at) + } + (None, Some(rtt)) => { + ctx.log.count(Counter::RttFromEcho); + rtt + } + (None, None) => return None, + }; self.rtt.sample(rtt, flight.div_ceil(self.smss().saturating_mul(2).max(1)).max(1)); self.sampled = true; Some(rtt) @@ -1157,7 +1177,8 @@ impl Sync { } self.rto_pending = true; self.rtt.back_off(); - // An expiry puts every departure in doubt, whether or not its retransmission has left. + // RFC 6298 §5: after a back-off, a new RTT measurement "can only happen when new data has + // been sent and acknowledged", so the backed-off RTO stands until then. self.departures.clear(); (self.probe_at, self.probe_due, self.probe) = (None, false, None); let repeat = core::mem::replace(&mut self.timeout_rtx, true); @@ -1321,8 +1342,12 @@ impl Sync { if start == self.tx.una { self.rto_pending = false; } - } else if end.after(old) && self.departures.len() < TIMED { - self.departures.push_back((end, now)); + } else if end.after(old) { + let smss = self.smss(); + match self.departures.back_mut() { + Some(run) if run.end == start && (run.at == now || (run.end.since(run.start) < smss && len < smss)) => run.end = end, + _ => self.departures.push_back(Departure { start, end, at: now }), + } } self.arm(now); if len > 0 && !resent { diff --git a/system/netstack/shard/tcp/src/counters.rs b/system/netstack/shard/tcp/src/counters.rs index f3c3636810d..c8a19191b58 100644 --- a/system/netstack/shard/tcp/src/counters.rs +++ b/system/netstack/shard/tcp/src/counters.rs @@ -32,6 +32,8 @@ toyos_net_wire::counters! { TsMissing = "tcp.ts-missing", logged; PawsReject = "tcp.paws-reject"; TsEcrInvalid = "tcp.ts-ecr-invalid"; + RttFromDeparture = "tcp.rtt.departure"; + RttFromEcho = "tcp.rtt.echo"; RstChallenged = "tcp.rst-challenged", logged; SynChallenged = "tcp.syn-challenged", logged; AckOutOfRange = "tcp.ack-out-of-range", logged; diff --git a/system/netstack/shard/tcp/tests/rtt.rs b/system/netstack/shard/tcp/tests/rtt.rs index e4e1c270c2b..3c83c5e97a7 100644 --- a/system/netstack/shard/tcp/tests/rtt.rs +++ b/system/netstack/shard/tcp/tests/rtt.rs @@ -125,16 +125,6 @@ fn s_rt_016_no_timer_when_all_is_acknowledged() { assert_eq!(h.info().rtx_timer, None); } -/// A sample is the departure's own time, finer than the timestamp clock's millisecond: 1.5 ms -/// reads as 1.5 ms where an echoed TSval would say 1 or 2. -#[test] -fn s_rt_018_a_sample_below_the_timestamp_tick() { - let mut h = fixture_ef(); - h.send(0, 100); - h.input_past(1, 500, seg(5001).ack(1101).wnd(512).ts(50_001, 1000)); - assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(8_937))); -} - /// Karn (RFC 6298 §3): after a fast retransmission, the departures before it time nothing: the /// ACK that repairs the hole covers segments held behind it. #[test] @@ -149,12 +139,23 @@ fn s_rt_017_a_retransmission_forgets_the_departures_before_it() { assert_eq!(h.info().srtt, Some(ms(10)), "a segment held behind the hole times nothing"); } +/// A sample is the departure's own time, finer than the timestamp clock's millisecond: 1.5 ms +/// reads as 1.5 ms where an echoed TSval would say 1 or 2. +#[test] +fn s_rt_018_a_sample_below_the_timestamp_tick() { + let mut h = fixture_ef(); + h.send(0, 100); + h.input_past(1, 500, seg(5001).ack(1101).wnd(512).ts(50_001, 1000)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(8_937))); +} + /// Without timestamps too, every ACK that newly acknowledges a timed segment is a sample, not one /// a round trip: HyStart++ can collect its N_RTT_SAMPLE in a round. #[test] fn s_rt_019_each_ack_is_a_sample_without_timestamps() { let mut h = fixture_e(); - let (_, _) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + let (_, outs) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + nothing(&outs); h.send(0, 1460); h.send(0, 1460); h.input(20, seg(5001).ack(2461)); @@ -162,3 +163,50 @@ fn s_rt_019_each_ack_is_a_sample_without_timestamps() { h.input(28, seg(5001).ack(3921)); assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(13_343)), "and a 28 ms one"); } + +/// Fixture EF with a 256 KiB send buffer, and the peer's window at 512 KiB. +fn fixture_ef_deep() -> H { + let mut h = H::with(toyos_net_tcp::Config { send_buffer: 262_144, ..config(65_535) }); + h.start(-10); + h.conn = Some(h.tcp.connect(h.now(), A, Some(port(49152)), ep(B, 80)).unwrap()); + h.transmit(); + h.input(0, seg(5000).ack(1001).syn().wnd(65_535).mss(1460).sackok().ts(50_000, 990).ws(7)); + h +} + +/// RFC 8985 §6.1 for every segment in flight, however many: with more than 64 out, an ACK of +/// only the newest is timed from their departure, 0.5 ms finer than the echo's millisecond. +#[test] +fn s_rt_020_every_segment_in_flight_is_timed() { + let mut h = fixture_ef_deep(); + let mut t = 0; + h.send(t, 262_144); + while h.info().snd_nxt.since(h.info().snd_una) <= 70 * 1448 { + t += 1; + let nxt = h.info().snd_nxt.get(); + h.input(t, seg(5001).ack(nxt).wnd(4096).ts(50_000 + t as u32, 1000 + t as u32 - 1)); + let room = 262_144 - h.info().queued; + h.send(t, room); + } + let (una, nxt) = (h.info().snd_una.get(), h.info().snd_nxt.get()); + let ts = (50_001 + t as u32, 1000 + t as u32); + h.input_past(t, 300, seg(5001).ack(una + 64 * 1448).wnd(4096).ts(ts.0, ts.1)); + let departures = h.count(Counter::RttFromDeparture); + h.input_past(t, 500, seg(5001).ack(nxt).wnd(4096).ts(ts.0, ts.1)); + assert_eq!(h.count(Counter::RttFromDeparture), departures + 1, "the newest segment's departure"); + assert_eq!((h.count(Counter::RttFromEcho), h.count(Counter::TsEcrInvalid)), (0, 0), "every echo was valid, and none timed"); +} + +/// Small sends join one run, timed from its start, which bounds the record by the flight; a full +/// segment after them starts its own: the ACK of both is the segment's 15 ms, not the run's 20. +#[test] +fn s_rt_021_a_segment_after_small_sends_is_timed_from_its_own_departure() { + let mut h = fixture_e(); + let (_, outs) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + nothing(&outs); + h.send(0, 100); + h.send(2, 100); + h.send(5, 1460); + h.input(20, seg(5001).ack(2661)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(10_625)), "one 15 ms sample"); +} diff --git a/tests/guest/osbench/src/sys/toyos.rs b/tests/guest/osbench/src/sys/toyos.rs index e3715270c90..9b3ec140fb8 100644 --- a/tests/guest/osbench/src/sys/toyos.rs +++ b/tests/guest/osbench/src/sys/toyos.rs @@ -400,7 +400,7 @@ pub fn persistent_dir() -> Option<(String, String, String)> { /// What a transfer row reports of the stack beneath it, as netstack's `inspect` answers it: [tcp]'s /// and the card's counters. Those a card does not report are left out. pub fn net_counters() -> Result, String> { - const READ: [&str; 30] = [ + const READ: [&str; 32] = [ "net.tcp.sent.bytes", "net.tcp.retransmit.bytes", "net.tcp.rto", @@ -409,6 +409,8 @@ pub fn net_counters() -> Result, String> { "net.tcp.loss-probe", "net.tcp.loss-probe-recovery", "net.tcp.dsack-rcvd", + "net.tcp.rtt.departure", + "net.tcp.rtt.echo", "net.tcp.send-limited.user.ns", "net.tcp.send-limited.window.ns", "net.tcp.send-limited.congestion.ns", From 417b74b9f87a6cd830e4655ae6ec81ecfe88e911 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 31 Oct 2026 19:24:15 +0100 Subject: [PATCH 4/4] review r2: a run of small writes joins only until an ACK lands in it Small writes joined a run until it reached SMSS, and each ACK inside the run was timed from its first send. A Nagle-off sender of 100 B every 10 ms on a 20 ms path kept one run growing for 140 ms. Its ACKs were then timed 20, 30, ... 160 ms, which inflated srtt, the RTO, the PTO and HyStart++'s round minimum: a regression against main for app-limited flows. A run now keeps its first and last send instants, and whether an ACK has landed in it. - Sub-SMSS joins stop at that ACK. Same-instant joins continue, since they don't blur the run's timing. - An ACK of a whole run is timed from its last send. - An ACK partway into a run that left at one instant is timed from that instant, so a burst's stretch ACKs are all exact. - In a run of several instants, the first partway ACK is timed from the first send. It overstates by less than the joins took, and they ended at that ACK. A later partway ACK is not timed at all. A send's own length no longer matters to the joins: since a whole run is timed from its last send, a full segment after small ones costs nothing. That tightens the bound. Only the oldest run can be both short and closed, so the record holds at most a run per SMSS in flight plus two. A run is 32 bytes. - s_rt_022 (the review's case): 100 B every 10 ms, each ACKed 20 ms later; srtt stays 20 ms. It fails at 15fdb0e9b (21.25 ms at the ACK at 30 ms). - s_rt_023: three stretch ACKs into one burst are each timed. - s_rt_024: a later partway ACK into a run of writes is not timed, and the ACK of the rest is timed from the last write. Co-Authored-By: Claude Opus 5.5 --- system/netstack/shard/tcp/src/conn.rs | 30 +++++++++----- system/netstack/shard/tcp/tests/rtt.rs | 55 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/system/netstack/shard/tcp/src/conn.rs b/system/netstack/shard/tcp/src/conn.rs index 15964defcf6..14012209fce 100644 --- a/system/netstack/shard/tcp/src/conn.rs +++ b/system/netstack/shard/tcp/src/conn.rs @@ -351,9 +351,13 @@ pub struct Sync { timeout_rtx: bool, /// When the new data in flight left, while none of it has been resent since (RFC 6298 §3, /// Karn): the samples an ACK gives at the clock's own resolution (RFC 8985 §6.1). Every send - /// is timed. A send at the same instant as the last joins its run, and so does one shorter than - /// SMSS after a run shorter than SMSS. The record then holds at most two runs for each SMSS in - /// flight and one more, and a joined small send is timed from its run's start, never later. + /// is timed. A send at the same instant as the last joins its run, and so does any send after + /// a run shorter than SMSS until an ACK lands in the run. Only the oldest run can be both short + /// and closed, so the record holds at most a run for each SMSS in flight and two more. An ACK + /// of a whole run is timed from its last send, and one partway into a run that left at one + /// instant from that instant. In a run of several instants, the first ACK partway is timed + /// from its first send, which overstates it by less than the joins took before that ACK ended + /// them; a later partway ACK is not timed. departures: VecDeque, /// RFC 8985 §7: when the loss probe is due, whether it is, the probe in flight (SND.NXT once /// it left and whether it was a retransmission), and whether an RTT sample came since the last. @@ -409,12 +413,14 @@ impl SendLimit { } } -/// `[start, end)` of new data, which left at `at`. +/// `[start, end)` of new data, sent from `first` to `last`; `sampled` once an ACK landed in it. #[derive(Clone, Copy, Debug)] struct Departure { start: Seq, end: Seq, - at: Instant, + first: Instant, + last: Instant, + sampled: bool, } fn us32(n: usize) -> u32 { @@ -859,11 +865,15 @@ impl Sync { rtt }); let mut departed = None; - while let Some(run) = self.departures.front().copied().filter(|run| ack.after(run.start)) { - departed = Some(run.at); + while let Some(run) = self.departures.front_mut().filter(|run| ack.after(run.start)) { if ack.before(run.end) { + if core::mem::replace(&mut run.sampled, true) && run.first != run.last { + return None; + } + departed = Some(run.first); break; } + departed = Some(run.last); self.departures.pop_front(); } let rtt = match (departed, echoed) { @@ -1345,8 +1355,10 @@ impl Sync { } else if end.after(old) { let smss = self.smss(); match self.departures.back_mut() { - Some(run) if run.end == start && (run.at == now || (run.end.since(run.start) < smss && len < smss)) => run.end = end, - _ => self.departures.push_back(Departure { start, end, at: now }), + Some(run) if run.end == start && (run.last == now || (!run.sampled && run.end.since(run.start) < smss)) => { + (run.end, run.last) = (end, now); + } + _ => self.departures.push_back(Departure { start, end, first: now, last: now, sampled: false }), } } self.arm(now); diff --git a/system/netstack/shard/tcp/tests/rtt.rs b/system/netstack/shard/tcp/tests/rtt.rs index 3c83c5e97a7..fcd7246c9fc 100644 --- a/system/netstack/shard/tcp/tests/rtt.rs +++ b/system/netstack/shard/tcp/tests/rtt.rs @@ -210,3 +210,58 @@ fn s_rt_021_a_segment_after_small_sends_is_timed_from_its_own_departure() { h.input(20, seg(5001).ack(2661)); assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(10_625)), "one 15 ms sample"); } + +/// An app-limited sender of small writes: 100 bytes every 10 ms with Nagle off, each ACKed 20 ms +/// after it left. Its writes join a run only until an ACK lands in it, so every sample is the +/// path's 20 ms and srtt stays there, where timing each from its run's first send would grow it. +#[test] +fn s_rt_022_small_periodic_writes_are_each_timed_by_the_path() { + let mut h = H::new(65_535); + h.start(-20); + h.conn = Some(h.tcp.connect(h.now(), A, Some(port(49152)), ep(B, 80)).unwrap()); + h.transmit(); + h.input(0, seg(5000).ack(1001).syn().wnd(65_535).mss(1460)); + let (_, outs) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + nothing(&outs); + for i in 0..30u32 { + let t = i64::from(i) * 10; + if i >= 2 { + h.input(t, seg(5001).ack(1001 + (i - 1) * 100)); + assert_eq!(h.info().srtt, Some(ms(20)), "the ACK at {t} ms"); + } + assert_eq!(h.send(t, 100).len(), 1); + } + assert_eq!(h.count(Counter::RttFromDeparture), 28); +} + +/// A window that left at one instant is one run, and every ACK into it is timed exactly, however +/// many land partway. +#[test] +fn s_rt_023_each_ack_into_a_burst_is_timed() { + let mut h = fixture_e(); + assert_eq!(h.send(0, 14_600).len(), 10); + for (t, segments) in [(10, 3), (11, 6), (12, 10)] { + h.input(t, seg(5001).ack(1001 + segments * 1460)); + } + assert_eq!(h.count(Counter::RttFromDeparture), 3); +} + +/// Small writes at 0, 5 and 8 ms join one run. The ACK of the first is timed from it; the ACK of +/// the second lands where the run no longer says when its newest byte left, and is not timed; the +/// ACK of the rest is timed from the last write: 20 ms each time it is. +#[test] +fn s_rt_024_a_later_ack_partway_into_a_run_of_writes_is_not_timed() { + let mut h = fixture_e(); + let (_, outs) = h.call(0, |tcp, now, id| tcp.set_options(now, id, toyos_net_tcp::Options { nodelay: true, ..Default::default() })); + nothing(&outs); + for t in [0, 5, 8] { + h.send(t, 100); + } + h.input(20, seg(5001).ack(1101)); + let srtt = h.info().srtt; + assert_eq!(srtt, Some(std::time::Duration::from_micros(11_250)), "a 20 ms sample"); + h.input(25, seg(5001).ack(1201)); + assert_eq!((h.info().srtt, h.count(Counter::RttFromDeparture)), (srtt, 1), "no sample"); + h.input(28, seg(5001).ack(1301)); + assert_eq!(h.info().srtt, Some(std::time::Duration::from_micros(12_343)), "a 20 ms sample"); +}