From 9cb1035f04da2058d5a50e8e41b70dc782d556f9 Mon Sep 17 00:00:00 2001 From: David Emett Date: Mon, 29 Sep 2025 17:03:09 +0100 Subject: [PATCH 1/2] Fix some comparisons in the black hole detector Lost packets with size exactly matching the minimum MTU should not be treated as suspicious; by definition the MTU will never be reduced below this. Similarly, lost packets with size exactly matching a more recent successfully transmitted packet should not be treated as suspicious. --- quinn-proto/src/connection/mtud.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quinn-proto/src/connection/mtud.rs b/quinn-proto/src/connection/mtud.rs index b690731b41..51a5f4763e 100644 --- a/quinn-proto/src/connection/mtud.rs +++ b/quinn-proto/src/connection/mtud.rs @@ -450,9 +450,9 @@ impl BlackHoleDetector { }; // If a loss burst contains a packet smaller than the minimum MTU or a more recently // transmitted packet, it is not suspicious. - if burst.smallest_packet_size < self.min_mtu + if burst.smallest_packet_size <= self.min_mtu || (burst.latest_non_probe < self.largest_post_loss_packet - && burst.smallest_packet_size < self.acked_mtu) + && burst.smallest_packet_size <= self.acked_mtu) { return; } From 3e4cbb0e781273f6c1cca031026f8b2d80b0ad27 Mon Sep 17 00:00:00 2001 From: David Emett Date: Mon, 29 Sep 2025 17:15:36 +0100 Subject: [PATCH 2/2] Relax MTU discovery state assertion While we will never start probing before the connection has been established, it is possible for black hole detection to trigger, which will put MTU discovery into the Complete state. --- quinn-proto/src/connection/mtud.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/quinn-proto/src/connection/mtud.rs b/quinn-proto/src/connection/mtud.rs index 51a5f4763e..7e6fc1d8ae 100644 --- a/quinn-proto/src/connection/mtud.rs +++ b/quinn-proto/src/connection/mtud.rs @@ -83,9 +83,14 @@ impl MtuDiscovery { self.current_mtu = self.current_mtu.min(peer_max_udp_payload_size); if let Some(state) = self.state.as_mut() { - // MTUD is only active after the connection has been fully established, so it is - // guaranteed we will receive the peer's transport parameters before we start probing - debug_assert!(matches!(state.phase, Phase::Initial)); + // It is possible for black hole detection to trigger before the connection has been + // fully established, if the initial MTU is greater the minimum MTU. We should never + // send probes before the connection has been fully established and we have received + // the peer's transport parameters though. + debug_assert!( + !matches!(state.phase, Phase::Searching(_)), + "Transport parameters received after MTU probing started" + ); state.peer_max_udp_payload_size = peer_max_udp_payload_size; } } @@ -733,10 +738,14 @@ mod tests { #[cfg(debug_assertions)] #[test] - #[should_panic] - fn mtu_discovery_with_peer_max_udp_payload_size_after_search_panics() { + #[should_panic(expected = "Transport parameters received after MTU probing started")] + fn mtu_discovery_with_peer_max_udp_payload_size_during_search_panics() { let mut mtud = default_mtud(); - drive_to_completion(&mut mtud, Instant::now(), 1500); + assert!(mtud.poll_transmit(Instant::now(), 0).is_some()); + assert!(matches!( + mtud.state.as_ref().unwrap().phase, + Phase::Searching(_) + )); mtud.on_peer_max_udp_payload_size_received(1300); }