Only populate local_cid_state when CIDs are in use

Author: Ralith

quinn-proto/src/connection/cid_state.rs

@@ -21,19 +21,12 @@ pub(super) struct CidState {
     prev_retire_seq: u64,
     /// Sequence number to set in retire_prior_to field in NEW_CONNECTION_ID frame
     retire_seq: u64,
-    /// cid length used to decode short packet
-    cid_len: usize,
     //// cid lifetime
     cid_lifetime: Option<Duration>,
 }
 
 impl CidState {
-    pub(crate) fn new(
-        cid_len: usize,
-        cid_lifetime: Option<Duration>,
-        now: Instant,
-        issued: u64,
-    ) -> Self {
+    pub(crate) fn new(cid_lifetime: Option<Duration>, now: Instant, issued: u64) -> Self {
         let mut active_seq = FxHashSet::default();
         // Add sequence number of CIDs used in handshaking into tracking set
         for seq in 0..issued {
@@ -45,7 +38,6 @@ impl CidState {
             active_seq,
             prev_retire_seq: 0,
             retire_seq: 0,
-            cid_len,
             cid_lifetime,
         };
         // Track lifetime of CIDs used in handshaking
@@ -158,11 +150,6 @@ impl CidState {
         sequence: u64,
         limit: u64,
     ) -> Result<bool, TransportError> {
-        if self.cid_len == 0 {
-            return Err(TransportError::PROTOCOL_VIOLATION(
-                "RETIRE_CONNECTION_ID when CIDs aren't in use",
-            ));
-        }
         if sequence > self.issued {
             debug!(
                 sequence,
@@ -181,11 +168,6 @@ impl CidState {
         Ok(limit > self.active_seq.len() as u64)
     }
 
-    /// Length of local Connection IDs
-    pub(crate) fn cid_len(&self) -> usize {
-        self.cid_len
-    }
-
     /// The value for `retire_prior_to` field in `NEW_CONNECTION_ID` frame
     pub(crate) fn retire_prior_to(&self) -> u64 {
         self.retire_seq

quinn-proto/src/connection/mod.rs

@@ -231,8 +231,8 @@ pub struct Connection {
     streams: StreamsState,
     /// Surplus remote CIDs for future use on new paths
     rem_cids: CidQueue,
-    // Attributes of CIDs generated by local peer
-    local_cid_state: CidState,
+    /// Attributes of CIDs generated by local peer, if in use
+    local_cid_state: Option<CidState>,
     /// State of the unreliable datagram extension
     datagrams: DatagramState,
     /// Connection level statistics
@@ -281,12 +281,14 @@ impl Connection {
             crypto,
             handshake_cid: loc_cid,
             rem_handshake_cid: rem_cid,
-            local_cid_state: CidState::new(
-                cid_gen.cid_len(),
-                cid_gen.cid_lifetime(),
-                now,
-                if pref_addr_cid.is_some() { 2 } else { 1 },
-            ),
+            local_cid_state: match cid_gen.cid_len() {
+                0 => None,
+                _ => Some(CidState::new(
+                    cid_gen.cid_lifetime(),
+                    now,
+                    if pref_addr_cid.is_some() { 2 } else { 1 },
+                )),
+            },
             path: PathData::new(remote, allow_mtud, None, now, path_validated, &config),
             allow_mtud,
             local_ip,
@@ -1088,7 +1090,8 @@ impl Connection {
                 }
             }
             NewIdentifiers(ids, now) => {
-                self.local_cid_state.new_cids(&ids, now);
+                let cid_state = self.local_cid_state.as_mut().unwrap();
+                cid_state.new_cids(&ids, now);
                 ids.into_iter().rev().for_each(|frame| {
                     self.spaces[SpaceId::Data].pending.new_cids.push(frame);
                 });
@@ -1098,7 +1101,9 @@ impl Connection {
                     .get(Timer::PushNewCid)
                     .map_or(true, |x| x <= now)
                 {
-                    self.reset_cid_retirement();
+                    if let Some(t) = cid_state.next_timeout() {
+                        self.timers.set(Timer::PushNewCid, t);
+                    }
                 }
             }
         }
@@ -1149,12 +1154,13 @@ impl Connection {
                 }
                 Timer::Pacing => trace!("pacing timer expired"),
                 Timer::PushNewCid => {
+                    let cid_state = self.local_cid_state.as_mut().unwrap();
                     // Update `retire_prior_to` field in NEW_CONNECTION_ID frame
-                    let num_new_cid = self.local_cid_state.on_cid_timeout().into();
+                    let num_new_cid = cid_state.on_cid_timeout().into();
                     if !self.state.is_closed() {
                         trace!(
                             "push a new cid to peer RETIRE_PRIOR_TO field {}",
-                            self.local_cid_state.retire_prior_to()
+                            cid_state.retire_prior_to()
                         );
                         self.endpoint_events
                             .push_back(EndpointEventInner::NeedIdentifiers(now, num_new_cid));
@@ -1860,12 +1866,6 @@ impl Connection {
         self.timers.set(Timer::KeepAlive, now + interval);
     }
 
-    fn reset_cid_retirement(&mut self) {
-        if let Some(t) = self.local_cid_state.next_timeout() {
-            self.timers.set(Timer::PushNewCid, t);
-        }
-    }
-
     /// Handle the already-decrypted first packet from the client
     ///
     /// Decrypting the first packet in the `Endpoint` allows stateless packet handling to be more
@@ -2756,8 +2756,12 @@ impl Connection {
                     self.streams.received_stop_sending(id, error_code);
                 }
                 Frame::RetireConnectionId { sequence } => {
-                    let allow_more_cids = self
-                        .local_cid_state
+                    let cid_state = self.local_cid_state.as_mut().ok_or_else(|| {
+                        TransportError::PROTOCOL_VIOLATION(
+                            "RETIRE_CONNECTION_ID when CIDs aren't in use",
+                        )
+                    })?;
+                    let allow_more_cids = cid_state
                         .on_cid_retirement(sequence, self.peer_params.issue_cids_limit())?;
                     self.endpoint_events
                         .push_back(EndpointEventInner::RetireConnectionId(
@@ -2999,7 +3003,7 @@ impl Connection {
 
     /// Issue an initial set of connection IDs to the peer upon connection
     fn issue_first_cids(&mut self, now: Instant) {
-        if self.local_cid_state.cid_len() == 0 {
+        if self.local_cid_state.is_none() {
             return;
         }
 
@@ -3169,25 +3173,27 @@ impl Connection {
         }
 
         // NEW_CONNECTION_ID
-        while buf.len() + 44 < max_size {
-            let issued = match space.pending.new_cids.pop() {
-                Some(x) => x,
-                None => break,
-            };
-            trace!(
-                sequence = issued.sequence,
-                id = %issued.id,
-                "NEW_CONNECTION_ID"
-            );
-            frame::NewConnectionId {
-                sequence: issued.sequence,
-                retire_prior_to: self.local_cid_state.retire_prior_to(),
-                id: issued.id,
-                reset_token: issued.reset_token,
+        if let Some(cid_state) = self.local_cid_state.as_ref() {
+            while buf.len() + 44 < max_size {
+                let issued = match space.pending.new_cids.pop() {
+                    Some(x) => x,
+                    None => break,
+                };
+                trace!(
+                    sequence = issued.sequence,
+                    id = %issued.id,
+                    "NEW_CONNECTION_ID"
+                );
+                frame::NewConnectionId {
+                    sequence: issued.sequence,
+                    retire_prior_to: cid_state.retire_prior_to(),
+                    id: issued.id,
+                    reset_token: issued.reset_token,
+                }
+                .encode(buf);
+                sent.retransmits.get_or_create().new_cids.push(issued);
+                self.stats.frame_tx.new_connection_id += 1;
             }
-            .encode(buf);
-            sent.retransmits.get_or_create().new_cids.push(issued);
-            self.stats.frame_tx.new_connection_id += 1;
         }
 
         // RETIRE_CONNECTION_ID
@@ -3481,14 +3487,16 @@ impl Connection {
 
     #[cfg(test)]
     pub(crate) fn active_local_cid_seq(&self) -> (u64, u64) {
-        self.local_cid_state.active_seq()
+        self.local_cid_state
+            .as_ref()
+            .map_or((u64::MAX, u64::MIN), |state| state.active_seq())
     }
 
     /// Instruct the peer to replace previously issued CIDs by sending a NEW_CONNECTION_ID frame
     /// with updated `retire_prior_to` field set to `v`
     #[cfg(test)]
     pub(crate) fn rotate_local_cid(&mut self, v: u64, now: Instant) {
-        let n = self.local_cid_state.assign_retire_seq(v);
+        let n = self.local_cid_state.as_mut().unwrap().assign_retire_seq(v);
         self.endpoint_events
             .push_back(EndpointEventInner::NeedIdentifiers(now, n));
     }