gfe-relnote: Notify SendAlgorithm on neutered packets in QUIC. Protected by (enabling blocked) --gfe2_reloadable_flag_quic_avoid_overestimate_bandwidth_with_aggregation.
This is split from cl/279746200 for easier review. I'll unblock this flag in that cl.
PiperOrigin-RevId: 296301871
Change-Id: I499357505e3b817a82b6f4f2172f7b890b684b32
diff --git a/quic/core/congestion_control/bandwidth_sampler.cc b/quic/core/congestion_control/bandwidth_sampler.cc
index 6ecd886..6272664 100644
--- a/quic/core/congestion_control/bandwidth_sampler.cc
+++ b/quic/core/congestion_control/bandwidth_sampler.cc
@@ -73,6 +73,7 @@
: total_bytes_sent_(0),
total_bytes_acked_(0),
total_bytes_lost_(0),
+ total_bytes_neutered_(0),
total_bytes_sent_at_last_acked_packet_(0),
last_acked_packet_sent_time_(QuicTime::Zero()),
last_acked_packet_ack_time_(QuicTime::Zero()),
@@ -147,6 +148,14 @@
"in it.";
}
+void BandwidthSampler::OnPacketNeutered(QuicPacketNumber packet_number) {
+ connection_state_map_.Remove(
+ packet_number, [&](const ConnectionStateOnSentPacket& sent_packet) {
+ QUIC_CODE_COUNT(quic_bandwidth_sampler_packet_neutered);
+ total_bytes_neutered_ += sent_packet.size;
+ });
+}
+
BandwidthSamplerInterface::CongestionEventSample
BandwidthSampler::OnCongestionEvent(QuicTime ack_time,
const AckedPacketVector& acked_packets,
@@ -390,6 +399,10 @@
return total_bytes_lost_;
}
+QuicByteCount BandwidthSampler::total_bytes_neutered() const {
+ return total_bytes_neutered_;
+}
+
bool BandwidthSampler::is_app_limited() const {
return is_app_limited_;
}
diff --git a/quic/core/congestion_control/bandwidth_sampler.h b/quic/core/congestion_control/bandwidth_sampler.h
index df63fa5..90b20fc 100644
--- a/quic/core/congestion_control/bandwidth_sampler.h
+++ b/quic/core/congestion_control/bandwidth_sampler.h
@@ -149,6 +149,8 @@
QuicByteCount bytes_in_flight,
HasRetransmittableData has_retransmittable_data) = 0;
+ virtual void OnPacketNeutered(QuicPacketNumber packet_number) = 0;
+
// Notifies the sampler that the |packet_number| is acknowledged. Returns a
// bandwidth sample. If no bandwidth sample is available,
// QuicBandwidth::Zero() is returned.
@@ -206,10 +208,11 @@
// Remove all the packets lower than the specified packet number.
virtual void RemoveObsoletePackets(QuicPacketNumber least_unacked) = 0;
- // Total number of bytes sent/acked/lost in the connection.
+ // Total number of bytes sent/acked/lost/neutered in the connection.
virtual QuicByteCount total_bytes_sent() const = 0;
virtual QuicByteCount total_bytes_acked() const = 0;
virtual QuicByteCount total_bytes_lost() const = 0;
+ virtual QuicByteCount total_bytes_neutered() const = 0;
// Application-limited information exported for debugging.
virtual bool is_app_limited() const = 0;
@@ -308,6 +311,7 @@
QuicByteCount bytes,
QuicByteCount bytes_in_flight,
HasRetransmittableData has_retransmittable_data) override;
+ void OnPacketNeutered(QuicPacketNumber packet_number) override;
BandwidthSample OnPacketAcknowledged(QuicTime ack_time,
QuicPacketNumber packet_number) override;
CongestionEventSample OnCongestionEvent(
@@ -329,6 +333,7 @@
QuicByteCount total_bytes_sent() const override;
QuicByteCount total_bytes_acked() const override;
QuicByteCount total_bytes_lost() const override;
+ QuicByteCount total_bytes_neutered() const override;
bool is_app_limited() const override;
@@ -432,6 +437,9 @@
// The total number of congestion controlled bytes which were lost.
QuicByteCount total_bytes_lost_;
+ // The total number of congestion controlled bytes which have been neutered.
+ QuicByteCount total_bytes_neutered_;
+
// The value of |total_bytes_sent_| at the time the last acknowledged packet
// was sent. Valid only when |last_acked_packet_sent_time_| is valid.
QuicByteCount total_bytes_sent_at_last_acked_packet_;
diff --git a/quic/core/congestion_control/bandwidth_sampler_test.cc b/quic/core/congestion_control/bandwidth_sampler_test.cc
index 9a3c456..13fa1c0 100644
--- a/quic/core/congestion_control/bandwidth_sampler_test.cc
+++ b/quic/core/congestion_control/bandwidth_sampler_test.cc
@@ -566,16 +566,39 @@
EXPECT_EQ(0u, BandwidthSamplerPeer::GetNumberOfTrackedPackets(sampler_));
}
+TEST_F(BandwidthSamplerTest, NeuterPacket) {
+ SendPacket(1);
+ EXPECT_EQ(0u, sampler_.total_bytes_neutered());
+
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10));
+ sampler_.OnPacketNeutered(QuicPacketNumber(1));
+ EXPECT_LT(0u, sampler_.total_bytes_neutered());
+ EXPECT_EQ(0u, sampler_.total_bytes_acked());
+
+ // If packet 1 is acked it should not produce a bandwidth sample.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10));
+ BandwidthSampler::CongestionEventSample sample = sampler_.OnCongestionEvent(
+ clock_.Now(),
+ {AckedPacket(QuicPacketNumber(1), kRegularPacketSize, clock_.Now())}, {},
+ max_bandwidth_, est_bandwidth_upper_bound_, round_trip_count_);
+ EXPECT_EQ(0u, sampler_.total_bytes_acked());
+ EXPECT_EQ(QuicBandwidth::Zero(), sample.sample_max_bandwidth);
+ EXPECT_FALSE(sample.sample_is_app_limited);
+ EXPECT_EQ(QuicTime::Delta::Infinite(), sample.sample_rtt);
+ EXPECT_EQ(0u, sample.sample_max_inflight);
+ EXPECT_EQ(0u, sample.extra_acked);
+}
+
TEST_F(BandwidthSamplerTest, CongestionEventSampleDefaultValues) {
// Make sure a default constructed CongestionEventSample has the correct
// initial values for BandwidthSampler::OnCongestionEvent() to work.
BandwidthSampler::CongestionEventSample sample;
- DCHECK_EQ(QuicBandwidth::Zero(), sample.sample_max_bandwidth);
- DCHECK(!sample.sample_is_app_limited);
- DCHECK_EQ(QuicTime::Delta::Infinite(), sample.sample_rtt);
- DCHECK_EQ(0u, sample.sample_max_inflight);
- DCHECK_EQ(0u, sample.extra_acked);
+ EXPECT_EQ(QuicBandwidth::Zero(), sample.sample_max_bandwidth);
+ EXPECT_FALSE(sample.sample_is_app_limited);
+ EXPECT_EQ(QuicTime::Delta::Infinite(), sample.sample_rtt);
+ EXPECT_EQ(0u, sample.sample_max_inflight);
+ EXPECT_EQ(0u, sample.extra_acked);
}
// 1) Send 2 packets, 2) Ack both in 1 event, 3) Repeat.
diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 1d6fb35..2226566 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h
@@ -359,6 +359,10 @@
return bandwidth_sampler_.max_ack_height();
}
+ void OnPacketNeutered(QuicPacketNumber packet_number) {
+ bandwidth_sampler_.OnPacketNeutered(packet_number);
+ }
+
uint64_t num_ack_aggregation_epochs() const {
return bandwidth_sampler_.num_ack_aggregation_epochs();
}
diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index d795ad1..108cb34 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc
@@ -284,6 +284,10 @@
is_retransmittable);
}
+void Bbr2Sender::OnPacketNeutered(QuicPacketNumber packet_number) {
+ model_.OnPacketNeutered(packet_number);
+}
+
bool Bbr2Sender::CanSend(QuicByteCount bytes_in_flight) {
const bool result = bytes_in_flight < GetCongestionWindow();
return result;
diff --git a/quic/core/congestion_control/bbr2_sender.h b/quic/core/congestion_control/bbr2_sender.h
index 2bba285..64b6fcb 100644
--- a/quic/core/congestion_control/bbr2_sender.h
+++ b/quic/core/congestion_control/bbr2_sender.h
@@ -64,6 +64,8 @@
QuicByteCount bytes,
HasRetransmittableData is_retransmittable) override;
+ void OnPacketNeutered(QuicPacketNumber packet_number) override;
+
void OnRetransmissionTimeout(bool /*packets_retransmitted*/) override {}
void OnConnectionMigration() override {}
diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index 3fd6139..da723c4 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc
@@ -181,6 +181,10 @@
is_retransmittable);
}
+void BbrSender::OnPacketNeutered(QuicPacketNumber packet_number) {
+ sampler_.OnPacketNeutered(packet_number);
+}
+
bool BbrSender::CanSend(QuicByteCount bytes_in_flight) {
return bytes_in_flight < GetCongestionWindow();
}
diff --git a/quic/core/congestion_control/bbr_sender.h b/quic/core/congestion_control/bbr_sender.h
index 87d3272..a58b8d5 100644
--- a/quic/core/congestion_control/bbr_sender.h
+++ b/quic/core/congestion_control/bbr_sender.h
@@ -118,6 +118,7 @@
QuicPacketNumber packet_number,
QuicByteCount bytes,
HasRetransmittableData is_retransmittable) override;
+ void OnPacketNeutered(QuicPacketNumber packet_number) override;
void OnRetransmissionTimeout(bool /*packets_retransmitted*/) override {}
void OnConnectionMigration() override {}
bool CanSend(QuicByteCount bytes_in_flight) override;
diff --git a/quic/core/congestion_control/send_algorithm_interface.h b/quic/core/congestion_control/send_algorithm_interface.h
index a049e67..b242951 100644
--- a/quic/core/congestion_control/send_algorithm_interface.h
+++ b/quic/core/congestion_control/send_algorithm_interface.h
@@ -110,6 +110,9 @@
QuicByteCount bytes,
HasRetransmittableData is_retransmittable) = 0;
+ // Inform that |packet_number| has been neutered.
+ virtual void OnPacketNeutered(QuicPacketNumber packet_number) = 0;
+
// Called when the retransmission timeout fires. Neither OnPacketAbandoned
// nor OnPacketLost will be called for these packets.
virtual void OnRetransmissionTimeout(bool packets_retransmitted) = 0;
diff --git a/quic/core/congestion_control/tcp_cubic_sender_bytes.h b/quic/core/congestion_control/tcp_cubic_sender_bytes.h
index 9144b8a..7a36f76 100644
--- a/quic/core/congestion_control/tcp_cubic_sender_bytes.h
+++ b/quic/core/congestion_control/tcp_cubic_sender_bytes.h
@@ -61,6 +61,7 @@
QuicPacketNumber packet_number,
QuicByteCount bytes,
HasRetransmittableData is_retransmittable) override;
+ void OnPacketNeutered(QuicPacketNumber /*packet_number*/) override {}
void OnRetransmissionTimeout(bool packets_retransmitted) override;
bool CanSend(QuicByteCount bytes_in_flight) override;
QuicBandwidth PacingRate(QuicByteCount bytes_in_flight) const override;
