Revert the meaning of QUIC connection option 'B2NE':
- Without the option, exit STARTUP on loss only if bandwidth is below threshold.
- With the option, always exit STARTUP on loss, even if bandwidth exceeds threshold.
PiperOrigin-RevId: 370097972
Change-Id: Ie789842e7e389328046828fc07fd603fbb7e5700
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 8f5d418..14a0882 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -92,7 +92,7 @@
// If true, always exit STARTUP on loss, even if bandwidth exceeds threshold.
// If false, exit STARTUP on loss only if bandwidth is below threshold.
- bool always_exit_startup_on_excess_loss = true;
+ bool always_exit_startup_on_excess_loss = false;
/*
* DRAIN parameters.
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index d0f6127..b69a795 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -132,7 +132,7 @@
params_.ignore_inflight_lo = true;
}
if (ContainsQuicTag(connection_options, kB2NE)) {
- params_.always_exit_startup_on_excess_loss = false;
+ params_.always_exit_startup_on_excess_loss = true;
}
if (ContainsQuicTag(connection_options, kB2SL)) {
params_.startup_loss_exit_use_max_delivered_for_inflight_hi = false;
diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index e26760e..99acf2c 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -417,25 +417,6 @@
EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->smoothed_rtt(), 1.0f);
}
-TEST_F(Bbr2DefaultTopologyTest, SimpleTransferB2NE) {
- SetConnectionOption(kB2NE);
- DefaultTopologyParams params;
- CreateNetwork(params);
-
- // Transfer 12MB.
- DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35));
- EXPECT_TRUE(Bbr2ModeIsOneOf({Bbr2Mode::PROBE_BW, Bbr2Mode::PROBE_RTT}));
-
- EXPECT_APPROX_EQ(params.BottleneckBandwidth(),
- sender_->ExportDebugState().bandwidth_hi, 0.01f);
-
- EXPECT_LE(sender_loss_rate_in_packets(), 0.05);
- // The margin here is high, because the aggregation greatly increases
- // smoothed rtt.
- EXPECT_GE(params.RTT() * 4, rtt_stats()->smoothed_rtt());
- EXPECT_APPROX_EQ(params.RTT(), rtt_stats()->min_rtt(), 0.2f);
-}
-
TEST_F(Bbr2DefaultTopologyTest, SimpleTransferB2RC) {
SetConnectionOption(kB2RC);
DefaultTopologyParams params;
@@ -859,6 +840,42 @@
EXPECT_APPROX_EQ(sender_->ExportDebugState().inflight_hi, params.BDP(), 0.1f);
}
+// Verifies that in STARTUP, if we exceed loss threshold in a round, we exit
+// STARTUP at the end of the round even if there's enough bandwidth growth.
+TEST_F(Bbr2DefaultTopologyTest, ExitStartupDueToLossB2NE) {
+ // Set up flags such that any loss will be considered "too high".
+ SetQuicFlag(FLAGS_quic_bbr2_default_startup_full_loss_count, 0);
+ SetQuicFlag(FLAGS_quic_bbr2_default_loss_threshold, 0.0);
+
+ sender_ = SetupBbr2Sender(&sender_endpoint_, /*old_sender=*/nullptr);
+
+ SetConnectionOption(kB2NE);
+ DefaultTopologyParams params;
+ params.switch_queue_capacity_in_bdp = 0.5;
+ CreateNetwork(params);
+
+ // Run until the full bandwidth is reached and check how many rounds it was.
+ sender_endpoint_.AddBytesToTransfer(12 * 1024 * 1024);
+ QuicRoundTripCount max_bw_round = 0;
+ QuicBandwidth max_bw(QuicBandwidth::Zero());
+ bool simulator_result = simulator_.RunUntilOrTimeout(
+ [this, &max_bw, &max_bw_round]() {
+ if (max_bw < sender_->ExportDebugState().bandwidth_hi) {
+ max_bw = sender_->ExportDebugState().bandwidth_hi;
+ max_bw_round = sender_->ExportDebugState().round_trip_count;
+ }
+ return sender_->ExportDebugState().startup.full_bandwidth_reached;
+ },
+ QuicTime::Delta::FromSeconds(5));
+ ASSERT_TRUE(simulator_result);
+ EXPECT_EQ(Bbr2Mode::DRAIN, sender_->ExportDebugState().mode);
+ EXPECT_EQ(sender_->ExportDebugState().round_trip_count, max_bw_round);
+ EXPECT_EQ(
+ 0u,
+ sender_->ExportDebugState().startup.round_trips_without_bandwidth_growth);
+ EXPECT_NE(0u, sender_connection_stats().packets_lost);
+}
+
TEST_F(Bbr2DefaultTopologyTest, SenderPoliced) {
DefaultTopologyParams params;
params.sender_policer_params = TrafficPolicerParams();
diff --git a/quic/core/congestion_control/bbr2_startup.cc b/quic/core/congestion_control/bbr2_startup.cc
index 5a9a532..1bd89c6 100644
--- a/quic/core/congestion_control/bbr2_startup.cc
+++ b/quic/core/congestion_control/bbr2_startup.cc
@@ -112,8 +112,9 @@
new_inflight_hi = model_->max_bytes_delivered_in_round();
}
}
- QUIC_DVLOG(3) << sender_ << " Exiting STARTUP due to loss. inflight_hi:"
- << new_inflight_hi;
+ QUIC_DVLOG(3) << sender_ << " Exiting STARTUP due to loss at round "
+ << model_->RoundTripCount()
+ << ". inflight_hi:" << new_inflight_hi;
// TODO(ianswett): Add a shared method to set inflight_hi in the model.
model_->set_inflight_hi(new_inflight_hi);
model_->set_full_bandwidth_reached();
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index e08d90b..8a86255 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -127,9 +127,10 @@
const QuicTag kB2ON = TAG('B', '2', 'O', 'N'); // Enable BBRv2
const QuicTag kB2NA = TAG('B', '2', 'N', 'A'); // For BBRv2, do not add ack
// height to queueing threshold
-const QuicTag kB2NE = TAG('B', '2', 'N', 'E'); // For BBRv2, do not exit
- // STARTUP if there's enough
- // bandwidth growth
+const QuicTag kB2NE = TAG('B', '2', 'N', 'E'); // For BBRv2, always exit
+ // STARTUP on loss, even if
+ // bandwidth growth exceeds
+ // threshold.
const QuicTag kB2RP = TAG('B', '2', 'R', 'P'); // For BBRv2, run PROBE_RTT on
// the regular schedule
const QuicTag kB2CL = TAG('B', '2', 'C', 'L'); // For BBRv2, allow PROBE_BW
