Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fragmentation handling improvements #1547

Merged
merged 4 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bench/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions perf/src/bin/perf_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions perf/src/bin/perf_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MtuDiscoveryConfig>) -> &mut Self {
self.mtu_discovery_config = value;
self
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
),
Expand Down
14 changes: 13 additions & 1 deletion quinn-proto/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -77,6 +87,7 @@ impl Endpoint {
local_cid_generator: (config.connection_id_generator_factory.as_ref())(),
config,
server_config,
allow_mtud,
}
}

Expand Down Expand Up @@ -640,6 +651,7 @@ impl Endpoint {
self.local_cid_generator.as_ref(),
now,
version,
self.allow_mtud,
);

let id = self.connections.insert(ConnectionMeta {
Expand Down
15 changes: 9 additions & 6 deletions quinn-proto/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -61,6 +61,7 @@ fn version_negotiate_client() {
..Default::default()
}),
None,
true,
);
let (_, mut client_ch) = client
.connect(client_config(), server_addr, "localhost")
Expand Down Expand Up @@ -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");
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down
16 changes: 4 additions & 12 deletions quinn-proto/src/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion quinn-udp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
5 changes: 5 additions & 0 deletions quinn-udp/src/fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,9 @@ pub fn udp_state() -> super::UdpState {
}
}

#[inline]
Ralith marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn may_fragment() -> bool {
true
}

pub const BATCH_SIZE: usize = 1;
8 changes: 8 additions & 0 deletions quinn-udp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
21 changes: 16 additions & 5 deletions quinn-udp/src/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)?;
djc marked this conversation as resolved.
Show resolved Hide resolved
// 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(())
Expand Down Expand Up @@ -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::*;
Expand Down
5 changes: 5 additions & 0 deletions quinn-udp/src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,8 @@ pub fn udp_state() -> super::UdpState {
}

pub const BATCH_SIZE: usize = 1;

#[inline]
pub(crate) fn may_fragment() -> bool {
false
}
15 changes: 1 addition & 14 deletions quinn/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
Expand Down
4 changes: 1 addition & 3 deletions quinn/examples/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
18 changes: 1 addition & 17 deletions quinn/examples/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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))
}
Expand Down
4 changes: 1 addition & 3 deletions quinn/examples/insecure_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions quinn/examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading