gfe-relnote: In QUIC, send a PING when skipping packet number PTO fires and there is no data to send. Protected by gfe2_reloadable_flag_quic_send_ping_when_pto_skips_packet_number.
PiperOrigin-RevId: 300369878
Change-Id: I395da0107a24c218d9b96fc87df056a6213604f3
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 4a92ab7..f3d7bca 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2634,7 +2634,7 @@
}
#endif
- const QuicPacketNumber previous_created_packet_number =
+ QuicPacketNumber previous_created_packet_number =
packet_creator_.packet_number();
if (close_connection_after_five_rtos_ &&
sent_packet_manager_.GetConsecutiveRtoCount() >= 4) {
@@ -2663,6 +2663,10 @@
packet_creator_.SkipNPacketNumbers(
num_packet_numbers_to_skip, sent_packet_manager_.GetLeastUnacked(),
sent_packet_manager_.EstimateMaxPacketsInFlight(max_packet_length()));
+ if (GetQuicReloadableFlag(quic_send_ping_when_pto_skips_packet_number)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_send_ping_when_pto_skips_packet_number);
+ previous_created_packet_number += num_packet_numbers_to_skip;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnNPacketNumbersSkipped(num_packet_numbers_to_skip);
}
@@ -2689,17 +2693,11 @@
retransmission_mode == QuicSentPacketManager::RTO_MODE ||
retransmission_mode == QuicSentPacketManager::PTO_MODE) &&
!visitor_->WillingAndAbleToWrite()) {
- // Send PING if timer fires in RTO or PTO mode but there is no data to
+ // Send PING if timer fires in TLP/RTO/PTO mode but there is no data to
// send.
- // When TLP fires, either new data or tail loss probe should be sent.
- // There is corner case where TLP fires after RTO because packets get
- // acked. Two packets are marked RTO_RETRANSMITTED, but the first packet
- // is retransmitted as two packets because of packet number length
- // increases (please see QuicConnectionTest.RtoPacketAsTwo).
- QUIC_DLOG_IF(WARNING,
- retransmission_mode == QuicSentPacketManager::TLP_MODE &&
- stats_.rto_count == 0)
- << "No packet gets sent when timer fires in TLP mode, sending PING";
+ QUIC_DLOG(INFO) << ENDPOINT
+ << "No packet gets sent when timer fires in mode "
+ << retransmission_mode << ", send PING";
DCHECK_LT(0u, sent_packet_manager_.pending_timer_transmission_count());
visitor_->SendPing();
}
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index ce0829f..0940048 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1380,6 +1380,16 @@
void SendPing() { notifier_.WriteOrBufferPing(); }
+ MessageStatus SendMessage(quiche::QuicheStringPiece message) {
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ QuicMemSliceStorage storage(nullptr, 0, nullptr, 0);
+ return connection_.SendMessage(
+ 1,
+ MakeSpan(connection_.helper()->GetStreamSendBufferAllocator(), message,
+ &storage),
+ false);
+ }
+
void ProcessAckPacket(uint64_t packet_number, QuicAckFrame* frame) {
if (packet_number > 1) {
QuicPacketCreatorPeer::SetPacketNumber(&peer_creator_, packet_number - 1);
@@ -10158,6 +10168,44 @@
ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
}
+// Regression test for b/151220135.
+TEST_P(QuicConnectionTest, SendPingWhenSkipPacketNumberForPto) {
+ if (!VersionSupportsMessageFrames(connection_.transport_version())) {
+ return;
+ }
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(kPTOS);
+ connection_options.push_back(k1PTO);
+ config.SetConnectionOptionsToSend(connection_options);
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ connection_.SetFromConfig(config);
+ EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
+
+ EXPECT_EQ(MESSAGE_STATUS_SUCCESS, SendMessage("message"));
+ EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
+
+ // Although there are bytes in flight, no packet gets sent on PTO firing.
+ if (GetQuicReloadableFlag(quic_send_ping_when_pto_skips_packet_number)) {
+ // PTO fires, verify a PING packet gets sent because there is no data to
+ // send.
+ EXPECT_CALL(*send_algorithm_,
+ OnPacketSent(_, _, QuicPacketNumber(3), _, _));
+ EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() {
+ SendPing();
+ }));
+ } else {
+ // No packet gets sent.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
+ }
+ connection_.GetRetransmissionAlarm()->Fire();
+ if (GetQuicReloadableFlag(quic_send_ping_when_pto_skips_packet_number)) {
+ EXPECT_EQ(1u, connection_.GetStats().pto_count);
+ EXPECT_EQ(0u, connection_.GetStats().crypto_retransmit_count);
+ EXPECT_EQ(1u, writer_->ping_frames().size());
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
