gfe-relnote: only respect TLPR when there is pending non-crypto stream data. Flag protected by quic_ignore_tlpr_if_no_pending_stream_data and session_decides_what_to_write. When calculating the tail loss probe delay and TLPR option is enabled, only use half tail loss probe when session has pending stream data. Otherwise, ignore TLPR. PiperOrigin-RevId: 253286073 Change-Id: I9321c9d0608f68bb4ec0f7f4fbba1e470e4a0a3c
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index fad4a3b..c7f98ed 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2874,19 +2874,7 @@ void QuicConnection::OnPingTimeout() { if (!retransmission_alarm_->IsSet()) { - bool enable_half_rtt_tail_loss_probe = - sent_packet_manager_.enable_half_rtt_tail_loss_probe(); - if (enable_half_rtt_tail_loss_probe && - GetQuicReloadableFlag(quic_ignore_tlpr_if_sending_ping)) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_sending_ping, 1, 2); - sent_packet_manager_.set_enable_half_rtt_tail_loss_probe(false); - } visitor_->SendPing(); - if (enable_half_rtt_tail_loss_probe && - GetQuicReloadableFlag(quic_ignore_tlpr_if_sending_ping)) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_sending_ping, 2, 2); - sent_packet_manager_.set_enable_half_rtt_tail_loss_probe(true); - } } }
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 6cbb209..f96a7e6 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -3898,6 +3898,10 @@ } TEST_P(QuicConnectionTest, TailLossProbeDelayForStreamDataInTLPR) { + if (!connection_.session_decides_what_to_write()) { + return; + } + // Set TLPR from QuicConfig. EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); QuicConfig config; @@ -3929,6 +3933,10 @@ } TEST_P(QuicConnectionTest, TailLossProbeDelayForNonStreamDataInTLPR) { + if (!connection_.session_decides_what_to_write()) { + return; + } + // Set TLPR from QuicConfig. EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); QuicConfig config; @@ -4014,7 +4022,7 @@ QuicTime::Delta min_rto_timeout = QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs); srtt = manager_->GetRttStats()->SmoothedOrInitialRtt(); - if (GetQuicReloadableFlag(quic_ignore_tlpr_if_sending_ping)) { + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { // First TLP without unacked stream data will no longer use TLPR. expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); } else { @@ -4025,11 +4033,7 @@ EXPECT_EQ(expected_delay, connection_.GetRetransmissionAlarm()->deadline() - clock_.Now()); - // Verify the path degrading delay. - // Path degrading delay will count TLPR for the tail loss probe delay. - expected_delay = - std::max(QuicTime::Delta::FromMilliseconds(kMinTailLossProbeTimeoutMs), - srtt * 0.5); + // Verify the path degrading delay = TLP delay + 1st RTO + 2nd RTO. // Add 1st RTO. retransmission_delay = std::max(manager_->GetRttStats()->smoothed_rtt() + @@ -4047,6 +4051,26 @@ EXPECT_TRUE(connection_.GetPingAlarm()->IsSet()); EXPECT_EQ(QuicTime::Delta::FromSeconds(kPingTimeoutSecs), connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + + // Advance a small period of time: 5ms. And receive a retransmitted ACK. + // This will update the retransmission alarm, verify the retransmission delay + // is correct. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); + QuicAckFrame ack = InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}}); + ProcessAckPacket(&ack); + + // Verify the retransmission delay. + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + // First TLP without unacked stream data will no longer use TLPR. + expected_delay = std::max(2 * srtt, 1.5 * srtt + 0.5 * min_rto_timeout); + } else { + expected_delay = + std::max(QuicTime::Delta::FromMilliseconds(kMinTailLossProbeTimeoutMs), + srtt * 0.5); + } + expected_delay = expected_delay - QuicTime::Delta::FromMilliseconds(5); + EXPECT_EQ(expected_delay, + connection_.GetRetransmissionAlarm()->deadline() - clock_.Now()); } TEST_P(QuicConnectionTest, RTO) {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index c8ae53b..d92ee66 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -118,7 +118,9 @@ acked_packets_iter_(last_ack_frame_.packets.rbegin()), tolerate_reneging_(GetQuicReloadableFlag(quic_tolerate_reneging)), loss_removes_from_inflight_( - GetQuicReloadableFlag(quic_loss_removes_from_inflight)) { + GetQuicReloadableFlag(quic_loss_removes_from_inflight)), + ignore_tlpr_if_no_pending_stream_data_( + GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { if (tolerate_reneging_) { QUIC_RELOADABLE_FLAG_COUNT(quic_tolerate_reneging); } @@ -1017,7 +1019,16 @@ size_t consecutive_tlp_count) const { QuicTime::Delta srtt = rtt_stats_.SmoothedOrInitialRtt(); if (enable_half_rtt_tail_loss_probe_ && consecutive_tlp_count == 0u) { - return std::max(min_tlp_timeout_, srtt * 0.5); + if (!ignore_tlpr_if_no_pending_stream_data_ || + !session_decides_what_to_write()) { + return std::max(min_tlp_timeout_, srtt * 0.5); + } + QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_no_pending_stream_data, 1, + 5); + if (unacked_packets().HasUnackedStreamData()) { + // Enable TLPR if there are pending data packets. + return std::max(min_tlp_timeout_, srtt * 0.5); + } } if (ietf_style_tlp_) { return std::max(min_tlp_timeout_, 1.5 * srtt + rtt_stats_.max_ack_delay());
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 787bf8d..bba4887 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -360,15 +360,6 @@ delayed_ack_time_ = delayed_ack_time; } - bool enable_half_rtt_tail_loss_probe() const { - return enable_half_rtt_tail_loss_probe_; - } - - void set_enable_half_rtt_tail_loss_probe( - bool enable_half_rtt_tail_loss_probe) { - enable_half_rtt_tail_loss_probe_ = enable_half_rtt_tail_loss_probe; - } - const QuicUnackedPacketMap& unacked_packets() const { return unacked_packets_; } @@ -393,6 +384,10 @@ return unacked_packets_.use_uber_loss_algorithm(); } + bool ignore_tlpr_if_no_pending_stream_data() const { + return ignore_tlpr_if_no_pending_stream_data_; + } + private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -637,6 +632,9 @@ // Latched value of quic_loss_removes_from_inflight. const bool loss_removes_from_inflight_; + + // Latched value of quic_ignore_tlpr_if_no_pending_stream_data. + const bool ignore_tlpr_if_no_pending_stream_data_; }; } // namespace quic
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index a0746fe..1e8302f 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -267,6 +267,14 @@ return packet; } + SerializedPacket CreatePingPacket(uint64_t packet_number) { + SerializedPacket packet(QuicPacketNumber(packet_number), + PACKET_4BYTE_PACKET_NUMBER, nullptr, kDefaultLength, + false, false); + packet.retransmittable_frames.push_back(QuicFrame(QuicPingFrame())); + return packet; + } + void SendDataPacket(uint64_t packet_number) { SendDataPacket(packet_number, ENCRYPTION_INITIAL); } @@ -282,6 +290,17 @@ NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); } + void SendPingPacket(uint64_t packet_number, + EncryptionLevel encryption_level) { + EXPECT_CALL(*send_algorithm_, + OnPacketSent(_, BytesInFlight(), + QuicPacketNumber(packet_number), _, _)); + SerializedPacket packet(CreatePingPacket(packet_number)); + packet.encryption_level = encryption_level; + manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), + NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + } + void SendCryptoPacket(uint64_t packet_number) { EXPECT_CALL( *send_algorithm_, @@ -1761,6 +1780,118 @@ EXPECT_EQ(expected_time, manager_.GetRetransmissionTime()); } +TEST_P(QuicSentPacketManagerTest, TLPRWithPendingStreamData) { + if (!manager_.session_decides_what_to_write()) { + return; + } + + QuicConfig config; + QuicTagVector options; + + options.push_back(kTLPR); + QuicConfigPeer::SetReceivedConnectionOptions(&config, options); + EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*send_algorithm_, PacingRate(_)) + .WillRepeatedly(Return(QuicBandwidth::Zero())); + EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) + .WillOnce(Return(10 * kDefaultTCPMSS)); + manager_.SetFromConfig(config); + EXPECT_TRUE( + QuicSentPacketManagerPeer::GetEnableHalfRttTailLossProbe(&manager_)); + + QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); + + SendDataPacket(1); + SendDataPacket(2); + + // Test with a standard smoothed RTT. + RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); + rtt_stats->set_initial_rtt(QuicTime::Delta::FromMilliseconds(100)); + QuicTime::Delta srtt = rtt_stats->initial_rtt(); + // With pending stream data, TLPR is used. + QuicTime::Delta expected_tlp_delay = 0.5 * srtt; + EXPECT_CALL(notifier_, HasUnackedStreamData()).WillRepeatedly(Return(true)); + + EXPECT_EQ(expected_tlp_delay, + manager_.GetRetransmissionTime() - clock_.Now()); + + // Retransmit the packet by invoking the retransmission timeout. + clock_.AdvanceTime(expected_tlp_delay); + manager_.OnRetransmissionTimeout(); + EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); + EXPECT_FALSE(manager_.HasPendingRetransmissions()); + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); + EXPECT_TRUE(manager_.MaybeRetransmitTailLossProbe()); + + EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); + EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); + EXPECT_FALSE(manager_.HasPendingRetransmissions()); + + // 2nd TLP. + expected_tlp_delay = 2 * srtt; + EXPECT_EQ(expected_tlp_delay, + manager_.GetRetransmissionTime() - clock_.Now()); +} + +TEST_P(QuicSentPacketManagerTest, TLPRWithoutPendingStreamData) { + if (!manager_.session_decides_what_to_write()) { + return; + } + + QuicConfig config; + QuicTagVector options; + + options.push_back(kTLPR); + QuicConfigPeer::SetReceivedConnectionOptions(&config, options); + EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*send_algorithm_, PacingRate(_)) + .WillRepeatedly(Return(QuicBandwidth::Zero())); + EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) + .WillOnce(Return(10 * kDefaultTCPMSS)); + manager_.SetFromConfig(config); + EXPECT_TRUE( + QuicSentPacketManagerPeer::GetEnableHalfRttTailLossProbe(&manager_)); + QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); + + SendPingPacket(1, ENCRYPTION_INITIAL); + SendPingPacket(2, ENCRYPTION_INITIAL); + + // Test with a standard smoothed RTT. + RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); + rtt_stats->set_initial_rtt(QuicTime::Delta::FromMilliseconds(100)); + QuicTime::Delta srtt = rtt_stats->initial_rtt(); + QuicTime::Delta expected_tlp_delay = 0.5 * srtt; + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + // With no pending stream data, TLPR is ignored. + expected_tlp_delay = 2 * srtt; + } + EXPECT_CALL(notifier_, HasUnackedStreamData()).WillRepeatedly(Return(false)); + EXPECT_EQ(expected_tlp_delay, + manager_.GetRetransmissionTime() - clock_.Now()); + + // Retransmit the packet by invoking the retransmission timeout. + clock_.AdvanceTime(expected_tlp_delay); + manager_.OnRetransmissionTimeout(); + EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); + EXPECT_FALSE(manager_.HasPendingRetransmissions()); + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); + EXPECT_TRUE(manager_.MaybeRetransmitTailLossProbe()); + EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); + EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); + EXPECT_FALSE(manager_.HasPendingRetransmissions()); + + // 2nd TLP. + expected_tlp_delay = 2 * srtt; + EXPECT_EQ(expected_tlp_delay, + manager_.GetRetransmissionTime() - clock_.Now()); +} + TEST_P(QuicSentPacketManagerTest, GetTransmissionTimeSpuriousRTO) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(100),
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 33b9ddb..1e863b1 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -8,9 +8,11 @@ #include <string> #include <utility> +#include "base/logging.h" #include "net/third_party/quiche/src/quic/core/quic_connection.h" #include "net/third_party/quiche/src/quic/core/quic_flow_controller.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flag_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" @@ -869,6 +871,13 @@ if (stream->IsWaitingForAcks()) { zombie_streams_[stream->id()] = std::move(it->second); } else { + // Clean up the stream since it is no longer waiting for acks. + if (ignore_tlpr_if_no_pending_stream_data() && + session_decides_what_to_write()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_no_pending_stream_data, + 2, 5); + streams_waiting_for_acks_.erase(stream->id()); + } closed_streams_.push_back(std::move(it->second)); // Do not retransmit data of a closed stream. streams_with_pending_retransmission_.erase(stream_id); @@ -1543,6 +1552,13 @@ } void QuicSession::OnStreamDoneWaitingForAcks(QuicStreamId id) { + if (ignore_tlpr_if_no_pending_stream_data() && + session_decides_what_to_write()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_no_pending_stream_data, 3, + 5); + streams_waiting_for_acks_.erase(id); + } + auto it = zombie_streams_.find(id); if (it == zombie_streams_.end()) { return; @@ -1557,6 +1573,34 @@ streams_with_pending_retransmission_.erase(id); } +void QuicSession::OnStreamWaitingForAcks(QuicStreamId id) { + if (!ignore_tlpr_if_no_pending_stream_data() || + !session_decides_what_to_write()) + return; + + // Exclude crypto stream's status since it is counted in HasUnackedCryptoData. + if (GetCryptoStream() != nullptr && id == GetCryptoStream()->id()) { + return; + } + + QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_no_pending_stream_data, 4, + 5); + streams_waiting_for_acks_.insert(id); + + // The number of the streams waiting for acks should not be larger than the + // number of streams. + if (dynamic_stream_map_.size() + static_stream_map_.size() + + zombie_streams_.size() < + static_cast<int>(streams_waiting_for_acks_.size())) { + QUIC_BUG << "More streams are waiting for acks than the number of streams. " + << "Sizes: dynamic streams: " << dynamic_stream_map_.size() + << ", static streams: " << static_stream_map_.size() + << ", zombie streams: " << zombie_streams_.size() + << ", vs streams waiting for acks: " + << streams_waiting_for_acks_.size(); + } +} + QuicStream* QuicSession::GetStream(QuicStreamId id) const { if (id <= largest_static_stream_id_) { auto static_stream = static_stream_map_.find(id); @@ -1716,6 +1760,17 @@ return false; } +bool QuicSession::HasUnackedStreamData() const { + DCHECK(ignore_tlpr_if_no_pending_stream_data()); + if (ignore_tlpr_if_no_pending_stream_data()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_ignore_tlpr_if_no_pending_stream_data, 5, + 5); + return !streams_waiting_for_acks_.empty(); + } + + return true; +} + WriteStreamDataResult QuicSession::WriteStreamData(QuicStreamId id, QuicStreamOffset offset, QuicByteCount data_length,
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index ef4e804..a76e7df 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -150,6 +150,7 @@ TransmissionType type) override; bool IsFrameOutstanding(const QuicFrame& frame) const override; bool HasUnackedCryptoData() const override; + bool HasUnackedStreamData() const override; // Called on every incoming packet. Passes |packet| through to |connection_|. virtual void ProcessUdpPacket(const QuicSocketAddress& self_address, @@ -324,6 +325,9 @@ // a stream is reset because of an error). void OnStreamDoneWaitingForAcks(QuicStreamId id); + // Called when stream |id| is newly waiting for acks. + void OnStreamWaitingForAcks(QuicStreamId id); + // Called to cancel retransmission of unencypted crypto stream data. void NeuterUnencryptedData(); @@ -616,6 +620,11 @@ // stream. void PendingStreamOnRstStream(const QuicRstStreamFrame& frame); + bool ignore_tlpr_if_no_pending_stream_data() const { + return connection_->sent_packet_manager() + .ignore_tlpr_if_no_pending_stream_data(); + } + // Keep track of highest received byte offset of locally closed streams, while // waiting for a definitive final highest offset from the peer. std::map<QuicStreamId, QuicStreamOffset> @@ -654,6 +663,9 @@ // been consumed. QuicUnorderedSet<QuicStreamId> draining_streams_; + // Set of stream ids that are waiting for acks excluding crypto stream id. + QuicUnorderedSet<QuicStreamId> streams_waiting_for_acks_; + // TODO(fayang): Consider moving LegacyQuicStreamIdManager into // UberQuicStreamIdManager. // Manages stream IDs for Google QUIC. @@ -719,7 +731,7 @@ // list may be a superset of the connection framer's supported versions. ParsedQuicVersionVector supported_versions_; - // Latched value of quic_eliminate_static_stream_map. + // Latched value of quic_eliminate_static_stream_map. const bool eliminate_static_stream_map_; };
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index af8079c..55e910c 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -1011,6 +1011,10 @@ if (consumed_data.bytes_consumed > 0 || consumed_data.fin_consumed) { busy_counter_ = 0; } + + if (IsWaitingForAcks()) { + session_->OnStreamWaitingForAcks(id_); + } } uint64_t QuicStream::BufferedDataBytes() const {
diff --git a/quic/core/quic_stream_test.cc b/quic/core/quic_stream_test.cc index f13d867..d11a7a4 100644 --- a/quic/core/quic_stream_test.cc +++ b/quic/core/quic_stream_test.cc
@@ -314,6 +314,9 @@ NO_FIN); })); stream_->WriteOrBufferData(QuicStringPiece(kData1, 2), false, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); EXPECT_EQ(1u, stream_->BufferedDataBytes()); } @@ -331,6 +334,9 @@ NO_FIN); })); stream_->WriteOrBufferData(QuicStringPiece(kData1, 2), true, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -376,6 +382,10 @@ kDataLen - 1, 0u, NO_FIN); })); stream_->WriteOrBufferData(kData1, false, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } + EXPECT_EQ(1u, stream_->BufferedDataBytes()); EXPECT_TRUE(HasWriteBlockedStreams()); @@ -390,6 +400,9 @@ kDataLen - 1, kDataLen - 1, NO_FIN); })); stream_->OnCanWrite(); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } // And finally the end of the bytes_consumed. EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) @@ -398,6 +411,9 @@ 2 * kDataLen - 2, NO_FIN); })); stream_->OnCanWrite(); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, WriteOrBufferDataReachStreamLimit) { @@ -408,6 +424,9 @@ EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillOnce(Invoke(&(MockQuicSession::ConsumeData))); stream_->WriteOrBufferData(data, false, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_CALL(*connection_, CloseConnection(QUIC_STREAM_LENGTH_OVERFLOW, _, _)); EXPECT_QUIC_BUG(stream_->WriteOrBufferData("a", false, nullptr), "Write too many data via stream"); @@ -442,12 +461,18 @@ NO_FIN); })); stream_->WriteOrBufferData(QuicStringPiece(kData1, 1), false, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_FALSE(fin_sent()); EXPECT_FALSE(rst_sent()); // Now close the stream, and expect that we send a RST. EXPECT_CALL(*session_, SendRstStream(_, _, _)); stream_->OnClose(); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_FALSE(fin_sent()); EXPECT_TRUE(rst_sent()); } @@ -818,9 +843,15 @@ // Stream is not waiting for acks initially. EXPECT_FALSE(stream_->IsWaitingForAcks()); EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } // Send kData1. stream_->WriteOrBufferData(kData1, false, nullptr); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->IsWaitingForAcks()); QuicByteCount newly_acked_length = 0; @@ -829,11 +860,17 @@ EXPECT_EQ(9u, newly_acked_length); // Stream is not waiting for acks as all sent data is acked. EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); // Send kData2. stream_->WriteOrBufferData(kData2, false, nullptr); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // Send FIN. stream_->WriteOrBufferData("", true, nullptr); @@ -849,6 +886,9 @@ EXPECT_EQ(9u, newly_acked_length); // Stream is waiting for acks as FIN is not acked. EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); // FIN is acked. @@ -856,6 +896,9 @@ &newly_acked_length)); EXPECT_EQ(0u, newly_acked_length); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); } @@ -870,26 +913,43 @@ stream_->WriteOrBufferData("", true, nullptr); EXPECT_EQ(3u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->IsWaitingForAcks()); - + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } QuicByteCount newly_acked_length = 0; EXPECT_TRUE(stream_->OnStreamFrameAcked(9, 9, false, QuicTime::Delta::Zero(), &newly_acked_length)); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(9u, newly_acked_length); EXPECT_EQ(3u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->OnStreamFrameAcked(18, 9, false, QuicTime::Delta::Zero(), &newly_acked_length)); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(9u, newly_acked_length); EXPECT_EQ(3u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->OnStreamFrameAcked(0, 9, false, QuicTime::Delta::Zero(), &newly_acked_length)); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(9u, newly_acked_length); EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); // FIN is not acked yet. EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_TRUE(stream_->OnStreamFrameAcked(27, 0, true, QuicTime::Delta::Zero(), &newly_acked_length)); EXPECT_EQ(0u, newly_acked_length); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, CancelStream) { @@ -897,22 +957,43 @@ EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillRepeatedly(Invoke(MockQuicSession::ConsumeData)); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); stream_->WriteOrBufferData(kData1, false, nullptr); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // Cancel stream. stream_->Reset(QUIC_STREAM_NO_ERROR); // stream still waits for acks as the error code is QUIC_STREAM_NO_ERROR, and // data is going to be retransmitted. EXPECT_TRUE(stream_->IsWaitingForAcks()); - EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 9)); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } + EXPECT_CALL(*connection_, + OnStreamReset(stream_->id(), QUIC_STREAM_CANCELLED)); + EXPECT_CALL(*connection_, SendControlFrame(_)).Times(1); + EXPECT_CALL(*session_, SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 9)) + .WillOnce(InvokeWithoutArgs([this]() { + return QuicSessionPeer::SendRstStreamInner( + session_.get(), stream_->id(), QUIC_STREAM_CANCELLED, + stream_->stream_bytes_written(), + /*close_write_side_only=*/false); + })); + stream_->Reset(QUIC_STREAM_CANCELLED); EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // Stream stops waiting for acks as data is not going to be retransmitted. EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, RstFrameReceivedStreamNotFinishSending) { @@ -920,10 +1001,16 @@ EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillRepeatedly(Invoke(MockQuicSession::ConsumeData)); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); stream_->WriteOrBufferData(kData1, false, nullptr); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // RST_STREAM received. @@ -936,6 +1023,9 @@ // Stream stops waiting for acks as it does not finish sending and rst is // sent. EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, RstFrameReceivedStreamFinishSending) { @@ -943,10 +1033,16 @@ EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillRepeatedly(Invoke(MockQuicSession::ConsumeData)); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); stream_->WriteOrBufferData(kData1, true, nullptr); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } // RST_STREAM received. EXPECT_CALL(*session_, SendRstStream(_, _, _)).Times(0); @@ -955,6 +1051,9 @@ stream_->OnStreamReset(rst_frame); // Stream still waits for acks as it finishes sending and has unacked data. EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); } @@ -963,11 +1062,16 @@ EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillRepeatedly(Invoke(MockQuicSession::ConsumeData)); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); stream_->WriteOrBufferData(kData1, false, nullptr); EXPECT_TRUE(stream_->IsWaitingForAcks()); - + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } EXPECT_CALL(*session_, SendRstStream(stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, 9)); stream_->OnConnectionClosed(QUIC_INTERNAL_ERROR, @@ -975,6 +1079,9 @@ EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // Stream stops waiting for acks as connection is going to close. EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, CanWriteNewDataAfterData) { @@ -1003,6 +1110,8 @@ stream_->WriteOrBufferData(data, false, nullptr); stream_->WriteOrBufferData(data, false, nullptr); stream_->WriteOrBufferData(data, false, nullptr); + EXPECT_TRUE(stream_->IsWaitingForAcks()); + // Verify all data is saved. EXPECT_EQ(3 * data.length() - 100, stream_->BufferedDataBytes()); @@ -1215,12 +1324,20 @@ Initialize(); EXPECT_CALL(*session_, WritevData(_, _, _, _, _)) .WillRepeatedly(Invoke(MockQuicSession::ConsumeData)); + EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } + // Send [0, 27) and fin. stream_->WriteOrBufferData(kData1, false, nullptr); stream_->WriteOrBufferData(kData1, false, nullptr); stream_->WriteOrBufferData(kData1, true, nullptr); EXPECT_EQ(3u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } // Ack [0, 9), [5, 22) and [18, 26) // Verify [0, 9) 9 bytes are acked. @@ -1240,6 +1357,9 @@ EXPECT_EQ(4u, newly_acked_length); EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } // Ack [0, 27). Verify [26, 27) 1 byte is acked. EXPECT_TRUE(stream_->OnStreamFrameAcked(26, 1, false, QuicTime::Delta::Zero(), @@ -1247,6 +1367,9 @@ EXPECT_EQ(1u, newly_acked_length); EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_TRUE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_TRUE(session_->HasUnackedStreamData()); + } // Ack Fin. EXPECT_TRUE(stream_->OnStreamFrameAcked(27, 0, true, QuicTime::Delta::Zero(), @@ -1254,6 +1377,9 @@ EXPECT_EQ(0u, newly_acked_length); EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } // Ack [10, 27) and fin. No new data is acked. EXPECT_FALSE(stream_->OnStreamFrameAcked( @@ -1261,6 +1387,9 @@ EXPECT_EQ(0u, newly_acked_length); EXPECT_EQ(0u, QuicStreamPeer::SendBuffer(stream_).size()); EXPECT_FALSE(stream_->IsWaitingForAcks()); + if (GetQuicReloadableFlag(quic_ignore_tlpr_if_no_pending_stream_data)) { + EXPECT_FALSE(session_->HasUnackedStreamData()); + } } TEST_P(QuicStreamTest, OnStreamFrameLost) {
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h index 17589dd..f9f718e 100644 --- a/quic/core/quic_unacked_packet_map.h +++ b/quic/core/quic_unacked_packet_map.h
@@ -153,6 +153,12 @@ // HasUnackedCryptoData() when session_decides_what_to_write_ is default true. bool HasPendingCryptoPackets() const; + // Returns true if there is any unacked non-crypto stream data. + bool HasUnackedStreamData() const { + DCHECK(session_decides_what_to_write()); + return session_notifier_->HasUnackedStreamData(); + } + // Removes any retransmittable frames from this transmission or an associated // transmission. It removes now useless transmissions, and disconnects any // other packets from other transmissions.
diff --git a/quic/core/session_notifier_interface.h b/quic/core/session_notifier_interface.h index a601c3e..89b4a9c 100644 --- a/quic/core/session_notifier_interface.h +++ b/quic/core/session_notifier_interface.h
@@ -37,6 +37,9 @@ // Returns true if crypto stream is waiting for acks. virtual bool HasUnackedCryptoData() const = 0; + + // Returns true if any stream is waiting for acks. + virtual bool HasUnackedStreamData() const = 0; }; } // namespace quic
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index cb51a3b..82bdd20 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1061,6 +1061,7 @@ void(const QuicFrames&, TransmissionType type)); MOCK_CONST_METHOD1(IsFrameOutstanding, bool(const QuicFrame&)); MOCK_CONST_METHOD0(HasUnackedCryptoData, bool()); + MOCK_CONST_METHOD0(HasUnackedStreamData, bool()); }; // Creates a client session for testing.
diff --git a/quic/test_tools/simple_session_notifier.cc b/quic/test_tools/simple_session_notifier.cc index c6e3801..624e4f7 100644 --- a/quic/test_tools/simple_session_notifier.cc +++ b/quic/test_tools/simple_session_notifier.cc
@@ -434,6 +434,14 @@ return !bytes_to_ack.Empty(); } +bool SimpleSessionNotifier::HasUnackedStreamData() const { + for (auto it : stream_map_) { + if (StreamIsWaitingForAcks(it.first)) + return true; + } + return false; +} + bool SimpleSessionNotifier::OnControlFrameAcked(const QuicFrame& frame) { QuicControlFrameId id = GetControlFrameId(frame); if (id == kInvalidControlFrameId) {
diff --git a/quic/test_tools/simple_session_notifier.h b/quic/test_tools/simple_session_notifier.h index 25c9941..aab3769 100644 --- a/quic/test_tools/simple_session_notifier.h +++ b/quic/test_tools/simple_session_notifier.h
@@ -73,6 +73,7 @@ TransmissionType type) override; bool IsFrameOutstanding(const QuicFrame& frame) const override; bool HasUnackedCryptoData() const override; + bool HasUnackedStreamData() const override; private: struct StreamState {
diff --git a/quic/test_tools/simple_session_notifier_test.cc b/quic/test_tools/simple_session_notifier_test.cc index d9958c8..8b931fe 100644 --- a/quic/test_tools/simple_session_notifier_test.cc +++ b/quic/test_tools/simple_session_notifier_test.cc
@@ -88,6 +88,8 @@ EXPECT_CALL(connection_, SendStreamData(5, 1024, 0, FIN)) .WillOnce(Return(QuicConsumedData(1024, true))); notifier_.WriteOrBufferData(5, 1024, FIN); + EXPECT_TRUE(notifier_.StreamIsWaitingForAcks(5)); + EXPECT_TRUE(notifier_.HasUnackedStreamData()); // Reset stream 5 with no error. EXPECT_CALL(connection_, SendControlFrame(_)) @@ -96,10 +98,12 @@ notifier_.WriteOrBufferRstStream(5, QUIC_STREAM_NO_ERROR, 1024); // Verify stream 5 is waiting for acks. EXPECT_TRUE(notifier_.StreamIsWaitingForAcks(5)); + EXPECT_TRUE(notifier_.HasUnackedStreamData()); // Reset stream 5 with error. notifier_.WriteOrBufferRstStream(5, QUIC_ERROR_PROCESSING_STREAM, 1024); EXPECT_FALSE(notifier_.StreamIsWaitingForAcks(5)); + EXPECT_FALSE(notifier_.HasUnackedStreamData()); } TEST_F(SimpleSessionNotifierTest, WriteOrBufferPing) { @@ -158,10 +162,13 @@ QuicTime::Zero()); EXPECT_TRUE(notifier_.StreamIsWaitingForAcks( QuicUtils::GetCryptoStreamId(connection_.transport_version()))); + EXPECT_TRUE(notifier_.HasUnackedStreamData()); + // Neuters unencrypted data. notifier_.NeuterUnencryptedData(); EXPECT_FALSE(notifier_.StreamIsWaitingForAcks( QuicUtils::GetCryptoStreamId(connection_.transport_version()))); + EXPECT_FALSE(notifier_.HasUnackedStreamData()); } TEST_F(SimpleSessionNotifierTest, OnCanWrite) { @@ -178,6 +185,7 @@ notifier_.WriteOrBufferData( QuicUtils::GetCryptoStreamId(connection_.transport_version()), 1024, NO_FIN); + // Send crypto data [1024, 2048) in ENCRYPTION_ZERO_RTT. connection_.SetDefaultEncryptionLevel(ENCRYPTION_ZERO_RTT); EXPECT_CALL(connection_, SendStreamData(QuicUtils::GetCryptoStreamId(
diff --git a/quic/test_tools/simulator/quic_endpoint.cc b/quic/test_tools/simulator/quic_endpoint.cc index ba9d206..872a9b2 100644 --- a/quic/test_tools/simulator/quic_endpoint.cc +++ b/quic/test_tools/simulator/quic_endpoint.cc
@@ -293,6 +293,13 @@ return false; } +bool QuicEndpoint::HasUnackedStreamData() const { + if (notifier_ != nullptr) { + return notifier_->HasUnackedStreamData(); + } + return false; +} + QuicEndpoint::Writer::Writer(QuicEndpoint* endpoint) : endpoint_(endpoint), is_blocked_(false) {}
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h index 53bff3a..c547551 100644 --- a/quic/test_tools/simulator/quic_endpoint.h +++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -130,6 +130,7 @@ TransmissionType type) override; bool IsFrameOutstanding(const QuicFrame& frame) const override; bool HasUnackedCryptoData() const override; + bool HasUnackedStreamData() const override; // End SessionNotifierInterface implementation. private: