Add a new inflight_hi_limited_in_round to QUIC BBRv2 to replace the more complex logic about when to raise inflight_hi there is today. This is more in line with BBRv2's intended inflight_hi growth, but avoids a number of problems around being app-limited or aggregation. Activated by the BBHI connection option.
Protected by gfe2_reloadable_quic_bbr2_simplify_inflight_hi.
PiperOrigin-RevId: 490084354
diff --git a/quiche/quic/core/congestion_control/bbr2_misc.cc b/quiche/quic/core/congestion_control/bbr2_misc.cc
index 6cc2852..f85c602 100644
--- a/quiche/quic/core/congestion_control/bbr2_misc.cc
+++ b/quiche/quic/core/congestion_control/bbr2_misc.cc
@@ -81,6 +81,9 @@
if (bytes_in_flight < min_bytes_in_flight_in_round_) {
min_bytes_in_flight_in_round_ = bytes_in_flight;
}
+ if (bytes_in_flight + bytes >= inflight_hi_) {
+ inflight_hi_limited_in_round_ = true;
+ }
round_trip_counter_.OnPacketSent(packet_number);
bandwidth_sampler_.OnPacketSent(sent_time, packet_number, bytes,
@@ -383,6 +386,7 @@
loss_events_in_round_ = 0;
max_bytes_delivered_in_round_ = 0;
min_bytes_in_flight_in_round_ = std::numeric_limits<uint64_t>::max();
+ inflight_hi_limited_in_round_ = false;
}
void Bbr2NetworkModel::cap_inflight_lo(QuicByteCount cap) {
diff --git a/quiche/quic/core/congestion_control/bbr2_misc.h b/quiche/quic/core/congestion_control/bbr2_misc.h
index 39d4747..579a7b5 100644
--- a/quiche/quic/core/congestion_control/bbr2_misc.h
+++ b/quiche/quic/core/congestion_control/bbr2_misc.h
@@ -157,6 +157,7 @@
bool probe_up_includes_acks_after_cwnd_limited = false;
bool probe_up_dont_exit_if_no_queue_ = false;
bool probe_up_ignore_inflight_hi = true;
+ bool probe_up_simplify_inflight_hi = false;
/*
* PROBE_RTT parameters.
@@ -489,6 +490,10 @@
return min_bytes_in_flight_in_round_;
}
+ bool inflight_hi_limited_in_round() const {
+ return inflight_hi_limited_in_round_;
+ }
+
QuicPacketNumber end_of_app_limited_phase() const {
return bandwidth_sampler_.end_of_app_limited_phase();
}
@@ -563,6 +568,9 @@
QuicByteCount min_bytes_in_flight_in_round_ =
std::numeric_limits<uint64_t>::max();
+ // True if sending was limited by inflight_hi anytime in the current round.
+ bool inflight_hi_limited_in_round_ = false;
+
// Max bandwidth in the current round. Updated once per congestion event.
QuicBandwidth bandwidth_latest_ = QuicBandwidth::Zero();
// Max bandwidth of recent rounds. Updated once per round.
diff --git a/quiche/quic/core/congestion_control/bbr2_probe_bw.cc b/quiche/quic/core/congestion_control/bbr2_probe_bw.cc
index 88e87b6..9b9ab3c 100644
--- a/quiche/quic/core/congestion_control/bbr2_probe_bw.cc
+++ b/quiche/quic/core/congestion_control/bbr2_probe_bw.cc
@@ -354,20 +354,52 @@
// the number of bytes delivered in a round is larger inflight_hi.
return;
}
- if (Params().probe_bw_check_cwnd_limited_before_aggregation_epoch) {
- if (!model_->cwnd_limited_before_aggregation_epoch()) {
- QUIC_DVLOG(3) << sender_
- << " Raising inflight_hi early return: Not cwnd limited "
- "before aggregation epoch.";
- // Not fully utilizing cwnd, so can't safely grow.
+ if (Params().probe_up_simplify_inflight_hi) {
+ // Raise inflight_hi exponentially if it was utilized this round.
+ cycle_.probe_up_acked += congestion_event.bytes_acked;
+ if (!congestion_event.end_of_round_trip) {
return;
}
- } else if (Params().probe_up_includes_acks_after_cwnd_limited) {
- // Don't continue adding bytes to probe_up_acked if the sender was not
- // app-limited after being inflight_hi limited at least once.
- if (!cycle_.probe_up_app_limited_since_inflight_hi_limited_ ||
- congestion_event.last_packet_send_state.is_app_limited) {
- cycle_.probe_up_app_limited_since_inflight_hi_limited_ = false;
+ if (!model_->inflight_hi_limited_in_round() ||
+ model_->loss_events_in_round() > 0) {
+ cycle_.probe_up_acked = 0;
+ return;
+ }
+ } else {
+ if (Params().probe_bw_check_cwnd_limited_before_aggregation_epoch) {
+ if (!model_->cwnd_limited_before_aggregation_epoch()) {
+ QUIC_DVLOG(3) << sender_
+ << " Raising inflight_hi early return: Not cwnd limited "
+ "before aggregation epoch.";
+ // Not fully utilizing cwnd, so can't safely grow.
+ return;
+ }
+ } else if (Params().probe_up_includes_acks_after_cwnd_limited) {
+ // Don't continue adding bytes to probe_up_acked if the sender was not
+ // app-limited after being inflight_hi limited at least once.
+ if (!cycle_.probe_up_app_limited_since_inflight_hi_limited_ ||
+ congestion_event.last_packet_send_state.is_app_limited) {
+ cycle_.probe_up_app_limited_since_inflight_hi_limited_ = false;
+ if (congestion_event.prior_bytes_in_flight <
+ congestion_event.prior_cwnd) {
+ QUIC_DVLOG(3)
+ << sender_
+ << " Raising inflight_hi early return: Not cwnd limited.";
+ // Not fully utilizing cwnd, so can't safely grow.
+ return;
+ }
+
+ if (congestion_event.prior_cwnd < model_->inflight_hi()) {
+ QUIC_DVLOG(3) << sender_
+ << " Raising inflight_hi early return: inflight_hi not "
+ "fully used.";
+ // Not fully using inflight_hi, so don't grow it.
+ return;
+ }
+ }
+ // Start a new period of adding bytes_acked, because inflight_hi limited.
+ cycle_.probe_up_app_limited_since_inflight_hi_limited_ = true;
+ } else {
if (congestion_event.prior_bytes_in_flight <
congestion_event.prior_cwnd) {
QUIC_DVLOG(3) << sender_
@@ -375,36 +407,21 @@
// Not fully utilizing cwnd, so can't safely grow.
return;
}
-
- if (congestion_event.prior_cwnd < model_->inflight_hi()) {
- QUIC_DVLOG(3)
- << sender_
- << " Raising inflight_hi early return: inflight_hi not fully used.";
- // Not fully using inflight_hi, so don't grow it.
- return;
- }
}
- // Start a new period of adding bytes_acked, because inflight_hi limited.
- cycle_.probe_up_app_limited_since_inflight_hi_limited_ = true;
- } else {
- if (congestion_event.prior_bytes_in_flight < congestion_event.prior_cwnd) {
- QUIC_DVLOG(3) << sender_
- << " Raising inflight_hi early return: Not cwnd limited.";
- // Not fully utilizing cwnd, so can't safely grow.
+
+ if (congestion_event.prior_cwnd < model_->inflight_hi()) {
+ QUIC_DVLOG(3)
+ << sender_
+ << " Raising inflight_hi early return: inflight_hi not fully used.";
+ // Not fully using inflight_hi, so don't grow it.
return;
}
+
+ // Increase inflight_hi by the number of probe_up_bytes within
+ // probe_up_acked.
+ cycle_.probe_up_acked += congestion_event.bytes_acked;
}
- if (congestion_event.prior_cwnd < model_->inflight_hi()) {
- QUIC_DVLOG(3)
- << sender_
- << " Raising inflight_hi early return: inflight_hi not fully used.";
- // Not fully using inflight_hi, so don't grow it.
- return;
- }
-
- // Increase inflight_hi by the number of probe_up_bytes within probe_up_acked.
- cycle_.probe_up_acked += congestion_event.bytes_acked;
if (cycle_.probe_up_acked >= cycle_.probe_up_bytes) {
uint64_t delta = cycle_.probe_up_acked / cycle_.probe_up_bytes;
cycle_.probe_up_acked -= delta * cycle_.probe_up_bytes;
diff --git a/quiche/quic/core/congestion_control/bbr2_sender.cc b/quiche/quic/core/congestion_control/bbr2_sender.cc
index 33fe2e6..86e00ed 100644
--- a/quiche/quic/core/congestion_control/bbr2_sender.cc
+++ b/quiche/quic/core/congestion_control/bbr2_sender.cc
@@ -203,6 +203,14 @@
// Derived constant to ensure fairness.
params_.probe_bw_probe_down_pacing_gain = 0.91;
}
+ if (GetQuicReloadableFlag(quic_bbr2_simplify_inflight_hi) &&
+ ContainsQuicTag(connection_options, kBBHI)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_bbr2_simplify_inflight_hi);
+ params_.probe_up_simplify_inflight_hi = true;
+ // Simplify inflight_hi is intended as an alternative to ignoring it,
+ // so ensure we're not ignoring it.
+ params_.probe_up_ignore_inflight_hi = false;
+ }
}
Limits<QuicByteCount> Bbr2Sender::GetCwndLimitsByMode() const {
diff --git a/quiche/quic/core/congestion_control/bbr2_simulator_test.cc b/quiche/quic/core/congestion_control/bbr2_simulator_test.cc
index d1cf602..08237bb 100644
--- a/quiche/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quiche/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -989,6 +989,120 @@
sender_->ExportDebugState().bandwidth_hi, 0.91f);
}
+// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with BBHI
+TEST_F(Bbr2DefaultTopologyTest, QUIC_SLOW_TEST(BandwidthIncreaseBBHI)) {
+ SetQuicReloadableFlag(quic_bbr2_simplify_inflight_hi, true);
+ SetConnectionOption(kBBHI);
+ DefaultTopologyParams params;
+ params.local_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(15000);
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(100);
+ CreateNetwork(params);
+
+ sender_endpoint_.AddBytesToTransfer(10 * 1024 * 1024);
+
+ simulator_.RunFor(QuicTime::Delta::FromSeconds(15));
+ EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT}));
+ QUIC_LOG(INFO) << "Bandwidth increasing at time " << SimulatedNow();
+
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_est, 0.1f);
+ EXPECT_LE(sender_loss_rate_in_packets(), 0.30);
+
+ // Now increase the bottleneck bandwidth from 100Kbps to 10Mbps.
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(10000);
+ TestLink()->set_bandwidth(params.test_link.bandwidth);
+
+ bool simulator_result = simulator_.RunUntilOrTimeout(
+ [this]() { return sender_endpoint_.bytes_to_transfer() == 0; },
+ QuicTime::Delta::FromSeconds(50));
+ EXPECT_TRUE(simulator_result);
+ // Ensure the full bandwidth is discovered.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_hi, 0.02f);
+}
+
+// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with BBHI
+// in the presence of ACK aggregation.
+TEST_F(Bbr2DefaultTopologyTest,
+ QUIC_SLOW_TEST(BandwidthIncreaseBBHIAggregation)) {
+ SetQuicReloadableFlag(quic_bbr2_simplify_inflight_hi, true);
+ SetConnectionOption(kBBHI);
+ DefaultTopologyParams params;
+ params.local_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(15000);
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(100);
+ CreateNetwork(params);
+
+ // 2 RTTs of aggregation, with a max of 10kb.
+ EnableAggregation(10 * 1024, 2 * params.RTT());
+
+ // Reduce the payload to 2MB because 10MB takes too long.
+ sender_endpoint_.AddBytesToTransfer(2 * 1024 * 1024);
+
+ simulator_.RunFor(QuicTime::Delta::FromSeconds(15));
+ EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT}));
+ QUIC_LOG(INFO) << "Bandwidth increasing at time " << SimulatedNow();
+
+ // This is much farther off when aggregation is present,
+ // Ideally BSAO or another option would fix this.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_est, 0.60f);
+ EXPECT_LE(sender_loss_rate_in_packets(), 0.35);
+
+ // Now increase the bottleneck bandwidth from 100Kbps to 10Mbps.
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(10000);
+ TestLink()->set_bandwidth(params.test_link.bandwidth);
+
+ bool simulator_result = simulator_.RunUntilOrTimeout(
+ [this]() { return sender_endpoint_.bytes_to_transfer() == 0; },
+ QuicTime::Delta::FromSeconds(50));
+ EXPECT_TRUE(simulator_result);
+ // Ensure the full bandwidth is discovered.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_hi, 0.90f);
+}
+
+// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with BBHI
+// and B202, which changes the exit criteria to be based on
+// min_bytes_in_flight_in_round, in the presence of ACK aggregation.
+TEST_F(Bbr2DefaultTopologyTest,
+ QUIC_SLOW_TEST(BandwidthIncreaseBBHI_B202Aggregation)) {
+ SetQuicReloadableFlag(quic_bbr2_simplify_inflight_hi, true);
+ SetConnectionOption(kBBHI);
+ SetConnectionOption(kB202);
+ DefaultTopologyParams params;
+ params.local_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(15000);
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(100);
+ CreateNetwork(params);
+
+ // 2 RTTs of aggregation, with a max of 10kb.
+ EnableAggregation(10 * 1024, 2 * params.RTT());
+
+ // Reduce the payload to 2MB because 10MB takes too long.
+ sender_endpoint_.AddBytesToTransfer(2 * 1024 * 1024);
+
+ simulator_.RunFor(QuicTime::Delta::FromSeconds(15));
+ EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT}));
+ QUIC_LOG(INFO) << "Bandwidth increasing at time " << SimulatedNow();
+
+ // This is much farther off when aggregation is present,
+ // Ideally BSAO or another option would fix this.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_est, 0.60f);
+ EXPECT_LE(sender_loss_rate_in_packets(), 0.35);
+
+ // Now increase the bottleneck bandwidth from 100Kbps to 10Mbps.
+ params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(10000);
+ TestLink()->set_bandwidth(params.test_link.bandwidth);
+
+ bool simulator_result = simulator_.RunUntilOrTimeout(
+ [this]() { return sender_endpoint_.bytes_to_transfer() == 0; },
+ QuicTime::Delta::FromSeconds(50));
+ EXPECT_TRUE(simulator_result);
+ // Ensure at least 18% of the bandwidth is discovered.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_hi, 0.85f);
+}
+
// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with B204
TEST_F(Bbr2DefaultTopologyTest, QUIC_SLOW_TEST(BandwidthIncreaseB204)) {
SetConnectionOption(kB204);
diff --git a/quiche/quic/core/crypto/crypto_protocol.h b/quiche/quic/core/crypto/crypto_protocol.h
index d72183c..bc9556d 100644
--- a/quiche/quic/core/crypto/crypto_protocol.h
+++ b/quiche/quic/core/crypto/crypto_protocol.h
@@ -127,6 +127,9 @@
const QuicTag kBBQ0 = TAG('B', 'B', 'Q', '0'); // Increase bytes_acked in
// PROBE_UP when app limited.
const QuicTag kBBPD = TAG('B', 'B', 'P', 'D'); // Use 0.91 PROBE_DOWN gain.
+const QuicTag kBBHI = TAG('B', 'B', 'H', 'I'); // Increase inflight_hi in
+ // PROBE_UP if ever inflight_hi
+ // limited in round
const QuicTag kRENO = TAG('R', 'E', 'N', 'O'); // Reno Congestion Control
const QuicTag kTPCC = TAG('P', 'C', 'C', '\0'); // Performance-Oriented
// Congestion Control
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 7f855cb..d88f628 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -95,6 +95,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr, false)
// When true, support draft-ietf-quic-v2-01
QUIC_FLAG(quic_reloadable_flag_quic_enable_version_2_draft_01, false)
+// When true, the BBHI copt causes QUIC BBRv2 to use a simpler algorithm for raising inflight_hi in PROBE_UP.
+QUIC_FLAG(quic_reloadable_flag_quic_bbr2_simplify_inflight_hi, true)
// When true, the BBR4 copt sets the extra_acked window to 20 RTTs and BBR5 sets it to 40 RTTs.
QUIC_FLAG(quic_reloadable_flag_quic_bbr2_extra_acked_window, true)
