Cleanup tracking

neqo-crypto/src/agent.rs

@@ -875,14 +875,13 @@ impl Client {
         arg: *mut c_void,
     ) -> ssl::SECStatus {
         let mut info: MaybeUninit<ssl::SSLResumptionTokenInfo> = MaybeUninit::uninit();
-        if ssl::SSL_GetResumptionTokenInfo(
+        let info_res = &ssl::SSL_GetResumptionTokenInfo(
             token,
             len,
             info.as_mut_ptr(),
             c_uint::try_from(mem::size_of::<ssl::SSLResumptionTokenInfo>()).unwrap(),
-        )
-        .is_err()
-        {
+        );
+        if info_res.is_err() {
             // Ignore the token.
             return ssl::SECSuccess;
         }

neqo-transport/src/cc/classic_cc.rs

@@ -17,9 +17,9 @@ use crate::{
     cc::MAX_DATAGRAM_SIZE,
     packet::PacketNumber,
     qlog::{self, QlogMetric},
+    recovery::SentPacket,
     rtt::RttEstimate,
     sender::PACING_BURST_SIZE,
-    tracking::SentPacket,
 };
 #[rustfmt::skip] // to keep `::` and thus prevent conflict with `crate::qlog`
 use ::qlog::events::{quic::CongestionStateUpdated, EventData};
@@ -167,21 +167,21 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
             qdebug!(
                 "packet_acked this={:p}, pn={}, ps={}, ignored={}, lost={}, rtt_est={:?}",
                 self,
-                pkt.pn,
-                pkt.size,
+                pkt.pn(),
+                pkt.len(),
                 i32::from(!pkt.cc_outstanding()),
                 i32::from(pkt.lost()),
                 rtt_est,
             );
             if !pkt.cc_outstanding() {
                 continue;
             }
-            if pkt.pn < self.first_app_limited {
+            if pkt.pn() < self.first_app_limited {
                 is_app_limited = false;
             }
             // BIF is set to 0 on a path change, but in case that was because of a simple rebinding
             // event, we may still get ACKs for packets sent before the rebinding.
-            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size);
+            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.len());
 
             if !self.after_recovery_start(pkt) {
                 // Do not increase congestion window for packets sent before
@@ -194,7 +194,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
                 qlog::metrics_updated(&mut self.qlog, &[QlogMetric::InRecovery(false)]);
             }
 
-            new_acked += pkt.size;
+            new_acked += pkt.len();
         }
 
         if is_app_limited {
@@ -269,12 +269,12 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
             qdebug!(
                 "packet_lost this={:p}, pn={}, ps={}",
                 self,
-                pkt.pn,
-                pkt.size
+                pkt.pn(),
+                pkt.len()
             );
             // BIF is set to 0 on a path change, but in case that was because of a simple rebinding
             // event, we may still declare packets lost that were sent before the rebinding.
-            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size);
+            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.len());
         }
         qlog::metrics_updated(
             &mut self.qlog,
@@ -308,13 +308,13 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
 
     fn discard(&mut self, pkt: &SentPacket) {
         if pkt.cc_outstanding() {
-            assert!(self.bytes_in_flight >= pkt.size);
-            self.bytes_in_flight -= pkt.size;
+            assert!(self.bytes_in_flight >= pkt.len());
+            self.bytes_in_flight -= pkt.len();
             qlog::metrics_updated(
                 &mut self.qlog,
                 &[QlogMetric::BytesInFlight(self.bytes_in_flight)],
             );
-            qtrace!([self], "Ignore pkt with size {}", pkt.size);
+            qtrace!([self], "Ignore pkt with size {}", pkt.len());
         }
     }
 
@@ -329,7 +329,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
     fn on_packet_sent(&mut self, pkt: &SentPacket) {
         // Record the recovery time and exit any transient state.
         if self.state.transient() {
-            self.recovery_start = Some(pkt.pn);
+            self.recovery_start = Some(pkt.pn());
             self.state.update();
         }
 
@@ -341,15 +341,15 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
             // window. Assume that all in-flight packets up to this one are NOT app-limited.
             // However, subsequent packets might be app-limited. Set `first_app_limited` to the
             // next packet number.
-            self.first_app_limited = pkt.pn + 1;
+            self.first_app_limited = pkt.pn() + 1;
         }
 
-        self.bytes_in_flight += pkt.size;
+        self.bytes_in_flight += pkt.len();
         qdebug!(
             "packet_sent this={:p}, pn={}, ps={}",
             self,
-            pkt.pn,
-            pkt.size
+            pkt.pn(),
+            pkt.len()
         );
         qlog::metrics_updated(
             &mut self.qlog,
@@ -448,20 +448,20 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> {
         let cutoff = max(first_rtt_sample_time, prev_largest_acked_sent);
         for p in lost_packets
             .iter()
-            .skip_while(|p| Some(p.time_sent) < cutoff)
+            .skip_while(|p| Some(p.time_sent()) < cutoff)
         {
-            if p.pn != last_pn + 1 {
+            if p.pn() != last_pn + 1 {
                 // Not a contiguous range of lost packets, start over.
                 start = None;
             }
-            last_pn = p.pn;
+            last_pn = p.pn();
             if !p.cc_in_flight() {
                 // Not interesting, keep looking.
                 continue;
             }
             if let Some(t) = start {
                 let elapsed = p
-                    .time_sent
+                    .time_sent()
                     .checked_duration_since(t)
                     .expect("time is monotonic");
                 if elapsed > pc_period {
@@ -476,7 +476,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> {
                     return true;
                 }
             } else {
-                start = Some(p.time_sent);
+                start = Some(p.time_sent());
             }
         }
         false
@@ -490,7 +490,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> {
         // state and update the variable `self.recovery_start`. Before the
         // first recovery, all packets were sent after the recovery event,
         // allowing to reduce the cwnd on congestion events.
-        !self.state.transient() && self.recovery_start.map_or(true, |pn| packet.pn >= pn)
+        !self.state.transient() && self.recovery_start.map_or(true, |pn| packet.pn() >= pn)
     }
 
     /// Handle a congestion event.
@@ -560,8 +560,8 @@ mod tests {
             CongestionControl, CongestionControlAlgorithm, CWND_INITIAL_PKTS, MAX_DATAGRAM_SIZE,
         },
         packet::{PacketNumber, PacketType},
+        recovery::SentPacket,
         rtt::RttEstimate,
-        tracking::SentPacket,
     };
 
     const PTO: Duration = Duration::from_millis(100);
@@ -923,13 +923,13 @@ mod tests {
     fn persistent_congestion_ack_eliciting() {
         let mut lost = make_lost(&[1, PERSISTENT_CONG_THRESH + 2]);
         lost[0] = SentPacket::new(
-            lost[0].pt,
-            lost[0].pn,
-            lost[0].ecn_mark,
-            lost[0].time_sent,
+            lost[0].packet_type(),
+            lost[0].pn(),
+            lost[0].ecn_mark(),
+            lost[0].time_sent(),
             false,
             Vec::new(),
-            lost[0].size,
+            lost[0].len(),
         );
         assert!(!persistent_congestion_by_pto(
             ClassicCongestionControl::new(NewReno::default()),

neqo-transport/src/cc/mod.rs

@@ -14,7 +14,7 @@ use std::{
 
 use neqo_common::qlog::NeqoQlog;
 
-use crate::{path::PATH_MTU_V6, rtt::RttEstimate, tracking::SentPacket, Error};
+use crate::{path::PATH_MTU_V6, recovery::SentPacket, rtt::RttEstimate, Error};
 
 mod classic_cc;
 mod cubic;

neqo-transport/src/cc/tests/cubic.rs

@@ -25,8 +25,8 @@ use crate::{
         CongestionControl, MAX_DATAGRAM_SIZE, MAX_DATAGRAM_SIZE_F64,
     },
     packet::PacketType,
+    recovery::SentPacket,
     rtt::RttEstimate,
-    tracking::SentPacket,
 };
 
 const RTT: Duration = Duration::from_millis(100);

neqo-transport/src/cc/tests/new_reno.rs

@@ -17,8 +17,8 @@ use crate::{
         MAX_DATAGRAM_SIZE,
     },
     packet::PacketType,
+    recovery::SentPacket,
     rtt::RttEstimate,
-    tracking::SentPacket,
 };
 
 const PTO: Duration = Duration::from_millis(100);
@@ -133,14 +133,14 @@ fn issue_876() {
 
     // and ack it. cwnd increases slightly
     cc.on_packets_acked(&sent_packets[6..], &RTT_ESTIMATE, time_now);
-    assert_eq!(cc.acked_bytes(), sent_packets[6].size);
+    assert_eq!(cc.acked_bytes(), sent_packets[6].len());
     cwnd_is_halved(&cc);
     assert_eq!(cc.bytes_in_flight(), 5 * MAX_DATAGRAM_SIZE - 2);
 
     // Packet from before is lost. Should not hurt cwnd.
     cc.on_packets_lost(Some(time_now), None, PTO, &sent_packets[1..2]);
     assert!(!cc.recovery_packet());
-    assert_eq!(cc.acked_bytes(), sent_packets[6].size);
+    assert_eq!(cc.acked_bytes(), sent_packets[6].len());
     cwnd_is_halved(&cc);
     assert_eq!(cc.bytes_in_flight(), 4 * MAX_DATAGRAM_SIZE);
 }

neqo-transport/src/connection/mod.rs

@@ -46,7 +46,7 @@ use crate::{
     path::{Path, PathRef, Paths},
     qlog,
     quic_datagrams::{DatagramTracking, QuicDatagrams},
-    recovery::{LossRecovery, RecoveryToken, SendProfile},
+    recovery::{LossRecovery, RecoveryToken, SendProfile, SentPacket},
     recv_stream::RecvStreamStats,
     rtt::{RttEstimate, GRANULARITY},
     send_stream::SendStream,
@@ -57,7 +57,7 @@ use crate::{
         self, TransportParameter, TransportParameterId, TransportParameters,
         TransportParametersHandler,
     },
-    tracking::{AckTracker, PacketNumberSpace, RecvdPackets, SentPacket},
+    tracking::{AckTracker, PacketNumberSpace, RecvdPackets},
     version::{Version, WireVersion},
     AppError, CloseReason, Error, Res, StreamId,
 };
@@ -2370,7 +2370,7 @@ impl Connection {
                         packets.len(),
                         mtu
                     );
-                    initial.size += mtu - packets.len();
+                    initial.track_padding(mtu - packets.len());
                     // These zeros aren't padding frames, they are an invalid all-zero coalesced
                     // packet, which is why we don't increase `frame_tx.padding` count here.
                     packets.resize(mtu, 0);
@@ -2894,7 +2894,7 @@ impl Connection {
     /// to retransmit the frame as needed.
     fn handle_lost_packets(&mut self, lost_packets: &[SentPacket]) {
         for lost in lost_packets {
-            for token in &lost.tokens {
+            for token in lost.tokens() {
                 qdebug!([self], "Lost: {:?}", token);
                 match token {
                     RecoveryToken::Ack(_) => {}
@@ -2930,13 +2930,13 @@ impl Connection {
     fn handle_ack<R>(
         &mut self,
         space: PacketNumberSpace,
-        largest_acknowledged: u64,
+        largest_acknowledged: PacketNumber,
         ack_ranges: R,
         ack_ecn: Option<EcnCount>,
         ack_delay: u64,
         now: Instant,
     ) where
-        R: IntoIterator<Item = RangeInclusive<u64>> + Debug,
+        R: IntoIterator<Item = RangeInclusive<PacketNumber>> + Debug,
         R::IntoIter: ExactSizeIterator,
     {
         qdebug!([self], "Rx ACK space={}, ranges={:?}", space, ack_ranges);
@@ -2954,7 +2954,7 @@ impl Connection {
             now,
         );
         for acked in acked_packets {
-            for token in &acked.tokens {
+            for token in acked.tokens() {
                 match token {
                     RecoveryToken::Stream(stream_token) => self.streams.acked(stream_token),
                     RecoveryToken::Ack(at) => self.acks.acked(at),

neqo-transport/src/ecn.rs

@@ -9,7 +9,7 @@ use std::ops::{AddAssign, Deref, DerefMut, Sub};
 use enum_map::EnumMap;
 use neqo_common::{qdebug, qinfo, qwarn, IpTosEcn};
 
-use crate::{packet::PacketNumber, tracking::SentPacket};
+use crate::{packet::PacketNumber, recovery::SentPacket};
 
 /// The number of packets to use for testing a path for ECN capability.
 pub const ECN_TEST_COUNT: usize = 10;
@@ -159,7 +159,7 @@ impl EcnInfo {
         // > Validating ECN counts from reordered ACK frames can result in failure. An endpoint MUST
         // > NOT fail ECN validation as a result of processing an ACK frame that does not increase
         // > the largest acknowledged packet number.
-        let largest_acked = acked_packets.first().expect("must be there").pn;
+        let largest_acked = acked_packets.first().expect("must be there").pn();
         if largest_acked <= self.largest_acked {
             return;
         }
@@ -186,7 +186,7 @@ impl EcnInfo {
         // > ECT(0) marking.
         let newly_acked_sent_with_ect0: u64 = acked_packets
             .iter()
-            .filter(|p| p.ecn_mark == IpTosEcn::Ect0)
+            .filter(|p| p.ecn_mark() == IpTosEcn::Ect0)
             .count()
             .try_into()
             .unwrap();

neqo-transport/src/path.rs

@@ -25,11 +25,11 @@ use crate::{
     ecn::{EcnCount, EcnInfo},
     frame::{FRAME_TYPE_PATH_CHALLENGE, FRAME_TYPE_PATH_RESPONSE, FRAME_TYPE_RETIRE_CONNECTION_ID},
     packet::PacketBuilder,
-    recovery::RecoveryToken,
+    recovery::{RecoveryToken, SentPacket},
     rtt::RttEstimate,
     sender::PacketSender,
     stats::FrameStats,
-    tracking::{PacketNumberSpace, SentPacket},
+    tracking::PacketNumberSpace,
     Stats,
 };
 
@@ -954,12 +954,12 @@ impl Path {
             qinfo!(
                 [self],
                 "discarding a packet without an RTT estimate; guessing RTT={:?}",
-                now - sent.time_sent
+                now - sent.time_sent()
             );
             stats.rtt_init_guess = true;
             self.rtt.update(
                 &mut self.qlog,
-                now - sent.time_sent,
+                now - sent.time_sent(),
                 Duration::new(0, 0),
                 false,
                 now,

neqo-transport/src/qlog.rs

@@ -27,9 +27,9 @@ use crate::{
     frame::{CloseError, Frame},
     packet::{DecryptedPacket, PacketNumber, PacketType, PublicPacket},
     path::PathRef,
+    recovery::SentPacket,
     stream_id::StreamType as NeqoStreamType,
     tparams::{self, TransportParametersHandler},
-    tracking::SentPacket,
     version::{Version, VersionConfig, WireVersion},
 };
 
@@ -254,7 +254,8 @@ pub fn packet_dropped(qlog: &mut NeqoQlog, public_packet: &PublicPacket) {
 pub fn packets_lost(qlog: &mut NeqoQlog, pkts: &[SentPacket]) {
     qlog.add_event_with_stream(|stream| {
         for pkt in pkts {
-            let header = PacketHeader::with_type(pkt.pt.into(), Some(pkt.pn), None, None, None);
+            let header =
+                PacketHeader::with_type(pkt.packet_type().into(), Some(pkt.pn()), None, None, None);
 
             let ev_data = EventData::PacketLost(PacketLost {
                 header: Some(header),