Skip to content

Commit

Permalink
proto: use deterministic packet numbers in tests that count ACKs
Browse files Browse the repository at this point in the history
Prevents spurious failures when a skipped packet number causes an
extra ACK to be sent by reordering detection logic.
  • Loading branch information
Ralith committed Aug 21, 2023
1 parent bef7249 commit f07a40d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 10 deletions.
20 changes: 10 additions & 10 deletions quinn-proto/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2212,8 +2212,8 @@ fn packet_splitting_not_necessary_after_higher_mtu_discovered() {
#[test]
fn single_ack_eliciting_packet_triggers_ack_after_delay() {
let _guard = subscribe();
let mut pair = Pair::default();
let (client_ch, _) = pair.connect();
let mut pair = Pair::default_with_deterministic_pns();
let (client_ch, _) = pair.connect_with(client_config_with_deterministic_pns());
pair.drive();

let stats_after_connect = pair.client_conn_mut(client_ch).stats();
Expand Down Expand Up @@ -2275,8 +2275,8 @@ fn single_ack_eliciting_packet_triggers_ack_after_delay() {
#[test]
fn immediate_ack_triggers_ack() {
let _guard = subscribe();
let mut pair = Pair::default();
let (client_ch, _) = pair.connect();
let mut pair = Pair::default_with_deterministic_pns();
let (client_ch, _) = pair.connect_with(client_config_with_deterministic_pns());
pair.drive();

let acks_after_connect = pair.client_conn_mut(client_ch).stats().frame_rx.acks;
Expand All @@ -2294,8 +2294,8 @@ fn immediate_ack_triggers_ack() {
#[test]
fn out_of_order_ack_eliciting_packet_triggers_ack() {
let _guard = subscribe();
let mut pair = Pair::default();
let (client_ch, server_ch) = pair.connect();
let mut pair = Pair::default_with_deterministic_pns();
let (client_ch, server_ch) = pair.connect_with(client_config_with_deterministic_pns());
pair.drive();

let default_mtu = pair.mtu;
Expand Down Expand Up @@ -2352,8 +2352,8 @@ fn out_of_order_ack_eliciting_packet_triggers_ack() {
#[test]
fn single_ack_eliciting_packet_with_ce_bit_triggers_immediate_ack() {
let _guard = subscribe();
let mut pair = Pair::default();
let (client_ch, _) = pair.connect();
let mut pair = Pair::default_with_deterministic_pns();
let (client_ch, _) = pair.connect_with(client_config_with_deterministic_pns());
pair.drive();

let stats_after_connect = pair.client_conn_mut(client_ch).stats();
Expand Down Expand Up @@ -2388,7 +2388,7 @@ fn single_ack_eliciting_packet_with_ce_bit_triggers_immediate_ack() {
}

fn setup_ack_frequency_test(max_ack_delay: Duration) -> (Pair, ConnectionHandle, ConnectionHandle) {
let mut client_config = client_config();
let mut client_config = client_config_with_deterministic_pns();
let mut ack_freq_config = AckFrequencyConfig::default();
ack_freq_config
.ack_eliciting_threshold(10u32.into())
Expand All @@ -2398,7 +2398,7 @@ fn setup_ack_frequency_test(max_ack_delay: Duration) -> (Pair, ConnectionHandle,
.ack_frequency_config(Some(ack_freq_config))
.mtu_discovery_config(None); // To keep traffic cleaner

let mut pair = Pair::default();
let mut pair = Pair::default_with_deterministic_pns();
pair.latency = Duration::from_millis(10); // Need latency to avoid an RTT = 0
let (client_ch, server_ch) = pair.connect_with(client_config);
pair.drive();
Expand Down
16 changes: 16 additions & 0 deletions quinn-proto/src/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ pub(super) struct Pair {
}

impl Pair {
pub(super) fn default_with_deterministic_pns() -> Self {
let mut cfg = server_config();
let mut transport = TransportConfig::default();
transport.deterministic_packet_numbers(true);
cfg.transport = Arc::new(transport);
Self::new(Default::default(), cfg)
}

pub(super) fn new(endpoint_config: Arc<EndpointConfig>, server_config: ServerConfig) -> Self {
let server = Endpoint::new(endpoint_config.clone(), Some(Arc::new(server_config)), true);
let client = Endpoint::new(endpoint_config, None, true);
Expand Down Expand Up @@ -466,6 +474,14 @@ pub(super) fn client_config() -> ClientConfig {
ClientConfig::new(Arc::new(client_crypto()))
}

pub(super) fn client_config_with_deterministic_pns() -> ClientConfig {
let mut cfg = ClientConfig::new(Arc::new(client_crypto()));
let mut transport = TransportConfig::default();
transport.deterministic_packet_numbers(true);
cfg.transport = Arc::new(transport);
cfg
}

pub(super) fn client_config_with_certs(certs: Vec<rustls::Certificate>) -> ClientConfig {
ClientConfig::new(Arc::new(client_crypto_with_certs(certs)))
}
Expand Down

0 comments on commit f07a40d

Please sign in to comment.