Skip to content

Commit

Permalink
feat(s2n-quic-dc): shrink path secret & fix fixed-map allocation (#2340)
Browse files Browse the repository at this point in the history
* Replace Option<Duration> with Option<NonZeroU32>

This cuts us down to 254 bytes with no practical tradeoff.

* feat(dc): drop unused field

* Fix fixed map to allocate full size

Previously we always allocated just one slot.

* Fix typo
  • Loading branch information
Mark-Simulacrum authored Oct 1, 2024
1 parent cb5087c commit c9a86c3
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 14 deletions.
3 changes: 2 additions & 1 deletion dc/s2n-quic-dc/src/fixed_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ where
S: BuildHasher,
{
pub fn with_capacity(entries: usize, hasher: S) -> Self {
let slots = std::cmp::max(1, (entries + SLOT_CAPACITY) / SLOT_CAPACITY).next_power_of_two();
let map = Map {
slots: (0..std::cmp::min(1, (entries + SLOT_CAPACITY) / SLOT_CAPACITY))
slots: (0..slots)
.map(|_| Slot::new())
.collect::<Vec<_>>()
.into_boxed_slice(),
Expand Down
9 changes: 9 additions & 0 deletions dc/s2n-quic-dc/src/fixed_map/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ fn slot_clear() {

assert_eq!(slot.len(), 0);
}

#[test]
fn capacity_size() {
let map: Map<u32, ()> = Map::with_capacity(500_000, Default::default());
for idx in 0..500_000 {
map.insert(idx, ());
}
assert!(map.len() >= 400_000, "{}", map.len());
}
6 changes: 4 additions & 2 deletions dc/s2n-quic-dc/src/path/secret/map/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,9 @@ fn check_invariants_no_overflow() {
fn no_memory_growth() {
let signer = stateless_reset::Signer::new(b"secret");
let map = Map::new(signer);
for idx in 0..500_000_000 {
map.state.cleaner.stop();
for idx in 0..500_000 {
// FIXME: this ends up 2**16 peers in the `peers` map
map.insert(fake_entry(idx as u16));
}
}
Expand All @@ -327,6 +329,6 @@ fn no_memory_growth() {
fn entry_size() {
// This gates to running only on specific GHA to reduce false positives.
if std::env::var("S2N_QUIC_RUN_VERSION_SPECIFIC_TESTS").is_ok() {
assert_eq!(fake_entry(0).size(), 270);
assert_eq!(fake_entry(0).size(), 238);
}
}
2 changes: 1 addition & 1 deletion dc/s2n-quic-dc/src/stream/recv/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl State {
recovery_ack: Default::default(),
state: Default::default(),
idle_timer: Default::default(),
idle_timeout: params.max_idle_timeout.unwrap_or(DEFAULT_IDLE_TIMEOUT),
idle_timeout: params.max_idle_timeout().unwrap_or(DEFAULT_IDLE_TIMEOUT),
tick_timer: Default::default(),
_should_transmit: false,
max_data: initial_max_data,
Expand Down
2 changes: 1 addition & 1 deletion dc/s2n-quic-dc/src/stream/send/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl State {
pto_backoff: INITIAL_PTO_BACKOFF,
inflight_timer: Default::default(),
idle_timer: Default::default(),
idle_timeout: params.max_idle_timeout.unwrap_or(DEFAULT_IDLE_TIMEOUT),
idle_timeout: params.max_idle_timeout().unwrap_or(DEFAULT_IDLE_TIMEOUT),
error: None,
unacked_ranges,
max_sent_offset,
Expand Down
17 changes: 12 additions & 5 deletions quic/s2n-quic-core/src/dc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
transport::parameters::{DcSupportedVersions, InitialFlowControlLimits},
varint::VarInt,
};
use core::time::Duration;
use core::{num::NonZeroU32, time::Duration};

mod disabled;
mod traits;
Expand Down Expand Up @@ -98,8 +98,8 @@ pub struct ApplicationParams {
pub remote_max_data: VarInt,
pub local_send_max_data: VarInt,
pub local_recv_max_data: VarInt,
pub max_idle_timeout: Option<Duration>,
pub max_ack_delay: Duration,
// Actually a Duration, stored as milliseconds to shrink this struct
pub max_idle_timeout: Option<NonZeroU32>,
}

impl ApplicationParams {
Expand All @@ -113,8 +113,15 @@ impl ApplicationParams {
remote_max_data: peer_flow_control_limits.max_data,
local_send_max_data: limits.initial_stream_limits().max_data_bidi_local,
local_recv_max_data: limits.initial_stream_limits().max_data_bidi_remote,
max_idle_timeout: limits.max_idle_timeout(),
max_ack_delay: limits.max_ack_delay.into(),
max_idle_timeout: limits
.max_idle_timeout()
// If > u32::MAX, treat as not having an idle timeout, that's ~50 days.
.and_then(|v| v.as_millis().try_into().ok())
.and_then(NonZeroU32::new),
}
}

pub fn max_idle_timeout(&self) -> Option<Duration> {
Some(Duration::from_millis(self.max_idle_timeout?.get() as u64))
}
}
5 changes: 2 additions & 3 deletions quic/s2n-quic-core/src/dc/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
stateless_reset, transport,
varint::VarInt,
};
use core::time::Duration;
use core::{num::NonZeroU32, time::Duration};
use std::sync::{
atomic::{AtomicU8, Ordering},
Arc,
Expand Down Expand Up @@ -83,8 +83,7 @@ pub const TEST_APPLICATION_PARAMS: ApplicationParams = ApplicationParams {
remote_max_data: VarInt::from_u32(1u32 << 25),
local_send_max_data: VarInt::from_u32(1u32 << 25),
local_recv_max_data: VarInt::from_u32(1u32 << 25),
max_idle_timeout: Some(Duration::from_secs(30)),
max_ack_delay: Duration::from_millis(25),
max_idle_timeout: NonZeroU32::new(Duration::from_secs(30).as_millis() as _),
};

pub const TEST_REHANDSHAKE_PERIOD: Duration = Duration::from_secs(3600 * 12);
2 changes: 1 addition & 1 deletion tools/xdp/s2n-quic-xdp/src/ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::{io, os::unix::io::AsRawFd};
struct Ring<T: Copy + fmt::Debug> {
cursor: Cursor<T>,
flags: NonNull<RingFlags>,
// make the area clonable in test mode
// make the area cloneable in test mode
#[cfg(test)]
area: std::sync::Arc<Mmap>,
#[cfg(not(test))]
Expand Down

0 comments on commit c9a86c3

Please sign in to comment.