checked config

quic/s2n-quic-core/src/path/mtu.rs

@@ -36,7 +36,8 @@ pub mod testing {
             Config {
                 max_mtu: max_mtu.try_into().unwrap(),
                 ..Default::default()
-            },
+            }
+            .into(),
             &addr,
         )
     }
@@ -256,6 +257,11 @@ impl<'a> ConnectionInfo<'a> {
 
 /// Creates MTU config for the given connection.
 pub trait Configurator: 'static + Send {
+    /// Provide connection specific MTU config.
+    ///
+    /// By default this uses the MTU config specified on the IO provider. Application
+    /// must ensure that connection specific max_mtu is <= the max_mtu configured on
+    /// the IO provider.
     fn on_connection(&mut self, info: &mtu::ConnectionInfo) -> Result<mtu::Config, MtuError> {
         Ok(info.endpoint_config)
     }
@@ -300,6 +306,39 @@ impl Config {
     pub fn is_valid(&self) -> bool {
         self.base_mtu.0 <= self.initial_mtu.0 && self.initial_mtu.0 <= self.max_mtu.0
     }
+
+    // Check that the Application provided MTU values are valid for this endpoint.
+    pub fn checked(self, info: &mtu::ConnectionInfo) -> Result<CheckedConfig, MtuError> {
+        ensure!(self.is_valid(), Err(MtuError::Validation));
+        // ensure that the max_mtu is <= the value configured on the IO provider.
+        ensure!(
+            u16::from(self.max_mtu) <= u16::from(info.endpoint_config.max_mtu),
+            Err(MtuError::Validation)
+        );
+
+        Ok(CheckedConfig(self))
+    }
+}
+
+// A checked MTU Config created after validating the values from the
+// IO provider against the values from mtu provider.
+#[derive(Copy, Clone, Debug, Default)]
+pub struct CheckedConfig(Config);
+
+impl CheckedConfig {
+    pub fn initial_mtu(&self) -> InitialMtu {
+        self.0.initial_mtu
+    }
+}
+
+// Only allow conversion in testing
+//
+// A CheckedConfig should be created via the Config::checked() in production
+#[cfg(feature = "testing")]
+impl From<Config> for CheckedConfig {
+    fn from(value: Config) -> Self {
+        CheckedConfig(value)
+    }
 }
 
 #[derive(Debug, Default)]
@@ -425,43 +464,47 @@ impl Controller {
     /// max_mtu is the maximum allowed mtu, e.g. for jumbo frames this value is expected to
     /// be over 9000.
     #[inline]
-    pub fn new(config: Config, peer_socket_address: &inet::SocketAddress) -> Self {
-        debug_assert!(config.is_valid(), "Invalid MTU configuration {:?}", config);
+    pub fn new(config: CheckedConfig, peer_socket_address: &inet::SocketAddress) -> Self {
+        debug_assert!(
+            config.0.is_valid(),
+            "Invalid MTU configuration {:?}",
+            config
+        );
 
         //= https://www.rfc-editor.org/rfc/rfc9000#section-14.3
         //# Endpoints SHOULD set the initial value of BASE_PLPMTU (Section 5.1 of
         //# [DPLPMTUD]) to be consistent with QUIC's smallest allowed maximum
         //# datagram size.
-        let base_plpmtu = config.base_mtu.max_datagram_size(peer_socket_address);
-        let max_udp_payload = config.max_mtu.max_datagram_size(peer_socket_address);
+        let base_plpmtu = config.0.base_mtu.max_datagram_size(peer_socket_address);
+        let max_udp_payload = config.0.max_mtu.max_datagram_size(peer_socket_address);
 
         //= https://www.rfc-editor.org/rfc/rfc9000#section-14.1
         //# Datagrams containing Initial packets MAY exceed 1200 bytes if the sender
         //# believes that the network path and peer both support the size that it chooses.
-        let plpmtu = config.initial_mtu.max_datagram_size(peer_socket_address);
-
-        let initial_probed_size = if u16::from(config.initial_mtu) > ETHERNET_MTU - PROBE_THRESHOLD
-        {
-            // An initial MTU was provided within the probe threshold of the Ethernet MTU, so we can
-            // instead try probing for an MTU larger than the Ethernet MTU
-            Self::next_probe_size(plpmtu, max_udp_payload)
-        } else {
-            // The UDP payload size for the most likely MTU is based on standard Ethernet MTU minus
-            // the minimum length IP headers (without IPv4 options or IPv6 extensions) and UPD header
-            let min_ip_header_len = match peer_socket_address {
-                inet::SocketAddress::IpV4(_) => IPV4_MIN_HEADER_LEN,
-                inet::SocketAddress::IpV6(_) => IPV6_MIN_HEADER_LEN,
-            };
-            ETHERNET_MTU - UDP_HEADER_LEN - min_ip_header_len
-        }
-        .min(max_udp_payload);
+        let plpmtu = config.0.initial_mtu.max_datagram_size(peer_socket_address);
+
+        let initial_probed_size =
+            if u16::from(config.0.initial_mtu) > ETHERNET_MTU - PROBE_THRESHOLD {
+                // An initial MTU was provided within the probe threshold of the Ethernet MTU, so we can
+                // instead try probing for an MTU larger than the Ethernet MTU
+                Self::next_probe_size(plpmtu, max_udp_payload)
+            } else {
+                // The UDP payload size for the most likely MTU is based on standard Ethernet MTU minus
+                // the minimum length IP headers (without IPv4 options or IPv6 extensions) and UPD header
+                let min_ip_header_len = match peer_socket_address {
+                    inet::SocketAddress::IpV4(_) => IPV4_MIN_HEADER_LEN,
+                    inet::SocketAddress::IpV6(_) => IPV6_MIN_HEADER_LEN,
+                };
+                ETHERNET_MTU - UDP_HEADER_LEN - min_ip_header_len
+            }
+            .min(max_udp_payload);
 
         Self {
             state: State::Disabled,
             base_plpmtu,
             plpmtu,
             probed_size: initial_probed_size,
-            max_mtu: config.max_mtu,
+            max_mtu: config.0.max_mtu,
             max_udp_payload,
             max_probe_size: max_udp_payload,
             probe_count: 0,

quic/s2n-quic-core/src/path/mtu/tests.rs

@@ -126,7 +126,7 @@ fn base_plpmtu_is_1200() {
     //# default BASE_PLPMTU of 1200 bytes is RECOMMENDED.
     let ip = IpV4Address::new([127, 0, 0, 1]);
     let addr = inet::SocketAddress::IpV4(SocketAddressV4::new(ip, 443));
-    let controller = Controller::new(Config::default(), &addr);
+    let controller = Controller::new(Config::default().into(), &addr);
     assert_eq!(controller.base_plpmtu, 1200);
 }
 
@@ -138,7 +138,8 @@ fn min_max_mtu() {
         Config {
             max_mtu: MaxMtu::MIN,
             ..Default::default()
-        },
+        }
+        .into(),
         &addr.into(),
     );
     assert_eq!(MINIMUM_MAX_DATAGRAM_SIZE, controller.plpmtu);
@@ -166,7 +167,8 @@ fn new_ipv4() {
         Config {
             max_mtu: 1600.try_into().unwrap(),
             ..Default::default()
-        },
+        }
+        .into(),
         &addr.into(),
     );
     assert_eq!(1600_u16, u16::from(controller.max_mtu));
@@ -201,7 +203,8 @@ fn new_ipv6() {
         Config {
             max_mtu: 2000.try_into().unwrap(),
             ..Default::default()
-        },
+        }
+        .into(),
         &addr.into(),
     );
     assert_eq!(2000_u16, u16::from(controller.max_mtu));
@@ -239,7 +242,8 @@ fn new_initial_and_base_mtu() {
             max_mtu: 2600.try_into().unwrap(),
             base_mtu: 1400.try_into().unwrap(),
             initial_mtu: 2500.try_into().unwrap(),
-        },
+        }
+        .into(),
         &addr.into(),
     );
     assert_eq!(2600_u16, u16::from(controller.max_mtu));
