Internal change PiperOrigin-RevId: 547813805
diff --git a/quiche/quic/core/batch_writer/quic_batch_writer_base.cc b/quiche/quic/core/batch_writer/quic_batch_writer_base.cc index fae805f..666406f 100644 --- a/quiche/quic/core/batch_writer/quic_batch_writer_base.cc +++ b/quiche/quic/core/batch_writer/quic_batch_writer_base.cc
@@ -62,6 +62,7 @@ bool buffered = false; bool flush = can_batch_result.must_flush; + uint32_t packet_batch_id = 0; if (can_batch_result.can_batch) { QuicBatchWriterBuffer::PushResult push_result = @@ -72,6 +73,7 @@ buffered = true; // If there's no space left after the packet is buffered, force a flush. flush = flush || (batch_buffer_->GetNextWriteLocation() == nullptr); + packet_batch_id = push_result.batch_id; } else { // If there's no space without this packet, force a flush. flush = true; @@ -93,9 +95,10 @@ if (result.status != WRITE_STATUS_OK) { if (IsWriteBlockedStatus(result.status)) { - return WriteResult( - buffered ? WRITE_STATUS_BLOCKED_DATA_BUFFERED : WRITE_STATUS_BLOCKED, - result.error_code); + return WriteResult(buffered ? WRITE_STATUS_BLOCKED_DATA_BUFFERED + : WRITE_STATUS_BLOCKED, + result.error_code) + .set_batch_id(packet_batch_id); } // Drop all packets, including the one being written. @@ -116,6 +119,7 @@ peer_address, options, params, release_time.actual_release_time); buffered = push_result.succeeded; + packet_batch_id = push_result.batch_id; // Since buffered_writes has been emptied, this write must have been // buffered successfully. @@ -126,6 +130,7 @@ } result.send_time_offset = release_time.release_time_offset; + result.batch_id = packet_batch_id; return result; }
diff --git a/quiche/quic/core/batch_writer/quic_batch_writer_buffer.cc b/quiche/quic/core/batch_writer/quic_batch_writer_buffer.cc index d283e2b..6fa7059 100644 --- a/quiche/quic/core/batch_writer/quic_batch_writer_buffer.cc +++ b/quiche/quic/core/batch_writer/quic_batch_writer_buffer.cc
@@ -82,6 +82,17 @@ } else { // In place push, do nothing. } + if (buffered_writes_.empty()) { + // Starting a new batch. + ++batch_id_; + + // |batch_id| is a 32-bit unsigned int that is possibly shared by a lot of + // QUIC connections(because writer can be shared), so wrap around happens, + // when it happens we skip id=0, which indicates "not batched". + if (batch_id_ == 0) { + ++batch_id_; + } + } buffered_writes_.emplace_back( next_write_location, buf_len, self_address, peer_address, options ? options->Clone() : std::unique_ptr<PerPacketOptions>(), params, @@ -90,6 +101,7 @@ QUICHE_DCHECK(Invariants()); result.succeeded = true; + result.batch_id = batch_id_; return result; }
diff --git a/quiche/quic/core/batch_writer/quic_batch_writer_buffer.h b/quiche/quic/core/batch_writer/quic_batch_writer_buffer.h index 62282a3..369ecff 100644 --- a/quiche/quic/core/batch_writer/quic_batch_writer_buffer.h +++ b/quiche/quic/core/batch_writer/quic_batch_writer_buffer.h
@@ -38,6 +38,8 @@ // in-place push. // Only valid if |succeeded| is true. bool buffer_copied; + // The batch ID of the packet. Only valid if |succeeded|. + uint32_t batch_id = 0; }; PushResult PushBufferedWrite(const char* buffer, size_t buf_len, @@ -88,6 +90,11 @@ const char* buffer_end() const { return buffer_ + sizeof(buffer_); } ABSL_CACHELINE_ALIGNED char buffer_[kBufferSize]; quiche::QuicheCircularDeque<BufferedWrite> buffered_writes_; + // 0 if a batch has never started. Otherwise + // - If |buffered_writes_| is empty, this is the ID of the previous batch. + // - If |buffered_writes_| is not empty, this is the ID of the current batch. + // For debugging only. + uint32_t batch_id_ = 0; }; } // namespace quic
diff --git a/quiche/quic/core/batch_writer/quic_batch_writer_buffer_test.cc b/quiche/quic/core/batch_writer/quic_batch_writer_buffer_test.cc index ffa88f0..747ff1f 100644 --- a/quiche/quic/core/batch_writer/quic_batch_writer_buffer_test.cc +++ b/quiche/quic/core/batch_writer/quic_batch_writer_buffer_test.cc
@@ -278,6 +278,34 @@ nullptr, params); } +TEST_F(QuicBatchWriterBufferTest, BatchID) { + const int kNumBufferedWrites = 10; + QuicPacketWriterParams params; + auto first_push_result = batch_buffer_->PushBufferedWrite( + packet_buffer_, kDefaultMaxPacketSize, self_addr_, peer_addr_, nullptr, + params, release_time_); + ASSERT_TRUE(first_push_result.succeeded); + ASSERT_NE(first_push_result.batch_id, 0); + for (int i = 1; i < kNumBufferedWrites; ++i) { + EXPECT_EQ(batch_buffer_ + ->PushBufferedWrite(packet_buffer_, kDefaultMaxPacketSize, + self_addr_, peer_addr_, nullptr, params, + release_time_) + .batch_id, + first_push_result.batch_id); + } + + batch_buffer_->PopBufferedWrite(kNumBufferedWrites); + EXPECT_TRUE(batch_buffer_->buffered_writes().empty()); + + EXPECT_NE( + batch_buffer_ + ->PushBufferedWrite(packet_buffer_, kDefaultMaxPacketSize, self_addr_, + peer_addr_, nullptr, params, release_time_) + .batch_id, + first_push_result.batch_id); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 0c29444..e95c8d1 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -3540,7 +3540,7 @@ sent_packet_manager_.unacked_packets() .rbegin() ->retransmittable_frames, - packet->nonretransmittable_frames, packet_send_time); + packet->nonretransmittable_frames, packet_send_time, result.batch_id); } } if (packet->encryption_level == ENCRYPTION_HANDSHAKE) { @@ -5020,6 +5020,8 @@ packet->encrypted_buffer, packet->encrypted_length, self_address.host(), peer_address, writer, GetEcnCodepointToSend(peer_address)); + const uint32_t writer_batch_id = result.batch_id; + // If using a batch writer and the probing packet is buffered, flush it. if (writer->IsBatchMode() && result.status == WRITE_STATUS_OK && result.bytes_written == 0) { @@ -5051,7 +5053,7 @@ sent_packet_manager_.unacked_packets() .rbegin() ->retransmittable_frames, - packet->nonretransmittable_frames, packet_send_time); + packet->nonretransmittable_frames, packet_send_time, writer_batch_id); } }
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 6191fdd..cce16d0 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -281,7 +281,7 @@ EncryptionLevel /*encryption_level*/, const QuicFrames& /*retransmittable_frames*/, const QuicFrames& /*nonretransmittable_frames*/, - QuicTime /*sent_time*/) {} + QuicTime /*sent_time*/, uint32_t /*batch_id*/) {} // Called when a coalesced packet is successfully serialized. virtual void OnCoalescedPacketSent(
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 12cea2f..d91c041 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -6984,10 +6984,10 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)).Times(1); connection_.SendStreamDataWithString(1, "foo", 0, NO_FIN); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)).Times(1); connection_.SendConnectivityProbingPacket(writer_.get(), connection_.peer_address()); } @@ -7114,7 +7114,7 @@ CongestionBlockWrites(); connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)).Times(1); EXPECT_CALL(debug_visitor, OnPingSent()).Times(1); connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); EXPECT_FALSE(connection_.HasQueuedData()); @@ -7126,7 +7126,7 @@ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)).Times(1); EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); connection_.SendControlFrame(QuicFrame(QuicBlockedFrame(1, 3, 0))); EXPECT_EQ(1u, connection_.GetStats().blocked_frames_sent); @@ -7142,7 +7142,7 @@ QuicBlockedFrame blocked(1, 3, 0); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0); + 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); @@ -9907,7 +9907,7 @@ } MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(3); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)).Times(3); EXPECT_CALL(debug_visitor, OnCoalescedPacketSent(_, _)).Times(1); EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1); { @@ -15700,7 +15700,7 @@ connection_.set_debug_visitor(&debug_visitor); uint64_t debug_visitor_sent_count = 0; - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)) + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _, _)) .WillRepeatedly([&]() { debug_visitor_sent_count++; }); EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
diff --git a/quiche/quic/core/quic_trace_visitor.cc b/quiche/quic/core/quic_trace_visitor.cc index 388eebb..7906536 100644 --- a/quiche/quic/core/quic_trace_visitor.cc +++ b/quiche/quic/core/quic_trace_visitor.cc
@@ -52,7 +52,8 @@ 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) { + const QuicFrames& /*nonretransmittable_frames*/, QuicTime sent_time, + uint32_t /*batch_id*/) { quic_trace::Event* event = trace_.add_events(); event->set_event_type(quic_trace::PACKET_SENT); event->set_time_us(ConvertTimestampToRecordedFormat(sent_time));
diff --git a/quiche/quic/core/quic_trace_visitor.h b/quiche/quic/core/quic_trace_visitor.h index 6a8c442..28e4d68 100644 --- a/quiche/quic/core/quic_trace_visitor.h +++ b/quiche/quic/core/quic_trace_visitor.h
@@ -24,7 +24,7 @@ EncryptionLevel encryption_level, const QuicFrames& retransmittable_frames, const QuicFrames& nonretransmittable_frames, - QuicTime sent_time) override; + QuicTime sent_time, uint32_t batch_id) override; void OnIncomingAck(QuicPacketNumber ack_packet_number, EncryptionLevel ack_decrypted_level,
diff --git a/quiche/quic/core/quic_types.h b/quiche/quic/core/quic_types.h index 961e904..ae9a4fb 100644 --- a/quiche/quic/core/quic_types.h +++ b/quiche/quic/core/quic_types.h
@@ -158,10 +158,19 @@ QUIC_EXPORT_PRIVATE friend std::ostream& operator<<(std::ostream& os, const WriteResult& s); + WriteResult& set_batch_id(uint32_t batch_id) { + this->batch_id = batch_id; + return *this; + } + WriteStatus status; // Number of packets dropped as a result of this write. // Only used by batch writers. Otherwise always 0. uint16_t dropped_packets = 0; + // The batch id the packet being written belongs to. For debugging only. + // Only used by batch writers. Only valid if the packet being written started + // a new batch, or added to an existing batch. + uint32_t batch_id = 0; // The delta between a packet's ideal and actual send time: // actual_send_time = ideal_send_time + send_time_offset // = (now + release_time_delay) + send_time_offset
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index 4a054f3..3d58222 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -1318,7 +1318,8 @@ MOCK_METHOD(void, OnPacketSent, (QuicPacketNumber, QuicPacketLength, bool, TransmissionType, - EncryptionLevel, const QuicFrames&, const QuicFrames&, QuicTime), + EncryptionLevel, const QuicFrames&, const QuicFrames&, QuicTime, + uint32_t), (override)); MOCK_METHOD(void, OnCoalescedPacketSent, (const QuicCoalescedPacket&, size_t),