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

Also reset the MTU when the network paths has changed #15

Merged
merged 1 commit into from
Oct 24, 2024
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
21 changes: 9 additions & 12 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1306,19 +1306,16 @@ impl Connection {
self.path.congestion.as_ref()
}

/// Resets the congestion controller and round-trip estimator for the current path.
/// Resets path-specific settings.
///
/// This force-resets the congestion controller and round-trip estimator for the current
/// path.
pub fn reset_congestion_state(&mut self) {
let now = Instant::now();
self.path.rtt = RttEstimator::new(self.config.initial_rtt);
self.path.congestion = self
.config
.congestion_controller_factory
.clone()
.build(now, self.config.get_initial_mtu());
// TODO: probably needs MTU discovery reset as well.
/// This will force-reset several subsystems related to a specific network path.
/// Currently this is the congestion controller, round-trip estimator, and the MTU
/// discovery.
///
/// This is useful when it is know the underlying network path has changed and the old
/// state of these subsystems is no longer valid.
pub fn network_path_changed(&mut self) {
self.path.reset(&self.config);
}

/// Modify the number of remotely initiated streams that may be concurrently open
Expand Down
9 changes: 9 additions & 0 deletions quinn-proto/src/connection/mtud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ impl MtuDiscovery {
}
}

pub(super) fn reset(&mut self, current_mtu: u16, min_mtu: u16) {
self.current_mtu = current_mtu;
if let Some(state) = self.state.take() {
self.state = Some(EnabledMtuDiscovery::new(state.config));
self.on_peer_max_udp_payload_size_received(state.peer_max_udp_payload_size);
}
self.black_hole_detector = BlackHoleDetector::new(min_mtu);
}

/// Returns the current MTU
pub(crate) fn current_mtu(&self) -> u16 {
self.current_mtu
Expand Down
13 changes: 13 additions & 0 deletions quinn-proto/src/connection/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ impl PathData {
}
}

/// Resets RTT, congestion control and MTU states.
///
/// This is useful when it is known the underlying path has changed.
pub(super) fn reset(&mut self, config: &TransportConfig) {
let now = Instant::now();
self.rtt = RttEstimator::new(config.initial_rtt);
self.congestion = config
.congestion_controller_factory
.clone()
.build(now, config.get_initial_mtu());
self.mtud.reset(config.get_initial_mtu(), config.min_mtu);
}

/// Indicates whether we're a server that hasn't validated the peer's address and hasn't
/// received enough data from the peer to permit sending `bytes_to_send` additional bytes
pub(super) fn anti_amplification_blocked(&self, bytes_to_send: u64) -> bool {
Expand Down
15 changes: 10 additions & 5 deletions quinn/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,19 +956,24 @@ impl WeakConnectionHandle {
self.0.upgrade().is_some()
}

/// Resets the congestion controller and round-trip estimator for the current path.
/// Resets path-specific state.
///
/// This force-resets the congestion controller and round-trip estimator for the current
/// path.
/// This resets several subsystems keeping state for a specific network path. It is
/// useful if it is known that the underlying network path changed substantially.
///
/// Currently resets:
/// - RTT Estimator
/// - Congestion Controller
/// - MTU Discovery
///
/// # Returns
///
/// `true` if the connection still existed and the congestion controller state was
/// reset. `false` otherwise.
pub fn reset_congestion_state(&self) -> bool {
pub fn network_path_changed(&self) -> bool {
if let Some(inner) = self.0.upgrade() {
let mut inner_state = inner.state.lock("reset-congestion-state");
inner_state.inner.reset_congestion_state();
inner_state.inner.network_path_changed();
true
} else {
false
Expand Down