All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing
@ 2015-10-25  0:59 Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
                   ` (12 more replies)
  0 siblings, 13 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-relay  | 4 ++++
 package/dhcp/S80dhcp-server | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 5ee06c7..0f383e6 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -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
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index 3df14ff..f7907e2 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-11-04  9:52   ` Maxime Hadjinlian
  2015-12-24 14:00   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options Benoît Thébaudeau
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

Use the same EnvironmentFile name as the SysV init script for
consistency. The filenames under /etc/default/ are usually just the
package/daemon/service/feature name without any extension, so remove the
".conf" extension from EnvironmentFile.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

---
Changes v2 -> v3: new patch.
---
 package/dhcp/dhcpd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
index 7b265cb..5989506 100644
--- a/package/dhcp/dhcpd.service
+++ b/package/dhcp/dhcpd.service
@@ -7,7 +7,7 @@ Type=forking
 PIDFile=/run/dhcpd.pid
 ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
 KillSignal=SIGINT
-EnvironmentFile=/etc/default/dhcpd.conf
+EnvironmentFile=/etc/default/dhcpd
 
 [Install]
 WantedBy=multi-user.target
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-12-24 14:00   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: " Benoît Thébaudeau
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-server | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index f7907e2..c1ef53b 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: support extra options
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-11-04 10:09   ` Maxime Hadjinlian
  2015-12-24 14:01   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES Benoît Thébaudeau
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

Add an OPTIONS configuration variable in order to make it possible to
pass custom extra options to dhcpd. This keeps the systemd support
consistent with the SysV init script.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

---
Changes v2 -> v3: new patch.
---
 package/dhcp/dhcpd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
index 5989506..ad1300c 100644
--- a/package/dhcp/dhcpd.service
+++ b/package/dhcp/dhcpd.service
@@ -5,7 +5,7 @@ After=network.target
 [Service]
 Type=forking
 PIDFile=/run/dhcpd.pid
-ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
+ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $OPTIONS $INTERFACES
 KillSignal=SIGINT
 EnvironmentFile=/etc/default/dhcpd
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (2 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: " Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-12-24 14:02   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file Benoît Thébaudeau
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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.

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-server | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index c1ef53b..079727b 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (3 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-11-04 10:14   ` Maxime Hadjinlian
  2015-12-24 14:03   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text Benoît Thébaudeau
                   ` (7 subsequent siblings)
  12 siblings, 2 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

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.

Also, no extra options are required on the command line, which means
that the EnvironmentFile file does not have to be present, so make it
optional.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

---
Changes v2 -> v3: new patch.
---
 package/dhcp/dhcpd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
index ad1300c..98cc54b 100644
--- a/package/dhcp/dhcpd.service
+++ b/package/dhcp/dhcpd.service
@@ -7,7 +7,7 @@ Type=forking
 PIDFile=/run/dhcpd.pid
 ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $OPTIONS $INTERFACES
 KillSignal=SIGINT
-EnvironmentFile=/etc/default/dhcpd
+EnvironmentFile=-/etc/default/dhcpd
 
 [Install]
 WantedBy=multi-user.target
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (4 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-12-24 14:05   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart Benoît Thébaudeau
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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.

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

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

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 0f383e6..6493eda 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -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
 
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index 079727b..c56e7c3 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (5 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-12-24 14:06   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop Benoît Thébaudeau
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-relay  | 1 -
 package/dhcp/S80dhcp-server | 1 -
 2 files changed, 2 deletions(-)

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 6493eda..211431b 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -43,7 +43,6 @@ case "$1" in
 		;;
 	restart | force-reload)
 		$0 stop
-		sleep 2
 		$0 start
 		;;
 	*)
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index c56e7c3..dc9c433 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -33,7 +33,6 @@ case "$1" in
 		;;
 	restart | force-reload)
 		$0 stop
-		sleep 2
 		$0 start
 		if [ "$?" != "0" ]; then
 			exit 1
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (6 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-12-24 14:26   ` Thomas Petazzoni
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 10/13] package/dhcp/S80dhcp-server: support IPv6 lease file Benoît Thébaudeau
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-relay  | 12 ++++++++++--
 package/dhcp/S80dhcp-server | 12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 211431b..9b8d65f 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -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
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index dc9c433..1c2ff74 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 10/13] package/dhcp/S80dhcp-server: support IPv6 lease file
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (7 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: " Benoît Thébaudeau
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

The IPv6 lease file has a different name.

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

Changes v1 -> v2:
 - Rebase.
---
 package/dhcp/S80dhcp-server | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index 1c2ff74..b6f76ee 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -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"
 		;;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (8 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 10/13] package/dhcp/S80dhcp-server: support IPv6 lease file Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-11-04 10:21   ` Maxime Hadjinlian
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 12/13] package/dhcp: SysV init scripts: refactor using functions Benoît Thébaudeau
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

The IPv6 lease file has a different name.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

---
Changes v2 -> v3: new patch.
---
 package/dhcp/dhcp.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
index c05e694..3aa1577 100644
--- a/package/dhcp/dhcp.mk
+++ b/package/dhcp/dhcp.mk
@@ -83,6 +83,8 @@ define DHCP_INSTALL_INIT_SYSTEMD
 		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
 	echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
 		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
+	echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
+		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
 endef
 endif
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 12/13] package/dhcp: SysV init scripts: refactor using functions
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (9 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: " Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file Benoît Thébaudeau
  2015-12-24 13:59 ` [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Thomas Petazzoni
  12 siblings, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

From: Beno?t Th?baudeau <benoit@wsystem.com>

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

By the way, clean up these scripts.

Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

---
Changes v2 -> v3: none.

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

diff --git a/package/dhcp/S80dhcp-relay b/package/dhcp/S80dhcp-relay
index 9b8d65f..b48f72c 100755
--- a/package/dhcp/S80dhcp-relay
+++ b/package/dhcp/S80dhcp-relay
@@ -1,61 +1,68 @@
 #!/bin/sh
-#
-# $Id: dhcp3-relay,v 1.1 2004/04/16 15:41:08 ml Exp $
-#
 
-# What servers should the DHCP relay forward requests to?
-# e.g: SERVERS="192.168.0.1"
-SERVERS=""
+NAME="dhcrelay"
+DESC="DHCP relay"
+DAEMON="/usr/sbin/${NAME}"
 
-# On what interfaces should the DHCP relay (dhrelay) serve DHCP requests?
+# 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"
+SERVERS=""
+
 # 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
diff --git a/package/dhcp/S80dhcp-server b/package/dhcp/S80dhcp-server
index b6f76ee..607f1ca 100755
--- a/package/dhcp/S80dhcp-server
+++ b/package/dhcp/S80dhcp-server
@@ -1,57 +1,65 @@
 #!/bin/sh
-#
-# $Id: dhcp3-server.init.d,v 1.4 2003/07/13 19:12:41 mdz Exp $
-#
 
-# On what interfaces should the DHCP server (dhcpd) serve DHCP requests?
-#       Separate multiple interfaces with spaces, e.g. "eth0 eth1".
+NAME="dhcpd"
+DESC="DHCP server"
+DAEMON="/usr/sbin/${NAME}"
+DAEMON_CONF="/etc/dhcp/${NAME}.conf"
+DAEMON_LIB_DIR="/var/lib/dhcp"
+LEASE_FILES="${DAEMON_LIB_DIR}/${NAME}.leases ${DAEMON_LIB_DIR}/${NAME}6.leases"
+
+# On what interfaces should the DHCP server serve DHCP requests?
+# 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"
+CFG_FILE="/etc/default/${NAME}"
 [ -r "${CFG_FILE}" ] && . "${CFG_FILE}"
 
 # PID files generated by the daemon
-PID_FILES="/var/run/dhcpd.pid /var/run/dhcpd6.pid"
+PID_FILES="/var/run/${NAME}.pid /var/run/${NAME}6.pid"
 
 # Sanity checks
-test -f /usr/sbin/dhcpd || exit 0
-test -f /etc/dhcp/dhcpd.conf || exit 0
+test -f "${DAEMON}" || exit 0
+test -f "${DAEMON_CONF}" || exit 0
+
+start()
+{
+	printf "Starting ${DESC}: "
+	test -d "${DAEMON_LIB_DIR}" || mkdir -p "${DAEMON_LIB_DIR}"
+	for lease_file in ${LEASE_FILES}; do
+		test -f "${lease_file}" || touch "${lease_file}"
+	done
+	start-stop-daemon -S -q -x "${DAEMON}" -- -q $OPTIONS $INTERFACES &&
+			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 server: "
-		test -d /var/lib/dhcp/ || mkdir -p /var/lib/dhcp/
-		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"
+		start
 		;;
 	stop)
-		printf "Stopping DHCP server: "
-		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
+		stop
 		;;
-	restart | force-reload)
-		$0 stop
-		$0 start
-		if [ "$?" != "0" ]; then
-			exit 1
-		fi
+	restart|force-reload)
+		stop
+		start
 		;;
 	*)
 		echo "Usage: $0 {start|stop|restart|force-reload}"
 		exit 1
 esac
-
-exit 0
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (10 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 12/13] package/dhcp: SysV init scripts: refactor using functions Benoît Thébaudeau
@ 2015-10-25  0:59 ` Benoît Thébaudeau
  2015-11-04 11:24   ` Maxime Hadjinlian
  2015-12-24 13:59 ` [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Thomas Petazzoni
  12 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-10-25  0:59 UTC (permalink / raw)
  To: buildroot

Add a systemd service file to start dhcrelay.

The network interfaces to listen on must be set via the variable IFCMD
in /etc/default/dhcrelay, e.g.:

    IFCMD="-i eth0 -i eth1"

The "upstream" servers to pass the queries along to must be set via the
variable SERVERS in the same file, e.g.:

    SERVERS="server0 server1"

Extra command line options can be set via the variable OPTIONS in the
same file.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

---
Changes v2 -> v3: new patch.
---
 package/dhcp/dhcp.mk          | 45 ++++++++++++++++++++++++++++---------------
 package/dhcp/dhcrelay.service | 13 +++++++++++++
 2 files changed, 42 insertions(+), 16 deletions(-)
 create mode 100644 package/dhcp/dhcrelay.service

diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
index 3aa1577..4ba302b 100644
--- a/package/dhcp/dhcp.mk
+++ b/package/dhcp/dhcp.mk
@@ -37,6 +37,23 @@ define DHCP_INSTALL_SERVER
 	$(INSTALL) -m 0644 -D package/dhcp/dhcpd.conf \
 		$(TARGET_DIR)/etc/dhcp/dhcpd.conf
 endef
+
+define DHCP_INSTALL_SERVER_INIT_SYSTEMD
+	$(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
+		$(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
+
+	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
+
+	ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
+		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
+
+	echo "d /var/lib/dhcp 0755 - - - -" > \
+		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
+	echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
+		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
+	echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
+		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
+endef
 endif
 
 ifeq ($(BR2_PACKAGE_DHCP_RELAY),y)
@@ -46,6 +63,16 @@ define DHCP_INSTALL_RELAY
 	$(INSTALL) -m 0755 -D $(DHCP_DIR)/relay/dhcrelay \
 		$(TARGET_DIR)/usr/sbin/dhcrelay
 endef
+
+define DHCP_INSTALL_RELAY_INIT_SYSTEMD
+	$(INSTALL) -D -m 644 package/dhcp/dhcrelay.service \
+		$(TARGET_DIR)/usr/lib/systemd/system/dhcrelay.service
+
+	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
+
+	ln -sf ../../../../usr/lib/systemd/system/dhcrelay.service \
+		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcrelay.service
+endef
 endif
 
 ifeq ($(BR2_PACKAGE_DHCP_CLIENT),y)
@@ -69,24 +96,10 @@ define DHCP_INSTALL_INIT_SYSV
 		$(TARGET_DIR)/etc/init.d/S80dhcp-relay
 endef
 
-ifeq ($(BR2_PACKAGE_DHCP_SERVER),y)
 define DHCP_INSTALL_INIT_SYSTEMD
-	$(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
-		$(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
-
-	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
-
-	ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
-		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
-
-	echo "d /var/lib/dhcp 0755 - - - -" > \
-		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
-	echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
-		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
-	echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
-		$(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
+	$(DHCP_INSTALL_SERVER_INIT_SYSTEMD)
+	$(DHCP_INSTALL_RELAY_INIT_SYSTEMD)
 endef
-endif
 
 define DHCP_INSTALL_TARGET_CMDS
 	$(DHCP_INSTALL_RELAY)
diff --git a/package/dhcp/dhcrelay.service b/package/dhcp/dhcrelay.service
new file mode 100644
index 0000000..5a1410e
--- /dev/null
+++ b/package/dhcp/dhcrelay.service
@@ -0,0 +1,13 @@
+[Unit]
+Description=DHCP relay
+After=network.target
+
+[Service]
+Type=forking
+PIDFile=/run/dhcrelay.pid
+ExecStart=/usr/sbin/dhcrelay -q -pf /run/dhcrelay.pid $OPTIONS $IFCMD $SERVERS
+KillSignal=SIGINT
+EnvironmentFile=/etc/default/dhcrelay
+
+[Install]
+WantedBy=multi-user.target
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
@ 2015-11-04  9:52   ` Maxime Hadjinlian
  2015-11-04  9:54     ` Maxime Hadjinlian
  2015-11-04 10:18     ` Thomas Petazzoni
  2015-12-24 14:00   ` Thomas Petazzoni
  1 sibling, 2 replies; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04  9:52 UTC (permalink / raw)
  To: buildroot

Hi Benoit, all

On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
benoit.thebaudeau.dev@gmail.com> wrote:

> Use the same EnvironmentFile name as the SysV init script for
> consistency. The filenames under /etc/default/ are usually just the
> package/daemon/service/feature name without any extension, so remove the
> ".conf" extension from EnvironmentFile.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcpd.service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> index 7b265cb..5989506 100644
> --- a/package/dhcp/dhcpd.service
> +++ b/package/dhcp/dhcpd.service
> @@ -7,7 +7,7 @@ Type=forking
>  PIDFile=/run/dhcpd.pid
>  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>  KillSignal=SIGINT
> -EnvironmentFile=/etc/default/dhcpd.conf
> +EnvironmentFile=/etc/default/dhcpd
>
Maybe this should be:
EnvironmentFile-=/etc/default/dhcpd

Notice the '-' before the '=', in case the file doesn't exists, it won't
print a warning or error the whole service.

When I build dhcp, I did not find a /etc/default folder in my target
directory, am I missing something there ?

>
>  [Install]
>  WantedBy=multi-user.target
> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/2651351d/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04  9:52   ` Maxime Hadjinlian
@ 2015-11-04  9:54     ` Maxime Hadjinlian
  2015-11-04 10:01       ` Benoît Thébaudeau
  2015-11-04 10:18     ` Thomas Petazzoni
  1 sibling, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04  9:54 UTC (permalink / raw)
  To: buildroot

On Wed, Nov 4, 2015 at 10:52 AM, Maxime Hadjinlian <
maxime.hadjinlian@gmail.com> wrote:

> Hi Benoit, all
>
> On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
> benoit.thebaudeau.dev at gmail.com> wrote:
>
>> Use the same EnvironmentFile name as the SysV init script for
>> consistency. The filenames under /etc/default/ are usually just the
>> package/daemon/service/feature name without any extension, so remove the
>> ".conf" extension from EnvironmentFile.
>>
>> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>>
>> ---
>> Changes v2 -> v3: new patch.
>> ---
>>  package/dhcp/dhcpd.service | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
>> index 7b265cb..5989506 100644
>> --- a/package/dhcp/dhcpd.service
>> +++ b/package/dhcp/dhcpd.service
>> @@ -7,7 +7,7 @@ Type=forking
>>  PIDFile=/run/dhcpd.pid
>>  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>>  KillSignal=SIGINT
>> -EnvironmentFile=/etc/default/dhcpd.conf
>> +EnvironmentFile=/etc/default/dhcpd
>>
> Maybe this should be:
> EnvironmentFile-=/etc/default/dhcpd
>
> Notice the '-' before the '=', in case the file doesn't exists, it won't
> print a warning or error the whole service.
>
> My mistake, the '-' should be *AFTER* the '='.


> When I build dhcp, I did not find a /etc/default folder in my target
> directory, am I missing something there ?
>
>>
>>  [Install]
>>  WantedBy=multi-user.target
>> --
>> 2.1.4
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/1452c283/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04  9:54     ` Maxime Hadjinlian
@ 2015-11-04 10:01       ` Benoît Thébaudeau
  2015-11-04 10:04         ` Maxime Hadjinlian
  0 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 10:01 UTC (permalink / raw)
  To: buildroot

Hi Maxime, all,

On 04/11/2015 10:54, Maxime Hadjinlian wrote:
> 
> 
> On Wed, Nov 4, 2015 at 10:52 AM, Maxime Hadjinlian <maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com>> wrote:
> 
>     Hi Benoit, all
> 
>     On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>> wrote:
> 
>         Use the same EnvironmentFile name as the SysV init script for
>         consistency. The filenames under /etc/default/ are usually just the
>         package/daemon/service/feature name without any extension, so remove the
>         ".conf" extension from EnvironmentFile.
> 
>         Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> 
>         ---
>         Changes v2 -> v3: new patch.
>         ---
>          package/dhcp/dhcpd.service | 2 +-
>          1 file changed, 1 insertion(+), 1 deletion(-)
> 
>         diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
>         index 7b265cb..5989506 100644
>         --- a/package/dhcp/dhcpd.service
>         +++ b/package/dhcp/dhcpd.service
>         @@ -7,7 +7,7 @@ Type=forking
>          PIDFile=/run/dhcpd.pid
>          ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>          KillSignal=SIGINT
>         -EnvironmentFile=/etc/default/dhcpd.conf
>         +EnvironmentFile=/etc/default/dhcpd
> 
>     Maybe this should be:
>     EnvironmentFile-=/etc/default/dhcpd
> 
>     Notice the '-' before the '=', in case the file doesn't exists, it won't print a warning or error the whole service.
> 
> My mistake, the '-' should be *AFTER* the '='.

See 06/13.

>     When I build dhcp, I did not find a /etc/default folder in my target directory, am I missing something there ?

You might have this question about 13/13 which requires a file in /etc/default,
but not here because of 06/13. But a default file provided by Buildroot would
not make sense for 13/13 since the file contents would be application-specific
though required. In this case, it is up to users to add a rootfs overlay with
such files.

>          [Install]
>          WantedBy=multi-user.target
>         --
>         2.1.4

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:01       ` Benoît Thébaudeau
@ 2015-11-04 10:04         ` Maxime Hadjinlian
  2015-11-04 10:07           ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:04 UTC (permalink / raw)
  To: buildroot

On Wed, Nov 4, 2015 at 11:01 AM, Beno?t Th?baudeau <benoit@wsystem.com>
wrote:

> Hi Maxime, all,
>
> On 04/11/2015 10:54, Maxime Hadjinlian wrote:
> >
> >
> > On Wed, Nov 4, 2015 at 10:52 AM, Maxime Hadjinlian <
> maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com>> wrote:
> >
> >     Hi Benoit, all
> >
> >     On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
> benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> wrote:
> >
> >         Use the same EnvironmentFile name as the SysV init script for
> >         consistency. The filenames under /etc/default/ are usually just
> the
> >         package/daemon/service/feature name without any extension, so
> remove the
> >         ".conf" extension from EnvironmentFile.
> >
> >         Signed-off-by: Beno?t Th?baudeau <
> benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> >
> >         ---
> >         Changes v2 -> v3: new patch.
> >         ---
> >          package/dhcp/dhcpd.service | 2 +-
> >          1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >         diff --git a/package/dhcp/dhcpd.service
> b/package/dhcp/dhcpd.service
> >         index 7b265cb..5989506 100644
> >         --- a/package/dhcp/dhcpd.service
> >         +++ b/package/dhcp/dhcpd.service
> >         @@ -7,7 +7,7 @@ Type=forking
> >          PIDFile=/run/dhcpd.pid
> >          ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
> >          KillSignal=SIGINT
> >         -EnvironmentFile=/etc/default/dhcpd.conf
> >         +EnvironmentFile=/etc/default/dhcpd
> >
> >     Maybe this should be:
> >     EnvironmentFile-=/etc/default/dhcpd
> >
> >     Notice the '-' before the '=', in case the file doesn't exists, it
> won't print a warning or error the whole service.
> >
> > My mistake, the '-' should be *AFTER* the '='.
>
> See 06/13.
>
Ah yes, should they be squashed ?

>
> >     When I build dhcp, I did not find a /etc/default folder in my target
> directory, am I missing something there ?
>
> You might have this question about 13/13 which requires a file in
> /etc/default,
> but not here because of 06/13. But a default file provided by Buildroot
> would
> not make sense for 13/13 since the file contents would be
> application-specific
> though required. In this case, it is up to users to add a rootfs overlay
> with
> such files.
>
Ok, that's fair enough.

>
> >          [Install]
> >          WantedBy=multi-user.target
> >         --
> >         2.1.4
>
> Best regards,
> Beno?t
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/5f236a75/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:04         ` Maxime Hadjinlian
@ 2015-11-04 10:07           ` Benoît Thébaudeau
  2015-11-04 10:13             ` Maxime Hadjinlian
  0 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 10:07 UTC (permalink / raw)
  To: buildroot

On 04/11/2015 11:04, Maxime Hadjinlian wrote:
> 
> 
> On Wed, Nov 4, 2015 at 11:01 AM, Beno?t Th?baudeau <benoit at wsystem.com <mailto:benoit@wsystem.com>> wrote:
> 
>     Hi Maxime, all,
> 
>     On 04/11/2015 10:54, Maxime Hadjinlian wrote:
>     >
>     >
>     > On Wed, Nov 4, 2015 at 10:52 AM, Maxime Hadjinlian <maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com> <mailto:maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com>>> wrote:
>     >
>     >     Hi Benoit, all
>     >
>     >     On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com> <mailto:benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>> wrote:
>     >
>     >         Use the same EnvironmentFile name as the SysV init script for
>     >         consistency. The filenames under /etc/default/ are usually just the
>     >         package/daemon/service/feature name without any extension, so remove the
>     >         ".conf" extension from EnvironmentFile.
>     >
>     >         Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com> <mailto:benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>>
>     >
>     >         ---
>     >         Changes v2 -> v3: new patch.
>     >         ---
>     >          package/dhcp/dhcpd.service | 2 +-
>     >          1 file changed, 1 insertion(+), 1 deletion(-)
>     >
>     >         diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
>     >         index 7b265cb..5989506 100644
>     >         --- a/package/dhcp/dhcpd.service
>     >         +++ b/package/dhcp/dhcpd.service
>     >         @@ -7,7 +7,7 @@ Type=forking
>     >          PIDFile=/run/dhcpd.pid
>     >          ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>     >          KillSignal=SIGINT
>     >         -EnvironmentFile=/etc/default/dhcpd.conf
>     >         +EnvironmentFile=/etc/default/dhcpd
>     >
>     >     Maybe this should be:
>     >     EnvironmentFile-=/etc/default/dhcpd
>     >
>     >     Notice the '-' before the '=', in case the file doesn't exists, it won't print a warning or error the whole service.
>     >
>     > My mistake, the '-' should be *AFTER* the '='.
> 
>     See 06/13.
> 
> Ah yes, should they be squashed ?

No: these patches have different purposes.

>     >     When I build dhcp, I did not find a /etc/default folder in my target directory, am I missing something there ?
> 
>     You might have this question about 13/13 which requires a file in /etc/default,
>     but not here because of 06/13. But a default file provided by Buildroot would
>     not make sense for 13/13 since the file contents would be application-specific
>     though required. In this case, it is up to users to add a rootfs overlay with
>     such files.
> 
> Ok, that's fair enough.
> 
> 
>     >          [Install]
>     >          WantedBy=multi-user.target
>     >         --
>     >         2.1.4

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: support extra options
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: " Benoît Thébaudeau
@ 2015-11-04 10:09   ` Maxime Hadjinlian
  2015-12-24 14:01   ` Thomas Petazzoni
  1 sibling, 0 replies; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:09 UTC (permalink / raw)
  To: buildroot

On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
benoit.thebaudeau.dev@gmail.com> wrote:

> Add an OPTIONS configuration variable in order to make it possible to
> pass custom extra options to dhcpd. This keeps the systemd support
> consistent with the SysV init script.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcpd.service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> index 5989506..ad1300c 100644
> --- a/package/dhcp/dhcpd.service
> +++ b/package/dhcp/dhcpd.service
> @@ -5,7 +5,7 @@ After=network.target
>  [Service]
>  Type=forking
>  PIDFile=/run/dhcpd.pid
> -ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
> +ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $OPTIONS $INTERFACES
>  KillSignal=SIGINT
>  EnvironmentFile=/etc/default/dhcpd
>

Reviewed-by:  "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>

>
> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/104b6aa5/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:07           ` Benoît Thébaudeau
@ 2015-11-04 10:13             ` Maxime Hadjinlian
  0 siblings, 0 replies; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:13 UTC (permalink / raw)
  To: buildroot

[-SNIP-]

> >     >     Maybe this should be:
> >     >     EnvironmentFile-=/etc/default/dhcpd
> >     >
> >     >     Notice the '-' before the '=', in case the file doesn't
> exists, it won't print a warning or error the whole service.
> >     >
> >     > My mistake, the '-' should be *AFTER* the '='.
> >
> >     See 06/13.
> >
> > Ah yes, should they be squashed ?
>
> No: these patches have different purposes.
>
Yes, on that bombshell

Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>
[-SNIP-]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/07dd322d/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file Benoît Thébaudeau
@ 2015-11-04 10:14   ` Maxime Hadjinlian
  2015-12-24 14:03   ` Thomas Petazzoni
  1 sibling, 0 replies; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:14 UTC (permalink / raw)
  To: buildroot

On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
benoit.thebaudeau.dev@gmail.com> wrote:

> 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.
>
> Also, no extra options are required on the command line, which means
> that the EnvironmentFile file does not have to be present, so make it
> optional.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcpd.service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> index ad1300c..98cc54b 100644
> --- a/package/dhcp/dhcpd.service
> +++ b/package/dhcp/dhcpd.service
> @@ -7,7 +7,7 @@ Type=forking
>  PIDFile=/run/dhcpd.pid
>  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $OPTIONS $INTERFACES
>  KillSignal=SIGINT
> -EnvironmentFile=/etc/default/dhcpd
> +EnvironmentFile=-/etc/default/dhcpd
>
>  [Install]
>  WantedBy=multi-user.target
>
Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>

> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/c3ae58c8/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04  9:52   ` Maxime Hadjinlian
  2015-11-04  9:54     ` Maxime Hadjinlian
@ 2015-11-04 10:18     ` Thomas Petazzoni
  2015-11-04 10:24       ` Benoît Thébaudeau
                         ` (2 more replies)
  1 sibling, 3 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-11-04 10:18 UTC (permalink / raw)
  To: buildroot

Maxime, Beno?t, Gabe,

On Wed, 4 Nov 2015 10:52:10 +0100, Maxime Hadjinlian wrote:

> > diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> > index 7b265cb..5989506 100644
> > --- a/package/dhcp/dhcpd.service
> > +++ b/package/dhcp/dhcpd.service
> > @@ -7,7 +7,7 @@ Type=forking
> >  PIDFile=/run/dhcpd.pid
> >  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
> >  KillSignal=SIGINT
> > -EnvironmentFile=/etc/default/dhcpd.conf
> > +EnvironmentFile=/etc/default/dhcpd
> >
> Maybe this should be:
> EnvironmentFile-=/etc/default/dhcpd

On the dropbear package, Gabe Evans said it may not be such a good idea
to do this EnvironmentFile thing.

Can we settle on a common decision ?

(From my PoV, using this EnvironmentFile looked good as it would be a
common mechanism between the systemd and Busybox/sysvinit cases to tune
the configuration of services. But Gabe seemed to disagree.)

Could systemd people in Buildroot decide what to do ? :-)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: " Benoît Thébaudeau
@ 2015-11-04 10:21   ` Maxime Hadjinlian
  2015-11-04 10:36     ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:21 UTC (permalink / raw)
  To: buildroot

Hi Benoit, all

On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
benoit.thebaudeau.dev@gmail.com> wrote:

> The IPv6 lease file has a different name.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcp.mk | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
> index c05e694..3aa1577 100644
> --- a/package/dhcp/dhcp.mk
> +++ b/package/dhcp/dhcp.mk
> @@ -83,6 +83,8 @@ define DHCP_INSTALL_INIT_SYSTEMD
>                 $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>         echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
>                 $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
> +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>  endef
>  endif
>
> Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>

A question though, from the manual page of dhcpd, by default it starts as
an IPv6 server should we have, two services one with '-4' and one with '-6'
? And it's left to the user to specify it in the "$OPTIONS" ? Maybe this
should be mentioned somewhere ?

> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/9be81378/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:18     ` Thomas Petazzoni
@ 2015-11-04 10:24       ` Benoît Thébaudeau
  2015-11-04 10:25       ` Maxime Hadjinlian
  2015-11-04 18:42       ` Gabe Evans
  2 siblings, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 10:24 UTC (permalink / raw)
  To: buildroot

Thomas, Gabe, Maxime, all,

On 04/11/2015 11:18, Thomas Petazzoni wrote:
> On Wed, 4 Nov 2015 10:52:10 +0100, Maxime Hadjinlian wrote:
> 
>>> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
>>> index 7b265cb..5989506 100644
>>> --- a/package/dhcp/dhcpd.service
>>> +++ b/package/dhcp/dhcpd.service
>>> @@ -7,7 +7,7 @@ Type=forking
>>>  PIDFile=/run/dhcpd.pid
>>>  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>>>  KillSignal=SIGINT
>>> -EnvironmentFile=/etc/default/dhcpd.conf
>>> +EnvironmentFile=/etc/default/dhcpd
>>>
>> Maybe this should be:
>> EnvironmentFile-=/etc/default/dhcpd
> 
> On the dropbear package, Gabe Evans said it may not be such a good idea
> to do this EnvironmentFile thing.

Why? What was the rationale? What would be the alternatives?

> Can we settle on a common decision ?
> 
> (From my PoV, using this EnvironmentFile looked good as it would be a
> common mechanism between the systemd and Busybox/sysvinit cases to tune
> the configuration of services. But Gabe seemed to disagree.)
> 
> Could systemd people in Buildroot decide what to do ? :-)

Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:18     ` Thomas Petazzoni
  2015-11-04 10:24       ` Benoît Thébaudeau
@ 2015-11-04 10:25       ` Maxime Hadjinlian
  2015-11-04 18:42       ` Gabe Evans
  2 siblings, 0 replies; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:25 UTC (permalink / raw)
  To: buildroot

Hi Thomas, all

On Wed, Nov 4, 2015 at 11:18 AM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Maxime, Beno?t, Gabe,
>
> On Wed, 4 Nov 2015 10:52:10 +0100, Maxime Hadjinlian wrote:
>
> > > diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> > > index 7b265cb..5989506 100644
> > > --- a/package/dhcp/dhcpd.service
> > > +++ b/package/dhcp/dhcpd.service
> > > @@ -7,7 +7,7 @@ Type=forking
> > >  PIDFile=/run/dhcpd.pid
> > >  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
> > >  KillSignal=SIGINT
> > > -EnvironmentFile=/etc/default/dhcpd.conf
> > > +EnvironmentFile=/etc/default/dhcpd
> > >
> > Maybe this should be:
> > EnvironmentFile-=/etc/default/dhcpd
>
> On the dropbear package, Gabe Evans said it may not be such a good idea
> to do this EnvironmentFile thing.
>
> Can we settle on a common decision ?
>
> (From my PoV, using this EnvironmentFile looked good as it would be a
> common mechanism between the systemd and Busybox/sysvinit cases to tune
> the configuration of services. But Gabe seemed to disagree.)
>
I still have to read that thread, I'll get to it in a few minutes.
The mechanism in itself is good, adding the '-' simply makes the file
optional. For me, a program must be able to start without options, or they
are not options but mandatory parameters (which is a bit contradictory)

>
> Could systemd people in Buildroot decide what to do ? :-)
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/e0fc767a/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-04 10:21   ` Maxime Hadjinlian
@ 2015-11-04 10:36     ` Benoît Thébaudeau
  2015-11-04 10:53       ` Maxime Hadjinlian
  0 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 10:36 UTC (permalink / raw)
  To: buildroot

Maxime, all,

On 04/11/2015 11:21, Maxime Hadjinlian wrote:
> Hi Benoit, all
> 
> On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>> wrote:
> 
>     The IPv6 lease file has a different name.
> 
>     Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> 
>     ---
>     Changes v2 -> v3: new patch.
>     ---
>      package/dhcp/dhcp.mk <http://dhcp.mk> | 2 ++
>      1 file changed, 2 insertions(+)
> 
>     diff --git a/package/dhcp/dhcp.mk <http://dhcp.mk> b/package/dhcp/dhcp.mk <http://dhcp.mk>
>     index c05e694..3aa1577 100644
>     --- a/package/dhcp/dhcp.mk <http://dhcp.mk>
>     +++ b/package/dhcp/dhcp.mk <http://dhcp.mk>
>     @@ -83,6 +83,8 @@ define DHCP_INSTALL_INIT_SYSTEMD
>                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>             echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
>                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
>     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>      endef
>      endif
> 
> Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com>>
> 
> A question though, from the manual page of dhcpd, by default it starts as an IPv6 server should we have, two services one with '-4' and one with '-6' ? And it's left to the user to specify it in the "$OPTIONS" ? Maybe this should be mentioned somewhere ?

'-4' is the default, not '-6'. A single instance of dhcpd cannot be run with
both '-4' and '-6'. I don't know if it's possible (or desirable) to safely run
simultaneously two instances of dhcpd, one with '-4' and the other one with
'-6'.

"$OPTIONS" is designed exactly for such things. Users should read the man page
of dhcpd to know what may fit in there.

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-04 10:36     ` Benoît Thébaudeau
@ 2015-11-04 10:53       ` Maxime Hadjinlian
  2015-11-04 11:15         ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 10:53 UTC (permalink / raw)
  To: buildroot

On Wed, Nov 4, 2015 at 11:36 AM, Beno?t Th?baudeau <benoit@wsystem.com>
wrote:

> Maxime, all,
>
> On 04/11/2015 11:21, Maxime Hadjinlian wrote:
> > Hi Benoit, all
> >
> > On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
> benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> wrote:
> >
> >     The IPv6 lease file has a different name.
> >
> >     Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com
> <mailto:benoit.thebaudeau.dev@gmail.com>>
> >
> >     ---
> >     Changes v2 -> v3: new patch.
> >     ---
> >      package/dhcp/dhcp.mk <http://dhcp.mk> | 2 ++
> >      1 file changed, 2 insertions(+)
> >
> >     diff --git a/package/dhcp/dhcp.mk <http://dhcp.mk> b/package/dhcp/
> dhcp.mk <http://dhcp.mk>
> >     index c05e694..3aa1577 100644
> >     --- a/package/dhcp/dhcp.mk <http://dhcp.mk>
> >     +++ b/package/dhcp/dhcp.mk <http://dhcp.mk>
> >     @@ -83,6 +83,8 @@ define DHCP_INSTALL_INIT_SYSTEMD
> >                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> >             echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
> >                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> >     +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
> >     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> >      endef
> >      endif
> >
> > Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com <mailto:
> maxime.hadjinlian at gmail.com>>
> >
> > A question though, from the manual page of dhcpd, by default it starts
> as an IPv6 server should we have, two services one with '-4' and one with
> '-6' ? And it's left to the user to specify it in the "$OPTIONS" ? Maybe
> this should be mentioned somewhere ?
>
> '-4' is the default, not '-6'.

If I look at ./server/dhcpd.8 in the sources of dhcp-4.1-ESVR12, it clearly
states:

-6 Run as a DHCPv6 server. This is the default and cannot be combined with
-4.

A single instance of dhcpd cannot be run with
> both '-4' and '-6'. I don't know if it's possible (or desirable) to safely
> run
> simultaneously two instances of dhcpd, one with '-4' and the other one with
> '-6'.
>
Well it could be useful if you are trying to build a network appliance and
you want to support both stack. So you would want to start two processes
with different options. Maybe that's something that should be left to the
users to figure out.

>
> "$OPTIONS" is designed exactly for such things. Users should read the man
> page
> of dhcpd to know what may fit in there.
>
I understood what was "$OPTIONS" about, I just worry that it's a bit
"hidden" for the users, but maybe they should know what they are doing.

>
> Best regards,
> Beno?t
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/e8920b7b/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-04 10:53       ` Maxime Hadjinlian
@ 2015-11-04 11:15         ` Benoît Thébaudeau
  2015-11-04 11:31           ` Maxime Hadjinlian
  0 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 11:15 UTC (permalink / raw)
  To: buildroot

On 04/11/2015 11:53, Maxime Hadjinlian wrote:
> 
> 
> On Wed, Nov 4, 2015 at 11:36 AM, Beno?t Th?baudeau <benoit at wsystem.com <mailto:benoit@wsystem.com>> wrote:
> 
>     Maxime, all,
> 
>     On 04/11/2015 11:21, Maxime Hadjinlian wrote:
>     > Hi Benoit, all
>     >
>     > On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com> <mailto:benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>> wrote:
>     >
>     >     The IPv6 lease file has a different name.
>     >
>     >     Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com> <mailto:benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>>
>     >
>     >     ---
>     >     Changes v2 -> v3: new patch.
>     >     ---
>     >      package/dhcp/dhcp.mk <http://dhcp.mk> <http://dhcp.mk> | 2 ++
>     >      1 file changed, 2 insertions(+)
>     >
>     >     diff --git a/package/dhcp/dhcp.mk <http://dhcp.mk> <http://dhcp.mk> b/package/dhcp/dhcp.mk <http://dhcp.mk> <http://dhcp.mk>
>     >     index c05e694..3aa1577 100644
>     >     --- a/package/dhcp/dhcp.mk <http://dhcp.mk> <http://dhcp.mk>
>     >     +++ b/package/dhcp/dhcp.mk <http://dhcp.mk> <http://dhcp.mk>
>     >     @@ -83,6 +83,8 @@ define DHCP_INSTALL_INIT_SYSTEMD
>     >                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     >             echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
>     >                     $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     >     +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
>     >     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     >      endef
>     >      endif
>     >
>     > Reviewed-by: "Maxime Hadjinlian" <maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com> <mailto:maxime.hadjinlian at gmail.com <mailto:maxime.hadjinlian@gmail.com>>>
>     >
>     > A question though, from the manual page of dhcpd, by default it starts as an IPv6 server should we have, two services one with '-4' and one with '-6' ? And it's left to the user to specify it in the "$OPTIONS" ? Maybe this should be mentioned somewhere ?
> 
>     '-4' is the default, not '-6'. 
> 
> If I look at ./server/dhcpd.8 in the sources of dhcp-4.1-ESVR12, it clearly states:
> 
> -6 Run as a DHCPv6 server. This is the default and cannot be combined with -4. 

My bad: I had looked at the man page of another version.

>     A single instance of dhcpd cannot be run with
>     both '-4' and '-6'. I don't know if it's possible (or desirable) to safely run
>     simultaneously two instances of dhcpd, one with '-4' and the other one with
>     '-6'.
> 
> Well it could be useful if you are trying to build a network appliance and you want to support both stack. So you would want to start two processes with different options. Maybe that's something that should be left to the users to figure out.

Yes, but should everything be duplicated in Buildroot just for this purpose? For
SysV, it's easy not to duplicate things, but how would you do that with systemd?
How do distributions handle this?

>     "$OPTIONS" is designed exactly for such things. Users should read the man page
>     of dhcpd to know what may fit in there.
> 
> I understood what was "$OPTIONS" about, I just worry that it's a bit "hidden" for the users, but maybe they should know what they are doing. 

It's not possible to document everything, but there are comments about the
meaning of these variables in the SysV init scripts, so I can add such comments
to the .service files too.

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file Benoît Thébaudeau
@ 2015-11-04 11:24   ` Maxime Hadjinlian
  2015-11-04 13:35     ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 11:24 UTC (permalink / raw)
  To: buildroot

