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

Improve quic:parameters_set #369

Closed
rmarx opened this issue Jan 19, 2024 · 3 comments
Closed

Improve quic:parameters_set #369

rmarx opened this issue Jan 19, 2024 · 3 comments

Comments

@rmarx
Copy link
Contributor

rmarx commented Jan 19, 2024

As reported by @hlandau on the mailing list:

quic:parameters_set:

As things stand, there is no facility for logging the reception of unknown
transport parameters by an endpoint. Since transport parameters are used to
negotiate extensions to QUIC, having logged data about the presence of
transport parameters sent by a peer but not understood, and the consequential
ability to get information about the prevalence of any given unsupported
transport parameter in incoming traffic, seems like it would be quite useful.

I think this event needs more clarify in terms of the distinction between
attempting to negotiate a feature and it being enabled. It should be possible
to log the feature set a local or remote endpoint is requesting and it
should be possible to log the actually negotiated feature set. Possibly
this can be resolved via simple clarification. If a client connects to
a server, does it start by emitting parameters_set (owner=local) with (e.g.)
max_datagram_frame_size set, and then when the server responds without
it, emit parameters_set (owner=remote) without it? I guess this allows
the necessary information to be inferred.

If the absence of a field indicates it was not negotiated after receiving
transport parameters from a server, this creates a problem if this event has
to be used to log other parameters determined at different times (which seems
plausible for tls_cipher, early_data_enabled, etc.), as then logging
those parameters could be misinterpreted as a sign that the transport
parameters have been received and a feature has not been negotiated.
Either the cause of a parameters_set event should be clarified (maybe a trigger
field) or possibly this should be split into different event types.

I think it would be valuable to have the ability to log information
such as spin bit policy (e.g. enabled, disabled due to random chance,
disabled administratively), independent of the actual spin_bit_updated
event).

@LPardue
Copy link
Member

LPardue commented Jan 24, 2024

This is a bunch of different issues that need to be split out.

Supporting unknown TPs is already tracked via #176 - we should fix that.

Regarding "negotiation" each TP will do things differently. For example, some TPs like initial flow control are a statement of what an endpoint is willing to receive. Other fields, like idle timeout, are a declaration that then goes on to decide the connection's idle timeout via combination of both TPs.

The datagram example is a good one. If a server doesn't send a max_datagram_frame_size TP or it sends a value of 0, the client can't send them. If its a problem, the client would close the connection. If its not a problem, the client won't close the connection. But that's a facet of the application that uses QUIC. I'm not really sure there is anything we can standardise easily here. Even more so once we allow for logging of arbitrary unknown TPs.

Keeping things as a matter of what locally set, vs remotely received helps alleviate the trigger question IMO.

Regarding spin bit policy, that's a separate issue. Personally I'm not interested in spending effort on that but if others want to propose text that fine.

@hlandau
Copy link

hlandau commented Jan 24, 2024

I think our understanding of the issues is in agreement. I do think some work is needed here and that the "owner" field isn't enough.

@rmarx
Copy link
Contributor Author

rmarx commented Feb 28, 2024

I decided to split this off for easier tracking.

The first point is indeed tracked in #176 and will be continued there.
The second point is now at #398, the third at #399.

Closing this one because of that :)

@rmarx rmarx closed this as completed Feb 28, 2024
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

3 participants