| Message ID | 20181218050235.27187-3-casantos@datacom.com.br |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add a sysctl init script | expand |
Carlos, On Mon, Dec 17, 2018 at 11:03 PM Carlos Santos <casantos@datacom.com.br> wrote: > > Add a simple init script that invokes sysctl early in the initialization > process to configure kernel parameters. This is already performed by > systemd (systemd-sysctl) but there is no sysvinit/busybox counterpart. > > Files are read from directories in the following list in the given order > from top to bottom: > > /run/sysctl.d/*.conf > /etc/sysctl.d/*.conf > /usr/local/lib/sysctl.d/*.conf > /usr/lib/sysctl.d/*.conf > /lib/sysctl.d/*.conf > /etc/sysctl.conf > > A file may be used more than once, since there can be multiple symlinks > to it. No attempt is made to prevent this. > > Busybox's sysctl does not support "--quiet --system" arguments like the > sysctl provided by procps-ng, so we resort to some scripting to mimic > its behavior. > > Signed-off-by: Carlos Santos <casantos@datacom.com.br> Thanks for sending this one in. We usually have some version of a script that does similar in each product rootfs overlay. I've reviewed the general function of the script is correct, I did not verify the SYSCTL_SOURCES are all the right plans to include. We primarily just use the /etc/sysctl.conf default config and /etc/sysctl.d/. Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com> > --- > package/busybox/S01sysctl | 63 ++++++++++++++++++++++++++++++++++++++ > package/busybox/busybox.mk | 12 ++++++++ > 2 files changed, 75 insertions(+) > create mode 100644 package/busybox/S01sysctl > > diff --git a/package/busybox/S01sysctl b/package/busybox/S01sysctl > new file mode 100644 > index 0000000000..35e0cef291 > --- /dev/null > +++ b/package/busybox/S01sysctl > @@ -0,0 +1,63 @@ > +#!/bin/sh > + > +PROGRAM="sysctl" > + > +SYSCTL_ARGS="" > + > +# shellcheck source=/dev/null > +[ -r "/etc/default/$PROGRAM" ] && . "/etc/default/$PROGRAM" > + > +# Files are read from directories in the SYSCTL_SOURCES list, in given order. > +SYSCTL_SOURCES="/etc/sysctl.d/ /usr/local/lib/sysctl.d/ /usr/lib/sysctl.d/ /lib/sysctl.d/ /etc/sysctl.conf" > + > +# A file may be used more than once, since there can be multiple symlinks to > +# it. No attempt is made to prevent this. > +# > +# Busybox's sysctl does not support "--quiet --system" arguments like the > +# sysctl provided by procps-ng, so we resort to some scripting to mimic its > +# behavior. > +# > +# The "return 1" below is fruitless, at the moment, since sysctl ends with > +# status zero even if errors happen. Hopefully this will be fixed in a future > +# version of Busybox. > +apply_configs() { > + # shellcheck disable=SC2086 # we need the word splitting > + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ Why restrict to .conf? > + xargs -0 -r readlink -f | \ > + { > + read -r file > + [ -z "$file" ] || /sbin/sysctl -q -p "$file" $SYSCTL_ARGS || return 1 > + } > +} > + > +start() { > + printf 'Starting %s: ' "$PROGRAM" > + apply_configs > + status=$? > + if [ "$status" -eq 0 ]; then > + echo "OK" > + else > + echo "FAIL" > + fi > + return "$status" > +} > + > +stop() { > + printf 'Stopping %s: OK\n' "$PROGRAM" > +} > + > +restart() { > + stop > + sleep 1 > + start > +} > + > +case "$1" in > + start|stop) > + "$1";; > + restart|reload) > + restart;; > + *) > + echo "Usage: $0 {start|stop|restart|reload}" > + exit 1 > +esac > diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk > index bfcca6ed3e..bf101d7d46 100644 > --- a/package/busybox/busybox.mk > +++ b/package/busybox/busybox.mk > @@ -259,6 +259,17 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT > endef > endif > > +# Only install our sysctl scripts if no other package does it. > +ifeq ($(BR2_PACKAGE_PROCPS_NG),) > +define BUSYBOX_INSTALL_SYSCTL_SCRIPT > + if grep -q CONFIG_BB_SYSCTL=y $(@D)/.config; \ > + then \ > + $(INSTALL) -m 0755 -D package/busybox/S01sysctl \ > + $(TARGET_DIR)/etc/init.d/S01sysctl ; \ > + fi > +endef > +endif > + > ifeq ($(BR2_INIT_BUSYBOX),y) > define BUSYBOX_INSTALL_INITTAB > $(INSTALL) -D -m 0644 package/busybox/inittab $(TARGET_DIR)/etc/inittab > @@ -344,6 +355,7 @@ define BUSYBOX_INSTALL_INIT_SYSV > $(BUSYBOX_INSTALL_MDEV_SCRIPT) > $(BUSYBOX_INSTALL_LOGGING_SCRIPT) > $(BUSYBOX_INSTALL_WATCHDOG_SCRIPT) > + $(BUSYBOX_INSTALL_SYSCTL_SCRIPT) > $(BUSYBOX_INSTALL_TELNET_SCRIPT) > $(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES) > endef > -- > 2.19.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
> From: "Matthew Weber" <Matthew.Weber@collins.com> > To: "Carlos Santos" <casantos@datacom.com.br> > Cc: "buildroot" <buildroot@buildroot.org>, "ratbert90" <aduskett@gmail.com>, "DATACOM" <rodrigo.rebello@datacom.com.br> > Sent: Terça-feira, 18 de dezembro de 2018 11:49:11 > Subject: Re: [Buildroot] [PATCH 2/2] package/busybox: add init script for sysctl > Carlos, > > > > On Mon, Dec 17, 2018 at 11:03 PM Carlos Santos <casantos@datacom.com.br> wrote: >> >> Add a simple init script that invokes sysctl early in the initialization >> process to configure kernel parameters. This is already performed by >> systemd (systemd-sysctl) but there is no sysvinit/busybox counterpart. >> >> Files are read from directories in the following list in the given order >> from top to bottom: >> >> /run/sysctl.d/*.conf >> /etc/sysctl.d/*.conf >> /usr/local/lib/sysctl.d/*.conf >> /usr/lib/sysctl.d/*.conf >> /lib/sysctl.d/*.conf >> /etc/sysctl.conf >> >> A file may be used more than once, since there can be multiple symlinks >> to it. No attempt is made to prevent this. >> >> Busybox's sysctl does not support "--quiet --system" arguments like the >> sysctl provided by procps-ng, so we resort to some scripting to mimic >> its behavior. >> >> Signed-off-by: Carlos Santos <casantos@datacom.com.br> > > Thanks for sending this one in. We usually have some version of a > script that does similar in each product rootfs overlay. Same case here. > I've reviewed the general function of the script is correct, I did not > verify the SYSCTL_SOURCES are all the right plans to include. We > primarily just use the /etc/sysctl.conf default config and > /etc/sysctl.d/. > > Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com> > >> --- >> package/busybox/S01sysctl | 63 ++++++++++++++++++++++++++++++++++++++ >> package/busybox/busybox.mk | 12 ++++++++ >> 2 files changed, 75 insertions(+) >> create mode 100644 package/busybox/S01sysctl >> >> diff --git a/package/busybox/S01sysctl b/package/busybox/S01sysctl >> new file mode 100644 >> index 0000000000..35e0cef291 >> --- /dev/null >> +++ b/package/busybox/S01sysctl >> @@ -0,0 +1,63 @@ >> +#!/bin/sh >> + >> +PROGRAM="sysctl" >> + >> +SYSCTL_ARGS="" >> + >> +# shellcheck source=/dev/null >> +[ -r "/etc/default/$PROGRAM" ] && . "/etc/default/$PROGRAM" >> + >> +# Files are read from directories in the SYSCTL_SOURCES list, in given order. >> +SYSCTL_SOURCES="/etc/sysctl.d/ /usr/local/lib/sysctl.d/ /usr/lib/sysctl.d/ >> /lib/sysctl.d/ /etc/sysctl.conf" >> + >> +# A file may be used more than once, since there can be multiple symlinks to >> +# it. No attempt is made to prevent this. >> +# >> +# Busybox's sysctl does not support "--quiet --system" arguments like the >> +# sysctl provided by procps-ng, so we resort to some scripting to mimic its >> +# behavior. >> +# >> +# The "return 1" below is fruitless, at the moment, since sysctl ends with >> +# status zero even if errors happen. Hopefully this will be fixed in a future >> +# version of Busybox. >> +apply_configs() { >> + # shellcheck disable=SC2086 # we need the word splitting >> + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ > > Why restrict to .conf? That's what procps-ng's sysctl does and I wanted to mimic it as faithfully as possible. It also prevents attempting to use some README file. [...]
On 18/12/2018 06:02, Carlos Santos wrote: > + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ > + xargs -0 -r readlink -f | \ > + { > + read -r file > + [ -z "$file" ] || /sbin/sysctl -q -p "$file" $SYSCTL_ARGS || return 1 > + } So it will only read the first file it finds ?
> From: "Nicolas Cavallari" <Nicolas.Cavallari@green-communications.fr> > To: "Carlos Santos" <casantos@datacom.com.br>, "buildroot" <buildroot@buildroot.org> > Cc: "ratbert90" <aduskett@gmail.com>, "DATACOM" <rodrigo.rebello@datacom.com.br> > Sent: Terça-feira, 18 de dezembro de 2018 14:32:18 > Subject: Re: [Buildroot] [PATCH 2/2] package/busybox: add init script for sysctl > On 18/12/2018 06:02, Carlos Santos wrote: >> + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ >> + xargs -0 -r readlink -f | \ >> + { >> + read -r file >> + [ -z "$file" ] || /sbin/sysctl -q -p "$file" $SYSCTL_ARGS || return 1 >> + } > > So it will only read the first file it finds ? The "||" operator means "do the RHS if the LHS is false", so if "$file" is empty it doesn't run sysctl; otherwise if syscl fails (which currently never happens due to a bug in Busybox) it returns from the function.
> From: "Carlos Santos" <casantos@datacom.com.br> > To: "Nicolas Cavallari" <Nicolas.Cavallari@green-communications.fr> > Cc: "DATACOM" <rodrigo.rebello@datacom.com.br>, "ratbert90" <aduskett@gmail.com>, "buildroot" <buildroot@buildroot.org> > Sent: Terça-feira, 18 de dezembro de 2018 15:19:49 > Subject: Re: [Buildroot] [PATCH 2/2] package/busybox: add init script for sysctl >> From: "Nicolas Cavallari" <Nicolas.Cavallari@green-communications.fr> >> To: "Carlos Santos" <casantos@datacom.com.br>, "buildroot" >> <buildroot@buildroot.org> >> Cc: "ratbert90" <aduskett@gmail.com>, "DATACOM" <rodrigo.rebello@datacom.com.br> >> Sent: Terça-feira, 18 de dezembro de 2018 14:32:18 >> Subject: Re: [Buildroot] [PATCH 2/2] package/busybox: add init script for sysctl > >> On 18/12/2018 06:02, Carlos Santos wrote: >>> + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ >>> + xargs -0 -r readlink -f | \ >>> + { >>> + read -r file >>> + [ -z "$file" ] || /sbin/sysctl -q -p "$file" $SYSCTL_ARGS || return 1 >>> + } >> >> So it will only read the first file it finds ? > > The "||" operator means "do the RHS if the LHS is false", so if "$file" > is empty it doesn't run sysctl; otherwise if syscl fails (which currently > never happens due to a bug in Busybox) it returns from the function. Dooh, you were right. There should be a while there, so it is reading only the first file name. I will fix this and send a new patch. Thanks
diff --git a/package/busybox/S01sysctl b/package/busybox/S01sysctl new file mode 100644 index 0000000000..35e0cef291 --- /dev/null +++ b/package/busybox/S01sysctl @@ -0,0 +1,63 @@ +#!/bin/sh + +PROGRAM="sysctl" + +SYSCTL_ARGS="" + +# shellcheck source=/dev/null +[ -r "/etc/default/$PROGRAM" ] && . "/etc/default/$PROGRAM" + +# Files are read from directories in the SYSCTL_SOURCES list, in given order. +SYSCTL_SOURCES="/etc/sysctl.d/ /usr/local/lib/sysctl.d/ /usr/lib/sysctl.d/ /lib/sysctl.d/ /etc/sysctl.conf" + +# A file may be used more than once, since there can be multiple symlinks to +# it. No attempt is made to prevent this. +# +# Busybox's sysctl does not support "--quiet --system" arguments like the +# sysctl provided by procps-ng, so we resort to some scripting to mimic its +# behavior. +# +# The "return 1" below is fruitless, at the moment, since sysctl ends with +# status zero even if errors happen. Hopefully this will be fixed in a future +# version of Busybox. +apply_configs() { + # shellcheck disable=SC2086 # we need the word splitting + find $SYSCTL_SOURCES -maxdepth 1 -name '*.conf' -print0 2> /dev/null | \ + xargs -0 -r readlink -f | \ + { + read -r file + [ -z "$file" ] || /sbin/sysctl -q -p "$file" $SYSCTL_ARGS || return 1 + } +} + +start() { + printf 'Starting %s: ' "$PROGRAM" + apply_configs + status=$? + if [ "$status" -eq 0 ]; then + echo "OK" + else + echo "FAIL" + fi + return "$status" +} + +stop() { + printf 'Stopping %s: OK\n' "$PROGRAM" +} + +restart() { + stop + sleep 1 + start +} + +case "$1" in + start|stop) + "$1";; + restart|reload) + restart;; + *) + echo "Usage: $0 {start|stop|restart|reload}" + exit 1 +esac diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk index bfcca6ed3e..bf101d7d46 100644 --- a/package/busybox/busybox.mk +++ b/package/busybox/busybox.mk @@ -259,6 +259,17 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT endef endif +# Only install our sysctl scripts if no other package does it. +ifeq ($(BR2_PACKAGE_PROCPS_NG),) +define BUSYBOX_INSTALL_SYSCTL_SCRIPT + if grep -q CONFIG_BB_SYSCTL=y $(@D)/.config; \ + then \ + $(INSTALL) -m 0755 -D package/busybox/S01sysctl \ + $(TARGET_DIR)/etc/init.d/S01sysctl ; \ + fi +endef +endif + ifeq ($(BR2_INIT_BUSYBOX),y) define BUSYBOX_INSTALL_INITTAB $(INSTALL) -D -m 0644 package/busybox/inittab $(TARGET_DIR)/etc/inittab @@ -344,6 +355,7 @@ define BUSYBOX_INSTALL_INIT_SYSV $(BUSYBOX_INSTALL_MDEV_SCRIPT) $(BUSYBOX_INSTALL_LOGGING_SCRIPT) $(BUSYBOX_INSTALL_WATCHDOG_SCRIPT) + $(BUSYBOX_INSTALL_SYSCTL_SCRIPT) $(BUSYBOX_INSTALL_TELNET_SCRIPT) $(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES) endef
Add a simple init script that invokes sysctl early in the initialization process to configure kernel parameters. This is already performed by systemd (systemd-sysctl) but there is no sysvinit/busybox counterpart. Files are read from directories in the following list in the given order from top to bottom: /run/sysctl.d/*.conf /etc/sysctl.d/*.conf /usr/local/lib/sysctl.d/*.conf /usr/lib/sysctl.d/*.conf /lib/sysctl.d/*.conf /etc/sysctl.conf A file may be used more than once, since there can be multiple symlinks to it. No attempt is made to prevent this. Busybox's sysctl does not support "--quiet --system" arguments like the sysctl provided by procps-ng, so we resort to some scripting to mimic its behavior. Signed-off-by: Carlos Santos <casantos@datacom.com.br> --- package/busybox/S01sysctl | 63 ++++++++++++++++++++++++++++++++++++++ package/busybox/busybox.mk | 12 ++++++++ 2 files changed, 75 insertions(+) create mode 100644 package/busybox/S01sysctl