Skip to contentSkip to conversation

package/dhcp: fix SysV init scripts option passing

PATCH New Benoît Thébaudeau 3 participants 12 messages Oct 13, 2015 – Oct 14, 2015

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/8 package/dhcp: fix SysV init scripts option passing

The SysV init scripts have configuration variables like INTERFACES whose contents have to be passed to the daemon. These variables are initialized as empty strings, but some of them are not allowed to be empty and there was no means of filling them apart from creating a root FS overlay to overwrite these scripts.

This commit adds support for files under /etc/default/ to set these configuration variables. Such light files can now be added to the root FS skeleton or overlays without having to duplicate most of the SysV init scripts.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-relay  | 4 ++++
 package/dhcp/S80dhcp-server | 4 ++++
 2 files changed, 8 insertions(+)
package/dhcp/S80dhcp-relay +4 -0
index 5ee06c7..0f383e6 100755
@@ -13,6 +13,10 @@ INTERFACES=""
 # Additional options that are passed to the DHCP relay daemon?
 OPTIONS=""
 
+# Read configuration variable file if it is present
+CFG_FILE="/etc/default/dhcrelay"
+[ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
+
 # Sanity checks
 test -f /usr/sbin/dhcrelay || exit 0
 test -n "$INTERFACES" || exit 0
package/dhcp/S80dhcp-server +4 -0
index 3df14ff..f7907e2 100755
@@ -7,6 +7,10 @@
 #       Separate multiple interfaces with spaces, e.g. "eth0 eth1".
 INTERFACES=""
 
+# Read configuration variable file if it is present
+CFG_FILE="/etc/default/dhcpd"
+[ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
+
 # Sanity checks
 test -f /usr/sbin/dhcpd || exit 0
 test -f /etc/dhcp/dhcpd.conf || exit 0
Signature · 1 line
2.1.4
#1 lore raw

PATCH v2 2/8 package/dhcp/S80dhcp-server: support extra options

Add an OPTIONS configuration variable in order to make it possible to pass custom extra options to dhcpd.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-server | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
package/dhcp/S80dhcp-server +4 -1
index f7907e2..c1ef53b 100755
@@ -7,6 +7,9 @@
 #       Separate multiple interfaces with spaces, e.g. "eth0 eth1".
 INTERFACES=""
 
+# Additional options that are passed to the DHCP server daemon?
+OPTIONS=""
+
 # Read configuration variable file if it is present
 CFG_FILE="/etc/default/dhcpd"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
@@ -21,7 +24,7 @@ case "$1" in
 		printf "Starting DHCP server: "
 		test -d /var/lib/dhcp/ || mkdir -p /var/lib/dhcp/
 		test -f /var/lib/dhcp/dhcpd.leases || touch /var/lib/dhcp/dhcpd.leases
-		start-stop-daemon -S -x /usr/sbin/dhcpd -- -q $INTERFACES
+		start-stop-daemon -S -x /usr/sbin/dhcpd -- -q $OPTIONS $INTERFACES
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
Signature · 1 line
2.1.4
#2 lore raw

PATCH v2 3/8 package/dhcp/S80dhcp-server: allow empty INTERFACES

In reply to Benoît Thébaudeau

The dhcpd daemon does not require network interface names to be specified on the command line.

From dhcpd(8):
"The names of the network interfaces on which dhcpd should listen for broadcasts may be specified on the command line. This should be done on systems where dhcpd is unable to identify non-broadcast interfaces, but should not be required on other systems. If no interface names are specified on the command line dhcpd will identify all network interfaces which are up, eliminating non-broadcast interfaces if possible, and listen for DHCP broadcasts on each interface."

dhcpd exits with "Not configured to listen on any interfaces!" only if no requested (those in INTERFACES, or all if empty) non-broadcast interfaces matching the subnet declarations in dhcpd.conf are up.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-server | 1 -
 1 file changed, 1 deletion(-)
package/dhcp/S80dhcp-server +0 -1
index c1ef53b..079727b 100755
@@ -17,7 +17,6 @@ CFG_FILE="/etc/default/dhcpd"
 # Sanity checks
 test -f /usr/sbin/dhcpd || exit 0
 test -f /etc/dhcp/dhcpd.conf || exit 0
-test -n "$INTERFACES" || exit 0
 
 case "$1" in
 	start)
Signature · 1 line
2.1.4
#3 lore raw

PATCH v2 4/8 package/dhcp: fix SysV init scripts output text

In reply to Benoît Thébaudeau

Fix various messages displayed by these scripts:
- remove extra "dhcpd3" for S80dhcp-server stop,
- make start-stop-daemon quiet in order to avoid extra messages like "stopped /usr/sbin/dhcpd (pid 174)" being output between the command description and its result,
- fix the script names in the usage strings.

---
Changes v1 -> v2:
- Rebase.
- Use 'printf' instead of 'echo -n' following
0f75b2635ee564fbbdb9ea631cf39fa8731d6d6c.

---
 package/dhcp/S80dhcp-relay  | 6 +++---
 package/dhcp/S80dhcp-server | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)
package/dhcp/S80dhcp-relay +3 -3
index 0f383e6..6493eda 100755
@@ -33,12 +33,12 @@ DHCRELAYPID=/var/run/dhcrelay.pid
 case "$1" in
 	start)
 		printf "Starting DHCP relay: "
-		start-stop-daemon -S -x /usr/sbin/dhcrelay -- -q $OPTIONS $IFCMD $SERVERS
+		start-stop-daemon -S -q -x /usr/sbin/dhcrelay -- -q $OPTIONS $IFCMD $SERVERS
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
 		printf "Stopping DHCP relay: "
-		start-stop-daemon -K -x /usr/sbin/dhcrelay
+		start-stop-daemon -K -q -x /usr/sbin/dhcrelay
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	restart | force-reload)
@@ -47,7 +47,7 @@ case "$1" in
 		$0 start
 		;;
 	*)
-		echo "Usage: /etc/init.d/dhcp-relay {start|stop|restart|force-reload}"
+		echo "Usage: $0 {start|stop|restart|force-reload}"
 		exit 1
 esac
 
package/dhcp/S80dhcp-server +4 -4
index 079727b..c56e7c3 100755
@@ -23,12 +23,12 @@ case "$1" in
 		printf "Starting DHCP server: "
 		test -d /var/lib/dhcp/ || mkdir -p /var/lib/dhcp/
 		test -f /var/lib/dhcp/dhcpd.leases || touch /var/lib/dhcp/dhcpd.leases
-		start-stop-daemon -S -x /usr/sbin/dhcpd -- -q $OPTIONS $INTERFACES
+		start-stop-daemon -S -q -x /usr/sbin/dhcpd -- -q $OPTIONS $INTERFACES
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
-		printf "Stopping DHCP server: dhcpd3"
-		start-stop-daemon -K -x /usr/sbin/dhcpd
+		printf "Stopping DHCP server: "
+		start-stop-daemon -K -q -x /usr/sbin/dhcpd
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	restart | force-reload)
@@ -40,7 +40,7 @@ case "$1" in
 		fi
 		;;
 	*)
-		echo "Usage: /etc/init.d/dhcp-server {start|stop|restart|force-reload}"
+		echo "Usage: $0 {start|stop|restart|force-reload}"
 		exit 1
 esac
 
Signature · 1 line
2.1.4
#4 lore raw

PATCH v2 5/8 package/dhcp: remove sleep from SysV init scripts restart

In reply to Benoît Thébaudeau

No sleep is required for the restart and force-reload operations to succeed.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-relay  | 1 -
 package/dhcp/S80dhcp-server | 1 -
 2 files changed, 2 deletions(-)
package/dhcp/S80dhcp-relay +0 -1
index 6493eda..211431b 100755
@@ -43,7 +43,6 @@ case "$1" in
 		;;
 	restart | force-reload)
 		$0 stop
-		sleep 2
 		$0 start
 		;;
 	*)
package/dhcp/S80dhcp-server +0 -1
index c56e7c3..dc9c433 100755
@@ -33,7 +33,6 @@ case "$1" in
 		;;
 	restart | force-reload)
 		$0 stop
-		sleep 2
 		$0 start
 		if [ "$?" != "0" ]; then
 			exit 1
Signature · 1 line
2.1.4
#5 lore raw

PATCH v2 6/8 package/dhcp: SysV init scripts: remove PID files after stop

In reply to Benoît Thébaudeau

These daemons do not remove their PID files, so do it manually in the scripts.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-relay  | 12 ++++++++++--
 package/dhcp/S80dhcp-server | 12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)
package/dhcp/S80dhcp-relay +10 -2
index 211431b..9b8d65f 100755
@@ -17,6 +17,9 @@ OPTIONS=""
 CFG_FILE="/etc/default/dhcrelay"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
