Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kernel-modules): Add sysctl to initramfs to handle modprobe files #2037

Merged
merged 1 commit into from Nov 11, 2022

Conversation

Conan-Kudo
Copy link
Member

This pull request changes...

Changes

dracut-systemd module now will incorporate sysctl into initramfs so that modprobe configuration files that call sysctl will function properly.

Checklist

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

@github-actions github-actions bot added dracut-systemd Issues related to the dracut-systemd module modules Issue tracker for all modules labels Nov 5, 2022
@johannbg
Copy link
Collaborator

johannbg commented Nov 5, 2022

We already have a systemd sysctl module so adding this to dracut-systemd module should not be necessary. What usecase does this solve which is not solved by including that module?

@Conan-Kudo
Copy link
Member Author

The systemd-sysctl module doesn't use the sysctl binary, whereas this one will because various modprobe.d config files can use it. For example:

./etc/modprobe.d/firewalld-sysctls.conf:install nf_conntrack /sbin/modprobe --ignore-install nf_conntrack $CMDLINE_OPTS && /sbin/sysctl --quiet --pattern 'net[.]netfilter[.]nf_conntrack.*' --system
./usr/lib/modprobe.d/50-nfs.conf:install sunrpc /sbin/modprobe --ignore-install sunrpc $CMDLINE_OPTS && { /sbin/sysctl -q --pattern sunrpc --system; exit 0; }
./usr/lib/modprobe.d/50-nfs.conf:install rpcrdma /sbin/modprobe --ignore-install rpcrdma $CMDLINE_OPTS && { /sbin/sysctl -q --pattern sunrpc.svc_rdma --system; exit 0; }
./usr/lib/modprobe.d/50-nfs.conf:install lockd /sbin/modprobe --ignore-install lockd $CMDLINE_OPTS && { /sbin/sysctl -q --pattern fs.nfs.n[sl]m --system; exit 0; }
./usr/lib/modprobe.d/50-nfs.conf:install nfsv4 /sbin/modprobe --ignore-install nfsv4 $CMDLINE_OPTS && { /sbin/sysctl -q --pattern 'fs.nfs.(nfs_callback_tcpport|idmap_cache_timeout)' --system; exit 0; }
./usr/lib/modprobe.d/50-nfs.conf:install nfs /sbin/modprobe --ignore-install nfs $CMDLINE_OPTS && { /sbin/sysctl -q --pattern fs.nfs --system; exit 0; }

@johannbg
Copy link
Collaborator

johannbg commented Nov 5, 2022

If this gets added it belongs in the systemd sysctl module not in the dracut systemd module.

@Conan-Kudo
Copy link
Member Author

Obviously I could add it there and it'd coincidentally work, but the systemd-sysctl module isn't where the breakage is occurring. It makes sense to me to add the binary in the module that winds up invoking the tool.

@johannbg
Copy link
Collaborator

johannbg commented Nov 5, 2022

Arguably the modules that require sysctl be invoked in the firstplace are broken and those should be fixed so there is high probably this gets nacked

@iammattcoleman
Copy link
Contributor

Arguably the modules that require sysctl be invoked in the firstplace are broken and those should be fixed so there is high probably this gets nacked

While I agree with this philosophically, the practical reality is that systemd-sysctl is not universally adopted. To try to dictate that every other distribution and creator of modprobe drop-in config files needs to use it seems like an uphill battle.

@LaszloGombos
Copy link
Collaborator

How do you all feel about adding sysctl to the kernel-modules dracut module as an optional dependency ?
If usage of sysctl is indeed modprobe related, than kernel-modules could be a better home for this as kernel-modules dracut module is responsible for installing the modprobe *.conf files into initrd.

@johannbg
Copy link
Collaborator

johannbg commented Nov 5, 2022

Arguably the modules that require sysctl be invoked in the firstplace are broken and those should be fixed so there is high probably this gets nacked

While I agree with this philosophically, the practical reality is that systemd-sysctl is not universally adopted. To try to dictate that every other distribution and creator of modprobe drop-in config files needs to use it seems like an uphill battle.

We make the assumption that distribution that adopted systemd did so wholesale trying to support ca 250 systemd based distribution that randomly decided which component of systemd it uses is madness.

@johannbg
Copy link
Collaborator

johannbg commented Nov 5, 2022

How do you all feel about adding sysctl to the kernel-modules dracut module as an optional dependency ? If usage of sysctl is indeed modprobe related, than kernel-modules could be a better home for this as kernel-modules dracut module is responsible for installing the modprobe *.conf files into initrd.

That's an ack from me.

@Conan-Kudo
Copy link
Member Author

I'm fine with that too, that's where I initially thought to do it, but when I did the search for what loaded the files, this module came up instead.

@Conan-Kudo Conan-Kudo changed the title fix(dracut-systemd): Add sysctl to initramfs to handle modprobe files fix(kernel-modules): Add sysctl to initramfs to handle modprobe files Nov 5, 2022
@github-actions github-actions bot added kernel-modules Issues related to the kernel-modules module and removed dracut-systemd Issues related to the dracut-systemd module labels Nov 5, 2022
@Conan-Kudo
Copy link
Member Author

@LaszloGombos I just moved it over to the kernel-modules module now.

Users were seeing errors like this:

[     2.917246] dracut-pre-udev[717]: sh: line 1: /sbin/sysctl: No such file or directory

This was the result of modprobe.d files that needed to call sysctl
and failing because sysctl wasn't included in the initramfs.

This change makes it so that we have the binary included so those
modprobe configuration files work properly.
Copy link
Collaborator

@LaszloGombos LaszloGombos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@LaszloGombos
Copy link
Collaborator

Perhaps once this PR lands, we no longer need this code in the test - 4a5785a..

@aafeijoo-suse aafeijoo-suse merged commit 33679ff into dracutdevs:master Nov 11, 2022
@Conan-Kudo Conan-Kudo deleted the add-sysctl-binary branch November 11, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel-modules Issues related to the kernel-modules module modules Issue tracker for all modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants