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

quic/core/quic_connection.cc

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();
   }

quic/core/quic_connection_test.cc

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