Enable PMTUD by default when appropriate

bench/src/lib.rs

@@ -142,8 +142,6 @@ pub fn transport_config(opt: &Opt) -> quinn::TransportConfig {
     // High stream windows are chosen because the amount of concurrent streams
     // is configurable as a parameter.
     let mut config = quinn::TransportConfig::default();
-    #[cfg(any(windows, os = "linux"))]
-    config.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
     config.max_concurrent_uni_streams(opt.max_streams.try_into().unwrap());
     config.initial_mtu(opt.initial_mtu);
     config

perf/src/bin/perf_client.rs

@@ -129,8 +129,6 @@ async fn run(opt: Opt) -> Result<()> {
     }
 
     let mut transport = quinn::TransportConfig::default();
-    #[cfg(any(windows, os = "linux"))]
-    transport.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
     transport.initial_mtu(opt.initial_mtu);
 
     let mut cfg = if opt.no_protection {

perf/src/bin/perf_server.rs

@@ -88,8 +88,6 @@ async fn run(opt: Opt) -> Result<()> {
     }
 
     let mut transport = quinn::TransportConfig::default();
-    #[cfg(any(windows, os = "linux"))]
-    transport.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
     transport.initial_mtu(opt.initial_mtu);
 
     let mut server_config = if opt.no_protection {

quinn-proto/src/config.rs

@@ -315,7 +315,7 @@ impl Default for TransportConfig {
             initial_rtt: Duration::from_millis(333), // per spec, intentionally distinct from EXPECTED_RTT
             initial_mtu: INITIAL_MTU,
             min_mtu: INITIAL_MTU,
-            mtu_discovery_config: None,
+            mtu_discovery_config: Some(MtuDiscoveryConfig::default()),
 
             persistent_congestion_threshold: 3,
             keep_alive_interval: None,

quinn-proto/src/connection/mod.rs

@@ -236,6 +236,7 @@ impl Connection {
         cid_gen: &dyn ConnectionIdGenerator,
         now: Instant,
         version: u32,
+        allow_mtud: bool,
     ) -> Self {
         let side = if server_config.is_some() {
             Side::Server
@@ -269,7 +270,10 @@ impl Connection {
                 config.get_initial_mtu(),
                 config.min_mtu,
                 None,
-                config.mtu_discovery_config.clone(),
+                match allow_mtud {
+                    true => config.mtu_discovery_config.clone(),
+                    false => None,
+                },
                 now,
                 path_validated,
             ),

quinn-proto/src/endpoint.rs

@@ -61,11 +61,21 @@ pub struct Endpoint {
     local_cid_generator: Box<dyn ConnectionIdGenerator>,
     config: Arc<EndpointConfig>,
     server_config: Option<Arc<ServerConfig>>,
+    /// Whether the underlying UDP socket promises not to fragment packets
+    allow_mtud: bool,
 }
 
 impl Endpoint {
     /// Create a new endpoint
-    pub fn new(config: Arc<EndpointConfig>, server_config: Option<Arc<ServerConfig>>) -> Self {
+    ///
+    /// `allow_mtud` enables path MTU detection when requested by `Connection` configuration for
+    /// better performance. This requires that outgoing packets are never fragmented, which can be
+    /// achieved via e.g. the `IPV6_DONTFRAG` socket option.
+    pub fn new(
+        config: Arc<EndpointConfig>,
+        server_config: Option<Arc<ServerConfig>>,
+        allow_mtud: bool,
+    ) -> Self {
         Self {
             rng: StdRng::from_entropy(),
             transmits: VecDeque::new(),
@@ -77,6 +87,7 @@ impl Endpoint {
             local_cid_generator: (config.connection_id_generator_factory.as_ref())(),
             config,
             server_config,
+            allow_mtud,
         }
     }
 
@@ -640,6 +651,7 @@ impl Endpoint {
             self.local_cid_generator.as_ref(),
             now,
             version,
+            self.allow_mtud,
         );
 
         let id = self.connections.insert(ConnectionMeta {

quinn-proto/src/tests/mod.rs

@@ -25,7 +25,7 @@ use util::*;
 fn version_negotiate_server() {
     let _guard = subscribe();
     let client_addr = "[::2]:7890".parse().unwrap();
-    let mut server = Endpoint::new(Default::default(), Some(Arc::new(server_config())));
+    let mut server = Endpoint::new(Default::default(), Some(Arc::new(server_config())), true);
     let now = Instant::now();
     let event = server.handle(
         now,
@@ -61,6 +61,7 @@ fn version_negotiate_client() {
             ..Default::default()
         }),
         None,
+        true,
     );
     let (_, mut client_ch) = client
         .connect(client_config(), server_addr, "localhost")
@@ -176,7 +177,7 @@ fn server_stateless_reset() {
     let mut pair = Pair::new(endpoint_config.clone(), server_config());
     let (client_ch, _) = pair.connect();
     pair.drive(); // Flush any post-handshake frames
-    pair.server.endpoint = Endpoint::new(endpoint_config, Some(Arc::new(server_config())));
+    pair.server.endpoint = Endpoint::new(endpoint_config, Some(Arc::new(server_config())), true);
     // Force the server to generate the smallest possible stateless reset
     pair.client.connections.get_mut(&client_ch).unwrap().ping();
     info!("resetting");
@@ -201,7 +202,7 @@ fn client_stateless_reset() {
 
     let mut pair = Pair::new(endpoint_config.clone(), server_config());
     let (_, server_ch) = pair.connect();
-    pair.client.endpoint = Endpoint::new(endpoint_config, Some(Arc::new(server_config())));
+    pair.client.endpoint = Endpoint::new(endpoint_config, Some(Arc::new(server_config())), true);
     // Send something big enough to allow room for a smaller stateless reset.
     pair.server.connections.get_mut(&server_ch).unwrap().close(
         pair.time,
@@ -1323,8 +1324,9 @@ fn cid_rotation() {
             ..EndpointConfig::default()
         }),
         Some(Arc::new(server_config())),
+        true,
     );
-    let client = Endpoint::new(Arc::new(EndpointConfig::default()), None);
+    let client = Endpoint::new(Arc::new(EndpointConfig::default()), None, true);
 
     let mut pair = Pair::new_from_endpoint(client, server);
     let (_, server_ch) = pair.connect();
@@ -1902,7 +1904,7 @@ fn big_cert_and_key() -> (rustls::Certificate, rustls::PrivateKey) {
 fn malformed_token_len() {
     let _guard = subscribe();
     let client_addr = "[::2]:7890".parse().unwrap();
-    let mut server = Endpoint::new(Default::default(), Some(Arc::new(server_config())));
+    let mut server = Endpoint::new(Default::default(), Some(Arc::new(server_config())), true);
     server.handle(
         Instant::now(),
         client_addr,
@@ -1977,12 +1979,13 @@ fn migrate_detects_new_mtu_and_respects_original_peer_max_udp_payload_size() {
     let server = Endpoint::new(
         Arc::new(server_endpoint_config),
         Some(Arc::new(server_config())),
+        true,
     );
     let client_endpoint_config = EndpointConfig {
         max_udp_payload_size: VarInt::from(client_max_udp_payload_size),
         ..EndpointConfig::default()
     };
-    let client = Endpoint::new(Arc::new(client_endpoint_config), None);
+    let client = Endpoint::new(Arc::new(client_endpoint_config), None, true);
     let mut pair = Pair::new_from_endpoint(client, server);
     pair.mtu = 1300;
 

quinn-proto/src/tests/util.rs

@@ -35,8 +35,8 @@ pub(super) struct Pair {
 
 impl Pair {
     pub(super) fn new(endpoint_config: Arc<EndpointConfig>, server_config: ServerConfig) -> Self {
-        let server = Endpoint::new(endpoint_config.clone(), Some(Arc::new(server_config)));
-        let client = Endpoint::new(endpoint_config, None);
+        let server = Endpoint::new(endpoint_config.clone(), Some(Arc::new(server_config)), true);
+        let client = Endpoint::new(endpoint_config, None, true);
 
         Self::new_from_endpoint(client, server)
     }
@@ -430,11 +430,7 @@ impl Write for TestWriter {
 }
 
 pub(super) fn server_config() -> ServerConfig {
-    let mut config = ServerConfig::with_crypto(Arc::new(server_crypto()));
-    Arc::get_mut(&mut config.transport)
-        .unwrap()
-        .mtu_discovery_config(Some(MtuDiscoveryConfig::default()));
-    config
+    ServerConfig::with_crypto(Arc::new(server_crypto()))
 }
 
 pub(super) fn server_config_with_cert(cert: Certificate, key: PrivateKey) -> ServerConfig {
@@ -452,11 +448,7 @@ pub(super) fn server_crypto_with_cert(cert: Certificate, key: PrivateKey) -> rus
 }
 
 pub(super) fn client_config() -> ClientConfig {
-    let mut config = ClientConfig::new(Arc::new(client_crypto()));
-    Arc::get_mut(&mut config.transport)
-        .unwrap()
-        .mtu_discovery_config(Some(MtuDiscoveryConfig::default()));
-    config
+    ClientConfig::new(Arc::new(client_crypto()))
 }
 
 pub(super) fn client_config_with_certs(certs: Vec<rustls::Certificate>) -> ClientConfig {

quinn/benches/bench.rs

@@ -83,16 +83,11 @@ impl Context {
             quinn::ServerConfig::with_single_cert(vec![cert.clone()], key).unwrap();
         let transport_config = Arc::get_mut(&mut server_config.transport).unwrap();
         transport_config.max_concurrent_uni_streams(1024_u16.into());
-        enable_mtud_if_supported(transport_config);
 
         let mut roots = rustls::RootCertStore::empty();
         roots.add(&cert).unwrap();
 
-        let mut client_config = quinn::ClientConfig::with_root_certificates(roots);
-        let mut transport_config = quinn::TransportConfig::default();
-        enable_mtud_if_supported(&mut transport_config);
-        client_config.transport_config(Arc::new(transport_config));
-
+        let client_config = quinn::ClientConfig::with_root_certificates(roots);
         Self {
             server_config,
             client_config,
@@ -164,14 +159,6 @@ impl Context {
     }
 }
 
-#[cfg(any(windows, os = "linux"))]
-fn enable_mtud_if_supported(transport_config: &mut quinn::TransportConfig) {
-    transport_config.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
-}
-
-#[cfg(not(any(windows, os = "linux")))]
-fn enable_mtud_if_supported(_transport_config: &mut quinn::TransportConfig) {}
-
 fn rt() -> Runtime {
     Builder::new_current_thread().enable_all().build().unwrap()
 }

quinn/examples/client.rs

@@ -95,9 +95,7 @@ async fn run(options: Opt) -> Result<()> {
         client_crypto.key_log = Arc::new(rustls::KeyLogFile::new());
     }
 
-    let mut client_config = quinn::ClientConfig::new(Arc::new(client_crypto));
-    common::enable_mtud_if_supported(&mut client_config);
-
+    let client_config = quinn::ClientConfig::new(Arc::new(client_crypto));
     let mut endpoint = quinn::Endpoint::client("[::]:0".parse().unwrap())?;
     endpoint.set_default_client_config(client_config);
 

quinn/examples/common/mod.rs

@@ -45,24 +45,10 @@ fn configure_client(server_certs: &[&[u8]]) -> Result<ClientConfig, Box<dyn Erro
         certs.add(&rustls::Certificate(cert.to_vec()))?;
     }
 
-    let mut client_config = ClientConfig::with_root_certificates(certs);
-    enable_mtud_if_supported(&mut client_config);
-
+    let client_config = ClientConfig::with_root_certificates(certs);
     Ok(client_config)
 }
 
-/// Enables MTUD if supported by the operating system
-#[cfg(any(windows, os = "linux"))]
-pub fn enable_mtud_if_supported(client_config: &mut ClientConfig) {
-    let mut transport_config = quinn::TransportConfig::default();
-    transport_config.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
-    client_config.transport_config(Arc::new(transport_config));
-}
-
-/// Enables MTUD if supported by the operating system
-#[cfg(not(any(windows, os = "linux")))]
-pub fn enable_mtud_if_supported(_client_config: &mut ClientConfig) {}
-
 /// Returns default server configuration along with its certificate.
 fn configure_server() -> Result<(ServerConfig, Vec<u8>), Box<dyn Error>> {
     let cert = rcgen::generate_simple_self_signed(vec!["localhost".into()]).unwrap();
@@ -74,8 +60,6 @@ fn configure_server() -> Result<(ServerConfig, Vec<u8>), Box<dyn Error>> {
     let mut server_config = ServerConfig::with_single_cert(cert_chain, priv_key)?;
     let transport_config = Arc::get_mut(&mut server_config.transport).unwrap();
     transport_config.max_concurrent_uni_streams(0_u8.into());
-    #[cfg(any(windows, os = "linux"))]
-    transport_config.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
 
     Ok((server_config, cert_der))
 }

quinn/examples/insecure_connection.rs

@@ -31,10 +31,8 @@ async fn run_server(addr: SocketAddr) {
 }
 
 async fn run_client(server_addr: SocketAddr) -> Result<(), Box<dyn Error>> {
-    let mut client_cfg = configure_client();
-    common::enable_mtud_if_supported(&mut client_cfg);
     let mut endpoint = Endpoint::client("127.0.0.1:0".parse().unwrap())?;
-    endpoint.set_default_client_config(client_cfg);
+    endpoint.set_default_client_config(configure_client());
 
     // connect to server
     let connection = endpoint

quinn/examples/server.rs

@@ -133,8 +133,6 @@ async fn run(options: Opt) -> Result<()> {
     let mut server_config = quinn::ServerConfig::with_crypto(Arc::new(server_crypto));
     let transport_config = Arc::get_mut(&mut server_config.transport).unwrap();
     transport_config.max_concurrent_uni_streams(0_u8.into());
-    #[cfg(any(windows, os = "linux"))]
-    transport_config.mtu_discovery_config(Some(quinn::MtuDiscoveryConfig::default()));
     if options.stateless_retry {
         server_config.use_retry(true);
     }

quinn/src/endpoint.rs

@@ -114,9 +114,10 @@ impl Endpoint {
         runtime: Arc<dyn Runtime>,
     ) -> io::Result<Self> {
         let addr = socket.local_addr()?;
+        let allow_mtud = !socket.may_fragment();
         let rc = EndpointRef::new(
             socket,
-            proto::Endpoint::new(Arc::new(config), server_config.map(Arc::new)),
+            proto::Endpoint::new(Arc::new(config), server_config.map(Arc::new), allow_mtud),
             addr.is_ipv6(),
             runtime.clone(),
         );

quinn/src/runtime.rs

@@ -50,6 +50,14 @@ pub trait AsyncUdpSocket: Send + Debug + 'static {
 
     /// Look up the local IP address and port used by this socket
     fn local_addr(&self) -> io::Result<SocketAddr>;
+
+    /// Whether datagrams might get fragmented into multiple parts
+    ///
+    /// Sockets should prevent this for best performance. See e.g. the `IPV6_DONTFRAG` socket
+    /// option.
+    fn may_fragment(&self) -> bool {
+        true
+    }
 }
 
 /// Automatically select an appropriate runtime from those enabled at compile time

quinn/src/runtime/async_std.rs

@@ -80,4 +80,8 @@ impl AsyncUdpSocket for UdpSocket {
     fn local_addr(&self) -> io::Result<std::net::SocketAddr> {
         self.io.as_ref().local_addr()
     }
+
+    fn may_fragment(&self) -> bool {
+        udp::may_fragment()
+    }
 }

quinn/src/runtime/tokio.rs

@@ -88,4 +88,8 @@ impl AsyncUdpSocket for UdpSocket {
     fn local_addr(&self) -> io::Result<std::net::SocketAddr> {
         self.io.local_addr()
     }
+
+    fn may_fragment(&self) -> bool {
+        udp::may_fragment()
+    }
 }