gfe-relnote: Deprecate gfe2_restart_flag_quic_enable_accept_random_ipn.
PiperOrigin-RevId: 253603387
Change-Id: I462d960d7cccaab09c908e0fefb151367ca58007
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index c7f98ed..a7733cb 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2263,61 +2263,24 @@
return true;
}
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- QUIC_RESTART_FLAG_COUNT_N(quic_enable_accept_random_ipn, 2, 2);
- // Configured to accept any packet number in range 1...0x7fffffff as initial
- // packet number.
- bool out_of_bound = false;
- std::string error_detail = "Packet number out of bounds.";
- if (last_header_.packet_number.IsInitialized()) {
- out_of_bound = !Near(packet_number, last_header_.packet_number);
- } else if ((packet_number > MaxRandomInitialPacketNumber())) {
- out_of_bound = true;
- error_detail = "Initial packet number out of bounds.";
- }
- if (out_of_bound) {
- QUIC_DLOG(INFO) << ENDPOINT << "Packet " << packet_number
- << " out of bounds. Discarding";
- CloseConnection(QUIC_INVALID_PACKET_HEADER, error_detail,
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
- return false;
- }
- return true;
+ // Configured to accept any packet number in range 1...0x7fffffff as initial
+ // packet number.
+ bool out_of_bound = false;
+ std::string error_detail = "Packet number out of bounds.";
+ if (last_header_.packet_number.IsInitialized()) {
+ out_of_bound = !Near(packet_number, last_header_.packet_number);
+ } else if ((packet_number > MaxRandomInitialPacketNumber())) {
+ out_of_bound = true;
+ error_detail = "Initial packet number out of bounds.";
}
-
- if (packet_number > received_packet_manager_.PeerFirstSendingPacketNumber() &&
- packet_number <= MaxRandomInitialPacketNumber()) {
- QUIC_CODE_COUNT_N(had_possibly_random_ipn, 2, 2);
+ if (out_of_bound) {
+ QUIC_DLOG(INFO) << ENDPOINT << "Packet " << packet_number
+ << " out of bounds. Discarding";
+ CloseConnection(QUIC_INVALID_PACKET_HEADER, error_detail,
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return false;
}
- const bool out_of_bound =
- last_header_.packet_number.IsInitialized()
- ? !Near(packet_number, last_header_.packet_number)
- : packet_number >=
- (received_packet_manager_.PeerFirstSendingPacketNumber() +
- kMaxPacketGap);
- if (!out_of_bound) {
- return true;
- }
- QUIC_DLOG(INFO) << ENDPOINT << "Packet " << packet_number
- << " out of bounds. Discarding";
- QuicStringPiece packet_data = GetCurrentPacket();
- const size_t kMaxPacketLengthInErrorDetails = 64;
- CloseConnection(
- QUIC_INVALID_PACKET_HEADER,
- QuicStrCat(
- "Packet number out of bounds. ",
- last_header_.packet_number.IsInitialized()
- ? QuicStrCat("last_pkn=", last_header_.packet_number.ToUint64())
- : "first received packet",
- ", current_pkn=", packet_number.ToUint64(),
- ", current_pkt_len=", packet_data.length(), ", current_hdr=",
- QuicTextUtils::HexEncode(
- packet_data.length() > kMaxPacketLengthInErrorDetails
- ? QuicStringPiece(packet_data.data(),
- kMaxPacketLengthInErrorDetails)
- : packet_data)),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
- return false;
+ return true;
}
void QuicConnection::WriteQueuedPackets() {
@@ -4148,8 +4111,7 @@
const bool enable_multiple_packet_number_spaces =
version().handshake_protocol == PROTOCOL_TLS1_3 &&
use_uber_received_packet_manager_ &&
- sent_packet_manager_.use_uber_loss_algorithm() &&
- GetQuicRestartFlag(quic_enable_accept_random_ipn);
+ sent_packet_manager_.use_uber_loss_algorithm();
if (!enable_multiple_packet_number_spaces) {
return;
}
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 314898b..d60e3d6 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -2470,11 +2470,7 @@
// Call ProcessDataPacket rather than ProcessPacket, as we should not get a
// packet call to the visitor.
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- ProcessDataPacket(MaxRandomInitialPacketNumber() + 6000);
- } else {
- ProcessDataPacket(6000);
- }
+ ProcessDataPacket(MaxRandomInitialPacketNumber() + 6000);
EXPECT_FALSE(QuicConnectionPeer::GetConnectionClosePacket(&connection_) ==
nullptr);
}
@@ -2505,38 +2501,20 @@
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
ProcessPacket(3);
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- // Should not cause an ack.
- EXPECT_EQ(0u, writer_->packets_write_attempts());
- } else {
- // Should ack immediately since we have missing packets.
- EXPECT_EQ(1u, writer_->packets_write_attempts());
- }
+ // Should not cause an ack.
+ EXPECT_EQ(0u, writer_->packets_write_attempts());
ProcessPacket(2);
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- // Should ack immediately, since this fills the last hole.
- EXPECT_EQ(1u, writer_->packets_write_attempts());
- } else {
- // Should ack immediately since we have missing packets.
- EXPECT_EQ(2u, writer_->packets_write_attempts());
- }
+ // Should ack immediately, since this fills the last hole.
+ EXPECT_EQ(1u, writer_->packets_write_attempts());
ProcessPacket(1);
// Should ack immediately, since this fills the last hole.
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_EQ(2u, writer_->packets_write_attempts());
- } else {
- EXPECT_EQ(3u, writer_->packets_write_attempts());
- }
+ EXPECT_EQ(2u, writer_->packets_write_attempts());
ProcessPacket(4);
// Should not cause an ack.
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_EQ(2u, writer_->packets_write_attempts());
- } else {
- EXPECT_EQ(3u, writer_->packets_write_attempts());
- }
+ EXPECT_EQ(2u, writer_->packets_write_attempts());
}
TEST_P(QuicConnectionTest, OutOfOrderAckReceiptCausesNoAck) {
@@ -6299,20 +6277,9 @@
TEST_P(QuicConnectionTest, NoAckOnOldNacks) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
- // Drop one packet, triggering a sequence of acks.
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
ProcessPacket(2);
size_t frames_per_ack = GetParam().no_stop_waiting ? 1 : 2;
- if (!GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- size_t padding_frame_count = writer_->padding_frames().size();
- EXPECT_EQ(padding_frame_count + frames_per_ack, writer_->frame_count());
- EXPECT_FALSE(writer_->ack_frames().empty());
- writer_->Reset();
- }
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
ProcessPacket(3);
@@ -6321,19 +6288,9 @@
EXPECT_FALSE(writer_->ack_frames().empty());
writer_->Reset();
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
ProcessPacket(4);
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_EQ(0u, writer_->frame_count());
- } else {
- EXPECT_EQ(padding_frame_count + frames_per_ack, writer_->frame_count());
- EXPECT_FALSE(writer_->ack_frames().empty());
- writer_->Reset();
- }
+ EXPECT_EQ(0u, writer_->frame_count());
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
ProcessPacket(5);
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 495163c..1025ed1 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -565,28 +565,10 @@
if (!header.packet_number.IsInitialized()) {
return kFateTimeWait;
}
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- QUIC_RESTART_FLAG_COUNT_N(quic_enable_accept_random_ipn, 1, 2);
- // Accepting Initial Packet Numbers in 1...((2^31)-1) range... check
- // maximum accordingly.
- if (header.packet_number > MaxRandomInitialPacketNumber()) {
- return kFateTimeWait;
- }
- } else {
- // Count those that would have been accepted if FLAGS..random_ipn
- // were true -- to detect/diagnose potential issues prior to
- // enabling the flag.
- if ((header.packet_number >
- QuicPacketNumber(kMaxReasonableInitialPacketNumber)) &&
- (header.packet_number <= MaxRandomInitialPacketNumber())) {
- QUIC_CODE_COUNT_N(had_possibly_random_ipn, 1, 2);
- }
- // Check that the sequence number is within the range that the client is
- // expected to send before receiving a response from the server.
- if (header.packet_number >
- QuicPacketNumber(kMaxReasonableInitialPacketNumber)) {
- return kFateTimeWait;
- }
+ // Accepting Initial Packet Numbers in 1...((2^31)-1) range... check
+ // maximum accordingly.
+ if (header.packet_number > MaxRandomInitialPacketNumber()) {
+ return kFateTimeWait;
}
}
return kFateProcess;
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index 0a694a3..91794c0 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -868,44 +868,6 @@
EXPECT_EQ(server_address_, dispatcher_->current_self_address());
}
-TEST_F(QuicDispatcherTest, TooBigSeqNoPacketToTimeWaitListManager) {
- if (CurrentSupportedVersions().front().HasHeaderProtection() ||
- GetQuicRestartFlag(quic_no_framer_object_in_dispatcher)) {
- // When header protection is in use, we don't put packets in the time wait
- // list manager based on packet number.
- return;
- }
- CreateTimeWaitListManager();
- SetQuicRestartFlag(quic_enable_accept_random_ipn, false);
- QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
- QuicConnectionId connection_id = TestConnectionId(1);
-
- // Dispatcher forwards this packet for this connection_id to the time wait
- // list manager.
- EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, QuicStringPiece("hq"), _))
- .Times(0);
- EXPECT_CALL(*time_wait_list_manager_,
- ProcessPacket(_, _, TestConnectionId(1), _, _))
- .Times(1);
- EXPECT_CALL(*time_wait_list_manager_,
- ProcessPacket(_, _, TestConnectionId(2), _, _))
- .Times(1);
- EXPECT_CALL(*time_wait_list_manager_,
- AddConnectionIdToTimeWait(_, _, _, _, _))
- .Times(2);
- // A packet whose packet number is one to large to be allowed to start a
- // connection.
- ProcessPacket(client_address, connection_id, true, SerializeCHLO(),
- CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER,
- QuicDispatcher::kMaxReasonableInitialPacketNumber + 1);
- connection_id = TestConnectionId(2);
- SetQuicRestartFlag(quic_enable_accept_random_ipn, true);
- ProcessPacket(client_address, connection_id, true, SerializeCHLO(),
- CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER,
- MaxRandomInitialPacketNumber().ToUint64() +
- QuicDispatcher::kMaxReasonableInitialPacketNumber + 1);
-}
-
TEST_F(QuicDispatcherTest, SupportedTransportVersionsChangeInFlight) {
static_assert(QUIC_ARRAYSIZE(kSupportedTransportVersions) == 6u,
"Supported versions out of sync");
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 6fb8c49..4512952 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -321,12 +321,6 @@
if (ack_frame_.packets.NumIntervals() > 1) {
return true;
}
- if (!GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- return ack_frame_.packets.Min() >
- (peer_least_packet_awaiting_ack_.IsInitialized()
- ? peer_least_packet_awaiting_ack_
- : QuicPacketNumber(1));
- }
return peer_least_packet_awaiting_ack_.IsInitialized() &&
ack_frame_.packets.Min() > peer_least_packet_awaiting_ack_;
}
@@ -346,9 +340,6 @@
QuicPacketNumber QuicReceivedPacketManager::PeerFirstSendingPacketNumber()
const {
- if (!GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- return QuicPacketNumber(1);
- }
if (!least_received_packet_number_.IsInitialized()) {
QUIC_BUG << "No packets have been received yet";
return QuicPacketNumber(1);
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index 7c9bd7f..8abe5f9 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -85,11 +85,10 @@
QuicPacketNumber GetLargestObserved() const;
- // Returns peer first sending packet number to our best knowledge. If
- // GetQuicRestartFlag(quic_enable_accept_random_ipn) is false, returns 1.
- // Otherwise considers least_received_packet_number_ as peer first sending
- // packet number. Please note, this function should only be called when at
- // least one packet has been received.
+ // Returns peer first sending packet number to our best knowledge. Considers
+ // least_received_packet_number_ as peer first sending packet number. Please
+ // note, this function should only be called when at least one packet has been
+ // received.
QuicPacketNumber PeerFirstSendingPacketNumber() const;
void set_connection_stats(QuicConnectionStats* stats) { stats_ = stats; }
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index 0512cb7..a9cc387 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -223,33 +223,16 @@
}
TEST_P(QuicReceivedPacketManagerTest, HasMissingPackets) {
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_QUIC_BUG(received_manager_.PeerFirstSendingPacketNumber(),
- "No packets have been received yet");
- } else {
- EXPECT_EQ(QuicPacketNumber(1),
- received_manager_.PeerFirstSendingPacketNumber());
- }
+ EXPECT_QUIC_BUG(received_manager_.PeerFirstSendingPacketNumber(),
+ "No packets have been received yet");
RecordPacketReceipt(4, QuicTime::Zero());
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_EQ(QuicPacketNumber(4),
- received_manager_.PeerFirstSendingPacketNumber());
- EXPECT_FALSE(received_manager_.HasMissingPackets());
- } else {
- EXPECT_TRUE(received_manager_.HasMissingPackets());
- EXPECT_EQ(QuicPacketNumber(1),
- received_manager_.PeerFirstSendingPacketNumber());
- }
+ EXPECT_EQ(QuicPacketNumber(4),
+ received_manager_.PeerFirstSendingPacketNumber());
+ EXPECT_FALSE(received_manager_.HasMissingPackets());
RecordPacketReceipt(3, QuicTime::Zero());
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- EXPECT_FALSE(received_manager_.HasMissingPackets());
- EXPECT_EQ(QuicPacketNumber(3),
- received_manager_.PeerFirstSendingPacketNumber());
- } else {
- EXPECT_TRUE(received_manager_.HasMissingPackets());
- EXPECT_EQ(QuicPacketNumber(1),
- received_manager_.PeerFirstSendingPacketNumber());
- }
+ EXPECT_FALSE(received_manager_.HasMissingPackets());
+ EXPECT_EQ(QuicPacketNumber(3),
+ received_manager_.PeerFirstSendingPacketNumber());
RecordPacketReceipt(1, QuicTime::Zero());
EXPECT_EQ(QuicPacketNumber(1),
received_manager_.PeerFirstSendingPacketNumber());
@@ -268,13 +251,8 @@
RecordPacketReceipt(3, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 3);
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- // Delayed ack is scheduled.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
- } else {
- // Should ack immediately since we have missing packets.
- CheckAckTimeout(clock_.ApproximateNow());
- }
+ // Delayed ack is scheduled.
+ CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
RecordPacketReceipt(2, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 2);
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 9c4027d..f0b5416 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -431,7 +431,6 @@
SetQuicReloadableFlag(quic_simplify_stop_waiting, true);
SetQuicRestartFlag(quic_no_server_conn_ver_negotiation2, true);
SetQuicRestartFlag(quic_server_drop_version_negotiation, true);
- SetQuicRestartFlag(quic_enable_accept_random_ipn, true);
SetQuicRestartFlag(quic_allow_variable_length_connection_id_for_negotiation,
true);
SetQuicRestartFlag(quic_do_not_override_connection_id, true);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc
index b717178..d7d49ee 100644
--- a/quic/core/uber_received_packet_manager_test.cc
+++ b/quic/core/uber_received_packet_manager_test.cc
@@ -239,13 +239,8 @@
RecordPacketReceipt(3, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 3);
- if (GetQuicRestartFlag(quic_enable_accept_random_ipn)) {
- // Delayed ack is scheduled.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
- } else {
- // Should ack immediately since we have missing packets.
- CheckAckTimeout(clock_.ApproximateNow());
- }
+ // Delayed ack is scheduled.
+ CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
RecordPacketReceipt(2, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 2);
@@ -764,7 +759,6 @@
TEST_F(UberReceivedPacketManagerTest, AckSendingDifferentPacketNumberSpaces) {
manager_->EnableMultiplePacketNumberSpacesSupport();
- SetQuicRestartFlag(quic_enable_accept_random_ipn, true);
EXPECT_FALSE(HasPendingAck());
EXPECT_FALSE(manager_->IsAckFrameUpdated());