+# PID files generated by the daemon
+PID_FILES="/var/run/dhcrelay.pid /var/run/dhcrelay6.pid"
+
 # Sanity checks
 test -f /usr/sbin/dhcrelay || exit 0
 test -n "$INTERFACES" || exit 0
@@ -38,8 +41,13 @@ case "$1" in
 		;;
 	stop)
 		printf "Stopping DHCP relay: "
-		start-stop-daemon -K -q -x /usr/sbin/dhcrelay
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		if start-stop-daemon -K -q -x /usr/sbin/dhcrelay; then
+			# This daemon does not remove its PID file when it exits.
+			rm -f ${PID_FILES}
+			echo "OK"
+		else
+			echo "FAIL"
+		fi
 		;;
 	restart | force-reload)
 		$0 stop
package/dhcp/S80dhcp-server +10 -2
index dc9c433..1c2ff74 100755
@@ -14,6 +14,9 @@ OPTIONS=""
 CFG_FILE="/etc/default/dhcpd"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
+# PID files generated by the daemon
+PID_FILES="/var/run/dhcpd.pid /var/run/dhcpd6.pid"
+
 # Sanity checks
 test -f /usr/sbin/dhcpd || exit 0
 test -f /etc/dhcp/dhcpd.conf || exit 0
@@ -28,8 +31,13 @@ case "$1" in
 		;;
 	stop)
 		printf "Stopping DHCP server: "
-		start-stop-daemon -K -q -x /usr/sbin/dhcpd
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		if start-stop-daemon -K -q -x /usr/sbin/dhcpd; then
+			# This daemon does not remove its PID file when it exits.
+			rm -f ${PID_FILES}
+			echo "OK"
+		else
+			echo "FAIL"
+		fi
 		;;
 	restart | force-reload)
 		$0 stop
Signature · 1 line
2.1.4
#6 lore raw

PATCH v2 7/8 package/dhcp/S80dhcp-server: support IPv6 lease file

In reply to Benoît Thébaudeau

The IPv6 lease file has a different name.

---
Changes v1 -> v2:
- Rebase.

---
 package/dhcp/S80dhcp-server | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
package/dhcp/S80dhcp-server +4 -1
index 1c2ff74..b6f76ee 100755
@@ -25,7 +25,10 @@ case "$1" in
 	start)
 		printf "Starting DHCP server: "
 		test -d /var/lib/dhcp/ || mkdir -p /var/lib/dhcp/
-		test -f /var/lib/dhcp/dhcpd.leases || touch /var/lib/dhcp/dhcpd.leases
+		for lease_file in /var/lib/dhcp/dhcpd.leases \
+					/var/lib/dhcp/dhcpd6.leases; do
+			test -f "${lease_file}" || touch "${lease_file}"
+		done
 		start-stop-daemon -S -q -x /usr/sbin/dhcpd -- -q $OPTIONS $INTERFACES
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
Signature · 1 line
2.1.4
#7 lore raw

PATCH v2 8/8 package/dhcp: SysV init scripts: refactor using functions

In reply to Benoît Thébaudeau

Refactor these scripts using start()/stop() functions rather than having these scripts invoke themselves.

By the way, clean up these scripts.

---
Changes v1 -> v2:
- Rebase.
- Use 'printf' instead of 'echo -n' following
0f75b2635ee564fbbdb9ea631cf39fa8731d6d6c.

---
 package/dhcp/S80dhcp-relay  | 63 +++++++++++++++++++++-----------------
 package/dhcp/S80dhcp-server | 74 +++++++++++++++++++++++++--------------------
 2 files changed, 76 insertions(+), 61 deletions(-)
package/dhcp/S80dhcp-relay +35 -28
index 9b8d65f..b48f72c 100755
@@ -1,61 +1,68 @@
 #!/bin/sh
-#
-# $Id: dhcp3-relay,v 1.1 2004/04/16 15:41:08 ml Exp $
-#
+
+NAME="dhcrelay"
+DESC="DHCP relay"
+DAEMON="/usr/sbin/${NAME}"
+
+# On what interfaces should the DHCP relay serve DHCP requests?
+INTERFACES=""
 
 # What servers should the DHCP relay forward requests to?
-# e.g: SERVERS="192.168.0.1"
+# E.g: SERVERS="192.168.0.1"
 SERVERS=""
 
-# On what interfaces should the DHCP relay (dhrelay) serve DHCP requests?
-INTERFACES=""
-
 # Additional options that are passed to the DHCP relay daemon?
 OPTIONS=""
 
 # Read configuration variable file if it is present
