Page MenuHomeFreeBSD

Ratelimit and rack
ClosedPublic

Authored by rrs on May 7 2021, 4:54 PM.

Details

Summary

It is preferable to have the ratelimit option in rack, but we should
really allow rack to be built without it. The alternative to this
patch is to add to the ratelimit code and rack a module declare (like hpts)
so that rack will fail to build if you miss the option.

Test Plan

Make sure we can have rack (all the alternate stacks) and still build it
if RATELIMIT is not a configured option validate the kernel compiles.

Diff Detail

Repository
rG 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 requested review of this revision.May 7 2021, 4:54 PM

While #ifdefs are a bit ugly, I'm not sure there's a better, performant way to do this...

Are there platforms we can't build the ratelimit code, but want to use rack?

imp requested changes to this revision.May 7 2021, 6:06 PM

Actually, I changed my mind...

I think it would be better to fix this in tcp_ratelimit.h to have these functions return 0 for !RATELIMIT

This revision now requires changes to proceed.May 7 2021, 6:06 PM

Warner:

Ok thats another option too ;)

R

Lets take Warners alternative.. i.e. have the tcp_ratelimit code return 0 for the highest hw rate when
ratelimit is not in the kernel.

It needs to be static and match the signature duhh

Lets get the right function here

Turns out I was wrong, we need both functions not just one.

It is still not clear what problem we are solving here. On which platform can we not build a kernel with RATELIMIT but want to run RACK on?

Define a kernel that has all the TCP options to get the alternate
stacks but don't add RATELIMIT.

You will get the compile blowing up.

One of the PPC LINT images evidently fails to include RATELIMIT for some reason.

In D30163#677390, @rrs wrote:

Define a kernel that has all the TCP options to get the alternate
stacks but don't add RATELIMIT.

I would consider that a configuration error.

You will get the compile blowing up.

One of the PPC LINT images evidently fails to include RATELIMIT for some reason.

It fails because it contains

nooptions RATELIMIT

and I think this is a bug in the configuration file. Is there a reason why we can't build
and use RATELIMIT on 32-bit PPC? I use it on 64-bit PPC.

Does FreeBSD have a policy that you can always compile a kernel, even with
invalid kernel configs? If that is the case, I'm fine with this change.
If not, I think we should fix kernel configs to be valid, but not add more
complexity to he configurations we support.

Does FreeBSD have a policy that you can always compile a kernel, even with invalid kernel configs?

Generally, yes. The current failure mode is a bit opaque.

In D30163#677462, @imp wrote:

Does FreeBSD have a policy that you can always compile a kernel, even with invalid kernel configs?

Generally, yes. The current failure mode is a bit opaque.

Especially since this is a module build error... This is the least painful way to fix that, though not building the module for illegal kernel configurations is the other option had this hack not been so easy.

In D30163#677462, @imp wrote:

Does FreeBSD have a policy that you can always compile a kernel, even with invalid kernel configs?

Generally, yes. The current failure mode is a bit opaque.

Ahh, OK. I wasn't aware of that. Then I'm fine with this change.

This revision is now accepted and ready to land.May 7 2021, 9:15 PM
In D30163#677463, @imp wrote:
In D30163#677462, @imp wrote:

Does FreeBSD have a policy that you can always compile a kernel, even with invalid kernel configs?

Generally, yes. The current failure mode is a bit opaque.

Especially since this is a module build error... This is the least painful way to fix that, though not building the module for illegal kernel configurations is the other option had this hack not been so easy.

I just test building RACK without TCPHPTS. I builds just fine a module, which can't be loaded. So the user will detect the configuration error, just not when building the code, but when trying to use the code. So being able to build RACK without RATELIMIT is consistent.