Hi Benoit, all

On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <
benoit.thebaudeau.dev@gmail.com> wrote:

> Add a systemd service file to start dhcrelay.
>
> The network interfaces to listen on must be set via the variable IFCMD
> in /etc/default/dhcrelay, e.g.:
>
>     IFCMD="-i eth0 -i eth1"
>
> The "upstream" servers to pass the queries along to must be set via the
> variable SERVERS in the same file, e.g.:
>
>     SERVERS="server0 server1"
>
> Extra command line options can be set via the variable OPTIONS in the
> same file.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcp.mk          | 45
> ++++++++++++++++++++++++++++---------------
>  package/dhcp/dhcrelay.service | 13 +++++++++++++
>  2 files changed, 42 insertions(+), 16 deletions(-)
>  create mode 100644 package/dhcp/dhcrelay.service
>
> diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
> index 3aa1577..4ba302b 100644
> --- a/package/dhcp/dhcp.mk
> +++ b/package/dhcp/dhcp.mk
> @@ -37,6 +37,23 @@ define DHCP_INSTALL_SERVER
>         $(INSTALL) -m 0644 -D package/dhcp/dhcpd.conf \
>                 $(TARGET_DIR)/etc/dhcp/dhcpd.conf
>  endef
> +
> +define DHCP_INSTALL_SERVER_INIT_SYSTEMD
> +       $(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
> +               $(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
> +
> +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
> +
> +       ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
> +
>  $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
> +
> +       echo "d /var/lib/dhcp 0755 - - - -" > \
> +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> +       echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
> +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
> +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> +endef
>  endif
>
>  ifeq ($(BR2_PACKAGE_DHCP_RELAY),y)
> @@ -46,6 +63,16 @@ define DHCP_INSTALL_RELAY
>         $(INSTALL) -m 0755 -D $(DHCP_DIR)/relay/dhcrelay \
>                 $(TARGET_DIR)/usr/sbin/dhcrelay
>  endef
> +
> +define DHCP_INSTALL_RELAY_INIT_SYSTEMD
> +       $(INSTALL) -D -m 644 package/dhcp/dhcrelay.service \
> +               $(TARGET_DIR)/usr/lib/systemd/system/dhcrelay.service
> +
> +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
> +
> +       ln -sf ../../../../usr/lib/systemd/system/dhcrelay.service \
> +
>  $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcrelay.service
> +endef
>  endif
>
>  ifeq ($(BR2_PACKAGE_DHCP_CLIENT),y)
> @@ -69,24 +96,10 @@ define DHCP_INSTALL_INIT_SYSV
>                 $(TARGET_DIR)/etc/init.d/S80dhcp-relay
>  endef
>
> -ifeq ($(BR2_PACKAGE_DHCP_SERVER),y)
>  define DHCP_INSTALL_INIT_SYSTEMD
> -       $(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
> -               $(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
> -
> -       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
> -
> -       ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
> -
>  $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
> -
> -       echo "d /var/lib/dhcp 0755 - - - -" > \
> -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> -       echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
> -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> -       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
> -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
> +       $(DHCP_INSTALL_SERVER_INIT_SYSTEMD)
> +       $(DHCP_INSTALL_RELAY_INIT_SYSTEMD)
>  endef
> -endif
>
>  define DHCP_INSTALL_TARGET_CMDS
>         $(DHCP_INSTALL_RELAY)
> diff --git a/package/dhcp/dhcrelay.service b/package/dhcp/dhcrelay.service
> new file mode 100644
> index 0000000..5a1410e
> --- /dev/null
> +++ b/package/dhcp/dhcrelay.service
> @@ -0,0 +1,13 @@
> +[Unit]
> +Description=DHCP relay
> +After=network.target
> +
> +[Service]
> +Type=forking
> +PIDFile=/run/dhcrelay.pid
> +ExecStart=/usr/sbin/dhcrelay -q -pf /run/dhcrelay.pid $OPTIONS $IFCMD
> $SERVERS
>
Why don't we want /var/run/dhcrelay.pid which is the default ?

> +KillSignal=SIGINT
>
I did not try to run it, but it doesn't react to SIGTERM ? I did not see
anything in the code to prevent it but I only looked at it pretty quickly.

> +EnvironmentFile=/etc/default/dhcrelay
>
Should be 'EnvironmentFile=-/etc/default/dhcrelay'

> +
> +[Install]
> +WantedBy=multi-user.target
> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/23c97039/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-04 11:15         ` Benoît Thébaudeau
@ 2015-11-04 11:31           ` Maxime Hadjinlian
  2015-11-09 10:49             ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-04 11:31 UTC (permalink / raw)
  To: buildroot

[-SNIP-]

>
> >     A single instance of dhcpd cannot be run with
> >     both '-4' and '-6'. I don't know if it's possible (or desirable) to
> safely run
> >     simultaneously two instances of dhcpd, one with '-4' and the other
> one with
> >     '-6'.
> >
> > Well it could be useful if you are trying to build a network appliance
> and you want to support both stack. So you would want to start two
> processes with different options. Maybe that's something that should be
> left to the users to figure out.
>
> Yes, but should everything be duplicated in Buildroot just for this
> purpose? For
> SysV, it's easy not to duplicate things, but how would you do that with
> systemd?
> How do distributions handle this?
>
I only looked at Arch Linux, they duplicate the service file. It's also
possible to use the template mechanisms in systemd, to have something like
dhcpd at .service and when you enable it, you create a symlink called
dhcpd at 4.service, and the '4' is passed as argument to the template and you
can use it however you want it to.

>
> >     "$OPTIONS" is designed exactly for such things. Users should read
> the man page
> >     of dhcpd to know what may fit in there.
> >
> > I understood what was "$OPTIONS" about, I just worry that it's a bit
> "hidden" for the users, but maybe they should know what they are doing.
>
> It's not possible to document everything, but there are comments about the
> meaning of these variables in the SysV init scripts, so I can add such
> comments
> to the .service files too.
>
Maybe that would be a good thing, I don't have a clear opinion on this, do
other want to weigh in ?

>
> Best regards,
> Beno?t
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/d07e34d5/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file
  2015-11-04 11:24   ` Maxime Hadjinlian
@ 2015-11-04 13:35     ` Benoît Thébaudeau
  2015-11-04 21:26       ` Arnout Vandecappelle
  2015-11-15 17:36       ` Benoît Thébaudeau
  0 siblings, 2 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-04 13:35 UTC (permalink / raw)
  To: buildroot

Hi Maxime, all,

On 04/11/2015 12:24, Maxime Hadjinlian wrote:
> On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>> wrote:
> 
>     Add a systemd service file to start dhcrelay.
> 
>     The network interfaces to listen on must be set via the variable IFCMD
>     in /etc/default/dhcrelay, e.g.:
> 
>         IFCMD="-i eth0 -i eth1"
> 
>     The "upstream" servers to pass the queries along to must be set via the
>     variable SERVERS in the same file, e.g.:
> 
>         SERVERS="server0 server1"
> 
>     Extra command line options can be set via the variable OPTIONS in the
>     same file.
> 
>     Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>>
> 
>     ---
>     Changes v2 -> v3: new patch.
>     ---
>      package/dhcp/dhcp.mk <http://dhcp.mk>          | 45 ++++++++++++++++++++++++++++---------------
>      package/dhcp/dhcrelay.service | 13 +++++++++++++
>      2 files changed, 42 insertions(+), 16 deletions(-)
>      create mode 100644 package/dhcp/dhcrelay.service
> 
>     diff --git a/package/dhcp/dhcp.mk <http://dhcp.mk> b/package/dhcp/dhcp.mk <http://dhcp.mk>
>     index 3aa1577..4ba302b 100644
>     --- a/package/dhcp/dhcp.mk <http://dhcp.mk>
>     +++ b/package/dhcp/dhcp.mk <http://dhcp.mk>
>     @@ -37,6 +37,23 @@ define DHCP_INSTALL_SERVER
>             $(INSTALL) -m 0644 -D package/dhcp/dhcpd.conf \
>                     $(TARGET_DIR)/etc/dhcp/dhcpd.conf
>      endef
>     +
>     +define DHCP_INSTALL_SERVER_INIT_SYSTEMD
>     +       $(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
>     +               $(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
>     +
>     +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
>     +
>     +       ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
>     +               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
>     +
>     +       echo "d /var/lib/dhcp 0755 - - - -" > \
>     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     +       echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
>     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     +       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
>     +               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     +endef
>      endif
> 
>      ifeq ($(BR2_PACKAGE_DHCP_RELAY),y)
>     @@ -46,6 +63,16 @@ define DHCP_INSTALL_RELAY
>             $(INSTALL) -m 0755 -D $(DHCP_DIR)/relay/dhcrelay \
>                     $(TARGET_DIR)/usr/sbin/dhcrelay
>      endef
>     +
>     +define DHCP_INSTALL_RELAY_INIT_SYSTEMD
>     +       $(INSTALL) -D -m 644 package/dhcp/dhcrelay.service \
>     +               $(TARGET_DIR)/usr/lib/systemd/system/dhcrelay.service
>     +
>     +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
>     +
>     +       ln -sf ../../../../usr/lib/systemd/system/dhcrelay.service \
>     +               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcrelay.service
>     +endef
>      endif
> 
>      ifeq ($(BR2_PACKAGE_DHCP_CLIENT),y)
>     @@ -69,24 +96,10 @@ define DHCP_INSTALL_INIT_SYSV
>                     $(TARGET_DIR)/etc/init.d/S80dhcp-relay
>      endef
> 
>     -ifeq ($(BR2_PACKAGE_DHCP_SERVER),y)
>      define DHCP_INSTALL_INIT_SYSTEMD
>     -       $(INSTALL) -D -m 644 package/dhcp/dhcpd.service \
>     -               $(TARGET_DIR)/usr/lib/systemd/system/dhcpd.service
>     -
>     -       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
>     -
>     -       ln -sf ../../../../usr/lib/systemd/system/dhcpd.service \
>     -               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/dhcpd.service
>     -
>     -       echo "d /var/lib/dhcp 0755 - - - -" > \
>     -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     -       echo "f /var/lib/dhcp/dhcpd.leases - - - - -" >> \
>     -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     -       echo "f /var/lib/dhcp/dhcpd6.leases - - - - -" >> \
>     -               $(TARGET_DIR)/usr/lib/tmpfiles.d/dhcpd.conf
>     +       $(DHCP_INSTALL_SERVER_INIT_SYSTEMD)
>     +       $(DHCP_INSTALL_RELAY_INIT_SYSTEMD)
>      endef
>     -endif
> 
>      define DHCP_INSTALL_TARGET_CMDS
>             $(DHCP_INSTALL_RELAY)
>     diff --git a/package/dhcp/dhcrelay.service b/package/dhcp/dhcrelay.service
>     new file mode 100644
>     index 0000000..5a1410e
>     --- /dev/null
>     +++ b/package/dhcp/dhcrelay.service
>     @@ -0,0 +1,13 @@
>     +[Unit]
>     +Description=DHCP relay
>     +After=network.target
>     +
>     +[Service]
>     +Type=forking
>     +PIDFile=/run/dhcrelay.pid
>     +ExecStart=/usr/sbin/dhcrelay -q -pf /run/dhcrelay.pid $OPTIONS $IFCMD $SERVERS
> 
> Why don't we want /var/run/dhcrelay.pid which is the default ? 

I did that to be consistent with what had been done for dhcpd.service.

/run is the new norm, but Buildroot configures the dhcp package to use /var/run
as the default, and the SysV init scripts stick to that.

I don't know Buildroot's rules about that if any. Maybe dhcp.mk should just be
updated to use /run. Thomas?

>     +KillSignal=SIGINT
> 
> I did not try to run it, but it doesn't react to SIGTERM ? I did not see anything in the code to prevent it but I only looked at it pretty quickly.

It's also from dhcpd.service. I've searched around and I've not found a good
reason for it, but there must be one. I'll try and dig more.

>     +EnvironmentFile=/etc/default/dhcrelay
> 
> Should be 'EnvironmentFile=-/etc/default/dhcrelay'

This is on purpose. This file is required because IFCMD and SERVERS are required
too.

>     +
>     +[Install]
>     +WantedBy=multi-user.target
>     --
>     2.1.4

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-11-04 10:18     ` Thomas Petazzoni
  2015-11-04 10:24       ` Benoît Thébaudeau
  2015-11-04 10:25       ` Maxime Hadjinlian
@ 2015-11-04 18:42       ` Gabe Evans
  2 siblings, 0 replies; 47+ messages in thread
From: Gabe Evans @ 2015-11-04 18:42 UTC (permalink / raw)
  To: buildroot

Hi Thomas, all,

On Wed, Nov 4, 2015 at 2:18 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Maxime, Beno?t, Gabe,
>
> On Wed, 4 Nov 2015 10:52:10 +0100, Maxime Hadjinlian wrote:
>
>> > diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
>> > index 7b265cb..5989506 100644
>> > --- a/package/dhcp/dhcpd.service
>> > +++ b/package/dhcp/dhcpd.service
>> > @@ -7,7 +7,7 @@ Type=forking
>> >  PIDFile=/run/dhcpd.pid
>> >  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $INTERFACES
>> >  KillSignal=SIGINT
>> > -EnvironmentFile=/etc/default/dhcpd.conf
>> > +EnvironmentFile=/etc/default/dhcpd
>> >
>> Maybe this should be:
>> EnvironmentFile-=/etc/default/dhcpd
>
> On the dropbear package, Gabe Evans said it may not be such a good idea
> to do this EnvironmentFile thing.
>
> Can we settle on a common decision ?
>
> (From my PoV, using this EnvironmentFile looked good as it would be a
> common mechanism between the systemd and Busybox/sysvinit cases to tune
> the configuration of services. But Gabe seemed to disagree.)
>
> Could systemd people in Buildroot decide what to do ? :-)
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Maxime cleared everything up for me. I'm okay with this as long as the
environment file is optional with:
'EnvironmentFile=-/etc/default/dhcpd'. Under the rationale of sharing
configuration between Busybox/SysVinit it makes the most sense.

Thanks,
Gabe

-- 
Gabe Evans | Co-Founder & CTO
hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file
  2015-11-04 13:35     ` Benoît Thébaudeau
@ 2015-11-04 21:26       ` Arnout Vandecappelle
  2015-11-15 17:36       ` Benoît Thébaudeau
  1 sibling, 0 replies; 47+ messages in thread
From: Arnout Vandecappelle @ 2015-11-04 21:26 UTC (permalink / raw)
  To: buildroot

On 04-11-15 14:35, Beno?t Th?baudeau wrote:
> Hi Maxime, all,
> 
> On 04/11/2015 12:24, Maxime Hadjinlian wrote:
>> On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>> wrote:
[snip]
>>     +ExecStart=/usr/sbin/dhcrelay -q -pf /run/dhcrelay.pid $OPTIONS $IFCMD $SERVERS
>>
>> Why don't we want /var/run/dhcrelay.pid which is the default ? 
> 
> I did that to be consistent with what had been done for dhcpd.service.
> 
> /run is the new norm, but Buildroot configures the dhcp package to use /var/run
> as the default, and the SysV init scripts stick to that.
> 
> I don't know Buildroot's rules about that if any. Maybe dhcp.mk should just be
> updated to use /run. Thomas?

 Since /var/run is currently a symlink to /run, it would be good to update
dhcp.mk as you propose. Or perhaps even remove those configures since I think
they're the default anyway.


 Regards,
 Arnout

> 
>>     +KillSignal=SIGINT
>>
>> I did not try to run it, but it doesn't react to SIGTERM ? I did not see anything in the code to prevent it but I only looked at it pretty quickly.
> 
> It's also from dhcpd.service. I've searched around and I've not found a good
> reason for it, but there must be one. I'll try and dig more.
> 
>>     +EnvironmentFile=/etc/default/dhcrelay
>>
>> Should be 'EnvironmentFile=-/etc/default/dhcrelay'
> 
> This is on purpose. This file is required because IFCMD and SERVERS are required
> too.
> 
>>     +
>>     +[Install]
>>     +WantedBy=multi-user.target
>>     --
>>     2.1.4
> 
> Best regards,
> Beno?t
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-04 11:31           ` Maxime Hadjinlian
@ 2015-11-09 10:49             ` Benoît Thébaudeau
  2015-11-12 10:42               ` Maxime Hadjinlian
  0 siblings, 1 reply; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-09 10:49 UTC (permalink / raw)
  To: buildroot

Hi Maxime, all,

On 04/11/2015 12:31, Maxime Hadjinlian wrote:
>>>     > A question though, from the manual page of dhcpd, by default it starts as an IPv6 server should we have, two services one with '-4' and one with '-6' ? And it's left to the user to specify it in the "$OPTIONS" ? Maybe this should be mentioned somewhere ?
>>>
>>>     '-4' is the default, not '-6'. 
>>>
>>> If I look at ./server/dhcpd.8 in the sources of dhcp-4.1-ESVR12, it clearly states:
>>>
>>> -6 Run as a DHCPv6 server. This is the default and cannot be combined with -4. 
>> 
>> My bad: I had looked at the man page of another version.

Actually, this dhcpd.8 is wrong. Looking at common/discover.c:
    int local_family = AF_INET;

So IPv4 really is the default, and this is confirmed by the behavior.

This documentation issue is fixed in 4.3.3. A version bump would be great, all
the more the EoL date of 4.1-ESV-R12 is Dec. 2015. I'll see if I can quickly add
that to my series.

>     >     A single instance of dhcpd cannot be run with
>     >     both '-4' and '-6'. I don't know if it's possible (or desirable) to safely run
>     >     simultaneously two instances of dhcpd, one with '-4' and the other one with
>     >     '-6'.
>     >
>     > Well it could be useful if you are trying to build a network appliance and you want to support both stack. So you would want to start two processes with different options. Maybe that's something that should be left to the users to figure out.
> 
>     Yes, but should everything be duplicated in Buildroot just for this purpose? For
>     SysV, it's easy not to duplicate things, but how would you do that with systemd?
>     How do distributions handle this?
> 
> I only looked at Arch Linux, they duplicate the service file. It's also possible to use the template mechanisms in systemd, to have something like dhcpd at .service and when you enable it, you create a symlink called dhcpd at 4.service, and the '4' is passed as argument to the template and you can use it however you want it to.

Ubuntu duplicates the service files too. And all the service files come from the
same package.

How do we want to handle that in Buildroot?
1/ Always start IPv4 and IPv6 instances like Ubuntu does. This is wasting
   resources if one of them is actually unused.
2/ Add a Kconfig choice - common to all the selected DHCP daemons - between
   "only IPv4", "only IPv6", and "IPv4 + IPv6".
3/ Use /etc/default/dhcpd and the like. We could detect the presence of
   /etc/default/dhcpd vs. /etc/default/dhcpd6, or of some variables in them, or
   of /etc/dhcp/dhcpd.conf vs. /etc/dhcp/dhcpd6.conf. Something like that seems
   to be easier to use, but less obvious for users unless well documented.

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-09 10:49             ` Benoît Thébaudeau
@ 2015-11-12 10:42               ` Maxime Hadjinlian
  2015-11-15 17:55                 ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Maxime Hadjinlian @ 2015-11-12 10:42 UTC (permalink / raw)
  To: buildroot

Hi Benoit, all

On Mon, Nov 9, 2015 at 11:49 AM, Beno?t Th?baudeau <benoit@wsystem.com>
wrote:

> Hi Maxime, all,
>
> On 04/11/2015 12:31, Maxime Hadjinlian wrote:
> >>>     > A question though, from the manual page of dhcpd, by default it
> starts as an IPv6 server should we have, two services one with '-4' and one
> with '-6' ? And it's left to the user to specify it in the "$OPTIONS" ?
> Maybe this should be mentioned somewhere ?
> >>>
> >>>     '-4' is the default, not '-6'.
> >>>
> >>> If I look at ./server/dhcpd.8 in the sources of dhcp-4.1-ESVR12, it
> clearly states:
> >>>
> >>> -6 Run as a DHCPv6 server. This is the default and cannot be combined
> with -4.
> >>
> >> My bad: I had looked at the man page of another version.
>
> Actually, this dhcpd.8 is wrong. Looking at common/discover.c:
>     int local_family = AF_INET;
>
> So IPv4 really is the default, and this is confirmed by the behavior.
>
> This documentation issue is fixed in 4.3.3. A version bump would be great,
> all
> the more the EoL date of 4.1-ESV-R12 is Dec. 2015. I'll see if I can
> quickly add
> that to my series.
>
Ah, that explains everything. Indeed, a bump would be nice if you could
squeeze it, otherwise I'm sure it'll be done pretty quickly, DHCP is a
pretty used package after all :).

>
> >     >     A single instance of dhcpd cannot be run with
> >     >     both '-4' and '-6'. I don't know if it's possible (or
> desirable) to safely run
> >     >     simultaneously two instances of dhcpd, one with '-4' and the
> other one with
> >     >     '-6'.
> >     >
> >     > Well it could be useful if you are trying to build a network
> appliance and you want to support both stack. So you would want to start
> two processes with different options. Maybe that's something that should be
> left to the users to figure out.
> >
> >     Yes, but should everything be duplicated in Buildroot just for this
> purpose? For
> >     SysV, it's easy not to duplicate things, but how would you do that
> with systemd?
> >     How do distributions handle this?
> >
> > I only looked at Arch Linux, they duplicate the service file. It's also
> possible to use the template mechanisms in systemd, to have something like
> dhcpd at .service and when you enable it, you create a symlink called
> dhcpd at 4.service, and the '4' is passed as argument to the template and
> you can use it however you want it to.
>
> Ubuntu duplicates the service files too. And all the service files come
> from the
> same package.
>
> How do we want to handle that in Buildroot?
> 1/ Always start IPv4 and IPv6 instances like Ubuntu does. This is wasting
>    resources if one of them is actually unused.
> 2/ Add a Kconfig choice - common to all the selected DHCP daemons - between
>    "only IPv4", "only IPv6", and "IPv4 + IPv6".
> 3/ Use /etc/default/dhcpd and the like. We could detect the presence of
>    /etc/default/dhcpd vs. /etc/default/dhcpd6, or of some variables in
> them, or
>    of /etc/dhcp/dhcpd.conf vs. /etc/dhcp/dhcpd6.conf. Something like that
> seems
>    to be easier to use, but less obvious for users unless well documented.
>
> In my humble opinion, I would go for the Kconfig options as I find this
easier for the end user and to maintain.

> Best regards,
> Beno?t
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151112/3f60a172/attachment.html>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file
  2015-11-04 13:35     ` Benoît Thébaudeau
  2015-11-04 21:26       ` Arnout Vandecappelle
@ 2015-11-15 17:36       ` Benoît Thébaudeau
  1 sibling, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-15 17:36 UTC (permalink / raw)
  To: buildroot

Hi Maxime, all,

On Wed, Nov 4, 2015 at 2:35 PM, Beno?t Th?baudeau <benoit@wsystem.com> wrote:
> On 04/11/2015 12:24, Maxime Hadjinlian wrote:
>> On Sun, Oct 25, 2015 at 2:59 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev at gmail.com <mailto:benoit.thebaudeau.dev@gmail.com>> wrote:
[...]
>>     diff --git a/package/dhcp/dhcrelay.service b/package/dhcp/dhcrelay.service
>>     new file mode 100644
>>     index 0000000..5a1410e
>>     --- /dev/null
>>     +++ b/package/dhcp/dhcrelay.service
>>     @@ -0,0 +1,13 @@
>>     +[Unit]
>>     +Description=DHCP relay
>>     +After=network.target
>>     +
>>     +[Service]
>>     +Type=forking
>>     +PIDFile=/run/dhcrelay.pid
>>     +ExecStart=/usr/sbin/dhcrelay -q -pf /run/dhcrelay.pid $OPTIONS $IFCMD $SERVERS
[...]
>>     +KillSignal=SIGINT
>>
>> I did not try to run it, but it doesn't react to SIGTERM ? I did not see anything in the code to prevent it but I only looked at it pretty quickly.
>
> It's also from dhcpd.service. I've searched around and I've not found a good
> reason for it, but there must be one. I'll try and dig more.

There is a reference to it in this very old thread:
http://www.gossamer-threads.com/lists/gentoo/dev/31150

Here too:
https://kb.isc.org/article/AA-01043/0/Recommendations-for-restarting-a-DHCP-failover-pair.html
But this has been implemented in 4.2.0 according to:
https://kb.isc.org/article/AA-01297
But includes/site.h advises against using this feature.

This SIGINT seems to have been copied from Arch Linux for
dhcpd.service. This originates from the following commit, which wants
to make a difference between reload and stop, but without giving any
reason:
https://projects.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/dhcp&id=f60fc1997126ce5f104335d8f09a317300ce8323

Then this commit removed the reload thing:
https://projects.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/dhcp&id=4d45b1bb254995199386eb2c1c8ab703c738d9d6

Looking at the sources, neither 4.1-ESV-R12 nor 4.3.3 seem to make any
difference between SIGTERM and SIGINT. Also, contrary to Arch Linux,
Ubuntu does not use SIGINT at all for dhcpd or dhcrelay, and it's not
because of something in its patches. So I would be in favor of not
using SIGINT in Buildroot too.

>>     +EnvironmentFile=/etc/default/dhcrelay
[...]
>>     +
>>     +[Install]
>>     +WantedBy=multi-user.target
[...]

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: support IPv6 lease file
  2015-11-12 10:42               ` Maxime Hadjinlian
@ 2015-11-15 17:55                 ` Benoît Thébaudeau
  0 siblings, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2015-11-15 17:55 UTC (permalink / raw)
  To: buildroot

Hi Maxime, all,

On Thu, Nov 12, 2015 at 11:42 AM, Maxime Hadjinlian
<maxime.hadjinlian@gmail.com> wrote:
> On Mon, Nov 9, 2015 at 11:49 AM, Beno?t Th?baudeau <benoit@wsystem.com>
> wrote:
>> On 04/11/2015 12:31, Maxime Hadjinlian wrote:
>> >     >     A single instance of dhcpd cannot be run with
>> >     >     both '-4' and '-6'. I don't know if it's possible (or
>> > desirable) to safely run
>> >     >     simultaneously two instances of dhcpd, one with '-4' and the
>> > other one with
>> >     >     '-6'.
>> >     >
>> >     > Well it could be useful if you are trying to build a network
>> > appliance and you want to support both stack. So you would want to start two
>> > processes with different options. Maybe that's something that should be left
>> > to the users to figure out.
>> >
>> >     Yes, but should everything be duplicated in Buildroot just for this
>> > purpose? For
>> >     SysV, it's easy not to duplicate things, but how would you do that
>> > with systemd?
>> >     How do distributions handle this?
>> >
>> > I only looked at Arch Linux, they duplicate the service file. It's also
>> > possible to use the template mechanisms in systemd, to have something like
>> > dhcpd at .service and when you enable it, you create a symlink called
>> > dhcpd at 4.service, and the '4' is passed as argument to the template and you
>> > can use it however you want it to.
>>
>> Ubuntu duplicates the service files too. And all the service files come
>> from the
>> same package.
>>
>> How do we want to handle that in Buildroot?
>> 1/ Always start IPv4 and IPv6 instances like Ubuntu does. This is wasting
>>    resources if one of them is actually unused.
>> 2/ Add a Kconfig choice - common to all the selected DHCP daemons -
>> between
>>    "only IPv4", "only IPv6", and "IPv4 + IPv6".
>> 3/ Use /etc/default/dhcpd and the like. We could detect the presence of
>>    /etc/default/dhcpd vs. /etc/default/dhcpd6, or of some variables in
>> them, or
>>    of /etc/dhcp/dhcpd.conf vs. /etc/dhcp/dhcpd6.conf. Something like that
>> seems
>>    to be easier to use, but less obvious for users unless well documented.
>>
> In my humble opinion, I would go for the Kconfig options as I find this
> easier for the end user and to maintain.

Actually, I've just noticed that, while it installs both IPv4 and IPv6
startup mechanisms, Ubuntu uses ConditionPathExists with
/etc/dhcp/dhcpd.conf and /etc/dhcp/dhcpd6.conf to skip the dhcpd
service if its required configuration file is not present, just like
what is currently done by Buildroot's S80dhcp-server. In Buildroot,
that would mean having to tweak the rootfs overlay and/or the
post-build script to choose what to run. This would have to be done
anyway with Kconfig options in order not to keep the default
dhcpd.conf, which is not very useful, so it may be more flexible not
to have Kconfig options. Thomas?

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing
  2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
                   ` (11 preceding siblings ...)
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file Benoît Thébaudeau
@ 2015-12-24 13:59 ` Thomas Petazzoni
  12 siblings, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 13:59 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:27 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> 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@wsystem.com>
> 
> ---
> Changes v2 -> v3: none.

I've applied, after adjusting the commit title and explanation, since
the change is now only needed for S80dhcp-relay, as S80dhcp-server had
already been modified to read /etc/default/dhcpd by a previous commit.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
  2015-11-04  9:52   ` Maxime Hadjinlian
@ 2015-12-24 14:00   ` Thomas Petazzoni
  1 sibling, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:00 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:28 +0200, Beno?t Th?baudeau wrote:
> Use the same EnvironmentFile name as the SysV init script for
> consistency. The filenames under /etc/default/ are usually just the
> package/daemon/service/feature name without any extension, so remove the
> ".conf" extension from EnvironmentFile.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

I already did this change in a commit that also adjusted S80dhcp-server:

  https://git.busybox.net/buildroot/commit/?id=6f81baaf47e3f47b131ec2b1c6c5f7d062a48d84

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options Benoît Thébaudeau
@ 2015-12-24 14:00   ` Thomas Petazzoni
  0 siblings, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:00 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:29 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> Add an OPTIONS configuration variable in order to make it possible to
> pass custom extra options to dhcpd.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> ---
> Changes v2 -> v3: none.

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: support extra options
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: " Benoît Thébaudeau
  2015-11-04 10:09   ` Maxime Hadjinlian
@ 2015-12-24 14:01   ` Thomas Petazzoni
  1 sibling, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:01 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:30 +0200, Beno?t Th?baudeau wrote:
> Add an OPTIONS configuration variable in order to make it possible to
> pass custom extra options to dhcpd. This keeps the systemd support
> consistent with the SysV init script.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> 
> ---
> Changes v2 -> v3: new patch.
> ---
>  package/dhcp/dhcpd.service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES Benoît Thébaudeau
@ 2015-12-24 14:02   ` Thomas Petazzoni
  0 siblings, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:02 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:31 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> 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.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> ---
> Changes v2 -> v3: none.

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file Benoît Thébaudeau
  2015-11-04 10:14   ` Maxime Hadjinlian
@ 2015-12-24 14:03   ` Thomas Petazzoni
  1 sibling, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:03 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:32 +0200, Beno?t Th?baudeau wrote:

> diff --git a/package/dhcp/dhcpd.service b/package/dhcp/dhcpd.service
> index ad1300c..98cc54b 100644
> --- a/package/dhcp/dhcpd.service
> +++ b/package/dhcp/dhcpd.service
> @@ -7,7 +7,7 @@ Type=forking
>  PIDFile=/run/dhcpd.pid
>  ExecStart=/usr/sbin/dhcpd -q -pf /run/dhcpd.pid $OPTIONS $INTERFACES
>  KillSignal=SIGINT
> -EnvironmentFile=/etc/default/dhcpd
> +EnvironmentFile=-/etc/default/dhcpd
>  
>  [Install]
>  WantedBy=multi-user.target

A similar change had already been done in commit
21aa707a126155533748998f4fae328b1c63d1d6.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text Benoît Thébaudeau
@ 2015-12-24 14:05   ` Thomas Petazzoni
  0 siblings, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:05 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:33 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> Fix various messages displayed by these scripts:
>  - remove extra "dhcpd3" for `S80dhcp-server stop`,

I had already done this change in a previous commit.

>  - 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.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

So, I've applied your patch, after adjusting it to not do the dhcpd3
removal.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart Benoît Thébaudeau
@ 2015-12-24 14:06   ` Thomas Petazzoni
  0 siblings, 0 replies; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:06 UTC (permalink / raw)
  To: buildroot

Dear Beno?t Th?baudeau,

On Sun, 25 Oct 2015 02:59:34 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> No sleep is required for the restart and force-reload operations to
> succeed.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> ---
> Changes v2 -> v3: none.

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop
  2015-10-25  0:59 ` [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop Benoît Thébaudeau
@ 2015-12-24 14:26   ` Thomas Petazzoni
  2016-01-07 20:40     ` Benoît Thébaudeau
  0 siblings, 1 reply; 47+ messages in thread
From: Thomas Petazzoni @ 2015-12-24 14:26 UTC (permalink / raw)
  To: buildroot

Beno?t,

On Sun, 25 Oct 2015 02:59:35 +0200, Beno?t Th?baudeau wrote:
> From: Beno?t Th?baudeau <benoit@wsystem.com>
> 
> These daemons do not remove their PID files, so do it manually in the
> scripts.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>

I think this is not the right way of handling this and the IPv6
problem.

What I would suggest is that the S80dhcp-server script should start two
DHCP servers: one with -4 and one with -6. To avoid IPv6 failures, you
can test if /proc/net/if_inet6 exists.

Then, I was hoping you could use the --remove-pidfile option of
start-stop-daemon, but it unfortunately doesn't exist in the Busybox
implementation of start-stop-daemon.

That being said, I still believe starting two daemons is the best
solution. I don't think a Config.in option should be added for that.
Just always start both, with as I propose an exception to not start the
IPv6 daemon if /proc/net/if_inet6 does not exist.

Could you rework your patches 9 to 13 with this goal in mind ?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 47+ messages in thread

* [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop
  2015-12-24 14:26   ` Thomas Petazzoni
@ 2016-01-07 20:40     ` Benoît Thébaudeau
  0 siblings, 0 replies; 47+ messages in thread
From: Benoît Thébaudeau @ 2016-01-07 20:40 UTC (permalink / raw)
  To: buildroot

Thomas,

On Thu, Dec 24, 2015 at 3:26 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Sun, 25 Oct 2015 02:59:35 +0200, Beno?t Th?baudeau wrote:
>> From: Beno?t Th?baudeau <benoit@wsystem.com>
>>
>> These daemons do not remove their PID files, so do it manually in the
>> scripts.
>>
>> Signed-off-by: Beno?t Th?baudeau <benoit@wsystem.com>
>
> I think this is not the right way of handling this and the IPv6
> problem.
>
> What I would suggest is that the S80dhcp-server script should start two
> DHCP servers: one with -4 and one with -6. To avoid IPv6 failures, you
> can test if /proc/net/if_inet6 exists.

OK, I'll do that.

> Then, I was hoping you could use the --remove-pidfile option of
> start-stop-daemon, but it unfortunately doesn't exist in the Busybox
> implementation of start-stop-daemon.

Indeed. I also went that way first. So what are you eventually
suggesting to change for 09/13 since there is apparently no other
solution for SysV? Even with two daemons started, the PID files will
have to be removed with rm for SysV, so 09/13 would only have to be
split. This is already handled for systemd.

> That being said, I still believe starting two daemons is the best
> solution. I don't think a Config.in option should be added for that.
> Just always start both, with as I propose an exception to not start the
> IPv6 daemon if /proc/net/if_inet6 does not exist.
>
> Could you rework your patches 9 to 13 with this goal in mind ?
>
> Thanks a lot!

Will do.

Beno?t

^ permalink raw reply	[flat|nested] 47+ messages in thread

end of thread, other threads:[~2016-01-07 20:40 UTC | newest]

Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-25  0:59 [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Benoît Thébaudeau
2015-10-25  0:59 ` [Buildroot] [PATCH v3 02/13] package/dhcp: systemd: rename environment file Benoît Thébaudeau
2015-11-04  9:52   ` Maxime Hadjinlian
2015-11-04  9:54     ` Maxime Hadjinlian
2015-11-04 10:01       ` Benoît Thébaudeau
2015-11-04 10:04         ` Maxime Hadjinlian
2015-11-04 10:07           ` Benoît Thébaudeau
2015-11-04 10:13             ` Maxime Hadjinlian
2015-11-04 10:18     ` Thomas Petazzoni
2015-11-04 10:24       ` Benoît Thébaudeau
2015-11-04 10:25       ` Maxime Hadjinlian
2015-11-04 18:42       ` Gabe Evans
2015-12-24 14:00   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 03/13] package/dhcp/S80dhcp-server: support extra options Benoît Thébaudeau
2015-12-24 14:00   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 04/13] package/dhcp: systemd: " Benoît Thébaudeau
2015-11-04 10:09   ` Maxime Hadjinlian
2015-12-24 14:01   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 05/13] package/dhcp/S80dhcp-server: allow empty INTERFACES Benoît Thébaudeau
2015-12-24 14:02   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 06/13] package/dhcp: systemd: allow missing environment file Benoît Thébaudeau
2015-11-04 10:14   ` Maxime Hadjinlian
2015-12-24 14:03   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 07/13] package/dhcp: fix SysV init scripts output text Benoît Thébaudeau
2015-12-24 14:05   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 08/13] package/dhcp: remove sleep from SysV init scripts restart Benoît Thébaudeau
2015-12-24 14:06   ` Thomas Petazzoni
2015-10-25  0:59 ` [Buildroot] [PATCH v3 09/13] package/dhcp: SysV init scripts: remove PID files after stop Benoît Thébaudeau
2015-12-24 14:26   ` Thomas Petazzoni
2016-01-07 20:40     ` Benoît Thébaudeau
2015-10-25  0:59 ` [Buildroot] [PATCH v3 10/13] package/dhcp/S80dhcp-server: support IPv6 lease file Benoît Thébaudeau
2015-10-25  0:59 ` [Buildroot] [PATCH v3 11/13] package/dhcp: systemd: " Benoît Thébaudeau
2015-11-04 10:21   ` Maxime Hadjinlian
2015-11-04 10:36     ` Benoît Thébaudeau
2015-11-04 10:53       ` Maxime Hadjinlian
2015-11-04 11:15         ` Benoît Thébaudeau
2015-11-04 11:31           ` Maxime Hadjinlian
2015-11-09 10:49             ` Benoît Thébaudeau
2015-11-12 10:42               ` Maxime Hadjinlian
2015-11-15 17:55                 ` Benoît Thébaudeau
2015-10-25  0:59 ` [Buildroot] [PATCH v3 12/13] package/dhcp: SysV init scripts: refactor using functions Benoît Thébaudeau
2015-10-25  0:59 ` [Buildroot] [PATCH v3 13/13] package/dhcp: systemd: add dhcrelay service file Benoît Thébaudeau
2015-11-04 11:24   ` Maxime Hadjinlian
2015-11-04 13:35     ` Benoît Thébaudeau
2015-11-04 21:26       ` Arnout Vandecappelle
2015-11-15 17:36       ` Benoît Thébaudeau
2015-12-24 13:59 ` [Buildroot] [PATCH v3 01/13] package/dhcp: fix SysV init scripts option passing Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.