Skip to content

Commit bd0c3bf

Browse files
0xdeafbeefdjc
authored andcommitted
congestion: preserve excess CUBIC cwnd increment
CUBIC congestion avoidance can accumulate more than one MTU of cwnd_inc credit. After growing the window by one MTU, Quinn reset cwnd_inc to zero, discarding the excess credit. RFC 9002 section 7.3.3 limits a single congestion-avoidance increase to one max_datagram_size per congestion window acknowledged; it does not require throwing away the remainder. Subtract one MTU from cwnd_inc instead, and saturate the accumulator so retained credit cannot wrap on large CUBIC targets.
1 parent 3c43d45 commit bd0c3bf

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

quinn-proto/src/congestion/cubic.rs

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,20 @@ impl Controller for Cubic {
162162
}
163163

164164
// Update the increment and increase cwnd by MSS.
165-
self.state.cwnd_inc += cubic_cwnd - self.state.window;
165+
// Keep release builds from wrapping the retained credit.
166+
self.state.cwnd_inc = self
167+
.state
168+
.cwnd_inc
169+
.saturating_add(cubic_cwnd - self.state.window);
166170

167171
// cwnd_inc can be more than 1 MSS in the late stage of max probing.
168172
// however RFC9002 §7.3.3 (Congestion Avoidance) limits
169173
// the increase of cwnd to 1 max_datagram_size per cwnd acknowledged.
174+
// Keep the excess credit for later ACKs instead of discarding it.
175+
// https://www.rfc-editor.org/rfc/rfc9002.html#section-7.3.3
170176
if self.state.cwnd_inc >= self.current_mtu {
171177
self.state.window += self.current_mtu;
172-
self.state.cwnd_inc = 0;
178+
self.state.cwnd_inc -= self.current_mtu;
173179
}
174180
}
175181
}
@@ -298,7 +304,6 @@ impl ControllerFactory for CubicConfig {
298304
Box::new(Cubic::new(self, now, current_mtu))
299305
}
300306
}
301-
302307
#[cfg(test)]
303308
mod tests {
304309
use super::*;
@@ -320,4 +325,35 @@ mod tests {
320325
assert_eq!(cubic.state.ssthresh, (window as f64 * BETA_CUBIC) as u64);
321326
assert_eq!(cubic.state.window, cubic.state.ssthresh);
322327
}
328+
329+
#[test]
330+
fn congestion_avoidance_preserves_excess_cwnd_increment() {
331+
let now = Instant::now();
332+
let rtt = RttEstimator::new(Duration::from_millis(100));
333+
let config = Arc::new(CubicConfig::default());
334+
let mut cubic = Cubic::new(config, now, BASE_DATAGRAM_SIZE as u16);
335+
336+
// Put CUBIC directly into congestion avoidance.
337+
cubic.state.ssthresh = cubic.state.window;
338+
cubic.state.recovery_start_time = Some(now);
339+
cubic.state.w_max = cubic.state.window as f64;
340+
341+
// Simulate accumulated credit from earlier ACKs. One ACK may only grow
342+
// the window by one MTU, but the extra credit must remain for later ACKs.
343+
cubic.state.cwnd_inc = 2 * BASE_DATAGRAM_SIZE + 1;
344+
let window = cubic.state.window;
345+
346+
cubic.on_ack(
347+
now,
348+
now + Duration::from_millis(1),
349+
BASE_DATAGRAM_SIZE,
350+
false,
351+
&rtt,
352+
);
353+
354+
// Before this fix, the window grew by one MTU and the remainder was
355+
// reset to zero.
356+
assert_eq!(cubic.state.window, window + BASE_DATAGRAM_SIZE);
357+
assert_eq!(cubic.state.cwnd_inc, BASE_DATAGRAM_SIZE + 1);
358+
}
323359
}

quinn-proto/src/connection/paths.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ pub struct RttEstimator {
311311
}
312312

313313
impl RttEstimator {
314-
fn new(initial_rtt: Duration) -> Self {
314+
pub(crate) fn new(initial_rtt: Duration) -> Self {
315315
Self {
316316
latest: initial_rtt,
317317
smoothed: None,

0 commit comments

Comments
 (0)