-CFG_FILE="/etc/default/dhcrelay"
+CFG_FILE="/etc/default/${NAME}"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
 # PID files generated by the daemon
-PID_FILES="/var/run/dhcrelay.pid /var/run/dhcrelay6.pid"
+PID_FILES="/var/run/${NAME}.pid /var/run/${NAME}6.pid"
 
 # Sanity checks
-test -f /usr/sbin/dhcrelay || exit 0
+test -f "${DAEMON}" || exit 0
 test -n "$INTERFACES" || exit 0
 test -n "$SERVERS" || exit 0
 
-# Build command line for interfaces (will be passed to dhrelay below.)
+# Build command line for interfaces (will be passed to dhcrelay below).
 IFCMD=""
 for I in $INTERFACES; do
 	IFCMD=${IFCMD}"-i "${I}" "
 done
 
-DHCRELAYPID=/var/run/dhcrelay.pid
+start()
+{
+	printf "Starting ${DESC}: "
+	start-stop-daemon -S -q -x "${DAEMON}" -- -q $OPTIONS $IFCMD $SERVERS &&
+			echo "OK" || echo "FAIL"
+}
+
+stop()
+{
+	printf "Stopping ${DESC}: "
+	if start-stop-daemon -K -q -x "${DAEMON}"; then
+		# This daemon does not remove its PID file when it exits.
+		rm -f ${PID_FILES}
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+}
 
 case "$1" in
 	start)
-		printf "Starting DHCP relay: "
-		start-stop-daemon -S -q -x /usr/sbin/dhcrelay -- -q $OPTIONS $IFCMD $SERVERS
-		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		start
 		;;
 	stop)
-		printf "Stopping DHCP relay: "
-		if start-stop-daemon -K -q -x /usr/sbin/dhcrelay; then
-			# This daemon does not remove its PID file when it exits.
-			rm -f ${PID_FILES}
-			echo "OK"
-		else
-			echo "FAIL"
-		fi
+		stop
 		;;
-	restart | force-reload)
-		$0 stop
-		$0 start
+	restart|force-reload)
+		stop
+		start
 		;;
 	*)
 		echo "Usage: $0 {start|stop|restart|force-reload}"
 		exit 1
 esac
-
-exit 0
package/dhcp/S80dhcp-server +41 -33

98 lines not on this page · raw message

Signature · 1 line
2.1.4
#8 lore raw

PATCH v2 1/8 package/dhcp: fix SysV init scripts option passing

In reply to Benoît Thébaudeau

Hi Benoit,

Quoted 6 lines · Beno?t Th?baudeau

The SysV init scripts have configuration variables like INTERFACES whose contents have to be passed to the daemon. These variables are initialized as empty strings, but some of them are not allowed to be empty and there was no means of filling them apart from creating a root FS overlay to overwrite these scripts.

Quoted 8 lines

This commit adds support for files under /etc/default/ to set these configuration variables. Such light files can now be added to the root FS skeleton or overlays without having to duplicate most of the SysV init scripts.

Thanks a lot for your work, I am afraid I am going to add to your work and maybe be a bit of a kill joy:

What about systemd ?

There's already a service file in the dhcp package, so any changes made to the init scripts should also be made to the .service to keep the whole package coherent.

If you need any help/tips, do ask.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151014/d405aafd/attachment.html>

#9 lore raw

PATCH v2 1/8 package/dhcp: fix SysV init scripts option passing

Hi Maxime,

Quoted 16 lines · Maxime Hadjinlian

On Tue, Oct 13, 2015 at 11:35 AM, Beno?t Th?baudeau <benoit at wsystem.com <mailto:[email protected]>> wrote:

The SysV init scripts have configuration variables like INTERFACES whose contents have to be passed to the daemon. These variables are initialized as empty strings, but some of them are not allowed to be empty and there was no means of filling them apart from creating a root FS overlay to overwrite these scripts.

This commit adds support for files under /etc/default/ to set these configuration variables. Such light files can now be added to the root FS skeleton or overlays without having to duplicate most of the SysV init scripts.

Signed-off-by Beno?t Th?baudeau <benoit at wsystem.com <mailto:[email protected]>>
Quoted 7 lines

Thanks a lot for your work, I am afraid I am going to add to your work and maybe be a bit of a kill joy:

What about systemd ?

There's already a service file in the dhcp package, so any changes made to the init scripts should also be made to the .service to keep the whole package coherent.

If you need any help/tips, do ask.

I don't have any experience with systemd. I see the basics from the online documentation.

