lib, qlog: Track and Log Unknown Transport Parameters #1861
Conversation
|
@LPardue Let me know what you think about the direction of this PR. If you think that it is on the right track, I will gladly do whatever you say so that it meets the high standards of the code that you have already written in this library. In re: quicwg/qlog#436 and (potentially) quicwg/qlog#176. |
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
13da4a9
to
7e6b807
Compare
quiche/src/lib.rs
Outdated
| _ => (), | ||
| // Track unknown transport parameters specially. | ||
| unknown_tp_id => { | ||
| tp.unknown_params.push(UnknownTransportParameter { |
There was a problem hiding this comment.
we probably want to put an upper bound on the number of these that we would store, because I don't think there's limit in TLS (other than the full length of the QUIC crypto stream).
I don't know what the bounds should be right now, but as inspiration we have something alongthe same lines for SETTINGS -
Line 50 in 9ea2152
There was a problem hiding this comment.
Let me know if you like the latest revision. I decided against using smallvec because I don't think that users will see unknown parameters often enough to warrant pre-allocating space for any entries.
quiche/src/lib.rs
Outdated
| @@ -8952,6 +8989,7 @@ mod tests { | |||
| initial_source_connection_id: Some(b"woot woot".to_vec().into()), | |||
| retry_source_connection_id: Some(b"retry".to_vec().into()), | |||
| max_datagram_frame_size: Some(32), | |||
| unknown_params: vec![], | |||
There was a problem hiding this comment.
Its sad we can't test this, but that's because we don't allow apps to send unknown values via the quiche API. I was partly suprised to see we don't send grease TPS (https://datatracker.ietf.org/doc/html/rfc9000#name-reserved-transport-paramete) so maybe that's something we could consider. Although maybe doing it separately might be wise (and the tests could be updated if/when that happens)
There was a problem hiding this comment.
I believe that I have found a way to do at least a little testing on the new feature. Let me know what you think!
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
2 times, most recently
from
1e51a7a
to
8e69291
Compare
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
8e69291
to
82ec65f
Compare
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
2 times, most recently
from
90c52bc
to
1394996
Compare
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
1394996
to
746c9cb
Compare
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
746c9cb
to
c17adb1
Compare
|
Sorry it took me so long, @LPardue ! If this version looks okay, I will start in on determining how to handle FFI! |
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
c17adb1
to
708c724
Compare
Track (and make available to qlog for logging) transport parameters and their values that this implementation does not specifically recognize.
hawkinsw
force-pushed
the
unknown_transport_parameters
branch
from
708c724
to
272230d
Compare
Track (and make available to qlog for logging) transport parameters and their values that this implementation does not specifically recognize.