diff --git a/noq-proto/src/connection/qlog.rs b/noq-proto/src/connection/qlog.rs index 260231d22..aa08325ee 100644 --- a/noq-proto/src/connection/qlog.rs +++ b/noq-proto/src/connection/qlog.rs @@ -212,7 +212,7 @@ impl QlogSink { }), frames: None, trigger: Some( - match info.time_sent.saturating_duration_since(now) >= loss_delay { + match now.saturating_duration_since(info.time_sent) >= loss_delay { true => PacketLostTrigger::TimeThreshold, false => PacketLostTrigger::ReorderingThreshold, }, diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index 892529371..269dff251 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -5135,3 +5135,105 @@ fn sent_packets_stats_snapshot_test() { assert_eq!(client_stats2.sent_packets, 14); assert_eq!(server_stats2.sent_packets, 11); } + +#[cfg(feature = "qlog")] +#[test] +fn qlog_packet_lost_trigger() { + use std::{ + io::{self, Write}, + sync::Mutex, + }; + + use qlog::{ + events::{EventData, quic::PacketLostTrigger}, + reader::{Event as QlogEvent, QlogSeqReader}, + }; + + use crate::{ConnectionId, QlogConfig, QlogFactory, Side}; + + /// Captures the qlog trace of every connection into one buffer + #[derive(Clone, Default)] + struct SharedBuffer(Arc>>); + + impl QlogFactory for SharedBuffer { + fn for_connection( + &self, + _side: Side, + _remote: SocketAddr, + _initial_dst_cid: ConnectionId, + _now: Instant, + ) -> Option { + Some(QlogConfig::new(Box::new(self.clone()))) + } + } + + impl Write for SharedBuffer { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + let _guard = subscribe(); + let qlog = SharedBuffer::default(); + let mut transport = TransportConfig::default(); + transport + .deterministic_packet_numbers(true) + .qlog_factory(Arc::new(qlog.clone())); + let mut config = client_config(); + config.transport_config(Arc::new(transport)); + let mut pair = Pair::default(); + let (client_ch, _) = pair.connect_with(config); + pair.drive(); + + // Drop a packet, then deliver fewer later packets than the packet threshold, so that only the + // time threshold can declare it lost + pair.client_conn_mut(client_ch).ping(); + pair.client.drive(pair.time); + assert_eq!(pair.client.outbound.len(), 1); + pair.client.outbound.clear(); + pair.client_conn_mut(client_ch).ping(); + pair.drive(); + + // Drop a packet, then deliver as many later packets as the packet threshold without advancing + // time, so that only the packet threshold can declare it lost + pair.client_conn_mut(client_ch).ping(); + pair.client.drive(pair.time); + assert_eq!(pair.client.outbound.len(), 1); + pair.client.outbound.clear(); + for _ in 0..3 { + pair.client_conn_mut(client_ch).ping(); + pair.client.drive(pair.time); + } + assert_eq!(pair.client.outbound.len(), 3); + pair.drive(); + + assert_eq!( + pair.client_conn_mut(client_ch) + .path_stats(PathId::ZERO) + .unwrap() + .lost_packets, + 2 + ); + let triggers = QlogSeqReader::new(Box::new(&qlog.0.lock().unwrap()[..])) + .unwrap() + .filter_map(|event| match event { + QlogEvent::Qlog(event) => match event.data { + EventData::QuicPacketLost(lost) => lost.trigger, + _ => None, + }, + QlogEvent::Json(_) => None, + }) + .collect::>(); + assert_eq!( + triggers, + [ + PacketLostTrigger::TimeThreshold, + PacketLostTrigger::ReorderingThreshold + ] + ); +}