Page MenuHomeFreeBSD
Differential D36537

tcp: make cubic the default cc.
ClosedPublic
Actions

Authored by rscheff on Sep 12 2022, 1:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 20, 2:29 AM
Unknown Object (File)
Thu, Jun 18, 1:58 PM
Unknown Object (File)
Wed, Jun 10, 3:19 PM
Unknown Object (File)
Mon, Jun 1, 7:10 PM
Unknown Object (File)
Fri, May 29, 1:59 PM
Unknown Object (File)
Thu, May 28, 1:09 PM
Unknown Object (File)
Thu, May 28, 11:41 AM
Unknown Object (File)
Thu, May 28, 10:42 AM
View All Files
Subscribers
andrew
emaste
imp
jhibbits
melifaro

Details

Reviewers
rrs
thj
jtl
tuexen
glebius
mav
manu
cc
emaste
Group Reviewers
transport
Commits
rGbb1d472d79f7: tcp: make CUBIC the default congestion control mechanism.
Summary

As discussed in the last transport call, changing the default
Congestion Control (CC) to Cubic. For small, transactional
exchanges (e.g. web objects <15kB), this will not have a material
effect. However, for long duration data transfers, Cubic allocates
a slightly higher fraction of the available bandwidth, when
competing against NewReno CC.

All major stacks operating in the internet have moved towards
the use of Cubic, this follows their lead.

Test Plan

verify that cubic is compiled as kernel module, and the default:

# sysctl net.inet.tcp.cc.available
net.inet.tcp.cc.available:
CCmod           D PCB count
cubic           * 12
# sysctl net.inet.tcp.cc.algorithm
net.inet.tcp.cc.algorithm: cubic

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 47351
Build 44238: arc lint + arc unit

Event Timeline

rscheff created this revision.Sep 12 2022, 1:47 PM
Herald added a reviewer: manu. · View Herald TranscriptSep 12 2022, 1:47 PM
Herald added a reviewer: transport. · View Herald Transcript
Herald added subscribers: melifaro, jhibbits, emaste and 2 others. · View Herald Transcript
rscheff requested review of this revision.Sep 12 2022, 1:47 PM
Harbormaster completed remote builds in B47338: Diff 110464.Sep 12 2022, 1:47 PM
tuexen added a comment.Sep 12 2022, 2:05 PM

Why were there some configs setting CC_DEFAULT and some, which did not?

rscheff added a comment.Sep 12 2022, 2:11 PM
In D36537#829666, @tuexen wrote:

Why were there some configs setting CC_DEFAULT and some, which did not?

I presume that https://reviews.freebsd.org/D32964 did not touch all these more architecture specific ones....

CC_DEFAULT would nowadays be set by cc.c since then, thus wouldn't have been required in the old style...

emaste added inline comments.Sep 12 2022, 2:47 PM
share/man/man4/mod_cc.4
50

Is it properly "Cubic" or "CUBIC"?
[Or "cubic" as below?]

rscheff edited the test plan for this revision. (Show Details)Sep 12 2022, 2:58 PM
rscheff added a reviewer: cc.
rscheff edited the test plan for this revision. (Show Details)
rscheff added inline comments.Sep 12 2022, 3:03 PM
share/man/man4/mod_cc.4
50

The RFC refers to the algorithm as CUBIC, to avoid confusion with the cubic function used for scaling up bandwidth; The naming convention is already non-uniform in the fbsd man/doc. I didn't use all-uppercase throughout here, as uppercase string tend to signify a compile option or other tunable.

Whatever style is preferred, I can redo the cubic / CUBIC related man pages accordingly. The mixed case here was in reflection of "NewReno", quite frankly.

tuexen added inline comments.Sep 12 2022, 3:14 PM
share/man/man4/mod_cc.4
50

I would suggest to use CUBIC as the upcoming RFC 8312bis does.

cc added inline comments.Sep 12 2022, 3:31 PM
share/man/man4/mod_cc.4
50

+1

50

I would suggest to use CUBIC as the upcoming RFC 8312bis does.

+1

rscheff updated this revision to Diff 110466.Sep 12 2022, 4:44 PM
rscheff edited the test plan for this revision. (Show Details)
  • Use "CUBIC" when talking about the algorithm