@@ -283,7 +287,8 @@ fn new_initial_mtu_less_than_ethernet_mtu() {
             max_mtu: 9000.try_into().unwrap(),
             initial_mtu: 1400.try_into().unwrap(),
             ..Default::default()
-        },
+        }
+        .into(),
         &addr.into(),
     );
     // probe the ethernet MTU
@@ -303,7 +308,8 @@ fn new_initial_mtu_equal_to_ethernet_mtu() {
             max_mtu: 9000.try_into().unwrap(),
             initial_mtu: ETHERNET_MTU.try_into().unwrap(),
             ..Default::default()
-        },
+        }
+        .into(),
         &addr.into(),
     );
     // probe halfway to the max MTU
@@ -750,7 +756,7 @@ fn on_packet_loss_initial_mtu_configured() {
                     initial_mtu: initial_mtu.min(max_mtu).try_into().unwrap(),
                     base_mtu: base_mtu.min(initial_mtu).min(max_mtu).try_into().unwrap(),
                 };
-                let mut controller = Controller::new(mtu_config, &addr);
+                let mut controller = Controller::new(mtu_config.into(), &addr);
                 let base_plpmtu = controller.base_plpmtu;
                 let original_plpmtu = controller.plpmtu;
                 let pn = pn(1);

quic/s2n-quic-transport/src/connection/connection_container/tests.rs

@@ -240,7 +240,7 @@ impl connection::Trait for TestConnection {
         _datagram: &DatagramInfo,
         _congestion_controller_endpoint: &mut <Self::Config as endpoint::Config>::CongestionControllerEndpoint,
         _path_migration: &mut <Self::Config as endpoint::Config>::PathMigrationValidator,
-        _mtu_config: mtu::Config,
+        _mtu_config: mtu::CheckedConfig,
         _subscriber: &mut <Self::Config as endpoint::Config>::EventSubscriber,
     ) -> Result<path::Id, DatagramDropReason> {
         todo!()

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

@@ -1134,7 +1134,7 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
         datagram: &DatagramInfo,
         congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
         path_migration: &mut Config::PathMigrationValidator,
-        mtu_config: mtu::Config,
+        mtu_config: mtu::CheckedConfig,
         subscriber: &mut Config::EventSubscriber,
     ) -> Result<path::Id, DatagramDropReason> {
         let mut publisher = self.event_context.publisher(datagram.timestamp, subscriber);

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

@@ -209,7 +209,7 @@ pub trait ConnectionTrait: 'static + Send + Sized {
         datagram: &DatagramInfo,
         congestion_controller_endpoint: &mut <Self::Config as endpoint::Config>::CongestionControllerEndpoint,
         migration_validator: &mut <Self::Config as endpoint::Config>::PathMigrationValidator,
-        mtu_config: mtu::Config,
+        mtu_config: mtu::CheckedConfig,
         subscriber: &mut <Self::Config as endpoint::Config>::EventSubscriber,
     ) -> Result<path::Id, DatagramDropReason>;
 

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

@@ -73,7 +73,7 @@ pub struct Parameters<'a, Cfg: endpoint::Config> {
     /// The limits that were advertised to the peer
     pub limits: connection::Limits,
     /// Configuration for the maximum transmission unit (MTU) that can be sent on a path
-    pub mtu_config: mtu::Config,
+    pub mtu_config: mtu::CheckedConfig,
     /// The context that should be passed to all related connection events
     pub event_context: <Cfg::EventSubscriber as event::Subscriber>::ConnectionContext,
     /// The context passed to the connection supervisor

quic/s2n-quic-transport/src/endpoint/initial.rs

