fix(server): order an echo reply's timestamp pair
This commit is contained in:
4 files changed
+94
No files matched your search
@@ -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
|
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
|
they bracket the server's own handling. The monotonic origin belongs to the
|
||||||
clock source and is stable for its life.
|
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
|
- 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
|
not reproduce upstream 0.9.1's dual-field midpoint; `irtt-proto` still decodes
|
||||||
that form from a peer.
|
that form from a peer.
|
||||||
|
|||||||
@@ -44,6 +44,28 @@ impl ClockSample {
|
|||||||
mono_ns: mean_ns(self.mono_ns, other.mono_ns),
|
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.
|
/// A source of clock samples.
|
||||||
|
|||||||
@@ -421,7 +421,13 @@ struct EchoRequest {
|
|||||||
/// [`Clock::Unspecified`] selects no domain and so produces no field. It cannot
|
/// [`Clock::Unspecified`] selects no domain and so produces no field. It cannot
|
||||||
/// be reached with a non-none `StampAt` on an acknowledged session, because
|
/// be reached with a non-none `StampAt` on an acknowledged session, because
|
||||||
/// that combination is refused at open; nothing here needs to repair it.
|
/// 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 {
|
fn timestamp_fields(params: &Params, received: ClockSample, sent: ClockSample) -> TimestampFields {
|
||||||
|
let received = received.not_after(sent);
|
||||||
let per_clock = |sample: ClockSample| {
|
let per_clock = |sample: ClockSample| {
|
||||||
(
|
(
|
||||||
params.clock.has_wall().then_some(sample.wall_ns),
|
params.clock.has_wall().then_some(sample.wall_ns),
|
||||||
|
|||||||
@@ -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(), ¶ms, 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(), ¶ms, 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]
|
#[test]
|
||||||
fn a_single_clock_midpoint_emits_exactly_one_field() {
|
fn a_single_clock_midpoint_emits_exactly_one_field() {
|
||||||
// A guard against reproducing upstream 0.9.1's dual-field midpoint, which
|
// A guard against reproducing upstream 0.9.1's dual-field midpoint, which
|
||||||
|
|||||||
Reference in new issue
Block a user