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

fix: Make neqo pass amplificationlimit QNS test #1875

Merged
merged 27 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ impl Connection {
qtrace!([self], "Idle/keepalive timer {:?}", idle_time);
delays.push(idle_time);

if let Some(lr_time) = self.loss_recovery.next_timeout(rtt) {
if let Some(lr_time) = self.loss_recovery.next_timeout(&path) {
qtrace!([self], "Loss recovery timer {:?}", lr_time);
delays.push(lr_time);
}
Expand Down Expand Up @@ -1664,7 +1664,9 @@ impl Connection {
.clone(),
),
);
path.borrow_mut().set_valid(now);
if self.role == Role::Client {
larseggert marked this conversation as resolved.
Show resolved Hide resolved
path.borrow_mut().set_valid(now);
}
}

/// If the path isn't permanent, assign it a connection ID to make it so.
Expand Down Expand Up @@ -2375,7 +2377,7 @@ impl Connection {
}

if encoder.is_empty() {
qdebug!("TX blocked, profile={:?} ", profile);
qdebug!("TX blocked, profile={:?}", profile);
Ok(SendOption::No(profile.paced()))
} else {
// Perform additional padding for Initial packets as necessary.
Expand Down
12 changes: 4 additions & 8 deletions neqo-transport/src/connection/tests/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,19 +783,15 @@ fn anti_amplification() {
let c_init = client.process_output(now).dgram();
now += DEFAULT_RTT / 2;
let s_init1 = server.process(c_init.as_ref(), now).dgram().unwrap();
assert_eq!(s_init1.len(), client.plpmtu());
assert_eq!(s_init1.len(), server.plpmtu());
let s_init2 = server.process_output(now).dgram().unwrap();
assert_eq!(s_init2.len(), server.plpmtu());

// Skip the gap for pacing here.
let s_pacing = server.process_output(now).callback();
assert_ne!(s_pacing, Duration::new(0, 0));
now += s_pacing;

let s_init3 = server.process_output(now).dgram().unwrap();
assert_eq!(s_init3.len(), server.plpmtu());
let cb = server.process_output(now).callback();
assert_ne!(cb, Duration::new(0, 0));

// We are blocked by the amplification limit now.
assert_eq!(cb, server.conn_params.get_idle_timeout());

now += DEFAULT_RTT / 2;
client.process_input(&s_init1, now);
Expand Down
4 changes: 4 additions & 0 deletions neqo-transport/src/connection/tests/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,10 @@ fn loss_time_past_largest_acked() {
let s_hs3 = server.process(None, now + s_pto).dgram();
assert!(s_hs3.is_some());

// We are blocked by the amplification limit now.
let cb = server.process_output(now).callback();
assert_eq!(cb, server.conn_params.get_idle_timeout());

// Get some Handshake packets from the client.
// We need one to be left unacknowledged before one that is acknowledged.
// So that the client engages the loss recovery timer.
Expand Down
11 changes: 8 additions & 3 deletions neqo-transport/src/recovery/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,14 @@ impl LossRecovery {

/// Calculate when the next timeout is likely to be. This is the earlier of the loss timer
/// and the PTO timer; either or both might be disabled, so this can return `None`.
martinthomson marked this conversation as resolved.
Show resolved Hide resolved
pub fn next_timeout(&self, rtt: &RttEstimate) -> Option<Instant> {
pub fn next_timeout(&self, path: &Path) -> Option<Instant> {
let rtt = path.rtt();
let loss_time = self.earliest_loss_time(rtt);
let pto_time = self.earliest_pto(rtt);
let pto_time = if path.amplification_limit() > path.plpmtu() {
larseggert marked this conversation as resolved.
Show resolved Hide resolved
self.earliest_pto(rtt)
} else {
None
};
qtrace!(
[self],
"next_timeout loss={:?} pto={:?}",
Expand Down Expand Up @@ -1013,7 +1018,7 @@ mod tests {
}

pub fn next_timeout(&self) -> Option<Instant> {
self.lr.next_timeout(self.path.borrow().rtt())
self.lr.next_timeout(&self.path.borrow())
}

pub fn discard(&mut self, space: PacketNumberSpace, now: Instant) {
Expand Down
Loading