Skip to content
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

Rotation of Connection IDs #1880

Closed
thynson opened this issue May 23, 2024 · 5 comments
Closed

Rotation of Connection IDs #1880

thynson opened this issue May 23, 2024 · 5 comments

Comments

@thynson
Copy link
Contributor

thynson commented May 23, 2024

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
Copy link
Collaborator

djc commented May 23, 2024

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
Copy link
Contributor Author

thynson commented May 23, 2024

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:

  • Add fn cid_expire_time(duration: Option<Duration>) to EndpointConfig.

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

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
Copy link
Collaborator

djc commented May 23, 2024

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
Copy link
Collaborator

Ralith commented May 23, 2024

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
Copy link
Contributor Author

thynson commented May 23, 2024

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.

@Ralith Ralith closed this as completed May 23, 2024
@djc djc reopened this May 23, 2024
@djc djc closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2024
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

No branches or pull requests

3 participants