If bytes in flight has dipped below 1.25*MaxBW in the last round, do not exit PROBE_UP due to excess queue buildup. Also exits PROBE_UP even if app limited if queueing or excess loss is detected. Activated by the B202 connection option.
Protected by quic_reloadable_flag_quic_bbr2_no_probe_up_exit_if_no_queue.
PiperOrigin-RevId: 399536464
diff --git a/quic/core/congestion_control/bbr2_misc.cc b/quic/core/congestion_control/bbr2_misc.cc
index 8c9059e..b67b7df 100644
--- a/quic/core/congestion_control/bbr2_misc.cc
+++ b/quic/core/congestion_control/bbr2_misc.cc
@@ -164,6 +164,10 @@
congestion_event->last_packet_send_state.total_bytes_acked;
max_bytes_delivered_in_round_ =
std::max(max_bytes_delivered_in_round_, bytes_delivered);
+ if (min_bytes_in_flight_in_round_ == 0 ||
+ congestion_event->bytes_in_flight < min_bytes_in_flight_in_round_) {
+ min_bytes_in_flight_in_round_ = congestion_event->bytes_in_flight;
+ }
}
// |bandwidth_latest_| and |inflight_latest_| only increased within a round.
@@ -376,6 +380,7 @@
bytes_lost_in_round_ = 0;
loss_events_in_round_ = 0;
max_bytes_delivered_in_round_ = 0;
+ min_bytes_in_flight_in_round_ = 0;
}
void Bbr2NetworkModel::cap_inflight_lo(QuicByteCount cap) {
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 0161a6a..5df19d5 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -146,6 +146,7 @@
* PROBE_UP parameters.
*/
bool probe_up_includes_acks_after_cwnd_limited = false;
+ bool probe_up_dont_exit_if_no_queue_ = false;
/*
* PROBE_RTT parameters.
@@ -474,6 +475,10 @@
return max_bytes_delivered_in_round_;
}
+ QuicByteCount min_bytes_in_flight_in_round() const {
+ return min_bytes_in_flight_in_round_;
+ }
+
QuicPacketNumber end_of_app_limited_phase() const {
return bandwidth_sampler_.end_of_app_limited_phase();
}
@@ -544,6 +549,9 @@
// congestion event) was sent and acked, respectively.
QuicByteCount max_bytes_delivered_in_round_ = 0;
+ // The minimum bytes in flight during this round.
+ QuicByteCount min_bytes_in_flight_in_round_ = 0;
+
// 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/quic/core/congestion_control/bbr2_probe_bw.cc b/quic/core/congestion_control/bbr2_probe_bw.cc
index 83c2585..1c1963f 100644
--- a/quic/core/congestion_control/bbr2_probe_bw.cc
+++ b/quic/core/congestion_control/bbr2_probe_bw.cc
@@ -36,8 +36,7 @@
}
Bbr2Mode Bbr2ProbeBwMode::OnCongestionEvent(
- QuicByteCount prior_in_flight,
- QuicTime event_time,
+ QuicByteCount prior_in_flight, QuicTime event_time,
const AckedPacketVector& /*acked_packets*/,
const LostPacketVector& /*lost_packets*/,
const Bbr2CongestionEvent& congestion_event) {
@@ -187,12 +186,20 @@
<< congestion_event.last_packet_send_state.total_bytes_acked << ")";
}
}
+ // TODO(ianswett): Inflight too high is really checking for loss, not
+ // inflight.
if (model_->IsInflightTooHigh(congestion_event,
Params().probe_bw_full_loss_count)) {
if (cycle_.is_sample_from_probing) {
cycle_.is_sample_from_probing = false;
-
- if (!send_state.is_app_limited) {
+ if (!send_state.is_app_limited ||
+ Params().probe_up_dont_exit_if_no_queue_) {
+ if (send_state.is_app_limited) {
+ // If there's excess loss or a queue is building, exit even if the
+ // last sample was app limited.
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr2_no_probe_up_exit_if_no_queue,
+ 2, 2);
+ }
const QuicByteCount inflight_target =
sender_->GetTargetBytesInflight() * (1.0 - Params().beta);
if (inflight_at_send >= inflight_target) {
@@ -469,23 +476,32 @@
} else if (cycle_.rounds_in_phase > 0) {
const QuicByteCount bdp = model_->BDP();
QuicByteCount queuing_threshold_extra_bytes = 2 * kDefaultTCPMSS;
- if (Params().add_ack_height_to_queueing_threshold) {
- queuing_threshold_extra_bytes += model_->MaxAckHeight();
+ if (Params().probe_up_dont_exit_if_no_queue_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_bbr2_no_probe_up_exit_if_no_queue, 1,
+ 2);
+ is_queuing = congestion_event.end_of_round_trip &&
+ model_->min_bytes_in_flight_in_round() >
+ (bdp * Params().probe_bw_probe_inflight_gain +
+ queuing_threshold_extra_bytes);
+ } else {
+ if (Params().add_ack_height_to_queueing_threshold) {
+ queuing_threshold_extra_bytes += model_->MaxAckHeight();
+ }
+ QuicByteCount queuing_threshold =
+ (Params().probe_bw_probe_inflight_gain * bdp) +
+ queuing_threshold_extra_bytes;
+
+ is_queuing = congestion_event.bytes_in_flight >= queuing_threshold;
+
+ QUIC_DVLOG(3) << sender_
+ << " Checking if building up a queue. prior_in_flight:"
+ << prior_in_flight
+ << ", post_in_flight:" << congestion_event.bytes_in_flight
+ << ", threshold:" << queuing_threshold
+ << ", is_queuing:" << is_queuing
+ << ", max_bw:" << model_->MaxBandwidth()
+ << ", min_rtt:" << model_->MinRtt();
}
- QuicByteCount queuing_threshold =
- (Params().probe_bw_probe_inflight_gain * bdp) +
- queuing_threshold_extra_bytes;
-
- is_queuing = congestion_event.bytes_in_flight >= queuing_threshold;
-
- QUIC_DVLOG(3) << sender_
- << " Checking if building up a queue. prior_in_flight:"
- << prior_in_flight
- << ", post_in_flight:" << congestion_event.bytes_in_flight
- << ", threshold:" << queuing_threshold
- << ", is_queuing:" << is_queuing
- << ", max_bw:" << model_->MaxBandwidth()
- << ", min_rtt:" << model_->MinRtt();
}
if (is_risky || is_queuing) {
@@ -495,8 +511,7 @@
}
void Bbr2ProbeBwMode::EnterProbeDown(bool probed_too_high,
- bool stopped_risky_probe,
- QuicTime now) {
+ bool stopped_risky_probe, QuicTime now) {
QUIC_DVLOG(2) << sender_ << " Phase change: " << cycle_.phase << " ==> "
<< CyclePhase::PROBE_DOWN << " after "
<< now - cycle_.phase_start_time << ", or "
@@ -637,9 +652,7 @@
return os;
}
-const Bbr2Params& Bbr2ProbeBwMode::Params() const {
- return sender_->Params();
-}
+const Bbr2Params& Bbr2ProbeBwMode::Params() const { return sender_->Params(); }
float Bbr2ProbeBwMode::PacingGainForPhase(
Bbr2ProbeBwMode::CyclePhase phase) const {
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index d811681..582451e 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -175,6 +175,10 @@
ContainsQuicTag(connection_options, kBBQ0)) {
params_.probe_up_includes_acks_after_cwnd_limited = true;
}
+ if (GetQuicReloadableFlag(quic_bbr2_no_probe_up_exit_if_no_queue) &&
+ ContainsQuicTag(connection_options, kB202)) {
+ params_.probe_up_dont_exit_if_no_queue_ = 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 c913a10..074e56b 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -684,7 +684,7 @@
sender_->ExportDebugState().bandwidth_est, 0.6f);
EXPECT_LE(sender_loss_rate_in_packets(), 0.35);
- // Now increase the bottleneck bandwidth from 100Kbps to 1Mbps.
+ // Now increase the bottleneck bandwidth from 100Kbps to 10Mbps.
params.test_link.bandwidth = QuicBandwidth::FromKBitsPerSecond(10000);
TestLink()->set_bandwidth(params.test_link.bandwidth);
@@ -692,11 +692,83 @@
[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.
+ // Ensure at least 10% of 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 B202
+TEST_F(Bbr2DefaultTopologyTest, QUIC_SLOW_TEST(BandwidthIncreaseB202)) {
+ SetQuicReloadableFlag(quic_bbr2_no_probe_up_exit_if_no_queue, true);
+ SetConnectionOption(kB202);
+ 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.1f);
+}
+
+// Test Bbr2's reaction to a 100x bandwidth increase during a transfer with B202
+// in the presence of ACK aggregation.
+TEST_F(Bbr2DefaultTopologyTest,
+ QUIC_SLOW_TEST(BandwidthIncreaseB202Aggregation)) {
+ SetQuicReloadableFlag(quic_bbr2_no_probe_up_exit_if_no_queue, true);
+ 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.45f);
+ 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 10% of full bandwidth is discovered.
+ EXPECT_APPROX_EQ(params.test_link.bandwidth,
+ sender_->ExportDebugState().bandwidth_hi, 0.92f);
+}
+
// Test the number of losses incurred by the startup phase in a situation when
// the buffer is less than BDP.
TEST_F(Bbr2DefaultTopologyTest, PacketLossOnSmallBufferStartup) {
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 4522dfd..2eb697a 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -159,6 +159,8 @@
const QuicTag kB201 = TAG('B', '2', '0', '1'); // In PROBE_UP, check if cwnd
// limited before aggregation
// epoch, instead of ack event.
+const QuicTag kB202 = TAG('B', '2', '0', '2'); // Do not exit PROBE_UP if
+ // inflight dips below 1.25*BW.
const QuicTag kNTLP = TAG('N', 'T', 'L', 'P'); // No tail loss probe
const QuicTag k1TLP = TAG('1', 'T', 'L', 'P'); // 1 tail loss probe
const QuicTag k1RTO = TAG('1', 'R', 'T', 'O'); // Send 1 packet upon RTO
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 8c750b5..5641006 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -15,6 +15,8 @@
QUIC_FLAG(FLAGS_quic_restart_flag_quic_testonly_default_false, false)
// A testonly restart flag that will always default to true.
QUIC_FLAG(FLAGS_quic_restart_flag_quic_testonly_default_true, true)
+// If bytes in flight has dipped below 1.25*MaxBW in the last round, do not exit PROBE_UP due to excess queue buildup.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_no_probe_up_exit_if_no_queue, true)
// If true and connection option B201 is used, check if cwnd limited before aggregation epoch, instead of ack event, in PROBE_UP phase.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_check_cwnd_limited_before_aggregation_epoch, true)
// If true, GFE will explicitly configure its signature algorithm preference.
