From 39d5ed9c962d30c19a05c09b61ac210796e081d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Mon, 10 Nov 2025 10:39:18 +0100 Subject: [PATCH 1/3] Refactor `QlogSink::emit_packet_lost` to take `loss_delay` instead of `lost_send_time` --- quinn-proto/src/connection/mod.rs | 2 +- quinn-proto/src/connection/qlog.rs | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 869fa3df4d..a33bbb7a28 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1796,7 +1796,7 @@ impl Connection { self.config.qlog_sink.emit_packet_lost( packet, &info, - lost_send_time, + loss_delay, pn_space, now, self.orig_rem_cid, diff --git a/quinn-proto/src/connection/qlog.rs b/quinn-proto/src/connection/qlog.rs index a324746f13..dafba3c477 100644 --- a/quinn-proto/src/connection/qlog.rs +++ b/quinn-proto/src/connection/qlog.rs @@ -3,6 +3,7 @@ #[cfg(feature = "qlog")] use std::sync::{Arc, Mutex}; +use std::time::Duration; #[cfg(feature = "qlog")] use qlog::{ @@ -86,7 +87,7 @@ impl QlogSink { &self, pn: u64, info: &SentPacket, - lost_send_time: Instant, + loss_delay: Duration, space: SpaceId, now: Instant, orig_rem_cid: ConnectionId, @@ -105,10 +106,12 @@ impl QlogSink { ..Default::default() }), frames: None, - trigger: Some(match info.time_sent <= lost_send_time { - true => PacketLostTrigger::TimeThreshold, - false => PacketLostTrigger::ReorderingThreshold, - }), + trigger: Some( + match info.time_sent.saturating_duration_since(now) >= loss_delay { + true => PacketLostTrigger::TimeThreshold, + false => PacketLostTrigger::ReorderingThreshold, + }, + ), }; stream.emit_event(orig_rem_cid, EventData::PacketLost(event), now); From e820c01e41540e69b657ca2ee4e6ffdfcefc961e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Mon, 3 Nov 2025 14:41:18 +0100 Subject: [PATCH 2/3] Remove `instant_saturating_sub` fn in favor of `Instant::saturating_duration_since` --- quinn-proto/src/connection/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index a33bbb7a28..26f8f95e16 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1523,7 +1523,7 @@ impl Connection { Duration::from_micros(ack.delay << self.peer_params.ack_delay_exponent.0), ) }; - let rtt = instant_saturating_sub(now, self.spaces[space].largest_acked_packet_sent); + let rtt = now.saturating_duration_since(self.spaces[space].largest_acked_packet_sent); self.path.rtt.update(ack_delay, rtt); if self.path.first_packet_after_rtt_sample.is_none() { self.path.first_packet_after_rtt_sample = @@ -4056,10 +4056,6 @@ pub enum Event { DatagramsUnblocked, } -fn instant_saturating_sub(x: Instant, y: Instant) -> Duration { - if x > y { x - y } else { Duration::ZERO } -} - fn get_max_ack_delay(params: &TransportParameters) -> Duration { Duration::from_micros(params.max_ack_delay.0 * 1000) } From 815f2630ede06da374e5713bc86adc4ac40ad49e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Mon, 10 Nov 2025 15:05:06 +0100 Subject: [PATCH 3/3] Avoid underflow panic in packet loss `Instant` calculations --- quinn-proto/src/connection/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 26f8f95e16..b4a953597e 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1712,8 +1712,6 @@ impl Connection { let rtt = self.path.rtt.conservative(); let loss_delay = cmp::max(rtt.mul_f32(self.config.time_threshold), TIMER_GRANULARITY); - // Packets sent before this time are deemed lost. - let lost_send_time = now.checked_sub(loss_delay).unwrap(); let largest_acked_packet = self.spaces[pn_space].largest_acked_packet.unwrap(); let packet_threshold = self.config.packet_threshold as u64; let mut size_of_lost_packets = 0u64; @@ -1737,8 +1735,11 @@ impl Connection { persistent_congestion_start = None; } - if info.time_sent <= lost_send_time || largest_acked_packet >= packet + packet_threshold - { + // Packets sent before now - loss_delay are deemed lost. + // However, we avoid this subtraction as it can panic and there's no + // saturating equivalent of this substraction operation with a Duration. + let packet_too_old = now.saturating_duration_since(info.time_sent) >= loss_delay; + if packet_too_old || largest_acked_packet >= packet + packet_threshold { if Some(packet) == in_flight_mtu_probe { // Lost MTU probes are not included in `lost_packets`, because they should not // trigger a congestion control response