@@ -169,14 +169,13 @@ impl<Config: endpoint::Config> endpoint::Endpoint<Config> {
             .context()
             .connection_limits
             .on_connection(&LimitsInfo::new(&remote_address));
+        let info = mtu::ConnectionInfo::new(&remote_address, self.endpoint_mtu_limits);
         let mtu_config = self
             .config
             .context()
             .mtu
-            .on_connection(&mtu::ConnectionInfo::new(
-                &remote_address,
-                self.endpoint_mtu_limits,
-            ))
+            .on_connection(&info)
+            .and_then(|config| config.checked(&info))
             .map_err(|_| {
                 // TODO emit datagram dropped event
                 connection::Error::validation("failed to instantiate a valid MTU provider")

quic/s2n-quic-transport/src/endpoint/mod.rs

@@ -555,19 +555,18 @@ impl<Cfg: Config> Endpoint<Cfg> {
             .lookup_internal_connection_id(&destination_connection_id)
         {
             let mut check_for_stateless_reset = false;
-
+            let info = mtu::ConnectionInfo::new(&remote_address, self.endpoint_mtu_limits);
             // TODO can we make this cleaner?
-            let mtu_provider = endpoint_context
+            let mtu_config = endpoint_context
                 .mtu
-                .on_connection(&mtu::ConnectionInfo::new(
-                    &remote_address,
-                    self.endpoint_mtu_limits,
-                ));
-            let mtu_config = if let Ok(mtu_config) = mtu_provider {
-                mtu_config
-            } else {
-                // TODO emit datagram dropped event
-                return;
+                .on_connection(&info)
+                .and_then(|config| config.checked(&info));
+            let mtu_config = match mtu_config {
+                Ok(config) => config,
+                Err(_) => {
+                    // TODO emit datagram dropped event
+                    return;
+                }
             };
 
             datagram.destination_connection_id_classification = dcid_classification;
@@ -1066,16 +1065,15 @@ impl<Cfg: Config> Endpoint<Cfg> {
         let limits = endpoint_context
             .connection_limits
             .on_connection(&LimitsInfo::new(&remote_address));
-        let mtu_provider = endpoint_context
+        let info = mtu::ConnectionInfo::new(&remote_address, self.endpoint_mtu_limits);
+        let mtu_config = endpoint_context
             .mtu
-            .on_connection(&mtu::ConnectionInfo::new(
-                &remote_address,
-                self.endpoint_mtu_limits,
-            ));
-        let mtu_config = mtu_provider.map_err(|_err| {
-            // TODO emit datagram dropped event
-            connection::Error::validation("failed to instantiate a valid MTU provider")
-        })?;
+            .on_connection(&info)
+            .and_then(|config| config.checked(&info))
+            .map_err(|_err| {
+                // TODO emit datagram dropped event
+                connection::Error::validation("failed to instantiate a valid MTU provider")
+            })?;
 
         transport_parameters.load_limits(&limits);
 

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

@@ -244,7 +244,7 @@ impl<Config: endpoint::Config> Manager<Config> {
         handshake_confirmed: bool,
         congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
         migration_validator: &mut Config::PathMigrationValidator,
-        mtu_config: mtu::Config,
+        mtu_config: mtu::CheckedConfig,
         limits: &Limits,
         publisher: &mut Pub,
     ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {
@@ -320,7 +320,7 @@ impl<Config: endpoint::Config> Manager<Config> {
         datagram: &DatagramInfo,
         congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
         migration_validator: &mut Config::PathMigrationValidator,
-        mtu_config: mtu::Config,
+        mtu_config: mtu::CheckedConfig,
         limits: &Limits,
         publisher: &mut Pub,
     ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {

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

@@ -176,7 +176,7 @@ impl Model {
             true,
             &mut Default::default(),
             &mut migration_validator,
-            mtu::Config::default(),
+            mtu::Config::default().into(),
             &Limits::default(),
             &mut publisher,
         ) {