gfe-relnote: In QUIC, skipping a packet number, when a single PTO packet will be sent, to elicit an immediate ACK. Protected by gfe2_reloadable_flag_quic_skip_packet_number_for_pto.
PiperOrigin-RevId: 268899762
Change-Id: I5c4313b8c9b2a01392f4728bd6364071657fa0d5
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 9b3559d..8adf0ae 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -190,6 +190,8 @@
// consecutive PTOs.
const QuicTag k8PTO = TAG('8', 'P', 'T', 'O'); // Closes connection on 8
// consecutive PTOs.
+const QuicTag kPTOS = TAG('P', 'T', 'O', 'S'); // Skip packet number before
+ // sending the last PTO.
// Optional support of truncated Connection IDs. If sent by a peer, the value
// is the minimum number of bytes allowed for the connection ID sent to the
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index ebd2f45..49fc498 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -325,7 +325,8 @@
max_consecutive_ptos_(0),
bytes_received_before_address_validation_(0),
bytes_sent_before_address_validation_(0),
- address_validated_(false) {
+ address_validated_(false),
+ skip_packet_number_for_pto_(false) {
QUIC_DLOG(INFO) << ENDPOINT << "Created connection with server connection ID "
<< server_connection_id
<< " and version: " << ParsedQuicVersionToString(version());
@@ -432,6 +433,11 @@
max_consecutive_ptos_ = 7;
QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_pto, 4, 4);
}
+ if (GetQuicReloadableFlag(quic_skip_packet_number_for_pto) &&
+ config.HasClientSentConnectionOption(kPTOS, perspective_)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_skip_packet_number_for_pto);
+ skip_packet_number_for_pto_ = true;
+ }
}
if (config.HasClientSentConnectionOption(kNSTP, perspective_)) {
no_stop_waiting_frames_ = true;
@@ -2585,6 +2591,15 @@
const auto retransmission_mode =
sent_packet_manager_.OnRetransmissionTimeout();
+ if (skip_packet_number_for_pto_ &&
+ retransmission_mode == QuicSentPacketManager::PTO_MODE &&
+ sent_packet_manager_.pending_timer_transmission_count() == 1) {
+ // Skip a packet number when a single PTO packet is sent to elicit an
+ // immediate ACK.
+ packet_generator_.SkipNPacketNumbers(
+ 1, sent_packet_manager_.GetLeastUnacked(),
+ sent_packet_manager_.EstimateMaxPacketsInFlight(max_packet_length()));
+ }
WriteIfNotBlocked();
// A write failure can result in the connection being closed, don't attempt to
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index e214a99..c811e96 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1483,6 +1483,9 @@
// packet has been successfully processed. Only used when
// EnforceAntiAmplificationLimit returns true.
bool address_validated_;
+
+ // If true, skip packet number before sending the last PTO retransmission.
+ bool skip_packet_number_for_pto_;
};
} // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 85489bc..2d07981 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -9160,6 +9160,37 @@
connection_.GetRetransmissionAlarm()->Fire();
}
+TEST_P(QuicConnectionTest, PtoSkipsPacketNumber) {
+ if (!connection_.session_decides_what_to_write() ||
+ !GetQuicReloadableFlag(quic_fix_rto_retransmission3)) {
+ return;
+ }
+ SetQuicReloadableFlag(quic_enable_pto, true);
+ SetQuicReloadableFlag(quic_skip_packet_number_for_pto, true);
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(k1PTO);
+ connection_options.push_back(kPTOS);
+ config.SetConnectionOptionsToSend(connection_options);
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ connection_.SetFromConfig(config);
+ EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
+
+ QuicStreamId stream_id = 2;
+ QuicPacketNumber last_packet;
+ SendStreamDataToPeer(stream_id, "foooooo", 0, NO_FIN, &last_packet);
+ SendStreamDataToPeer(stream_id, "foooooo", 7, NO_FIN, &last_packet);
+ EXPECT_EQ(QuicPacketNumber(2), last_packet);
+ EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
+
+ // Fire PTO and verify the PTO retransmission skips one packet number.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ connection_.GetRetransmissionAlarm()->Fire();
+ EXPECT_EQ(1u, writer_->stream_frames().size());
+ EXPECT_EQ(QuicPacketNumber(4), writer_->last_packet_header().packet_number);
+ EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 0b7a9f2..4c381d4 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -159,6 +159,33 @@
framer_->transport_version(), QuicPacketNumber(delta * 4));
}
+void QuicPacketCreator::SkipNPacketNumbers(
+ QuicPacketCount count,
+ QuicPacketNumber least_packet_awaited_by_peer,
+ QuicPacketCount max_packets_in_flight) {
+ if (!queued_frames_.empty()) {
+ // Don't change creator state if there are frames queued.
+ QUIC_BUG << "Called SkipNPacketNumbers with " << queued_frames_.size()
+ << " queued_frames. First frame type:"
+ << queued_frames_.front().type
+ << " last frame type:" << queued_frames_.back().type;
+ return;
+ }
+ if (packet_.packet_number > packet_.packet_number + count) {
+ // Skipping count packet numbers causes packet number wrapping around,
+ // reject it.
+ QUIC_LOG(WARNING) << "Skipping " << count
+ << " packet numbers causes packet number wrapping "
+ "around, least_packet_awaited_by_peer: "
+ << least_packet_awaited_by_peer
+ << " packet_number:" << packet_.packet_number;
+ return;
+ }
+ packet_.packet_number += count;
+ // Packet number changes, update packet number length if necessary.
+ UpdatePacketNumberLength(least_packet_awaited_by_peer, max_packets_in_flight);
+}
+
bool QuicPacketCreator::ConsumeCryptoData(EncryptionLevel level,
size_t write_length,
QuicStreamOffset offset,
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index 8e3ea3f..9f00842 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -82,6 +82,11 @@
void UpdatePacketNumberLength(QuicPacketNumber least_packet_awaited_by_peer,
QuicPacketCount max_packets_in_flight);
+ // Skip |count| packet numbers.
+ void SkipNPacketNumbers(QuicPacketCount count,
+ QuicPacketNumber least_packet_awaited_by_peer,
+ QuicPacketCount max_packets_in_flight);
+
// The overhead the framing will add for a packet with one frame.
static size_t StreamFramePacketOverhead(
QuicTransportVersion version,
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index e7c82d1..d0cfdae 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -1181,6 +1181,36 @@
QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
}
+TEST_P(QuicPacketCreatorTest, SkipNPacketNumbers) {
+ QuicPacketCreatorPeer::SetPacketNumber(&creator_, 1);
+ if (VersionHasIetfInvariantHeader(GetParam().version.transport_version) &&
+ !GetParam().version.SendsVariableLengthPacketNumberInLongHeader()) {
+ EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
+ QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+ creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
+ } else {
+ EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+ QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+ }
+ creator_.SkipNPacketNumbers(63, QuicPacketNumber(2),
+ 10000 / kDefaultMaxPacketSize);
+ EXPECT_EQ(QuicPacketNumber(64), creator_.packet_number());
+ EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+ QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+ creator_.SkipNPacketNumbers(64 * 255, QuicPacketNumber(2),
+ 10000 / kDefaultMaxPacketSize);
+ EXPECT_EQ(QuicPacketNumber(64 * 256), creator_.packet_number());
+ EXPECT_EQ(PACKET_2BYTE_PACKET_NUMBER,
+ QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+ creator_.SkipNPacketNumbers(64 * 256 * 255, QuicPacketNumber(2),
+ 10000 / kDefaultMaxPacketSize);
+ EXPECT_EQ(QuicPacketNumber(64 * 256 * 256), creator_.packet_number());
+ EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
+ QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+}
+
TEST_P(QuicPacketCreatorTest, SerializeFrame) {
if (!GetParam().version_serialization) {
creator_.StopSendingVersion();
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc
index 160222f..d4e879f 100644
--- a/quic/core/quic_packet_generator.cc
+++ b/quic/core/quic_packet_generator.cc
@@ -337,6 +337,14 @@
max_packets_in_flight);
}
+void QuicPacketGenerator::SkipNPacketNumbers(
+ QuicPacketCount count,
+ QuicPacketNumber least_packet_awaited_by_peer,
+ QuicPacketCount max_packets_in_flight) {
+ packet_creator_.SkipNPacketNumbers(count, least_packet_awaited_by_peer,
+ max_packets_in_flight);
+}
+
void QuicPacketGenerator::SetServerConnectionIdLength(uint32_t length) {
if (length == 0) {
packet_creator_.SetServerConnectionIdIncluded(CONNECTION_ID_ABSENT);
diff --git a/quic/core/quic_packet_generator.h b/quic/core/quic_packet_generator.h
index c6902e0..69c69ea 100644
--- a/quic/core/quic_packet_generator.h
+++ b/quic/core/quic_packet_generator.h
@@ -172,6 +172,11 @@
void UpdatePacketNumberLength(QuicPacketNumber least_packet_awaited_by_peer,
QuicPacketCount max_packets_in_flight);
+ // Skip |count| packet numbers.
+ void SkipNPacketNumbers(QuicPacketCount count,
+ QuicPacketNumber least_packet_awaited_by_peer,
+ QuicPacketCount max_packets_in_flight);
+
// Set the minimum number of bytes for the server connection id length;
void SetServerConnectionIdLength(uint32_t length);
