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

Notifying Quinn about network path changes #2018

Open
flub opened this issue Oct 24, 2024 · 2 comments
Open

Notifying Quinn about network path changes #2018

flub opened this issue Oct 24, 2024 · 2 comments

Comments

@flub
Copy link
Contributor

flub commented Oct 24, 2024

Sometimes the application knows about network path changes that Quinn may not detect immediately to. If the network changed significantly it can often be faster to reach optimal throughput and latency again by resetting things like the congestion controller, rtt estimator and mtu detection to initial states.

This was explored a bit in #1842 but that was ultimately closed. However this topic was recently brought up again in another issue and there was an indication that something like this could belong in Quinn. So this is an attempt to find out if we can get any closer to that.

It seems the biggest difference in thinking is whether this should be per Connection or for the entire Endpoint.

  • The motivation for per-connection is that these details are stored per-connection since each connection has its own network path.
  • While for the entire endpoint the argument is that an endpoint only has a single socket and this is probably the most common way for an application to know the network path changed.

Perhaps it is easier to agree on what the API and implementation in quinn-proto should be, because there an API on the Connection seems better suited. Currently we have https://github.com/n0-computer/quinn/blob/02f3b33de039951bfaf4cc383d9f0ec6cdf8dcb1/quinn-proto/src/connection/mod.rs#L1317 as the entrypoint to this in the fork for iroh. The implementation currently does 3 things:

  • Creates a new RttEstimator from the original TransportConfig settings.
  • Creates a new congestion controller from the original TransportConfig settings.
  • Resets the MTU discovery, essentially re-creating a new one with the same settings it was originally created with.

What is the opinion on adding this in some form to quinn-proto?

@djc
Copy link
Member

djc commented Oct 25, 2024

It seems reasonable to me, let's see what @Ralith thinks.

@Ralith
Copy link
Collaborator

Ralith commented Oct 25, 2024

Thanks for the summary, and for digging up the old discussion! The proposal sounds good to me. Even if we only wanted to expose this on the endpoint, quinn would still need to iterate over all connections and reset those states, which is exactly what the proposed interface allows.

whether this should be per Connection or for the entire Endpoint.

I don't think these are mutually exclusive. An endpoint bound to a wildcard address can have connections with different source addresses, and an endpoint bound to a specific address can be rebound. It's also not an obviously complex interface or implementation, so I'm happy to have both, or whichever is of most interest to you.

Connection-granularity active migration may require a little extra testing and debugging since we don't currently use explicit source addresses for outgoing connections, but doing so should fit gracefully into the existing architecture. It's not necessary to implement that if all you actually want is the state reset, though.

flub added a commit to flub/quinn that referenced this issue Oct 31, 2024
This gives the proto state the ability to be externally notified about
network path changes it might not be aware off.  In this case it will
re-set the RTT, Congestion Controller and MTU settings to the initial
states based on the TransportConfig.

This addresses the quinn-proto part of quinn-rs#2018.
flub added a commit to flub/quinn that referenced this issue Nov 5, 2024
This gives the proto state the ability to be externally notified about
network path changes it might not be aware off.  In this case it will
re-set the RTT, Congestion Controller and MTU settings to the initial
states based on the TransportConfig.

This addresses the quinn-proto part of quinn-rs#2018.
github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
This gives the proto state the ability to be externally notified about
network path changes it might not be aware off.  In this case it will
re-set the RTT, Congestion Controller and MTU settings to the initial
states based on the TransportConfig.

This addresses the quinn-proto part of #2018.
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