From 62253527bca6888ae87d42decee90c4cb44e74fa Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sun, 26 Jan 2025 12:54:10 +0100 Subject: [PATCH] feat: use Cubic CC by default (#2295) * feat: use Cubic CC by default Firefox uses Cubic by default: ``` yaml - name: network.http.http3.cc_algorithm type: RelaxedAtomicUint32 value: 1 mirror: always rust: true ``` https://searchfox.org/mozilla-central/rev/f9517009d8a4946dbdd3acd72a31dc34fca79586/modules/libpref/init/StaticPrefList.yaml This commit updates Neqo to use Cubic instead of New Reno by default. * test(cc): support both cubic and new reno * Trigger CI --- neqo-bin/src/lib.rs | 4 +- neqo-transport/src/cc/tests/cubic.rs | 2 +- neqo-transport/src/connection/params.rs | 2 +- neqo-transport/src/connection/tests/cc.rs | 66 +++++++++-------------- 4 files changed, 29 insertions(+), 45 deletions(-) diff --git a/neqo-bin/src/lib.rs b/neqo-bin/src/lib.rs index c4c0bf855c..1e25f8690f 100644 --- a/neqo-bin/src/lib.rs +++ b/neqo-bin/src/lib.rs @@ -112,7 +112,7 @@ pub struct QuicParameters { /// The idle timeout for connections, in seconds. pub idle_timeout: u64, - #[arg(long = "cc", default_value = "newreno")] + #[arg(long = "cc", default_value = "cubic")] /// The congestion controller to use. pub congestion_control: CongestionControlAlgorithm, @@ -145,7 +145,7 @@ impl Default for QuicParameters { max_streams_bidi: 16, max_streams_uni: 16, idle_timeout: 30, - congestion_control: CongestionControlAlgorithm::NewReno, + congestion_control: CongestionControlAlgorithm::Cubic, no_pacing: false, no_pmtud: false, preferred_address_v4: None, diff --git a/neqo-transport/src/cc/tests/cubic.rs b/neqo-transport/src/cc/tests/cubic.rs index edf0984d9d..cf76ae1300 100644 --- a/neqo-transport/src/cc/tests/cubic.rs +++ b/neqo-transport/src/cc/tests/cubic.rs @@ -180,7 +180,7 @@ fn tcp_phase() { assert!(num_acks2 < expected_ack_tcp_increase2); // The time needed to increase cwnd by MAX_DATAGRAM_SIZE using the cubic equation will be - // calculates from: W_cubic(elapsed_time + t_to_increase) - W_cubic(elapsed_time) = + // calculated from: W_cubic(elapsed_time + t_to_increase) - W_cubic(elapsed_time) = // MAX_DATAGRAM_SIZE => CUBIC_C * (elapsed_time + t_to_increase)^3 * MAX_DATAGRAM_SIZE + // CWND_INITIAL - CUBIC_C * elapsed_time^3 * MAX_DATAGRAM_SIZE + CWND_INITIAL = // MAX_DATAGRAM_SIZE => t_to_increase = cbrt((1 + CUBIC_C * elapsed_time^3) / CUBIC_C) - diff --git a/neqo-transport/src/connection/params.rs b/neqo-transport/src/connection/params.rs index 5260415b2d..3b25b6e234 100644 --- a/neqo-transport/src/connection/params.rs +++ b/neqo-transport/src/connection/params.rs @@ -91,7 +91,7 @@ impl Default for ConnectionParameters { fn default() -> Self { Self { versions: VersionConfig::default(), - cc_algorithm: CongestionControlAlgorithm::NewReno, + cc_algorithm: CongestionControlAlgorithm::Cubic, max_data: LOCAL_MAX_DATA, max_stream_data_bidi_remote: u64::try_from(RECV_BUFFER_SIZE).unwrap(), max_stream_data_bidi_local: u64::try_from(RECV_BUFFER_SIZE).unwrap(), diff --git a/neqo-transport/src/connection/tests/cc.rs b/neqo-transport/src/connection/tests/cc.rs index 0209229b5b..b1d2509f27 100644 --- a/neqo-transport/src/connection/tests/cc.rs +++ b/neqo-transport/src/connection/tests/cc.rs @@ -19,8 +19,7 @@ use crate::{ recovery::{ACK_ONLY_SIZE_LIMIT, PACKET_THRESHOLD}, sender::PACING_BURST_SIZE, stream_id::StreamType, - tracking::DEFAULT_ACK_PACKET_TOLERANCE, - ConnectionParameters, + CongestionControlAlgorithm, ConnectionParameters, }; #[test] @@ -211,12 +210,11 @@ fn single_packet_on_recovery() { assert!(dgram.is_some()); } -#[test] /// Verify that CC moves out of recovery period when packet sent after start /// of recovery period is acked. -fn cc_cong_avoidance_recovery_period_to_cong_avoidance() { - let mut client = default_client(); - let mut server = default_server(); +fn cc_cong_avoidance_recovery_period_to_cong_avoidance(cc_algorithm: CongestionControlAlgorithm) { + let mut client = new_client(ConnectionParameters::default().cc_algorithm(cc_algorithm)); + let mut server = new_server(ConnectionParameters::default().cc_algorithm(cc_algorithm)); let now = connect_rtt_idle(&mut client, &mut server, DEFAULT_RTT); // Create stream 0 @@ -233,59 +231,45 @@ fn cc_cong_avoidance_recovery_period_to_cong_avoidance() { now += DEFAULT_RTT / 2; let s_ack = ack_bytes(&mut server, stream_id, c_tx_dgrams, now); + let cwnd_before_loss = cwnd(&client); + // Client: Process ack now += DEFAULT_RTT / 2; client.process_input(s_ack, now); + let cwnd_after_loss = cwnd(&client); + // Should be in CARP now. now += DEFAULT_RTT / 2; + assert!(cwnd_before_loss > cwnd_after_loss); qinfo!("moving to congestion avoidance {}", cwnd(&client)); - // Now make sure that we increase congestion window according to the - // accurate byte counting version of congestion avoidance. - // Check over several increases to be sure. - let mut expected_cwnd = cwnd(&client); - // Fill cwnd. - let (mut c_tx_dgrams, next_now) = fill_cwnd(&mut client, stream_id, now); - now = next_now; - for i in 0..5 { + for i in 0..6 { qinfo!("iteration {i}"); - let c_tx_size: usize = c_tx_dgrams.iter().map(Datagram::len).sum(); + let (c_tx_dgrams, next_now) = fill_cwnd(&mut client, stream_id, now); qinfo!( - "client sending {c_tx_size} bytes into cwnd of {}", + "client sending {} bytes into cwnd of {}", + c_tx_dgrams.iter().map(Datagram::len).sum::(), cwnd(&client) ); - assert_eq!(c_tx_size, expected_cwnd); - - // As acks arrive we will continue filling cwnd and save all packets - // from this cycle will be stored in next_c_tx_dgrams. - let mut next_c_tx_dgrams: Vec = Vec::new(); - - // Until we process all the packets, the congestion window remains the same. - // Note that we need the client to process ACK frames in stages, so split the - // datagrams into two, ensuring that we allow for an ACK for each batch. - let most = c_tx_dgrams.len() - usize::try_from(DEFAULT_ACK_PACKET_TOLERANCE).unwrap() - 1; - let s_ack = ack_bytes(&mut server, stream_id, c_tx_dgrams.drain(..most), now); - assert_eq!(cwnd(&client), expected_cwnd); - client.process_input(s_ack, now); - // make sure to fill cwnd again. - let (mut new_pkts, next_now) = fill_cwnd(&mut client, stream_id, now); now = next_now; - next_c_tx_dgrams.append(&mut new_pkts); let s_ack = ack_bytes(&mut server, stream_id, c_tx_dgrams, now); - assert_eq!(cwnd(&client), expected_cwnd); client.process_input(s_ack, now); - // make sure to fill cwnd again. - let (mut new_pkts, next_now) = fill_cwnd(&mut client, stream_id, now); - now = next_now; - next_c_tx_dgrams.append(&mut new_pkts); - - expected_cwnd += client.plpmtu(); - assert_eq!(cwnd(&client), expected_cwnd); - c_tx_dgrams = next_c_tx_dgrams; } + + assert!(cwnd_before_loss < cwnd(&client)); +} + +#[test] +fn cc_cong_avoidance_recovery_period_to_cong_avoidance_new_reno() { + cc_cong_avoidance_recovery_period_to_cong_avoidance(CongestionControlAlgorithm::NewReno); +} + +#[test] +fn cc_cong_avoidance_recovery_period_to_cong_avoidance_cubic() { + cc_cong_avoidance_recovery_period_to_cong_avoidance(CongestionControlAlgorithm::Cubic); } #[test]