Do not use PING only packet for RTT measure or congestion control. Client only and behind IGNP config.
PiperOrigin-RevId: 343891727
Change-Id: I814dc2623ea6533fe36ed73d9d69c09392a63d36
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 96c1be1..8093a68 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -396,6 +396,10 @@
const QuicTag kMAD = TAG('M', 'A', 'D', 0); // Max Ack Delay (IETF QUIC)
+const QuicTag kIGNP = TAG('I', 'G', 'N', 'P'); // Do not use PING only packet
+ // for RTT measure or
+ // congestion control.
+
// Rejection tags
const QuicTag kRREJ = TAG('R', 'R', 'E', 'J'); // Reasons for server sending
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index c823582..fba8b67 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -116,7 +116,8 @@
first_pto_srtt_multiplier_(0),
use_standard_deviation_for_pto_(false),
pto_multiplier_without_rtt_samples_(3),
- num_ptos_for_path_degrading_(0) {
+ num_ptos_for_path_degrading_(0),
+ ignore_pings_(false) {
SetSendAlgorithm(congestion_control_type);
if (pto_enabled_) {
QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_on_pto, 1, 2);
@@ -276,6 +277,10 @@
send_algorithm_->SetInitialCongestionWindowInPackets(10);
}
+ if (config.HasClientRequestedIndependentOption(kIGNP, perspective)) {
+ ignore_pings_ = true;
+ }
+
using_pacing_ = !GetQuicFlag(FLAGS_quic_disable_pacing_for_perf_tests);
if (config.HasClientSentConnectionOption(kNTLP, perspective)) {
@@ -798,6 +803,12 @@
}
bool in_flight = has_retransmittable_data == HAS_RETRANSMITTABLE_DATA;
+ if (ignore_pings_ && mutable_packet->retransmittable_frames.size() == 1 &&
+ mutable_packet->retransmittable_frames[0].type == PING_FRAME) {
+ // Dot not use PING only packet for RTT measure or congestion control.
+ in_flight = false;
+ measure_rtt = false;
+ }
if (using_pacing_) {
pacing_sender_.OnPacketSent(sent_time, unacked_packets_.bytes_in_flight(),
packet_number, packet.encrypted_length,
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index f6a6fa8..2dba799 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -736,6 +736,10 @@
// The number of PTOs needed for path degrading alarm. If equals to 0, the
// traditional path degrading mechanism will be used.
int num_ptos_for_path_degrading_;
+
+ // If true, do not use PING only packets for RTT measurement or congestion
+ // control.
+ bool ignore_pings_;
};
} // namespace quic
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 4ef1664..1aa4974 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -3882,6 +3882,55 @@
EXPECT_EQ(expected_delay, manager_.GetPathDegradingDelay());
}
+TEST_F(QuicSentPacketManagerTest, ClientsIgnorePings) {
+ QuicSentPacketManagerPeer::SetPerspective(&manager_, Perspective::IS_CLIENT);
+ QuicConfig client_config;
+ QuicTagVector options;
+ QuicTagVector client_options;
+ client_options.push_back(kIGNP);
+ client_config.SetConnectionOptionsToSend(options);
+ client_config.SetClientConnectionOptions(client_options);
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange());
+ manager_.SetFromConfig(client_config);
+
+ EXPECT_CALL(*send_algorithm_, PacingRate(_))
+ .WillRepeatedly(Return(QuicBandwidth::Zero()));
+ EXPECT_CALL(*send_algorithm_, GetCongestionWindow())
+ .WillRepeatedly(Return(10 * kDefaultTCPMSS));
+ EXPECT_CALL(*send_algorithm_, CanSend(_)).WillRepeatedly(Return(true));
+
+ SendPingPacket(1, ENCRYPTION_INITIAL);
+ // Verify PING only packet is not considered in flight.
+ EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime());
+ SendDataPacket(2, ENCRYPTION_INITIAL);
+ EXPECT_NE(QuicTime::Zero(), manager_.GetRetransmissionTime());
+
+ uint64_t acked[] = {1};
+ ExpectAcksAndLosses(/*rtt_updated=*/false, acked, ABSL_ARRAYSIZE(acked),
+ nullptr, 0);
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(90));
+ manager_.OnAckFrameStart(QuicPacketNumber(1), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_INITIAL));
+ RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats());
+ // Verify no RTT samples for PING only packet.
+ EXPECT_TRUE(rtt_stats->smoothed_rtt().IsZero());
+
+ ExpectAck(2);
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(10));
+ manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(3));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
+ ENCRYPTION_INITIAL));
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100), rtt_stats->smoothed_rtt());
+}
+
// Regression test for b/154050235.
TEST_F(QuicSentPacketManagerTest, ExponentialBackoffWithNoRttMeasurement) {
QuicSentPacketManagerPeer::SetPerspective(&manager_, Perspective::IS_CLIENT);
