Skip to content

Commit

Permalink
Export constants for send and recv buffer sizes
Browse files Browse the repository at this point in the history
Followup to mozilla#806, this is simpler than const methods on Connection.

Note that while we can change these constants now, setting them to
different values will cause some tests to fail. Spending time to fix
tests for different const values is not justified yet.
  • Loading branch information
Andy Grover committed Jul 10, 2020
1 parent ff6dc50 commit bd1d5a9
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 71 deletions.
26 changes: 12 additions & 14 deletions neqo-http3/src/connection_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ mod tests {
use neqo_qpack::encoder::QPackEncoder;
use neqo_transport::{
CloseError, ConnectionEvent, FixedConnectionIdManager, QuicVersion, State,
RECV_BUFFER_SIZE, SEND_BUFFER_SIZE,
};
use test_fixture::{
default_server, fixture_init, loopback, now, DEFAULT_ALPN, DEFAULT_SERVER_NAME,
Expand Down Expand Up @@ -1886,7 +1887,7 @@ mod tests {
if let ConnectionEvent::RecvStreamReadable { stream_id } = e {
if stream_id == request_stream_id {
// Read the DATA frame.
let mut buf = vec![1_u8; Connection::stream_recv_buffer_size()];
let mut buf = vec![1_u8; RECV_BUFFER_SIZE];
let (amount, fin) = server.conn.stream_recv(stream_id, &mut buf).unwrap();
assert_eq!(fin, true);
assert_eq!(
Expand Down Expand Up @@ -1958,10 +1959,7 @@ mod tests {
assert_eq!(sent, Ok(first_frame.len()));

// The second frame cannot fit.
let sent = client.send_request_body(
request_stream_id,
&vec![0_u8; Connection::stream_recv_buffer_size()],
);
let sent = client.send_request_body(request_stream_id, &vec![0_u8; SEND_BUFFER_SIZE]);
assert_eq!(sent, Ok(expected_second_data_frame.len()));

// Close stream.
Expand All @@ -1970,7 +1968,7 @@ mod tests {
let mut out = client.process(None, now());
// We need to loop a bit until all data has been sent. Once for every 1K
// of data.
for _i in 0..Connection::stream_send_buffer_size() / 1000 {
for _i in 0..SEND_BUFFER_SIZE / 1000 {
out = server.conn.process(out.dgram(), now());
out = client.process(out.dgram(), now());
}
Expand All @@ -1980,7 +1978,7 @@ mod tests {
if let ConnectionEvent::RecvStreamReadable { stream_id } = e {
if stream_id == request_stream_id {
// Read DATA frames.
let mut buf = vec![1_u8; Connection::stream_recv_buffer_size()];
let mut buf = vec![1_u8; RECV_BUFFER_SIZE];
let (amount, fin) = server.conn.stream_recv(stream_id, &mut buf).unwrap();
assert_eq!(fin, true);
assert_eq!(
Expand Down Expand Up @@ -2033,7 +2031,7 @@ mod tests {
// After the first frame there is exactly 63+2 bytes left in the send buffer.
#[test]
fn fetch_two_data_frame_second_63bytes() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 88);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 88);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x3f], &[0_u8; 63]);
}

Expand All @@ -2042,7 +2040,7 @@ mod tests {
// but we can only send 63 bytes.
#[test]
fn fetch_two_data_frame_second_63bytes_place_for_66() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 89);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 89);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x3f], &[0_u8; 63]);
}

Expand All @@ -2051,39 +2049,39 @@ mod tests {
// but we can only send 64 bytes.
#[test]
fn fetch_two_data_frame_second_64bytes_place_for_67() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 90);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 90);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x40, 0x40], &[0_u8; 64]);
}

// Send 2 frames. For the second one we can only send 16383 bytes.
// After the first frame there is exactly 16383+3 bytes left in the send buffer.
#[test]
fn fetch_two_data_frame_second_16383bytes() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 16409);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 16409);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x7f, 0xff], &[0_u8; 16383]);
}

// Send 2 frames. For the second one we can only send 16383 bytes.
// After the first frame there is exactly 16383+4 bytes left in the send buffer, but we can only send 16383 bytes.
#[test]
fn fetch_two_data_frame_second_16383bytes_place_for_16387() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 16410);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 16410);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x7f, 0xff], &[0_u8; 16383]);
}

// Send 2 frames. For the second one we can only send 16383 bytes.
// After the first frame there is exactly 16383+5 bytes left in the send buffer, but we can only send 16383 bytes.
#[test]
fn fetch_two_data_frame_second_16383bytes_place_for_16388() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 16411);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 16411);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x7f, 0xff], &[0_u8; 16383]);
}

// Send 2 frames. For the second one we can send 16384 bytes.
// After the first frame there is exactly 16384+5 bytes left in the send buffer, but we can send 16384 bytes.
#[test]
fn fetch_two_data_frame_second_16384bytes_place_for_16389() {
let (buf, hdr) = alloc_buffer(Connection::stream_send_buffer_size() - 16412);
let (buf, hdr) = alloc_buffer(SEND_BUFFER_SIZE - 16412);
fetch_with_two_data_frames(&buf, &hdr, &[0x0, 0x80, 0x0, 0x40, 0x0], &[0_u8; 16384]);
}

