Refactor imquic to use picoquic as the underlying QUIC stack#22
Merged
Refactor imquic to use picoquic as the underlying QUIC stack#22
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a big refactoring that I had planned for a while but always delayed, because I knew it would take some considerable work.
When I first created imquic, it was based on a QUIC stack I wrote from the grounds up, while I was learning the ropes. This stack, while simple, did the job for a while, and definitely allowed me to do a deep dive in the protocol and experiment with different testbeds, MoQ included. At the same time, though, I always acknowledged it as the weak link in the library: QUIC is not a simple protocol, and it's hard to get right, and as a matter of fact there was a ton of stuff I had not yet added, e.g., congestion control and proper flow control.
Long story short, I eventually decided to replace the QUIC stack part, but keep everything else I had written pretty much the same, to keep the library and its APIs mostly as it was. After a bit of research on existing C stacks, I figured out that either ngtcp2 or picoquic would be the way to go. A few conversations later (at FOSDEM too!), I decided to go the picoquic way, which is what this PR is about.
This PR basically gets rid of all the code related to my native QUIC stack (including the crypto utilities) and replaces that with picoquic integrations. This effort had the nice side effect of simplifying the dependencies too: where before I had an awkward dependency on quictls (awkward simply because of the diffulty to reliably detect and use it as part of my configure scripts), now picoquic is built as a shared object and embedded in the library directly, which makes it way easier.
The end result is that demos are supposed to work pretty much exactly as before, and with pretty much (apart from a few changes) the same APIs too. There are a few things that are done differently, mostly because of how stuff is implemented and exposed in picoquic compared to how I implemented it in my own homemade stack.
A more detailed discussion is available here, but some things to take into account are:
Apart from that, even though from a first cursory run of the demos all seems to be working, this is just a first integration, and I haven't spent much time fine tuning everything. I did notice a bit of instability, for instance, especially when closing connections, which is something I'll have to improve before this is merged.
Once this lands, I plan to dig a bit deeper in things picoquic provides I didn't have access to before. I haven't checked if, as an application, there's anything I need to do in terms of pacing or congestion control, for instance. All this can wait until I have more solid grounds to build from, and opening this PR is a first step towards exactly that.