Refactor QUIC Bbr2's CheckBandwidthGrowth() into HasBandwidthGrowth() so when the excess queue criteria is used, we can stay in STARTUP if there was bandwidth growth that round.
Protected by quic_reloadable_flag_quic_bbr2_exit_startup_on_persistent_queue2.
PiperOrigin-RevId: 404873466
diff --git a/quic/core/congestion_control/bbr2_misc.cc b/quic/core/congestion_control/bbr2_misc.cc
index 6374e0b..9600f13 100644
--- a/quic/core/congestion_control/bbr2_misc.cc
+++ b/quic/core/congestion_control/bbr2_misc.cc
@@ -396,13 +396,10 @@
return inflight_hi_ > headroom ? inflight_hi_ - headroom : 0;
}
-Bbr2NetworkModel::BandwidthGrowth Bbr2NetworkModel::CheckBandwidthGrowth(
+bool Bbr2NetworkModel::HasBandwidthGrowth(
const Bbr2CongestionEvent& congestion_event) {
QUICHE_DCHECK(!full_bandwidth_reached_);
QUICHE_DCHECK(congestion_event.end_of_round_trip);
- if (congestion_event.last_packet_send_state.is_app_limited) {
- return APP_LIMITED;
- }
QuicBandwidth threshold =
full_bandwidth_baseline_ * Params().startup_full_bw_threshold;
@@ -413,14 +410,15 @@
<< " (Still growing) @ " << congestion_event.event_time;
full_bandwidth_baseline_ = MaxBandwidth();
rounds_without_bandwidth_growth_ = 0;
- return GROWTH;
+ return true;
}
-
++rounds_without_bandwidth_growth_;
- BandwidthGrowth return_value = NO_GROWTH;
- if (rounds_without_bandwidth_growth_ >= Params().startup_full_bw_rounds) {
+
+ // full_bandwidth_reached is only set to true when not app-limited, except
+ // when exit_startup_on_persistent_queue is true.
+ if (rounds_without_bandwidth_growth_ >= Params().startup_full_bw_rounds &&
+ !congestion_event.last_packet_send_state.is_app_limited) {
full_bandwidth_reached_ = true;
- return_value = EXIT;
}
QUIC_DVLOG(3) << " CheckBandwidthGrowth at end of round. max_bandwidth:"
<< MaxBandwidth() << ", threshold:" << threshold
@@ -428,13 +426,12 @@
<< " full_bw_reached:" << full_bandwidth_reached_ << " @ "
<< congestion_event.event_time;
- return return_value;
+ return false;
}
bool Bbr2NetworkModel::CheckPersistentQueue(
const Bbr2CongestionEvent& congestion_event, float bdp_gain) {
QUICHE_DCHECK(congestion_event.end_of_round_trip);
- QUICHE_DCHECK_NE(0u, min_bytes_in_flight_in_round_);
if (min_bytes_in_flight_in_round_ >
(bdp_gain * BDP() + QueueingThresholdExtraBytes())) {
full_bandwidth_reached_ = true;
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 6890534..8c8cfe1 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -456,22 +456,11 @@
bool IsInflightTooHigh(const Bbr2CongestionEvent& congestion_event,
int64_t max_loss_events) const;
- enum BandwidthGrowth {
- APP_LIMITED = 0,
- NO_GROWTH = 1,
- GROWTH = 2,
- EXIT = 3, // Too many rounds without bandwidth growth.
- };
-
// Check bandwidth growth in the past round. Must be called at the end of a
- // round.
- // Return APP_LIMITED if the bandwidth sample was app-limited.
- // Return GROWTH if the bandwidth grew as expected.
- // Return NO_GROWTH if the bandwidth didn't increase enough.
- // Return TOO_MANY_ROUNDS_WITH_NO_GROWTH if enough rounds have elapsed without
- // growth, also sets |full_bandwidth_reached_| to true.
- BandwidthGrowth CheckBandwidthGrowth(
- const Bbr2CongestionEvent& congestion_event);
+ // round. Returns true if there was sufficient bandwidth growth and false
+ // otherwise. If it's been too many rounds without growth, also sets
+ // |full_bandwidth_reached_| to true.
+ bool HasBandwidthGrowth(const Bbr2CongestionEvent& congestion_event);
// Returns true if the minimum bytes in flight during the round is greater
// than the BDP * |bdp_gain|.
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index b016ba1..3f1c449 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -194,7 +194,7 @@
QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr2_startup_extra_acked, 2, 2);
params_.startup_include_extra_acked = true;
}
- if (GetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue) &&
+ if (GetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue2) &&
ContainsQuicTag(connection_options, kB207)) {
params_.exit_startup_on_persistent_queue = true;
}
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index 0968bb1..333267f 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -391,7 +391,7 @@
}
TEST_F(Bbr2DefaultTopologyTest, NormalStartupB207) {
- SetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue, true);
+ SetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue2, true);
SetConnectionOption(kB207);
DefaultTopologyParams params;
CreateNetwork(params);
@@ -411,9 +411,9 @@
QuicTime::Delta::FromSeconds(5));
ASSERT_TRUE(simulator_result);
EXPECT_EQ(Bbr2Mode::DRAIN, sender_->ExportDebugState().mode);
- EXPECT_EQ(0u, sender_->ExportDebugState().round_trip_count - max_bw_round);
+ EXPECT_EQ(3u, sender_->ExportDebugState().round_trip_count - max_bw_round);
EXPECT_EQ(
- 0u,
+ 3u,
sender_->ExportDebugState().startup.round_trips_without_bandwidth_growth);
EXPECT_EQ(0u, sender_connection_stats().packets_lost);
}
@@ -512,7 +512,7 @@
}
TEST_F(Bbr2DefaultTopologyTest, SimpleTransferB207) {
- SetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue, true);
+ SetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue2, true);
SetConnectionOption(kB207);
DefaultTopologyParams params;
CreateNetwork(params);
diff --git a/quic/core/congestion_control/bbr2_startup.cc b/quic/core/congestion_control/bbr2_startup.cc
index 5821a0b..690bdb2 100644
--- a/quic/core/congestion_control/bbr2_startup.cc
+++ b/quic/core/congestion_control/bbr2_startup.cc
@@ -54,21 +54,18 @@
if (!congestion_event.end_of_round_trip) {
return Bbr2Mode::STARTUP;
}
- if (Params().exit_startup_on_persistent_queue) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_bbr2_exit_startup_on_persistent_queue);
+ bool has_bandwidth_growth = model_->HasBandwidthGrowth(congestion_event);
+ if (Params().exit_startup_on_persistent_queue && !has_bandwidth_growth) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_bbr2_exit_startup_on_persistent_queue2);
model_->CheckPersistentQueue(congestion_event, Params().startup_cwnd_gain);
}
- if (!model_->full_bandwidth_reached()) {
- // TCP BBR always exits upon excessive losses. QUIC BBRv1 does not exit
- // upon excessive losses, if enough bandwidth growth is observed.
- Bbr2NetworkModel::BandwidthGrowth bw_growth =
- model_->CheckBandwidthGrowth(congestion_event);
-
- if (Params().always_exit_startup_on_excess_loss ||
- (bw_growth == Bbr2NetworkModel::NO_GROWTH ||
- bw_growth == Bbr2NetworkModel::EXIT)) {
- CheckExcessiveLosses(congestion_event);
- }
+ // TCP BBR always exits upon excessive losses. QUIC BBRv1 does not exit
+ // upon excessive losses, if enough bandwidth growth is observed or if the
+ // sample was app limited.
+ if (Params().always_exit_startup_on_excess_loss ||
+ (!congestion_event.last_packet_send_state.is_app_limited &&
+ !has_bandwidth_growth)) {
+ CheckExcessiveLosses(congestion_event);
}
if (Params().decrease_startup_pacing_at_end_of_round) {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 1c70b08..a4884ab 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -140,7 +140,7 @@
// When true, the B204 connection option enables extra acked in STARTUP, but also adds new logic to decrease it whenever max bandwidth increases.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_startup_extra_acked, true)
// When true, the B207 connection option causes BBR2 to exit STARTUP if a persistent queue of 2*BDP has existed for the entire round.
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_exit_startup_on_persistent_queue, true)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_exit_startup_on_persistent_queue2, true)
// When true, the BBQ0 connection option causes QUIC BBR2 to add bytes_acked to probe_up_acked if the connection hasn\'t been app-limited since inflight_hi was utilized.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_add_bytes_acked_after_inflight_hi_limited, true)
// When true, the BBR4 copt sets the extra_acked window to 20 RTTs and BBR5 sets it to 40 RTTs.
