Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extensibility for all events and official extension points #417

Merged
merged 17 commits into from
Jun 24, 2024

Conversation

rmarx
Copy link
Contributor

@rmarx rmarx commented Mar 18, 2024

Fixes #379.
Also closes #261, #176, #170, #124, #192, #297.

Adds extensibility for all events through the * $$category-name-ext pattern first used in #400, now extended to ALL events as discussed in #379.

Makes all official protocol extension points also extensible in qlog. This was mostly already the case except for a few holdouts ($H3StreamType, $ProtocolType, $TransportError). The full list currently is:

  • frames: $QuicFrame and $H3Frame and $H3Datagram
  • TPs: $$quic-parametersset-extension and $$quic-parametersrestored-extension
  • settings: $$h3-parameters-extension (and inherently H3Setting by the way it's defined)
  • H3 stream types: $H3StreamType
  • error codes: $ApplicationError, $TransportError
  • ALPN (at least qlog-level APLN; the other one is inherent in ALPNInformation): $ProtocolType

I don't think we need to make QUIC's StreamType extensible (since that's also in the RFC as always just uni- or bidirectional, with no way to change that through IANA (or even wire image, due to looking at the 2 LSBs)). Thoughts on this @LPardue ?

@rmarx
Copy link
Contributor Author

rmarx commented Mar 18, 2024

TODO for myself: we also need to make PacketHeader extensible (e.g., to be able to add lossbits, new fields for QUIC v3 etc.). Same for PacketType and PacketNumberSpace (TODO: also double-check others)

@rmarx
Copy link
Contributor Author

rmarx commented Jun 20, 2024

Did some final work on this today. Should be ready for final review + merge @LPardue @marten-seemann

A note: as mentioned in the original PR message, this intentionally does NOT make QUIC Stream Type (uni vs bidi) extensible, since that's not supported by v1 and would require quite a different approach (though not really wire image changes I guess?). So still not sure if we should include this or not... would welcome some thoughts ;)

@rmarx rmarx requested a review from LPardue June 20, 2024 14:35
@rmarx rmarx merged commit 41fec12 into main Jun 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants