Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Replace HashMap with EnumMap #2434

Merged
merged 11 commits into from
Feb 12, 2025
Merged

feat: Replace HashMap with EnumMap #2434

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
254d56e
feat: Replace `HashMap` with `EnumMap`
larseggert Feb 10, 2025
7d8dc68
Merge branch 'main' into feat-enum-map
larseggert Feb 10, 2025
5cc7461
Fix
larseggert Feb 10, 2025
d5cc1c7
Merge branch 'feat-enum-map' of github.com:larseggert/neqo into feat-…
larseggert Feb 10, 2025
07bc739
Undo
larseggert Feb 10, 2025
b3f3916
Fix
larseggert Feb 10, 2025
1374a47
Update neqo-transport/src/tparams.rs
larseggert Feb 11, 2025
3602bec
Suggestions from @martinthomson
larseggert Feb 11, 2025
c61be38
Merge branch 'feat-enum-map' of github.com:larseggert/neqo into feat-…
larseggert Feb 11, 2025
98885f9
Merge branch 'main' into feat-enum-map
larseggert Feb 11, 2025
1b2059a
Merge branch 'main' into feat-enum-map
larseggert Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 29 additions & 20 deletions neqo-transport/src/connection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use crate::{
stats::{Stats, StatsCell},
stream_id::StreamType,
streams::{SendOrder, Streams},
tparams::{self, TransportParameters, TransportParametersHandler},
tparams::{self, TransportParameterId, TransportParameters, TransportParametersHandler},
tracking::{AckTracker, PacketNumberSpace, RecvdPackets},
version::{Version, WireVersion},
AppError, CloseReason, Error, Res, StreamId,
Expand Down Expand Up @@ -374,7 +374,7 @@ impl Connection {
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,
TransportParameterId::InitialSourceConnectionId,
local_initial_source_cid.to_vec(),
);

Expand Down Expand Up @@ -525,8 +525,14 @@ 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(
TransportParameterId::OriginalDestinationConnectionId,
larseggert marked this conversation as resolved.
Show resolved Hide resolved
odcid.to_vec(),
);
local_tps.set_bytes(
TransportParameterId::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(TransportParameterId::RetrySourceConnectionId)
.is_some()
}

Expand Down Expand Up @@ -1408,7 +1414,7 @@ impl Connection {
// the TLS handshake has all it needs.
if !self.retry_sent() {
self.tps.borrow_mut().local.set_bytes(
tparams::ORIGINAL_DESTINATION_CONNECTION_ID,
TransportParameterId::OriginalDestinationConnectionId,
packet.dcid().to_vec(),
);
}
Expand Down Expand Up @@ -1894,7 +1900,7 @@ impl Connection {
.tps
.borrow()
.remote()
.get_empty(tparams::DISABLE_MIGRATION)
.get_empty(TransportParameterId::DisableMigration)
{
return Err(Error::InvalidMigration);
}
Expand Down Expand Up @@ -2113,9 +2119,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(TransportParameterId::GreaseQuicBit)
} else if let Some(r) = &tph.remote_0rtt {
r.get_empty(tparams::GREASE_QUIC_BIT)
r.get_empty(TransportParameterId::GreaseQuicBit)
} else {
false
}
Expand Down Expand Up @@ -2621,7 +2627,7 @@ impl Connection {
.tps
.borrow()
.remote()
.get_integer(tparams::IDLE_TIMEOUT);
.get_integer(TransportParameterId::IdleTimeout);
larseggert marked this conversation as resolved.
Show resolved Hide resolved
if peer_timeout > 0 {
self.idle_timeout
.set_peer_timeout(Duration::from_millis(peer_timeout));
Expand All @@ -2631,7 +2637,7 @@ impl Connection {
self.tps
.borrow()
.remote()
.get_integer(tparams::MAX_DATAGRAM_FRAME_SIZE),
.get_integer(TransportParameterId::MaxDatagramFrameSize),
);
}

Expand Down Expand Up @@ -2662,17 +2668,19 @@ impl Connection {
}

let reset_token = remote
.get_bytes(tparams::STATELESS_RESET_TOKEN)
.get_bytes(TransportParameterId::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(TransportParameterId::MaxAckDelay));
let min_ad = if remote.has_value(TransportParameterId::MinAckDelay) {
let min_ad =
Duration::from_micros(remote.get_integer(TransportParameterId::MinAckDelay));
if min_ad > max_ad {
return Err(Error::TransportParameterError);
}
Expand All @@ -2683,7 +2691,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(TransportParameterId::ActiveConnectionIdLimit);
self.cid_manager.set_limit(max_active_cids);
}
self.set_initial_limits();
Expand All @@ -2695,7 +2703,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(TransportParameterId::InitialSourceConnectionId);
if self
.remote_initial_source_cid
.as_ref()
Expand All @@ -2711,7 +2719,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(TransportParameterId::OriginalDestinationConnectionId);
if self
.original_destination_cid
.as_ref()
Expand All @@ -2726,7 +2734,7 @@ impl Connection {
return Err(Error::ProtocolViolation);
}

let tp = remote_tps.get_bytes(tparams::RETRY_SOURCE_CONNECTION_ID);
let tp = remote_tps.get_bytes(TransportParameterId::RetrySourceConnectionId);
let expected = if let AddressValidationInfo::Retry {
retry_source_cid, ..
} = &self.address_validation
Expand Down Expand Up @@ -3120,7 +3128,8 @@ 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(TransportParameterId::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
44 changes: 26 additions & 18 deletions neqo-transport/src/connection/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use crate::{
recv_stream::RECV_BUFFER_SIZE,
rtt::GRANULARITY,
stream_id::StreamType,
tparams::{self, PreferredAddress, TransportParameter, TransportParametersHandler},
tparams::{
PreferredAddress, TransportParameter, TransportParameterId, TransportParametersHandler,
},
tracking::DEFAULT_ACK_DELAY,
version::{Version, VersionConfig},
CongestionControlAlgorithm, Res,
Expand Down Expand Up @@ -408,52 +410,56 @@ impl ConnectionParameters {
let mut tps = TransportParametersHandler::new(role, self.versions.clone());
// default parameters
tps.local.set_integer(
tparams::ACTIVE_CONNECTION_ID_LIMIT,
TransportParameterId::ActiveConnectionIdLimit,
u64::try_from(LOCAL_ACTIVE_CID_LIMIT)?,
);
if self.disable_migration {
tps.local.set_empty(tparams::DISABLE_MIGRATION);
tps.local.set_empty(TransportParameterId::DisableMigration);
}
if self.grease {
tps.local.set_empty(tparams::GREASE_QUIC_BIT);
tps.local.set_empty(TransportParameterId::GreaseQuicBit);
}
tps.local.set_integer(
tparams::MAX_ACK_DELAY,
TransportParameterId::MaxAckDelay,
u64::try_from(DEFAULT_ACK_DELAY.as_millis())?,
);
tps.local.set_integer(
tparams::MIN_ACK_DELAY,
TransportParameterId::MinAckDelay,
u64::try_from(GRANULARITY.as_micros())?,
);

// set configurable parameters
tps.local
.set_integer(tparams::INITIAL_MAX_DATA, self.max_data);
.set_integer(TransportParameterId::InitialMaxData, self.max_data);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_LOCAL,
TransportParameterId::InitialMaxStreamDataBidiLocal,
self.max_stream_data_bidi_local,
);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_REMOTE,
TransportParameterId::InitialMaxStreamDataBidiRemote,
self.max_stream_data_bidi_remote,
);
tps.local.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_UNI,
TransportParameterId::InitialMaxStreamDataUni,
self.max_stream_data_uni,
);
tps.local
.set_integer(tparams::INITIAL_MAX_STREAMS_BIDI, self.max_streams_bidi);
tps.local
.set_integer(tparams::INITIAL_MAX_STREAMS_UNI, self.max_streams_uni);
tps.local.set_integer(
tparams::IDLE_TIMEOUT,
TransportParameterId::InitialMaxStreamsBidi,
self.max_streams_bidi,
);
tps.local.set_integer(
TransportParameterId::InitialMaxStreamsUni,
self.max_streams_uni,
);
tps.local.set_integer(
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 @@ -463,8 +469,10 @@ impl ConnectionParameters {
);
}
}
tps.local
.set_integer(tparams::MAX_DATAGRAM_FRAME_SIZE, self.datagram_size);
tps.local.set_integer(
TransportParameterId::MaxDatagramFrameSize,
self.datagram_size,
);
Ok(tps)
}
}
4 changes: 2 additions & 2 deletions neqo-transport/src/connection/tests/close.rs
View file
Open in desktop
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},
AppError, CloseReason, Error, ERROR_APPLICATION_CLOSE,
};

