rc.d/kld: Set sysctls after loading modules.

[markj]

Summary

Currently we load sysctl.conf twice:

1. when /etc/rc.d/sysctl runs (fairly early during boot)
2. when /etc/rc.d/securelevel runs (fairly late)

/etc/rc.d/kld runs in between. When it loads modules, there is a window
where the system will run with default sysctl values. This causes problems if
startup scripts between rc.d/kld and rc.d/securelevel do anything that
might rely on non-default sysctl values. For example, if rc.d/kld is
used to load an alternate TCP stack, and /etc/sysctl.conf configures a
non-default TCP stack, then sshd will end up using the "wrong" TCP stack
for its listening socket.

In many cases the solution is to use /boot/loader.conf instead, but this
can be awkward in some environments with limited loader(8) support
(e.g., arm64 devices booted using LINUX_BOOT_ABI, or powerpc64 booted
with petitboot). loader.conf is less friendly to automation as well,
for example because sysrc does not really handle it properly.

Since it is easy to reload sysctl.conf when klds are loaded, this change
implements that.

[imp]

I don't think this is a good idea....

Setting sysctl values multiple times may be unwise.

Most modules should be loaded in the loader, and most config of modules should be done via tunables, not sysctls.

[markj]

In D25601#566409, @imp wrote:

I don't think this is a good idea....

Setting sysctl values multiple times may be unwise.

We already do that today, via /etc/rc.d/sysctl and /etc/rc.d/securelevel.

Most modules should be loaded in the loader, and most config of modules should be done via tunables, not sysctls.

Sure, but that's not always easy to do.

[imp]

For modules, I think, we should document tunables in loader.conf not in sysctl.conf, even though many tuneables are now exported there and often writable.

[imp]

In D25601#566410, @markj wrote:

In D25601#566409, @imp wrote:

I don't think this is a good idea....

Setting sysctl values multiple times may be unwise.

We already do that today, via /etc/rc.d/sysctl and /etc/rc.d/securelevel.

we shouldn't be doing it in securelevel... It wasn't in the original design.

Most modules should be loaded in the loader, and most config of modules should be done via tunables, not sysctls.

Sure, but that's not always easy to do.

True. However, we should document it as tunable init and make our docs clearer.

[tuexen]

In D25601#566409, @imp wrote:

Most modules should be loaded in the loader, and most config of modules should be done via tunables, not sysctls.

So you think the parameters of the TCP stacks should be tunables?

Do you think we should use for the SCTP parameters sysctl variables when compiled into the kernel and tunables when loaded as a module?

[markj]

In D25601#566423, @imp wrote:

In D25601#566410, @markj wrote:

In D25601#566409, @imp wrote:

I don't think this is a good idea....

Setting sysctl values multiple times may be unwise.

We already do that today, via /etc/rc.d/sysctl and /etc/rc.d/securelevel.

we shouldn't be doing it in securelevel... It wasn't in the original design.

It has been there since 2003. I'm sure that removing that will break some setup. Without that, nothing applies /etc/sysctl.conf after rc.d/kld runs, so sysctls that are missing during rc.d/sysctl will never get set.

Most modules should be loaded in the loader, and most config of modules should be done via tunables, not sysctls.

Sure, but that's not always easy to do.

True. However, we should document it as tunable init and make our docs clearer.

I don't quite follow - I'm just pointing out that some of our platforms can't use loader.conf.

[rgrimes]

In D25601#566423, @imp wrote:

In D25601#566410, @markj wrote:

In D25601#566409, @imp wrote:

I don't think this is a good idea....

Setting sysctl values multiple times may be unwise.o

I concur on this point. Not all sysctl are idempotent.

We already do that today, via /etc/rc.d/sysctl and /etc/rc.d/securelevel.

we shouldn't be doing it in securelevel... It wasn't in the original design.

Again, concur, and we should investigate why this was done and seek a better solution to what ever the problem was.

Also, imho, failure of a sysctl setting to apply should probably be a fatal boot time error and drop one to single user as it may lead to serous security breaches, aka a firewall that is not behaving per intention because a sysctl did not get applied.

[markj]

In D25601#566431, @rgrimes wrote:

I concur on this point. Not all sysctl are idempotent.

If a sysctl is not idempotent, then either it is a kernel bug, or setting it in sysctl.conf will cause problems today, or setting it multiple times doesn't matter.

We already do that today, via /etc/rc.d/sysctl and /etc/rc.d/securelevel.

we shouldn't be doing it in securelevel... It wasn't in the original design.

Again, concur, and we should investigate why this was done and seek a better solution to what ever the problem was.

The problem is conflicting requirements: 1) rc.d/sysctl should run as early as possible, 2) some rc scripts will do things that add new sysctls.

Also, imho, failure of a sysctl setting to apply should probably be a fatal boot time error and drop one to single user as it may lead to serous security breaches, aka a firewall that is not behaving per intention because a sysctl did not get applied.

That's unrelated to this change. If we start doing it by default it will break tons of systems. Some mechanism to opt-in to that behaviour on a sysctl-by-sysctl basis is needed.

As an alternative to this patch might be nice for sysctl(8) to be able to determine the KLD to which a given sysctl belongs, and have a way to tell it to only set sysctls belonging to a given KLD. That is a fair bit of work to do in the absence of a concrete scenario where this patch would cause a problem.

[allanjude]

For modules that have tunables, you need to set them via kenv (or have them set at boot via loader.conf) BEFORE you load the module.

Maybe the solution is /etc/sysctl.conf.d/kldname.conf
so we can set ONLY the sysctls from each kld as we load it.

so /etc/rc.d/kld could do:

for $kld in $kld_load; do
  kldload $kld
  if [ -f "/etc/sysctl.conf.d/${kld}.conf" ]; then
    sysctl -f /etc/sysctl.conf.d/${kld}.conf
  fi
done

while /etc/rc.d/sysctl could add /etc/sysctl.conf.d/*.conf to the list of files it checks