/etc/rc.d/kld: add support for modifying sysctl variables

[dfr]

Summary

For kernel modules loaded by /etc/rc.d/kld, if there is a file in
/etc/sysctl.conf.d named <kld name>.conf, then this will be loaded using
the sysctl(8) utility. For instance, sysctl variable changes for the pf
kernel module would be placed in the file /etc/sysctl.conf.d/pf.conf.

PR: 272129

[imp]

I like this a lot.
The module specific stuff will normally be loaded early in boot, but if we have to load something, just the module specific stuff is loaded after kld time.
My only question here is 'do we need to wait at all' for the kld_load module to finish initializing? I don't think we have any modules that put off work to after the KLD_LOAD routine finishes, but I've seen enough weird stuff to at least ask the question.

[markj]

This is handy and I don't object, but it's not very ergonomic IMHO:

• Users have to know which kernel module provides a given sysctl, and there's no interface to find that. One has to look at the source, I believe.
• Loading a kernel module via kld_list can cause other dependent kernel modules to be loaded implicitly. If you want to set a sysctl in one of those modules, you have to be aware of the dependency, then list it explicitly in kld_list.

Beyond that we have devmatch, which automatically loads drivers when matching devices are present. We might want to have the same mechanism available for devmatch?

[imp]

In D40886#930749, @markj wrote:

Beyond that we have devmatch, which automatically loads drivers when matching devices are present. We might want to have the same mechanism available for devmatch?

Ugg. I think I just threw up a little...

However, after I got over that initial reaction, I'm not at all sure I object... Though devmatch, proper, is not the right place for it. devd calls devmatch to get the modules to load and that script might not be bad.
It would be ideal, though, if kldload could return a list of all modules loaded to stdout (maybe with a flag) so we don't have to guess this before/after.

The 'what module' question is made super-ugly by our two-tier parallel kernel config regime. Hacks are easy, but correct behavior is much harder.

[freebsd_igalic.co]

In D40886#930749, @markj wrote:

Beyond that we have devmatch, which automatically loads drivers when matching devices are present. We might want to have the same mechanism available for devmatch?

👍

[dfr]

In an ideal world, we could modify kldload (and probably the syscall interface) to let it optionally emit the set of modules and/or sysctls that were added but that would be a lot of effort for a small gain so I will move ahead with this approach unless someone has an objection.

[imp]

Would it be better to put that in the load_kld function from rc.subr?

[imp]

Also: don't let the perfect be the enemy of the better...

[dfr]

In D40886#931165, @imp wrote:

Would it be better to put that in the load_kld function from rc.subr?

That's a good idea - I'll try that and make sure it doesn't break anything

[dfr]

Move the logic from /etc/rc.d/kld to /etc/rc.subr and re-word manpage changes

[freebsd_igalic.co]

libexec/rc/rc.subr

1974 ↗

(On Diff #124351)

for consistency

[dfr]

Consistent quoting for /etc/sysconf.d/*

[imp]

I like the quoting for consistency bit...

libexec/rc/rc.subr

1974 ↗

(On Diff #124352)

I like this suggestion...
And I like this.. I'll have to see if devmatch uses load_kld()

[agh_riseup.net]

share/man/man5/sysctl.conf.5

60

Is /etc/sysctl.d correct here? I am probably wonrg, but I thought /etc/sysctl.conf.d/ was the location?

[agh_riseup.net]

Does this also invalidate or obsolete https://reviews.freebsd.org/D32128?

[dfr]

In D40886#934337, @agh_riseup.net wrote:

Does this also invalidate or obsolete https://reviews.freebsd.org/D32128?

This change is addressing a different issue - I wanted a way to apply sysctl changes to late-loaded kernel modules whereas D32128 allows for splitting sysctl.conf - all the fragments are applied at the same time. @kevans suggested changing the directory to sysctl.kld.d to better reflect the intent and that would remove the conflict with D32128 as well. I will create a new diff for that renaming which will also fix the typo.

share/man/man5/sysctl.conf.5

60

This is a typo :(

[agh_riseup.net]

In D40886#934344, @dfr wrote:

Wow, amazing. The sysctl.kld.d name is a great idea too, clears things right up :-) thanks.