search

quicwg / qlog

The IETF I-D documents for the qlog format
Other
84 stars 12 forks source link

Clean up connection states in connection_state_updated #239

Closed rmarx closed 3 months ago

rmarx commented 2 years ago

This QUIC event currently has a long list of possible connection states (both "simple" and "full"), but it hasn't been validated if these states are correct/logical wrt the final RFCs (list has been in there for a long while since early drafts). 

ConnectivityConnectionStateUpdated = {
    ? old: ConnectionState / SimpleConnectionState
    new: ConnectionState / SimpleConnectionState
}

ConnectionState =
    ; initial sent/received
    "attempted" /
    ; peer address validated by: client sent Handshake packet OR
    ; client used CONNID chosen by the server.
    ; transport-draft-32, section-8.1
    "peer_validated" /
    "handshake_started" /
    ; 1 RTT can be sent, but handshake isn't done yet
    "early_write" /
    ; TLS handshake complete: Finished received and sent
    ; tls-draft-32, section-4.1.1
    "handshake_complete" /
    ; HANDSHAKE_DONE sent/received (connection is now "active", 1RTT
    ; can be sent). tls-draft-32, section-4.1.2
    "handshake_confirmed" /
    "closing" /
    ; connection_close sent/received
    "draining" /
    ; draining period done, connection state discarded
    "closed"

SimpleConnectionState =
    "attempted" /
    "handshake_started" /
    "handshake_confirmed" /
    "closed"

Unlike for Stream states there doesn't seem to be a clear flow graph for Connection states in RFC9000, so it's not easy to distill something "RFC-esque".

My gut feeling would be to just keep the SimpleConnectionState and remove the rest, but that does miss some info...

This could use input from someone with deployment experience that wants to track connection state in this way. CC @LPardue @marten-seemann @lnicco

LPardue commented 2 years ago

Seems like the qlog crate didn't implement this properly, at the time of writing I only support the ConnectionState type. We don't actually generate any connectivity event types is quiche yet but could in future. If we did, I suspect having the option to log a value from ConnectionState would be useful. Especially the draining to closed transition, which seems to catch people out.

I'd probably be in favor of a single ConnectionState type and we can bikeshed some more on what values are actually kept inside that. Having both types seems odd, especially as one is a subset of the other.