Deprecate gfe2_reloadable_flag_quic_fix_missing_initial_keys2. PiperOrigin-RevId: 346103987 Change-Id: I9850a26dad6de632cb7b0f36ae0358e5b4d977c0
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index b6cff48..5d90546 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2636,9 +2636,6 @@ void QuicConnection::NeuterUnencryptedPackets() { sent_packet_manager_.NeuterUnencryptedPackets(); - if (!fix_missing_initial_keys_ && version().CanSendCoalescedPackets()) { - coalesced_packet_.NeuterInitialPacket(); - } // This may have changed the retransmission timer, so re-arm it. SetRetransmissionAlarm(); if (default_enable_5rto_blackhole_detection_) { @@ -2865,19 +2862,6 @@ // Failed to flush coalesced packet, write error has been handled. return false; } - if (!fix_missing_initial_keys_ && - GetQuicReloadableFlag( - quic_discard_initial_packet_with_key_dropped)) { - QUIC_RELOADABLE_FLAG_COUNT( - quic_discard_initial_packet_with_key_dropped); - if (packet->encryption_level == ENCRYPTION_INITIAL && - !framer_.HasEncrypterOfEncryptionLevel(ENCRYPTION_INITIAL)) { - // Discard initial packet since flush of coalesce packet could - // cause initial keys to be dropped. - ++stats_.packets_discarded; - return true; - } - } if (!coalesced_packet_.MaybeCoalescePacket( *packet, self_address(), send_to_address, helper_->GetStreamSendBufferAllocator(), @@ -4235,9 +4219,7 @@ connection_->FlushCoalescedPacket(); } connection_->FlushPackets(); - if (connection_->fix_missing_initial_keys_ && !handshake_packet_sent_ && - connection_->handshake_packet_sent_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_fix_missing_initial_keys2, 1, 2); + if (!handshake_packet_sent_ && connection_->handshake_packet_sent_) { // This would cause INITIAL key to be dropped. Drop keys here to avoid // missing the write keys in the middle of writing. connection_->visitor_->OnHandshakePacketSent(); @@ -5049,17 +5031,14 @@ QUIC_BUG_IF(coalesced_packet_.length() > 0); return true; } - if (fix_missing_initial_keys_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_fix_missing_initial_keys2, 2, 2); - if (coalesced_packet_.ContainsPacketOfEncryptionLevel(ENCRYPTION_INITIAL) && - !framer_.HasEncrypterOfEncryptionLevel(ENCRYPTION_INITIAL)) { - // Initial packet will be re-serialized. Neuter it in case initial key has - // been dropped. - QUIC_BUG << ENDPOINT - << "Coalescer contains initial packet while initial key has " - "been dropped."; - coalesced_packet_.NeuterInitialPacket(); - } + if (coalesced_packet_.ContainsPacketOfEncryptionLevel(ENCRYPTION_INITIAL) && + !framer_.HasEncrypterOfEncryptionLevel(ENCRYPTION_INITIAL)) { + // Initial packet will be re-serialized. Neuter it in case initial key has + // been dropped. + QUIC_BUG << ENDPOINT + << "Coalescer contains initial packet while initial key has " + "been dropped."; + coalesced_packet_.NeuterInitialPacket(); } if (coalesced_packet_.length() == 0) { return true; @@ -5083,13 +5062,6 @@ if (debug_visitor_ != nullptr) { debug_visitor_->OnCoalescedPacketSent(coalesced_packet_, length); } - if (!fix_missing_initial_keys_ && - coalesced_packet_.ContainsPacketOfEncryptionLevel( - ENCRYPTION_HANDSHAKE)) { - // This is only called in coalescer because all ENCRYPTION_HANDSHAKE - // packets go through the coalescer. - visitor_->OnHandshakePacketSent(); - } return true; } @@ -5113,12 +5085,6 @@ if (debug_visitor_ != nullptr) { debug_visitor_->OnCoalescedPacketSent(coalesced_packet_, length); } - if (!fix_missing_initial_keys_ && - coalesced_packet_.ContainsPacketOfEncryptionLevel(ENCRYPTION_HANDSHAKE)) { - // This is only called in coalescer because all ENCRYPTION_HANDSHAKE - // packets go through the coalescer. - visitor_->OnHandshakePacketSent(); - } // Account for added padding. if (length > coalesced_packet_.length()) { size_t padding_size = length - coalesced_packet_.length();
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index c59b040..80c70eb 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1959,9 +1959,6 @@ // True after the first 1-RTT packet has successfully decrypted. bool have_decrypted_first_one_rtt_packet_ = false; - const bool fix_missing_initial_keys_ = - GetQuicReloadableFlag(quic_fix_missing_initial_keys2); - const bool encrypted_control_frames_; const bool use_encryption_level_context_;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 6f35892..b68b519 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -9845,9 +9845,6 @@ ? QuicPacketNumber(3) : QuicPacketNumber(4), _, _)); - if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } connection_.GetRetransmissionAlarm()->Fire(); // Verify 1-RTT packet gets coalesced with handshake retransmission. EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); @@ -9869,9 +9866,6 @@ OnPacketSent(_, _, handshake_retransmission + 1, _, _)); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, handshake_retransmission, _, _)); - if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } connection_.GetRetransmissionAlarm()->Fire(); // Verify 1-RTT packet gets coalesced with handshake retransmission. EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); @@ -10647,11 +10641,7 @@ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, std::make_unique<TaggingEncrypter>(0x02)); connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); - if (GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } else { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(2); - } + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE); // Verify PTO time does not change. EXPECT_EQ(expected_pto_time, @@ -10690,11 +10680,7 @@ // Receives packet 1000 in handshake data. ProcessCryptoPacketAtLevel(1000, ENCRYPTION_HANDSHAKE); EXPECT_TRUE(connection_.HasPendingAcks()); - if (GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } else { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(2); - } + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE); // Receives packet 1001 in handshake data. @@ -10757,11 +10743,7 @@ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, std::make_unique<TaggingEncrypter>(0x02)); connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); - if (GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } else { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(3); - } + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); // Send HANDSHAKE 2 and 3. connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE); connection_.SendCryptoDataWithString("bar", 3, ENCRYPTION_HANDSHAKE); @@ -10827,11 +10809,7 @@ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, std::make_unique<TaggingEncrypter>(0x02)); connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); - if (GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } else { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(2); - } + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); std::string handshake_crypto_data(1024, 'a'); connection_.SendCryptoDataWithString(handshake_crypto_data, 0, ENCRYPTION_HANDSHAKE); @@ -10903,11 +10881,7 @@ std::make_unique<TaggingEncrypter>(0x02)); connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); // Verify HANDSHAKE packet is coalesced with INITIAL retransmission. - if (GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } else { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(2); - } + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); std::string handshake_crypto_data(1024, 'a'); connection_.SendCryptoDataWithString(handshake_crypto_data, 0, ENCRYPTION_HANDSHAKE); @@ -11158,10 +11132,7 @@ // Regression test for b/166255274 TEST_P(QuicConnectionTest, ReserializeInitialPacketInCoalescerAfterDiscardingInitialKey) { - if (!connection_.version().CanSendCoalescedPackets() || - !GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - // Cannot set quic_fix_missing_initial_keys in the test since connection_ is - // created since the setup. + if (!connection_.version().CanSendCoalescedPackets()) { return; } use_tagging_decrypter(); @@ -11805,9 +11776,6 @@ connection_.GetSendAlarm()->Set(clock_.ApproximateNow()); // Fire ACK alarm. - if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } connection_.GetAckAlarm()->Fire(); if (GetQuicReloadableFlag(quic_fix_pto_pending_timer_count)) { // Verify 1-RTT packet is coalesced with handshake packet. @@ -11821,15 +11789,8 @@ connection_.GetSendAlarm()->Fire(); ASSERT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); - if (GetQuicReloadableFlag(quic_fix_pto_pending_timer_count)) { - if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } - } else if (GetQuicReloadableFlag(quic_let_connection_handle_pings)) { - if (!GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { - EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); - } - } else { + if (!GetQuicReloadableFlag(quic_fix_pto_pending_timer_count) && + !GetQuicReloadableFlag(quic_let_connection_handle_pings)) { EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(0); EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); @@ -11855,8 +11816,7 @@ // Regression test for b/168294218. TEST_P(QuicConnectionTest, CoalescerHandlesInitialKeyDiscard) { - if (!connection_.version().CanSendCoalescedPackets() || - !GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { + if (!connection_.version().CanSendCoalescedPackets()) { return; } SetQuicReloadableFlag(quic_discard_initial_packet_with_key_dropped, true); @@ -11887,8 +11847,7 @@ // Regresstion test for b/168294218 TEST_P(QuicConnectionTest, ZeroRttRejectionAndMissingInitialKeys) { - if (!connection_.SupportsMultiplePacketNumberSpaces() || - !GetQuicReloadableFlag(quic_fix_missing_initial_keys2)) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { return; } // Not defer send in response to packet.
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index cb4ec93..9eebe63 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -36,7 +36,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_server_on_wire_ping, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_control_frames, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_extract_x509_subject_using_certificate_view, true) -QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_missing_initial_keys2, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_pto_pending_timer_count, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_undecryptable_packets2, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)