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

(CC refactor 9) recovery: move congestion into module #1780

Open
wants to merge 5 commits into
base: cc-refactor-8-cc-struct
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17288,7 +17288,7 @@ pub use crate::path::PathEvent;
pub use crate::path::PathStats;
pub use crate::path::SocketAddrIter;

pub use crate::recovery::CongestionControlAlgorithm;
pub use crate::recovery::congestion::CongestionControlAlgorithm;

pub use crate::stream::StreamIter;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ use crate::recovery::*;

use std::time::Duration;

pub static BBR: CongestionControlOps = CongestionControlOps {
use super::CongestionControlOps;

pub(crate) static BBR: CongestionControlOps = CongestionControlOps {
on_init,
reset,
on_packet_sent,
on_packets_acked,
congestion_event,
collapse_cwnd,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why reset() and collape_cwnd() is removed? (I understand it's not used, but no plan to implement persistent congestion in the future?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably want to do it at some point... but that's been the case for years, so for now removing them simplifies the refactoring a bit. We can add them back when needed.

checkpoint,
rollback,
has_custom_pacing,
Expand Down Expand Up @@ -281,12 +281,6 @@ fn on_init(r: &mut Congestion) {
init::bbr_init(r);
}

fn reset(r: &mut Congestion) {
r.bbr_state = State::new();

init::bbr_init(r);
}

fn on_packet_sent(
r: &mut Congestion, _sent_bytes: usize, bytes_in_flight: usize, _now: Instant,
) {
Expand Down Expand Up @@ -333,12 +327,6 @@ fn congestion_event(
}
}

fn collapse_cwnd(r: &mut Congestion, bytes_in_flight: usize) {
r.bbr_state.prior_cwnd = per_ack::bbr_save_cwnd(r);

reno::collapse_cwnd(r, bytes_in_flight);
}

fn checkpoint(_r: &mut Congestion) {}

fn rollback(_r: &mut Congestion) -> bool {
Expand All @@ -365,8 +353,14 @@ mod tests {

use crate::recovery;

use self::congestion::test_sender::TestSender;

use smallvec::smallvec;

fn test_sender() -> TestSender {
TestSender::new(recovery::CongestionControlAlgorithm::BBR, false)
}

#[test]
fn bbr_init() {
let mut cfg = crate::Config::new(crate::PROTOCOL_VERSION).unwrap();
Expand All @@ -383,86 +377,34 @@ mod tests {
assert_eq!(r.congestion.bbr_state.state, BBRStateMachine::Startup);
}

#[test]
fn bbr_send() {
let mut cfg = crate::Config::new(crate::PROTOCOL_VERSION).unwrap();
cfg.set_cc_algorithm(recovery::CongestionControlAlgorithm::BBR);

let mut r = Recovery::new(&cfg);
let now = Instant::now();

r.on_packet_sent_cc(0, 1000, now);

assert_eq!(r.bytes_in_flight, 1000);
}

#[test]
fn bbr_startup() {
let mut cfg = crate::Config::new(crate::PROTOCOL_VERSION).unwrap();
cfg.set_cc_algorithm(recovery::CongestionControlAlgorithm::BBR);
let mut sender = test_sender();
let mss = sender.max_datagram_size;

let mut r = Recovery::new(&cfg);
let now = Instant::now();
let mss = r.max_datagram_size;
let rtt = Duration::from_millis(50);
sender.update_rtt(rtt);
sender.advance_time(rtt);

// Send 5 packets.
for pn in 0..5 {
let pkt = Sent {
pkt_num: pn,
frames: smallvec![],
time_sent: now,
time_acked: None,
time_lost: None,
size: mss,
ack_eliciting: true,
in_flight: true,
delivered: 0,
delivered_time: now,
first_sent_time: now,
is_app_limited: false,
tx_in_flight: 0,
lost: 0,
has_data: false,
pmtud: false,
};

r.on_packet_sent(
pkt,
packet::Epoch::Application,
HandshakeStatus::default(),
now,
"",
);
for _ in 0..5 {
sender.send_packet(mss);
}

let rtt = Duration::from_millis(50);
let now = now + rtt;
let cwnd_prev = r.cwnd();
sender.advance_time(rtt);

let mut acked = ranges::RangeSet::default();
acked.insert(0..5);
let cwnd_prev = sender.congestion_window;

assert_eq!(
r.on_ack_received(
&acked,
25,
packet::Epoch::Application,
HandshakeStatus::default(),
now,
"",
&mut Vec::new(),
),
Ok((0, 0)),
);
sender.ack_n_packets(5, mss);

assert_eq!(r.congestion.bbr_state.state, BBRStateMachine::Startup);
assert_eq!(r.cwnd(), cwnd_prev + mss * 5);
assert_eq!(r.bytes_in_flight, 0);
assert_eq!(sender.bbr_state.state, BBRStateMachine::Startup);
assert_eq!(sender.congestion_window, cwnd_prev + mss * 5);
assert_eq!(sender.bytes_in_flight, 0);
assert_eq!(
r.delivery_rate(),
sender.delivery_rate(),
((mss * 5) as f64 / rtt.as_secs_f64()) as u64
);
assert_eq!(r.congestion.bbr_state.btlbw, r.delivery_rate());
assert_eq!(sender.bbr_state.btlbw, sender.delivery_rate());
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ use crate::recovery::*;
use std::time::Duration;
use std::time::Instant;

pub static BBR2: CongestionControlOps = CongestionControlOps {
use super::CongestionControlOps;

pub(crate) static BBR2: CongestionControlOps = CongestionControlOps {
on_init,
reset,
on_packet_sent,
on_packets_acked,
congestion_event,
collapse_cwnd,
checkpoint,
rollback,
has_custom_pacing,
Expand Down Expand Up @@ -548,12 +548,6 @@ fn on_init(r: &mut Congestion) {
init::bbr2_init(r);
}

fn reset(r: &mut Congestion) {
r.bbr2_state = State::new();

init::bbr2_init(r);
}

fn on_packet_sent(
r: &mut Congestion, _sent_bytes: usize, bytes_in_flight: usize, now: Instant,
) {
Expand Down Expand Up @@ -607,13 +601,6 @@ fn congestion_event(
}
}

fn collapse_cwnd(r: &mut Congestion, bytes_in_flight: usize) {
// BBROnEnterRTO()
r.bbr2_state.prior_cwnd = per_ack::bbr2_save_cwnd(r);

r.congestion_window = bytes_in_flight + r.max_datagram_size;
}

fn checkpoint(_r: &mut Congestion) {}

fn rollback(_r: &mut Congestion) -> bool {
Expand Down Expand Up @@ -697,19 +684,6 @@ mod tests {
assert_eq!(r.congestion.bbr2_state.state, BBR2StateMachine::Startup);
}

#[test]
fn bbr2_send() {
let mut cfg = crate::Config::new(crate::PROTOCOL_VERSION).unwrap();
cfg.set_cc_algorithm(recovery::CongestionControlAlgorithm::BBR2);

let mut r = Recovery::new(&cfg);
let now = Instant::now();

r.on_packet_sent_cc(0, 1000, now);

assert_eq!(r.bytes_in_flight, 1000);
}

#[test]
fn bbr2_startup() {
let mut cfg = crate::Config::new(crate::PROTOCOL_VERSION).unwrap();
Expand Down