diff --git a/quinn-proto/src/config.rs b/quinn-proto/src/config.rs index 90e3dc9134..ea4f47661e 100644 --- a/quinn-proto/src/config.rs +++ b/quinn-proto/src/config.rs @@ -9,8 +9,8 @@ use crate::{ cid_generator::{ConnectionIdGenerator, RandomConnectionIdGenerator}, congestion, crypto::{self, HandshakeTokenKey, HmacKey}, - VarInt, VarIntBoundsExceeded, DEFAULT_SUPPORTED_VERSIONS, INITIAL_MAX_UDP_PAYLOAD_SIZE, - MAX_UDP_PAYLOAD, + VarInt, VarIntBoundsExceeded, BASE_UDP_PAYLOAD_SIZE, DEFAULT_SUPPORTED_VERSIONS, + INITIAL_MAX_UDP_PAYLOAD_SIZE, MAX_UDP_PAYLOAD, }; /// Parameters governing the core QUIC state machine @@ -38,6 +38,7 @@ pub struct TransportConfig { pub(crate) time_threshold: f32, pub(crate) initial_rtt: Duration, pub(crate) initial_max_udp_payload_size: u16, + pub(crate) base_udp_payload_size: u16, pub(crate) mtu_discovery_config: Option, pub(crate) persistent_congestion_threshold: u32, @@ -160,16 +161,34 @@ impl TransportConfig { /// (see [`TransportConfig::mtu_discovery_config`]). /// /// Must be at least 1200, which is the default, and known to be safe for typical internet - /// applications. Larger values are more efficient, but increase the risk of unpredictable - /// catastrophic packet loss due to exceeding the network path's IP MTU. If the provided value - /// is higher than what the network path actually supports, packet loss will eventually trigger - /// black hole detection and bring it down to 1200. - /// - /// Real-world MTUs can vary according to ISP, VPN, and properties of intermediate network links - /// outside of either endpoint's control. Caution should be used when raising this value for - /// connections outside of private networks where these factors are fully controlled. + /// applications. Larger values are more efficient, but increase the risk of packet loss due to + /// exceeding the network path's IP MTU. If the provided value is higher than what the network + /// path actually supports, packet loss will eventually trigger black hole detection and bring + /// it down to [`TransportConfig::base_udp_payload_size`]. pub fn initial_max_udp_payload_size(&mut self, value: u16) -> &mut Self { self.initial_max_udp_payload_size = value.max(INITIAL_MAX_UDP_PAYLOAD_SIZE); + assert!(self.initial_max_udp_payload_size >= self.base_udp_payload_size); + self + } + + /// The UDP payload size guaranteed to be supported by the network. + /// + /// Must be at least 1200, which is the default, and lower than or equal to + /// [`TransportConfig::initial_max_udp_payload_size`]. + /// + /// Real-world MTUs can vary according to ISP, VPN, and properties of intermediate network links + /// outside of either endpoint's control. Extreme care should be used when raising this value + /// outside of private networks where these factors are fully controlled. If the provided value + /// is higher than what the network path actually supports, the result will be unpredictable and + /// catastrophic packet loss, without a possibility of repair. Prefer + /// [`TransportConfig::initial_max_udp_payload_size`] together with + /// [`TransportConfig::mtu_discovery_config`] to set a maximum UDP payload size that robustly + /// adapts to the network. + pub fn base_udp_payload_size(&mut self, value: u16) -> &mut Self { + self.base_udp_payload_size = value.max(BASE_UDP_PAYLOAD_SIZE); + self.initial_max_udp_payload_size = self + .initial_max_udp_payload_size + .max(self.base_udp_payload_size); self } @@ -296,6 +315,7 @@ impl Default for TransportConfig { time_threshold: 9.0 / 8.0, initial_rtt: Duration::from_millis(333), // per spec, intentionally distinct from EXPECTED_RTT initial_max_udp_payload_size: INITIAL_MAX_UDP_PAYLOAD_SIZE, + base_udp_payload_size: BASE_UDP_PAYLOAD_SIZE, mtu_discovery_config: None, persistent_congestion_threshold: 3, diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 430f120bbc..3d9be9c897 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -267,6 +267,7 @@ impl Connection { .congestion_controller_factory .build(now, config.initial_max_udp_payload_size), config.initial_max_udp_payload_size, + config.base_udp_payload_size, None, config.mtu_discovery_config.clone(), now, @@ -2739,6 +2740,7 @@ impl Connection { .congestion_controller_factory .build(now, self.config.initial_max_udp_payload_size), self.config.initial_max_udp_payload_size, + self.config.base_udp_payload_size, Some(peer_max_udp_payload_size), self.config.mtu_discovery_config.clone(), now, diff --git a/quinn-proto/src/connection/mtud.rs b/quinn-proto/src/connection/mtud.rs index 41941d3467..26b9129f1a 100644 --- a/quinn-proto/src/connection/mtud.rs +++ b/quinn-proto/src/connection/mtud.rs @@ -18,15 +18,20 @@ pub(crate) struct MtuDiscovery { impl MtuDiscovery { pub(crate) fn new( initial_plpmtu: u16, + base_udp_payload_size: u16, peer_max_udp_payload_size: Option, config: MtuDiscoveryConfig, ) -> Self { debug_assert!( - initial_plpmtu >= BASE_UDP_PAYLOAD_SIZE, - "initial_max_udp_payload_size must be at least {BASE_UDP_PAYLOAD_SIZE}" + initial_plpmtu >= base_udp_payload_size, + "initial_max_udp_payload_size must be at least {base_udp_payload_size}" ); - let mut mtud = Self::with_state(initial_plpmtu, Some(EnabledMtuDiscovery::new(config))); + let mut mtud = Self::with_state( + initial_plpmtu, + base_udp_payload_size, + Some(EnabledMtuDiscovery::new(config)), + ); // We might be migrating an existing connection to a new path, in which case the transport // parameters have already been transmitted, and we already know the value of @@ -39,15 +44,19 @@ impl MtuDiscovery { } /// MTU discovery will be disabled and the current MTU will be fixed to the provided value - pub(crate) fn disabled(plpmtu: u16) -> Self { - Self::with_state(plpmtu, None) + pub(crate) fn disabled(plpmtu: u16, base_udp_payload_size: u16) -> Self { + Self::with_state(plpmtu, base_udp_payload_size, None) } - fn with_state(current_mtu: u16, state: Option) -> Self { + fn with_state( + current_mtu: u16, + base_udp_payload_size: u16, + state: Option, + ) -> Self { Self { current_mtu, state, - black_hole_detector: BlackHoleDetector::new(), + black_hole_detector: BlackHoleDetector::new(base_udp_payload_size), } } @@ -145,7 +154,7 @@ impl MtuDiscovery { /// current MTU will be reset to `BASE_UDP_PAYLOAD_SIZE`. pub(crate) fn detect_black_hole(&mut self, now: Instant) -> bool { if self.black_hole_detector.detect_black_hole() { - self.current_mtu = BASE_UDP_PAYLOAD_SIZE; + self.current_mtu = self.black_hole_detector.base_udp_payload_size; if let Some(state) = &mut self.state { state.on_black_hole_detected(now); @@ -372,16 +381,19 @@ struct BlackHoleDetector { largest_non_probe_lost: Option, /// The largest acked packet of size `current_mtu` largest_acked_mtu_sized_packet: Option, + /// The UDP payload size guaranteed to be supported by the network + base_udp_payload_size: u16, } impl BlackHoleDetector { - fn new() -> Self { + fn new(base_udp_payload_size: u16) -> Self { Self { suspicious_loss_bursts: 0, largest_non_probe_lost: None, loss_burst_has_non_suspicious_packets: false, largest_suspicious_packet_lost: None, largest_acked_mtu_sized_packet: None, + base_udp_payload_size, } } @@ -414,7 +426,7 @@ impl BlackHoleDetector { self.finish_loss_burst(); } - if packet_bytes <= BASE_UDP_PAYLOAD_SIZE { + if packet_bytes <= self.base_udp_payload_size { self.loss_burst_has_non_suspicious_packets = true; } else { self.largest_suspicious_packet_lost = Some(packet_number); @@ -472,7 +484,6 @@ impl BlackHoleDetector { // Corresponds to the RFC's `MAX_PROBES` constant (see // https://www.rfc-editor.org/rfc/rfc8899#section-5.1.2) const MAX_PROBE_RETRANSMITS: usize = 3; -const BASE_UDP_PAYLOAD_SIZE: u16 = 1_200; const BLACK_HOLE_THRESHOLD: u8 = 3; const BINARY_SEARCH_MINIMUM_CHANGE: u16 = 20; @@ -486,7 +497,7 @@ mod tests { fn default_mtud() -> MtuDiscovery { let config = MtuDiscoveryConfig::default(); - MtuDiscovery::new(1_200, None, config) + MtuDiscovery::new(1_200, 1_200, None, config) } fn completed(mtud: &MtuDiscovery) -> bool { @@ -569,14 +580,14 @@ mod tests { #[test] fn mtu_discovery_disabled_does_nothing() { - let mut mtud = MtuDiscovery::disabled(1_200); + let mut mtud = MtuDiscovery::disabled(1_200, 1_200); let probe_size = mtud.poll_transmit(Instant::now(), 0); assert_eq!(probe_size, None); } #[test] fn mtu_discovery_disabled_lost_four_packet_bursts_triggers_black_hole_detection() { - let mut mtud = MtuDiscovery::disabled(1_400); + let mut mtud = MtuDiscovery::disabled(1_400, 1_250); let now = Instant::now(); for i in 0..4 { @@ -585,7 +596,7 @@ mod tests { } assert!(mtud.detect_black_hole(now)); - assert_eq!(mtud.current_mtu, 1200); + assert_eq!(mtud.current_mtu, 1250); assert_matches!(mtud.state, None); } @@ -623,7 +634,7 @@ mod tests { fn mtu_discovery_after_complete_reactivates_when_interval_elapsed() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(9_000); - let mut mtud = MtuDiscovery::new(1_200, None, config); + let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config); let now = Instant::now(); drive_to_completion(&mut mtud, now, 1_500); @@ -672,7 +683,7 @@ mod tests { assert!(fourth_probe_size < first_probe_size); assert_eq!( fourth_probe_size, - first_probe_size - (first_probe_size - BASE_UDP_PAYLOAD_SIZE) / 2 - 1 + first_probe_size - (first_probe_size - 1_200) / 2 - 1 ); } @@ -692,7 +703,7 @@ mod tests { #[test] fn mtu_discovery_with_previous_peer_max_udp_payload_size_clamps_upper_bound() { - let mut mtud = MtuDiscovery::new(1500, Some(1400), MtuDiscoveryConfig::default()); + let mut mtud = MtuDiscovery::new(1500, 1_200, Some(1400), MtuDiscoveryConfig::default()); assert_eq!(mtud.current_mtu, 1400); assert_eq!(mtud.state.as_ref().unwrap().peer_max_udp_payload_size, 1400); @@ -728,7 +739,7 @@ mod tests { fn mtu_discovery_with_1500_limit_and_10000_upper_bound() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(10_000); - let mut mtud = MtuDiscovery::new(1_200, None, config); + let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config); let probed_sizes = drive_to_completion(&mut mtud, Instant::now(), 1500); @@ -745,7 +756,7 @@ mod tests { fn mtu_discovery_no_lost_probes_finds_maximum_udp_payload() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(MAX_UDP_PAYLOAD); - let mut mtud = MtuDiscovery::new(1200, None, config); + let mut mtud = MtuDiscovery::new(1200, 1200, None, config); drive_to_completion(&mut mtud, Instant::now(), u16::MAX); @@ -757,7 +768,7 @@ mod tests { fn mtu_discovery_lost_half_of_probes_finds_maximum_udp_payload() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(MAX_UDP_PAYLOAD); - let mut mtud = MtuDiscovery::new(1200, None, config); + let mut mtud = MtuDiscovery::new(1200, 1200, None, config); let now = Instant::now(); let mut iterations = 0; diff --git a/quinn-proto/src/connection/paths.rs b/quinn-proto/src/connection/paths.rs index 9a690c4596..eca01e304c 100644 --- a/quinn-proto/src/connection/paths.rs +++ b/quinn-proto/src/connection/paths.rs @@ -38,6 +38,7 @@ impl PathData { initial_rtt: Duration, congestion: Box, initial_max_udp_payload_size: u16, + base_udp_payload_size: u16, peer_max_udp_payload_size: Option, mtud_config: Option, now: Instant, @@ -60,10 +61,11 @@ impl PathData { total_sent: 0, total_recvd: 0, mtud: mtud_config.map_or( - MtuDiscovery::disabled(initial_max_udp_payload_size), + MtuDiscovery::disabled(initial_max_udp_payload_size, base_udp_payload_size), |config| { MtuDiscovery::new( initial_max_udp_payload_size, + base_udp_payload_size, peer_max_udp_payload_size, config, ) diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index 9f9e34bf03..c9c7046e71 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -65,8 +65,6 @@ pub struct Endpoint { impl Endpoint { /// Create a new endpoint - /// - /// Returns `Err` if the configuration is invalid. pub fn new(config: Arc, server_config: Option>) -> Self { Self { rng: StdRng::from_entropy(), diff --git a/quinn-proto/src/lib.rs b/quinn-proto/src/lib.rs index ba233d96cb..4953d6d040 100644 --- a/quinn-proto/src/lib.rs +++ b/quinn-proto/src/lib.rs @@ -295,6 +295,7 @@ const MAX_CID_SIZE: usize = 20; const MIN_INITIAL_SIZE: u16 = 1200; /// const INITIAL_MAX_UDP_PAYLOAD_SIZE: u16 = 1200; +const BASE_UDP_PAYLOAD_SIZE: u16 = 1200; const MAX_UDP_PAYLOAD: u16 = 65527; const TIMER_GRANULARITY: Duration = Duration::from_millis(1); /// Maximum number of streams that can be uniquely identified by a stream ID