diff --git a/bench/src/lib.rs b/bench/src/lib.rs index 8321bb2db..9061b2839 100644 --- a/bench/src/lib.rs +++ b/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 diff --git a/perf/src/bin/perf_client.rs b/perf/src/bin/perf_client.rs index 6519d097a..fd90962fd 100644 --- a/perf/src/bin/perf_client.rs +++ b/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 { diff --git a/perf/src/bin/perf_server.rs b/perf/src/bin/perf_server.rs index 204487a48..839173575 100644 --- a/perf/src/bin/perf_server.rs +++ b/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 { diff --git a/quinn-proto/src/config.rs b/quinn-proto/src/config.rs index 23fcd1d99..1c4457620 100644 --- a/quinn-proto/src/config.rs +++ b/quinn-proto/src/config.rs @@ -212,7 +212,7 @@ impl TransportConfig { /// to disable UDP packet fragmentation (this is strongly recommended by [RFC /// 9000](https://www.rfc-editor.org/rfc/rfc9000.html#section-14-7), regardless of MTU /// discovery). They can build on top of the `quinn-udp` crate, used by `quinn` itself, which - /// provides Linux and Windows support for disabling packet fragmentation. + /// provides Linux, Windows, macOS, and FreeBSD support for disabling packet fragmentation. pub fn mtu_discovery_config(&mut self, value: Option) -> &mut Self { self.mtu_discovery_config = value; self @@ -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, diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 254dbb855..437c5dbd9 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/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, ), diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index ba9a9fd5a..715bd6429 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -61,11 +61,21 @@ pub struct Endpoint { local_cid_generator: Box, config: Arc, server_config: Option>, + /// Whether the underlying UDP socket promises not to fragment packets + allow_mtud: bool, } impl Endpoint { /// Create a new endpoint - pub fn new(config: Arc, server_config: Option>) -> 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, + server_config: Option>, + 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 { diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs index 4cb8754cb..807305e2c 100644 --- a/quinn-proto/src/tests/mod.rs +++ b/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; diff --git a/quinn-proto/src/tests/util.rs b/quinn-proto/src/tests/util.rs index 9bed347a8..9065655f4 100644 --- a/quinn-proto/src/tests/util.rs +++ b/quinn-proto/src/tests/util.rs @@ -35,8 +35,8 @@ pub(super) struct Pair { impl Pair { pub(super) fn new(endpoint_config: Arc, 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) -> ClientConfig { diff --git a/quinn-udp/Cargo.toml b/quinn-udp/Cargo.toml index 45347d01d..318fa3647 100644 --- a/quinn-udp/Cargo.toml +++ b/quinn-udp/Cargo.toml @@ -22,7 +22,7 @@ log = ["tracing/log"] maintenance = { status = "experimental" } [dependencies] -libc = "0.2.69" +libc = "0.2.113" socket2 = "0.4" # 0.5.1 has an MSRV of 1.63 tracing = "0.1.10" diff --git a/quinn-udp/src/fallback.rs b/quinn-udp/src/fallback.rs index 8a5351e46..68be07d98 100644 --- a/quinn-udp/src/fallback.rs +++ b/quinn-udp/src/fallback.rs @@ -106,4 +106,9 @@ pub fn udp_state() -> super::UdpState { } } +#[inline] +pub(crate) fn may_fragment() -> bool { + true +} + pub const BATCH_SIZE: usize = 1; diff --git a/quinn-udp/src/lib.rs b/quinn-udp/src/lib.rs index 99ceda0fb..c11f4a467 100644 --- a/quinn-udp/src/lib.rs +++ b/quinn-udp/src/lib.rs @@ -36,6 +36,14 @@ mod imp; pub use imp::UdpSocketState; +/// Whether transmitted datagrams might get fragmented by the IP layer +/// +/// Returns `false` on targets which employ e.g. the `IPV6_DONTFRAG` socket option. +#[inline] +pub fn may_fragment() -> bool { + imp::may_fragment() +} + /// Number of UDP packets to send/receive at a time pub const BATCH_SIZE: usize = imp::BATCH_SIZE; diff --git a/quinn-udp/src/unix.rs b/quinn-udp/src/unix.rs index e87746e26..25a565796 100644 --- a/quinn-udp/src/unix.rs +++ b/quinn-udp/src/unix.rs @@ -125,6 +125,12 @@ fn init(io: SockRef<'_>) -> io::Result<()> { )?; } } + #[cfg(any(target_os = "freebsd", target_os = "macos", target_os = "ios"))] + { + if is_ipv4 { + set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_DONTFRAG, OPTION_ON)?; + } + } #[cfg(any(target_os = "freebsd", target_os = "macos"))] // IP_RECVDSTADDR == IP_SENDSRCADDR on FreeBSD // macOS uses only IP_RECVDSTADDR, no IP_SENDSRCADDR on macOS @@ -135,14 +141,14 @@ fn init(io: SockRef<'_>) -> io::Result<()> { } } - // IPV6_RECVPKTINFO is standardized + // Options standardized in RFC 3542 if !is_ipv4 { set_socket_option(&*io, libc::IPPROTO_IPV6, libc::IPV6_RECVPKTINFO, OPTION_ON)?; set_socket_option(&*io, libc::IPPROTO_IPV6, libc::IPV6_RECVTCLASS, OPTION_ON)?; - } - - if !is_ipv4 { - set_socket_option(&*io, libc::IPPROTO_IPV6, libc::IPV6_RECVTCLASS, OPTION_ON)?; + // Linux's IP_PMTUDISC_PROBE allows us to operate under interface MTU rather than the + // kernel's path MTU guess, but actually disabling fragmentation requires this too. See + // __ip6_append_data in ip6_output.c. + set_socket_option(&*io, libc::IPPROTO_IPV6, libc::IPV6_DONTFRAG, OPTION_ON)?; } Ok(()) @@ -667,6 +673,11 @@ pub(crate) const BATCH_SIZE: usize = 32; #[cfg(any(target_os = "macos", target_os = "ios"))] pub(crate) const BATCH_SIZE: usize = 1; +#[inline] +pub(crate) fn may_fragment() -> bool { + false +} + #[cfg(target_os = "linux")] mod gso { use super::*; diff --git a/quinn-udp/src/windows.rs b/quinn-udp/src/windows.rs index 648d9e49e..62b7f7149 100644 --- a/quinn-udp/src/windows.rs +++ b/quinn-udp/src/windows.rs @@ -154,3 +154,8 @@ pub fn udp_state() -> super::UdpState { } pub const BATCH_SIZE: usize = 1; + +#[inline] +pub(crate) fn may_fragment() -> bool { + false +} diff --git a/quinn/benches/bench.rs b/quinn/benches/bench.rs index aefc60292..4f672fe95 100644 --- a/quinn/benches/bench.rs +++ b/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() } diff --git a/quinn/examples/client.rs b/quinn/examples/client.rs index b0386c882..85e69a3d5 100644 --- a/quinn/examples/client.rs +++ b/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); diff --git a/quinn/examples/common/mod.rs b/quinn/examples/common/mod.rs index 9f6b39af6..294b79191 100644 --- a/quinn/examples/common/mod.rs +++ b/quinn/examples/common/mod.rs @@ -45,24 +45,10 @@ fn configure_client(server_certs: &[&[u8]]) -> Result Result<(ServerConfig, Vec), Box> { let cert = rcgen::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); @@ -74,8 +60,6 @@ fn configure_server() -> Result<(ServerConfig, Vec), Box> { 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)) } diff --git a/quinn/examples/insecure_connection.rs b/quinn/examples/insecure_connection.rs index 03e16bb10..994e34718 100644 --- a/quinn/examples/insecure_connection.rs +++ b/quinn/examples/insecure_connection.rs @@ -31,10 +31,8 @@ async fn run_server(addr: SocketAddr) { } async fn run_client(server_addr: SocketAddr) -> Result<(), Box> { - 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 diff --git a/quinn/examples/server.rs b/quinn/examples/server.rs index fc1ff75f2..037cdab45 100644 --- a/quinn/examples/server.rs +++ b/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); } diff --git a/quinn/src/endpoint.rs b/quinn/src/endpoint.rs index 454fbaa7c..cba3a0181 100644 --- a/quinn/src/endpoint.rs +++ b/quinn/src/endpoint.rs @@ -114,9 +114,10 @@ impl Endpoint { runtime: Arc, ) -> io::Result { 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(), ); diff --git a/quinn/src/runtime.rs b/quinn/src/runtime.rs index 5a327ceb5..9bcb73600 100644 --- a/quinn/src/runtime.rs +++ b/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; + + /// 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 diff --git a/quinn/src/runtime/async_std.rs b/quinn/src/runtime/async_std.rs index a20c5ac05..da01201b7 100644 --- a/quinn/src/runtime/async_std.rs +++ b/quinn/src/runtime/async_std.rs @@ -80,4 +80,8 @@ impl AsyncUdpSocket for UdpSocket { fn local_addr(&self) -> io::Result { self.io.as_ref().local_addr() } + + fn may_fragment(&self) -> bool { + udp::may_fragment() + } } diff --git a/quinn/src/runtime/tokio.rs b/quinn/src/runtime/tokio.rs index 6a76c25ec..d8ac4ac10 100644 --- a/quinn/src/runtime/tokio.rs +++ b/quinn/src/runtime/tokio.rs @@ -88,4 +88,8 @@ impl AsyncUdpSocket for UdpSocket { fn local_addr(&self) -> io::Result { self.io.local_addr() } + + fn may_fragment(&self) -> bool { + udp::may_fragment() + } }