cloudflare / cloudflare/quiche
Weird sawtooth cwnd in response to spurious congestion events
- Dominant language
- Rust
- Stars
- 11.8k
- Forks
- 1.1k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 16
Description
Hello! I am plotting the cwnd of tcp and curl+quiche over a long-running connection in the same simulation environment as I've previously described. The BDP of this connection is ~250 packets, which is the optimal cwnd. You can see that TCP grows to this cwnd, reduces it in response to congestion loss, then grows and repeats. _However, when quic's cwnd reaches ~250 packets, it starts to exhibit this weird sawtooth behavior._ The quic client oscillates between decreasing its cwnd based on a [congestion_event](https://github.com/cloudflare/quiche/blob/master/quiche/src/recovery/cubic.rs#L373) and doing a [rollback](https://github.com/cloudflare/quiche/blob/master/quiche/src/recovery/cubic.rs#L415). This is unexpected behavior.
I think I've traced the issue to an incorrect interpretation of [draft-ietf-tcpm-rfc8312bis-00](https://datatracker.ietf.org/doc/id/draft-ietf-tcpm-rfc8312bis-00.html#section-4.9) (as cited in the code):
"In cases where CUBIC reduces its congestion window in response to having detected packet loss via duplicate ACKs or timeouts, there is a possibility that the missing ACK would arrive after the congestion window reduction and a corresponding packet retransmission."
In these lines of code: https://github.com/cloudflare/quiche/blob/master/quiche/src/recovery/cubic.rs#L229-L249, a spurious congestion event is implemented as when not that many packets, in general, have been lost since the last cubic state was saved. But the RFC states that spurious congestion events are when we mark a packet as lost and start congestion recovery, but then that packet is ACKed. Does that seem right? I don't think I should be observing any spurious loss events in this simulation.
```
// Detecting spurious congestion events.
//
//
// When the recovery episode ends with recovering
// a few packets (less than cwnd / mss * ROLLBACK_THRESHOLD_PERCENT(%)), it's
// considered as spurious and restore to the previous state.
if r.congestion_recovery_start_time.is_some() {
let new_lost = r.lost_count - r.cubic_state.prior.lost_count;
let rollback_threshold = (r.congestion_window / r.max_datagram_size) *
ROLLBACK_THRESHOLD_PERCENT /
100;
let rollback_threshold = rollback_threshold.max(MIN_ROLLBACK_THRESHOLD);
if new_lost < rollback_threshold {
let did_rollback = rollback(r);
if did_rollback {
return;
}
}
}
```
Contributor guide
Research direction
Start in quiche/src/recovery/cubic.rs, especially the spurious congestion detection around lines 229-249 and the congestion_event and rollback paths around lines 373 and 415. Compare the implementation with section 4.9 of draft-ietf-tcpm-rfc8312bis-00 and reproduce the long-running simulation described in the issue. Done means the cwnd no longer shows unexpected sawtooth oscillation from incorrectly detected spurious events.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100