Deprecate gfe2_restart_flag_quic_allow_smaller_packets. PiperOrigin-RevId: 516301763
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 7a96366..09b3b4b 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -57,8 +57,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_remove_connection_migration_connection_option_v2, false) // If true, include stream information in idle timeout connection close detail. QUIC_FLAG(quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true) -// If true, lowers the minimum packet size to that in the spec. -QUIC_FLAG(quic_restart_flag_quic_allow_smaller_packets, true) // If true, quic server will send ENABLE_CONNECT_PROTOCOL setting and and endpoint will validate required request/response headers and extended CONNECT mechanism and update code counts of valid/invalid headers. QUIC_FLAG(quic_reloadable_flag_quic_verify_request_headers_2, true) // If true, reject or send error response code upon receiving invalid request or response headers. This flag depends on --gfe2_reloadable_flag_quic_verify_request_headers_2.
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc index 5599f8f..c04a418 100644 --- a/quiche/quic/core/quic_packet_creator.cc +++ b/quiche/quic/core/quic_packet_creator.cc
@@ -630,16 +630,6 @@ MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()); if (plaintext_bytes_written < min_plaintext_size) { needs_padding = true; - if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { - // Recalculate sizes with the stream frame not being marked as the last - // frame in the packet. - min_frame_size = QuicFramer::GetMinStreamFrameSize( - framer_->transport_version(), id, stream_offset, - /* last_frame_in_packet= */ false, remaining_data_size); - available_size = max_plaintext_size_ - writer.length() - min_frame_size; - bytes_consumed = std::min<size_t>(available_size, remaining_data_size); - plaintext_bytes_written = min_frame_size + bytes_consumed; - } } const bool set_fin = fin && (bytes_consumed == remaining_data_size); @@ -653,8 +643,7 @@ // TODO(ianswett): AppendTypeByte and AppendStreamFrame could be optimized // into one method that takes a QuicStreamFrame, if warranted. - if (needs_padding && GetQuicRestartFlag(quic_allow_smaller_packets)) { - QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 2, 5); + if (needs_padding) { if (!writer.WritePaddingBytes(min_plaintext_size - plaintext_bytes_written)) { QUIC_BUG(quic_bug_10752_12) << ENDPOINT << "Unable to add padding bytes"; @@ -765,13 +754,6 @@ size_t QuicPacketCreator::BytesFreeForPadding() const { size_t consumed = PacketSize(); - - if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { - consumed += ExpansionOnNewFrame(); - } else { - // The next frame (which is PADDING) will be prepended to the packet. - QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 5, 5); - } return max_plaintext_size_ - std::min(max_plaintext_size_, consumed); } @@ -1865,20 +1847,9 @@ MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength())) { return; } - QuicByteCount min_header_protection_padding; - if (GetQuicRestartFlag(quic_allow_smaller_packets)) { - QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 4, 5); - min_header_protection_padding = - MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()) - - frame_bytes; - } else { - min_header_protection_padding = - std::max(1 + ExpansionOnNewFrame(), - MinPlaintextPacketSize(framer_->version(), - GetPacketNumberLength()) - - frame_bytes) - - ExpansionOnNewFrame(); - } + QuicByteCount min_header_protection_padding = + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()) - + frame_bytes; // Update pending_padding_bytes_. pending_padding_bytes_ = std::max(pending_padding_bytes_, min_header_protection_padding); @@ -1966,11 +1937,9 @@ pending_padding_bytes_ -= padding_bytes; } - if (!queued_frames_.empty() && - GetQuicRestartFlag(quic_allow_smaller_packets)) { + if (!queued_frames_.empty()) { // Insert PADDING before the other frames to avoid adding a length field // to any trailing STREAM frame. - QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 3, 5); if (needs_full_padding_) { padding_bytes = BytesFreeForPadding(); } @@ -2157,11 +2126,7 @@ // 1.3 is used, unittests still use NullEncrypter/NullDecrypter (and other // test crypters) which also only use 12 byte tags. // - if (GetQuicRestartFlag(quic_allow_smaller_packets)) { - QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 1, 5); - return (version.UsesTls() ? 4 : 8) - packet_number_length; - } - return 7; + return (version.UsesTls() ? 4 : 8) - packet_number_length; } QuicPacketNumber QuicPacketCreator::NextSendingPacketNumber() const {
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc index 63a2dc1..f1aa297 100644 --- a/quiche/quic/core/quic_packet_creator_test.cc +++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -307,10 +307,7 @@ } frames_.clear(); ASSERT_GT(payload_len, 0); // Must have a frame! - size_t min_payload = - (version.UsesTls() && GetQuicRestartFlag(quic_allow_smaller_packets)) - ? 3 - : 7; + size_t min_payload = version.UsesTls() ? 3 : 7; bool need_padding = (version.HasHeaderProtection() && (payload_len < min_payload)); { @@ -320,7 +317,7 @@ EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - if (need_padding && GetQuicRestartFlag(quic_allow_smaller_packets)) { + if (need_padding) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } if (has_ack) { @@ -335,9 +332,6 @@ if (has_stream) { EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } - if (need_padding && !GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - } EXPECT_CALL(framer_visitor_, OnPacketComplete()); } ProcessPacket(serialized); @@ -497,12 +491,9 @@ overhead += QuicFramer::GetMinCryptoFrameSize(kOffset, kMaxOutgoingPacketSize); } else { - overhead += - GetQuicRestartFlag(quic_allow_smaller_packets) - ? QuicFramer::GetMinStreamFrameSize( - client_framer_.transport_version(), - GetNthClientInitiatedStreamId(1), kOffset, false, 0) - : GetStreamFrameOverhead(client_framer_.transport_version()); + overhead += QuicFramer::GetMinStreamFrameSize( + client_framer_.transport_version(), GetNthClientInitiatedStreamId(1), + kOffset, false, 0); } ASSERT_GT(kMaxOutgoingPacketSize, overhead); size_t capacity = kDefaultMaxPacketSize - overhead; @@ -540,9 +531,7 @@ // (1 byte) and to expand the stream frame (another 2 bytes) the packet // will not be padded. // Padding is skipped when we try to send coalesced packets. - if ((!GetQuicRestartFlag(quic_allow_smaller_packets) && bytes_free < 3 && - !QuicVersionUsesCryptoFrames(client_framer_.transport_version())) || - client_framer_.version().CanSendCoalescedPackets()) { + if (client_framer_.version().CanSendCoalescedPackets()) { EXPECT_EQ(kDefaultMaxPacketSize - bytes_free, serialized_packet_->encrypted_length); } else { @@ -1556,13 +1545,8 @@ EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); if (client_framer_.version().HasHeaderProtection()) { - if (GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - } else { - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - } + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); + EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } else { EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } @@ -1685,14 +1669,13 @@ // 1. QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( client_framer_.transport_version(), Perspective::IS_CLIENT); - size_t length = GetPacketHeaderOverhead(client_framer_.transport_version()) + - GetEncryptionOverhead() + - QuicFramer::GetMinStreamFrameSize( - client_framer_.transport_version(), stream_id, 0, - /*last_frame_in_packet=*/ - GetQuicRestartFlag(quic_allow_smaller_packets), - kStreamFramePayloadSize + 1) + - kStreamFramePayloadSize + 1; + size_t length = + GetPacketHeaderOverhead(client_framer_.transport_version()) + + GetEncryptionOverhead() + + QuicFramer::GetMinStreamFrameSize( + client_framer_.transport_version(), stream_id, 0, + /*last_frame_in_packet=*/true, kStreamFramePayloadSize + 1) + + kStreamFramePayloadSize + 1; creator_.SetMaxPacketLength(length); creator_.AddPendingPadding(kMaxNumRandomPaddingBytes); QuicByteCount pending_padding_bytes = creator_.pending_padding_bytes(); @@ -2145,15 +2128,10 @@ EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)) .WillOnce(DoAll(SaveArg<0>(&header), Return(true))); - if (client_framer_.version().HasHeaderProtection() && - GetQuicRestartFlag(quic_allow_smaller_packets)) { + if (client_framer_.version().HasHeaderProtection()) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } EXPECT_CALL(framer_visitor_, OnPingFrame(_)); - if (client_framer_.version().HasHeaderProtection() && - !GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - } EXPECT_CALL(framer_visitor_, OnPacketComplete()); } ProcessPacket(serialized); @@ -2329,8 +2307,7 @@ EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); if (i != ENCRYPTION_ZERO_RTT) { - if (GetQuicRestartFlag(quic_allow_smaller_packets) && - i != ENCRYPTION_INITIAL) { + if (i != ENCRYPTION_INITIAL) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)) .Times(testing::AtMost(1)); } @@ -2340,26 +2317,17 @@ OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2))) .WillOnce(Return(true)); EXPECT_CALL(framer_visitor_, OnAckFrameEnd(_)).WillOnce(Return(true)); - if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)) - .Times(testing::AtMost(1)); - } } if (i == ENCRYPTION_INITIAL) { // Verify padding is added. EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } - if (GetQuicRestartFlag(quic_allow_smaller_packets) && - i == ENCRYPTION_ZERO_RTT) { + if (i == ENCRYPTION_ZERO_RTT) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } if (i != ENCRYPTION_INITIAL && i != ENCRYPTION_HANDSHAKE) { EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } - if (!GetQuicRestartFlag(quic_allow_smaller_packets) && - i == ENCRYPTION_ZERO_RTT) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - } EXPECT_CALL(framer_visitor_, OnPacketComplete()); server_framer_.ProcessPacket(*packets[i]); } @@ -2472,8 +2440,6 @@ PACKET_3BYTE_PACKET_NUMBER, PACKET_4BYTE_PACKET_NUMBER}) { if (!version.HasHeaderProtection()) { EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), 0); - } else if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), 7); } else { EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), (version.UsesTls() ? 4 : 8) - pn_length); @@ -2507,13 +2473,7 @@ EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); creator_.FlushCurrentPacket(); - /* Without the fix, the packet is not full-length. */ - if (GetQuicRestartFlag(quic_allow_smaller_packets)) { - EXPECT_EQ(serialized_packet_->encrypted_length, kDefaultMaxPacketSize); - } else { - EXPECT_EQ(serialized_packet_->encrypted_length, - kDefaultMaxPacketSize - bytes_free); - } + EXPECT_EQ(serialized_packet_->encrypted_length, kDefaultMaxPacketSize); DeleteSerializedPacket(); } } @@ -3074,10 +3034,6 @@ // frames, so the expected packet length differs slightly. expected_packet_length = 32; } - if (framer_.version().HasHeaderProtection() && - !GetQuicRestartFlag(quic_allow_smaller_packets)) { - expected_packet_length = 33; - } EXPECT_EQ(expected_packet_length, packets_[0].encrypted_length); } @@ -4035,8 +3991,7 @@ ASSERT_FALSE(packets_[0].nonretransmittable_frames.empty()); QuicFrame padding = packets_[0].nonretransmittable_frames[0]; // Verify stream frame expansion is excluded. - EXPECT_EQ(padding.padding_frame.num_padding_bytes, - GetQuicRestartFlag(quic_allow_smaller_packets) ? 1 : 4); + EXPECT_EQ(padding.padding_frame.num_padding_bytes, 1); } TEST_F(QuicPacketCreatorMultiplePacketsTest,