Internal change
PiperOrigin-RevId: 398749726
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 82a4c82..0161a6a 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -143,6 +143,11 @@
float probe_bw_cwnd_gain = 2.0;
/*
+ * PROBE_UP parameters.
+ */
+ bool probe_up_includes_acks_after_cwnd_limited = false;
+
+ /*
* PROBE_RTT parameters.
*/
float probe_rtt_inflight_target_bdp_fraction = 0.5;
diff --git a/quic/core/congestion_control/bbr2_probe_bw.cc b/quic/core/congestion_control/bbr2_probe_bw.cc
index 9350f39..83c2585 100644
--- a/quic/core/congestion_control/bbr2_probe_bw.cc
+++ b/quic/core/congestion_control/bbr2_probe_bw.cc
@@ -347,6 +347,32 @@
// Not fully utilizing cwnd, so can't safely grow.
return;
}
+ } else if (Params().probe_up_includes_acks_after_cwnd_limited) {
+ QUIC_RELOADABLE_FLAG_COUNT(
+ quic_bbr2_add_bytes_acked_after_inflight_hi_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_sample_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_
@@ -503,6 +529,7 @@
Params().probe_bw_probe_max_rand_duration.ToMicroseconds()));
cycle_.probe_up_bytes = std::numeric_limits<QuicByteCount>::max();
+ cycle_.probe_up_app_limited_since_inflight_hi_limited_ = false;
cycle_.has_advanced_max_bw = false;
model_->RestartRoundEarly();
}
diff --git a/quic/core/congestion_control/bbr2_probe_bw.h b/quic/core/congestion_control/bbr2_probe_bw.h
index 45b1df7..832dfa1 100644
--- a/quic/core/congestion_control/bbr2_probe_bw.h
+++ b/quic/core/congestion_control/bbr2_probe_bw.h
@@ -118,6 +118,7 @@
uint64_t probe_up_rounds = 0;
QuicByteCount probe_up_bytes = std::numeric_limits<QuicByteCount>::max();
QuicByteCount probe_up_acked = 0;
+ bool probe_up_app_limited_since_inflight_hi_limited_ = false;
// Whether max bandwidth filter window has advanced in this cycle. It is
// advanced once per cycle.
bool has_advanced_max_bw = false;
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index cf75e31..d811681 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -170,6 +170,11 @@
ContainsQuicTag(connection_options, kBBRA)) {
model_.SetStartNewAggregationEpochAfterFullRound(true);
}
+ if (GetQuicReloadableFlag(
+ quic_bbr2_add_bytes_acked_after_inflight_hi_limited) &&
+ ContainsQuicTag(connection_options, kBBQ0)) {
+ params_.probe_up_includes_acks_after_cwnd_limited = true;
+ }
}
Limits<QuicByteCount> Bbr2Sender::GetCwndLimitsByMode() const {
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index 052e186..ca88157 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -201,16 +201,6 @@
GetUnackedMap(endpoint->connection()), kDefaultInitialCwndPackets,
GetQuicFlag(FLAGS_quic_max_congestion_window), &random_,
QuicConnectionPeer::GetStats(endpoint->connection()), old_sender);
- if (GetQuicReloadableFlag(
- quic_bbr2_check_cwnd_limited_before_aggregation_epoch)) {
- // TODO(wub): Remove this option when deprecating the flag. There are
- // dedicated tests for this option.
- SetConnectionOption(kB201, sender);
- }
- if (GetQuicReloadableFlag(
- quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
- SetConnectionOption(kBBRA, sender);
- }
QuicConnectionPeer::SetSendAlgorithm(endpoint->connection(), sender);
endpoint->RecordTrace();
return sender;
@@ -451,6 +441,8 @@
}
TEST_F(Bbr2DefaultTopologyTest, SimpleTransferB201) {
+ SetQuicReloadableFlag(quic_bbr2_check_cwnd_limited_before_aggregation_epoch,
+ true);
SetConnectionOption(kB201);
DefaultTopologyParams params;
CreateNetwork(params);
@@ -522,6 +514,8 @@
}
TEST_F(Bbr2DefaultTopologyTest, SimpleTransfer2RTTAggregationBytesB201) {
+ SetQuicReloadableFlag(quic_bbr2_check_cwnd_limited_before_aggregation_epoch,
+ true);
SetConnectionOption(kB201);
DefaultTopologyParams params;
CreateNetwork(params);
@@ -626,6 +620,81 @@
sender_->ExportDebugState().bandwidth_hi, 0.02f);
}
+// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with BBQ0
+TEST_F(Bbr2DefaultTopologyTest, QUIC_SLOW_TEST(BandwidthIncreaseBBQ0)) {
+ SetQuicReloadableFlag(quic_bbr2_add_bytes_acked_after_inflight_hi_limited,
+ true);
+ SetConnectionOption(kBBQ0);
+ 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 BBQ0
+// in the presence of ACK aggregation.
+TEST_F(Bbr2DefaultTopologyTest,
+ QUIC_SLOW_TEST(BandwidthIncreaseBBQ0Aggregation)) {
+ SetQuicReloadableFlag(quic_bbr2_add_bytes_acked_after_inflight_hi_limited,
+ true);
+ SetConnectionOption(kBBQ0);
+ 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.
+ // TODO(ianswett) Make these bound tighter once overestimation is reduced.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_est, 0.6f);
+ EXPECT_LE(sender_loss_rate_in_packets(), 0.35);
+
+ // Now increase the bottleneck bandwidth from 100Kbps to 1Mbps.
+ 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 15% of full bandwidth is discovered.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_hi, 0.90f);
+}
+
// Test the number of losses incurred by the startup phase in a situation when
// the buffer is less than BDP.
TEST_F(Bbr2DefaultTopologyTest, PacketLossOnSmallBufferStartup) {
@@ -1443,6 +1512,12 @@
kDefaultInitialCwndPackets,
GetQuicFlag(FLAGS_quic_max_congestion_window), &random_,
QuicConnectionPeer::GetStats(endpoint->connection()), nullptr);
+ if (GetQuicReloadableFlag(
+ quic_bbr_start_new_aggregation_epoch_after_a_full_round)) {
+ // TODO(ianswett): Add dedicated tests for this option until it becomes
+ // the default behavior.
+ SetConnectionOption(sender, kBBRA);
+ }
QuicConnectionPeer::SetSendAlgorithm(endpoint->connection(), sender);
endpoint->RecordTrace();
return sender;
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 6de2419..4522dfd 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -119,6 +119,8 @@
const QuicTag kBBQ8 = TAG('B', 'B', 'Q', '8'); // Reduce bw_lo by
// bw_lo * bytes_lost/inflight
const QuicTag kBBQ9 = TAG('B', 'B', 'Q', '9'); // Reduce bw_lo by
+const QuicTag kBBQ0 = TAG('B', 'B', 'Q', '0'); // Increase bytes_acked in
+ // PROBE_UP when app limited.
// bw_lo * bytes_lost/cwnd
const QuicTag kRENO = TAG('R', 'E', 'N', 'O'); // Reno Congestion Control
const QuicTag kTPCC = TAG('P', 'C', 'C', '\0'); // Performance-Oriented
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index f8fab11..f0a05a6 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -129,6 +129,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_randomize_transport_parameter_order, true)
// When true, set the initial congestion control window from connection options in QuicSentPacketManager rather than TcpCubicSenderBytes.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_unified_iw_options, false)
+// 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)
#endif