Expand Down
35 changes: 14 additions & 21 deletions neqo-transport/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ use crate::packet::{
use crate::path::Path;
use crate::qlog;
use crate::recovery::{LossRecovery, RecoveryToken, SendProfile, GRANULARITY};
use crate::recv_stream::{RecvStream, RecvStreams, RX_STREAM_DATA_WINDOW};
use crate::send_stream::{SendStream, SendStreams, TxBuffer};
use crate::recv_stream::{RecvStream, RecvStreams, RECV_BUFFER_SIZE};
use crate::send_stream::{SendStream, SendStreams};
use crate::stats::Stats;
use crate::stream_id::{StreamId, StreamIndex, StreamIndexes};
use crate::tparams::{
Expand Down Expand Up @@ -526,13 +526,16 @@ impl Connection {
fn set_tp_defaults(tps: &mut TransportParameters) {
tps.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_LOCAL,
RX_STREAM_DATA_WINDOW,
u64::try_from(RECV_BUFFER_SIZE).unwrap(),
);
tps.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_BIDI_REMOTE,
RX_STREAM_DATA_WINDOW,
u64::try_from(RECV_BUFFER_SIZE).unwrap(),
);
tps.set_integer(
tparams::INITIAL_MAX_STREAM_DATA_UNI,
u64::try_from(RECV_BUFFER_SIZE).unwrap(),
);
tps.set_integer(tparams::INITIAL_MAX_STREAM_DATA_UNI, RX_STREAM_DATA_WINDOW);
tps.set_integer(tparams::INITIAL_MAX_STREAMS_BIDI, LOCAL_STREAM_LIMIT_BIDI);
tps.set_integer(tparams::INITIAL_MAX_STREAMS_UNI, LOCAL_STREAM_LIMIT_UNI);
tps.set_integer(tparams::INITIAL_MAX_DATA, LOCAL_MAX_DATA);
Expand Down Expand Up @@ -2588,16 +2591,6 @@ impl Connection {
Ok(())
}

/// The per-stream send buffer size.
pub const fn stream_send_buffer_size() -> usize {
TxBuffer::BUFFER_SIZE
}

/// The per-stream receive max data size.
pub const fn stream_recv_buffer_size() -> usize {
RX_STREAM_DATA_WINDOW as usize
}

/// Get all current events. Best used just in debug/testing code, use
/// next_event() instead.
pub fn events(&mut self) -> impl Iterator<Item = ConnectionEvent> {
Expand Down Expand Up @@ -2633,7 +2626,7 @@ mod tests {
use crate::path::PATH_MTU_V6;
use crate::recovery::ACK_ONLY_SIZE_LIMIT;
use crate::recovery::PTO_PACKET_COUNT;
use crate::send_stream::TxBuffer;
use crate::send_stream::SEND_BUFFER_SIZE;
use crate::tracking::{ACK_DELAY, MAX_UNACKED_PKTS};
use std::convert::TryInto;

Expand Down Expand Up @@ -3591,7 +3584,7 @@ mod tests {
);
assert_eq!(
client
.stream_send(stream_id, &[b'a'; RX_STREAM_DATA_WINDOW as usize])
.stream_send(stream_id, &[b'a'; RECV_BUFFER_SIZE])
.unwrap(),
SMALL_MAX_DATA
);
Expand Down Expand Up @@ -3619,7 +3612,7 @@ mod tests {
client.handle_max_data(100_000_000);
assert_eq!(
client.stream_avail_send_space(stream_id).unwrap(),
TxBuffer::BUFFER_SIZE - SMALL_MAX_DATA
SEND_BUFFER_SIZE - SMALL_MAX_DATA
);

// Increase max stream data. Avail space now limited by tx buffer
Expand All @@ -3630,7 +3623,7 @@ mod tests {
.set_max_stream_data(100_000_000);
assert_eq!(
client.stream_avail_send_space(stream_id).unwrap(),
TxBuffer::BUFFER_SIZE - SMALL_MAX_DATA + 4096
SEND_BUFFER_SIZE - SMALL_MAX_DATA + 4096
);

let evts = client.events().collect::<Vec<_>>();
Expand Down Expand Up @@ -5786,7 +5779,7 @@ mod tests {
server
.flow_mgr
.borrow_mut()
.stream_data_blocked(3.into(), RX_STREAM_DATA_WINDOW * 4);
.stream_data_blocked(3.into(), RECV_BUFFER_SIZE as u64 * 4);

let out = server.process(None, now);
assert!(out.as_dgram_ref().is_some());
Expand All @@ -5804,7 +5797,7 @@ mod tests {
// window value.
assert!(frames.iter().any(
|(f, _)| matches!(f, Frame::MaxStreamData { maximum_stream_data, .. }
if *maximum_stream_data == RX_STREAM_DATA_WINDOW)
if *maximum_stream_data == RECV_BUFFER_SIZE as u64)
));
}
}
2 changes: 2 additions & 0 deletions neqo-transport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub use self::packet::QuicVersion;
pub use self::stream_id::StreamId;

const LOCAL_IDLE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); // 30 second
pub use self::recv_stream::RECV_BUFFER_SIZE;
pub use self::send_stream::SEND_BUFFER_SIZE;

type TransportError = u64;
const ERROR_APPLICATION_CLOSE: TransportError = 12;
Expand Down
5 changes: 4 additions & 1 deletion neqo-transport/src/recv_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ use crate::stream_id::StreamId;
use crate::{AppError, Error, Res};
use neqo_common::qtrace;

pub const RX_STREAM_DATA_WINDOW: u64 = 0x10_0000; // 1MiB
const RX_STREAM_DATA_WINDOW: u64 = 0x10_0000; // 1MiB

// Export as usize for consistency with SEND_BUFFER_SIZE
pub const RECV_BUFFER_SIZE: usize = RX_STREAM_DATA_WINDOW as usize;

pub(crate) type RecvStreams = BTreeMap<StreamId, RecvStream>;

Expand Down
Loading

0 comments on commit bd1d5a9

Please sign in to comment.