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 BBR congestion control #514

Closed
VMatrix1900 opened this issue May 11, 2020 · 10 comments
Closed

Support BBR congestion control #514

VMatrix1900 opened this issue May 11, 2020 · 10 comments

Comments

@VMatrix1900
Copy link
Contributor

I use configurable congestion control to implement BBR congestion control. It is doable with some caveats.

  1. Need an accurate pacing implementation. I tried to implement a pacing module on the C part using libev/libuv timer but the accuracy of the timer is not good enough. I also tried to use set_socket_opt to leverage kernel pacing but it can not support multiple pacing rates on same socket. I read your blog. Is there any update on the pacing implementation?
  2. I implemented the BBR congestion control by porting the BBR implementation in lsquic into the Rust part. However, their data structure is different from QUICHE's. It involves a lot of structs translation and duplicate. I notice that some parts of the BBR code such as delivery rate estimation(Implementation of Delivery rate estimation #375) and windowed min-max filter over (recovery: windowed min and max filter implementation #398) is ported. Are you actively working on the BBR implementation?
@junhochoi
Copy link
Contributor

junhochoi commented May 11, 2020

@VMatrix1900 as mentioned in our blog post we plan to work on pacing/BBR. rate estimation and minmax filter are one of the requirements.

@VMatrix1900
Copy link
Contributor Author

@VMatrix1900 as mentioned in our blog post we plan to work on pacing/BBR. rate estimation and minmax filter are one of the requirements.

Any thoughts on the pacing module?

And the minmax filter is supposed to be used both for max bandwidth and min RTT. But current implementation can only works for min RTT. It should be changed into generic types for max bandwidth. I can create a pull request for that if you need.

@VMatrix1900
Copy link
Contributor Author

I already port the BBR state machine using the old configurable congestion control API before #421. I can rebase it based on the vtable API for a start point to port BBR if such a pull request is welcomed.

@junhochoi
Copy link
Contributor

@VMatrix1900 we are working on those new features (pacing, BBR, etc) but we don't have a date yet. External contribution is generally welcomed. Also,

  • pacing: It will be implemented in quiche library but may depend on the OS support described in the blog post.
  • minmax: Current minmax filter (src/minmax.rs) supports min and max both. @lohith-bellad can clarify if needed.

@VMatrix1900
Copy link
Contributor Author

VMatrix1900 commented May 13, 2020

  • pacing: It will be implemented in quiche library but may depend on the OS support described in the blog post.

In my point of view, the QUICHE library only provides a QUIC state machine. The IO is done by other parts. The pacing is related to the I/O. The best QUICHE library can do is providing a pacing rate for an external pacing module to execute.

  • minmax: Current minmax filter (src/minmax.rs) supports min and max both. @lohith-bellad can clarify if needed.

It is a data type issue. The current minmax filter using Duration as the value type so it can be used to track minimal RTT but not max bandwidth.

@lohith-bellad
Copy link
Contributor

minmax filter implementation is more generic now.

@xanathar
Copy link
Contributor

xanathar commented Sep 3, 2020

@VMatrix1900 is there a public branch where I can take a peek at your BBR implementation?

@ibis85
Copy link

ibis85 commented Sep 10, 2020

I'd also be interested in any WIP BBR branch.

@rafibaum
Copy link

rafibaum commented Feb 5, 2021

Hi, I was wondering if there's been any progress on this issue internally? I'm considering using QUICHE as part of a research project which requires using congestion control algorithms with pacing (BBR/Copa). If there isn't work progressing on this, I was planning on authoring potentially authoring a PR myself but I didn't want to reinvent the wheel.

Edit: I see #770 is under active review. I'm guessing the BBR implementation is essentially imminent.

@junhochoi
Copy link
Contributor

Now we have bbr (v1) via #1180.

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 a pull request may close this issue.

6 participants