Page MenuHomeFreeBSD

Third patchset in the set of patches to bring BBRv1 into the FreeBSD tree
ClosedPublic

Authored by rrs on Jul 15 2019, 12:08 PM.

Details

Summary

This is the third patchset to work on getting BBR into the tree (and the latest rack we use as well). This brings
in the tcp_ratelimit.c file so that TCP transports (and possibly others) have a mechanism that can adequately
use the ratelimit code from the transport. The original ratelimit code was designed so the user at the
socket option level could "use the ratelimit" code. But this is really not practical and should be the job
of TCP (i.e. the transport).

Test Plan

This set has been tested at NF with both BBR and Rack. There are limitations
of cards that support it but we have only tested two of the cards that are
known to have ratelimit support. Other updates will be needed to expand
that to include future cards (some already out there).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rrs created this revision.Jul 15 2019, 12:08 PM
rrs added a reviewer: hselasky.
rrs updated this revision to Diff 59988.Jul 21 2019, 4:13 PM

Update to address Michael's comments about copyright (missing new magic declarations at the top) and
also address an issue where if we have an interface that marks it to "not supporting" we properly
disable the rate limiting for that interface and do not allow the user to enable it (since it does
not support it).

rrs added a comment.Jul 29 2019, 6:07 PM

I plan on commiting this August 1st unless I hear screams.. its been pending since July 15th....

hselasky accepted this revision.Jul 31 2019, 9:19 AM

Changes look good to me.

conf/files
4258 ↗(On Diff #59988)

Can you double check this syntax? I thought "config" was very simple and did not understand multiple items after "|" .

This revision is now accepted and ready to land.Jul 31 2019, 9:19 AM
rrs added inline comments.Aug 1 2019, 12:15 PM
conf/files
4258 ↗(On Diff #59988)

I think it used to be that simple, but the fact that we have tcp_pcap (line below) setup the same way makes
me think it was fixed.

I can try to configure without inet and inet6 and see what happens :)

I wonder though if it should not imitate the tcp_pcap line below that has inet and inet6 first.

rrs added inline comments.Aug 1 2019, 12:24 PM
conf/files
4258 ↗(On Diff #59988)

Ok a quick check shows that it blows up nicely on compile when I
take away INET and INET6.

But this will be true for any of the ones with multiple entries. I suspect
that this is the hint, the compile blows up, you go look at the RATELIMIT line
and see you dropped both INET and INET6 and go aahh...

Until someone fixes config to be a bit smarter of course.

Note there are 100's of lines that have multiple inet foo | inet6 foo
so it is not alone ... and I guess just gives us the "human" hint in the file

This revision was automatically updated to reflect the committed changes.
Herald added subscribers: ae, imp. · View Herald Transcript