gfe-relnote: (n/a) Ensure BandwidthSampler.max_ack_height_tracker_.aggregation_epoch_start_time_ is initialized before Update(). Protected by --gfe2_reloadable_flag_quic_track_ack_height_in_bandwidth_sampler2, which replaces the old v1 flag.

This fixes b/139652317. Without the initialization, (ack_time - aggregation_epoch_start_time_) at http://shortn/_NUjGL3bG9a can be a very large number, which cause signed integer overflow when multiplied with bandwidth.

PiperOrigin-RevId: 264208119
Change-Id: Ibc6e5c81127f155d72f573781c81bc07db0a33f8

quic/core/congestion_control/bandwidth_sampler.cc

diff --git a/quic/core/congestion_control/bandwidth_sampler.cc b/quic/core/congestion_control/bandwidth_sampler.cc
index 37f678d..1ee1f9d 100644
--- a/quic/core/congestion_control/bandwidth_sampler.cc
+++ b/quic/core/congestion_control/bandwidth_sampler.cc
@@ -18,6 +18,12 @@
                                           QuicRoundTripCount round_trip_count,
                                           QuicTime ack_time,
                                           QuicByteCount bytes_acked) {
+  if (aggregation_epoch_start_time_ == QuicTime::Zero()) {
+    aggregation_epoch_bytes_ = bytes_acked;
+    aggregation_epoch_start_time_ = ack_time;
+    return 0;
+  }
+
   // Compute how many bytes are expected to be delivered, assuming max bandwidth
   // is correct.
   QuicByteCount expected_bytes_acked =
@@ -54,7 +60,13 @@
       max_tracked_packets_(GetQuicFlag(FLAGS_quic_max_tracked_packet_count)),
       unacked_packet_map_(unacked_packet_map),
       max_ack_height_tracker_(max_height_tracker_window_length),
-      total_bytes_acked_after_last_ack_event_(0) {}
+      total_bytes_acked_after_last_ack_event_(0),
+      quic_track_ack_height_in_bandwidth_sampler_(
+          GetQuicReloadableFlag(quic_track_ack_height_in_bandwidth_sampler2)) {
+  if (quic_track_ack_height_in_bandwidth_sampler_) {
+    QUIC_RELOADABLE_FLAG_COUNT(quic_track_ack_height_in_bandwidth_sampler2);
+  }
+}
 
 BandwidthSampler::~BandwidthSampler() {}
 

quic/core/congestion_control/bandwidth_sampler.h

diff --git a/quic/core/congestion_control/bandwidth_sampler.h b/quic/core/congestion_control/bandwidth_sampler.h
index 93ec351..8b279c6 100644
--- a/quic/core/congestion_control/bandwidth_sampler.h
+++ b/quic/core/congestion_control/bandwidth_sampler.h
@@ -78,6 +78,8 @@
       : bandwidth(QuicBandwidth::Zero()), rtt(QuicTime::Delta::Zero()) {}
 };
 
+// MaxAckHeightTracker is part of the BandwidthSampler. It is called after every
+// ack event to keep track the degree of ack aggregation(a.k.a "ack height").
 class QUIC_EXPORT_PRIVATE MaxAckHeightTracker {
  public:
   explicit MaxAckHeightTracker(QuicRoundTripCount initial_filter_window)
@@ -280,6 +282,10 @@
     max_ack_height_tracker_.Reset(new_height, new_time);
   }
 
+  bool quic_track_ack_height_in_bandwidth_sampler() const {
+    return quic_track_ack_height_in_bandwidth_sampler_;
+  }
+
  private:
   friend class test::BandwidthSamplerPeer;
 
@@ -393,6 +399,9 @@
 
   MaxAckHeightTracker max_ack_height_tracker_;
   QuicByteCount total_bytes_acked_after_last_ack_event_;
+
+  // Latched value of --quic_track_ack_height_in_bandwidth_sampler2.
+  const bool quic_track_ack_height_in_bandwidth_sampler_;
 };
 
 }  // namespace quic

quic/core/congestion_control/bbr_sender.cc

diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index efae7c5..ae1432a 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc
@@ -137,12 +137,7 @@
       probe_rtt_skipped_if_similar_rtt_(false),
       probe_rtt_disabled_if_app_limited_(false),
       app_limited_since_last_probe_rtt_(false),
