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.

Sign up for GitHub

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

Jump to bottom

Logging unknown TransportParameters #436

Open
LPardue opened this issue Oct 15, 2024 · 7 comments · May be fixed by #438 or #440
Open

Logging unknown TransportParameters #436

LPardue opened this issue Oct 15, 2024 · 7 comments · May be fixed by #438 or #440

Comments

@LPardue
Copy link
Member

LPardue commented Oct 15, 2024 •
edited
Loading

Back in #176 I said

The spec would be improved by having a well-defined "extension bucket" field that TPs can go in, an array of integers tuples would be fine, for example.

and we addressed that with a structured solution using group sockets in #417.

However, I think one original problem still stands. As a receiver, if I receive an unknown TP, there's not a standard way to log the codepoint and value. H3 already has a solution to this via the SETTINGS frame but QUIC TPs are different and it isn't so straightforward.

@hawkinsw has this problem right now. He is doing research on the Accurate ECN extension and very kindly proposed some en/decode to quiche. And while a small patch, adding code for an I-D we don't implement (and might want to track all I-D revisions that change the code point) adds overhead. So it got me wondering could we use qlog instead and require zero changes, leading back here.

For example, by extending parameters_set as below, we could easily externalise the code that checks supported TPs to log tooling

QUICParametersSet = {
  // all the existing stuff
  
  unknown_parameters: [* UnknownTransportParameter]
  
}

UnknownTransportParameter= {
    id: uint64,
    value: uint64
}
@hawkinsw
Copy link

@LPardue This sounds like an excellent idea! In fact, it sounds like something that I would have fun implementing! Would you allow me a crack at it?

@rmarx
Copy link
Contributor

rmarx commented Oct 15, 2024

@LPardue Looking it over, it's a bit of a mess :)

For H3 we indeed have the option to log as unknown in the H3SettingsFrame, but not in the h3:parameters_set event.
In QUIC, we don't have the equivalent to H3SettingsFrame (not seriously considering adding TLS events though ;)) and so indeed this kind of falls through the cracks a bit.

So in general the proposed solution of adding unknown_parameters to quic:parameters_set looks good to me, and let's do that, but I'm wondering if the h3:parameters_set discrepancy is a problem in practice or not (probably not, but still... it's an annoying asymmetry).

Also: hey @hawkinsw, great to see you're still doing QUIC stuff and wandering over into qlog country :)

@hawkinsw
Copy link

@rmarx I am just trying to keep up with you and @LPardue !! I will take a crack at doing some "good" here and let you know how I progress. I won't get a chance to start on it until tomorrow AM. Thank you for both being so welcoming!

@LPardue
Copy link
Member Author

LPardue commented Oct 15, 2024

@rmarx yeah I don't like the asymmetry much either. Although I think some of that can be put down to H3 extensions being a little more loosy goosy than QUIC. Adding a bucket to h3 params set sound fine to me.

@hawkinsw I'm not sure if your volunteering to update the qlog spec, or to experiment with quiche, or both. But whatever you'd like to volunteer is appreciated!

@hawkinsw hawkinsw mentioned this issue Oct 16, 2024
Open
@hawkinsw
Copy link

@LPardue and @rmarx Thank you both for your support. If the PR (referenced above) looks like it is on the right track, I will take a whack and opening something for the qlog spec!

Thanks again!

@rmarx
Copy link
Contributor

rmarx commented Oct 17, 2024

Discussed in editors meeting. No clear preference to add symmetric option for this in h3:parameters-set event as well, so the one who creates the PR gets to choose what it'll be :)

@rmarx rmarx linked a pull request Oct 18, 2024 that will close this issue
Open
hawkinsw added a commit to hawkinsw/qlog that referenced this issue Oct 18, 2024
@hawkinsw
Add support for logging unknown parameters
03fb0f8 
Add support for logging unknown parameters (including parameters set
during connection recovery and parameters restored during 0-RTT).

Closes quicwg#436.
@hawkinsw hawkinsw linked a pull request Oct 18, 2024 that will close this issue
Open
@hawkinsw
Copy link

@rmarx It looks like you might have beaten me to the punch. I'm sorry I was slow. I tried my best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@rmarx @LPardue @hawkinsw