Skip to content

Conversation

@TheJokr
Copy link
Contributor

@TheJokr TheJokr commented Dec 17, 2025

This PR consists of 4 separate commits. The first two are refactors to simplify bits of the internal and public API of tokio-quiche. On the public side, I removed the socket_cookie parameter completely and made the CID generator a QuicListener property. This means our users are expected to provide a generator for each socket now, with TryFrom defaulting to SimpleConnectionIdGenerator. Users that relied on socket_cookie have more flexibility now, as they can embed the cookie (or any other data they may require) directly in the CID generator instance. This is a breaking change.

The third commit changes the IoWorker loop to top up quiche's supply of SCIDs in every iteration, as well as removing retired SCIDs from the ConnectionMap. SCIDs are generated by the generator from the QuicListener (which was the reason for refactoring that API in the first place.) Each active connection receives its own Arc to the shared generator. This unblocks active connection migration for clients, as we already support passive migration where only the IP/port changes.

Finally, the fourth commit adds a test to verify that active migration works correctly. Since active and passive migration scenarios are extremely similar, and both require low-level quiche access, I decided to add the active-migration-specific bits to the existing passive migration test case rather than copy-pasting the entire thing.

@TheJokr TheJokr requested a review from a team as a code owner December 17, 2025 15:30
@TheJokr TheJokr force-pushed the lblocher/tq-extra-cids branch from 1cef63e to 34e4052 Compare December 17, 2025 15:59
@TheJokr
Copy link
Contributor Author

TheJokr commented Dec 17, 2025

CI failures are due to missing apt packages, I'm pretty sure it's a GitHub issue. Will re-trigger CI tomorrow.

Took some time to investigate, it's because we don't call apt-get update: #2292

@TheJokr TheJokr force-pushed the lblocher/tq-extra-cids branch from 34e4052 to a57fa83 Compare December 18, 2025 11:04
This ID is redundant as we can identify a connection by its (possibly
multiple) QUIC connection IDs. There is no place where we actually
use the internal ID.
We can support socket-specific CID generators by moving the trait object
from the top-level entrypoints to `QuicListener`, which exists
separately for each socket. This allows the implementor full flexibility
in terms of what properties to consider when generating CIDs for a given
socket.

To avoid a type parameter in `QuicListener`, we require the user to pass
us an `Arc<dyn ConnectionIdGenerator>` (which is the internal
representation we want to use anyway.) This also allows users to share
a generator across all of their sockets.
We aim to always provide as many CIDs to the peer as the local+remote
transport parameters allow. At the same time, retired connection IDs are
unmapped to keep the ConnectionMap compact.

To produce new connection IDs, we wire up the socket's
`Arc<dyn ConnectionIdGenerator>` all the way to the IoWorker parameters.
@TheJokr TheJokr force-pushed the lblocher/tq-extra-cids branch from a57fa83 to 8f4cfe5 Compare December 18, 2025 15:17
Now that the tokio-quiche work loop issues extra connection IDs, we can
properly support active connection migration. This is not a feature
available to tokio-quiche clients today, but other QUIC clients talking
to tokio-quiche servers can take advantage of it.
@TheJokr TheJokr force-pushed the lblocher/tq-extra-cids branch from 8f4cfe5 to 3841db7 Compare December 18, 2025 15:21
client_config.set_initial_max_streams_bidi(10);
client_config.set_initial_max_streams_uni(3);
client_config.set_active_connection_id_limit(2);
// We don't need to allow the server to initiate connection migration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: QUICv1 does not permit servers to initiate migration any way, so perhaps a comment like below is more accurate

    // Server's can't initiate migration but disable it explicitly anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants