fix(server): order an echo reply's timestamp pair

This commit is contained in:
Nils Andreas Svee committed 2026-08-12 23:47:34 +07:00
1 parent 8abab7ee38
commit cce2ff731a
4 files changed
+94

No files matched your search

+7
View File
@@ -131,6 +131,13 @@ effective parameters must be safe for the server to run.
classified as an echo and the send instant just before the reply is built, so
they bracket the server's own handling. The monotonic origin belongs to the
clock source and is stable for its life.
- A reply's receive instant is held back to its send instant where the wall
clock stepped backwards between the two readings, so the required ordering
holds. That correction is per reply and stateless on purpose: latching a
pre-step wall value across packets would keep reporting a time the host has
corrected away, which is the smoothing the specification forbids, and
anchoring the wall clock to the monotonic origin would drift and never pick up
a legitimate correction. The server reports its own wall clock honestly.
- For a single-clock midpoint, `irtt-rs` emits the one negotiated field. It does
not reproduce upstream 0.9.1's dual-field midpoint; `irtt-proto` still decodes
that form from a peer.
+22
View File
@@ -44,6 +44,28 @@ impl ClockSample {
mono_ns: mean_ns(self.mono_ns, other.mono_ns),
}
}
/// This sample, held back in any domain where it runs ahead of `later`.
///
/// A reply's receive instant must not be later than its send instant. The
/// monotonic domain gets that from its source, which only moves forward.
/// The wall clock does not: it can be stepped backwards between two
/// readings by NTP, a hypervisor or an administrator, which would otherwise
/// invert the pair.
///
/// The *earlier* reading is the one that moves, so the pair settles on the
/// clock as it now stands rather than on a value the clock has already
/// disowned. Nothing is remembered between calls: a latch that carried a
/// pre-step value forward would keep reporting a wall time the host has
/// corrected away, which is the smoothing across packets the specification
/// forbids — and it would make one-way delays wrong for as long as the
/// latch held, rather than for one reply.
pub(crate) fn not_after(self, later: Self) -> Self {
Self {
wall_ns: self.wall_ns.min(later.wall_ns),
mono_ns: self.mono_ns.min(later.mono_ns),
}
}
}
/// A source of clock samples.
+6
View File
@@ -421,7 +421,13 @@ struct EchoRequest {
/// [`Clock::Unspecified`] selects no domain and so produces no field. It cannot
/// be reached with a non-none `StampAt` on an acknowledged session, because
/// that combination is refused at open; nothing here needs to repair it.
///
/// The pair is ordered before anything is built from it, because a reply's
/// receive instant must not be later than its send instant and the wall clock
/// can be stepped backwards between the two readings. See
/// [`ClockSample::not_after`]; the correction is confined to this one reply.
fn timestamp_fields(params: &Params, received: ClockSample, sent: ClockSample) -> TimestampFields {
let received = received.not_after(sent);
let per_clock = |sample: ClockSample| {
(
params.clock.has_wall().then_some(sample.wall_ns),
+59
View File
@@ -594,6 +594,65 @@ fn only_the_negotiated_timestamps_are_emitted() {
}
}
#[test]
fn a_backward_wall_step_never_reports_receive_after_send() {
// A reply's receive instant must not be later than its send instant, and
// the wall clock can be stepped backwards between the two readings by NTP,
// a hypervisor or an administrator. The pair settles on the later reading,
// and only the wall domain moves: the monotonic one was already ordered.
let mut core = core_with_sources(
ServerConfig::default(),
ScriptedTokens::new([TOKEN_A]),
ScriptedClock::new([sample(1_000, 10_000), sample(990, 10_300)]),
);
let params = echo_params(ReceivedStats::None, StampAt::Both, Clock::Both, 0);
let (token, negotiated) = open_negotiated(&mut core, peer(), &params, None);
let packet = core
.handle_datagram(peer(), &echo_request(token, 0, &negotiated, &[], None))
.unwrap()
.expect("a backward wall step must not stop the echo being answered");
let reply = expect_echo_reply(&packet, &negotiated, None);
assert_eq!(
reply.timestamps,
TimestampFields {
recv_wall: Some(990),
recv_mono: Some(10_000),
send_wall: Some(990),
send_mono: Some(10_300),
..TimestampFields::default()
},
"the receive wall value is held back to the send reading, and only it"
);
assert!(reply.timestamps.recv_wall <= reply.timestamps.send_wall);
assert!(reply.timestamps.recv_mono <= reply.timestamps.send_mono);
// A midpoint drawn from the same stepped pair is ordered for the same
// reason: it is the mean of two values that are no longer inverted.
let mut core = core_with_sources(
ServerConfig::default(),
ScriptedTokens::new([TOKEN_A]),
ScriptedClock::new([sample(1_000, 10_000), sample(990, 10_300)]),
);
let params = echo_params(ReceivedStats::None, StampAt::Midpoint, Clock::Wall, 0);
let (token, negotiated) = open_negotiated(&mut core, peer(), &params, None);
let packet = core
.handle_datagram(peer(), &echo_request(token, 0, &negotiated, &[], None))
.unwrap()
.expect("a backward wall step must not stop the echo being answered");
let reply = expect_echo_reply(&packet, &negotiated, None);
assert_eq!(
reply.timestamps,
TimestampFields {
midpoint_wall: Some(990),
..TimestampFields::default()
}
);
}
#[test]
fn a_single_clock_midpoint_emits_exactly_one_field() {
// A guard against reproducing upstream 0.9.1's dual-field midpoint, which