tcp: build rack and bbr as part of normal kernel builds

[gallatin]

Summary

• Build RACK and BBR normally, as part of every normal kernel build (for kernels that include INET or INET6), without requiring MK_EXTRA_TCP_STACKS=yes

Rack and BBR have been hidden behind the MK_EXTRA_TCP_STACKS=yes option. This causes them to be left out of CI build testing, increasing the likelihood of accidental breakage. By building normally, they will be built as part of universe builds, and any breakage will be noticed early.

• Fix existing breakage when various combinations of INET/INET6 are present in the kernel

• Emit a warning when BBR is loaded, indicating that it is deprecated. This is needed because BBR implements an older version of the BBR standard.

[gbe]

@gallatin thanks for the differential. I have created https://reviews.freebsd.org/D36538 a while ago. Could you integrate the changes into your differential?

[jtl]

The rationale for the present state is found here.

What you are proposing seems like a very fundamental change. I believe this change would implicitly promote these TCP stacks from being experimental features which a user must explicitly build to being fully-supported features which are available by default. Are we ready to make that change? Do we believe that both of these stacks are sufficiently tested to take this step?

I believe two implications of the changes you are making are that:

• All developers would be impacted by build breakage specific to these extra TCP stacks. While I think this is exactly the result you are seeking, I believe we found this to be problematic in the early days. Perhaps, we are past those days by now and this is no longer a concern.
• Any bugs (including security vulnerabilities) would likely need to be treated with a higher severity, perhaps even with the same severity as bugs in the default TCP stack (possibly requiring errata notices or security advisories)

I feel like this is an area which merits alignment across the TCP developers (and, perhaps, more broadly -- for example, possibly including release engineering and the security officer). If we want to take this step of making the stacks available by default, we should mark this with Relnotes: yes, advertise the change in the quarterly report, and probably advertise it more broadly. If we are not ready to make these available by default, maybe we should think further about ways to fix the problem you have identified.

Of course, maybe I am over-thinking this, and building these by default is not a big deal. So, others can feel free to correct me.

This causes them to be left out of CI build testing

The stacks should be build as part of a LINT kernel or tinderbox build. Is the CI test not doing that?

[jtl]

By the way, I should clarify that I am not opposed to this change; rather, my previous comment was merely intended to help explain what I view as the implications of the change and advocate that we make sure there is sufficiently broad agreement to proceed.

[gallatin]

In D37715#857654, @jtl wrote:

This causes them to be left out of CI build testing

The stacks should be build as part of a LINT kernel or tinderbox build. Is the CI test not doing that?

I think they are not being built, as they didn't compile with the various LINT_NO{IP,INET,INET6} and I didn't see any warnings about it. I'm not sure how they could be, as they are built when MK_EXTRA_TCP_STACKS=yes exists in /etc/src.conf. Is there a way to specify this inside a kernel config file, rather than in src.conf?

[gallatin]

In D37715#857714, @gallatin wrote:

In D37715#857654, @jtl wrote:

This causes them to be left out of CI build testing

The stacks should be build as part of a LINT kernel or tinderbox build. Is the CI test not doing that?

I think they are not being built, as they didn't compile with the various LINT_NO{IP,INET,INET6} and I didn't see any warnings about it. I'm not sure how they could be, as they are built when MK_EXTRA_TCP_STACKS=yes exists in /etc/src.conf. Is there a way to specify this inside a kernel config file, rather than in src.conf?

Ah, I'm an idiot -- this is how we build the modules at netflix

The problem is that nobody ever added "makeoptions WITH_EXTRA_TCP_STACKS=1" to NOTES
I'd be happy to drop this in favor of just adding that to NOTES (and fixing the same compile issues as this review does). My goal is just to make sure this stuff compiles..

[gallatin]

Abandoning this in favor of D37903