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.

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(s2n-quic-dc): shrink path secret & fix fixed-map allocation #2340

Merged
merged 4 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading