feat(s2n-quic): allow configuring initial RTT (#2047)

* feat(s2n-quic): allow configuring initial RTT

* refactor

* add integration test

* fix tests

* fix path manager test

* fix more tests

* fix more tests

* PR feedback

* Use ensure macro

* update rust nightly toolchain

Author: WesleyRosenblum

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ env:
   RUST_BACKTRACE: 1
   # Pin the nightly toolchain to prevent breakage.
   # This should be occasionally updated.
-  RUST_NIGHTLY_TOOLCHAIN: nightly-2023-06-12
+  RUST_NIGHTLY_TOOLCHAIN: nightly-2023-11-27
   CDN: https://dnglbrstg7yg.cloudfront.net
   # enable unstable features for testing
   S2N_UNSTABLE_CRYPTO_OPT_TX: 100

quic/s2n-quic-core/src/connection/limits.rs

@@ -4,7 +4,7 @@
 use crate::{
     ack,
     event::{api::SocketAddress, IntoEvent},
-    inet, stream,
+    inet, recovery, stream,
     transport::parameters::{
         AckDelayExponent, ActiveConnectionIdLimit, InitialFlowControlLimits, InitialMaxData,
         InitialMaxStreamDataBidiLocal, InitialMaxStreamDataBidiRemote, InitialMaxStreamDataUni,
@@ -65,6 +65,7 @@ pub struct Limits {
     pub(crate) max_handshake_duration: Duration,
     pub(crate) max_keep_alive_period: Duration,
     pub(crate) max_datagram_frame_size: MaxDatagramFrameSize,
+    pub(crate) initial_round_trip_time: Duration,
 }
 
 impl Default for Limits {
@@ -107,6 +108,7 @@ impl Limits {
             max_handshake_duration: MAX_HANDSHAKE_DURATION_DEFAULT,
             max_keep_alive_period: MAX_KEEP_ALIVE_PERIOD_DEFAULT,
             max_datagram_frame_size: MaxDatagramFrameSize::DEFAULT,
+            initial_round_trip_time: recovery::DEFAULT_INITIAL_RTT,
         }
     }
 
@@ -219,6 +221,26 @@ impl Limits {
     );
     setter!(with_max_keep_alive_period, max_keep_alive_period, Duration);
 
+    /// Sets the initial round trip time (RTT) for use in recovery mechanisms prior to
+    /// measuring an actual RTT sample.
+    ///
+    /// This is useful for environments where RTTs are mostly predictable (e.g. data centers)
+    /// and are much lower than the default 333 milliseconds.
+    pub fn with_initial_round_trip_time(
+        mut self,
+        value: Duration,
+    ) -> Result<Self, ValidationError> {
+        ensure!(
+            value >= recovery::MIN_RTT,
+            Err(ValidationError(
+                "provided value must be at least 1 microsecond",
+            ))
+        );
+
+        self.initial_round_trip_time = value;
+        Ok(self)
+    }
+
     // internal APIs
 
     #[doc(hidden)]
@@ -291,6 +313,12 @@ impl Limits {
     pub fn max_keep_alive_period(&self) -> Duration {
         self.max_keep_alive_period
     }
+
+    #[doc(hidden)]
+    #[inline]
+    pub fn initial_round_trip_time(&self) -> Duration {
+        self.initial_round_trip_time
+    }
 }
 
 /// Creates limits for a given connection

quic/s2n-quic-core/src/recovery/rtt_estimator.rs

@@ -50,21 +50,25 @@ pub struct RttEstimator {
 }
 
 impl Default for RttEstimator {
+    /// Creates a new RTT Estimator with default initial values
     fn default() -> Self {
-        RttEstimator::new(Duration::ZERO)
+        RttEstimator::new(DEFAULT_INITIAL_RTT)
     }
 }
 
 impl RttEstimator {
-    /// Creates a new RTT Estimator with default initial values using the given `max_ack_delay`.
+    /// Creates a new RTT Estimator with the given `initial_rtt`
+    ///
+    /// `on_max_ack_delay` must be called when the `max_ack_delay` transport
+    /// parameter is received to initialize the `max_ack_delay` value.
     #[inline]
-    pub fn new(max_ack_delay: Duration) -> Self {
-        Self::new_with_initial(max_ack_delay, DEFAULT_INITIAL_RTT)
+    pub fn new(initial_rtt: Duration) -> Self {
+        Self::new_with_max_ack_delay(Duration::ZERO, initial_rtt)
     }
 
     /// Creates a new RTT Estimator with the provided initial values using the given `max_ack_delay`.
     #[inline]
-    pub fn new_with_initial(max_ack_delay: Duration, initial_rtt: Duration) -> Self {
+    fn new_with_max_ack_delay(max_ack_delay: Duration, initial_rtt: Duration) -> Self {
         debug_assert!(initial_rtt >= MIN_RTT);
         let initial_rtt = initial_rtt.max(MIN_RTT);
 
@@ -91,6 +95,11 @@ impl RttEstimator {
         }
     }
 
+    /// Creates a new RTT Estimator with the `max_ack_delay` from the current instance
+    pub fn for_new_path(&self, initial_rtt: Duration) -> Self {
+        Self::new_with_max_ack_delay(self.max_ack_delay, initial_rtt)
+    }
+
     /// Gets the latest round trip time sample
     #[inline]
     pub fn latest_rtt(&self) -> Duration {
@@ -385,7 +394,8 @@ mod test {
     /// Test the initial values before any RTT samples
     #[test]
     fn initial_rtt_across_spaces() {
-        let rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(10), DEFAULT_INITIAL_RTT);
         assert_eq!(rtt_estimator.min_rtt, DEFAULT_INITIAL_RTT);
         assert_eq!(rtt_estimator.latest_rtt(), DEFAULT_INITIAL_RTT);
         assert_eq!(rtt_estimator.smoothed_rtt(), DEFAULT_INITIAL_RTT);
@@ -407,7 +417,7 @@ mod test {
     /// Test a zero RTT value is treated as 1 µs
     #[test]
     fn zero_rtt_sample() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let mut rtt_estimator = RttEstimator::new(DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
         rtt_estimator.update_rtt(
             Duration::from_millis(10),
@@ -425,17 +435,26 @@ mod test {
         );
     }
 
+    #[test]
+    fn for_new_path() {
+        let mut rtt_estimator = RttEstimator::default();
+        let max_ack_delay = Duration::from_millis(10);
+        rtt_estimator.on_max_ack_delay(max_ack_delay.try_into().unwrap());
+        let new_path_rtt_estimator = rtt_estimator.for_new_path(DEFAULT_INITIAL_RTT);
+        assert_eq!(max_ack_delay, new_path_rtt_estimator.max_ack_delay)
+    }
+
     //= https://www.rfc-editor.org/rfc/rfc9002#section-5.3
     //= type=test
     //# *  MUST use the lesser of the acknowledgement delay and the peer's
     //#    max_ack_delay after the handshake is confirmed;
     #[test]
     fn max_ack_delay() {
         let mut rtt_estimator = RttEstimator::default();
-        assert_eq!(Duration::ZERO, rtt_estimator.max_ack_delay());
+        assert_eq!(Duration::ZERO, rtt_estimator.max_ack_delay);
 
         rtt_estimator.on_max_ack_delay(MaxAckDelay::new(VarInt::from_u8(10)).unwrap());
-        assert_eq!(Duration::from_millis(10), rtt_estimator.max_ack_delay());
+        assert_eq!(Duration::from_millis(10), rtt_estimator.max_ack_delay);
 
         let now = NoopClock.get_time();
         rtt_estimator.update_rtt(
@@ -495,7 +514,8 @@ mod test {
     /// Test several rounds of RTT updates
     #[test]
     fn update_rtt() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(10), DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
         let rtt_sample = Duration::from_millis(500);
         assert!(rtt_estimator.first_rtt_sample.is_none());
@@ -574,7 +594,8 @@ mod test {
     //#    the resulting value is smaller than the min_rtt.
     #[test]
     fn must_not_subtract_acknowledgement_delay_if_result_smaller_than_min_rtt() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(200));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(200), DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
 
         rtt_estimator.min_rtt = Duration::from_millis(500);
@@ -606,7 +627,8 @@ mod test {
     //# the min_rtt.
     #[test]
     fn prior_to_handshake_ignore_if_less_than_min_rtt() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(200));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(200), DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
         let smoothed_rtt = Duration::from_millis(700);
 
@@ -634,7 +656,8 @@ mod test {
     //     of [QUIC-TRANSPORT]);
     #[test]
     fn initial_space() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(10), DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
         let rtt_sample = Duration::from_millis(500);
         rtt_estimator.update_rtt(
@@ -671,7 +694,8 @@ mod test {
     #[test]
     fn persistent_congestion_duration() {
         let max_ack_delay = Duration::from_millis(10);
-        let mut rtt_estimator = RttEstimator::new(max_ack_delay);
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(max_ack_delay, DEFAULT_INITIAL_RTT);
 
         rtt_estimator.smoothed_rtt = Duration::from_millis(100);
         rtt_estimator.rttvar = Duration::from_millis(50);
@@ -703,7 +727,8 @@ mod test {
 
     #[test]
     fn set_min_rtt_to_latest_sample_after_persistent_congestion() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(10), DEFAULT_INITIAL_RTT);
         let now = NoopClock.get_time();
         let mut rtt_sample = Duration::from_millis(500);
         rtt_estimator.update_rtt(
@@ -743,9 +768,8 @@ mod test {
     #[test]
     fn pto_must_be_at_least_k_granularity() {
         let space = PacketNumberSpace::Handshake;
-        let max_ack_delay = Duration::from_millis(0);
         let now = NoopClock.get_time();
-        let mut rtt_estimator = RttEstimator::new(max_ack_delay);
+        let mut rtt_estimator = RttEstimator::new(DEFAULT_INITIAL_RTT);
 
         // Update RTT with the smallest possible sample
         rtt_estimator.update_rtt(
@@ -795,7 +819,8 @@ mod test {
     //# RTT multiplier, is 9/8.
     #[test]
     fn time_threshold_multiplier_equals_nine_eighths() {
-        let mut rtt_estimator = RttEstimator::new(Duration::from_millis(10));
+        let mut rtt_estimator =
+            RttEstimator::new_with_max_ack_delay(Duration::from_millis(10), DEFAULT_INITIAL_RTT);
         rtt_estimator.update_rtt(
             Duration::from_millis(10),
             Duration::from_secs(1),

quic/s2n-quic-core/src/transport/parameters/mod.rs

@@ -215,7 +215,7 @@ where
 }
 
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub struct ValidationError(&'static str);
+pub struct ValidationError(pub(crate) &'static str);
 
 const MAX_ENCODABLE_VALUE: ValidationError =
     ValidationError("provided value exceeds maximum encodable value");

quic/s2n-quic-transport/src/connection/connection_impl.rs

@@ -582,12 +582,11 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
             quic_version: parameters.quic_version,
         };
 
-        // The path manager always starts with a single path containing the known peer and local
-        // connection ids.
-        let rtt_estimator = RttEstimator::default();
+        let rtt_estimator = RttEstimator::new(parameters.limits.initial_round_trip_time());
         // Assume clients validate the server's address implicitly.
         let peer_validated = Self::Config::ENDPOINT_TYPE.is_server();
-
+        // The path manager always starts with a single path containing the known peer and local
+        // connection ids.
         let initial_path = path::Path::new(
             parameters.path_handle,
             parameters.peer_connection_id,
@@ -1152,6 +1151,7 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
             congestion_controller_endpoint,
             path_migration,
             max_mtu,
+            self.limits.initial_round_trip_time(),
             &mut publisher,
         )?;
 

quic/s2n-quic-transport/src/path/manager.rs

@@ -9,6 +9,7 @@ use crate::{
     path::{challenge, Path},
     transmission,
 };
+use core::time::Duration;
 use s2n_quic_core::{
     ack,
     connection::{self, PeerId},
@@ -22,10 +23,7 @@ use s2n_quic_core::{
         Handle as _, Id, MaxMtu,
     },
     random,
-    recovery::{
-        congestion_controller::{self, Endpoint as _},
-        RttEstimator,
-    },
+    recovery::congestion_controller::{self, Endpoint as _},
     stateless_reset,
     time::{timer, Timestamp},
     transport,
@@ -243,6 +241,7 @@ impl<Config: endpoint::Config> Manager<Config> {
         congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
         migration_validator: &mut Config::PathMigrationValidator,
         max_mtu: MaxMtu,
+        initial_rtt: Duration,
         publisher: &mut Pub,
     ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {
         let valid_initial_received = self.valid_initial_received();
@@ -305,6 +304,7 @@ impl<Config: endpoint::Config> Manager<Config> {
             congestion_controller_endpoint,
             migration_validator,
             max_mtu,
+            initial_rtt,
             publisher,
         )
     }
@@ -317,6 +317,7 @@ impl<Config: endpoint::Config> Manager<Config> {
         congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
         migration_validator: &mut Config::PathMigrationValidator,
         max_mtu: MaxMtu,
+        initial_rtt: Duration,
         publisher: &mut Pub,
     ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {
         //= https://www.rfc-editor.org/rfc/rfc9000#section-9
@@ -398,7 +399,7 @@ impl<Config: endpoint::Config> Manager<Config> {
         // estimator for the new path, and they are initialized with initial values,
         // we do not need to reset congestion controller and round-trip time estimator
         // again on confirming the peer's ownership of its new address.
-        let rtt = RttEstimator::new(self.active_path().rtt_estimator.max_ack_delay());
+        let rtt = self.active_path().rtt_estimator.for_new_path(initial_rtt);
         let path_info = congestion_controller::PathInfo::new(&remote_address);
         let cc = congestion_controller_endpoint.new_congestion_controller(path_info);
 

quic/s2n-quic-transport/src/path/manager/fuzz_target.rs

@@ -15,6 +15,7 @@ use s2n_quic_core::{
     inet::{DatagramInfo, ExplicitCongestionNotification},
     random,
     random::testing::Generator,
+    recovery::DEFAULT_INITIAL_RTT,
     time::{testing::Clock, Clock as _},
     transport,
 };
@@ -176,6 +177,7 @@ impl Model {
             &mut Default::default(),
             &mut migration_validator,
             MaxMtu::default(),
+            DEFAULT_INITIAL_RTT,
             &mut publisher,
         ) {
             Ok(_) => {