[PATCH] Support dhcpcd in dhcp-client-service-type

  • Open
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Sergey Trofimov
  • soeren
Owner
unassigned
Submitted by
soeren
Severity
normal
S
S
soeren wrote on 23 Jan 17:12 +0100
(address . guix-patches@gnu.org)
cover.1706026000.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

This patch series add support for an alternative DHCP client to
dhcp-client-service-type. This is necessary as, currently,
dhcp-client-service-type only supports the ISC DHCP implementation
and this implementation has reached its end-of-life in 2022(!).
For more information, refer to https://issues.guix.gnu.org/68619.

Specifically, this patch series adds support for dhcpcd. However, it
is not yet enabled by default. Nonetheless, I would strongly suggest
enabling it by default at a later point in time.

In order to enable dhcpcd add the following to /etc/config.scm:

(service dhcp-client-service-type
(dhcp-client-configuration
(package dhcpcd)))

I have done some basics tests with this change applied with both
dhcpcd and isc-dhcp. Further tests and feedback would be much
appreciated!

Sören Tempel (2):
gnu: Add dhcpcd.
services: dhcp: Support the dhcpcd implementation.

gnu/packages/admin.scm | 42 +++++++++++++++++++
gnu/services/networking.scm | 84 ++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 24 deletions(-)


base-commit: 29c26a8d308286cf378ce9cfa3d73e3d1454263d
S
S
soeren wrote on 23 Jan 17:14 +0100
[PATCH] gnu: Add dhcpcd.
(address . 68675@debbugs.gnu.org)
026d5ef86d6351918ee989f48be225bd97490d4d.1706026000.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

* gnu/packages/admin.scm (dhcpcd): new procedure.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
gnu/packages/admin.scm | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)

