quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.57k stars 364 forks source link

Rotation of Connection IDs #1880

Closed thynson closed 1 month ago

thynson commented 1 month ago

Seeing some changes to ConnectionIdGenerator are going to happen in #1879 , I wish the ability to rotate Connection IDs could be taken into consideration.

The QUIC-LB draft proposed a CID Format that allows the load balancer to find the correct server to which a packet should be forwarded to. To avoid ossification of CID format configuration in a deployment, the first byte in the CID Format contains a few bits for Config Rotation.

To properly implement config rotation, the server must be able to

  1. maintains the expiration time of each Connection ID in a connection(it may still be supported that a Connection ID can never be expired); and

  2. request the peer to retire Connection IDs that has been expired, this can be done by sending a NEW_CONNECTION_ID frame that contains a Retire Prior To field.

I'm willing to contribute to it and look forward to any discussion.

djc commented 1 month ago

I think we'd be open to that, but it's unclear to me how this would affect the API. Do you want to submit a PR in that direction? Should we hold the release for that so that we can change the API some more?

thynson commented 1 month ago

I think we'd be open to that, but it's unclear to me how this would affect the API.

I think the first part can be implemented without API change. On the second part, I currently come up two way to implementing it:

The former should be cleaner and requires no breaking change, the latter seems reasonable that the implementation should know an expiration time of a cid and can even perform cid validation when parsing but could be over complicated.

Do you want to submit a PR in that direction? Should we hold the release for that so that we can change the API some more?

Yes, I definitely want to submit PRs for it but I don't have a schedule for it yet. And unless we decided to add expire_time to ConnectionIdGenerator so it'd better to land it together with #1879, I think we don't need hold the release for it.

djc commented 1 month ago

I suppose we could add ConnectionIdGenerator::expire_time() with a default implementation that yields None such that it wouldn't be breaking, so that sounds like we don't have to hold the release for it.

Ralith commented 1 month ago

Add fn expire_time() -> Option<Duration> to the trait ConnectionIdGenerator.

This already exists: ConnectionIdGenerator::cid_lifetime. Is anything else needed?

See also the queue machinery in CidState, Timer::PushNewCid, etc.

thynson commented 1 month ago

This already exists: ConnectionIdGenerator::cid_lifetime. Is anything else needed?

Ahh, no idea why I missed it, it should be enough for rotating Connection ID.