Internal QUICHE change PiperOrigin-RevId: 331651056 Change-Id: Ic1b3d126e19f4294358b99d5a7099699a892cabd
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 7eb0202..6b694a0 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2879,7 +2879,8 @@ packet_send_time = packet_send_time + result.send_time_offset; } - if (debug_visitor_ != nullptr) { + if (!sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() && + debug_visitor_ != nullptr) { // Pass the write result to the visitor. debug_visitor_->OnPacketSent(*packet, packet->transmission_type, packet_send_time); @@ -2927,6 +2928,24 @@ << ENDPOINT << "Trying to start blackhole detection without no bytes in flight"; + if (sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() && + debug_visitor_ != nullptr) { + QUIC_RELOADABLE_FLAG_COUNT_N( + quic_give_sent_packet_to_debug_visitor_after_sent, 1, 2); + if (sent_packet_manager_.unacked_packets().empty()) { + QUIC_BUG << "Unacked map is empty right after packet is sent"; + } else { + debug_visitor_->OnPacketSent( + packet->packet_number, packet->encrypted_length, + packet->has_crypto_handshake, packet->transmission_type, + packet->encryption_level, + sent_packet_manager_.unacked_packets() + .rbegin() + ->retransmittable_frames, + packet->nonretransmittable_frames, packet_send_time); + } + } + if (in_flight || !retransmission_alarm_->IsSet()) { SetRetransmissionAlarm(); } @@ -4026,7 +4045,8 @@ return false; } - if (debug_visitor_ != nullptr) { + if (!sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() && + debug_visitor_ != nullptr) { debug_visitor_->OnPacketSent( *probing_packet, probing_packet->transmission_type, packet_send_time); } @@ -4036,6 +4056,24 @@ probing_packet.get(), packet_send_time, probing_packet->transmission_type, NO_RETRANSMITTABLE_DATA, /*measure_rtt=*/true); + if (sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() && + debug_visitor_ != nullptr) { + QUIC_RELOADABLE_FLAG_COUNT_N( + quic_give_sent_packet_to_debug_visitor_after_sent, 2, 2); + if (sent_packet_manager_.unacked_packets().empty()) { + QUIC_BUG << "Unacked map is empty right after packet is sent"; + } else { + debug_visitor_->OnPacketSent( + probing_packet->packet_number, probing_packet->encrypted_length, + probing_packet->has_crypto_handshake, + probing_packet->transmission_type, probing_packet->encryption_level, + sent_packet_manager_.unacked_packets() + .rbegin() + ->retransmittable_frames, + probing_packet->nonretransmittable_frames, packet_send_time); + } + } + if (IsWriteBlockedStatus(result.status)) { if (probing_writer == writer_) { // Visitor should not be write blocked if the probing writer is not the
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 4e2c507..9b62a98 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -202,10 +202,21 @@ ~QuicConnectionDebugVisitor() override {} // Called when a packet has been sent. + // TODO(wub): Delete when deprecating + // --quic_give_sent_packet_to_debug_visitor_after_sent. virtual void OnPacketSent(const SerializedPacket& /*serialized_packet*/, TransmissionType /*transmission_type*/, QuicTime /*sent_time*/) {} + virtual void OnPacketSent(QuicPacketNumber /*packet_number*/, + QuicPacketLength /*packet_length*/, + bool /*has_crypto_handshake*/, + TransmissionType /*transmission_type*/, + EncryptionLevel /*encryption_level*/, + const QuicFrames& /*retransmittable_frames*/, + const QuicFrames& /*nonretransmittable_frames*/, + QuicTime /*sent_time*/) {} + // Called when a coalesced packet has been sent. virtual void OnCoalescedPacketSent( const QuicCoalescedPacket& /*coalesced_packet*/,
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 1070c77..7b9ece2 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -7170,10 +7170,20 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + } connection_.SendStreamDataWithString(1, "foo", 0, NO_FIN); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + } connection_.SendConnectivityProbingPacket(writer_.get(), connection_.peer_address()); } @@ -7300,7 +7310,12 @@ CongestionBlockWrites(); connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + } EXPECT_CALL(debug_visitor, OnPingSent()).Times(1); connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); EXPECT_FALSE(connection_.HasQueuedData()); @@ -7312,7 +7327,12 @@ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + } EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); connection_.SendControlFrame(QuicFrame(new QuicBlockedFrame(1, 3))); EXPECT_EQ(1u, connection_.GetStats().blocked_frames_sent); @@ -7328,7 +7348,12 @@ QuicBlockedFrame blocked(1, 3); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); + } EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); connection_.SendControlFrame(QuicFrame(&blocked)); EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); @@ -8051,21 +8076,45 @@ } // Expect them retransmitted in cyclic order (foo, bar, test, foo, bar...). QuicPacketCount sent_count = 0; - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)) - .WillRepeatedly(Invoke([this, &sent_count](const SerializedPacket&, - TransmissionType, QuicTime) { - ASSERT_EQ(1u, writer_->stream_frames().size()); - if (connection_.version().CanSendCoalescedPackets()) { - // There is a delay of sending coalesced packet, so (6, 0, 3, 6, - // 0...). - EXPECT_EQ(3 * ((sent_count + 2) % 3), - writer_->stream_frames()[0]->offset); - } else { - // Identify the frames by stream offset (0, 3, 6, 0, 3...). - EXPECT_EQ(3 * (sent_count % 3), writer_->stream_frames()[0]->offset); - } - sent_count++; - })); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)) + .WillRepeatedly( + Invoke([this, &sent_count](QuicPacketNumber, QuicPacketLength, bool, + TransmissionType, EncryptionLevel, + const QuicFrames&, const QuicFrames&, + QuicTime) { + ASSERT_EQ(1u, writer_->stream_frames().size()); + if (connection_.version().CanSendCoalescedPackets()) { + // There is a delay of sending coalesced packet, so (6, 0, 3, 6, + // 0...). + EXPECT_EQ(3 * ((sent_count + 2) % 3), + writer_->stream_frames()[0]->offset); + } else { + // Identify the frames by stream offset (0, 3, 6, 0, 3...). + EXPECT_EQ(3 * (sent_count % 3), + writer_->stream_frames()[0]->offset); + } + sent_count++; + })); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)) + .WillRepeatedly(Invoke([this, &sent_count](const SerializedPacket&, + TransmissionType, QuicTime) { + ASSERT_EQ(1u, writer_->stream_frames().size()); + if (connection_.version().CanSendCoalescedPackets()) { + // There is a delay of sending coalesced packet, so (6, 0, 3, 6, + // 0...). + EXPECT_EQ(3 * ((sent_count + 2) % 3), + writer_->stream_frames()[0]->offset); + } else { + // Identify the frames by stream offset (0, 3, 6, 0, 3...). + EXPECT_EQ(3 * (sent_count % 3), + writer_->stream_frames()[0]->offset); + } + sent_count++; + })); + } EXPECT_CALL(*send_algorithm_, ShouldSendProbingPacket()) .WillRepeatedly(Return(true)); EXPECT_CALL(visitor_, SendProbingData()).WillRepeatedly([this] { @@ -8089,7 +8138,12 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); + } EXPECT_CALL(*send_algorithm_, ShouldSendProbingPacket()) .WillRepeatedly(Return(true)); EXPECT_CALL(visitor_, SendProbingData()).WillRepeatedly([this] { @@ -9975,7 +10029,12 @@ } MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(3); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(3); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(3); + } EXPECT_CALL(debug_visitor, OnCoalescedPacketSent(_, _)).Times(1); EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); { @@ -10017,7 +10076,12 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + if (connection_.sent_packet_manager() + .give_sent_packet_to_debug_visitor_after_sent()) { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); + } // Our TestPacketWriter normally parses the sent packet using the version // from the connection, so here we need to tell it to use the encapsulation
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 678ce04..6719a94 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -438,6 +438,10 @@ void OnUserAgentIdKnown() { loss_algorithm_->OnUserAgentIdKnown(); } + bool give_sent_packet_to_debug_visitor_after_sent() const { + return give_sent_packet_to_debug_visitor_after_sent_; + } + private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -694,6 +698,9 @@ // The multiplier for caculating PTO timeout before any RTT sample is // available. float pto_multiplier_without_rtt_samples_; + + const bool give_sent_packet_to_debug_visitor_after_sent_ = + GetQuicReloadableFlag(quic_give_sent_packet_to_debug_visitor_after_sent); }; } // namespace quic
diff --git a/quic/core/quic_trace_visitor.cc b/quic/core/quic_trace_visitor.cc index eeaf297..bf352f3 100644 --- a/quic/core/quic_trace_visitor.cc +++ b/quic/core/quic_trace_visitor.cc
@@ -108,6 +108,75 @@ } } +void QuicTraceVisitor::OnPacketSent( + QuicPacketNumber packet_number, + QuicPacketLength packet_length, + bool /*has_crypto_handshake*/, + TransmissionType /*transmission_type*/, + EncryptionLevel encryption_level, + const QuicFrames& retransmittable_frames, + const QuicFrames& /*nonretransmittable_frames*/, + QuicTime sent_time) { + quic_trace::Event* event = trace_.add_events(); + event->set_event_type(quic_trace::PACKET_SENT); + event->set_time_us(ConvertTimestampToRecordedFormat(sent_time)); + event->set_packet_number(packet_number.ToUint64()); + event->set_packet_size(packet_length); + event->set_encryption_level(EncryptionLevelToProto(encryption_level)); + + for (const QuicFrame& frame : retransmittable_frames) { + switch (frame.type) { + case STREAM_FRAME: + case RST_STREAM_FRAME: + case CONNECTION_CLOSE_FRAME: + case WINDOW_UPDATE_FRAME: + case BLOCKED_FRAME: + case PING_FRAME: + case HANDSHAKE_DONE_FRAME: + case ACK_FREQUENCY_FRAME: + PopulateFrameInfo(frame, event->add_frames()); + break; + + case PADDING_FRAME: + case MTU_DISCOVERY_FRAME: + case STOP_WAITING_FRAME: + case ACK_FRAME: + QUIC_BUG + << "Frames of type are not retransmittable and are not supposed " + "to be in retransmittable_frames"; + break; + + // New IETF frames, not used in current gQUIC version. + case NEW_CONNECTION_ID_FRAME: + case RETIRE_CONNECTION_ID_FRAME: + case MAX_STREAMS_FRAME: + case STREAMS_BLOCKED_FRAME: + case PATH_RESPONSE_FRAME: + case PATH_CHALLENGE_FRAME: + case STOP_SENDING_FRAME: + case MESSAGE_FRAME: + case CRYPTO_FRAME: + case NEW_TOKEN_FRAME: + break; + + // Ignore gQUIC-specific frames. + case GOAWAY_FRAME: + break; + + case NUM_FRAME_TYPES: + QUIC_BUG << "Unknown frame type encountered"; + break; + } + } + + // Output PCC DebugState on packet sent for analysis. + if (connection_->sent_packet_manager() + .GetSendAlgorithm() + ->GetCongestionControlType() == kPCC) { + PopulateTransportState(event->mutable_transport_state()); + } +} + void QuicTraceVisitor::PopulateFrameInfo(const QuicFrame& frame, quic_trace::Frame* frame_record) { switch (frame.type) {
diff --git a/quic/core/quic_trace_visitor.h b/quic/core/quic_trace_visitor.h index c9732cf..889d90a 100644 --- a/quic/core/quic_trace_visitor.h +++ b/quic/core/quic_trace_visitor.h
@@ -18,10 +18,21 @@ public: explicit QuicTraceVisitor(const QuicConnection* connection); + // TODO(wub): Delete when deprecating + // --quic_give_sent_packet_to_debug_visitor_after_sent. void OnPacketSent(const SerializedPacket& serialized_packet, TransmissionType transmission_type, QuicTime sent_time) override; + void OnPacketSent(QuicPacketNumber packet_number, + QuicPacketLength packet_length, + bool has_crypto_handshake, + TransmissionType transmission_type, + EncryptionLevel encryption_level, + const QuicFrames& retransmittable_frames, + const QuicFrames& nonretransmittable_frames, + QuicTime sent_time) override; + void OnIncomingAck(QuicPacketNumber ack_packet_number, EncryptionLevel ack_decrypted_level, const QuicAckFrame& ack_frame,
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h index 601daeb..595f435 100644 --- a/quic/core/quic_unacked_packet_map.h +++ b/quic/core/quic_unacked_packet_map.h
@@ -115,10 +115,13 @@ typedef std::deque<QuicTransmissionInfo> UnackedPacketMap; typedef UnackedPacketMap::const_iterator const_iterator; + typedef UnackedPacketMap::const_reverse_iterator const_reverse_iterator; typedef UnackedPacketMap::iterator iterator; const_iterator begin() const { return unacked_packets_.begin(); } const_iterator end() const { return unacked_packets_.end(); } + const_reverse_iterator rbegin() const { return unacked_packets_.rbegin(); } + const_reverse_iterator rend() const { return unacked_packets_.rend(); } iterator begin() { return unacked_packets_.begin(); } iterator end() { return unacked_packets_.end(); }
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index cf9969f..86f2003 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1366,12 +1366,26 @@ MockQuicConnectionDebugVisitor(); ~MockQuicConnectionDebugVisitor() override; + // TODO(wub): Delete when deprecating + // --quic_give_sent_packet_to_debug_visitor_after_sent. MOCK_METHOD(void, OnPacketSent, (const SerializedPacket&, TransmissionType, QuicTime), (override)); MOCK_METHOD(void, + OnPacketSent, + (QuicPacketNumber, + QuicPacketLength, + bool, + TransmissionType, + EncryptionLevel, + const QuicFrames&, + const QuicFrames&, + QuicTime), + (override)); + + MOCK_METHOD(void, OnCoalescedPacketSent, (const QuicCoalescedPacket&, size_t), (override));