From 158776a9795f1678c60bb868b5b8267886e8da78 Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Fri, 17 Jun 2022 17:34:46 -0400 Subject: [PATCH] Consumer: strip ring buffer header when consuming ctf2 ring buffer packet On the kernel tracer side, providing a header free packet would be quite intrusive in term of ABI/API modification. Thus for now, the kernel tracers keeps the notion of header. On the consumer side, we can drop the header as necessary based on the format we expect or we could even inspect the header if necessary. For now, we chose to trust the configured format for the session since we have access to the information. On the userspace side, historically the usage of a "ring buffer" for the metadata was somewhat driven by the ctf1 "requirement" of providing header for a given metadata packet. Essentially in ctf1 the ring buffer packet "metadata" leaks into the actual ctf1 spec. It also happened that the same machinery could then be reused for both the kernelspace and userspace metadata ingestions. CTF2 drop all this so could lttng-tools! But, considering that ctf1 still need to be supported for a while and that we still need to have all the machinery for consuming from the kernelspace metadata ringbuffer, keeping the current data path for both userspace and kernelspace for metadata is probably a safer bet. The only "drawback" here is that headers for the userspace metadata ringbuffer contain version fields that will be equivalent to 1.8 despite the fact that it contains ctf2 metadata. This is because the version is currently hard coded by lttng-ust into the metadata packet headers. Signed-off-by: Jonathan Rajotte Change-Id: I761c8a7d9742cbc9c875767233f2d30a43b9743f --- src/common/consumer/consumer-stream.cpp | 75 +++++++++++++++++++++---- src/common/consumer/consumer.hpp | 3 +- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/common/consumer/consumer-stream.cpp b/src/common/consumer/consumer-stream.cpp index 292f9cfbd..f7437947d 100644 --- a/src/common/consumer/consumer-stream.cpp +++ b/src/common/consumer/consumer-stream.cpp @@ -12,13 +12,11 @@ #include #include +#include #include #include -#include -#include #include #include -#include #include #include #include @@ -29,6 +27,25 @@ #include "consumer-stream.hpp" +struct metadata_packet_header { + uint32_t magic; /* 0x75D11D57 */ + uint8_t uuid[16]; /* Unique Universal Identifier */ + uint32_t checksum; /* 0 if unused */ + uint32_t content_size; /* in bits */ + uint32_t packet_size; /* in bits */ + uint8_t compression_scheme; /* 0 if unused */ + uint8_t encryption_scheme; /* 0 if unused */ + uint8_t checksum_scheme; /* 0 if unused */ + uint8_t major; /* CTF spec major version number */ + uint8_t minor; /* CTF spec minor version number */ + uint8_t header_end[0]; +}; + +static size_t metadata_length(void) +{ + return offsetof(struct metadata_packet_header, header_end); +} + /* * RCU call to free stream. MUST only be used with call_rcu(). */ @@ -80,11 +97,12 @@ static void consumer_stream_metadata_assert_locked_all(struct lttng_consumer_str } /* Only used for data streams. */ -static int consumer_stream_update_stats(struct lttng_consumer_stream *stream, - const struct stream_subbuffer *subbuf) +static int consumer_stream_update_stats( + struct lttng_consumer_stream *stream, struct stream_subbuffer *subbuf_) { int ret = 0; uint64_t sequence_number; + const stream_subbuffer *subbuf = subbuf_; const uint64_t discarded_events = subbuf->info.data.events_discarded; if (!subbuf->info.data.sequence_number.is_set) { @@ -469,12 +487,11 @@ end: * of the metadata stream in the kernel. If it was updated, set the reset flag * on the stream. */ -static -int metadata_stream_check_version(struct lttng_consumer_stream *stream, - const struct stream_subbuffer *subbuffer) +static void metadata_stream_check_version( + struct lttng_consumer_stream *stream, const struct stream_subbuffer *subbuffer) { if (stream->metadata_version == subbuffer->info.metadata.version) { - goto end; + return; } DBG("New metadata version detected"); @@ -484,8 +501,35 @@ int metadata_stream_check_version(struct lttng_consumer_stream *stream, if (stream->read_subbuffer_ops.reset_metadata) { stream->read_subbuffer_ops.reset_metadata(stream); } +} -end: +static void strip_packet_header_from_subbuffer(struct stream_subbuffer *buffer) +{ + /* + * Change the view and hide the packer header and padding from the view + */ + size_t new_subbuf_size = buffer->info.metadata.subbuf_size - metadata_length(); + + buffer->buffer.buffer = lttng_buffer_view_from_view( + &buffer->buffer.buffer, metadata_length(), new_subbuf_size); + + buffer->info.metadata.subbuf_size = new_subbuf_size; + /* Padding is not present in the view anymore */ + buffer->info.metadata.padded_subbuf_size = new_subbuf_size; +} + +static int metadata_stream_pre_consume_ctf1( + struct lttng_consumer_stream *stream, struct stream_subbuffer *subbuffer) +{ + (void) metadata_stream_check_version(stream, subbuffer); + return 0; +} + +static int metadata_stream_pre_consume_ctf2( + struct lttng_consumer_stream *stream, struct stream_subbuffer *subbuffer) +{ + (void) metadata_stream_check_version(stream, subbuffer); + (void) strip_packet_header_from_subbuffer(subbuffer); return 0; } @@ -747,8 +791,15 @@ struct lttng_consumer_stream *consumer_stream_create(struct lttng_consumer_chann consumer_stream_metadata_unlock_all; stream->read_subbuffer_ops.assert_locked = consumer_stream_metadata_assert_locked_all; - stream->read_subbuffer_ops.pre_consume_subbuffer = - metadata_stream_check_version; + if (trace_format == 1) { + stream->read_subbuffer_ops.pre_consume_subbuffer = + metadata_stream_pre_consume_ctf1; + } else if (trace_format == 2) { + stream->read_subbuffer_ops.pre_consume_subbuffer = + metadata_stream_pre_consume_ctf2; + } else { + abort(); + } } else { const post_consume_cb post_consume_index_op = channel->is_live ? consumer_stream_sync_metadata_index : diff --git a/src/common/consumer/consumer.hpp b/src/common/consumer/consumer.hpp index f8ae37c5a..891b650b5 100644 --- a/src/common/consumer/consumer.hpp +++ b/src/common/consumer/consumer.hpp @@ -348,8 +348,7 @@ typedef int (*extract_subbuffer_info_cb)( * * Stream and channel locks are acquired during this call. */ -typedef int (*pre_consume_subbuffer_cb)(struct lttng_consumer_stream *, - const struct stream_subbuffer *); +typedef int (*pre_consume_subbuffer_cb)(struct lttng_consumer_stream *, struct stream_subbuffer *); /* * Consume subbuffer contents. -- 2.34.1