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
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() {}
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
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_) {
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,
