Is it possible to include a packet sequence count?

[jonlamb-gh]

Looking through the docs under packet features, it doesn't look like including a packet_seq_num is supported. If I didn't overlook the docs, is this a feature that would be considered given that it's called out in the spec? It'd be really handy to have this in the CTF packet rather than having to manage it elsewhere.

[jonlamb-gh]

I thought I could manage this in my platform impl by adding packet_seq_num to my packet-context-field-type-extra-members as a workaround, but it appears to be a reserved field type member name.

[jonlamb-gh]

I posted this question to the mailing list, feel free to close if that's the preferred comms channel.

[eepp]

I posted this question to the mailing list, feel free to close if that's the preferred comms channel.

Here is alright.

Having such an automatic field could work indeed.

I don't have much time to spend on implementing barectf features currently, but feel free to suggest an approach, then we can discuss it, and you can submit a corresponding PR.

[jonlamb-gh]

After digging around a bit, it seems like this would fit well under the packet features object, probably named seq-num-field-type and the generated metadata field type name packet_seq_num to be compatible with other parts of the ecosystem. I think making it default to off makes sense so we don't change the behavior of existing configs, but I suppose it could go
either way to align with the other packet features. I'm not sure whether to generate the new state as part of the parent struct barectf_ctx or the generated stream-specific sctx. The latter is where other config related generated fields end up. Does mean we make the assumption that the platform will zero-initialize the data since we'd just be incrementing it (probably when closing a packet), but that doesn't seem unreasonable to me. How does that sound @eepp ?

[eepp]

The packet sequence number was added to CTF to support the overwrite mode of an LTTng channel:

Since LTTng 2.8, with this mode, LTTng writes to a given sub-buffer its sequence number within its data stream. With a local, network streaming, or live recording session, a trace reader can use such sequence numbers to report lost packets. A trace reader can use the saved discarded sub-buffer (packet) count of the trace to decide whether or not to perform some analysis even if trace data is known to be missing.

With this mode, LTTng doesn’t write to the trace the exact number of lost event records in the lost sub-buffers.

Am I understanding you correctly that you need the packet sequence number feature in barectf without having support for an equivalent overwrite mode?

If not, what's your use case then?

Indeed I don't see why we couldn't introduce this feature without some overwrite mode one.

After digging around a bit, it seems like this would fit well under the packet features object

Yes, in $features/packet, named sequence-number-field-type.

I think making it default to off makes sense so we don't change the behavior of existing configs, but I suppose it could go either way to align with the other packet features.

I believe it needs to be disabled by default for backward compatibility reasons as having it enabled by default would mean that a barectf minor update would bring:

• Fewer packet content bits.
• CPU clocks spent to write it.

The latter is where other config related generated fields end up.

Stream-specific yes.

Does mean we make the assumption that the platform will zero-initialize

Fields are individually initialized in barectf_init() (as of 3.0.0):

{% include 'c/ctx-init-func-proto.j2' %}

{
    struct {{ ctx_struct_name }} * const ctx = _FROM_VOID_PTR(struct {{ ctx_struct_name }}, vctx);
    ctx->cbs = cbs;
    ctx->data = data;
    ctx->buf = buf;
    ctx->packet_size = _BYTES_TO_BITS(buf_size);
    ctx->at = 0;
    ctx->events_discarded = 0;
    ctx->packet_is_open = 0;
    ctx->in_tracing_section = 0;
    ctx->is_tracing_enabled = 1;
    ctx->use_cur_last_event_ts = 0;
}

Tell me, would it make sense to provide some generated C API to set or reset the sequence number of a given barectf context? Something that the platform could call at packet opening/closing time? I'm not sure; if you don't need it, then maybe I can add those functions in a future version if there's demand.

You want to give this a try?

[jonlamb-gh]

The packet sequence number was added to CTF to support the overwrite mode of an LTTng channel:

Thanks for the link and info, I hadn't come across that before.

Am I understanding you correctly that you need the packet sequence number feature in barectf without having support for an equivalent overwrite mode?

That's right. FWIW my use case is two fold. The MCUs I've instrumented are writing data to a packet ring buffer in memory. Depending on the deployment, I'm either sending the packets over a serial interface or I'm reading the blob of memory containing the ring buffer over JTAG periodically.

Having the packet sequence number in the data is very convenient. I can establish ordering without looking at the clock and I can do missed packet detection on the recv side if I decide to send these out over UDP at some point.

I think it'll also be handy if I want to forward it along to lttng-relayd for live viewing in trace compass or inspection with babeltrace CLI.

Fields are individually initialized in barectf_init() (as of 3.0.0):

:+1: I was just curious if it should be put in struct barectf_ctx or the generated struct {{ prefix }}{{ dst.name }}_ctx. The former is initialized like you mention, but the later is not IIUC. Sounds like putting it inside struct barectf_ctx is the write spot, and we can initialize it alongside the other fields.

Tell me, would it make sense to provide some generated C API to set or reset the sequence number of a given barectf context? Something that the platform could call at packet opening/closing time? I'm not sure; if you don't need it, then maybe I can add those functions in a future version if there's demand.

I don't need those API calls for my use case, my application semantics will interpret a reset of the sequence number as either a rollover or a device reset, which is domain specific. I can see that being generally useful for others however.

You want to give this a try?

Definitely!

[jonlamb-gh]

I remember now why I thought the new field had to go in struct {{ prefix }}{{ dst.name }}_ctx rather than struct {{ prefix }}ctx: the field type is part of the stream/packet features and struct {{ prefix }}{{ dst.name }}_ctx is generated based on that so it already has the necessary context in the templates, I think we'd have to rework things a bit in order for struct {{ prefix }}ctx to get this field.

[jonlamb-gh]

Should be pretty straight forward to do the initialization in barectf_init() still.

[eepp]

Yes it would make sense for it to be part of struct barectf_default_ctx (let's call it this way), but the current generated API only offers a common initialization function which only knows struct barectf_ctx.

Therefore, in the meantime, add it at the end of struct barectf_ctx, where events_discarded is.

[eepp]

Please let me handle any documentation (no changes in docs/).