In order to switch an existing SysV + BusyBox defconfig to systemd, is there anything else to tweak than setting the init system to systemd in the configuration?

Looking at the files, most of this series seems to already be fine with systemd. The SysV init scripts have not been kept consistent when systemd was added. But there are a few things to do for systemd though.

For 1/8, would it be OK to change "EnvironmentFile=/etc/default/dhcpd.conf" to "EnvironmentFile=/etc/default/dhcpd" for systemd in order to share the filename with SysV? If I look into the /etc/default/ folder on my Ubuntu 15.04 (which uses systemd), none of the files present has a .conf extension. The purpose of these files is to set some environment variables, so they're more scripts than configuration files.

When a change would affect both SysV and systemd, should there be a single patch common to both, or one patch per init system?

Best regards,
Beno?t

#10 lore raw

PATCH v2 1/8 package/dhcp: fix SysV init scripts option passing

Hello Beno?t,

Quoted 18 lines · Beno?t Th?baudeau

I don't have any experience with systemd. I see the basics from the online documentation.

In order to switch an existing SysV + BusyBox defconfig to systemd, is there anything else to tweak than setting the init system to systemd in the configuration?

Looking at the files, most of this series seems to already be fine with systemd. The SysV init scripts have not been kept consistent when systemd was added. But there are a few things to do for systemd though.

For 1/8, would it be OK to change "EnvironmentFile=/etc/default/dhcpd.conf" to "EnvironmentFile=/etc/default/dhcpd" for systemd in order to share the filename with SysV? If I look into the /etc/default/ folder on my Ubuntu 15.04 (which uses systemd), none of the files present has a .conf extension. The purpose of these files is to set some environment variables, so they're more scripts than configuration files.

Quoted 2 lines

When a change would affect both SysV and systemd, should there be a single patch common to both, or one patch per init system?

Separate patches. And note that despite Maxime's question on systemd, you are definitely not forced to add systemd support to the dhcp package. It is fine if you do only SysV support, and someone else interested in systemd does the systemd part.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#11 lore raw

PATCH v2 1/8 package/dhcp: fix SysV init scripts option passing

In reply to Benoît Thébaudeau
Quoted 35 lines · Beno?t Th?baudeau

Hi Maxime,

Quoted 1 line · Maxime Hadjinlian

On Tue, Oct 13, 2015 at 11:35 AM, Beno?t Th?baudeau <[email protected]

Quoted 2 lines · <mailto:[email protected]>> wrote:

The SysV init scripts have configuration variables like INTERFACES

whose

Quoted 3 lines

contents have to be passed to the daemon. These variables are initialized as empty strings, but some of them are not allowed to be empty and there was no means of filling them apart from creating a

root

Quoted 4 lines

FS overlay to overwrite these scripts.

This commit adds support for files under /etc/default/ to set these configuration variables. Such light files can now be added to the

root

Quoted 4 lines

FS skeleton or overlays without having to duplicate most of the SysV init scripts.

benoit at wsystem.com>>

Quoted 3 lines

Thanks a lot for your work, I am afraid I am going to add to your work

and maybe be a bit of a kill joy:

Quoted 4 lines

What about systemd ?

There's already a service file in the dhcp package, so any changes made

to the init scripts should also be made to the .service to keep the whole package coherent.

Quoted 2 lines

If you need any help/tips, do ask.

Quoted 8 lines

I don't have any experience with systemd. I see the basics from the online documentation.

In order to switch an existing SysV + BusyBox defconfig to systemd, is there
anything else to tweak than setting the init system to systemd in the configuration?

That's it, that's all you gotta do.

Quoted 7 lines

Looking at the files, most of this series seems to already be fine with systemd.
The SysV init scripts have not been kept consistent when systemd was added. But
there are a few things to do for systemd though.

Quoted 12 lines

For 1/8, would it be OK to change "EnvironmentFile=/etc/default/dhcpd.conf" to "EnvironmentFile=/etc/default/dhcpd" for systemd in order to share the filename
with SysV? If I look into the /etc/default/ folder on my Ubuntu 15.04 (which
uses systemd), none of the files present has a .conf extension. The purpose of
these files is to set some environment variables, so they're more scripts than
configuration files.

Yep that's a good idea.

Quoted 5 lines

When a change would affect both SysV and systemd, should there be a single patch
common to both, or one patch per init system?

Separate patch would be better I think.

Quoted 4 lines

Best regards,
Beno?t

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151014/7007b7b7/attachment.html>