Skip to content

Commit

Permalink
fix: enforce max data limit size for stream and connection
Browse files Browse the repository at this point in the history
  • Loading branch information
toidiu committed Apr 1, 2023
1 parent 6967836 commit db910d9
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 21 deletions.
8 changes: 4 additions & 4 deletions quic/s2n-quic-core/src/connection/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,21 @@ impl Limits {
}

setter!(with_max_idle_timeout, max_idle_timeout, Duration);
setter!(with_data_window, data_window, u64);
setter!(with_data_window, data_window, u32);
setter!(
with_bidirectional_local_data_window,
bidirectional_local_data_window,
u64
u32
);
setter!(
with_bidirectional_remote_data_window,
bidirectional_remote_data_window,
u64
u32
);
setter!(
with_unidirectional_data_window,
unidirectional_data_window,
u64
u32
);

/// Sets both the max local and remote limits for bidirectional streams.
Expand Down
39 changes: 33 additions & 6 deletions quic/s2n-quic-core/src/transport/parameters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,16 @@ macro_rules! varint_transport_parameter {
($name:ident, $tag:expr $(, $default:expr)?) => {
transport_parameter!($name(VarInt), $tag $(, $default)?);

impl $name {
pub const fn as_varint(self) -> VarInt {
self.0
}
}
};
}

macro_rules! varint_from_u64 {
($name:ident) => {
impl TryFrom<u64> for $name {
type Error = ValidationError;

Expand All @@ -349,10 +359,17 @@ macro_rules! varint_transport_parameter {
Self::try_from(value)
}
}
};
}

