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

support of resetting the congestion controller? #2016

Closed
ibigbug opened this issue Oct 19, 2024 · 9 comments
Closed

support of resetting the congestion controller? #2016

ibigbug opened this issue Oct 19, 2024 · 9 comments

Comments

@ibigbug
Copy link

ibigbug commented Oct 19, 2024

is there an API like this for example https://github.com/google/quiche/blob/1da1f6810fe80d4e2126ce98a9e2101d7dae92aa/quiche/quic/core/quic_sent_packet_manager.h#L457

to reset the congestion controller on the fly?

  1. What I wanted to do is, I establish a connection to a remote
  2. negotiate a congestion algo with remote
  3. client side reset the congestion algo after connection established

Now I find the Endpoint has a https://docs.rs/quinn/0.11.5/quinn/struct.Endpoint.html#method.set_default_client_config which can be set before the connection established.

And the TransportConfig has a https://docs.rs/quinn/0.11.5/quinn/struct.TransportConfig.html#method.congestion_controller_factory - could be useful by building a new Factory with a Mutex with a congestion algo that is later mutated, but

  1. does this work, e.g. does changing the factory affect any new outgoing client connections?
  2. the factory signature fn build is non async and it's quite hard to work with async runtime and mutexts

or is there another way to do it?

tyty

@Ralith
Copy link
Collaborator

Ralith commented Oct 19, 2024

A method to return the congestion controller to the initial state doesn't exist, but we're interested in having it for cases where the application knows that network conditions have substantively changed, probably as an extra flag on, or alternative version of, rebind. See PathData::from_previous vs. PathData::new, and their callsites, in quinn-proto.

However, it sounds like you're asking for a method to replace the congestion controller dynamically. Can you expand on why this seems useful? Congestion controllers do not require negotiation with the remote, so there's no reason not to set the one you want up front; congestion control operates purely under the sender's discretion.

@ibigbug
Copy link
Author

ibigbug commented Oct 20, 2024

Hey thanks for the reply.

Reason of that is for example, client side wanted to use certain congestion control BBR say, and the remote side decided it doesn't want the client to send packets that aggressively and indicate the client to use another control.

One of the examples is something called Hysteria2 proxy protocol

@Ralith
Copy link
Collaborator

Ralith commented Oct 20, 2024

Why would you consider BBR aggressive? Why would the receiver bother expressing a preference? There is no way to prevent a sender from using whatever congestion control scheme they like.

@ibigbug
Copy link
Author

ibigbug commented Oct 21, 2024

I think I spoke too fast. I was mean to say Brutal. https://github.com/apernet/tcp-brutal

I just read around it and this proto relies on the remote side bandwidth limit to be setup working properly. Hence the negotiate.

Although I don't even know if it's a good pattern to do that.

@flub
Copy link
Contributor

flub commented Oct 21, 2024 via email

@Ralith
Copy link
Collaborator

Ralith commented Oct 22, 2024

We did PR this a while ago and the discussion lead to not including it. for our usecase we want to implement multipath and then won't need this anymore.

I don't recall exactly the previous discussion. I think such a mechanism has clear uses, even in the presence of multipath, when the application has sidechannel knowledge that network conditions have changed considerably, e.g. due to switching networks. This might be best expressed as part of rebind, since switching networks usually means switching addresses, although there are possible exceptions worth examining (what about sockets bound to the wildcard address, or cases where the address doesn't change but you believe network conditions have changed for other reasons?).

I think this is off-topic for the intent of this issue, though, so if you'd like to discuss further, best resurrect the old/open a new issue/PR.

@ibigbug

I just read around it and this proto relies on the remote side bandwidth limit to be setup working properly. Hence the negotiate.

Based on my brief scan, it sounds like the sender is responsible for knowing the capacity of the entire path in advance. The remote doesn't have much say in, or special knowledge of, that.

Although I don't even know if it's a good pattern to do that.

Sending at a fixed rate intending to saturate a path regardless of the effects on other network users is definitely not a good pattern. I'm willing to consider arguments that it might make sense in some cases, but any design like this runs the risk of making the internet worse for everyone. Imagine if two users on the same network tried to use this method to communicate with the same server!

@ibigbug
Copy link
Author

ibigbug commented Oct 22, 2024

yeah i agree with that.

in general do you think it's useful to change the congestion control in the case the sender decided to do so for a whatever reason?

@Ralith
Copy link
Collaborator

Ralith commented Oct 22, 2024

I'm not aware of a use case for replacing the congestion control algorithm in-situ. If you don't have a compelling use case either, maybe close the issue? If someone has a novel use case and/or wants to experiment, reconnecting isn't the end of the world, especially for prototyping purposes.

@ibigbug
Copy link
Author

ibigbug commented Oct 23, 2024

Yeah sg. Thanks!

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

4 participants