-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add support for QUIC v2 #16
Conversation
7ceb1eb
to
85ed196
Compare
85ed196
to
f65457f
Compare
@bbannier , any chance you might look this over? Not sure the C++ refactor makes it much better, but hope that with more versions more streamlining happens... |
f65457f
to
59834e9
Compare
59834e9
to
2ca929e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I am absolutely no fan of the static variables since they make it very hard to follow what is going on, but they seem safe right now and we seem to need them since otherwise getting the right data to the calls in the new class hierarchy is pretty tedious.
As an exercise I tested whether changing all the static std::vector
to std::array
made any difference (also required making HkdfCtxParam
templated over the array size, and changing QuicPacketProtection
a variadic function to allow different param types and inside handling a la https://stackoverflow.com/questions/7230621/how-can-i-iterate-over-a-packed-variadic-template-argument-list/60136761#60136761 since the types become heterogenous and a loop cannot work). That made no measurable performance difference, likely since this code is already executed as little as possible.
Attempt to refactor in order to re-use common code between the two versions.
QUIC v2 changed the version *and* the packet type enumeration to prevent protocol ossification. Use an intermediary unit to handle the difference.
Produced using examples from the go-quic project, patching the clients to force QUIC v2.
This makes the analyzer.log entry more informative by including the actual version and also allows to handle this scenario in script land if needed.
2ca929e
to
6c55e7f
Compare
No description provided.