Skip to content

Commit

Permalink
Use mtu instead of max_udp_payload_size in names
Browse files Browse the repository at this point in the history
Sponsored by Stormshield
  • Loading branch information
aochagavia authored and djc committed May 3, 2023
1 parent 7e495e2 commit 030a0e8
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 106 deletions.
2 changes: 1 addition & 1 deletion bench/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ pub fn transport_config(opt: &Opt) -> quinn::TransportConfig {
#[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_max_udp_payload_size(opt.initial_mtu);
config.initial_mtu(opt.initial_mtu);
config
}

Expand Down
4 changes: 2 additions & 2 deletions perf/src/bin/perf_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct Opt {
keylog: bool,
/// UDP payload size that the network must be capable of carrying
#[clap(long, default_value = "1200")]
initial_max_udp_payload_size: u16,
initial_mtu: u16,
/// Disable packet encryption/decryption (for debugging purpose)
#[clap(long = "no-protection")]
no_protection: bool,
Expand Down Expand Up @@ -131,7 +131,7 @@ 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_max_udp_payload_size(opt.initial_max_udp_payload_size);
transport.initial_mtu(opt.initial_mtu);

let mut cfg = if opt.no_protection {
quinn::ClientConfig::new(Arc::new(NoProtectionClientConfig::new(Arc::new(crypto))))
Expand Down
4 changes: 2 additions & 2 deletions perf/src/bin/perf_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct Opt {
keylog: bool,
/// UDP payload size that the network must be capable of carrying
#[clap(long, default_value = "1200")]
initial_max_udp_payload_size: u16,
initial_mtu: u16,
/// Disable packet encryption/decryption (for debugging purpose)
#[clap(long = "no-protection")]
no_protection: bool,
Expand Down Expand Up @@ -90,7 +90,7 @@ 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_max_udp_payload_size(opt.initial_max_udp_payload_size);
transport.initial_mtu(opt.initial_mtu);

let mut server_config = if opt.no_protection {
quinn::ServerConfig::with_crypto(Arc::new(NoProtectionServerConfig::new(Arc::new(crypto))))
Expand Down
32 changes: 15 additions & 17 deletions quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ 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, DEFAULT_SUPPORTED_VERSIONS, INITIAL_MTU, MAX_UDP_PAYLOAD,
};

/// Parameters governing the core QUIC state machine
Expand All @@ -37,8 +36,8 @@ pub struct TransportConfig {
pub(crate) packet_threshold: u32,
pub(crate) time_threshold: f32,
pub(crate) initial_rtt: Duration,
pub(crate) initial_max_udp_payload_size: u16,
pub(crate) max_guaranteed_udp_payload_size: u16,
pub(crate) initial_mtu: u16,
pub(crate) min_guaranteed_mtu: u16,
pub(crate) mtu_discovery_config: Option<MtuDiscoveryConfig>,

pub(crate) persistent_congestion_threshold: u32,
Expand Down Expand Up @@ -164,32 +163,31 @@ impl TransportConfig {
/// 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::max_guaranteed_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);
/// it down to [`TransportConfig::min_guaranteed_mtu`].
pub fn initial_mtu(&mut self, value: u16) -> &mut Self {
self.initial_mtu = value.max(INITIAL_MTU);
self
}

pub(crate) fn get_initial_max_udp_payload_size(&self) -> u16 {
self.initial_max_udp_payload_size
.max(self.max_guaranteed_udp_payload_size)
pub(crate) fn get_initial_mtu(&self) -> u16 {
self.initial_mtu.max(self.min_guaranteed_mtu)
}

/// The maximum 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`].
/// [`TransportConfig::initial_mtu`].
///
/// 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::initial_mtu`] together with
/// [`TransportConfig::mtu_discovery_config`] to set a maximum UDP payload size that robustly
/// adapts to the network.
pub fn max_guaranteed_udp_payload_size(&mut self, value: u16) -> &mut Self {
self.max_guaranteed_udp_payload_size = value.max(INITIAL_MAX_UDP_PAYLOAD_SIZE);
pub fn min_guaranteed_mtu(&mut self, value: u16) -> &mut Self {
self.min_guaranteed_mtu = value.max(INITIAL_MTU);
self
}

Expand Down Expand Up @@ -315,8 +313,8 @@ impl Default for TransportConfig {
packet_threshold: 3,
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,
max_guaranteed_udp_payload_size: INITIAL_MAX_UDP_PAYLOAD_SIZE,
initial_mtu: INITIAL_MTU,
min_guaranteed_mtu: INITIAL_MTU,
mtu_discovery_config: None,

persistent_congestion_threshold: 3,
Expand Down Expand Up @@ -406,7 +404,7 @@ impl fmt::Debug for TransportConfig {
///
/// Since the search space for MTUs is quite big (the smallest possible MTU is 1200, and the highest
/// is 65527), Quinn performs a binary search to keep the number of probes as low as possible. The
/// lower bound of the search is equal to [`TransportConfig::initial_max_udp_payload_size`] in the
/// lower bound of the search is equal to [`TransportConfig::initial_mtu`] in the
/// initial MTU discovery run, and is equal to the currently discovered MTU in subsequent runs. The
/// upper bound is determined by the minimum of [`MtuDiscoveryConfig::upper_bound`] and the
/// `max_udp_payload_size` transport parameter received from the peer during the handshake.
Expand Down
12 changes: 6 additions & 6 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,9 @@ impl Connection {
config.initial_rtt,
config
.congestion_controller_factory
.build(now, config.get_initial_max_udp_payload_size()),
config.get_initial_max_udp_payload_size(),
config.max_guaranteed_udp_payload_size,
.build(now, config.get_initial_mtu()),
config.get_initial_mtu(),
config.min_guaranteed_mtu,
None,
config.mtu_discovery_config.clone(),
now,
Expand Down Expand Up @@ -2739,9 +2739,9 @@ impl Connection {
self.config.initial_rtt,
self.config
.congestion_controller_factory
.build(now, self.config.get_initial_max_udp_payload_size()),
self.config.get_initial_max_udp_payload_size(),
self.config.max_guaranteed_udp_payload_size,
.build(now, self.config.get_initial_mtu()),
self.config.get_initial_mtu(),
self.config.min_guaranteed_mtu,
Some(peer_max_udp_payload_size),
self.config.mtu_discovery_config.clone(),
now,
Expand Down
55 changes: 26 additions & 29 deletions quinn-proto/src/connection/mtud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ pub(crate) struct MtuDiscovery {
impl MtuDiscovery {
pub(crate) fn new(
initial_plpmtu: u16,
max_guaranteed_udp_payload_size: u16,
min_guaranteed_mtu: u16,
peer_max_udp_payload_size: Option<u16>,
config: MtuDiscoveryConfig,
) -> Self {
debug_assert!(
initial_plpmtu >= max_guaranteed_udp_payload_size,
"initial_max_udp_payload_size must be at least {max_guaranteed_udp_payload_size}"
initial_plpmtu >= min_guaranteed_mtu,
"initial_max_udp_payload_size must be at least {min_guaranteed_mtu}"
);

let mut mtud = Self::with_state(
initial_plpmtu,
max_guaranteed_udp_payload_size,
min_guaranteed_mtu,
Some(EnabledMtuDiscovery::new(config)),
);

Expand All @@ -44,19 +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, max_guaranteed_udp_payload_size: u16) -> Self {
Self::with_state(plpmtu, max_guaranteed_udp_payload_size, None)
pub(crate) fn disabled(plpmtu: u16, min_guaranteed_mtu: u16) -> Self {
Self::with_state(plpmtu, min_guaranteed_mtu, None)
}

fn with_state(
current_mtu: u16,
max_guaranteed_udp_payload_size: u16,
min_guaranteed_mtu: u16,
state: Option<EnabledMtuDiscovery>,
) -> Self {
Self {
current_mtu,
state,
black_hole_detector: BlackHoleDetector::new(max_guaranteed_udp_payload_size),
black_hole_detector: BlackHoleDetector::new(min_guaranteed_mtu),
}
}

Expand Down Expand Up @@ -106,7 +106,7 @@ impl MtuDiscovery {
.and_then(|state| state.on_probe_acked(packet_number))
{
self.current_mtu = new_mtu;
trace!(max_udp_payload_size = self.current_mtu, "new MTU detected");
trace!(current_mtu = self.current_mtu, "new MTU detected");

self.black_hole_detector.on_probe_acked();
true
Expand Down Expand Up @@ -151,13 +151,13 @@ impl MtuDiscovery {
/// Returns true if a black hole was detected
///
/// Calling this function will close the previous loss burst. If a black hole is detected, the
/// current MTU will be reset to `max_guaranteed_udp_payload_size`.
/// current MTU will be reset to `min_guaranteed_mtu`.
pub(crate) fn black_hole_detected(&mut self, now: Instant) -> bool {
if !self.black_hole_detector.black_hole_detected() {
return false;
}

self.current_mtu = self.black_hole_detector.max_guaranteed_udp_payload_size;
self.current_mtu = self.black_hole_detector.min_guaranteed_mtu;

if let Some(state) = &mut self.state {
state.on_black_hole_detected(now);
Expand Down Expand Up @@ -220,7 +220,7 @@ impl EnabledMtuDiscovery {
// Retransmit lost probes, if any
if 0 < state.lost_probe_count && state.lost_probe_count < MAX_PROBE_RETRANSMITS {
state.in_flight_probe = Some(next_packet_number);
return Some(state.last_probed_udp_payload_size);
return Some(state.last_probed_mtu);
}

let last_probe_succeeded = state.lost_probe_count == 0;
Expand All @@ -233,7 +233,7 @@ impl EnabledMtuDiscovery {

if let Some(probe_udp_payload_size) = state.next_mtu_to_probe(last_probe_succeeded) {
state.in_flight_probe = Some(next_packet_number);
state.last_probed_udp_payload_size = probe_udp_payload_size;
state.last_probed_mtu = probe_udp_payload_size;
return Some(probe_udp_payload_size);
} else {
let next_mtud_activation = now + self.config.interval;
Expand All @@ -253,7 +253,7 @@ impl EnabledMtuDiscovery {
Phase::Searching(state) if state.in_flight_probe == Some(packet_number) => {
state.in_flight_probe = None;
state.lost_probe_count = 0;
Some(state.last_probed_udp_payload_size)
Some(state.last_probed_mtu)
}
_ => None,
}
Expand Down Expand Up @@ -293,7 +293,7 @@ struct SearchState {
/// The upper bound for the current binary search
upper_bound: u16,
/// The UDP payload size we last sent a probe for
last_probed_udp_payload_size: u16,
last_probed_mtu: u16,
/// Packet number of an in-flight probe (if any)
in_flight_probe: Option<u64>,
/// Lost probes at the current probe size
Expand All @@ -320,7 +320,7 @@ impl SearchState {
upper_bound,
// During initialization, we consider the lower bound to have already been
// successfully probed
last_probed_udp_payload_size: lower_bound,
last_probed_mtu: lower_bound,
}
}

Expand All @@ -329,23 +329,20 @@ impl SearchState {
debug_assert_eq!(self.in_flight_probe, None);

if last_probe_succeeded {
self.lower_bound = self.last_probed_udp_payload_size;
self.lower_bound = self.last_probed_mtu;
} else {
self.upper_bound = self.last_probed_udp_payload_size - 1;
self.upper_bound = self.last_probed_mtu - 1;
}

let next_mtu = (self.lower_bound as i32 + self.upper_bound as i32) / 2;

// Binary search stopping condition
if ((next_mtu - self.last_probed_udp_payload_size as i32).unsigned_abs() as u16)
if ((next_mtu - self.last_probed_mtu as i32).unsigned_abs() as u16)
< BINARY_SEARCH_MINIMUM_CHANGE
{
// Special case: if the upper bound is far enough, we want to probe it as a last
// step (otherwise we will never achieve the upper bound)
if self
.upper_bound
.saturating_sub(self.last_probed_udp_payload_size)
>= BINARY_SEARCH_MINIMUM_CHANGE
if self.upper_bound.saturating_sub(self.last_probed_mtu) >= BINARY_SEARCH_MINIMUM_CHANGE
{
return Some(self.upper_bound);
}
Expand All @@ -371,29 +368,29 @@ struct BlackHoleDetector {
suspicious_loss_bursts: u8,
/// Indicates whether the current loss burst has any non-suspicious packets
///
/// Non-suspicious packets are non-probe packets of size <= `max_guaranteed_udp_payload_size`
/// Non-suspicious packets are non-probe packets of size <= `min_guaranteed_mtu`
loss_burst_has_non_suspicious_packets: bool,
/// The largest suspicious packet that was lost in the current burst
///
/// Suspicious packets are non-probe packets of size > `max_guaranteed_udp_payload_size`
/// Suspicious packets are non-probe packets of size > `min_guaranteed_mtu`
largest_suspicious_packet_lost: Option<u64>,
/// The largest non-probe packet that was lost (used to keep track of loss bursts)
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
max_guaranteed_udp_payload_size: u16,
min_guaranteed_mtu: u16,
}

impl BlackHoleDetector {
fn new(max_guaranteed_udp_payload_size: u16) -> Self {
fn new(min_guaranteed_mtu: 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,
max_guaranteed_udp_payload_size,
min_guaranteed_mtu,
}
}

Expand Down Expand Up @@ -426,7 +423,7 @@ impl BlackHoleDetector {
self.finish_loss_burst();
}

if packet_bytes <= self.max_guaranteed_udp_payload_size {
if packet_bytes <= self.min_guaranteed_mtu {
self.loss_burst_has_non_suspicious_packets = true;
} else {
self.largest_suspicious_packet_lost = Some(packet_number);
Expand Down
18 changes: 5 additions & 13 deletions quinn-proto/src/connection/pacing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,12 @@ pub(super) struct Pacer {

impl Pacer {
/// Obtains a new [`Pacer`].
pub(super) fn new(
smoothed_rtt: Duration,
window: u64,
max_udp_payload_size: u16,
now: Instant,
) -> Self {
let capacity = optimal_capacity(smoothed_rtt, window, max_udp_payload_size);
pub(super) fn new(smoothed_rtt: Duration, window: u64, mtu: u16, now: Instant) -> Self {
let capacity = optimal_capacity(smoothed_rtt, window, mtu);
Self {
capacity,
last_window: window,
last_mtu: max_udp_payload_size,
last_mtu: mtu,
tokens: capacity,
prev: now,
}
Expand Down Expand Up @@ -131,17 +126,14 @@ impl Pacer {
/// tokens for the extra-elapsed time can be stored.
///
/// Too long burst intervals make pacing less effective.
fn optimal_capacity(smoothed_rtt: Duration, window: u64, max_udp_payload_size: u16) -> u64 {
fn optimal_capacity(smoothed_rtt: Duration, window: u64, mtu: u16) -> u64 {
let rtt = smoothed_rtt.as_nanos().max(1);

let capacity = ((window as u128 * BURST_INTERVAL_NANOS) / rtt) as u64;

// Small bursts are less efficient (no GSO), could increase latency and don't effectively
// use the channel's buffer capacity. Large bursts might block the connection on sending.
capacity.clamp(
MIN_BURST_SIZE * max_udp_payload_size as u64,
MAX_BURST_SIZE * max_udp_payload_size as u64,
)
capacity.clamp(MIN_BURST_SIZE * mtu as u64, MAX_BURST_SIZE * mtu as u64)
}

/// The burst interval
Expand Down
Loading

0 comments on commit 030a0e8

Please sign in to comment.