Expand Down Expand Up @@ -214,7 +214,7 @@ fn stateless_reset_client() {
let mut server = default_server();
server
.set_local_tparam(
tparams::STATELESS_RESET_TOKEN,
TransportParameterId::StatelessResetToken,
TransportParameter::Bytes(vec![77; 16]),
)
.unwrap();
Expand Down
13 changes: 9 additions & 4 deletions neqo-transport/src/connection/tests/handshake.rs
View file
Open in desktop
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(TransportParameterId::TestTransportParameter, very_big)
.unwrap();

let c_init = client.process_output(now).dgram();
now += DEFAULT_RTT / 2;
Expand Down Expand Up @@ -1089,7 +1091,10 @@ 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(
TransportParameterId::MinAckDelay,
TransportParameter::Integer(max_ad + 1),
)
.unwrap();
let mut client = default_client();

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

for client_grease in [true, false] {
Expand Down
6 changes: 3 additions & 3 deletions neqo-transport/src/connection/tests/idle.rs
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions neqo-transport/src/connection/tests/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::{
path::MAX_PATH_PROBES,
pmtud::Pmtud,
stats::FrameStats,
tparams::{self, PreferredAddress, TransportParameter},
tparams::{PreferredAddress, TransportParameter, TransportParameterId},
CloseReason, ConnectionId, ConnectionIdDecoder as _, ConnectionIdGenerator, ConnectionIdRef,
ConnectionParameters, EmptyConnectionIdGenerator, Error, MIN_INITIAL_PACKET_SIZE,
};
Expand Down Expand Up @@ -904,7 +904,7 @@ fn preferred_address_server_empty_cid() {

server
.set_local_tparam(
tparams::PREFERRED_ADDRESS,
TransportParameterId::PreferredAddress,
TransportParameter::Bytes(SAMPLE_PREFERRED_ADDRESS.to_vec()),
)
.unwrap();
Expand All @@ -925,7 +925,7 @@ fn preferred_address_client() {

client
.set_local_tparam(
tparams::PREFERRED_ADDRESS,
TransportParameterId::PreferredAddress,
TransportParameter::Bytes(SAMPLE_PREFERRED_ADDRESS.to_vec()),
)
.unwrap();
Expand Down
Loading
Loading