Deprecate gfe2_restart_flag_quic_map_original_connection_id2. PiperOrigin-RevId: 476436544
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index 51c0ce0..706e23a 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -7076,7 +7076,6 @@ } TEST_P(EndToEndTest, OriginalConnectionIdClearedFromMap) { - SetQuicRestartFlag(quic_map_original_connection_ids2, true); connect_to_server_on_initialize_ = false; ASSERT_TRUE(Initialize()); if (override_client_connection_id_length_ != kLongConnectionIdLength) {
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index d4af137..ff3834b 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -177,9 +177,7 @@ QUICHE_DCHECK(connection_->connected()); QUIC_DLOG(INFO) << "0-RTT discard alarm fired"; connection_->RemoveDecrypter(ENCRYPTION_ZERO_RTT); - if (GetQuicRestartFlag(quic_map_original_connection_ids2)) { - connection_->RetireOriginalDestinationConnectionId(); - } + connection_->RetireOriginalDestinationConnectionId(); } }; @@ -984,7 +982,6 @@ void QuicConnection::RetireOriginalDestinationConnectionId() { if (original_destination_connection_id_.has_value()) { visitor_->OnServerConnectionIdRetired(*original_destination_connection_id_); - QUIC_RESTART_FLAG_COUNT_N(quic_map_original_connection_ids2, 3, 4); original_destination_connection_id_.reset(); } } @@ -1052,14 +1049,7 @@ if (debug_visitor_ != nullptr) { debug_visitor_->OnIncorrectConnectionId(server_connection_id); } - // The only way for a connection to get a packet with an invalid connection - // ID is if quic_map_original_connection_ids2 is false and a packet - // arrives with a connection ID that is deterministically replaced with one - // that the connection owns, but is different from - // original_destination_connection_id_. - if (GetQuicRestartFlag(quic_map_original_connection_ids2)) { - QUICHE_DCHECK_NE(Perspective::IS_SERVER, perspective_); - } + QUICHE_DCHECK_NE(Perspective::IS_SERVER, perspective_); return false; } @@ -6913,26 +6903,15 @@ if (!original_destination_connection_id_.has_value()) { return result; } - bool add_original_connection_id = false; - if (GetQuicRestartFlag(quic_map_original_connection_ids2)) { - QUIC_RESTART_FLAG_COUNT_N(quic_map_original_connection_ids2, 4, 4); - add_original_connection_id = true; - } else if (!IsHandshakeComplete()) { - QUIC_CODE_COUNT(quic_active_original_connection_id_pre_handshake); - add_original_connection_id = true; - } - if (add_original_connection_id) { - if (std::find(result.begin(), result.end(), - original_destination_connection_id_.value()) != - result.end()) { - QUIC_BUG(quic_unexpected_original_destination_connection_id) - << "original_destination_connection_id: " - << original_destination_connection_id_.value() - << " is unexpectedly in active " - "list"; - } else { - result.insert(result.end(), original_destination_connection_id_.value()); - } + // Add the original connection ID + if (std::find(result.begin(), result.end(), + original_destination_connection_id_.value()) != result.end()) { + QUIC_BUG(quic_unexpected_original_destination_connection_id) + << "original_destination_connection_id: " + << original_destination_connection_id_.value() + << " is unexpectedly in active list"; + } else { + result.insert(result.end(), original_destination_connection_id_.value()); } return result; }
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 2095700..054e8d8 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -13709,22 +13709,22 @@ .WillOnce(Invoke([this]() { connection_.SendControlFrame(QuicFrame(QuicWindowUpdateFrame(1, 0, 0))); })); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)) - .WillOnce(Invoke([&]() { - EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address()); - EXPECT_FALSE(writer_->path_challenge_frames().empty()); - // Retry path validation shouldn't bundle ACK. - EXPECT_TRUE(writer_->ack_frames().empty()); - })) - .WillOnce(Invoke([&]() { - EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address()); - EXPECT_FALSE(writer_->ack_frames().empty()); - EXPECT_FALSE(writer_->window_update_frames().empty()); - })); - static_cast<TestAlarmFactory::TestAlarm*>( - QuicPathValidatorPeer::retry_timer( - QuicConnectionPeer::path_validator(&connection_))) - ->Fire(); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)) + .WillOnce(Invoke([&]() { + EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address()); + EXPECT_FALSE(writer_->path_challenge_frames().empty()); + // Retry path validation shouldn't bundle ACK. + EXPECT_TRUE(writer_->ack_frames().empty()); + })) + .WillOnce(Invoke([&]() { + EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address()); + EXPECT_FALSE(writer_->ack_frames().empty()); + EXPECT_FALSE(writer_->window_update_frames().empty()); + })); + static_cast<TestAlarmFactory::TestAlarm*>( + QuicPathValidatorPeer::retry_timer( + QuicConnectionPeer::path_validator(&connection_))) + ->Fire(); } TEST_P(QuicConnectionTest, DoNotIssueNewCidIfVisitorSaysNo) { @@ -15656,8 +15656,7 @@ // Send a 1-RTT packet to start the DiscardZeroRttDecryptionKeys timer. EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); ProcessDataPacketAtLevel(1, false, ENCRYPTION_FORWARD_SECURE); - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - connection_.version().UsesTls()) { + if (connection_.version().UsesTls()) { EXPECT_TRUE(connection_.GetDiscardZeroRttDecryptionKeysAlarm()->IsSet()); EXPECT_CALL(visitor_, OnServerConnectionIdRetired(original)); connection_.GetDiscardZeroRttDecryptionKeysAlarm()->Fire();
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index 74ba7d1..be05103 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -447,8 +447,7 @@ absl::optional<QuicConnectionId> QuicDispatcher::MaybeReplaceServerConnectionId( const QuicConnectionId& server_connection_id, const ParsedQuicVersion& version) { - if (GetQuicRestartFlag(quic_abstract_connection_id_generator) && - GetQuicRestartFlag(quic_map_original_connection_ids2)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { // If the Dispatcher doesn't map the original connection ID, then using a // connection ID generator that isn't deterministic may break the handshake // and will certainly drop all 0-RTT packets. @@ -622,34 +621,6 @@ packet_info.peer_address, packet_info.packet); return true; } - if (packet_info.version.IsKnown() && - !GetQuicRestartFlag(quic_map_original_connection_ids2)) { - // We did not find the connection ID, check if we've replaced it. - // This is only performed for supported versions because packets with - // unsupported versions can flow through this function in order to send - // a version negotiation packet, but we know that their connection ID - // did not get replaced since that is performed on connection creation, - // and that only happens for known verions. - // There is no need to perform this check if - // |reference_counted_session_map_| is storing original connection IDs - // separately. It can be counterproductive to do this check if that - // consumes a nonce or generates a random connection ID. - absl::optional<QuicConnectionId> replaced_connection_id = - MaybeReplaceServerConnectionId(server_connection_id, - packet_info.version); - if (replaced_connection_id.has_value()) { - // Search for the replacement. - auto it2 = reference_counted_session_map_.find(*replaced_connection_id); - if (it2 != reference_counted_session_map_.end()) { - QUICHE_DCHECK( - !buffered_packets_.HasBufferedPackets(*replaced_connection_id)); - it2->second->ProcessUdpPacket(packet_info.self_address, - packet_info.peer_address, - packet_info.packet); - return true; - } - } - } if (buffered_packets_.HasChloForConnection(server_connection_id)) { BufferEarlyPacket(packet_info); @@ -1286,10 +1257,6 @@ packets.front().self_address, packets.front().peer_address); if (session_ptr != nullptr) { DeliverPacketsToSession(packets, session_ptr.get()); - if (server_connection_id != session_ptr->connection_id() && - GetQuicRestartFlag(quic_map_original_connection_ids2)) { - QUIC_RESTART_FLAG_COUNT_N(quic_map_original_connection_ids2, 1, 4); - } } } } @@ -1359,9 +1326,6 @@ QUIC_CODE_COUNT( quic_delivered_buffered_packets_to_connection_with_replaced_id); } - if (GetQuicRestartFlag(quic_map_original_connection_ids2)) { - QUIC_RESTART_FLAG_COUNT_N(quic_map_original_connection_ids2, 2, 4); - } } // Process CHLO at first. session_ptr->ProcessUdpPacket(packet_info->self_address, @@ -1413,8 +1377,7 @@ if (!replaced_connection_id) { server_connection_id = original_connection_id; } - if (reference_counted_session_map_.count(*server_connection_id) > 0 && - GetQuicRestartFlag(quic_map_original_connection_ids2)) { + if (reference_counted_session_map_.count(*server_connection_id) > 0) { // The new connection ID is owned by another session. Avoid creating one // altogether, as this connection attempt cannot possibly succeed. if (replaced_connection_id) { @@ -1458,8 +1421,7 @@ << *server_connection_id; } else { ++num_sessions_in_session_map_; - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - replaced_connection_id) { + if (replaced_connection_id) { auto insertion_result2 = reference_counted_session_map_.insert( std::make_pair(original_connection_id, session_ptr)); QUIC_BUG_IF(quic_460317833_02, !insertion_result2.second)
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc index 74eb760..8d14a8c 100644 --- a/quiche/quic/core/quic_dispatcher_test.cc +++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -186,9 +186,6 @@ } std::vector<QuicConnectionId> GetActiveServerConnectionIds() const override { - if (!GetQuicRestartFlag(quic_map_original_connection_ids2)) { - return active_connection_ids_; - } std::vector<QuicConnectionId> result; for (const auto& cid : active_connection_ids_) { result.push_back(cid); @@ -431,7 +428,6 @@ const QuicConnectionId& client_connection_id, std::unique_ptr<QuicCryptoClientConfig> client_crypto_config) { if (expect_generator_is_called_ && - GetQuicRestartFlag(quic_map_original_connection_ids2) && GetQuicRestartFlag(quic_abstract_connection_id_generator)) { // Can't replace the connection ID in early gQUIC versions! ASSERT_TRUE(version.AllowsVariableLengthConnectionIds() || @@ -593,8 +589,7 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); QuicConnectionId original_connection_id, new_connection_id; if (long_connection_id) { - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { original_connection_id = TestConnectionIdNineBytesLong(1); new_connection_id = return_connection_id_; EXPECT_CALL(connection_id_generator_, @@ -609,8 +604,7 @@ } else { original_connection_id = TestConnectionId(); new_connection_id = original_connection_id; - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(original_connection_id, version_)) .WillOnce(Return(absl::nullopt)); @@ -1106,8 +1100,7 @@ QuicConnectionId bad_connection_id = TestConnectionIdNineBytesLong(2); generated_connection_id_ = - (GetQuicRestartFlag(quic_abstract_connection_id_generator) && - GetQuicRestartFlag(quic_map_original_connection_ids2)) + (GetQuicRestartFlag(quic_abstract_connection_id_generator)) ? return_connection_id_ : QuicUtils::CreateReplacementConnectionId(bad_connection_id); @@ -1138,8 +1131,7 @@ QuicConnectionId bad_connection_id = EmptyQuicConnectionId(); generated_connection_id_ = - (GetQuicRestartFlag(quic_abstract_connection_id_generator) && - GetQuicRestartFlag(quic_map_original_connection_ids2)) + GetQuicRestartFlag(quic_abstract_connection_id_generator) ? return_connection_id_ : QuicUtils::CreateReplacementConnectionId(bad_connection_id); @@ -1189,8 +1181,7 @@ ProcessFirstFlight(client_address, TestConnectionId(1)); generated_connection_id_ = - (GetQuicRestartFlag(quic_abstract_connection_id_generator) && - GetQuicRestartFlag(quic_map_original_connection_ids2)) + GetQuicRestartFlag(quic_abstract_connection_id_generator) ? return_connection_id_ : QuicUtils::CreateReplacementConnectionId(bad_connection_id); EXPECT_CALL(*dispatcher_, @@ -2447,8 +2438,7 @@ // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(conn_id, version_)) .WillOnce(Return(absl::nullopt)); @@ -2486,8 +2476,7 @@ data_connection_map_[conn_id].pop_back(); // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(conn_id, version_)) .WillOnce(Return(absl::nullopt)); @@ -2537,8 +2526,7 @@ for (size_t i = 1; i <= kNumConnections; ++i) { QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 20000 + i); QuicConnectionId conn_id = TestConnectionId(i); - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(conn_id, version_)) .WillOnce(Return(absl::nullopt)); @@ -2589,8 +2577,7 @@ // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(conn_id, version_)) .WillOnce(Return(absl::nullopt)); @@ -2660,8 +2647,7 @@ kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore + 1; for (uint64_t conn_id = 1; conn_id <= kNumCHLOs; ++conn_id) { if (conn_id <= kMaxNumSessionsToCreate) { - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL( connection_id_generator_, MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) @@ -2704,8 +2690,7 @@ for (uint64_t conn_id = kMaxNumSessionsToCreate + 1; conn_id <= kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore; ++conn_id) { - if (GetQuicRestartFlag(quic_map_original_connection_ids2) && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { EXPECT_CALL(connection_id_generator_, MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) .WillOnce(Return(absl::nullopt));
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 19dea8c..615ccf4 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -89,8 +89,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_connection_uses_abstract_connection_id_generator, true) // QuicDispatcher uses a library to generate connection IDs QUIC_FLAG(quic_restart_flag_quic_abstract_connection_id_generator, true) -// Store original QUIC connection IDs in the dispatcher\'s map -QUIC_FLAG(quic_restart_flag_quic_map_original_connection_ids2, true) // When true, defaults to BBR congestion control instead of Cubic. QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr, false) // When true, prevents QUIC\'s PacingSender from generating bursts when the congestion controller is CWND limited and not pacing limited.