From 683b52d334dbd58f322e98d71a539fce2d57ee92 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Thu, 13 Aug 2026 00:24:42 +0700 Subject: [PATCH] feat(server): add lifetime negotiation policy --- crates/irtt-server/src/clock.rs | 15 +- crates/irtt-server/src/config.rs | 171 +++++++++++++++++++- crates/irtt-server/src/lib.rs | 5 +- crates/irtt-server/src/negotiate.rs | 55 ++++++- crates/irtt-server/src/tests/mod.rs | 1 + crates/irtt-server/src/tests/negotiation.rs | 157 ++++++++++++++++++ crates/irtt-server/src/tests/no_test.rs | 6 +- crates/irtt-server/src/tests/open.rs | 28 ++-- crates/irtt-server/src/tests/params.rs | 14 +- crates/irtt-server/src/tests/support.rs | 19 ++- 10 files changed, 437 insertions(+), 34 deletions(-) create mode 100644 crates/irtt-server/src/tests/negotiation.rs diff --git a/crates/irtt-server/src/clock.rs b/crates/irtt-server/src/clock.rs index 8a5b424..b977632 100644 --- a/crates/irtt-server/src/clock.rs +++ b/crates/irtt-server/src/clock.rs @@ -125,11 +125,18 @@ fn wall_ns() -> i64 { /// A duration as nanoseconds, saturating rather than panicking. /// +/// This is the crate's one [`Duration`] → wire-nanoseconds conversion, used for +/// clock readings and for the configured intervals and deadlines that +/// negotiation and lifetime policy compare against them. Everything on both +/// sides is signed nanoseconds, so there is no second rule anywhere. +/// /// [`Duration`] counts nanoseconds in `u128` and reaches far beyond `i64`, -/// which the wire field is. A host clock set past the year 2262 is a local -/// misconfiguration; it must not become a panic that a remote peer can reach by -/// sending an echo request. -fn saturating_ns(duration: Duration) -> i64 { +/// which the wire field is. A host clock set past the year 2262, or an operator +/// configuring a millennium-long idle timeout, is a local matter; neither may +/// become a panic that a remote peer can reach by sending a packet. Saturating +/// loses nothing that could have been honored, since no wire interval or +/// duration can express more than `i64::MAX` nanoseconds either. +pub(crate) fn saturating_ns(duration: Duration) -> i64 { i64::try_from(duration.as_nanos()).unwrap_or(i64::MAX) } diff --git a/crates/irtt-server/src/config.rs b/crates/irtt-server/src/config.rs index abd9213..2cbd84c 100644 --- a/crates/irtt-server/src/config.rs +++ b/crates/irtt-server/src/config.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + /// Default bound on the number of simultaneously live sessions. /// /// This is `irtt-rs` resource policy, not an interoperability requirement. The @@ -26,18 +28,46 @@ pub const DEFAULT_MAX_SESSIONS: usize = 1024; /// allocation bounded, not to predict fragmentation. pub const DEFAULT_MAX_PACKET_LENGTH: usize = 65_507; +/// Default floor on the send interval a session may negotiate, and the cadence +/// its reply allowance replenishes at. +/// +/// This matches the reference server's default policy. It is policy on both +/// sides: the server specification lists the minimum send interval among the +/// reference server's freely changeable defaults, not among the values the wire +/// format fixes. +pub const DEFAULT_MIN_SEND_INTERVAL: Duration = Duration::from_millis(10); + +/// Default number of echo requests a session may have answered before its +/// allowance has to replenish. +/// +/// This matches the reference server's default policy, and like it, the +/// allowance is per session: one session's burst is not another's. +pub const DEFAULT_BURST_ALLOWANCE: u32 = 5; + +/// Default idle lifetime of a session. +/// +/// A session that goes this long without an echo request is released. This +/// matches the reference server's default of one minute, but the deadline it +/// measures is deliberately not upstream's: `irtt-rs` expires immediately at the +/// deadline, applies it to sessions that have never carried an echo request, and +/// emits no final reply. See [`ServerConfig::with_idle_timeout`]. +pub const DEFAULT_IDLE_TIMEOUT: Duration = Duration::from_secs(60); + /// Configuration for a [`ServerCore`](crate::ServerCore). /// /// Fields are private and set through consuming builder methods, so later -/// slices can add configuration without breaking construction. Only the -/// settings this OPEN/session slice actually uses exist yet; the remaining -/// parameter restriction knobs, lifetime policy and rate limits arrive with the -/// slices that implement them. +/// slices can add configuration without breaking construction. The remaining +/// parameter restriction knobs — the timestamp allowance, DSCP policy and the +/// server-fill allow-list — arrive with the slices that implement them. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ServerConfig { hmac_key: Option>, max_sessions: usize, max_packet_length: usize, + min_send_interval: Duration, + burst_allowance: u32, + idle_timeout: Duration, + max_test_duration: Option, } impl ServerConfig { @@ -90,6 +120,97 @@ impl ServerConfig { self } + /// Sets the floor on the send interval a session may negotiate, which is + /// also the cadence its reply allowance replenishes at. + /// + /// A requested interval below this is raised to it during negotiation, and + /// an absent Interval — which means the wire default of zero, not an + /// explicit zero, which is refused — is answered with it. The negotiated + /// value may still end up *below* this floor, because the idle-timeout cap + /// of [`with_idle_timeout`](Self::with_idle_timeout) is applied afterwards; + /// where that happens, the session's allowance replenishes at the shorter + /// negotiated interval instead, so this server never enforces a cadence + /// slower than the one it handed the client. + /// + /// [`Duration::ZERO`] disables both: no interval is raised, and an + /// otherwise admissible echo never waits for allowance. It does **not** + /// disable the burst allowance, which refuses everything when it is zero. + #[must_use] + pub fn with_min_send_interval(mut self, min_send_interval: Duration) -> Self { + self.min_send_interval = min_send_interval; + self + } + + /// Sets how many echo requests a session may have answered before its + /// allowance has to replenish. + /// + /// The allowance is per session, starts full, is spent one unit per served + /// echo and refills at one unit per + /// [`min_send_interval`](Self::min_send_interval), never above this value. + /// An echo arriving with no allowance is dropped without a reply and + /// without advancing any reception statistic. + /// + /// Zero is accepted and means no allowance at all: every echo is + /// rate-limited, whatever the interval is set to. It is deliberately not a + /// synonym for unlimited. + #[must_use] + pub fn with_burst_allowance(mut self, burst_allowance: u32) -> Self { + self.burst_allowance = burst_allowance; + self + } + + /// Sets how long a session may go without a served or rate-limited echo + /// request before it is released. + /// + /// The deadline runs from the open, not from the first echo, so a session + /// that never carries one still ages out. Release is silent — the protocol + /// defines no expiry notification — and immediate at the deadline. + /// + /// This additionally caps the negotiated send interval at a quarter of the + /// timeout, so a client sending at the interval it was given cannot idle + /// itself out. + /// + /// [`Duration::ZERO`] is accepted and is not a synonym for "never expire": + /// it releases a session the next time expiry is evaluated. There is + /// deliberately no unbounded setting. + #[must_use] + pub fn with_idle_timeout(mut self, idle_timeout: Duration) -> Self { + self.idle_timeout = idle_timeout; + self + } + + /// Sets the maximum test duration a session may negotiate. + /// + /// A requested Duration above this is reduced to it, and a *continuous* + /// request — an absent Duration, meaning the wire default of zero — is + /// answered with it too, since restricting continuous mode to a finite test + /// is the whole point of configuring a maximum. An explicit zero Duration + /// is refused before negotiation and never reaches this. + /// + /// [`Duration::ZERO`] means *no maximum*, exactly as + /// [`without_max_test_duration`](Self::without_max_test_duration) does. A + /// finite maximum of zero could only be honored by negotiating a Duration of + /// zero, which on the wire means continuous — the opposite of a limit — so + /// rewriting a client's finite request into a continuous one is refused + /// rather than encoded. + #[must_use] + pub fn with_max_test_duration(mut self, max_test_duration: Duration) -> Self { + self.max_test_duration = (!max_test_duration.is_zero()).then_some(max_test_duration); + self + } + + /// Removes any configured maximum test duration, which is the default. + /// + /// A session then negotiates the duration it asked for, including + /// continuous, and no server-initiated close is ever triggered by duration. + /// The session table bound and the idle timeout still apply, so this is not + /// an unbounded setting. + #[must_use] + pub fn without_max_test_duration(mut self) -> Self { + self.max_test_duration = None; + self + } + /// The configured HMAC key, if this server authenticates. #[must_use] pub fn hmac_key(&self) -> Option<&[u8]> { @@ -107,16 +228,56 @@ impl ServerConfig { pub fn max_packet_length(&self) -> usize { self.max_packet_length } + + /// The floor on a negotiated send interval, and the allowance refill + /// cadence. + #[must_use] + pub fn min_send_interval(&self) -> Duration { + self.min_send_interval + } + + /// How many echo requests a session may have answered before its allowance + /// has to replenish. + #[must_use] + pub fn burst_allowance(&self) -> u32 { + self.burst_allowance + } + + /// How long a session may go without an echo request before it is released. + #[must_use] + pub fn idle_timeout(&self) -> Duration { + self.idle_timeout + } + + /// The maximum test duration a session may negotiate, if one is configured. + /// + /// Never `Some(Duration::ZERO)`: a zero maximum is stored as `None`, since + /// a negotiated Duration of zero means continuous rather than instant. + #[must_use] + pub fn max_test_duration(&self) -> Option { + self.max_test_duration + } } impl Default for ServerConfig { /// An unauthenticated server bounded to [`DEFAULT_MAX_SESSIONS`] sessions, - /// each negotiating at most [`DEFAULT_MAX_PACKET_LENGTH`] bytes per echo. + /// each negotiating at most [`DEFAULT_MAX_PACKET_LENGTH`] bytes per echo, + /// no faster than [`DEFAULT_MIN_SEND_INTERVAL`] with a burst of + /// [`DEFAULT_BURST_ALLOWANCE`], and released after + /// [`DEFAULT_IDLE_TIMEOUT`] without an echo request. + /// + /// No maximum test duration is configured, matching the reference server's + /// ordinary policy. A session is still bounded, by the table size and the + /// idle timeout. fn default() -> Self { Self { hmac_key: None, max_sessions: DEFAULT_MAX_SESSIONS, max_packet_length: DEFAULT_MAX_PACKET_LENGTH, + min_send_interval: DEFAULT_MIN_SEND_INTERVAL, + burst_allowance: DEFAULT_BURST_ALLOWANCE, + idle_timeout: DEFAULT_IDLE_TIMEOUT, + max_test_duration: None, } } } diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs index eec7b7c..a93d657 100644 --- a/crates/irtt-server/src/lib.rs +++ b/crates/irtt-server/src/lib.rs @@ -53,6 +53,9 @@ mod token; #[cfg(test)] mod tests; -pub use config::{ServerConfig, DEFAULT_MAX_PACKET_LENGTH, DEFAULT_MAX_SESSIONS}; +pub use config::{ + ServerConfig, DEFAULT_BURST_ALLOWANCE, DEFAULT_IDLE_TIMEOUT, DEFAULT_MAX_PACKET_LENGTH, + DEFAULT_MAX_SESSIONS, DEFAULT_MIN_SEND_INTERVAL, +}; pub use core::ServerCore; pub use error::ServerError; diff --git a/crates/irtt-server/src/negotiate.rs b/crates/irtt-server/src/negotiate.rs index 21283b5..a776ecb 100644 --- a/crates/irtt-server/src/negotiate.rs +++ b/crates/irtt-server/src/negotiate.rs @@ -1,12 +1,10 @@ use irtt_proto::{Params, PROTOCOL_VERSION}; -use crate::config::ServerConfig; +use crate::{clock::saturating_ns, config::ServerConfig}; /// Produces the parameters the server returns in an open reply and will enforce /// for the session. /// -/// Two restrictions apply so far. -/// /// **Protocol version is rewritten to [`PROTOCOL_VERSION`].** A server must /// return version 1 so that version-1 clients accept the session; any requested /// value, including an absent, zero, negative or future one, is accepted and @@ -21,11 +19,31 @@ use crate::config::ServerConfig; /// floors echo sizing at, so there is nothing to reduce and rewriting them would /// answer with a session the client did not ask for. /// -/// Those two are the whole of the current policy. A server *may* reduce or -/// replace requested parameters, and later policy slices will add the maximum -/// test duration clamp, -/// minimum interval, idle-timeout interval cap, timestamp allowance, DSCP policy -/// and server-fill allow-list. +/// **Duration is reduced to the configured maximum test duration**, when one is +/// configured. A *continuous* request — Duration absent, which is the wire +/// default of zero — is answered with the maximum as well, because restricting +/// continuous mode to a finite test is exactly what configuring a maximum asks +/// for; the client models that as an ordinary duration reduction. An explicit +/// zero or negative Duration never reaches here, being refused during request +/// admission, and a configured maximum is never zero, so a finite request can +/// never be rewritten into a continuous session. +/// +/// **Interval is raised to the configured minimum and then capped at a quarter +/// of the idle timeout.** The floor bounds how fast a session may ask to be +/// answered; the cap keeps a well-behaved client from idling itself out by +/// sending at the very interval it was given. An absent Interval takes the floor +/// like any other value below it, since absence means the wire default of zero +/// rather than a refusal — an explicit zero is refused during request admission. +/// +/// The two can disagree: a configured minimum above a quarter of the idle +/// timeout ends with the cap winning, and the returned interval below the +/// configured minimum. That is deliberate, and `rate` is where the consequence +/// is settled — the session's allowance then replenishes at the *negotiated* +/// interval, so this server never enforces a cadence slower than the one it just +/// handed the client. +/// +/// Later policy slices will add the timestamp allowance, DSCP policy and the +/// server-fill allow-list. /// /// Negotiation is not by itself an acknowledgement: the core validates what this /// function returns and silently discards an open whose effective parameters it @@ -46,5 +64,26 @@ pub(crate) fn negotiate_params(mut requested: Params, config: &ServerConfig) -> requested.length = max_length; } + if let Some(maximum) = config.max_test_duration() { + // Never zero — `with_max_test_duration` stores a zero maximum as no + // maximum — so this cannot turn a finite request into a continuous one. + let maximum = saturating_ns(maximum); + if requested.duration_ns == 0 || requested.duration_ns > maximum { + requested.duration_ns = maximum; + } + } + + // Floor first, cap second. The order is what lets the cap produce a + // negotiated interval below the configured minimum, which is the case the + // rate limiter is written to honor rather than contradict. + let minimum = saturating_ns(config.min_send_interval()); + if requested.interval_ns < minimum { + requested.interval_ns = minimum; + } + let idle_cap = saturating_ns(config.idle_timeout()) / 4; + if idle_cap > 0 && requested.interval_ns > idle_cap { + requested.interval_ns = idle_cap; + } + requested } diff --git a/crates/irtt-server/src/tests/mod.rs b/crates/irtt-server/src/tests/mod.rs index f9a4f60..b5bafdb 100644 --- a/crates/irtt-server/src/tests/mod.rs +++ b/crates/irtt-server/src/tests/mod.rs @@ -15,6 +15,7 @@ mod admission; mod close; mod echo; mod hmac; +mod negotiation; mod no_test; mod open; mod params; diff --git a/crates/irtt-server/src/tests/negotiation.rs b/crates/irtt-server/src/tests/negotiation.rs new file mode 100644 index 0000000..3f1ca44 --- /dev/null +++ b/crates/irtt-server/src/tests/negotiation.rs @@ -0,0 +1,157 @@ +//! Parameter restrictions the server applies to an open request. +//! +//! Every case drives a real open through the core and reads the restriction out +//! of the open reply, because the reply is the contract: a client enforces its +//! own restriction policy against exactly these values, and the server has to +//! honor what it returned. + +use std::time::Duration; + +use irtt_proto::Params; + +use super::support::{ + client_params, core_with_tokens, expect_normal_open_reply, open_request, peer, ScriptedTokens, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x2f45_9a01_c37d_6e88; + +/// Negotiates one open against `config` and returns the restricted parameters. +fn negotiated(config: ServerConfig, requested: &Params) -> Params { + let mut core = core_with_tokens(config, ScriptedTokens::new([TOKEN_A])); + let packet = core + .handle_datagram(peer(), &open_request(requested, None)) + .unwrap() + .expect("the open must be answered"); + expect_normal_open_reply(&packet, None).params +} + +fn requesting(duration_ns: i64, interval_ns: i64) -> Params { + Params { + duration_ns, + interval_ns, + ..client_params() + } +} + +#[test] +fn a_requested_duration_is_reduced_to_the_configured_maximum() { + let config = || ServerConfig::default().with_max_test_duration(Duration::from_secs(2)); + + assert_eq!( + negotiated(config(), &requesting(10_000_000_000, 1_000_000_000)).duration_ns, + 2_000_000_000, + "a longer duration is reduced to the maximum" + ); + assert_eq!( + negotiated(config(), &requesting(1_000_000_000, 1_000_000_000)).duration_ns, + 1_000_000_000, + "a duration under the maximum is left alone" + ); + // Duration zero is absent from the request, which means continuous. A + // server with a finite maximum is restricting continuous mode to a finite + // test, which is a reduction the client already models. + assert_eq!( + negotiated(config(), &requesting(0, 1_000_000_000)).duration_ns, + 2_000_000_000, + "a continuous request takes the maximum" + ); +} + +#[test] +fn without_a_configured_maximum_a_duration_is_never_restricted() { + let config = || ServerConfig::default(); + + assert_eq!( + negotiated(config(), &requesting(86_400_000_000_000, 1_000_000_000)).duration_ns, + 86_400_000_000_000, + "a day-long test is the operator's business, not negotiation's" + ); + assert_eq!( + negotiated(config(), &requesting(0, 1_000_000_000)).duration_ns, + 0, + "continuous stays continuous" + ); + + // A zero maximum is stored as no maximum rather than negotiated: Duration + // zero on the wire means continuous, so a finite maximum of zero could only + // be expressed by turning a client's finite test into an endless one. + let zero = ServerConfig::default().with_max_test_duration(Duration::ZERO); + assert_eq!(zero.max_test_duration(), None); + assert_eq!( + negotiated(zero, &requesting(1_000_000_000, 1_000_000_000)).duration_ns, + 1_000_000_000 + ); +} + +#[test] +fn a_requested_interval_is_raised_to_the_configured_minimum() { + let config = || ServerConfig::default().with_min_send_interval(Duration::from_millis(100)); + + assert_eq!( + negotiated(config(), &requesting(3_000_000_000, 20_000_000)).interval_ns, + 100_000_000, + "a faster interval is raised to the minimum" + ); + assert_eq!( + negotiated(config(), &requesting(3_000_000_000, 500_000_000)).interval_ns, + 500_000_000, + "an interval above the minimum is left alone" + ); + // An absent Interval is the wire default zero, not a refusal — an explicit + // zero is rejected during request admission and never reaches negotiation — + // so it is raised like any other value below the floor. + assert_eq!( + negotiated(config(), &requesting(3_000_000_000, 0)).interval_ns, + 100_000_000, + "an absent interval takes the minimum" + ); +} + +#[test] +fn a_negotiated_interval_is_capped_at_a_quarter_of_the_idle_timeout() { + // Without the cap, a client sending at the interval it was handed would + // idle its own session out. + let config = ServerConfig::default().with_idle_timeout(Duration::from_secs(8)); + + assert_eq!( + negotiated(config, &requesting(3_000_000_000, 5_000_000_000)).interval_ns, + 2_000_000_000 + ); +} + +#[test] +fn the_idle_cap_wins_over_a_larger_configured_minimum_interval() { + // The floor is applied first and the cap second, so a minimum above a + // quarter of the idle timeout produces a negotiated interval below that + // minimum. This is the configuration the specification records as an + // upstream hazard: upstream returns 2 s here and still replenishes reply + // allowance every 5 s, so a client obeying the reply is rate-limited. + // + // `irtt-rs` returns the same 2 s — the cap has to win, or the session + // cannot stay alive — and refills at 2 s instead. `rate` proves the + // enforcement half; this pins the value the client is told. + let config = ServerConfig::default() + .with_min_send_interval(Duration::from_secs(5)) + .with_idle_timeout(Duration::from_secs(8)); + + assert_eq!( + negotiated(config, &requesting(3_000_000_000, 10_000_000)).interval_ns, + 2_000_000_000 + ); +} + +#[test] +fn a_zero_idle_timeout_caps_nothing() { + // A zero timeout expires a session at the first evaluation, so there is no + // liveness to protect and a quarter of zero must not be read as a cap of + // zero — which would negotiate an interval the client would reject. + let config = ServerConfig::default() + .with_idle_timeout(Duration::ZERO) + .with_min_send_interval(Duration::from_millis(100)); + + assert_eq!( + negotiated(config, &requesting(3_000_000_000, 3_600_000_000_000)).interval_ns, + 3_600_000_000_000 + ); +} diff --git a/crates/irtt-server/src/tests/no_test.rs b/crates/irtt-server/src/tests/no_test.rs index 06bd90d..9c70597 100644 --- a/crates/irtt-server/src/tests/no_test.rs +++ b/crates/irtt-server/src/tests/no_test.rs @@ -2,7 +2,7 @@ use irtt_proto::{verify_packet_hmac, Clock, Params, StampAt}; use super::support::{ client_params, core_with_tokens, expect_no_test_reply, no_test_request, open_request, - open_request_with_raw_params, param_int, peer, ScriptedTokens, KEY, + open_request_with_raw_params, param_int, peer, unthrottled, ScriptedTokens, KEY, }; use crate::ServerConfig; @@ -200,7 +200,9 @@ fn an_authenticated_no_test_reply_is_authenticated_and_still_creates_nothing() { #[test] fn an_empty_no_test_open_is_answered() { - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + // No interval floor, so the reply shows the wire defaults an empty payload + // decoded to rather than the timing policy negotiation would apply to them. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let mut request = open_request_with_raw_params(&[], None); request[3] |= irtt_proto::FLAG_CLOSE; diff --git a/crates/irtt-server/src/tests/open.rs b/crates/irtt-server/src/tests/open.rs index 6bcd4eb..3cf7c35 100644 --- a/crates/irtt-server/src/tests/open.rs +++ b/crates/irtt-server/src/tests/open.rs @@ -2,7 +2,7 @@ use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt}; use super::support::{ client_params, core_with_tokens, expect_normal_open_reply, open_request, - open_request_with_raw_params, param_int, peer, ScriptedTokens, KEY, + open_request_with_raw_params, param_int, peer, unthrottled, ScriptedTokens, KEY, }; use crate::{ServerConfig, DEFAULT_MAX_PACKET_LENGTH}; @@ -84,7 +84,11 @@ fn any_requested_protocol_version_is_accepted_and_answered_with_one() { #[test] fn an_empty_parameter_payload_is_answered_with_only_a_version() { - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + // The interval floor is disabled here so that the reply carries nothing but + // the version: with one configured, an absent Interval negotiates up to it + // and is encoded, which is interval policy rather than what this test is + // about. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let packet = core .handle_datagram(peer(), &open_request_with_raw_params(&[], None)) @@ -114,7 +118,7 @@ fn an_empty_parameter_payload_is_answered_with_only_a_version() { #[test] fn unknown_parameter_tags_are_ignored_and_not_reflected_in_the_reply() { - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let mut payload = param_int(1, 1); payload.extend_from_slice(¶m_int(42, 7)); @@ -187,12 +191,14 @@ fn selecting_timestamps_without_a_clock_is_silently_refused() { #[test] fn current_non_length_parameters_survive_negotiation_unchanged() { - // Duration, interval, statistics, timestamps, DSCP and server fill are the - // values a later restriction-policy slice will deliberately start changing. - // Pinning them now makes that a visible decision rather than an accident. - // Version and an oversized packet length are already restricted, and have - // their own tests; the length here is deliberately under the default - // maximum so that this test says nothing about the packet-length policy. + // Statistics, timestamps, DSCP and server fill are the values a later + // restriction-policy slice will deliberately start changing. Pinning them + // now makes that a visible decision rather than an accident. Version, an + // oversized packet length, the duration maximum and the interval floor and + // cap are already restricted and have their own tests; the length here is + // deliberately under the default maximum, and the configuration below + // leaves duration and interval unrestricted, so this test says nothing + // about either policy. // // This is about what *negotiation* rewrites, which is a separate question // from whether the core will acknowledge the result: an open whose @@ -212,7 +218,9 @@ fn current_non_length_parameters_survive_negotiation_unchanged() { }), }; - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + // No duration maximum is configured by default, and the interval floor is + // disabled, so a one-nanosecond interval is left where it was asked for. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let packet = core .handle_datagram(peer(), &open_request(&unrestricted, None)) .unwrap() diff --git a/crates/irtt-server/src/tests/params.rs b/crates/irtt-server/src/tests/params.rs index d2158b0..f665c11 100644 --- a/crates/irtt-server/src/tests/params.rs +++ b/crates/irtt-server/src/tests/params.rs @@ -2,7 +2,7 @@ use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt}; use super::support::{ core_with_tokens, expect_normal_open_reply, open_request, open_request_with_raw_params, - param_int, param_server_fill, peer, ScriptedTokens, + param_int, param_server_fill, peer, unthrottled, ScriptedTokens, }; use crate::ServerConfig; @@ -70,7 +70,12 @@ fn malformed_or_out_of_range_parameters_are_dropped_without_a_session() { fn absent_duration_interval_and_clock_are_accepted_as_wire_defaults() { // The whole reason the decoder reports presence: an omitted tag and one // explicitly encoded as zero share a value but not a verdict. - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + // + // The interval floor is disabled so that the accepted wire default is + // visible in the reply. An absent Interval is a value like any other to + // negotiation, and the default floor would raise it; that is interval + // policy, and `negotiation` tests it. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let packet = core .handle_datagram( @@ -138,7 +143,10 @@ fn valid_optional_parameter_values_survive_unchanged() { }, ), ] { - let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + // No interval floor: these rows are about which *values* survive + // decoding and negotiation, not about the timing policy applied to + // them. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); let packet = core .handle_datagram(peer(), &open_request(&requested, None)) diff --git a/crates/irtt-server/src/tests/support.rs b/crates/irtt-server/src/tests/support.rs index e780cee..2f12853 100644 --- a/crates/irtt-server/src/tests/support.rs +++ b/crates/irtt-server/src/tests/support.rs @@ -2,6 +2,7 @@ use std::{ collections::VecDeque, net::SocketAddr, sync::{Arc, Mutex}, + time::Duration, }; use irtt_proto::{ @@ -20,6 +21,22 @@ use crate::{ pub(crate) const KEY: &[u8] = b"correctkey"; pub(crate) const OTHER_KEY: &[u8] = b"wrongkey"; +/// A default configuration with the minimum send interval disabled. +/// +/// One knob, two consequences, and tests about neither of them want both gone: +/// no requested interval is raised during negotiation, and no otherwise +/// admissible echo ever waits for rate allowance. That keeps a test about +/// parameter pass-through, receive statistics or timestamps from silently +/// becoming a test of interval policy — and, since these tests drive the core as +/// fast as the machine will run, from being rate-limited by a floor they never +/// meant to configure. +/// +/// Tests that *are* about interval negotiation, rate limiting or lifetime build +/// their configuration explicitly. +pub(crate) fn unthrottled() -> ServerConfig { + ServerConfig::default().with_min_send_interval(Duration::ZERO) +} + pub(crate) fn peer() -> SocketAddr { "198.51.100.7:41234".parse().unwrap() } @@ -135,7 +152,7 @@ pub(crate) fn core_with_sources( /// Most behavior has to hold identically with and without authentication, so /// tests loop over both and build the core from the key. pub(crate) fn core_for(hmac_key: Option<&[u8]>, tokens: ScriptedTokens) -> ServerCore { - let mut config = ServerConfig::default(); + let mut config = unthrottled(); if let Some(key) = hmac_key { config = config.with_hmac_key(key); }