-      min_rtt_since_last_probe_rtt_(QuicTime::Delta::Infinite()),
-      quic_track_ack_height_in_bandwidth_sampler_(
-          GetQuicReloadableFlag(quic_track_ack_height_in_bandwidth_sampler)) {
-  if (quic_track_ack_height_in_bandwidth_sampler_) {
-    QUIC_RELOADABLE_FLAG_COUNT(quic_track_ack_height_in_bandwidth_sampler);
-  }
+      min_rtt_since_last_probe_rtt_(QuicTime::Delta::Infinite()) {
   if (stats_) {
     stats_->slowstart_count = 0;
     stats_->slowstart_start_time = QuicTime::Zero();
@@ -180,8 +175,10 @@
     exiting_quiescence_ = true;
   }
 
-  if (!aggregation_epoch_start_time_.IsInitialized()) {
-    aggregation_epoch_start_time_ = sent_time;
+  if (!sampler_.quic_track_ack_height_in_bandwidth_sampler()) {
+    if (!aggregation_epoch_start_time_.IsInitialized()) {
+      aggregation_epoch_start_time_ = sent_time;
+    }
   }
 
   sampler_.OnPacketSent(sent_time, packet_number, bytes, bytes_in_flight,
@@ -290,14 +287,14 @@
     startup_rate_reduction_multiplier_ = 2;
   }
   if (config.HasClientRequestedIndependentOption(kBBR4, perspective)) {
-    if (quic_track_ack_height_in_bandwidth_sampler_) {
+    if (sampler_.quic_track_ack_height_in_bandwidth_sampler()) {
       sampler_.SetMaxAckHeightTrackerWindowLength(2 * kBandwidthWindowSize);
     } else {
       max_ack_height_.SetWindowLength(2 * kBandwidthWindowSize);
     }
   }
   if (config.HasClientRequestedIndependentOption(kBBR5, perspective)) {
-    if (quic_track_ack_height_in_bandwidth_sampler_) {
+    if (sampler_.quic_track_ack_height_in_bandwidth_sampler()) {
       sampler_.SetMaxAckHeightTrackerWindowLength(4 * kBandwidthWindowSize);
     } else {
       max_ack_height_.SetWindowLength(4 * kBandwidthWindowSize);
@@ -419,7 +416,7 @@
     const QuicByteCount bytes_acked =
         sampler_.total_bytes_acked() - total_bytes_acked_before;
 
-    excess_acked = quic_track_ack_height_in_bandwidth_sampler_
+    excess_acked = sampler_.quic_track_ack_height_in_bandwidth_sampler()
                        ? sampler_.OnAckEventEnd(max_bandwidth_.GetBest(),
                                                 round_trip_count_)
                        : UpdateAckAggregationBytes(event_time, bytes_acked);
@@ -664,7 +661,7 @@
     rounds_without_bandwidth_gain_ = 0;
     if (expire_ack_aggregation_in_startup_) {
       // Expire old excess delivery measurements now that bandwidth increased.
-      if (quic_track_ack_height_in_bandwidth_sampler_) {
+      if (sampler_.quic_track_ack_height_in_bandwidth_sampler()) {
         sampler_.ResetMaxAckHeightTracker(0, round_trip_count_);
       } else {
         max_ack_height_.Reset(0, round_trip_count_);
@@ -868,7 +865,7 @@
       GetTargetCongestionWindow(congestion_window_gain_);
   if (is_at_full_bandwidth_) {
     // Add the max recently measured ack aggregation to CWND.
-    target_window += quic_track_ack_height_in_bandwidth_sampler_
+    target_window += sampler_.quic_track_ack_height_in_bandwidth_sampler()
                          ? sampler_.max_ack_height()
                          : max_ack_height_.GetBest();
   } else if (enable_ack_aggregation_during_startup_) {

quic/core/congestion_control/bbr_sender.h

diff --git a/quic/core/congestion_control/bbr_sender.h b/quic/core/congestion_control/bbr_sender.h
index b74b783..592893e 100644
--- a/quic/core/congestion_control/bbr_sender.h
+++ b/quic/core/congestion_control/bbr_sender.h
@@ -399,8 +399,6 @@
   bool probe_rtt_disabled_if_app_limited_;
   bool app_limited_since_last_probe_rtt_;
   QuicTime::Delta min_rtt_since_last_probe_rtt_;
-  // Latched value of --quic_track_ack_height_in_bandwidth_sampler.
-  const bool quic_track_ack_height_in_bandwidth_sampler_;
 };
 
 QUIC_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,