Skip to content

Commit

Permalink
Rename min_guaranteed_mtu to min_mtu for clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
Ralith authored and djc committed May 3, 2023
1 parent 030a0e8 commit 99e462f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 40 deletions.
12 changes: 6 additions & 6 deletions quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub struct TransportConfig {
pub(crate) time_threshold: f32,
pub(crate) initial_rtt: Duration,
pub(crate) initial_mtu: u16,
pub(crate) min_guaranteed_mtu: u16,
pub(crate) min_mtu: u16,
pub(crate) mtu_discovery_config: Option<MtuDiscoveryConfig>,

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

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

/// The maximum UDP payload size guaranteed to be supported by the network.
Expand All @@ -186,8 +186,8 @@ impl TransportConfig {
/// [`TransportConfig::initial_mtu`] together with
/// [`TransportConfig::mtu_discovery_config`] to set a maximum UDP payload size that robustly
/// adapts to the network.
pub fn min_guaranteed_mtu(&mut self, value: u16) -> &mut Self {
self.min_guaranteed_mtu = value.max(INITIAL_MTU);
pub fn min_mtu(&mut self, value: u16) -> &mut Self {
self.min_mtu = value.max(INITIAL_MTU);
self
}

Expand Down Expand Up @@ -314,7 +314,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_mtu: INITIAL_MTU,
min_guaranteed_mtu: INITIAL_MTU,
min_mtu: INITIAL_MTU,
mtu_discovery_config: None,

persistent_congestion_threshold: 3,
Expand Down
4 changes: 2 additions & 2 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl Connection {
.congestion_controller_factory
.build(now, config.get_initial_mtu()),
config.get_initial_mtu(),
config.min_guaranteed_mtu,
config.min_mtu,
None,
config.mtu_discovery_config.clone(),
now,
Expand Down Expand Up @@ -2741,7 +2741,7 @@ impl Connection {
.congestion_controller_factory
.build(now, self.config.get_initial_mtu()),
self.config.get_initial_mtu(),
self.config.min_guaranteed_mtu,
self.config.min_mtu,
Some(peer_max_udp_payload_size),
self.config.mtu_discovery_config.clone(),
now,
Expand Down
36 changes: 16 additions & 20 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,
min_guaranteed_mtu: u16,
min_mtu: u16,
peer_max_udp_payload_size: Option<u16>,
config: MtuDiscoveryConfig,
) -> Self {
debug_assert!(
initial_plpmtu >= min_guaranteed_mtu,
"initial_max_udp_payload_size must be at least {min_guaranteed_mtu}"
initial_plpmtu >= min_mtu,
"initial_max_udp_payload_size must be at least {min_mtu}"
);

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

Expand All @@ -44,19 +44,15 @@ impl MtuDiscovery {
}

/// MTU discovery will be disabled and the current MTU will be fixed to the provided value
pub(crate) fn disabled(plpmtu: u16, min_guaranteed_mtu: u16) -> Self {
Self::with_state(plpmtu, min_guaranteed_mtu, None)
pub(crate) fn disabled(plpmtu: u16, min_mtu: u16) -> Self {
Self::with_state(plpmtu, min_mtu, None)
}

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

Expand Down Expand Up @@ -151,13 +147,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 `min_guaranteed_mtu`.
/// current MTU will be reset to `min_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.min_guaranteed_mtu;
self.current_mtu = self.black_hole_detector.min_mtu;

if let Some(state) = &mut self.state {
state.on_black_hole_detected(now);
Expand Down Expand Up @@ -368,29 +364,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 <= `min_guaranteed_mtu`
/// Non-suspicious packets are non-probe packets of size <= `min_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 > `min_guaranteed_mtu`
/// Suspicious packets are non-probe packets of size > `min_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
min_guaranteed_mtu: u16,
min_mtu: u16,
}

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

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

if packet_bytes <= self.min_guaranteed_mtu {
if packet_bytes <= self.min_mtu {
self.loss_burst_has_non_suspicious_packets = true;
} else {
self.largest_suspicious_packet_lost = Some(packet_number);
Expand Down
16 changes: 4 additions & 12 deletions quinn-proto/src/connection/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl PathData {
initial_rtt: Duration,
congestion: Box<dyn congestion::Controller>,
initial_mtu: u16,
min_guaranteed_mtu: u16,
min_mtu: u16,
peer_max_udp_payload_size: Option<u16>,
mtud_config: Option<MtuDiscoveryConfig>,
now: Instant,
Expand All @@ -55,17 +55,9 @@ impl PathData {
validated,
total_sent: 0,
total_recvd: 0,
mtud: mtud_config.map_or(
MtuDiscovery::disabled(initial_mtu, min_guaranteed_mtu),
|config| {
MtuDiscovery::new(
initial_mtu,
min_guaranteed_mtu,
peer_max_udp_payload_size,
config,
)
},
),
mtud: mtud_config.map_or(MtuDiscovery::disabled(initial_mtu, min_mtu), |config| {
MtuDiscovery::new(initial_mtu, min_mtu, peer_max_udp_payload_size, config)
}),
first_packet_after_rtt_sample: None,
}
}
Expand Down

0 comments on commit 99e462f

Please sign in to comment.