Skip to content

Commit

Permalink
Only populate local_cid_state when CIDs are in use
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ralith
Ralith committed Jun 2, 2024
1 parent b34657f commit 61dbea6
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 60 deletions.
20 changes: 1 addition & 19 deletions quinn-proto/src/connection/cid_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
90 changes: 49 additions & 41 deletions quinn-proto/src/connection/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
Expand Down

0 comments on commit 61dbea6

Please sign in to comment.