qlog
qlog copied to clipboard
Should we use "* text => uint64" anywhere that a type can contain extension fields?
In H3 parameters_set , we have a
; additional settings for grease and extensions
* text => uint64
if I grok CDDL correctly, this allows us to have any number of pairs that would serialize like
"my_sekret_extension": 123,
"9999": 0
It got me thinking a bit. This isn't explained super clearly and people might overlook it. Maybe there's more use for this in some other types. Leading to, maybe we should define this as a generic tuple type in the base spec along with some text, then we can reuse it across various events?
So this is the intent of the * text => any
mention in the main schema here: https://www.ietf.org/archive/id/draft-ietf-quic-qlog-main-schema-07.html#name-events
So, arguably, the extra mention in parameters_set
should be removed/replaced with a reference to that instead.
I'm not a big fan of explicitly adding that to all events (too much clutter).
I don't think that works for my needs from a composition perspective. I want to extend $ProtocolEventBody
so that the extension is specific to a single type, not extend the entire event.
Discussed in meeting. Lucas: main intent is to formally allow any event to be extensible (instead of having to copy-paste all event parameters to a new event name). Currently possible in code of course, but not in proper CDDL schema. Robin: probably not possible without making everything a socket, but need to double check with CDDL defs.
So, turns out this is possible in a relatively simple way.
I thought we would have to make every event their own "type socket" (like $protocolEventBody
or $quicFrame
), but it turns out we can make do with "group sockets" (they use $$ instead of $ and don't require all the boilerplate the other sockets need). Both are described here: https://datatracker.ietf.org/doc/html/rfc8610#section-3.9
I have a more extensive example at #400, but that's only for 2 events (parameters_set
and parameters_restored
).
If we want to allow this for all events, this is possible, but would require adding something like this on the bottom of each event definition:
* $$quic-eventname-extension
That's basically all that's needed... (which is much less than what's needed for the "type sockets").
With that, you can specify new fields on a per-event basis by extending one specific extension point, which is IIUC what @LPardue asked here.
So say there's a new extension that adds 2 loss bits and you want to add those to the packet_sent event:
$$quic-packetsent-extension //= (
q_bit: bool
? l_bit: bool
)
Note that this can also be used for other things than events (e.g., H3Parameters
). It won't work for extending things like ENUMs (or e.g., H3Setting
), but for that we can still use the more verbose "type sockets" that we've been doing before if needed.
The main downside is that all the new fields are by definition optional when regarded in the complete schema, because for now, we of course don't have any extensions yet, so we have to do * $$quic-eventname-extension
(there will be zero or more) or ? $$quic-eventname-extension
(they are optional) in our current definition. So even if later extensions mandate a new field (as the example above, q_bit is mandatory), this wouldn't be enforced in the combined schema.
There are of course workarounds for this (e.g., script that gathers CDDL from all docs can remove the * or ? before the used $$ fields so they do properly track this), but it's good to be aware of this.
AFAICT, there is no way to prevent this without additional boilerplate (e.g., we could not use * or ?, but then we'd have to define an (empty) extension for each of these in the current documents... annoying).
I propose we track this general issue here going forward (instead of related #261, #176, #170, #124, #192, #297).
I could use some opinions on this here @LPardue @marten-seemann @lnicco :)
Nice. The caveats sound fine to me (but my understanding isn't as comprehensive as yours). It does sound to me that we'll probably want to have some support in place for future extensions to help get validated but that is outside the scope of the qlog documents themselves.
@LPardue yes, it's definitely possible to get future extensions to be properly validated by the CDDL tooling. The moment they have a concrete extension in place, it suffices to change * $$quic-eventname-extension
to $$quic-eventname-extension
(remove the *
) so the field will no longer be seen as "zero or more" but instead "one" and the tooling will properly track optional/required fields in the extension as expected (tested this with the ruby gem last week).
The proposal on #400 would allow me to support the send_at_time
custom field we have in quiche 👍
https://github.com/cloudflare/quiche/blob/master/qlog/src/events/quic.rs#L649
I think it would like like this
~~~~~~~~
$$quic-packetsent-extension //= (
? sent_at_time: float
)
~~~~~~~~
Discussed during editors meeting:
Merge #400 now
Robin: Create new PR that adds extension sockets to all events (and other types where needed, like H3Setting
), discuss during Brisbane meeting