From a9d126d9f8e8086aa46400428c12c6af96a4292c Mon Sep 17 00:00:00 2001 From: Matt Watts Date: Sat, 24 Jan 2026 01:46:39 -0600 Subject: [PATCH] feat(s2n-quic-core): add MtuConfigError with full diagnostic context --- quic/s2n-quic-core/events/common.rs | 16 ++++--- quic/s2n-quic-core/src/event/generated.rs | 18 ++++++++ quic/s2n-quic-core/src/path/mtu.rs | 49 ++++++++++++++++++--- quic/s2n-quic-core/src/path/mtu/tests.rs | 12 ++++- quic/s2n-quic-transport/src/path/manager.rs | 6 ++- 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/quic/s2n-quic-core/events/common.rs b/quic/s2n-quic-core/events/common.rs index 4cbda1694b..603149cc7b 100644 --- a/quic/s2n-quic-core/events/common.rs +++ b/quic/s2n-quic-core/events/common.rs @@ -267,6 +267,13 @@ impl From<&SocketAddress<'_>> for core::net::SocketAddr { } } +impl IntoEvent for &crate::inet::SocketAddress { + #[inline] + fn into_event(self) -> crate::inet::SocketAddress { + *self + } +} + enum DuplicatePacketError { /// The packet number was already received and is a duplicate. Duplicate, @@ -777,11 +784,10 @@ enum DatagramDropReason { InvalidMtuConfiguration { /// MTU configuration for the endpoint endpoint_mtu_config: MtuConfig, - // TODO expose connection level mtu config. - // TODO expose the remote address. - // https://github.com/aws/s2n-quic/issues/2254 - // error from mtu::Config - // remote_addr: SocketAddress<'a>, + /// The connection-specific MTU configuration that was invalid + conn_mtu_config: MtuConfig, + /// The remote address that caused the MTU configuration error + remote_addr: crate::inet::SocketAddress, }, /// The Destination Connection Id is unknown and does not map to a Connection. /// diff --git a/quic/s2n-quic-core/src/event/generated.rs b/quic/s2n-quic-core/src/event/generated.rs index 7e78629cf2..ed4c206311 100644 --- a/quic/s2n-quic-core/src/event/generated.rs +++ b/quic/s2n-quic-core/src/event/generated.rs @@ -874,6 +874,10 @@ pub mod api { InvalidMtuConfiguration { #[doc = " MTU configuration for the endpoint"] endpoint_mtu_config: MtuConfig, + #[doc = " The connection-specific MTU configuration that was invalid"] + conn_mtu_config: MtuConfig, + #[doc = " The remote address that caused the MTU configuration error"] + remote_addr: crate::inet::SocketAddress, }, #[non_exhaustive] #[doc = " The Destination Connection Id is unknown and does not map to a Connection."] @@ -3357,6 +3361,12 @@ pub mod api { } } } + impl IntoEvent for &crate::inet::SocketAddress { + #[inline] + fn into_event(self) -> crate::inet::SocketAddress { + *self + } + } impl IntoEvent for crate::packet::number::SlidingWindowError { #[inline] fn into_event(self) -> builder::DuplicatePacketError { @@ -5223,6 +5233,10 @@ pub mod builder { InvalidMtuConfiguration { #[doc = " MTU configuration for the endpoint"] endpoint_mtu_config: MtuConfig, + #[doc = " The connection-specific MTU configuration that was invalid"] + conn_mtu_config: MtuConfig, + #[doc = " The remote address that caused the MTU configuration error"] + remote_addr: crate::inet::SocketAddress, }, #[doc = " The Destination Connection Id is unknown and does not map to a Connection."] #[doc = ""] @@ -5260,8 +5274,12 @@ pub mod builder { Self::InvalidSourceConnectionId => InvalidSourceConnectionId {}, Self::InvalidMtuConfiguration { endpoint_mtu_config, + conn_mtu_config, + remote_addr, } => InvalidMtuConfiguration { endpoint_mtu_config: endpoint_mtu_config.into_event(), + conn_mtu_config: conn_mtu_config.into_event(), + remote_addr: remote_addr.into_event(), }, Self::UnknownDestinationConnectionId => UnknownDestinationConnectionId {}, Self::RejectedConnectionAttempt => RejectedConnectionAttempt {}, diff --git a/quic/s2n-quic-core/src/path/mtu.rs b/quic/s2n-quic-core/src/path/mtu.rs index 221e17351c..da13282cc7 100644 --- a/quic/s2n-quic-core/src/path/mtu.rs +++ b/quic/s2n-quic-core/src/path/mtu.rs @@ -161,7 +161,7 @@ const MINIMUM_MTU: u16 = MINIMUM_MAX_DATAGRAM_SIZE macro_rules! impl_mtu { ($name:ident, $default:expr) => { - #[derive(Clone, Copy, Debug, PartialEq)] + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct $name(NonZeroU16); impl $name { @@ -242,6 +242,29 @@ impl Display for MtuError { impl core::error::Error for MtuError {} +/// Error returned when runtime MTU configuration validation fails +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MtuConfigError { + /// The remote address for which the configuration was requested + pub remote_addr: inet::SocketAddress, + /// The connection-specific MTU config that was invalid + pub conn_config: Config, + /// The endpoint's MTU config for comparison + pub endpoint_config: Config, +} + +impl Display for MtuConfigError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!( + f, + "Invalid MTU configuration for {}: conn_config={:?}, endpoint_config={:?}", + self.remote_addr, self.conn_config, self.endpoint_config + ) + } +} + +impl core::error::Error for MtuConfigError {} + /// Information about the path that may be used when generating MTU configuration. #[non_exhaustive] pub struct PathInfo<'a> { @@ -275,13 +298,27 @@ impl Manager { } } - pub fn config(&mut self, remote_address: &inet::SocketAddress) -> Result { + pub fn config( + &mut self, + remote_address: &inet::SocketAddress, + ) -> Result { let info = mtu::PathInfo::new(remote_address); if let Some(conn_config) = self.provider.on_path(&info, self.endpoint_mtu_config) { - ensure!(conn_config.is_valid(), Err(MtuError)); ensure!( - u16::from(conn_config.max_mtu) <= u16::from(self.endpoint_mtu_config.max_mtu()), - Err(MtuError) + conn_config.is_valid(), + Err(MtuConfigError { + remote_addr: *remote_address, + conn_config, + endpoint_config: self.endpoint_mtu_config, + }) + ); + ensure!( + conn_config.max_mtu <= self.endpoint_mtu_config.max_mtu(), + Err(MtuConfigError { + remote_addr: *remote_address, + conn_config, + endpoint_config: self.endpoint_mtu_config, + }) ); Ok(conn_config) @@ -327,7 +364,7 @@ impl Endpoint for Inherit { } /// MTU configuration. -#[derive(Copy, Clone, Debug, Default)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub struct Config { initial_mtu: InitialMtu, base_mtu: BaseMtu, diff --git a/quic/s2n-quic-core/src/path/mtu/tests.rs b/quic/s2n-quic-core/src/path/mtu/tests.rs index ad8785f083..1b17771ad7 100644 --- a/quic/s2n-quic-core/src/path/mtu/tests.rs +++ b/quic/s2n-quic-core/src/path/mtu/tests.rs @@ -151,7 +151,10 @@ fn mtu_manager() { }; assert!(!mtu_provider.is_valid()); let mut manager: Manager = Manager::new(mtu_provider); - assert_eq!(manager.config(&remote).unwrap_err(), MtuError); + let err = manager.config(&remote).unwrap_err(); + assert_eq!(err.remote_addr, remote); + assert_eq!(err.conn_config, mtu_provider); + assert_eq!(err.endpoint_config, Default::default()); // invalid: mtu_provider.max_mtu > endpoint_config.max_mtu let mtu_provider = mtu::Config::builder() @@ -161,7 +164,12 @@ fn mtu_manager() { .unwrap(); assert!(mtu_provider.is_valid()); let mut manager: Manager = Manager::new(mtu_provider); - assert_eq!(manager.config(&remote).unwrap_err(), MtuError); + let err = manager.config(&remote).unwrap_err(); + assert_eq!(err.remote_addr, remote); + assert_eq!(err.conn_config, mtu_provider); + assert_eq!(err.endpoint_config, Default::default()); + // Verify it's the "exceeds endpoint max" case + assert!(err.conn_config.max_mtu() > err.endpoint_config.max_mtu()); } #[test] diff --git a/quic/s2n-quic-transport/src/path/manager.rs b/quic/s2n-quic-transport/src/path/manager.rs index 17ea60f8ea..f22b991f4d 100644 --- a/quic/s2n-quic-transport/src/path/manager.rs +++ b/quic/s2n-quic-transport/src/path/manager.rs @@ -416,9 +416,11 @@ impl Manager { .rtt_estimator .for_new_path(limits.initial_round_trip_time()); - let mtu_config = mtu.config(&remote_address).map_err(|_err| { + let mtu_config = mtu.config(&remote_address).map_err(|err| { event::builder::DatagramDropReason::InvalidMtuConfiguration { - endpoint_mtu_config: mtu.endpoint_config().into_event(), + endpoint_mtu_config: err.endpoint_config.into_event(), + conn_mtu_config: err.conn_config.into_event(), + remote_addr: err.remote_addr.into_event(), } })?;