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