Toggle diff (53 lines)
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index fcf05992d8..78a5bbd973 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -1608,6 +1608,48 @@ (define-public isc-dhcp
(license license:mpl2.0)
(properties '((cpe-name . "dhcp"))))))
+(define-public dhcpcd
+ (package
+ (name "dhcpcd")
+ (version "10.0.6")
+ (source
+ (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/NetworkConfiguration/dhcpcd")
+ (commit (string-append "v" version))))
+ (sha256
+ (base32 "07n7d5wsmy955i6l8rkcmxhgxjygj2cxgpw79id2hx9w41fbkl5l"))
+ (file-name (git-file-name name version))))
+ (native-inputs (list eudev))
+ (build-system gnu-build-system)
+ (arguments
+ (list
+ #:test-target "test"
+ #:configure-flags
+ #~(list "--enable-ipv6"
+ "--enable-privsep"
+ "--privsepuser=dhcpcd"
+ (string-append "--dbdir=" "/var/db/dhcpcd")
+ (string-append "--rundir=" "/var/run/dhcpcd")
+ "CC=gcc")
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-after 'unpack 'do-not-create-dbdir
+ (lambda _
+ (substitute* "src/Makefile"
+ (("\\$\\{INSTALL\\} -m \\$\\{DBMODE\\} -d \\$\\{DESTDIR\\}\\$\\{DBDIR\\}") ""))))
+ (add-before 'build 'setenv
+ (lambda _
+ (setenv "HOST_SH" (string-append #$bash-minimal "/bin/sh")))))))
+ (home-page "https://roy.marples.name/projects/dhcpcd")
+ (synopsis "Feature-rich DHCP and DHCPv6 client.")
+ (description "Provides a DHCP and a DHCPv6 client. Additionally,
+dhcpcd is also an IPv4LL (aka ZeroConf) client. In layperson's terms,
+dhcpcd runs on your machine and silently configures your computer to work
+on the attached networks without trouble and mostly without configuration.")
+ (license license:bsd-2)))
+
(define-public radvd
(package
(name "radvd")
S
S
soeren wrote on 23 Jan 17:14 +0100
[PATCH] services: dhcp: Support the dhcpcd implementation.
(address . 68675@debbugs.gnu.org)
68f1b45a7f8ced9bb57c661c0e7afa5136b5c673.1706026000.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

Prior to this commit, the isc-dhcp implementation was the only DHCP
implementation supported by dhcp-client-shepherd-service. This is
problematic as the ISC implementation has reached end-of-life in
2022(!). As a first step to migrate away from isc-dhcp, this commit
adds support for dhcpcd to dhcp-client-shepherd-service. Currently,
it has to be enabled explicitly via the package field of the
dhcp-client-configuration. In the future, it is intended to become
the default to migrate away from isc-dhcp.


* gnu/services/networking.scm (dhcp-client-shepherd-service): Add
support for the dhcpcd client implementation.
* gnu/services/networking.scm (dhcp-client-account-service): New
procedure.
* gnu/services/networking.scm (dhcp-client-service-type): Add optional
account-service-type extensions (needed for dhcpcd).

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
gnu/services/networking.scm | 84 ++++++++++++++++++++++++++-----------
1 file changed, 60 insertions(+), 24 deletions(-)

Toggle diff (119 lines)
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 495d049728..3621e2bda2 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -316,25 +316,21 @@ (define-record-type* <dhcp-client-configuration>
(define dhcp-client-shepherd-service
(match-lambda
((? dhcp-client-configuration? config)
- (let ((package (dhcp-client-configuration-package config))
- (requirement (dhcp-client-configuration-shepherd-requirement config))
- (provision (dhcp-client-configuration-shepherd-provision config))
- (interfaces (dhcp-client-configuration-interfaces config))
- (pid-file "/var/run/dhclient.pid"))
+ (let* ((package (dhcp-client-configuration-package config))
+ (client-name (package-name package))
+ (requirement (dhcp-client-configuration-shepherd-requirement config))
+ (provision (dhcp-client-configuration-shepherd-provision config))
+ (interfaces (dhcp-client-configuration-interfaces config)))
(list (shepherd-service
(documentation "Set up networking via DHCP.")
(requirement `(user-processes udev ,@requirement))
(provision provision)
- ;; XXX: Running with '-nw' ("no wait") avoids blocking for a minute when
- ;; networking is unavailable, but also means that the interface is not up
- ;; yet when 'start' completes. To wait for the interface to be ready, one
- ;; should instead monitor udev events.
(start #~(lambda _
- (define dhclient
- (string-append #$package "/sbin/dhclient"))
+ (use-modules (ice-9 popen)
+ (ice-9 rdelim))
- ;; When invoked without any arguments, 'dhclient' discovers all
+ ;; When invoked without any arguments, the client discovers all
;; non-loopback interfaces *that are up*. However, the relevant
;; interfaces are typically down at this point. Thus we perform
;; our own interface discovery here.
@@ -355,17 +351,40 @@ (define dhcp-client-shepherd-service
(_
#~'#$interfaces))))
- (false-if-exception (delete-file #$pid-file))
- (let ((pid (fork+exec-command
- ;; By default dhclient uses a
- ;; pre-standardization implementation of
- ;; DDNS, which is incompatable with
- ;; non-ISC DHCP servers; thus, pass '-I'.
- ;; <https://kb.isc.org/docs/aa-01091>.
- (cons* dhclient "-nw" "-I"
- "-pf" #$pid-file ifaces))))
- (and (zero? (cdr (waitpid pid)))
- (read-pid-file #$pid-file)))))
+ ;; Returns the execution configuration for the DHCP client
+ ;; selected by the package field of dhcp-client-configuration.
+ ;; The configuration is a pair of pidfile and execution command
+ ;; where the latter is a list.
+ (define exec-config
+ (case (string->symbol #$client-name)
+ ((isc-dhcp)
+ (let ((pid-file "/var/run/dhclient.pid"))
+ (cons
+ (cons* (string-append #$package "/sbin/dhclient")
+ "-nw" "-I" "-pf" pid-file ifaces)
+ pid-file)))
+ ((dhcpcd)
+ ;; For dhcpcd, the utilized pid-file depends on the
+ ;; command-line arguments. If multiple interfaces are
+ ;; given, a different pid-file is returned. Hence, we
+ ;; consult dhcpcd itself to determine the pid-file.
+ (let* ((cmd (string-append #$package "/sbin/dhcpcd"))
+ (arg (cons* cmd "-b" ifaces)))
+ (cons arg
+ (let* ((pipe (string-join (append arg '("-P")) " "))
+ (port (open-input-pipe pipe))
+ (path (read-line port)))
+ (close-pipe port)
+ path))))
+ (else
+ (error (G_ "unknown 'package' value in dhcp-client-configuration")))))
+
+ (let ((pid-file (cdr exec-config))
+ (exec-cmd (car exec-config)))
+ (false-if-exception (delete-file pid-file))
+ (let ((pid (fork+exec-command exec-cmd)))
+ (and (zero? (cdr (waitpid pid)))
+ (read-pid-file pid-file))))))
(stop #~(make-kill-destructor))))))
(package
(warning (G_ "'dhcp-client' service now expects a \
@@ -377,10 +396,27 @@ (define dhcp-client-shepherd-service
(dhcp-client-configuration
(package package))))))
+(define (dhcp-client-account-service config)
+ (let ((package (dhcp-client-configuration-package config)))
+ ;; Contrary to other DHCP clients (e.g. dhclient), dhcpcd supports
+ ;; privilege separation. Hence, we need to create an account here.
+ (if (string=? "dhcpcd" (package-name package))
+ (list (user-group (name "dhcpcd") (system? #t))
+ (user-account
+ (name "dhcpcd")
+ (group "dhcpcd")
+ (system? #t)
+ (comment "dhcpcd daemon user")
+ (home-directory "/var/empty")
+ (shell "/run/current-system/profile/sbin/nologin")))
+ '())))
+
(define dhcp-client-service-type
(service-type (name 'dhcp-client)
(extensions
- (list (service-extension shepherd-root-service-type
+ (list (service-extension account-service-type
+ dhcp-client-account-service)
+ (service-extension shepherd-root-service-type
dhcp-client-shepherd-service)))
(default-value (dhcp-client-configuration))
(description "Run @command{dhcp}, a Dynamic Host Configuration
S
S
Sergey Trofimov wrote on 23 Jan 20:52 +0100
Re: [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type
(address . soeren@soeren-tempel.net)(address . 68675@debbugs.gnu.org)
87zfwvyghg.fsf@sarg.org.ru
soeren@soeren-tempel.net writes:

Toggle quote (15 lines)
> Specifically, this patch series adds support for dhcpcd. However, it
> is not yet enabled by default. Nonetheless, I would strongly suggest
> enabling it by default at a later point in time.
>
> In order to enable dhcpcd add the following to /etc/config.scm:
>
> (service dhcp-client-service-type
> (dhcp-client-configuration
> (package dhcpcd)))
>
> I have done some basics tests with this change applied with both
> dhcpcd and isc-dhcp. Further tests and feedback would be much
> appreciated!
>

Basic functionality works for me - my laptop receives an address.

Some things to fix:
1. 20-resolv.conf hook needs to be wrapped, it can't find `resolvconf`
currently (from openresolv)
2. %base-packages still contains isc-dhcp... why is it even needed?
S
S
soeren wrote on 24 Jan 20:05 +0100
[PATCH v2] gnu: Add dhcpcd.
(address . 68675@debbugs.gnu.org)
2156325d2caa8d4298c9828d84fa5fff40592da4.1706123111.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

* gnu/packages/admin.scm (dhcpcd): new procedure.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
Change since v1:

* wrap dhcpcd-run-hooks to make sed and coreutils available
* fix various lint and style warnings for the dhcpcd package

gnu/packages/admin.scm | 57 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

Toggle diff (70 lines)
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index fcf05992d8..4b106f87a8 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -1608,6 +1608,63 @@ (define-public isc-dhcp
(license license:mpl2.0)
(properties '((cpe-name . "dhcp"))))))
+(define-public dhcpcd
+ (package
+ (name "dhcpcd")
+ (version "10.0.6")
+ (source
+ (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/NetworkConfiguration/dhcpcd")
+ (commit (string-append "v" version))))
+ (sha256
+ (base32 "07n7d5wsmy955i6l8rkcmxhgxjygj2cxgpw79id2hx9w41fbkl5l"))
+ (file-name (git-file-name name version))))
+ (inputs (list bash-minimal))
+ (native-inputs (list eudev))
+ (build-system gnu-build-system)
+ (arguments
+ (list
+ #:test-target "test"
+ #:configure-flags #~(list "--enable-ipv6"
+ "--enable-privsep"
+ "--privsepuser=dhcpcd"
+ (string-append "--dbdir=" "/var/db/dhcpcd")
+ (string-append "--rundir=" "/var/run/dhcpcd")
+ "CC=gcc")
+ #:phases #~(modify-phases %standard-phases
+ (add-after 'unpack 'do-not-create-dbdir
+ (lambda _
+ ;; Make sure that the Makefile doesn't attempt to create
+ ;; /var/db/dhcpcd for which it doesn't have permissions.
+ (substitute* "src/Makefile"
+ (("\\$\\{INSTALL\\} -m \\$\\{DBMODE\\} -d \\$\\{DESTDIR\\}\\$\\{DBDIR\\}")
+ ""))))
+ (add-before 'build 'setenv
+ (lambda _
+ (setenv "HOST_SH"
+ (string-append #$bash-minimal "/bin/sh"))))
+ (add-after 'install 'wrap-hooks
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let* ((out (assoc-ref outputs "out"))
+ (libexec (string-append out "/libexec"))
+ (sed (assoc-ref inputs "sed"))
+ (coreutils (assoc-ref inputs "coreutils")))
+ (wrap-program (string-append libexec
+ "/dhcpcd-run-hooks")
+ `("PATH" ":" suffix
+ (,(string-append coreutils "/bin")
+ ,(string-append sed "/bin"))))))))))
+ (home-page "https://roy.marples.name/projects/dhcpcd")
+ (synopsis "Feature-rich DHCP and DHCPv6 client")
+ (description
+ "Provides a DHCP and a DHCPv6 client. Additionally,
+dhcpcd is also an IPv4LL (aka ZeroConf) client. In layperson's terms,
+dhcpcd runs on your machine and silently configures your computer to work
+on the attached networks without trouble and mostly without configuration.")
+ (license license:bsd-2)))
+
(define-public radvd
(package
(name "radvd")

base-commit: 29c26a8d308286cf378ce9cfa3d73e3d1454263d
S
S
soeren wrote on 24 Jan 20:05 +0100
[PATCH v2] services: dhcp: Support the dhcpcd implementation.
(address . 68675@debbugs.gnu.org)
5aff02159575834de675684dfde71d2ec66f4b10.1706123111.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

Prior to this commit, the isc-dhcp implementation was the only DHCP
implementation supported by dhcp-client-shepherd-service. This is
problematic as the ISC implementation has reached end-of-life in
2022(!). As a first step to migrate away from isc-dhcp, this commit
adds support for dhcpcd to dhcp-client-shepherd-service. Currently,
it has to be enabled explicitly via the package field of the
dhcp-client-configuration. In the future, it is intended to become
the default to migrate away from isc-dhcp.

While at it, also remove isc-dhcp from %base-packages as it is no
longer necessarily needed and it will be pulled in by the DHCP
client service if required.


* gnu/services/networking.scm (dhcp-client-shepherd-service): Add
support for the dhcpcd client implementation.
* gnu/services/networking.scm (dhcp-client-account-service): New
procedure.
* gnu/services/networking.scm (dhcp-client-service-type): Add optional
account-service-type extensions (needed for dhcpcd).
* gnu/system.scm (%base-packages-networking): Remove isc-dhcp from
%base-packages (will be pulled in by dhcp-client-shepherd-service).

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
Changes since v1:

* Remove isc-dhcp from %base-packages

gnu/services/networking.scm | 84 ++++++++++++++++++++++++++-----------
gnu/system.scm | 2 +-
2 files changed, 61 insertions(+), 25 deletions(-)

Toggle diff (132 lines)
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 495d049728..3621e2bda2 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -316,25 +316,21 @@ (define-record-type* <dhcp-client-configuration>
(define dhcp-client-shepherd-service
(match-lambda
((? dhcp-client-configuration? config)
- (let ((package (dhcp-client-configuration-package config))
- (requirement (dhcp-client-configuration-shepherd-requirement config))
- (provision (dhcp-client-configuration-shepherd-provision config))
- (interfaces (dhcp-client-configuration-interfaces config))
- (pid-file "/var/run/dhclient.pid"))
+ (let* ((package (dhcp-client-configuration-package config))
+ (client-name (package-name package))
+ (requirement (dhcp-client-configuration-shepherd-requirement config))
+ (provision (dhcp-client-configuration-shepherd-provision config))
+ (interfaces (dhcp-client-configuration-interfaces config)))
(list (shepherd-service
(documentation "Set up networking via DHCP.")
(requirement `(user-processes udev ,@requirement))
(provision provision)
- ;; XXX: Running with '-nw' ("no wait") avoids blocking for a minute when
- ;; networking is unavailable, but also means that the interface is not up
- ;; yet when 'start' completes. To wait for the interface to be ready, one
- ;; should instead monitor udev events.
(start #~(lambda _
- (define dhclient
- (string-append #$package "/sbin/dhclient"))
+ (use-modules (ice-9 popen)
+ (ice-9 rdelim))
- ;; When invoked without any arguments, 'dhclient' discovers all
+ ;; When invoked without any arguments, the client discovers all
;; non-loopback interfaces *that are up*. However, the relevant
;; interfaces are typically down at this point. Thus we perform
;; our own interface discovery here.
@@ -355,17 +351,40 @@ (define dhcp-client-shepherd-service
(_
#~'#$interfaces))))
- (false-if-exception (delete-file #$pid-file))
- (let ((pid (fork+exec-command
- ;; By default dhclient uses a
- ;; pre-standardization implementation of
- ;; DDNS, which is incompatable with
- ;; non-ISC DHCP servers; thus, pass '-I'.
- ;; <https://kb.isc.org/docs/aa-01091>.
- (cons* dhclient "-nw" "-I"
- "-pf" #$pid-file ifaces))))
- (and (zero? (cdr (waitpid pid)))
- (read-pid-file #$pid-file)))))
+ ;; Returns the execution configuration for the DHCP client
+ ;; selected by the package field of dhcp-client-configuration.
+ ;; The configuration is a pair of pidfile and execution command
+ ;; where the latter is a list.
+ (define exec-config
+ (case (string->symbol #$client-name)
+ ((isc-dhcp)
+ (let ((pid-file "/var/run/dhclient.pid"))
+ (cons
+ (cons* (string-append #$package "/sbin/dhclient")
+ "-nw" "-I" "-pf" pid-file ifaces)
+ pid-file)))
+ ((dhcpcd)
+ ;; For dhcpcd, the utilized pid-file depends on the
+ ;; command-line arguments. If multiple interfaces are
+ ;; given, a different pid-file is returned. Hence, we
+ ;; consult dhcpcd itself to determine the pid-file.
+ (let* ((cmd (string-append #$package "/sbin/dhcpcd"))
+ (arg (cons* cmd "-b" ifaces)))
+ (cons arg
+ (let* ((pipe (string-join (append arg '("-P")) " "))
+ (port (open-input-pipe pipe))
+ (path (read-line port)))
+ (close-pipe port)
+ path))))
+ (else
+ (error (G_ "unknown 'package' value in dhcp-client-configuration")))))
+
+ (let ((pid-file (cdr exec-config))
+ (exec-cmd (car exec-config)))
+ (false-if-exception (delete-file pid-file))
+ (let ((pid (fork+exec-command exec-cmd)))
+ (and (zero? (cdr (waitpid pid)))
+ (read-pid-file pid-file))))))
(stop #~(make-kill-destructor))))))
(package
(warning (G_ "'dhcp-client' service now expects a \
@@ -377,10 +396,27 @@ (define dhcp-client-shepherd-service
(dhcp-client-configuration
(package package))))))
+(define (dhcp-client-account-service config)
+ (let ((package (dhcp-client-configuration-package config)))
+ ;; Contrary to other DHCP clients (e.g. dhclient), dhcpcd supports
+ ;; privilege separation. Hence, we need to create an account here.
+ (if (string=? "dhcpcd" (package-name package))
+ (list (user-group (name "dhcpcd") (system? #t))
+ (user-account
+ (name "dhcpcd")
+ (group "dhcpcd")
+ (system? #t)
+ (comment "dhcpcd daemon user")
+ (home-directory "/var/empty")
+ (shell "/run/current-system/profile/sbin/nologin")))
+ '())))
+
(define dhcp-client-service-type
(service-type (name 'dhcp-client)
(extensions
- (list (service-extension shepherd-root-service-type
+ (list (service-extension account-service-type
+ dhcp-client-account-service)
+ (service-extension shepherd-root-service-type
dhcp-client-shepherd-service)))
(default-value (dhcp-client-configuration))
(description "Run @command{dhcp}, a Dynamic Host Configuration
diff --git a/gnu/system.scm b/gnu/system.scm
index 3cd64a5c9f..a7676ec90e 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -917,7 +917,7 @@ (define %base-packages-interactive
(define %base-packages-networking
;; Default set of networking packages.
- (list inetutils isc-dhcp
+ (list inetutils
iproute
wget
;; wireless-tools is deprecated in favor of iw, but it's still what
S
S
Sören Tempel wrote on 24 Jan 20:11 +0100
Re: [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type
(name . Sergey Trofimov)(address . sarg@sarg.org.ru)(address . 68675@debbugs.gnu.org)
35004A3C7FWLY.2YGLY8PSSS0H5@8pit.net
Hi Sergey,

Sergey Trofimov <sarg@sarg.org.ru> wrote:
Toggle quote (2 lines)
> Basic functionality works for me - my laptop receives an address.

Great, thanks for your feedback!

Toggle quote (4 lines)
> Some things to fix:
> 1. 20-resolv.conf hook needs to be wrapped, it can't find `resolvconf`
> currently (from openresolv)

Good catch, it does need to be wrapped. However, it doesn't need
resolvconf (that's just an optional dependency) it does, however, need
coreutils and sed. I added a corresponding wrap-program invocation.

This should fix creation of /etc/resolv.conf via dhcpcd.

Toggle quote (2 lines)
> 2. %base-packages still contains isc-dhcp... why is it even needed?

I removed it from %base-packages together with the service changes.

Let me know if you find anything else!

Greetings
Sören
L
L
Ludovic Courtès wrote on 12 Feb 22:32 +0100
Re: [bug#68675] [PATCH v2] gnu: Add dhcpcd.
(address . soeren@soeren-tempel.net)(address . 68675@debbugs.gnu.org)
87y1bpbc6z.fsf@gnu.org
Hi,

soeren@soeren-tempel.net skribis:

Toggle quote (6 lines)
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> * gnu/packages/admin.scm (dhcpcd): new procedure.
>
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>

Overall LGTM modulo minor stylistic issues:

Toggle quote (7 lines)
> + #:configure-flags #~(list "--enable-ipv6"
> + "--enable-privsep"
> + "--privsepuser=dhcpcd"
> + (string-append "--dbdir=" "/var/db/dhcpcd")
> + (string-append "--rundir=" "/var/run/dhcpcd")
> + "CC=gcc")

Should be (string-append "CC=" #$(cc-for-target)).

Toggle quote (5 lines)
> + (add-before 'build 'setenv
> + (lambda _
> + (setenv "HOST_SH"
> + (string-append #$bash-minimal "/bin/sh"))))

Rather (setenv "HOST_SH" (which "sh")).

That way users can still override the shell used by the package.

Toggle quote (7 lines)
> + (add-after 'install 'wrap-hooks
> + (lambda* (#:key inputs outputs #:allow-other-keys)
> + (let* ((out (assoc-ref outputs "out"))
> + (libexec (string-append out "/libexec"))
> + (sed (assoc-ref inputs "sed"))
> + (coreutils (assoc-ref inputs "coreutils")))

Rather (search-input-file inputs "/bin/sed") and likewise for coreutils,
and then…

Toggle quote (6 lines)
> + (wrap-program (string-append libexec
> + "/dhcpcd-run-hooks")
> + `("PATH" ":" suffix
> + (,(string-append coreutils "/bin")
> + ,(string-append sed "/bin"))))))))))

… use (dirname sed) and (dirname ls), say, to get their /bin directory.

This is more robust than relying on the input label.

Ludo’.
L
L
Ludovic Courtès wrote on 12 Feb 22:41 +0100
Re: [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation.
(address . soeren@soeren-tempel.net)(address . 68675@debbugs.gnu.org)
87ttmdbbs9.fsf@gnu.org
soeren@soeren-tempel.net skribis:

Toggle quote (28 lines)
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> Prior to this commit, the isc-dhcp implementation was the only DHCP
> implementation supported by dhcp-client-shepherd-service. This is
> problematic as the ISC implementation has reached end-of-life in
> 2022(!). As a first step to migrate away from isc-dhcp, this commit
> adds support for dhcpcd to dhcp-client-shepherd-service. Currently,
> it has to be enabled explicitly via the package field of the
> dhcp-client-configuration. In the future, it is intended to become
> the default to migrate away from isc-dhcp.
>
> While at it, also remove isc-dhcp from %base-packages as it is no
> longer necessarily needed and it will be pulled in by the DHCP
> client service if required.
>
> See also: https://issues.guix.gnu.org/68619
>
> * gnu/services/networking.scm (dhcp-client-shepherd-service): Add
> support for the dhcpcd client implementation.
> * gnu/services/networking.scm (dhcp-client-account-service): New
> procedure.
> * gnu/services/networking.scm (dhcp-client-service-type): Add optional
> account-service-type extensions (needed for dhcpcd).
> * gnu/system.scm (%base-packages-networking): Remove isc-dhcp from
> %base-packages (will be pulled in by dhcp-client-shepherd-service).
>
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>

Much welcome improvement!

Some comments:

Toggle quote (6 lines)
> + (let* ((package (dhcp-client-configuration-package config))
> + (client-name (package-name package))
> + (requirement (dhcp-client-configuration-shepherd-requirement config))
> + (provision (dhcp-client-configuration-shepherd-provision config))
> + (interfaces (dhcp-client-configuration-interfaces config)))

Instead of calling ‘package-name’, which would prevent the use of
something other than a <package> record (such as an <inferior-package>)
or a package with a different name (like “dhcpcd-next”), what about
checking in the gexp which of /bin/dhclient and /bin/dhcpcd exists?

Toggle quote (6 lines)
> (start #~(lambda _
> - (define dhclient
> - (string-append #$package "/sbin/dhclient"))
> + (use-modules (ice-9 popen)
> + (ice-9 rdelim))

Instead of ‘use-modules’ within a function, which has ill-defined
semantics, add a ‘modules’ field to the ‘shepherd-service’ form.

Toggle quote (26 lines)
> + ;; Returns the execution configuration for the DHCP client
> + ;; selected by the package field of dhcp-client-configuration.
> + ;; The configuration is a pair of pidfile and execution command
> + ;; where the latter is a list.
> + (define exec-config
> + (case (string->symbol #$client-name)
> + ((isc-dhcp)
> + (let ((pid-file "/var/run/dhclient.pid"))
> + (cons
> + (cons* (string-append #$package "/sbin/dhclient")
> + "-nw" "-I" "-pf" pid-file ifaces)
> + pid-file)))
> + ((dhcpcd)
> + ;; For dhcpcd, the utilized pid-file depends on the
> + ;; command-line arguments. If multiple interfaces are
> + ;; given, a different pid-file is returned. Hence, we
> + ;; consult dhcpcd itself to determine the pid-file.
> + (let* ((cmd (string-append #$package "/sbin/dhcpcd"))
> + (arg (cons* cmd "-b" ifaces)))
> + (cons arg
> + (let* ((pipe (string-join (append arg '("-P")) " "))
> + (port (open-input-pipe pipe))
> + (path (read-line port)))
> + (close-pipe port)
> + path))))

That sounds quite complex. Surely there must be a way to force the name
of the PID file or to determine its name without running dhcpcd in a
pipe?

Toggle quote (3 lines)
> + (else
> + (error (G_ "unknown 'package' value in dhcp-client-configuration")))))

‘G_’ here is undefined, a ‘error’ doesn’t do what you perhaps think it
does (it throws to ‘misc-error’).

Instead, I would just print a message to the current error port (it’ll
be logged) and have ‘start’ return #f, meaning that the service failed
to start.
Toggle quote (8 lines)
> +++ b/gnu/system.scm
> @@ -917,7 +917,7 @@ (define %base-packages-interactive
>
> (define %base-packages-networking
> ;; Default set of networking packages.
> - (list inetutils isc-dhcp
> + (list inetutils

I would leave this change for a separate patch.

Or leave it out entirely: I find it useful to have a DHCP client at hand
just in case. Thoughts?

Could you send updated patches?

Thanks,
Ludo’.
S
S
soeren wrote on 13 Feb 13:50 +0100
[PATCH v3 1/2] gnu: Add dhcpcd.
(address . 68675@debbugs.gnu.org)
5c6f714f4802ec17bc247e701bcee82d54733005.1707828643.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

* gnu/packages/admin.scm (dhcpcd): new procedure.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
gnu/packages/admin.scm | 56 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)

Toggle diff (69 lines)
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index fcf05992d8..efd374fe5a 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -1608,6 +1608,62 @@ (define-public isc-dhcp
(license license:mpl2.0)
(properties '((cpe-name . "dhcp"))))))
+(define-public dhcpcd
+ (package
+ (name "dhcpcd")
+ (version "10.0.6")
+ (source
+ (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/NetworkConfiguration/dhcpcd")
+ (commit (string-append "v" version))))
+ (sha256
+ (base32 "07n7d5wsmy955i6l8rkcmxhgxjygj2cxgpw79id2hx9w41fbkl5l"))
+ (file-name (git-file-name name version))))
+ (inputs (list bash-minimal))
+ (native-inputs (list eudev))
+ (build-system gnu-build-system)
+ (arguments
+ (list
+ #:test-target "test"
+ #:configure-flags #~(list "--enable-ipv6"
+ "--enable-privsep"
+ "--privsepuser=dhcpcd"
+ (string-append "--dbdir=" "/var/db/dhcpcd")
+ (string-append "--rundir=" "/var/run/dhcpcd")
+ (string-append "CC=" #$(cc-for-target)))
+ #:phases #~(modify-phases %standard-phases
+ (add-after 'unpack 'do-not-create-dbdir
+ (lambda _
+ ;; Make sure that the Makefile doesn't attempt to create
+ ;; /var/db/dhcpcd for which it doesn't have permissions.
+ (substitute* "src/Makefile"
+ (("\\$\\{INSTALL\\} -m \\$\\{DBMODE\\} -d \\$\\{DESTDIR\\}\\$\\{DBDIR\\}")
+ ""))))
+ (add-before 'build 'setenv
+ (lambda _
+ (setenv "HOST_SH" (which "sh"))))
+ (add-after 'install 'wrap-hooks
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let* ((out (assoc-ref outputs "out"))
+ (libexec (string-append out "/libexec"))
+ (sed (search-input-file inputs "/bin/sed"))
+ (rm (search-input-file inputs "/bin/rm")))
+ (wrap-program (string-append libexec
+ "/dhcpcd-run-hooks")
+ `("PATH" ":" suffix
+ (,(dirname sed)
+ ,(dirname rm))))))))))
+ (home-page "https://roy.marples.name/projects/dhcpcd")
+ (synopsis "Feature-rich DHCP and DHCPv6 client")
+ (description
+ "Provides a DHCP and a DHCPv6 client. Additionally,
+dhcpcd is also an IPv4LL (aka ZeroConf) client. In layperson's terms,
+dhcpcd runs on your machine and silently configures your computer to work
+on the attached networks without trouble and mostly without configuration.")
+ (license license:bsd-2)))
+
(define-public radvd
(package
(name "radvd")

base-commit: 29c26a8d308286cf378ce9cfa3d73e3d1454263d
S
S
soeren wrote on 13 Feb 13:50 +0100
[PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation.
(address . 68675@debbugs.gnu.org)
bfd99f266c3fd749c6b24b386a7ac90dc89ce338.1707828643.git.soeren@soeren-tempel.net
From: Sören Tempel <soeren@soeren-tempel.net>

Prior to this commit, the isc-dhcp implementation was the only DHCP
implementation supported by dhcp-client-shepherd-service. This is
problematic as the ISC implementation has reached end-of-life in
2022(!). As a first step to migrate away from isc-dhcp, this commit
adds support for dhcpcd to dhcp-client-shepherd-service. Currently,
it has to be enabled explicitly via the package field of the
dhcp-client-configuration. In the future, it is intended to become
the default to migrate away from isc-dhcp.

While at it, also remove isc-dhcp from %base-packages as it is no
longer necessarily needed and it will be pulled in by the DHCP
client service if required.


* gnu/services/networking.scm (dhcp-client-shepherd-service): Add
support for the dhcpcd client implementation.
* gnu/services/networking.scm (dhcp-client-account-service): New
procedure.
* gnu/services/networking.scm (dhcp-client-service-type): Add optional
account-service-type extensions (needed for dhcpcd).
* gnu/system.scm (%base-packages-networking): Remove isc-dhcp from
%base-packages (will be pulled in by dhcp-client-shepherd-service).

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
gnu/services/networking.scm | 92 +++++++++++++++++++++++++++----------
1 file changed, 67 insertions(+), 25 deletions(-)

Toggle diff (126 lines)
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 495d049728..4e058e1880 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -316,25 +316,21 @@ (define-record-type* <dhcp-client-configuration>
(define dhcp-client-shepherd-service
(match-lambda
((? dhcp-client-configuration? config)
- (let ((package (dhcp-client-configuration-package config))
- (requirement (dhcp-client-configuration-shepherd-requirement config))
- (provision (dhcp-client-configuration-shepherd-provision config))
- (interfaces (dhcp-client-configuration-interfaces config))
- (pid-file "/var/run/dhclient.pid"))
+ (let* ((package (dhcp-client-configuration-package config))
+ (client-name (package-name package))
+ (requirement (dhcp-client-configuration-shepherd-requirement config))
+ (provision (dhcp-client-configuration-shepherd-provision config))
+ (interfaces (dhcp-client-configuration-interfaces config)))
(list (shepherd-service
(documentation "Set up networking via DHCP.")
(requirement `(user-processes udev ,@requirement))
(provision provision)
+ (modules `((ice-9 popen)
+ (ice-9 rdelim)
+ ,@%default-modules))
- ;; XXX: Running with '-nw' ("no wait") avoids blocking for a minute when
- ;; networking is unavailable, but also means that the interface is not up
- ;; yet when 'start' completes. To wait for the interface to be ready, one
- ;; should instead monitor udev events.
(start #~(lambda _
- (define dhclient
- (string-append #$package "/sbin/dhclient"))
-
- ;; When invoked without any arguments, 'dhclient' discovers all
+ ;; When invoked without any arguments, the client discovers all
;; non-loopback interfaces *that are up*. However, the relevant
;; interfaces are typically down at this point. Thus we perform
;; our own interface discovery here.
@@ -355,17 +351,46 @@ (define dhcp-client-shepherd-service
(_
#~'#$interfaces))))
- (false-if-exception (delete-file #$pid-file))
- (let ((pid (fork+exec-command
- ;; By default dhclient uses a
- ;; pre-standardization implementation of
- ;; DDNS, which is incompatable with
- ;; non-ISC DHCP servers; thus, pass '-I'.
- ;; <https://kb.isc.org/docs/aa-01091>.
- (cons* dhclient "-nw" "-I"
- "-pf" #$pid-file ifaces))))
- (and (zero? (cdr (waitpid pid)))
- (read-pid-file #$pid-file)))))
+ ;; Returns the execution configuration for the DHCP client
+ ;; selected by the package field of dhcp-client-configuration.
+ ;; The configuration is a pair of pidfile and execution command
+ ;; where the latter is a list.
+ (define exec-config
+ (case (string->symbol #$client-name)
+ ((isc-dhcp)
+ (let ((pid-file "/var/run/dhclient.pid"))
+ (cons
+ (cons* (string-append #$package "/sbin/dhclient")
+ "-nw" "-I" "-pf" pid-file ifaces)
+ pid-file)))
+ ((dhcpcd)
+ ;; For dhcpcd, the utilized pid-file depends on the
+ ;; command-line arguments. If multiple interfaces are
+ ;; given, a different pid-file is returned. Hence, we
+ ;; consult dhcpcd itself to determine the pid-file.
+ (let* ((cmd (string-append #$package "/sbin/dhcpcd"))
+ (arg (cons* cmd "-b" ifaces)))
+ (cons arg
+ (let* ((pipe (string-join (append arg '("-P")) " "))
+ (port (open-input-pipe pipe))
+ (path (read-line port)))
+ (close-pipe port)
+ path))))
+ (else
+ (display
+ "unknown 'package' value in dhcp-client-configuration"
+ (current-error-port))
+ (newline (current-error-port))
+ #f)))
+
+ (and
+ exec-config
+ (let ((pid-file (cdr exec-config))
+ (exec-cmd (car exec-config)))
+ (false-if-exception (delete-file pid-file))
+ (let ((pid (fork+exec-command exec-cmd)))
+ (and (zero? (cdr (waitpid pid)))
+ (read-pid-file pid-file)))))))
(stop #~(make-kill-destructor))))))
(package
(warning (G_ "'dhcp-client' service now expects a \
@@ -377,10 +402,27 @@ (define dhcp-client-shepherd-service
(dhcp-client-configuration
(package package))))))
+(define (dhcp-client-account-service config)
+ (let ((package (dhcp-client-configuration-package config)))
+ ;; Contrary to other DHCP clients (e.g. dhclient), dhcpcd supports
+ ;; privilege separation. Hence, we need to create an account here.
+ (if (string=? "dhcpcd" (package-name package))
+ (list (user-group (name "dhcpcd") (system? #t))
+ (user-account
+ (name "dhcpcd")
+ (group "dhcpcd")
+ (system? #t)
+ (comment "dhcpcd daemon user")
+ (home-directory "/var/empty")
+ (shell "/run/current-system/profile/sbin/nologin")))
+ '())))
+
(define dhcp-client-service-type
(service-type (name 'dhcp-client)
(extensions
- (list (service-extension shepherd-root-service-type
+ (list (service-extension account-service-type
+ dhcp-client-account-service)
+ (service-extension shepherd-root-service-type
dhcp-client-shepherd-service)))
(default-value (dhcp-client-configuration))
(description "Run @command{dhcp}, a Dynamic Host Configuration
S
S
Sören Tempel wrote on 13 Feb 13:52 +0100
Re: [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 68675@debbugs.gnu.org)
2W69OW2TSRJI1.2UCT83W2YH8FC@8pit.net
Hi Ludovic,

Thanks a lot for the feedback, really truly helpful! More comments below.

Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (5 lines)
> Instead of calling ‘package-name’, which would prevent the use of
> something other than a <package> record (such as an <inferior-package>)
> or a package with a different name (like “dhcpcd-next”), what about
> checking in the gexp which of /bin/dhclient and /bin/dhcpcd exists?

I think I haven't fully grasped G-Expressions yet. I do understand how
this would work inside the shepherd start gexp. However, I am not sure
how I would check if /bin/dhclient or /bin/dhcpcd exist in the package
within the dhcp-client-account-service since, at least as far as
I understand, the dhcp-client-account-service is itself not a gexp.

I think one could also argue that it might be preferable to check for
the package name as, just because a package provides /bin/dhclient,
doesn't necessarily mean that it is command-line compatible with the ISC
implementation. However, if you have any hints on updating the
account-service I would be willing to look into it.

Toggle quote (4 lines)
> That sounds quite complex. Surely there must be a way to force the name
> of the PID file or to determine its name without running dhcpcd in a
> pipe?

Unfortunately, I don't think there is any way to force the PID file
name. Furthermore, the pidfile path depends on the amount interfaces
specified. If there is only one interface, a single dhcpcd instance is
spawned and the pidfile refers to that. Otherwise, a management process
is started and the pidfile refers to the management process [1].

I think the best solution here would be to spawn dhcpcd as child process
of GNU shepherd and have shepherd supervise it that way. I have a service
doing that too, but its annoying to integrate with the existing dhclient
service [2]. Maybe that's something we can migrate to in the long run.

Toggle quote (3 lines)
> Or leave it out entirely: I find it useful to have a DHCP client at hand
> just in case. Thoughts?

I removed it, I agree that this change is best discussed separately.

Toggle quote (2 lines)
> Could you send updated patches?

Apart from the two comments I discussed in greater detail above, I hope
I implemented all of your feedback as requested in the v3 revision of
this patchset. Please let me know if I missed anything.

Greetings
Sören

L
L
Ludovic Courtès wrote on 28 Feb 21:46 +0100
(name . Sören Tempel)(address . soeren@soeren-tempel.net)(address . 68675@debbugs.gnu.org)
87v868fhbb.fsf@gnu.org
Hi Sören,

Apologies for the late reply.

Sören Tempel <soeren@soeren-tempel.net> skribis:

Toggle quote (12 lines)
> Ludovic Courtès <ludo@gnu.org> wrote:
>> Instead of calling ‘package-name’, which would prevent the use of
>> something other than a <package> record (such as an <inferior-package>)
>> or a package with a different name (like “dhcpcd-next”), what about
>> checking in the gexp which of /bin/dhclient and /bin/dhcpcd exists?
>
> I think I haven't fully grasped G-Expressions yet. I do understand how
> this would work inside the shepherd start gexp. However, I am not sure
> how I would check if /bin/dhclient or /bin/dhcpcd exist in the package
> within the dhcp-client-account-service since, at least as far as
> I understand, the dhcp-client-account-service is itself not a gexp.

I meant, within the ‘start’ method, something like:

(start #~(…
;; Check whether we’re dealing with ISC’s dhclient or dhcpcd.
(let ((type (if (file-exists?
(string-append #$package "/bin/dhclient"))
'isc-dhcp
'dhcpcd)))
(fork+exec-command …))))

Does that make sense?

Toggle quote (15 lines)
>> That sounds quite complex. Surely there must be a way to force the name
>> of the PID file or to determine its name without running dhcpcd in a
>> pipe?
>
> Unfortunately, I don't think there is any way to force the PID file
> name. Furthermore, the pidfile path depends on the amount interfaces
> specified. If there is only one interface, a single dhcpcd instance is
> spawned and the pidfile refers to that. Otherwise, a management process
> is started and the pidfile refers to the management process [1].
>
> I think the best solution here would be to spawn dhcpcd as child process
> of GNU shepherd and have shepherd supervise it that way. I have a service
> doing that too, but its annoying to integrate with the existing dhclient
> service [2]. Maybe that's something we can migrate to in the long run.

What do others do? I guess it’s not possible to have a systemd .service
file given those PID file semantics, is it?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 28 Feb 21:53 +0100
Re: [bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation.
(address . soeren@soeren-tempel.net)(address . 68675@debbugs.gnu.org)
87plwgfgyo.fsf@gnu.org
Hello,

soeren@soeren-tempel.net skribis:

Toggle quote (9 lines)
> * gnu/services/networking.scm (dhcp-client-shepherd-service): Add
> support for the dhcpcd client implementation.
> * gnu/services/networking.scm (dhcp-client-account-service): New
> procedure.
> * gnu/services/networking.scm (dhcp-client-service-type): Add optional
> account-service-type extensions (needed for dhcpcd).
> * gnu/system.scm (%base-packages-networking): Remove isc-dhcp from
> %base-packages (will be pulled in by dhcp-client-shepherd-service).

[...]

Toggle quote (33 lines)
> + ;; Returns the execution configuration for the DHCP client
> + ;; selected by the package field of dhcp-client-configuration.
> + ;; The configuration is a pair of pidfile and execution command
> + ;; where the latter is a list.
> + (define exec-config
> + (case (string->symbol #$client-name)
> + ((isc-dhcp)
> + (let ((pid-file "/var/run/dhclient.pid"))
> + (cons
> + (cons* (string-append #$package "/sbin/dhclient")
> + "-nw" "-I" "-pf" pid-file ifaces)
> + pid-file)))
> + ((dhcpcd)
> + ;; For dhcpcd, the utilized pid-file depends on the
> + ;; command-line arguments. If multiple interfaces are
> + ;; given, a different pid-file is returned. Hence, we
> + ;; consult dhcpcd itself to determine the pid-file.
> + (let* ((cmd (string-append #$package "/sbin/dhcpcd"))
> + (arg (cons* cmd "-b" ifaces)))
> + (cons arg
> + (let* ((pipe (string-join (append arg '("-P")) " "))
> + (port (open-input-pipe pipe))
> + (path (read-line port)))
> + (close-pipe port)
> + path))))
> + (else
> + (display
> + "unknown 'package' value in dhcp-client-configuration"
> + (current-error-port))
> + (newline (current-error-port))
> + #f)))
> +

I would very much like to avoid the ‘open-input-pipe’ dance here. Maybe
there’s a way to ask it to not fork, in which case we don’t need to wait
for a PID file? (I’ll give up if this really really can’t be avoided. :-))

Toggle quote (9 lines)
> + (and
> + exec-config
> + (let ((pid-file (cdr exec-config))
> + (exec-cmd (car exec-config)))
> + (false-if-exception (delete-file pid-file))
> + (let ((pid (fork+exec-command exec-cmd)))
> + (and (zero? (cdr (waitpid pid)))
> + (read-pid-file pid-file)))))))

Two notes: I would find it clearer to call ‘fork+exec-command’ above
instead of constructing ‘exec-config’.

Ideally, we’d use:

(start #~(if (file-exists? (string-append #$package "/bin/dhclient"))
(make-forkexec-constructor …) ;ISC version, with #:pid-file
(make-forkexec-constructor …))) ;dhcpcd, maybe without #:pid-file

Maybe that is too naive, but at least we should get as close as possible
to that, for instance by avoiding direct calls to ‘fork+exec-command’ +
‘waitpid’ + ‘read-pid-file’ as much as possible.

HTH!

Ludo’.
S
S
Sören Tempel wrote on 11 Mar 10:10 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 68675@debbugs.gnu.org)
36K6QAPI5E9CA.21QTTSML4N08U@8pit.net
Hi,

Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (2 lines)
> Apologies for the late reply.

No worries, I understand that you have a lot of other things to do :)

Toggle quote (4 lines)
> I would very much like to avoid the ‘open-input-pipe’ dance here. Maybe
> there’s a way to ask it to not fork, in which case we don’t need to wait
> for a PID file?

When using a PID file, AFAIK the only thing we can do is replicate the C
code that I referenced in my prior email for determining the PID file
name [1]. Is there a specific reason why you want to avoid using
open-input-pipe?

Toggle quote (3 lines)
> What do others do? I guess it’s not possible to have a systemd .service
> file given those PID file semantics, is it?

systemd and other service supervisor do not rely on PID files for
services supervision as they are inherently racy. Therefore, as
mentioned in my prior email, I also think it would be preferable to not
use a PID file for supervising either dhclient or dhcpcd.

I only ended up using a PID file here as dhclient already used one and I
wanted to keep the service changes as minimal as possible to ensure a
swift review since I believe this to be a security-critical issue (see
Guix issue #68619).

Toggle quote (9 lines)
> Two notes: I would find it clearer to call ‘fork+exec-command’ above
> instead of constructing ‘exec-config’.
>
> Ideally, we’d use:
>
> (start #~(if (file-exists? (string-append #$package "/bin/dhclient"))
> (make-forkexec-constructor …) ;ISC version, with #:pid-file
> (make-forkexec-constructor …))) ;dhcpcd, maybe without #:pid-file

If we separate the dhclient and the dhcpcd code like this, then it may
be worthwhile to have entirely separated services instead of combining
them both in a single service. This would also ease providing a
configuration for these services.

Toggle quote (4 lines)
> Maybe that is too naive, but at least we should get as close as possible
> to that, for instance by avoiding direct calls to ‘fork+exec-command’ +
> ‘waitpid’ + ‘read-pid-file’ as much as possible.

Ideally, I would not want to refactor existing service code as much in
this patchset. As mentioned above, I believe Guix using dhclient by
default (which has been EOL for 2 years) has security implications.
Therefore, I would want to migrate dhcp-client-service-type away from
dhclient to dhcpcd as soon as possible. As part of this migration, I
would strongly suggest a deprecation and removal of dhclient support
from dhcp-client-service-type. As soon as dhclient support is removed,
refactorings of dhcp-client-service-type can be conducted. Including a
removal of PID files for supervision, instead supervising dhcpcd by
spawning it as non-double-forking child process [2].

Greetings
Sören

?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 68675@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 68675
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch