Skip to content

Commit

Permalink
feat: Replace HashMap with EnumMap (#2434)
Browse files Browse the repository at this point in the history 
* feat: Replace `HashMap` with `EnumMap`

In several places.

* Fix

* Undo

* Fix

* Update neqo-transport/src/tparams.rs

Co-authored-by: Martin Thomson <[email protected]>
Signed-off-by: Lars Eggert <[email protected]>

* Suggestions from @martinthomson

---------

Signed-off-by: Lars Eggert <[email protected]>
Co-authored-by: Martin Thomson <[email protected]>
  • Loading branch information
@larseggert @martinthomson
larseggert and martinthomson authored Feb 12, 2025
1 parent db807a9 commit 1549b25
Show file tree
Hide file tree
Showing 17 changed files with 448 additions and 379 deletions.
79 changes: 37 additions & 42 deletions neqo-transport/src/connection/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,15 @@ use crate::{
stats::{Stats, StatsCell},
stream_id::StreamType,
streams::{SendOrder, Streams},
tparams::{self, TransportParameters, TransportParametersHandler},
tparams::{
self,
TransportParameterId::{
self, AckDelayExponent, ActiveConnectionIdLimit, DisableMigration, GreaseQuicBit,
InitialSourceConnectionId, MaxAckDelay, MaxDatagramFrameSize, MinAckDelay,
OriginalDestinationConnectionId, RetrySourceConnectionId, StatelessResetToken,
},
TransportParameters, TransportParametersHandler,
},
tracking::{AckTracker, PacketNumberSpace, RecvdPackets},
version::{Version, WireVersion},
AppError, CloseReason, Error, Res, StreamId,
Expand Down Expand Up @@ -373,10 +381,8 @@ impl Connection {
let mut cid_manager =
ConnectionIdManager::new(cid_generator, local_initial_source_cid.clone());
let mut tps = conn_params.create_transport_parameter(role, &mut cid_manager)?;
tps.local.set_bytes(
tparams::INITIAL_SOURCE_CONNECTION_ID,
local_initial_source_cid.to_vec(),
);
tps.local
.set_bytes(InitialSourceConnectionId, local_initial_source_cid.to_vec());

let tphandler = Rc::new(RefCell::new(tps));
let crypto = Crypto::new(
Expand Down Expand Up @@ -498,7 +504,7 @@ impl Connection {
#[cfg(test)]
pub fn set_local_tparam(
&self,
tp: tparams::TransportParameterId,
tp: TransportParameterId,
value: tparams::TransportParameter,
) -> Res<()> {
if *self.state() == State::Init {
Expand All @@ -525,8 +531,8 @@ impl Connection {
qtrace!("[{self}] Retry CIDs: odcid={odcid} remote={remote_cid} retry={retry_cid}");
// We advertise "our" choices in transport parameters.
let local_tps = &mut self.tps.borrow_mut().local;
local_tps.set_bytes(tparams::ORIGINAL_DESTINATION_CONNECTION_ID, odcid.to_vec());
local_tps.set_bytes(tparams::RETRY_SOURCE_CONNECTION_ID, retry_cid.to_vec());
local_tps.set_bytes(OriginalDestinationConnectionId, odcid.to_vec());
local_tps.set_bytes(RetrySourceConnectionId, retry_cid.to_vec());

// ...and save their choices for later validation.
self.remote_initial_source_cid = Some(remote_cid);
Expand All @@ -536,7 +542,7 @@ impl Connection {
self.tps
.borrow()
.local
.get_bytes(tparams::RETRY_SOURCE_CONNECTION_ID)
.get_bytes(RetrySourceConnectionId)
.is_some()
}

Expand Down Expand Up @@ -1407,10 +1413,10 @@ impl Connection {
// This has to happen prior to processing the packet so that
// the TLS handshake has all it needs.
if !self.retry_sent() {
self.tps.borrow_mut().local.set_bytes(
tparams::ORIGINAL_DESTINATION_CONNECTION_ID,
packet.dcid().to_vec(),
);
self.tps
.borrow_mut()
.local
.set_bytes(OriginalDestinationConnectionId, packet.dcid().to_vec());
}
}
(PacketType::VersionNegotiation, State::WaitInitial, Role::Client) => {
Expand Down Expand Up @@ -1890,12 +1896,7 @@ impl Connection {
if !matches!(self.state(), State::Confirmed) {
return Err(Error::InvalidMigration);
}
if self
.tps
.borrow()
.remote()
.get_empty(tparams::DISABLE_MIGRATION)
{
if self.tps.borrow().remote().get_empty(DisableMigration) {
return Err(Error::InvalidMigration);
}

Expand Down Expand Up @@ -2113,9 +2114,9 @@ impl Connection {
fn can_grease_quic_bit(&self) -> bool {
let tph = self.tps.borrow();
if let Some(r) = &tph.remote {
r.get_empty(tparams::GREASE_QUIC_BIT)
r.get_empty(GreaseQuicBit)
} else if let Some(r) = &tph.remote_0rtt {
r.get_empty(tparams::GREASE_QUIC_BIT)
r.get_empty(GreaseQuicBit)
} else {
false
}
Expand Down Expand Up @@ -2621,18 +2622,14 @@ impl Connection {
.tps
.borrow()
.remote()
.get_integer(tparams::IDLE_TIMEOUT);
.get_integer(TransportParameterId::IdleTimeout);
if peer_timeout > 0 {
self.idle_timeout
.set_peer_timeout(Duration::from_millis(peer_timeout));
}

self.quic_datagrams.set_remote_datagram_size(
self.tps
.borrow()
.remote()
.get_integer(tparams::MAX_DATAGRAM_FRAME_SIZE),
);
self.quic_datagrams
.set_remote_datagram_size(self.tps.borrow().remote().get_integer(MaxDatagramFrameSize));
}

#[must_use]
Expand Down Expand Up @@ -2661,18 +2658,16 @@ impl Connection {
return Err(Error::TransportParameterError);
}

let reset_token = remote
.get_bytes(tparams::STATELESS_RESET_TOKEN)
.map_or_else(
|| Ok(ConnectionIdEntry::random_srt()),
|token| <[u8; 16]>::try_from(token).map_err(|_| Error::TransportParameterError),
)?;
let reset_token = remote.get_bytes(StatelessResetToken).map_or_else(
|| Ok(ConnectionIdEntry::random_srt()),
|token| <[u8; 16]>::try_from(token).map_err(|_| Error::TransportParameterError),
)?;
let path = self.paths.primary().ok_or(Error::NoAvailablePath)?;
path.borrow_mut().set_reset_token(reset_token);

let max_ad = Duration::from_millis(remote.get_integer(tparams::MAX_ACK_DELAY));
let min_ad = if remote.has_value(tparams::MIN_ACK_DELAY) {
let min_ad = Duration::from_micros(remote.get_integer(tparams::MIN_ACK_DELAY));
let max_ad = Duration::from_millis(remote.get_integer(MaxAckDelay));
let min_ad = if remote.has_value(MinAckDelay) {
let min_ad = Duration::from_micros(remote.get_integer(MinAckDelay));
if min_ad > max_ad {
return Err(Error::TransportParameterError);
}
Expand All @@ -2683,7 +2678,7 @@ impl Connection {
path.borrow_mut()
.set_ack_delay(max_ad, min_ad, self.conn_params.get_ack_ratio());

let max_active_cids = remote.get_integer(tparams::ACTIVE_CONNECTION_ID_LIMIT);
let max_active_cids = remote.get_integer(ActiveConnectionIdLimit);
self.cid_manager.set_limit(max_active_cids);
}
self.set_initial_limits();
Expand All @@ -2695,7 +2690,7 @@ impl Connection {
let tph = self.tps.borrow();
let remote_tps = tph.remote.as_ref().ok_or(Error::TransportParameterError)?;

let tp = remote_tps.get_bytes(tparams::INITIAL_SOURCE_CONNECTION_ID);
let tp = remote_tps.get_bytes(InitialSourceConnectionId);
if self
.remote_initial_source_cid
.as_ref()
Expand All @@ -2711,7 +2706,7 @@ impl Connection {
}

if self.role == Role::Client {
let tp = remote_tps.get_bytes(tparams::ORIGINAL_DESTINATION_CONNECTION_ID);
let tp = remote_tps.get_bytes(OriginalDestinationConnectionId);
if self
.original_destination_cid
.as_ref()
Expand All @@ -2726,7 +2721,7 @@ impl Connection {
return Err(Error::ProtocolViolation);
}

let tp = remote_tps.get_bytes(tparams::RETRY_SOURCE_CONNECTION_ID);
let tp = remote_tps.get_bytes(RetrySourceConnectionId);
let expected = if let AddressValidationInfo::Retry {
retry_source_cid, ..
} = &self.address_validation
Expand Down Expand Up @@ -3120,7 +3115,7 @@ impl Connection {
self.tps.borrow().remote.as_ref().map_or_else(
|| Ok(Duration::default()),
|r| {
let exponent = u32::try_from(r.get_integer(tparams::ACK_DELAY_EXPONENT))?;
let exponent = u32::try_from(r.get_integer(AckDelayExponent))?;
// ACK_DELAY_EXPONENT > 20 is invalid per RFC9000. We already checked that in
// TransportParameter::decode.
let corrected = if v.leading_zeros() >= exponent {
Expand Down
52 changes: 27 additions & 25 deletions neqo-transport/src/connection/params.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,16 @@ use crate::{
recv_stream::RECV_BUFFER_SIZE,
rtt::GRANULARITY,
stream_id::StreamType,
tparams::{self, PreferredAddress, TransportParameter, TransportParametersHandler},
tparams::{
PreferredAddress, TransportParameter,
TransportParameterId::{
self, ActiveConnectionIdLimit, DisableMigration, GreaseQuicBit, InitialMaxData,
InitialMaxStreamDataBidiLocal, InitialMaxStreamDataBidiRemote, InitialMaxStreamDataUni,
InitialMaxStreamsBidi, InitialMaxStreamsUni, MaxAckDelay, MaxDatagramFrameSize,
MinAckDelay,
},
TransportParametersHandler,
},
tracking::DEFAULT_ACK_DELAY,
version::{Version, VersionConfig},
CongestionControlAlgorithm, Res,
Expand Down Expand Up @@ -408,52 +417,45 @@ impl ConnectionParameters {
let mut tps = TransportParametersHandler::new(role, self.versions.clone());
// default parameters
tps.local.set_integer(
tparams::ACTIVE_CONNECTION_ID_LIMIT,
ActiveConnectionIdLimit,
u64::try_from(LOCAL_ACTIVE_CID_LIMIT)?,
);
if self.disable_migration {
tps.local.set_empty(tparams::DISABLE_MIGRATION);
tps.local.set_empty(DisableMigration);
}
if self.grease {
tps.local.set_empty(tparams::GREASE_QUIC_BIT);
tps.local.set_empty(GreaseQuicBit);
}
tps.local.set_integer(
tparams::MAX_ACK_DELAY,
u64::try_from(DEFAULT_ACK_DELAY.as_millis())?,
);
tps.local.set_integer(
tparams::MIN_ACK_DELAY,
u64::try_from(GRANULARITY.as_micros())?,
);
tps.local
.set_integer(MaxAckDelay, u64::try_from(DEFAULT_ACK_DELAY.as_millis())?);
tps.local
.set_integer(MinAckDelay, u64::try_from(GRANULARITY.as_micros())?);

// set configurable parameters
tps.local
.set_integer(tparams::INITIAL_MAX_DATA, self.max_data);
tps.local.set_integer(InitialMaxData, self.max_data);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_LOCAL,
InitialMaxStreamDataBidiLocal,
self.max_stream_data_bidi_local,
);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_REMOTE,
InitialMaxStreamDataBidiRemote,
self.max_stream_data_bidi_remote,
);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_UNI,
self.max_stream_data_uni,
);
tps.local
.set_integer(tparams::INITIAL_MAX_STREAMS_BIDI, self.max_streams_bidi);
.set_integer(InitialMaxStreamDataUni, self.max_stream_data_uni);
tps.local
.set_integer(InitialMaxStreamsBidi, self.max_streams_bidi);
tps.local
.set_integer(tparams::INITIAL_MAX_STREAMS_UNI, self.max_streams_uni);
.set_integer(InitialMaxStreamsUni, self.max_streams_uni);
tps.local.set_integer(
tparams::IDLE_TIMEOUT,
TransportParameterId::IdleTimeout,
u64::try_from(self.idle_timeout.as_millis()).unwrap_or(0),
);
if let PreferredAddressConfig::Address(preferred) = &self.preferred_address {
if role == Role::Server {
let (cid, srt) = cid_manager.preferred_address_cid()?;
tps.local.set(
tparams::PREFERRED_ADDRESS,
TransportParameterId::PreferredAddress,
TransportParameter::PreferredAddress {
v4: preferred.ipv4(),
v6: preferred.ipv6(),
Expand All @@ -464,7 +466,7 @@ impl ConnectionParameters {
}
}
tps.local
.set_integer(tparams::MAX_DATAGRAM_FRAME_SIZE, self.datagram_size);
.set_integer(MaxDatagramFrameSize, self.datagram_size);
Ok(tps)
}
}
7 changes: 2 additions & 5 deletions neqo-transport/src/connection/tests/close.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use super::{
connect, connect_force_idle, default_client, default_server, send_something,
};
use crate::{
tparams::{self, TransportParameter},
tparams::{TransportParameter, TransportParameterId::StatelessResetToken},
AppError, CloseReason, Error, ERROR_APPLICATION_CLOSE,
};

Expand Down Expand Up @@ -213,10 +213,7 @@ fn stateless_reset_client() {
let mut client = default_client();
let mut server = default_server();
server
.set_local_tparam(
tparams::STATELESS_RESET_TOKEN,
TransportParameter::Bytes(vec![77; 16]),
)
.set_local_tparam(StatelessResetToken, TransportParameter::Bytes(vec![77; 16]))
.unwrap();
connect_force_idle(&mut client, &mut server);

Expand Down
10 changes: 6 additions & 4 deletions neqo-transport/src/connection/tests/handshake.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
events::ConnectionEvent,
server::ValidateAddress,
stats::FrameStats,
tparams::{self, TransportParameter, MIN_ACK_DELAY},
tparams::{TransportParameter, TransportParameterId::*},
tracking::DEFAULT_ACK_DELAY,
CloseReason, ConnectionParameters, Error, Pmtud, StreamType, Version,
};
Expand Down Expand Up @@ -840,7 +840,9 @@ fn anti_amplification() {
// With a gigantic transport parameter, the server is unable to complete
// the handshake within the amplification limit.
let very_big = TransportParameter::Bytes(vec![0; Pmtud::default_plpmtu(DEFAULT_ADDR.ip()) * 3]);
server.set_local_tparam(0xce16, very_big).unwrap();
server
.set_local_tparam(TestTransportParameter, very_big)
.unwrap();

let c_init = client.process_output(now).dgram();
now += DEFAULT_RTT / 2;
Expand Down Expand Up @@ -1089,7 +1091,7 @@ fn bad_min_ack_delay() {
let mut server = default_server();
let max_ad = u64::try_from(DEFAULT_ACK_DELAY.as_micros()).unwrap();
server
.set_local_tparam(MIN_ACK_DELAY, TransportParameter::Integer(max_ad + 1))
.set_local_tparam(MinAckDelay, TransportParameter::Integer(max_ad + 1))
.unwrap();
let mut client = default_client();

Expand Down Expand Up @@ -1371,7 +1373,7 @@ fn grease_quic_bit_transport_parameter() {
.remote
.as_ref()
.unwrap()
.get_empty(tparams::GREASE_QUIC_BIT)
.get_empty(GreaseQuicBit)
}

for client_grease in [true, false] {
Expand Down
6 changes: 3 additions & 3 deletions neqo-transport/src/connection/tests/idle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
packet::PacketBuilder,
stats::FrameStats,
stream_id::{StreamId, StreamType},
tparams::{self, TransportParameter},
tparams::{TransportParameter, TransportParameterId},
tracking::PacketNumberSpace,
};

Expand Down Expand Up @@ -97,7 +97,7 @@ fn asymmetric_idle_timeout() {
.tps
.borrow_mut()
.local
.set_integer(tparams::IDLE_TIMEOUT, LOWER_TIMEOUT_MS);
.set_integer(TransportParameterId::IdleTimeout, LOWER_TIMEOUT_MS);
server.idle_timeout = IdleTimeout::new(LOWER_TIMEOUT);

// Now connect and force idleness manually.
Expand Down Expand Up @@ -135,7 +135,7 @@ fn tiny_idle_timeout() {
// Overwrite the default at the server.
server
.set_local_tparam(
tparams::IDLE_TIMEOUT,
TransportParameterId::IdleTimeout,
TransportParameter::Integer(LOWER_TIMEOUT_MS),
)
.unwrap();
Expand Down
Loading

0 comments on commit 1549b25

Please sign in to comment.