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

feat(cca): cubic #1122

Merged
merged 8 commits into from
Jun 22, 2021
Merged

feat(cca): cubic #1122

merged 8 commits into from
Jun 22, 2021

Conversation

FrankSpitulski
Copy link
Contributor

mostly copied from quiche, modified to fit here

still working out the best way to get a ref to the current RTT inside the CCA

Relates to #693

@Ralith
Copy link
Collaborator

Ralith commented May 14, 2021

Thanks for taking a swing at this! I think it'd be fine to pass the current RTT to be made an argument to whatever congestion::Controller methods need it. The current interface for Controller isn't expected to be comprehensive, just a starting point to grow as needed.

e: Then that argument would be populated with self.path.rtt.get() from callsites inside Connection.

@FrankSpitulski
Copy link
Contributor Author

@Ralith thanks for the suggestion, I've pushed that impl up.

bench/src/bulk.rs Outdated Show resolved Hide resolved
@Matthias247
Copy link
Contributor

Some more discussions around Cubic and QUIC (also around min_rtt):

NTAP/rfc8312bis#45

quicwg/base-drafts#4833

@FrankSpitulski FrankSpitulski force-pushed the feat/cubic branch 2 times, most recently from f8a3d3d to 2250df0 Compare May 24, 2021 18:35
@FrankSpitulski FrankSpitulski marked this pull request as ready for review June 1, 2021 18:49
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RFC8312bis draft seems to be continuing to diverge from what we have here, but it's a moving target, so unless you're feeling super motivated to track it closely then I think we can call the underlying math here good.

quinn-proto/src/congestion/cubic.rs Outdated Show resolved Hide resolved
quinn-proto/src/congestion/cubic.rs Outdated Show resolved Hide resolved
Co-authored-by: Benjamin Saunders <ben.e.saunders@gmail.com>
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@djc djc merged commit 3f908a2 into quinn-rs:main Jun 22, 2021
@djc
Copy link
Member

djc commented Jun 22, 2021

Sorry, must have lost track of this one!

@FrankSpitulski FrankSpitulski deleted the feat/cubic branch June 24, 2021 21:48
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 this pull request may close these issues.

4 participants