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

Check that we can log version mismatch errors appropriately #241

Open
rmarx opened this issue Sep 28, 2022 · 1 comment
Open

Check that we can log version mismatch errors appropriately #241

rmarx opened this issue Sep 28, 2022 · 1 comment

Comments

@rmarx
Copy link
Contributor

rmarx commented Sep 28, 2022

As noted in #138 (comment), version mismatch problems causing a connection close are logged in 3 ways currently:

  1. with a version_mismatch trigger value in connection_closed
  2. by logging the on the wire versions in packet_sent and packet_received .supported_versions
  3. by emitting a separate version_information event grouping both external and internal (see below)
TransportVersionInformation = {
    ? server_versions: [+ QuicVersion]
    ? client_versions: [+ QuicVersion]
    ? chosen_version: QuicVersion
}

This feels clunky, but probably gets the job done, with 1 proposed change:
Currently QuicVersion is defined as : hexstring and I would define it as : hexstring / "none" to support the mismatch case explicitly in that event as well (currently you'd omit the chosen_version field to signal the same).

Another change we could do is to add ? supported_versions: [+ QuicVersion] to the server_listening event to make it more explicit which versions a server supports at startup (currently no real way to indicate that). Not sure this is crucial though.


One of the main points however is that I'm not sure this:

  1. allows logging of all the possible flows with the VersionNegotiation extension . Maybe someone who's implemented that logic can give some insight? CC @huitema

  2. should even be an issue for the qlog documents (as we're leaning towards having separate qlog documents for separate QUIC features/extensions). CC @LPardue

@LPardue
Copy link
Member

LPardue commented Dec 9, 2023

We closed the PR that was linked to this (#138) so can we just close the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants