Deprecate gfe2_reloadable_flag_quic_determine_serialized_packet_fate_early. PiperOrigin-RevId: 328245625 Change-Id: Ic25ebffbe1dc5f1788e5f252ec246656d7409ca4
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index db8d4e9..5f0a521 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2146,10 +2146,6 @@ max_packet_length -= minimum_overhead; } packet_creator_.SetMaxPacketLength(max_packet_length); - if (legacy_version_encapsulation_enabled_) { - packet_creator_.set_disable_padding_override( - legacy_version_encapsulation_in_progress_); - } } void QuicConnection::ProcessUdpPacket(const QuicSocketAddress& self_address, @@ -2599,11 +2595,6 @@ } bool QuicConnection::WritePacket(SerializedPacket* packet) { - if (!packet_creator_.determine_serialized_packet_fate_early() && - ShouldDiscardPacket(packet->encryption_level)) { - ++stats_.packets_discarded; - return true; - } if (sent_packet_manager_.GetLargestSentPacket().IsInitialized() && packet->packet_number < sent_packet_manager_.GetLargestSentPacket()) { QUIC_BUG << "Attempt to write packet:" << packet->packet_number @@ -2614,10 +2605,7 @@ } const bool is_mtu_discovery = QuicUtils::ContainsFrameType( packet->nonretransmittable_frames, MTU_DISCOVERY_FRAME); - const SerializedPacketFate fate = - packet_creator_.determine_serialized_packet_fate_early() - ? packet->fate - : GetSerializedPacketFate(is_mtu_discovery, packet->encryption_level); + const SerializedPacketFate fate = packet->fate; // Termination packets are encrypted and saved, so don't exit early. QuicErrorCode error_code = QUIC_NO_ERROR; const bool is_termination_packet = IsTerminationPacket(*packet, &error_code); @@ -2673,7 +2661,6 @@ WriteResult result(WRITE_STATUS_OK, encrypted_length); switch (fate) { case DISCARD: - DCHECK(packet_creator_.determine_serialized_packet_fate_early()); ++stats_.packets_discarded; return true; case COALESCE: @@ -2734,11 +2721,6 @@ result = writer_->Flush(); } break; - case FAILED_TO_WRITE_COALESCED_PACKET: - // Failed to send existing coalesced packet when determining packet fate, - // write error has been handled. - QUIC_BUG_IF(!version().CanSendCoalescedPackets()); - return false; case LEGACY_VERSION_ENCAPSULATE: { DCHECK(!is_mtu_discovery); DCHECK_EQ(perspective_, Perspective::IS_CLIENT); @@ -4654,28 +4636,18 @@ SerializedPacketFate QuicConnection::GetSerializedPacketFate( bool is_mtu_discovery, EncryptionLevel encryption_level) { - if (packet_creator_.determine_serialized_packet_fate_early()) { - if (ShouldDiscardPacket(encryption_level)) { - return DISCARD; - } + if (ShouldDiscardPacket(encryption_level)) { + return DISCARD; } if (legacy_version_encapsulation_in_progress_) { DCHECK(!is_mtu_discovery); return LEGACY_VERSION_ENCAPSULATE; } - if (version().CanSendCoalescedPackets()) { - // Disable coalescing when Legacy Version Encapsulation is in use to avoid - // having to reframe encapsulated packets. - if (!IsHandshakeConfirmed() && !is_mtu_discovery) { - // Before receiving ACK for any 1-RTT packets, always try to coalesce - // packet (except MTU discovery packet). - return COALESCE; - } - // Packet cannot be coalesced, flush existing coalesced packet. - if (!packet_creator_.determine_serialized_packet_fate_early() && - !FlushCoalescedPacket()) { - return FAILED_TO_WRITE_COALESCED_PACKET; - } + if (version().CanSendCoalescedPackets() && !IsHandshakeConfirmed() && + !is_mtu_discovery) { + // Before receiving ACK for any 1-RTT packets, always try to coalesce + // packet (except MTU discovery packet). + return COALESCE; } if (!buffered_packets_.empty() || HandleWriteBlocked()) { return BUFFER;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index b1f0e3c..3dd3cc2 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -7259,20 +7259,8 @@ ConnectionCloseBehavior::SILENT_CLOSE); EXPECT_FALSE(connection_.connected()); EXPECT_FALSE(connection_.CanWrite(HAS_RETRANSMITTABLE_DATA)); - if (GetQuicReloadableFlag(quic_determine_serialized_packet_fate_early)) { - EXPECT_EQ(DISCARD, connection_.GetSerializedPacketFate( - /*is_mtu_discovery=*/false, ENCRYPTION_INITIAL)); - return; - } - std::unique_ptr<QuicPacket> packet = - ConstructDataPacket(1, !kHasStopWaiting, ENCRYPTION_INITIAL); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(1), _, _)) - .Times(0); - connection_.SendPacket(ENCRYPTION_INITIAL, 1, std::move(packet), - HAS_RETRANSMITTABLE_DATA, false, false); - EXPECT_EQ(1, connection_close_frame_count_); - EXPECT_THAT(saved_connection_close_frame_.quic_error_code, - IsError(QUIC_PEER_GOING_AWAY)); + EXPECT_EQ(DISCARD, connection_.GetSerializedPacketFate( + /*is_mtu_discovery=*/false, ENCRYPTION_INITIAL)); } TEST_P(QuicConnectionTest, SendConnectivityProbingWhenDisconnected) {
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc index 507a87a..c7f1c22 100644 --- a/quic/core/quic_packet_creator.cc +++ b/quic/core/quic_packet_creator.cc
@@ -571,14 +571,11 @@ // Write out the packet header QuicPacketHeader header; FillPacketHeader(&header); - if (determine_serialized_packet_fate_early_) { - packet_.fate = delegate_->GetSerializedPacketFate( - /*is_mtu_discovery=*/false, packet_.encryption_level); - QUIC_DVLOG(1) << ENDPOINT << "fate of packet " << packet_.packet_number - << ": " << SerializedPacketFateToString(packet_.fate) - << " of " - << EncryptionLevelToString(packet_.encryption_level); - } + packet_.fate = delegate_->GetSerializedPacketFate( + /*is_mtu_discovery=*/false, packet_.encryption_level); + QUIC_DVLOG(1) << ENDPOINT << "fate of packet " << packet_.packet_number + << ": " << SerializedPacketFateToString(packet_.fate) << " of " + << EncryptionLevelToString(packet_.encryption_level); QUIC_CACHELINE_ALIGNED char stack_buffer[kMaxOutgoingPacketSize]; QuicOwnedPacketBuffer packet_buffer(delegate_->GetPacketBuffer()); @@ -760,8 +757,7 @@ QuicPacketHeader header; // FillPacketHeader increments packet_number_. FillPacketHeader(&header); - if (determine_serialized_packet_fate_early_ && delegate_ != nullptr) { - QUIC_RELOADABLE_FLAG_COUNT(quic_determine_serialized_packet_fate_early); + if (delegate_ != nullptr) { packet_.fate = delegate_->GetSerializedPacketFate( /*is_mtu_discovery=*/QuicUtils::ContainsFrameType(queued_frames_, MTU_DISCOVERY_FRAME), @@ -1765,36 +1761,10 @@ needs_full_padding_ = true; } - if (determine_serialized_packet_fate_early_) { - if (packet_.fate == COALESCE || - packet_.fate == LEGACY_VERSION_ENCAPSULATE) { - // Do not add full padding if the packet is going to be coalesced or - // encapsulated. - needs_full_padding_ = false; - } - } else { - // Packet coalescer pads INITIAL packets, so the creator should not. - if (framer_->version().CanSendCoalescedPackets() && - (packet_.encryption_level == ENCRYPTION_INITIAL || - packet_.encryption_level == ENCRYPTION_HANDSHAKE)) { - // TODO(fayang): MTU discovery packets should not ever be sent as - // ENCRYPTION_INITIAL or ENCRYPTION_HANDSHAKE. - bool is_mtu_discovery = false; - for (const auto& frame : packet_.nonretransmittable_frames) { - if (frame.type == MTU_DISCOVERY_FRAME) { - is_mtu_discovery = true; - break; - } - } - if (!is_mtu_discovery) { - // Do not add full padding if connection tries to coalesce packet. - needs_full_padding_ = false; - } - } - - if (disable_padding_override_) { - needs_full_padding_ = false; - } + if (packet_.fate == COALESCE || packet_.fate == LEGACY_VERSION_ENCAPSULATE) { + // Do not add full padding if the packet is going to be coalesced or + // encapsulated. + needs_full_padding_ = false; } // Header protection requires a minimum plaintext packet size.
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h index 0fe3621..7198da3 100644 --- a/quic/core/quic_packet_creator.h +++ b/quic/core/quic_packet_creator.h
@@ -465,14 +465,6 @@ // Returns true if max_packet_length_ is currently a soft value. bool HasSoftMaxPacketLength() const; - void set_disable_padding_override(bool should_disable_padding) { - disable_padding_override_ = should_disable_padding; - } - - bool determine_serialized_packet_fate_early() const { - return determine_serialized_packet_fate_early_; - } - bool coalesced_packet_of_higher_space() const { return coalesced_packet_of_higher_space_; } @@ -663,20 +655,12 @@ // negotiates this during the handshake. QuicByteCount max_datagram_frame_size_; - // When true, this will override the padding generation code to disable it. - // TODO(fayang): remove this when deprecating - // quic_determine_serialized_packet_fate_early. - bool disable_padding_override_ = false; - const bool update_packet_size_ = GetQuicReloadableFlag(quic_update_packet_size); const bool fix_extra_padding_bytes_ = GetQuicReloadableFlag(quic_fix_extra_padding_bytes); - const bool determine_serialized_packet_fate_early_ = - GetQuicReloadableFlag(quic_determine_serialized_packet_fate_early); - const bool coalesced_packet_of_higher_space_ = GetQuicReloadableFlag(quic_coalesced_packet_of_higher_space2); };
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc index 563f1eb..fcd9653 100644 --- a/quic/core/quic_packet_creator_test.cc +++ b/quic/core/quic_packet_creator_test.cc
@@ -164,10 +164,8 @@ creator_(connection_id_, &client_framer_, &delegate_, &producer_) { EXPECT_CALL(delegate_, GetPacketBuffer()) .WillRepeatedly(Return(QuicPacketBuffer())); - if (GetQuicReloadableFlag(quic_determine_serialized_packet_fate_early)) { - EXPECT_CALL(delegate_, GetSerializedPacketFate(_, _)) - .WillRepeatedly(Return(SEND_TO_WRITER)); - } + EXPECT_CALL(delegate_, GetSerializedPacketFate(_, _)) + .WillRepeatedly(Return(SEND_TO_WRITER)); creator_.SetEncrypter(ENCRYPTION_INITIAL, std::make_unique<NullEncrypter>( Perspective::IS_CLIENT)); creator_.SetEncrypter(ENCRYPTION_HANDSHAKE, std::make_unique<NullEncrypter>( @@ -510,8 +508,7 @@ EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillRepeatedly( Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - if (GetQuicReloadableFlag(quic_determine_serialized_packet_fate_early) && - client_framer_.version().CanSendCoalescedPackets()) { + if (client_framer_.version().CanSendCoalescedPackets()) { EXPECT_CALL(delegate_, GetSerializedPacketFate(_, _)) .WillRepeatedly(Return(COALESCE)); } @@ -2453,10 +2450,8 @@ ack_frame_(InitAckFrame(1)) { EXPECT_CALL(delegate_, GetPacketBuffer()) .WillRepeatedly(Return(QuicPacketBuffer())); - if (GetQuicReloadableFlag(quic_determine_serialized_packet_fate_early)) { - EXPECT_CALL(delegate_, GetSerializedPacketFate(_, _)) - .WillRepeatedly(Return(SEND_TO_WRITER)); - } + EXPECT_CALL(delegate_, GetSerializedPacketFate(_, _)) + .WillRepeatedly(Return(SEND_TO_WRITER)); creator_.SetEncrypter( ENCRYPTION_FORWARD_SECURE, std::make_unique<NullEncrypter>(Perspective::IS_CLIENT));
diff --git a/quic/core/quic_types.cc b/quic/core/quic_types.cc index fb2e9c8..4027ffa 100644 --- a/quic/core/quic_types.cc +++ b/quic/core/quic_types.cc
@@ -299,7 +299,6 @@ RETURN_STRING_LITERAL(COALESCE); RETURN_STRING_LITERAL(BUFFER); RETURN_STRING_LITERAL(SEND_TO_WRITER); - RETURN_STRING_LITERAL(FAILED_TO_WRITE_COALESCED_PACKET); RETURN_STRING_LITERAL(LEGACY_VERSION_ENCAPSULATE); default: return quiche::QuicheStrCat("Unknown(", static_cast<int>(fate), ")");
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h index 55c5046..c52293f 100644 --- a/quic/core/quic_types.h +++ b/quic/core/quic_types.h
@@ -709,10 +709,6 @@ COALESCE, // Try to coalesce packet. BUFFER, // Buffer packet in buffered_packets_. SEND_TO_WRITER, // Send packet to writer. - // TODO(fayang): remove FAILED_TO_WRITE_COALESCED_PACKET when deprecating - // quic_determine_serialized_packet_fate_early. - FAILED_TO_WRITE_COALESCED_PACKET, // Packet cannot be coalesced, error occurs - // when sending existing coalesced packet. LEGACY_VERSION_ENCAPSULATE, // Perform Legacy Version Encapsulation on this // packet. };