BBRv2: CWND plateaus at low value due to misinterpreted idleness

Hi,
We noticed that downloading a 1GB file over a network with an RTT of 1 second with Tokio-Quiche and BBR (v2) sometimes takes a lot longer than expected - instead of roughly 450 seconds, the Flow Completion Time jumps to 10,000 seconds. We initially noticed this in a plot showing the cwnd, which would plateau at an extremely low value, thus referring to this issue as a plateauing issue.

Out of over 4099 emulations, this issue occurred in 54 runs, resulting in a 1% rate, all using BBR as the congestion control algorithm, interestingly only on machines using ``Ubuntu 24.04```.

Looking at the Wireshark capture file from both a "normal" and a "plateauing" run, a difference at the start of the connection became evident.

In a normal run, the server would respond to the client's GET request with a separate packet containing only the HTTP/3 response "200 OK". This was followed 100ms later with packets containing the actual file data. One second after the HTTP3 response was sent, the server would receive the ACK for this response, 100ms before receiving the ACK for the first packet containing file data, just after sending another packet containing more file data.

In a plateau run, the server responds to the GET request not with a separate 200 OK HTTP3 header, but rather, this header is gathered together with initial file data. Notice that this packet is also not sent 100ms before the next packet containing more file data, as in a normal run, but rather sent basically simultaneously. One second later, an ACK was received from the client, ACKing both the packet containing the "200 OK" header and the following data.

Whenever an ACK is received, Quiche iterates over the sent packets and checks which have been acknowledged, understanding them as having arrived and therefore adding the packet size to the variable of ```acked_bytes```.

This variable is then subtracted from the total ```bytes_in_flight```, meaning that if all previously sent packets have just been acknowledged, no bytes are currently in flight, resulting in ```bytes_in_flight``` being set to ```0```. This is what happens in the plateau runs.

The consequence of the ```bytes_in_flight``` equaling ```0``` cause the pacer to assume that the connection is idle, entering the ``` on_enter_quiescence``` function. That method, in turn, sets ```last_quiescence_start``` to the given time.

Before the next packet is sent, BBR checks again whether ```bytes_in_flight == 0```, which is the case in the plateau run, thus the function ```on_exit_quiescence``` is called. In case the variable ```last_quiescence_start``` has a value (which is the case if we just entered quiescence on the last received ACK), this value is retrieved and handed over to the concrete BBR state to continue computation. If we are in the ProbeBW state, the following method is called:

*quiche/src/recovery/gcongestion/bbr2/probe_bw.rs:*
```
fn on_exit_quiescence(
mut self, now: Instant, quiescence_start_time: Instant, _params: &Params,
) -> Mode {
self.model
.postpone_min_rtt_timestamp(now - quiescence_start_time);
Mode::ProbeBW(self)
}
```

This method then calls ```postpone_min_rtt_timestamp```, which in turn forcibly updates the min_rtt_filter, setting it's ```min_rtt``` to the current min_rtt and modifying the timestamp of the ```min_rtt```.

While debugging, we also noticed that in a plateau run, the Probe_RTT state is never entered; instead the run is stuck in the Probe_BW state:

Normal run: States | Plateau run: States
:-------------------------:|:-------------------------:
|

Investigating the Probe_BW state further, we realized that the run seemed to be stuck in the UP cycle.

Normal run: Cycle | Plateau run: Cycle
:-------------------------:|:-------------------------:
|

As Quiche only checks whether to enter ProbeRTT when leaving the Down-cycle, the state is never switched in this plateauing case:

*quiche/src/recovery/gcongestion/bbr2/probe_bw.rs:*
```
fn on_congestion_event(
mut self, prior_in_flight: usize, event_time: Instant, _: &[Acked],
_: &[Lost], congestion_event: &mut BBRv2CongestionEvent,
target_bytes_inflight: usize, params: &Params,
_recovery_stats: &mut RecoveryStats, cwnd: usize,
) -> Mode {
[...]

match self.cycle.phase {
CyclePhase::NotStarted => unreachable!(),
CyclePhase::Up => self.update_probe_up(
prior_in_flight,
target_bytes_inflight,
congestion_event,
params,
),
CyclePhase::Down => {
self.update_probe_down(
target_bytes_inflight,
congestion_event,
params,
);
if self.cycle.phase != CyclePhase::Down &&
self.model.maybe_expire_min_rtt(congestion_event, params)
{
switch_to_probe_rtt = true;
}
},
CyclePhase::Cruise => self.update_probe_cruise(
target_bytes_inflight,
congestion_event,
params,
),
CyclePhase::Refill => self.update_probe_refill(
target_bytes_inflight,
congestion_event,
params,
),
}
[...]
}
```

We propose two fixes;
one to fix the wrongful determination of the idle state, similar to the fix in Cubic: https://blog.cloudflare.com/quic-death-spiral-fix/ and another to allow switching to ProbeRTT to be independent of the current cycle phase, as also written in the BBR draft (v2) (https://datatracker.ietf.org/doc/draft-ietf-ccwg-bbr/02/):
```
Entry conditions: In any state other than ProbeRTT itself, if the
BBR.probe_rtt_min_delay estimate has not been updated (i.e., by
getting a lower RTT measurement) for more than ProbeRTTInterval = 5
seconds, then BBR enters ProbeRTT and reduces the BBR.cwnd_gain to
BBRProbeRTTCwndGain = 0.5.
```

First we calculate the actual idle time based on the last ack received and the last packet sent, similar to the [Cubic fix](https://blog.cloudflare.com/quic-death-spiral-fix/) :
*quiche/src/recovery/gcongestion/bbr2.rs*
```
fn on_packet_sent(
&mut self, sent_time: Instant, bytes_in_flight: usize,
packet_number: u64, bytes: usize, is_retransmissible: bool,
) {
self.idle_start = Some(cmp::max(
self.last_ack_time.unwrap_or(sent_time),
self.last_sent_time.unwrap_or(sent_time),
));

self.last_sent_time = Some(sent_time);
if bytes_in_flight == 0 && self.params.avoid_unnecessary_probe_rtt {
self.on_exit_quiescence(sent_time);
}
let network_model = self.mode.network_model_mut();

network_model.on_packet_sent(
sent_time,
bytes_in_flight,
packet_number,
bytes,
is_retransmissible,
);
}
[...]
fn on_congestion_event(
&mut self, _rtt_updated: bool, prior_in_flight: usize,
_bytes_in_flight: usize, event_time: Instant, acked_packets: &[Acked],
lost_packets: &[Lost], least_unacked: u64, rtt_stats: &RttStats,
recovery_stats: &mut RecoveryStats, last_ack_time: Option,
) {
self.last_ack_time = last_ack_time;
[...]
if congestion_event.bytes_in_flight == 0 &&
self.params.avoid_unnecessary_probe_rtt
{
let delta = event_time -
self.idle_start.unwrap_or(event_time) -
rtt_stats.latest_rtt;
if delta.as_nanos() > 0 {
self.on_enter_quiescence(event_time);
}
}
```
Secondly we propose switching to the ProbeRTT state, regardless of the current cycle phase, as this is not mandated by the draft:
*quiche/src/recovery/gcongestion/bbr2/probe_bw.rs:*
```
fn on_congestion_event(
mut self, prior_in_flight: usize, event_time: Instant, _: &[Acked],
_: &[Lost], congestion_event: &mut BBRv2CongestionEvent,
target_bytes_inflight: usize, params: &Params,
_recovery_stats: &mut RecoveryStats, cwnd: usize,
) -> Mode {
[...]

match self.cycle.phase {
CyclePhase::NotStarted => unreachable!(),
CyclePhase::Up => self.update_probe_up(
prior_in_flight,
target_bytes_inflight,
congestion_event,
params,
),
CyclePhase::Down => {
self.update_probe_down(
target_bytes_inflight,
congestion_event,
params,
);
},
CyclePhase::Cruise => self.update_probe_cruise(
target_bytes_inflight,
congestion_event,
params,
),
CyclePhase::Refill => self.update_probe_refill(
target_bytes_inflight,
congestion_event,
params,
),
}
if self.cycle.phase != CyclePhase::Down &&
self.model.maybe_expire_min_rtt(congestion_event, params)
{
switch_to_probe_rtt = true;
}
[...]
}
```
We left in the check whether the current cycle is not the Down cycle from the original implementation.

Tests and initial experiments show that this fixes the issue, as now the states are cycled through correctly.

Start by tracing quiescence handling in quiche/src/recovery/gcongestion/bbr2.rs and ProbeBW transitions in quiche/src/recovery/gcongestion/bbr2/probe_bw.rs. Reproduce the plateau behavior with the existing BBRv2 experiments, then verify that idle detection and ProbeRTT entry behave correctly and that the reported state-cycling and completion-time issue no longer occur.

Written by the indexing model from the issue text.