From f31cd1edb9fe25385a24d074ce866c8cd0fe70d2 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 18 Jul 2025 17:26:07 +0200 Subject: [PATCH] Do not track lost_packets twice The number of lost packets is already tracked as path stats, there is no need to duplicate it on the connection struct itself. While access was only ever in tests, the field would still be present in release builds I think. --- quinn-proto/src/connection/mod.rs | 10 ---------- quinn-proto/src/tests/mod.rs | 18 +++++++++--------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 364860400..0152d9b81 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -167,8 +167,6 @@ pub struct Connection { /// The value that the server included in the Source Connection ID field of a Retry packet, if /// one was received retry_src_cid: Option, - /// Total number of outgoing packets that have been deemed lost - lost_packets: u64, events: VecDeque, endpoint_events: VecDeque, /// Whether the spin bit is in use for this connection @@ -307,7 +305,6 @@ impl Connection { orig_rem_cid: rem_cid, initial_dst_cid: init_cid, retry_src_cid: None, - lost_packets: 0, events: VecDeque::new(), endpoint_events: VecDeque::new(), spin_enabled: config.allow_spin && rng.random_ratio(7, 8), @@ -1800,7 +1797,6 @@ impl Connection { if let Some(largest_lost) = lost_packets.last().cloned() { let old_bytes_in_flight = self.path.in_flight.bytes; let largest_lost_sent = self.spaces[pn_space].sent_packets[&largest_lost].time_sent; - self.lost_packets += lost_packets.len() as u64; self.stats.path.lost_packets += lost_packets.len() as u64; self.stats.path.lost_bytes += size_of_lost_packets; trace!( @@ -3668,12 +3664,6 @@ impl Connection { .map_or(true, |(timer, _)| timer == Timer::Idle) } - /// Total number of outgoing packets that have been deemed lost - #[cfg(test)] - pub(crate) fn lost_packets(&self) -> u64 { - self.lost_packets - } - /// Whether explicit congestion notification is in use on outgoing packets. #[cfg(test)] pub(crate) fn using_ecn(&self) -> bool { diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs index bb7dd0b5e..1ecb39812 100644 --- a/quinn-proto/src/tests/mod.rs +++ b/quinn-proto/src/tests/mod.rs @@ -596,7 +596,7 @@ fn zero_rtt_happypath() { Ok(Some(chunk)) if chunk.offset == 0 && chunk.bytes == MSG ); let _ = chunks.finalize(); - assert_eq!(pair.client_conn_mut(client_ch).lost_packets(), 0); + assert_eq!(pair.client_conn_mut(client_ch).stats().path.lost_packets, 0); } #[test] @@ -671,7 +671,7 @@ fn zero_rtt_rejection() { let mut chunks = recv.read(false).unwrap(); assert_eq!(chunks.next(usize::MAX), Err(ReadError::Blocked)); let _ = chunks.finalize(); - assert_eq!(pair.client_conn_mut(client_ch).lost_packets(), 0); + assert_eq!(pair.client_conn_mut(client_ch).stats().path.lost_packets, 0); } fn test_zero_rtt_incoming_limit(configure_server: F) { @@ -762,7 +762,7 @@ fn test_zero_rtt_incoming_limit(configure_server: assert_eq!(offset, CLIENT_WRITES); let _ = chunks.finalize(); assert_eq!( - pair.client_conn_mut(client_ch).lost_packets(), + pair.client_conn_mut(client_ch).stats().path.lost_packets, EXPECTED_DROPPED ); } @@ -1011,8 +1011,8 @@ fn key_update_simple() { ); let _ = chunks.finalize(); - assert_eq!(pair.client_conn_mut(client_ch).lost_packets(), 0); - assert_eq!(pair.server_conn_mut(server_ch).lost_packets(), 0); + assert_eq!(pair.client_conn_mut(client_ch).stats().path.lost_packets, 0); + assert_eq!(pair.server_conn_mut(server_ch).stats().path.lost_packets, 0); } #[test] @@ -1044,7 +1044,7 @@ fn key_update_reordered() { pair.client.finish_delay(); pair.drive(); - assert_eq!(pair.client_conn_mut(client_ch).lost_packets(), 0); + assert_eq!(pair.client_conn_mut(client_ch).stats().path.lost_packets, 0); assert_matches!( pair.server_conn_mut(server_ch).poll(), Some(Event::Stream(StreamEvent::Opened { dir: Dir::Bi })) @@ -1059,8 +1059,8 @@ fn key_update_reordered() { assert_eq!(buf2.bytes, MSG2); let _ = chunks.finalize(); - assert_eq!(pair.client_conn_mut(client_ch).lost_packets(), 0); - assert_eq!(pair.server_conn_mut(server_ch).lost_packets(), 0); + assert_eq!(pair.client_conn_mut(client_ch).stats().path.lost_packets, 0); + assert_eq!(pair.server_conn_mut(server_ch).stats().path.lost_packets, 0); } #[test] @@ -1699,7 +1699,7 @@ fn handshake_1rtt_handling() { pair.drive(); - assert!(pair.client_conn_mut(client_ch).lost_packets() != 0); + assert!(pair.client_conn_mut(client_ch).stats().path.lost_packets != 0); let mut recv = pair.server_recv(server_ch, s); let mut chunks = recv.read(false).unwrap(); assert_matches!(