diff mbox series

[4/4] systemd: Apply all sysctl settings through udev rule when NFS-related modules are loaded

Message ID 20221125130725.1977606-5-carnil@debian.org (mailing list archive)
State New, archived
Headers show
Series Replace sysctl setting invocations triggered from udev rule instead of modprobe configuration | expand

Commit Message

Salvatore Bonaccorso Nov. 25, 2022, 1:07 p.m. UTC 
sysctl settings (e.g.  /etc/sysctl.conf and others) are normally loaded
once at boot.  If the module that implements some settings is no yet
loaded, those settings don't get applied.

Various NFS modules support various sysctl settings.  If they are loaded
after boot, they miss out.

Add a new udev rule configuration to udev/rules.d/60-nfs.rules to apply
the relevant settings when the module is loaded.

Placing it in the systemd directory similarly as the coice for the
original commit afc7132dfb21 ("systemd: Apply all sysctl settings when
NFS-related modules are loaded").

Link: https://lore.kernel.org/linux-nfs/Y1KoKwu88PulcokW@eldamar.lan/
Link: https://bugs.debian.org/1022172
Link: https://bugs.debian.org/1024082
Suggested-by: Marco d'Itri <md@linux.it>
Signed-off-by: Salvatore Bonaccorso <carnil@debian.org>
---
 systemd/60-nfs.rules | 21 +++++++++++++++++++++
 systemd/Makefile.am  |  9 +++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 systemd/60-nfs.rules

Comments

Michael Prokop Nov. 25, 2022, 1:29 p.m. UTC | #1 
Hi,

* Salvatore Bonaccorso [Fri Nov 25, 2022 at 02:07:25PM +0100]:

> sysctl settings (e.g.  /etc/sysctl.conf and others) are normally loaded
> once at boot.  If the module that implements some settings is no yet
> loaded, those settings don't get applied.
> 
> Various NFS modules support various sysctl settings.  If they are loaded
> after boot, they miss out.
> 
> Add a new udev rule configuration to udev/rules.d/60-nfs.rules to apply
> the relevant settings when the module is loaded.
> 
> Placing it in the systemd directory similarly as the coice for the
> original commit afc7132dfb21 ("systemd: Apply all sysctl settings when
> NFS-related modules are loaded").
[...]

> --- /dev/null
> +++ b/systemd/60-nfs.rules
> @@ -0,0 +1,21 @@
> +# Ensure all NFS systctl settings get applied when modules load
> +
> +# sunrpc module supports "sunrpc.*" sysctls
> +ACTION=="add", SUBSYSTEM=="module", KERNEL=="sunrpc", \
> +  RUN+="/sbin/sysctl -q --pattern ^sunrpc --system"
[...]

Thanks for taking care of this problem, Salvatore!

AFAICT even latest busybox's sysctl does not support the `--pattern`
option yet:

| sysctl: unrecognized option '--pattern'
| BusyBox v1.35.0 (Debian 1:1.35.0-4) multi-call binary.
| [....]

So any initramfs that uses busybox and its sysctl (like in Debian)
and trying to apply above udev rules might fail?

regards
-mika-
Salvatore Bonaccorso Nov. 25, 2022, 4:21 p.m. UTC | #2 
Hi Michael,

On Fri, Nov 25, 2022 at 02:29:35PM +0100, Michael Prokop wrote:
> Hi,
> 
> * Salvatore Bonaccorso [Fri Nov 25, 2022 at 02:07:25PM +0100]:
> 
> > sysctl settings (e.g.  /etc/sysctl.conf and others) are normally loaded
> > once at boot.  If the module that implements some settings is no yet
> > loaded, those settings don't get applied.
> > 
> > Various NFS modules support various sysctl settings.  If they are loaded
> > after boot, they miss out.
> > 
> > Add a new udev rule configuration to udev/rules.d/60-nfs.rules to apply
> > the relevant settings when the module is loaded.
> > 
> > Placing it in the systemd directory similarly as the coice for the
> > original commit afc7132dfb21 ("systemd: Apply all sysctl settings when
> > NFS-related modules are loaded").
> [...]
> 
> > --- /dev/null
> > +++ b/systemd/60-nfs.rules
> > @@ -0,0 +1,21 @@
> > +# Ensure all NFS systctl settings get applied when modules load
> > +
> > +# sunrpc module supports "sunrpc.*" sysctls
> > +ACTION=="add", SUBSYSTEM=="module", KERNEL=="sunrpc", \
> > +  RUN+="/sbin/sysctl -q --pattern ^sunrpc --system"
> [...]
> 
> Thanks for taking care of this problem, Salvatore!

Thanks to you for prodding about it, hope to bring the issue bit
forward with the series proposal.

> AFAICT even latest busybox's sysctl does not support the `--pattern`
> option yet:
> 
> | sysctl: unrecognized option '--pattern'
> | BusyBox v1.35.0 (Debian 1:1.35.0-4) multi-call binary.
> | [....]
> 
> So any initramfs that uses busybox and its sysctl (like in Debian)
> and trying to apply above udev rules might fail?

But would this actually be a problem for us here? There is no hook
script which would copy the 60-nfs.rules (not relevant in initrd) to
the initrd. The rule only would apply on module load outside the
initrd.

There is only a subset of rules which would be copied into initrd,
like the ones in hook/udev. But 60-nfs.rules would be specific to
nfs-utils, which does not provide a initramfs-tools hook to include
the rules into initrd.

Now the question you raise, is, do they need to be handled actually
already as well in initrd? You are correct, when handled through the
previous mechanism with modrobe.d configuration, 50-nfs.conf was added
to initramfs:

usr/lib/modprobe.d/50-nfs.conf 

(and causing the issues seen).

Please correct me if I missed something from the picture.

Regards,
Salvatore
Michael Prokop Nov. 25, 2022, 4:27 p.m. UTC | #3 
Hi!

* Salvatore Bonaccorso [Fri Nov 25, 2022 at 05:21:41PM +0100]:
> On Fri, Nov 25, 2022 at 02:29:35PM +0100, Michael Prokop wrote:
> > * Salvatore Bonaccorso [Fri Nov 25, 2022 at 02:07:25PM +0100]:

> > > --- /dev/null
> > > +++ b/systemd/60-nfs.rules
> > > @@ -0,0 +1,21 @@
> > > +# Ensure all NFS systctl settings get applied when modules load
> > > +
> > > +# sunrpc module supports "sunrpc.*" sysctls
> > > +ACTION=="add", SUBSYSTEM=="module", KERNEL=="sunrpc", \
> > > +  RUN+="/sbin/sysctl -q --pattern ^sunrpc --system"
> > [...]
> > 
> > Thanks for taking care of this problem, Salvatore!
> 
> Thanks to you for prodding about it, hope to bring the issue bit
> forward with the series proposal.

ACK, thanks, I highly appreciate your efforts!

> > AFAICT even latest busybox's sysctl does not support the `--pattern`
> > option yet:
> > 
> > | sysctl: unrecognized option '--pattern'
> > | BusyBox v1.35.0 (Debian 1:1.35.0-4) multi-call binary.
> > | [....]
> > 
> > So any initramfs that uses busybox and its sysctl (like in Debian)
> > and trying to apply above udev rules might fail?
> 
> But would this actually be a problem for us here? There is no hook
> script which would copy the 60-nfs.rules (not relevant in initrd) to
> the initrd. The rule only would apply on module load outside the
> initrd.

Indeed, I also think that as long as this udev rule doesn't end up
in the initrd there shouldn't be any problem with it.

> There is only a subset of rules which would be copied into initrd,
> like the ones in hook/udev. But 60-nfs.rules would be specific to
> nfs-utils, which does not provide a initramfs-tools hook to include
> the rules into initrd.

Good point, thanks for checking and clarifying.

> Now the question you raise, is, do they need to be handled actually
> already as well in initrd? You are correct, when handled through the
> previous mechanism with modrobe.d configuration, 50-nfs.conf was added
> to initramfs:
> 
> usr/lib/modprobe.d/50-nfs.conf 
> 
> (and causing the issues seen).
> 
> Please correct me if I missed something from the picture.

No, I think you're right and AFAICS we shouldn't see the issues we
originally noticed any longer. Thanks! :)

regards
-mika-
diff mbox series

Patch

diff --git a/systemd/60-nfs.rules b/systemd/60-nfs.rules
new file mode 100644
index 000000000000..188423c1d2e3
--- /dev/null
+++ b/systemd/60-nfs.rules
@@ -0,0 +1,21 @@ 
+# Ensure all NFS systctl settings get applied when modules load
+
+# sunrpc module supports "sunrpc.*" sysctls
+ACTION=="add", SUBSYSTEM=="module", KERNEL=="sunrpc", \
+  RUN+="/sbin/sysctl -q --pattern ^sunrpc --system"
+
+# rpcrdma module supports sunrpc.svc_rdma.*
+ACTION=="add", SUBSYSTEM=="module", KERNEL=="rpcrdma", \
+  RUN+="/sbin/sysctl -q --pattern ^sunrpc.svc_rdma --system"
+
+# lockd module supports "fs.nfs.nlm*" and "fs.nfs.nsm*" sysctls
+ACTION=="add", SUBSYSTEM=="module", KERNEL=="lockd", \
+  RUN+="/sbin/sysctl -q --pattern ^fs.nfs.n[sl]m --system"
+
+# nfsv4 module supports "fs.nfs.*" sysctls (nfs_callback_tcpport and idmap_cache_timeout)
+ACTION=="add", SUBSYSTEM=="module", KERNEL=="nfsv4", \
+  RUN+="/sbin/sysctl -q --pattern ^fs.nfs.(nfs_callback_tcpport|idmap_cache_timeout) --system"
+
+# nfs module supports "fs.nfs.*" sysctls
+ACTION=="add", SUBSYSTEM=="module", KERNEL=="nfs", \
+  RUN+="/sbin/sysctl -q --pattern ^fs.nfs --system"
diff --git a/systemd/Makefile.am b/systemd/Makefile.am
index e7f5d818a913..577c6a2286c0 100644
--- a/systemd/Makefile.am
+++ b/systemd/Makefile.am
@@ -2,6 +2,9 @@ 
 
 MAINTAINERCLEANFILES = Makefile.in
 
+udev_rulesdir = /usr/lib/udev/rules.d/
+udev_files = 60-nfs.rules
+
 unit_files =  \
     nfs-client.target \
     rpc_pipefs.target \
@@ -51,7 +54,7 @@  endif
 
 man5_MANS	= nfs.conf.man
 man7_MANS	= nfs.systemd.man
-EXTRA_DIST = $(unit_files) $(man5_MANS) $(man7_MANS)
+EXTRA_DIST = $(unit_files) $(udev_files) $(man5_MANS) $(man7_MANS)
 
 generator_dir = $(unitdir)/../system-generators
 
@@ -73,8 +76,10 @@  rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.la
 
 if INSTALL_SYSTEMD
 genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
-install-data-hook: $(unit_files)
+install-data-hook: $(unit_files) $(udev_files)
 	mkdir -p $(DESTDIR)/$(unitdir)
 	cp $(unit_files) $(DESTDIR)/$(unitdir)
 	cp $(rpc_pipefs_mount_file) $(DESTDIR)/$(unitdir)/$(rpc_pipefsmount)
+	mkdir -p $(DESTDIR)/$(udev_rulesdir)
+	cp $(udev_files) $(DESTDIR)/$(udev_rulesdir)
 endif