impl $name {
pub const fn as_varint(self) -> VarInt {
self.0
macro_rules! varint_from_u32 {
($name:ident) => {
impl TryFrom<u32> for $name {
type Error = ValidationError;

fn try_from(value: u32) -> Result<Self, Self::Error> {
let value = VarInt::from_u32(value);
Self::try_from(value)
}
}
};
Expand Down Expand Up @@ -607,9 +624,10 @@ impl TryFrom<u16> for MaxUdpPayloadSize {
//# immediately after completing the handshake.

varint_transport_parameter!(InitialMaxData, 0x04);
varint_from_u32!(InitialMaxData);

/// Computes a data window with the given configuration
pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> VarInt {
pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> u32 {
// ideal throughput in Mbps
let mut window = mbps;
// Mbit/sec * 125 -> bytes/ms
Expand All @@ -619,12 +637,15 @@ pub const fn compute_data_window(mbps: u64, rtt: Duration, rtt_count: u64) -> Va
// bytes/RTT * rtt_count -> N * bytes/RTT
window *= rtt_count;

VarInt::from_u32(window as u32)
window as u32
}

impl InitialMaxData {
/// Tuned for 150Mbps with a 100ms RTT
pub const RECOMMENDED: Self = Self(compute_data_window(150, Duration::from_millis(100), 2));
pub const RECOMMENDED: Self = {
let limit = compute_data_window(150, Duration::from_millis(100), 2);
Self(VarInt::from_u32(limit))
};
}

impl TransportParameterValidator for InitialMaxData {}
Expand All @@ -641,6 +662,7 @@ impl TransportParameterValidator for InitialMaxData {}
//# 0x01.

varint_transport_parameter!(InitialMaxStreamDataBidiLocal, 0x05);
varint_from_u32!(InitialMaxStreamDataBidiLocal);

impl InitialMaxStreamDataBidiLocal {
/// Tuned for 150Mbps throughput with a 100ms RTT
Expand All @@ -660,6 +682,7 @@ impl TransportParameterValidator for InitialMaxStreamDataBidiLocal {}
//# to streams with the least significant two bits set to 0x00.

varint_transport_parameter!(InitialMaxStreamDataBidiRemote, 0x06);
varint_from_u32!(InitialMaxStreamDataBidiRemote);

impl InitialMaxStreamDataBidiRemote {
/// Tuned for 150Mbps throughput with a 100ms RTT
Expand All @@ -679,6 +702,7 @@ impl TransportParameterValidator for InitialMaxStreamDataBidiRemote {}
//# with the least significant two bits set to 0x02.

varint_transport_parameter!(InitialMaxStreamDataUni, 0x07);
varint_from_u32!(InitialMaxStreamDataUni);

impl InitialMaxStreamDataUni {
/// Tuned for 150Mbps throughput with a 100ms RTT
Expand All @@ -698,6 +722,7 @@ impl TransportParameterValidator for InitialMaxStreamDataUni {}
//# corresponding type with the same value.

varint_transport_parameter!(InitialMaxStreamsBidi, 0x08);
varint_from_u64!(InitialMaxStreamsBidi);

impl InitialMaxStreamsBidi {
/// Allow up to 100 concurrent streams at any time
Expand Down Expand Up @@ -735,6 +760,7 @@ impl TransportParameterValidator for InitialMaxStreamsBidi {
//# (Section 19.11) of the corresponding type with the same value.

varint_transport_parameter!(InitialMaxStreamsUni, 0x09);
varint_from_u64!(InitialMaxStreamsUni);

impl InitialMaxStreamsUni {
/// Allow up to 100 concurrent streams at any time
Expand Down Expand Up @@ -1018,6 +1044,7 @@ impl EncoderValue for PreferredAddress {
//# active_connection_id_limit value received from its peer.

varint_transport_parameter!(ActiveConnectionIdLimit, 0x0e, VarInt::from_u8(2));
varint_from_u64!(ActiveConnectionIdLimit);

impl ActiveConnectionIdLimit {
/// The recommended value comes from the default of 2
Expand Down
8 changes: 4 additions & 4 deletions quic/s2n-quic-core/src/transport/parameters/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,19 @@ fn ignore_unknown_parameter() {
#[test]
fn compute_data_window_test() {
assert_eq!(
*compute_data_window(150, Duration::from_millis(10), 1),
compute_data_window(150, Duration::from_millis(10), 1),
187_500
);
assert_eq!(
*compute_data_window(150, Duration::from_millis(10), 2),
compute_data_window(150, Duration::from_millis(10), 2),
375_000
);
assert_eq!(
*compute_data_window(150, Duration::from_millis(100), 2),
compute_data_window(150, Duration::from_millis(100), 2),
3_750_000
);
assert_eq!(
*compute_data_window(1500, Duration::from_millis(100), 2),
compute_data_window(1500, Duration::from_millis(100), 2),
37_500_000
);
}
14 changes: 7 additions & 7 deletions quic/s2n-quic-qns/src/perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,27 +111,27 @@ pub struct Limits {
impl Limits {
pub fn limits(&self) -> s2n_quic::provider::limits::Limits {
let data_window = self.data_window();
let max_data_limit = u32::MAX;

s2n_quic::provider::limits::Limits::default()
.with_data_window(data_window)
.with_max_send_buffer_size(data_window)
.unwrap()
.with_max_send_buffer_size(data_window.min(u32::MAX as _) as _)
.with_data_window(max_data_limit)
.unwrap()
.with_bidirectional_local_data_window(data_window)
.with_bidirectional_local_data_window(max_data_limit)
.unwrap()
.with_bidirectional_remote_data_window(data_window)
.with_bidirectional_remote_data_window(max_data_limit)
.unwrap()
.with_unidirectional_data_window(data_window)
.with_unidirectional_data_window(max_data_limit)
.unwrap()
}

fn data_window(&self) -> u64 {
fn data_window(&self) -> u32 {
s2n_quic_core::transport::parameters::compute_data_window(
self.max_throughput,
core::time::Duration::from_millis(self.expected_rtt),
2,
)
.as_u64()
}
}

Expand Down

0 comments on commit db910d9

Please sign in to comment.