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.
};
