Bundle `ACK_FREQUENCY` in `QuicConnection::MaybeBundleOpportunistically` only if `transmission_type` is `NOT_RETRANSMISSION`. Protected by FLAGS_quic_restart_flag_quic_opport_bundle_qpack_decoder_data4 (Replaces the v3 flag). PiperOrigin-RevId: 621012797
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index fd35cc9..f106854 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -48,7 +48,7 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, "A testonly reloadable flag that will always default to false.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, "A testonly reloadable flag that will always default to true.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_received_client_addresses_cache, true, "If true, use a LRU cache to record client addresses of packets received on server's original address.") -QUICHE_FLAG(bool, quiche_restart_flag_quic_opport_bundle_qpack_decoder_data3, false, "If true, always bundle qpack decoder data with other frames opportunistically.") +QUICHE_FLAG(bool, quiche_restart_flag_quic_opport_bundle_qpack_decoder_data4, false, "If true, bundle qpack decoder data with other frames opportunistically.") QUICHE_FLAG(bool, quiche_restart_flag_quic_support_ect1, false, "When true, allows sending of QUIC packets marked ECT(1). A different flag (TBD) will actually utilize this capability to send ECT(1).") QUICHE_FLAG(bool, quiche_restart_flag_quic_support_release_time_for_gso, false, "If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so.") QUICHE_FLAG(bool, quiche_restart_flag_quic_testonly_default_false, false, "A testonly restart flag that will always default to false.")
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index e354a7f..249bc93 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -2847,7 +2847,7 @@ client_connection->GetStats().packets_sent; if (version_.UsesHttp3()) { - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // QPACK decoder instructions and RESET_STREAM and STOP_SENDING frames are // sent in a single packet. EXPECT_EQ(packets_sent_before + 1, packets_sent_now); @@ -3030,7 +3030,7 @@ return; } override_client_connection_id_length_ = kQuicDefaultConnectionIdLength; - SetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3, false); + SetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4, false); ASSERT_TRUE(Initialize()); SendSynchronousFooRequestAndCheckResponse();
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index df57d20..752f83f 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -866,7 +866,7 @@ } bool QuicSpdySession::CheckStreamWriteBlocked(QuicStream* stream) const { - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3) && + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4) && qpack_decoder_send_stream_ != nullptr && stream->id() == qpack_decoder_send_stream_->id()) { // Decoder data is always bundled opportunistically.
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc index 2dace2d..fc9ed5c 100644 --- a/quiche/quic/core/http/quic_spdy_session_test.cc +++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -1361,7 +1361,7 @@ // In HTTP/3, Qpack stream will send data on stream reset and cause packet to // be flushed. if (VersionUsesHttp3(transport_version()) && - !GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + !GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _, _)) .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0))); } @@ -1606,7 +1606,7 @@ // the STOP_SENDING, so set up the EXPECT there. EXPECT_CALL(*connection_, OnStreamReset(stream->id(), _)); EXPECT_CALL(*connection_, SendControlFrame(_)); - } else if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + } else if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _, _)) .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0))); }
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc index d693c9f..0848648 100644 --- a/quiche/quic/core/http/quic_spdy_stream_test.cc +++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -584,7 +584,7 @@ stream_->id(), QuicResetStreamError::FromInternal(QUIC_HEADERS_TOO_LARGE), 0)); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { auto qpack_decoder_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); // Stream type and stream cancellation. @@ -2256,7 +2256,7 @@ std::string headers = HeadersFrame(encoded_headers); EXPECT_CALL(debug_visitor, OnHeadersFrameReceived(stream_->id(), encoded_headers.length())); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Decoder stream type. EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), /* write_length = */ 1, @@ -2294,7 +2294,7 @@ EXPECT_CALL(debug_visitor, OnHeadersFrameReceived(stream_->id(), encoded_trailers.length())); // Header acknowledgement. - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _, _)); } @@ -2337,7 +2337,7 @@ auto decoder_send_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Decoder stream type. EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), /* write_length = */ 1, @@ -2377,7 +2377,7 @@ // Decoding is blocked because dynamic table entry has not been received yet. EXPECT_FALSE(stream_->trailers_decompressed()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Header acknowledgement. EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), _, _, _, _, _)); @@ -2478,7 +2478,7 @@ auto decoder_send_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Decoder stream type. EXPECT_CALL(*session_, WritevData(decoder_send_stream->id(), /* write_length = */ 1, @@ -2552,7 +2552,7 @@ // Decoding is blocked because dynamic table entry has not been received yet. EXPECT_FALSE(stream_->headers_decompressed()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Decoder stream type and stream cancellation instruction. auto decoder_send_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); @@ -2603,7 +2603,7 @@ // Decoding is blocked because dynamic table entry has not been received yet. EXPECT_FALSE(stream_->headers_decompressed()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Decoder stream type and stream cancellation instruction. auto decoder_send_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); @@ -3117,7 +3117,7 @@ auto qpack_decoder_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Stream type. EXPECT_CALL(*session_, WritevData(qpack_decoder_stream->id(), /* write_length = */ 1, @@ -3150,7 +3150,7 @@ auto qpack_decoder_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // Stream type. EXPECT_CALL(*session_, WritevData(qpack_decoder_stream->id(), /* write_length = */ 1, @@ -3544,7 +3544,7 @@ QuicStreamFrame frame(stream_->id(), /* fin = */ false, 0, data_frame); stream_->OnStreamFrame(frame); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { // As a result of resetting the stream, stream type and stream cancellation // are sent on the QPACK decoder stream. auto qpack_decoder_stream =
diff --git a/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc index 3b64e99..1a718a5 100644 --- a/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc +++ b/quiche/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -191,7 +191,7 @@ EXPECT_CALL(visitor_, OnHeadersDecoded(_, true)); qpack_decoder_.OnInsertWithoutNameReference("foo", "bar"); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { qpack_decoder_.FlushDecoderStream(); } } @@ -218,7 +218,7 @@ EXPECT_EQ(strlen("foo") + strlen("bar"), header_list.uncompressed_header_bytes()); EXPECT_EQ(encoded_data.size(), header_list.compressed_header_bytes()); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { qpack_decoder_.FlushDecoderStream(); } } @@ -247,7 +247,7 @@ accumulator_.EndHeaderBlock(); EXPECT_THAT(header_list, ElementsAre(Pair("foo", "bar"), Pair("foo", "bar"))); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { qpack_decoder_.FlushDecoderStream(); } }
diff --git a/quiche/quic/core/qpack/qpack_decoder.cc b/quiche/quic/core/qpack/qpack_decoder.cc index a0ce09b..02c590d 100644 --- a/quiche/quic/core/qpack/qpack_decoder.cc +++ b/quiche/quic/core/qpack/qpack_decoder.cc
@@ -31,7 +31,7 @@ void QpackDecoder::OnStreamReset(QuicStreamId stream_id) { if (header_table_.maximum_dynamic_table_capacity() > 0) { decoder_stream_sender_.SendStreamCancellation(stream_id); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { decoder_stream_sender_.Flush(); } } @@ -68,7 +68,7 @@ known_received_count_ = header_table_.inserted_entry_count(); } - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { decoder_stream_sender_.Flush(); } }
diff --git a/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc b/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc index da33bd3..98245c4 100644 --- a/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc +++ b/quiche/quic/core/qpack/qpack_decoder_stream_sender.cc
@@ -42,8 +42,8 @@ if (buffer_.empty() || delegate_ == nullptr) { return; } - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { - QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data3, 3, 4); + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { + QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data4, 3, 4); // Swap buffer_ before calling WriteStreamData, which might result in a // reentrant call to `Flush()`. std::string copy;
diff --git a/quiche/quic/core/qpack/qpack_decoder_test.cc b/quiche/quic/core/qpack/qpack_decoder_test.cc index ce37190..78fadd2 100644 --- a/quiche/quic/core/qpack/qpack_decoder_test.cc +++ b/quiche/quic/core/qpack/qpack_decoder_test.cc
@@ -420,13 +420,13 @@ .InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); } EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); @@ -443,7 +443,7 @@ // with value "ZZ". &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } @@ -453,13 +453,13 @@ .InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); } EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); @@ -476,7 +476,7 @@ // with value "ZZ". &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } @@ -486,13 +486,13 @@ .InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("ZZZ"))).InSequence(s); EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("ZZ"))).InSequence(s); - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); } EXPECT_CALL(handler_, OnDecodingCompleted()).InSequence(s); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))) .InSequence(s); @@ -509,7 +509,7 @@ // with value "ZZ". &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -550,7 +550,7 @@ "80", // Dynamic table entry with relative index 0, absolute index 0. &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -837,7 +837,7 @@ "80", // Emit dynamic table entry with relative index 0. &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -1032,7 +1032,7 @@ // Add literal entry with name "foo" and value "bar". ASSERT_TRUE(absl::HexStringToBytes("6294e703626172", &input)); DecodeEncoderStreamData(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -1079,7 +1079,7 @@ EXPECT_CALL(decoder_stream_sender_delegate_, WriteStreamData(Eq(kHeaderAcknowledgement))); EndDecoding(); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -1147,7 +1147,7 @@ // Insert Count is now 6, reaching Required Insert Count of the header block. ASSERT_TRUE(absl::HexStringToBytes("6294e70362617a", &input)); DecodeEncoderStreamData(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } } @@ -1199,7 +1199,7 @@ "80", // Dynamic table entry with relative index 0, absolute index 0. &input)); DecodeHeaderBlock(input); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { FlushDecoderStream(); } }
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 6a1e0a8..e5fcc38 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -3177,20 +3177,37 @@ return connected_ && !HandleWriteBlocked(); } -void QuicConnection::MaybeBundleOpportunistically() { - if (!ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency()) { - if (packet_creator_.NextSendingPacketNumber() >= - FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation) { +void QuicConnection::MaybeBundleOpportunistically( + TransmissionType transmission_type) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { + QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data4, 1, 4); + + const bool should_bundle_ack_frequency = + !ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency() && + transmission_type == NOT_RETRANSMISSION && + packet_creator_.NextSendingPacketNumber() >= + FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation; + + if (should_bundle_ack_frequency) { QUIC_RELOADABLE_FLAG_COUNT_N(quic_can_send_ack_frequency, 3, 3); ack_frequency_sent_ = true; auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame(); visitor_->SendAckFrequency(frame); } - } - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { - QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data3, 1, 4); - visitor_->MaybeBundleOpportunistically(); + if (transmission_type == NOT_RETRANSMISSION) { + visitor_->MaybeBundleOpportunistically(); + } + } else { + if (!ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency()) { + if (packet_creator_.NextSendingPacketNumber() >= + FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_can_send_ack_frequency, 3, 3); + ack_frequency_sent_ = true; + auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame(); + visitor_->SendAckFrequency(frame); + } + } } if (packet_creator_.has_ack() || !CanWrite(NO_RETRANSMITTABLE_DATA)) { @@ -5817,8 +5834,8 @@ uber_received_packet_manager_.GetEarliestAckTimeout(); QUIC_BUG_IF(quic_bug_12714_32, !earliest_ack_timeout.IsInitialized()); MaybeBundleCryptoDataWithAcks(); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { - QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data3, 2, 4); + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { + QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data4, 2, 4); visitor_->MaybeBundleOpportunistically(); } earliest_ack_timeout = uber_received_packet_manager_.GetEarliestAckTimeout();
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 0113dde..36f0aca 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -739,7 +739,8 @@ // QuicPacketCreator::DelegateInterface bool ShouldGeneratePacket(HasRetransmittableData retransmittable, IsHandshake handshake) override; - void MaybeBundleOpportunistically() override; + void MaybeBundleOpportunistically( + TransmissionType transmission_type) override; QuicByteCount GetFlowControlSendWindowSize(QuicStreamId id) override { return visitor_->GetFlowControlSendWindowSize(id); }
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 018d5ec..11c98e6 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -700,7 +700,7 @@ EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber()); EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber()); EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber()); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(visitor_, MaybeBundleOpportunistically()).Times(AnyNumber()); EXPECT_CALL(visitor_, GetFlowControlSendWindowSize(_)).Times(AnyNumber()); }
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index fc71a6c..1d371e4 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -141,7 +141,10 @@ return true; } - void MaybeBundleOpportunistically() override { QUICHE_DCHECK(false); } + void MaybeBundleOpportunistically( + TransmissionType /*transmission_type*/) override { + QUICHE_DCHECK(false); + } QuicByteCount GetFlowControlSendWindowSize(QuicStreamId /*id*/) override { QUICHE_DCHECK(false);
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 90f9a8d..4c7c44c 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -35,10 +35,10 @@ 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, always bundle qpack decoder data with other frames opportunistically. -QUIC_FLAG(quic_restart_flag_quic_opport_bundle_qpack_decoder_data3, false) // If true, an endpoint does not detect path degrading or blackholing until handshake gets confirmed. QUIC_FLAG(quic_reloadable_flag_quic_no_path_degrading_before_handshake_confirmed, true) +// If true, bundle qpack decoder data with other frames opportunistically. +QUIC_FLAG(quic_restart_flag_quic_opport_bundle_qpack_decoder_data4, false) // If true, default-enable 5RTO blachole detection. QUIC_FLAG(quic_reloadable_flag_quic_default_enable_5rto_blackhole_detection2, true) // If true, disable QUIC version Q046.
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc index ee22a63..27519df 100644 --- a/quiche/quic/core/quic_packet_creator.cc +++ b/quiche/quic/core/quic_packet_creator.cc
@@ -1313,15 +1313,15 @@ } void QuicPacketCreator::MaybeBundleOpportunistically() { - if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { - delegate_->MaybeBundleOpportunistically(); + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { + delegate_->MaybeBundleOpportunistically(next_transmission_type_); return; } // delegate_->MaybeBundleOpportunistically() may change // next_transmission_type_ for the bundled data. const TransmissionType next_transmission_type = next_transmission_type_; - delegate_->MaybeBundleOpportunistically(); + delegate_->MaybeBundleOpportunistically(next_transmission_type_); next_transmission_type_ = next_transmission_type; } @@ -1337,7 +1337,7 @@ const TransmissionType next_transmission_type = next_transmission_type_; MaybeBundleOpportunistically(); // TODO(wub): Remove this QUIC_BUG when deprecating - // quic_opport_bundle_qpack_decoder_data3. + // quic_opport_bundle_qpack_decoder_data4. QUIC_BUG_IF(quic_packet_creator_change_transmission_type, next_transmission_type != next_transmission_type_) << ENDPOINT @@ -1352,11 +1352,11 @@ // - It is not a retransmission. We check next_transmission_type_ for that. // - And it's not handshake data. This is always true for ConsumeData because // the function is not called for handshake data. - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3) && + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4) && next_transmission_type_ == NOT_RETRANSMISSION) { if (QuicByteCount send_window = delegate_->GetFlowControlSendWindowSize(id); write_length > send_window) { - QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data3, 4, 4); + QUIC_RESTART_FLAG_COUNT_N(quic_opport_bundle_qpack_decoder_data4, 4, 4); QUIC_DLOG(INFO) << ENDPOINT << "After bundled data, reducing (old) write_length:" << write_length << "to (new) send_window:" << send_window;
diff --git a/quiche/quic/core/quic_packet_creator.h b/quiche/quic/core/quic_packet_creator.h index e624dc1..6c3fc73 100644 --- a/quiche/quic/core/quic_packet_creator.h +++ b/quiche/quic/core/quic_packet_creator.h
@@ -61,8 +61,10 @@ virtual bool ShouldGeneratePacket(HasRetransmittableData retransmittable, IsHandshake handshake) = 0; // Called when there is data to be sent. Gives delegate a chance to bundle - // anything with to-be-sent data. - virtual void MaybeBundleOpportunistically() = 0; + // anything with to-be-sent data. |transmission_type| is the transmission + // type of the data being sent. + virtual void MaybeBundleOpportunistically( + TransmissionType transmission_type) = 0; // When sending flow controlled data, this will be called after // MaybeBundleOpportunistically(). If the returned flow control send window
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc index 2b67c0b..472dc7f 100644 --- a/quiche/quic/core/quic_packet_creator_test.cc +++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -2540,7 +2540,8 @@ MOCK_METHOD(bool, ShouldGeneratePacket, (HasRetransmittableData retransmittable, IsHandshake handshake), (override)); - MOCK_METHOD(void, MaybeBundleOpportunistically, (), (override)); + MOCK_METHOD(void, MaybeBundleOpportunistically, + (TransmissionType transmission_type), (override)); MOCK_METHOD(QuicByteCount, GetFlowControlSendWindowSize, (QuicStreamId), (override)); MOCK_METHOD(QuicPacketBuffer, GetPacketBuffer, (), (override)); @@ -2618,7 +2619,7 @@ if (bundle_ack) { frames.push_back(QuicFrame(&ack_frame_)); } - EXPECT_CALL(*delegate_, MaybeBundleOpportunistically()) + EXPECT_CALL(*delegate_, MaybeBundleOpportunistically(_)) .WillOnce(Invoke([this, frames = std::move(frames)] { FlushAckFrame(frames); return QuicFrames(); @@ -2644,7 +2645,7 @@ if (!data.empty()) { producer_->SaveStreamData(id, data); } - EXPECT_CALL(*delegate_, MaybeBundleOpportunistically()).Times(1); + EXPECT_CALL(*delegate_, MaybeBundleOpportunistically(_)).Times(1); return QuicPacketCreator::ConsumeData(id, data.length(), offset, state); } @@ -2652,7 +2653,7 @@ quiche::QuicheMemSlice message) { if (!has_ack() && delegate_->ShouldGeneratePacket(NO_RETRANSMITTABLE_DATA, NOT_HANDSHAKE)) { - EXPECT_CALL(*delegate_, MaybeBundleOpportunistically()).Times(1); + EXPECT_CALL(*delegate_, MaybeBundleOpportunistically(_)).Times(1); } return QuicPacketCreator::AddMessageFrame(message_id, absl::MakeSpan(&message, 1)); @@ -2661,7 +2662,7 @@ size_t ConsumeCryptoData(EncryptionLevel level, absl::string_view data, QuicStreamOffset offset) { producer_->SaveCryptoData(level, offset, data); - EXPECT_CALL(*delegate_, MaybeBundleOpportunistically()).Times(1); + EXPECT_CALL(*delegate_, MaybeBundleOpportunistically(_)).Times(1); return QuicPacketCreator::ConsumeCryptoData(level, data.length(), offset); } @@ -2930,7 +2931,7 @@ QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( framer_.transport_version(), Perspective::IS_CLIENT); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id)) .WillOnce(Return(data.length() - 1)); } else { @@ -2939,7 +2940,7 @@ QuicConsumedData consumed = creator_.ConsumeData(stream_id, data, 0u, FIN); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_EQ(consumed.bytes_consumed, data.length() - 1); EXPECT_FALSE(consumed.fin_consumed); } else { @@ -2960,7 +2961,7 @@ QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( framer_.transport_version(), Perspective::IS_CLIENT); - if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data3)) { + if (GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data4)) { EXPECT_CALL(delegate_, GetFlowControlSendWindowSize(stream_id)) .WillOnce(Return(data.length())); } else {
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index 72f5bd0..047fdbe 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -1415,7 +1415,8 @@ MOCK_METHOD(bool, ShouldGeneratePacket, (HasRetransmittableData retransmittable, IsHandshake handshake), (override)); - MOCK_METHOD(void, MaybeBundleOpportunistically, (), (override)); + MOCK_METHOD(void, MaybeBundleOpportunistically, + (TransmissionType transmission_type), (override)); MOCK_METHOD(QuicByteCount, GetFlowControlSendWindowSize, (QuicStreamId), (override)); MOCK_METHOD(SerializedPacketFate, GetSerializedPacketFate,