Harbormaster completed remote builds in B47340: Diff 110466.Sep 12 2022, 4:44 PM
rscheff marked 2 inline comments as done.Sep 12 2022, 4:45 PM
tuexen accepted this revision.Sep 12 2022, 6:51 PM
This revision is now accepted and ready to land.Sep 12 2022, 6:51 PM
cc added inline comments.Sep 12 2022, 6:55 PM
sys/conf/NOTES
653–654

I think "Generic" here means "GENERIC". If better, replace with "GENERIC".

cc accepted this revision.Sep 12 2022, 7:05 PM

In general, this looks good to me. Thanks

rscheff updated this revision to Diff 110482.Sep 12 2022, 9:14 PM
  • refer to GENERIC profile in uppercase
This revision now requires review to proceed.Sep 12 2022, 9:14 PM
Harbormaster completed remote builds in B47351: Diff 110482.Sep 12 2022, 9:14 PM
tuexen accepted this revision.Sep 12 2022, 9:22 PM
This revision is now accepted and ready to land.Sep 12 2022, 9:22 PM
emaste accepted this revision.Sep 12 2022, 9:25 PM

LGTM

I would suggest that the capitalization changes unrelated to making it the default e.g. sys/netinet/cc/cc_cubic.c be done as a separate commit first

emaste added a comment.Sep 12 2022, 9:46 PM

Also include Relnotes: Yes in the commit message

mav accepted this revision.EditedSep 13 2022, 10:54 AM

Looks good to me, though I am not really qualified to choose. I just saw CUBIC recovering much faster after packet losses in datacenter environments without flow control and WAN/WiFi. With NewReno we saw regular pauses in a traffic up to a second IIRC.

rscheff updated this revision to Diff 110505.Sep 13 2022, 1:30 PM
  • remove comment updates, whitespace cleanup.
This revision now requires review to proceed.Sep 13 2022, 1:30 PM
Harbormaster completed remote builds in B47361: Diff 110505.Sep 13 2022, 1:30 PM
emaste accepted this revision.Sep 13 2022, 1:34 PM
This revision is now accepted and ready to land.Sep 13 2022, 1:34 PM
tuexen accepted this revision.Sep 13 2022, 1:37 PM
Closed by commit rGbb1d472d79f7: tcp: make CUBIC the default congestion control mechanism. (authored by rscheff). · Explain WhySep 13 2022, 2:06 PM
This revision was automatically updated to reflect the committed changes.
rscheff added a commit: rGbb1d472d79f7: tcp: make CUBIC the default congestion control mechanism..

Revision Contents
Changeset List

PathSize
share/
man/
man4/
3 lines
3 lines
14 lines
sys/
amd64/
conf/
2 lines
arm/
conf/
2 lines
3 lines
3 lines
arm64/
conf/
2 lines
3 lines
conf/
14 lines
i386/
conf/
2 lines
netinet/
cc/
2 lines
2 lines
2 lines
2 lines
powerpc/
conf/
2 lines
riscv/
conf/
2 lines

Diff 110482

View Options

share/man/man4/cc_cubic.4

Loading...
View Options

share/man/man4/cc_newreno.4

Loading...
View Options

share/man/man4/mod_cc.4

Loading...
View Options

sys/amd64/conf/DEFAULTS

Loading...
View Options

sys/arm/conf/DEFAULTS

Loading...
View Options

sys/arm/conf/std.armv6

Loading...
View Options

sys/arm/conf/std.armv7

Loading...
View Options

sys/arm64/conf/DEFAULTS

Loading...
View Options

sys/arm64/conf/std.arm64

Loading...
View Options

sys/conf/NOTES

Loading...
View Options

sys/i386/conf/DEFAULTS

Loading...
View Options

sys/netinet/cc/cc.h

Loading...
View Options

sys/netinet/cc/cc.c

Loading...
View Options

sys/netinet/cc/cc_cubic.h

Loading...
View Options

sys/netinet/cc/cc_cubic.c

Loading...
View Options

sys/powerpc/conf/DEFAULTS

Loading...
View Options

sys/riscv/conf/DEFAULTS

Loading...