gfe-relnote: Do not flush clear coalesced packets while sending multiple connection close packets. Protected by --gfe2_reloadable_flag_quic_close_all_encryptions_levels2
PiperOrigin-RevId: 282844268
Change-Id: I005056ad594f776a9a36198d012f7169737622b3
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index ed1fac4..9ecfa94 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2964,7 +2964,7 @@
void QuicConnection::SendConnectionClosePacket(QuicErrorCode error,
const std::string& details) {
- if (!GetQuicReloadableFlag(quic_close_all_encryptions_levels)) {
+ if (!GetQuicReloadableFlag(quic_close_all_encryptions_levels2)) {
QUIC_DLOG(INFO) << ENDPOINT << "Sending connection close packet.";
SetDefaultEncryptionLevel(GetConnectionCloseEncryptionLevel());
if (version().CanSendCoalescedPackets()) {
@@ -2994,7 +2994,15 @@
}
const EncryptionLevel current_encryption_level = encryption_level_;
ScopedPacketFlusher flusher(this);
- QUIC_RELOADABLE_FLAG_COUNT(quic_close_all_encryptions_levels);
+ QUIC_RELOADABLE_FLAG_COUNT(quic_close_all_encryptions_levels2);
+
+ // Now that the connection is being closed, discard any unsent packets
+ // so the only packets to be sent will be connection close packets.
+ if (version().CanSendCoalescedPackets()) {
+ coalesced_packet_.Clear();
+ }
+ ClearQueuedPackets();
+
for (EncryptionLevel level :
{ENCRYPTION_INITIAL, ENCRYPTION_HANDSHAKE, ENCRYPTION_ZERO_RTT,
ENCRYPTION_FORWARD_SECURE}) {
@@ -3004,10 +3012,6 @@
QUIC_DLOG(INFO) << ENDPOINT << "Sending connection close packet at level: "
<< EncryptionLevelToString(level);
SetDefaultEncryptionLevel(level);
- if (version().CanSendCoalescedPackets()) {
- coalesced_packet_.Clear();
- }
- ClearQueuedPackets();
// If there was a packet write error, write the smallest close possible.
// When multiple packet number spaces are supported, an ACK frame will
// be bundled by the ScopedPacketFlusher. Otherwise, an ACK must be sent
@@ -3022,14 +3026,13 @@
framer_.current_received_frame_type());
packet_creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame));
packet_creator_.FlushCurrentPacket();
- if (!version().CanSendCoalescedPackets()) {
- ClearQueuedPackets();
- }
}
if (version().CanSendCoalescedPackets()) {
FlushCoalescedPacket();
- ClearQueuedPackets();
}
+ // Since the connection is closing, if the connection close packets were not
+ // sent, then they should be discarded.
+ ClearQueuedPackets();
SetDefaultEncryptionLevel(current_encryption_level);
}
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 0c6c542..37d3d36 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -337,6 +337,7 @@
final_bytes_of_previous_packet_(0),
use_tagging_decrypter_(false),
packets_write_attempts_(0),
+ connection_close_packets_(0),
clock_(clock),
write_pause_time_delta_(QuicTime::Delta::Zero()),
max_packet_size_(kMaxOutgoingPacketSize),
@@ -405,7 +406,9 @@
last_packet_size_ = packet.length();
last_packet_header_ = framer_.header();
-
+ if (!framer_.connection_close_frames().empty()) {
+ ++connection_close_packets_;
+ }
if (!write_pause_time_delta_.IsZero()) {
clock_->AdvanceTime(write_pause_time_delta_);
}
@@ -553,6 +556,10 @@
uint32_t packets_write_attempts() { return packets_write_attempts_; }
+ uint32_t connection_close_packets() const {
+ return connection_close_packets_;
+ }
+
void Reset() { framer_.Reset(); }
void SetSupportedVersions(const ParsedQuicVersionVector& versions) {
@@ -586,6 +593,7 @@
uint32_t final_bytes_of_previous_packet_;
bool use_tagging_decrypter_;
uint32_t packets_write_attempts_;
+ uint32_t connection_close_packets_;
MockClock* clock_;
// If non-zero, the clock will pause during WritePacket for this amount of
// time.
@@ -8044,7 +8052,7 @@
ProcessAckPacket(&ack);
}
-TEST_P(QuicConnectionTest, DonotForceSendingAckOnPacketTooLarge) {
+TEST_P(QuicConnectionTest, DoNotForceSendingAckOnPacketTooLarge) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
// Send an ack by simulating delayed ack alarm firing.
ProcessPacket(1);
@@ -8055,13 +8063,74 @@
EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
SimulateNextPacketTooLarge();
connection_.SendStreamDataWithString(3, "foo", 0, NO_FIN);
- EXPECT_EQ(1u, writer_->frame_count());
- EXPECT_FALSE(writer_->connection_close_frames().empty());
+ EXPECT_EQ(1u, writer_->connection_close_frames().size());
// Ack frame is not bundled in connection close packet.
EXPECT_TRUE(writer_->ack_frames().empty());
+ if (writer_->padding_frames().empty()) {
+ EXPECT_EQ(1u, writer_->frame_count());
+ } else {
+ EXPECT_EQ(2u, writer_->frame_count());
+ }
+
TestConnectionCloseQuicErrorCode(QUIC_PACKET_WRITE_ERROR);
}
+TEST_P(QuicConnectionTest, CloseConnectionAllLevels) {
+ SetQuicReloadableFlag(quic_close_all_encryptions_levels2, true);
+
+ EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
+ const QuicErrorCode kQuicErrorCode = QUIC_INTERNAL_ERROR;
+ connection_.CloseConnection(
+ kQuicErrorCode, "Some random error message",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+
+ EXPECT_EQ(2u, QuicConnectionPeer::GetNumEncryptionLevels(&connection_));
+
+ TestConnectionCloseQuicErrorCode(kQuicErrorCode);
+ EXPECT_EQ(1u, writer_->connection_close_frames().size());
+
+ if (!connection_.version().CanSendCoalescedPackets()) {
+ // Each connection close packet should be sent in distinct UDP packets.
+ EXPECT_EQ(QuicConnectionPeer::GetNumEncryptionLevels(&connection_),
+ writer_->connection_close_packets());
+ EXPECT_EQ(QuicConnectionPeer::GetNumEncryptionLevels(&connection_),
+ writer_->packets_write_attempts());
+ return;
+ }
+
+ // A single UDP packet should be sent with multiple connection close packets
+ // coalesced together.
+ EXPECT_EQ(1u, writer_->packets_write_attempts());
+
+ // Only the first packet has been processed yet.
+ EXPECT_EQ(1u, writer_->connection_close_packets());
+
+ // ProcessPacket resets the visitor and frees the coalesced packet.
+ ASSERT_TRUE(writer_->coalesced_packet() != nullptr);
+ auto packet = writer_->coalesced_packet()->Clone();
+ writer_->framer()->ProcessPacket(*packet);
+ EXPECT_EQ(1u, writer_->connection_close_packets());
+ ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
+}
+
+TEST_P(QuicConnectionTest, CloseConnectionOneLevel) {
+ SetQuicReloadableFlag(quic_close_all_encryptions_levels2, false);
+
+ EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
+ const QuicErrorCode kQuicErrorCode = QUIC_INTERNAL_ERROR;
+ connection_.CloseConnection(
+ kQuicErrorCode, "Some random error message",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+
+ EXPECT_EQ(2u, QuicConnectionPeer::GetNumEncryptionLevels(&connection_));
+
+ TestConnectionCloseQuicErrorCode(kQuicErrorCode);
+ EXPECT_EQ(1u, writer_->connection_close_frames().size());
+ EXPECT_EQ(1u, writer_->connection_close_packets());
+ EXPECT_EQ(1u, writer_->packets_write_attempts());
+ ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
+}
+
// Regression test for b/63620844.
TEST_P(QuicConnectionTest, FailedToWriteHandshakePacket) {
SimulateNextPacketTooLarge();
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 79fc7e3..7b9264c 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -333,5 +333,18 @@
connection->SendConnectionClosePacket(error, details);
}
+// static
+size_t QuicConnectionPeer::GetNumEncryptionLevels(QuicConnection* connection) {
+ size_t count = 0;
+ for (EncryptionLevel level :
+ {ENCRYPTION_INITIAL, ENCRYPTION_HANDSHAKE, ENCRYPTION_ZERO_RTT,
+ ENCRYPTION_FORWARD_SECURE}) {
+ if (connection->framer_.HasEncrypterOfEncryptionLevel(level)) {
+ ++count;
+ }
+ }
+ return count;
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h
index 3ded2e0..b7140de 100644
--- a/quic/test_tools/quic_connection_peer.h
+++ b/quic/test_tools/quic_connection_peer.h
@@ -133,6 +133,8 @@
static void SendConnectionClosePacket(QuicConnection* connection,
QuicErrorCode error,
const std::string& details);
+
+ static size_t GetNumEncryptionLevels(QuicConnection* connection);
};
} // namespace test
