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

Rename min_guaranteed_mtu to min_mtu for clarity #1552

Merged
merged 1 commit into from
May 3, 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
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