Make base_udp_payload_size user-configurable

Closes quinn-rs#1540

Sponsored by Stormshield

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<MtuDiscoveryConfig>,
 
     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,

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,

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<u16>,
         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<EnabledMtuDiscovery>) -> Self {
+    fn with_state(
+        current_mtu: u16,
+        base_udp_payload_size: u16,
+        state: Option<EnabledMtuDiscovery>,
+    ) -> 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<u64>,
     /// The largest acked packet of size `current_mtu`
     largest_acked_mtu_sized_packet: Option<u64>,
+    /// 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;

quinn-proto/src/connection/paths.rs

@@ -38,6 +38,7 @@ impl PathData {
         initial_rtt: Duration,
         congestion: Box<dyn congestion::Controller>,
         initial_max_udp_payload_size: u16,
+        base_udp_payload_size: u16,
         peer_max_udp_payload_size: Option<u16>,
         mtud_config: Option<MtuDiscoveryConfig>,
         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,
                     )

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<EndpointConfig>, server_config: Option<Arc<ServerConfig>>) -> Self {
         Self {
             rng: StdRng::from_entropy(),

quinn-proto/src/lib.rs

@@ -295,6 +295,7 @@ const MAX_CID_SIZE: usize = 20;
 const MIN_INITIAL_SIZE: u16 = 1200;
 /// <https://www.rfc-editor.org/rfc/rfc9000.html#name-datagram-size>
 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