commit 77e75d5dff24dacb6da4bac9085c13e0a3d3e148
author ianswett <ianswett@google.com> Mon Oct 25 08:25:00 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Oct 25 08:26:04 2021 -0700
tree 10e61f4e00471c0bf0b131c16bd7230628a94c24
parent 355a6d8738aaec62c73fb455a13960233404190b

Modify the B207 connection option subtract 2 packets from the queueing threshold instead of adding 2 in STARTUP.  Now exits STARTUP after 1 round of no bandwidth growth, as intended.

Protected by quic_reloadable_flag_quic_exit_startup_on_peristent_queue2.

PiperOrigin-RevId: 405405541

quic/core/congestion_control/bbr2_misc.cc

diff --git a/quic/core/congestion_control/bbr2_misc.cc b/quic/core/congestion_control/bbr2_misc.cc
index 9600f13..8472890 100644
--- a/quic/core/congestion_control/bbr2_misc.cc
+++ b/quic/core/congestion_control/bbr2_misc.cc
@@ -162,6 +162,8 @@
         congestion_event->last_packet_send_state.total_bytes_acked;
     max_bytes_delivered_in_round_ =
         std::max(max_bytes_delivered_in_round_, bytes_delivered);
+    // TODO(ianswett) Consider treating any bytes lost as decreasing inflight,
+    // because it's a sign of overutilization, not underutilization.
     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;
@@ -432,8 +434,17 @@
 bool Bbr2NetworkModel::CheckPersistentQueue(
     const Bbr2CongestionEvent& congestion_event, float bdp_gain) {
   QUICHE_DCHECK(congestion_event.end_of_round_trip);
-  if (min_bytes_in_flight_in_round_ >
-      (bdp_gain * BDP() + QueueingThresholdExtraBytes())) {
+  QuicByteCount target = bdp_gain * BDP();
+  if (bdp_gain >= 2) {
+    // Use a more conservative threshold for STARTUP because CWND gain is 2.
+    if (target <= QueueingThresholdExtraBytes()) {
+      return false;
+    }
+    target -= QueueingThresholdExtraBytes();
+  } else {
+    target += QueueingThresholdExtraBytes();
+  }
+  if (min_bytes_in_flight_in_round_ > target) {
     full_bandwidth_reached_ = true;
     return true;
   }

quic/core/congestion_control/bbr2_misc.h

diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 8c8cfe1..5716ce9 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -401,6 +401,7 @@
     return bandwidth_sampler_.max_ack_height();
   }
 
+  // 2 packets.  Used to indicate the typical number of bytes ACKed at once.
   QuicByteCount QueueingThresholdExtraBytes() const {
     return 2 * kDefaultTCPMSS;
   }

quic/core/congestion_control/bbr2_simulator_test.cc

diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index 333267f..1ad799b 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -387,6 +387,8 @@
       3u,
       sender_->ExportDebugState().startup.round_trips_without_bandwidth_growth);
   EXPECT_EQ(0u, sender_connection_stats().packets_lost);
+  EXPECT_APPROX_EQ(params.BottleneckBandwidth(),
+                   sender_->ExportDebugState().bandwidth_hi, 0.01f);
   EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited);
 }
 
@@ -411,10 +413,45 @@
       QuicTime::Delta::FromSeconds(5));
   ASSERT_TRUE(simulator_result);
   EXPECT_EQ(Bbr2Mode::DRAIN, sender_->ExportDebugState().mode);
-  EXPECT_EQ(3u, sender_->ExportDebugState().round_trip_count - max_bw_round);
+  EXPECT_EQ(1u, sender_->ExportDebugState().round_trip_count - max_bw_round);
   EXPECT_EQ(
-      3u,
+      1u,
       sender_->ExportDebugState().startup.round_trips_without_bandwidth_growth);
+  EXPECT_APPROX_EQ(params.BottleneckBandwidth(),
+                   sender_->ExportDebugState().bandwidth_hi, 0.01f);
+  EXPECT_EQ(0u, sender_connection_stats().packets_lost);
+}
+
+// Add extra_acked to CWND in STARTUP and exit STARTUP on a persistent queue.
+TEST_F(Bbr2DefaultTopologyTest, NormalStartupB207andB205) {
+  SetQuicReloadableFlag(quic_bbr2_startup_extra_acked, true);
+  SetQuicReloadableFlag(quic_bbr2_exit_startup_on_persistent_queue2, true);
+  SetConnectionOption(kB205);
+  SetConnectionOption(kB207);
+  DefaultTopologyParams params;
+  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(1u, sender_->ExportDebugState().round_trip_count - max_bw_round);
+  EXPECT_EQ(
+      2u,
+      sender_->ExportDebugState().startup.round_trips_without_bandwidth_growth);
+  EXPECT_APPROX_EQ(params.BottleneckBandwidth(),
+                   sender_->ExportDebugState().bandwidth_hi, 0.01f);
   EXPECT_EQ(0u, sender_connection_stats().packets_lost);
 }