qlog icon indicating copy to clipboard operation
qlog copied to clipboard

Difference between negotiated vs actually accepted/enabled TPs

Open rmarx opened this issue 4 months ago • 2 comments

Spinoff/splitoff from #369.

@hlandau originally reported for quic:parameters_set:

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.

rmarx avatar Feb 28 '24 11:02 rmarx

There are imo 2 things here, 1 per paragraph:

  1. The intent is indeed to log (at least) 2 parameters_set events. One with owner=local for the ones you send to the peer, and one with owner=remote with ones you receive from the peer. Comparing the values in the two SHOULD be enough to deduce the actually negotiated features/values. If not, the solution would be to emit a third parameters_set event reflecting the final value (e.g., at the client, you'd have owner=local, then owner=remote for what you got from the server and then again owner=local to reflect the final values you "set" at the client side that will be used). I don't immediately see an existing TP for which this approach would not work. Do you think this is sufficient to resolve your first paragraph @hlandau? If it's indeed a question of a "simple clarification" being needed, would you be able to suggest text for such a clarification that would make it easier for you?

  2. I must admit I don't fully understand this part. Specifically "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". Could you give a concrete example of such a case happening? An important aspect here is that the naming is a bit annoying, as it's parameters_set but it's not just the literal "transport parameters" but much more general "parameters" as a whole. So using the event to log non-TP parameters (e.g., early_data_enabled) MUST NOT be inferred to say that this also means that TPs have been received/negotiated (maybe that's also just a clarification then?). If you still feel a trigger would be a good solution, could you give a few examples of such triggers to help my understanding? Thanks :)

rmarx avatar Feb 28 '24 11:02 rmarx

1 sounds good to me and makes sense.

For 2, an example scenario:

  • Client sends handshake, TPs include idle timeout TP, parameters_set(owner=local, idle_timeout=...) logged
  • Server sends handshake, TP include idle timeout TP, parameters_set(owner=remote, idle_timeout=...) logged
  • Some other event on the client happens which causes parameters_set(owner=local, foo=...) to be logged
  • This obviously should not imply idle_timeout is no longer configured.
  • But for a parameters_set event which is TP-triggered, absence of idle_timeout seems to imply it was not sent.

OK, thinking about it, is the intention here to have "stateful" handling of this where a qlog consumer models the current state as the product of all events? i.e.,

curParams.idleTimeout = UNSET
for each params event:
  if event.idleTimeout is present:
    curParams.idleTimeout = event.idleTimeout

That works but should be made clear. Can a param transition back to its "unset" value using null explicitly? Or is null the same as a field being missing?

Here's some wording I've come up with quickly, feel free to mess with it:

Some parameters are the result of negotiation between peers. In this case, one
endpoint generally suggests a possible value for a parameter and its peer then
suggests another possible value. The value used is the result of some
parameter-specific function applied over those two suggested values.
Such negotiation SHOULD be expressed as a series of `parameters_set` events.
For example:

- The local endpoint logs a `parameters_set` event with an `owner` of `local`
  reflecting the negotiated parameter values it would like to use.
- The local endpoint receives parameter values from the peer and logs a
  `parameters_set` event with an `owner` of `remote` reflecting the desired
  values expressed by the peer.
- The local endpoint logs a `parameters_set` event with an `owner` of `local`
  reflecting the negotiated parameter values determined from the two sets
  of paramater values communicated by the local and remote endpoints.

Each parameter in a `parameters_set` event supersedes the previous value of that
parameter if present. If a parameter does not appear in a given `parameters_set`
event, its value is unchanged from the previous event which set that parameter.
If a parameter never appears in a `parameters_set` event, it retains its default
value. A default value may be restored by specifying a JSON value of `null` for
a parameter, which is semantically distinct from the absence of a parameter in a
`parameters_set` event.

hlandau avatar Mar 06 '24 00:03 hlandau