Page MenuHomeFreeBSD

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

Authored by dfr on Jul 6 2023, 4:50 PM.
Tags
None
Referenced Files
F103324469: D40886.id124664.diff
Sat, Nov 23, 1:48 PM
Unknown Object (File)
Tue, Nov 19, 1:32 PM
Unknown Object (File)
Tue, Nov 19, 1:32 PM
Unknown Object (File)
Tue, Nov 19, 1:05 PM
Unknown Object (File)
Sat, Nov 2, 8:08 PM
Unknown Object (File)
Oct 22 2024, 1:51 AM
Unknown Object (File)
Oct 1 2024, 7:24 PM
Unknown Object (File)
Sep 9 2024, 1:15 PM

Details

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dfr requested review of this revision.Jul 6 2023, 4:50 PM

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.

This revision is now accepted and ready to land.Jul 6 2023, 5:15 PM

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?

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.

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?

👍

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.

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

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

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

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

This revision now requires review to proceed.Jul 8 2023, 10:21 AM
libexec/rc/rc.subr
1974

for consistency

Consistent quoting for /etc/sysconf.d/*

dfr marked an inline comment as done.Jul 8 2023, 10:54 AM

I like the quoting for consistency bit...

libexec/rc/rc.subr
1974

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

This revision is now accepted and ready to land.Jul 10 2023, 3:08 AM
This revision was automatically updated to reflect the committed changes.
dfr marked an inline comment as done.
agh_riseup.net added inline comments.
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?

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 :(

In D40886#934344, @dfr wrote:

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