Allow QUIC control frames to be sent while a packet is being processed.
Removes the restriction added in cr/574987377. Instead, changes the
behavior of QuicPacketCreator::SetMaxPacketLength() to allow it
to be called while frames are queued. In this case, the new max packet
length is applied to the *next* packet.
Note: uses a restart flag instead of a reloadable flag to avoid needing to
coordinate the change in QuicSession with the change in QuicPacketCreator.
Protected by FLAGS_quic_restart_flag_quic_allow_control_frames_while_procesing.
PiperOrigin-RevId: 588249854
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index dc53509..d2a4308 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -47,6 +47,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_allow_client_enabled_bbr_v2, true)
// If true, allow quic to use new ALPS codepoint to negotiate during handshake for H3 if client sends new ALPS codepoint.
QUIC_FLAG(quic_reloadable_flag_quic_gfe_allow_alps_new_codepoint, false)
+// If true, allows QUIC control frames to be written while a packet is being processed.
+QUIC_FLAG(quic_restart_flag_quic_allow_control_frames_while_procesing, false)
// If true, always bundle qpack decoder data with other frames opportunistically.
QUIC_FLAG(quic_restart_flag_quic_opport_bundle_qpack_decoder_data2, false)
// If true, an endpoint does not detect path degrading or blackholing until handshake gets confirmed.
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc
index 793f417..ed6e94b 100644
--- a/quiche/quic/core/quic_packet_creator.cc
+++ b/quiche/quic/core/quic_packet_creator.cc
@@ -117,6 +117,7 @@
random_(random),
have_diversification_nonce_(false),
max_packet_length_(0),
+ next_max_packet_length_(0),
server_connection_id_included_(CONNECTION_ID_PRESENT),
packet_size_(0),
server_connection_id_(server_connection_id),
@@ -155,8 +156,18 @@
}
void QuicPacketCreator::SetMaxPacketLength(QuicByteCount length) {
- QUICHE_DCHECK(CanSetMaxPacketLength()) << ENDPOINT;
-
+ if (!GetQuicRestartFlag(quic_allow_control_frames_while_procesing)) {
+ QUICHE_DCHECK(CanSetMaxPacketLength()) << ENDPOINT;
+ } else {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_control_frames_while_procesing, 2, 3);
+ if (!CanSetMaxPacketLength()) {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_control_frames_while_procesing, 3,
+ 3);
+ // The new max packet length will be applied to the next packet.
+ next_max_packet_length_ = length;
+ return;
+ }
+ }
// Avoid recomputing |max_plaintext_size_| if the length does not actually
// change.
if (length == max_packet_length_) {
@@ -473,6 +484,11 @@
ClearPacket();
RemoveSoftMaxPacketLength();
delegate_->OnSerializedPacket(std::move(packet));
+ if (next_max_packet_length_ != 0) {
+ QUICHE_DCHECK(CanSetMaxPacketLength()) << ENDPOINT;
+ SetMaxPacketLength(next_max_packet_length_);
+ next_max_packet_length_ = 0;
+ }
}
void QuicPacketCreator::ClearPacket() {
diff --git a/quiche/quic/core/quic_packet_creator.h b/quiche/quic/core/quic_packet_creator.h
index bb2303e..1a4e725 100644
--- a/quiche/quic/core/quic_packet_creator.h
+++ b/quiche/quic/core/quic_packet_creator.h
@@ -623,6 +623,9 @@
DiversificationNonce diversification_nonce_;
// Maximum length including headers and encryption (UDP payload length.)
QuicByteCount max_packet_length_;
+ // Value of max_packet_length_ to be applied for the next packet, if not 0.
+ QuicByteCount next_max_packet_length_;
+
size_t max_plaintext_size_;
// Whether the server_connection_id is sent over the wire.
QuicConnectionIdIncluded server_connection_id_included_;
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc
index e1510f5..573e4ce 100644
--- a/quiche/quic/core/quic_packet_creator_test.cc
+++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -356,6 +356,76 @@
ProcessPacket(serialized);
}
+TEST_P(QuicPacketCreatorTest, SerializePacketWithPadding) {
+ creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
+
+ creator_.AddFrame(QuicFrame(QuicWindowUpdateFrame()), NOT_RETRANSMISSION);
+ creator_.AddFrame(QuicFrame(QuicPaddingFrame()), NOT_RETRANSMISSION);
+ EXPECT_CALL(delegate_, OnSerializedPacket(_))
+ .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
+ creator_.FlushCurrentPacket();
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+
+ EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_->encrypted_length);
+
+ DeleteSerializedPacket();
+}
+
+TEST_P(QuicPacketCreatorTest, SerializeLargerPacketWithPadding) {
+ creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
+ const QuicByteCount packet_size = 100 + kDefaultMaxPacketSize;
+ creator_.SetMaxPacketLength(packet_size);
+
+ creator_.AddFrame(QuicFrame(QuicWindowUpdateFrame()), NOT_RETRANSMISSION);
+ creator_.AddFrame(QuicFrame(QuicPaddingFrame()), NOT_RETRANSMISSION);
+ EXPECT_CALL(delegate_, OnSerializedPacket(_))
+ .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
+ creator_.FlushCurrentPacket();
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+
+ EXPECT_EQ(packet_size, serialized_packet_->encrypted_length);
+
+ DeleteSerializedPacket();
+}
+
+TEST_P(QuicPacketCreatorTest, IncreaseMaxPacketLengthWithFramesPending) {
+ if (!GetQuicRestartFlag(quic_allow_control_frames_while_procesing)) {
+ // When this flag is not set, the call to SetMaxPacketLength()
+ // is an error which triggers a QUICHE_DCHECK.
+ return;
+ }
+
+ creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
+ const QuicByteCount packet_size = 100 + kDefaultMaxPacketSize;
+
+ // Since the creator has a frame queued, the packet size will not change.
+ creator_.AddFrame(QuicFrame(QuicWindowUpdateFrame()), NOT_RETRANSMISSION);
+ creator_.SetMaxPacketLength(packet_size);
+ creator_.AddFrame(QuicFrame(QuicPaddingFrame()), NOT_RETRANSMISSION);
+ EXPECT_CALL(delegate_, OnSerializedPacket(_))
+ .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
+ creator_.FlushCurrentPacket();
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+
+ EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_->encrypted_length);
+
+ DeleteSerializedPacket();
+
+ // Now that the previous packet was generated, the next on will use
+ // the new larger size.
+ creator_.AddFrame(QuicFrame(QuicWindowUpdateFrame()), NOT_RETRANSMISSION);
+ creator_.AddFrame(QuicFrame(QuicPaddingFrame()), NOT_RETRANSMISSION);
+ EXPECT_CALL(delegate_, OnSerializedPacket(_))
+ .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
+ creator_.FlushCurrentPacket();
+ ASSERT_TRUE(serialized_packet_->encrypted_buffer);
+ EXPECT_EQ(packet_size, serialized_packet_->encrypted_length);
+
+ EXPECT_EQ(packet_size, serialized_packet_->encrypted_length);
+
+ DeleteSerializedPacket();
+}
+
TEST_P(QuicPacketCreatorTest, ConsumeCryptoDataToFillCurrentPacket) {
std::string data = "crypto data";
QuicFrame frame;
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index a6e8ed5..4a805aa 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -880,11 +880,15 @@
// Suppress the write before encryption gets established.
return false;
}
- if (limit_sending_max_streams_ &&
- connection_->framer().is_processing_packet()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_limit_sending_max_streams2, 3, 3);
- // The frame will be sent when OnCanWrite() is called.
- return false;
+ if (GetQuicRestartFlag(quic_allow_control_frames_while_procesing)) {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_control_frames_while_procesing, 1, 3);
+ } else {
+ if (limit_sending_max_streams_ &&
+ connection_->framer().is_processing_packet()) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_limit_sending_max_streams2, 3, 3);
+ // The frame will be sent when OnCanWrite() is called.
+ return false;
+ }
}
SetTransmissionType(type);
QuicConnection::ScopedEncryptionLevelContext context(
