Package: guix-patches;
Reported by: soeren <at> soeren-tempel.net
Date: Tue, 23 Jan 2024 16:14:01 UTC
Severity: normal
Tags: patch
To reply to this bug, email your comments to 68675 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 23 Jan 2024 16:14:02 GMT) Full text and rfc822 format available.soeren <at> soeren-tempel.net:guix-patches <at> gnu.org.
(Tue, 23 Jan 2024 16:14:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: guix-patches <at> gnu.org Subject: [PATCH] Support dhcpcd in dhcp-client-service-type Date: Tue, 23 Jan 2024 17:12:43 +0100
From: Sören Tempel <soeren <at> 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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 23 Jan 2024 16:16:02 GMT) Full text and rfc822 format available.Message #8 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH] gnu: Add dhcpcd. Date: Tue, 23 Jan 2024 17:14:55 +0100
From: Sören Tempel <soeren <at> soeren-tempel.net>
* gnu/packages/admin.scm (dhcpcd): new procedure.
Signed-off-by: Sören Tempel <soeren <at> soeren-tempel.net>
---
gnu/packages/admin.scm | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
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")
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 23 Jan 2024 16:16:02 GMT) Full text and rfc822 format available.Message #11 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH] services: dhcp: Support the dhcpcd implementation. Date: Tue, 23 Jan 2024 17:14:56 +0100
From: Sören Tempel <soeren <at> 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.
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).
Signed-off-by: Sören Tempel <soeren <at> soeren-tempel.net>
---
gnu/services/networking.scm | 84 ++++++++++++++++++++++++++-----------
1 file changed, 60 insertions(+), 24 deletions(-)
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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 23 Jan 2024 19:53:02 GMT) Full text and rfc822 format available.Message #14 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Sergey Trofimov <sarg <at> sarg.org.ru> To: soeren <at> soeren-tempel.net Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type Date: Tue, 23 Jan 2024 20:52:43 +0100
soeren <at> soeren-tempel.net writes: > 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?
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Wed, 24 Jan 2024 19:08:01 GMT) Full text and rfc822 format available.Message #17 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH v2] gnu: Add dhcpcd. Date: Wed, 24 Jan 2024 20:05:11 +0100
From: Sören Tempel <soeren <at> soeren-tempel.net>
* gnu/packages/admin.scm (dhcpcd): new procedure.
Signed-off-by: Sören Tempel <soeren <at> 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(+)
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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Wed, 24 Jan 2024 19:08:01 GMT) Full text and rfc822 format available.Message #20 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH v2] services: dhcp: Support the dhcpcd implementation. Date: Wed, 24 Jan 2024 20:05:13 +0100
From: Sören Tempel <soeren <at> 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 <at> 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(-)
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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Wed, 24 Jan 2024 19:12:03 GMT) Full text and rfc822 format available.Message #23 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Sören Tempel <soeren <at> soeren-tempel.net> To: Sergey Trofimov <sarg <at> sarg.org.ru> Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH] Support dhcpcd in dhcp-client-service-type Date: Wed, 24 Jan 2024 20:11:46 +0100
Hi Sergey, Sergey Trofimov <sarg <at> sarg.org.ru> wrote: > Basic functionality works for me - my laptop receives an address. Great, thanks for your feedback! > 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. > 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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Mon, 12 Feb 2024 21:41:02 GMT) Full text and rfc822 format available.Message #26 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: soeren <at> soeren-tempel.net Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v2] gnu: Add dhcpcd. Date: Mon, 12 Feb 2024 22:32:52 +0100
Hi,
soeren <at> soeren-tempel.net skribis:
> From: Sören Tempel <soeren <at> soeren-tempel.net>
>
> * gnu/packages/admin.scm (dhcpcd): new procedure.
>
> Signed-off-by: Sören Tempel <soeren <at> soeren-tempel.net>
Overall LGTM modulo minor stylistic issues:
> + #: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)).
> + (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.
> + (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…
> + (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’.
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Mon, 12 Feb 2024 21:43:02 GMT) Full text and rfc822 format available.Message #29 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: soeren <at> soeren-tempel.net Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation. Date: Mon, 12 Feb 2024 22:41:42 +0100
soeren <at> soeren-tempel.net skribis:
> From: Sören Tempel <soeren <at> 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 <at> soeren-tempel.net>
Much welcome improvement!
Some comments:
> + (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?
> (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.
> + ;; 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?
> + (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.
> +++ 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’.
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 13 Feb 2024 12:52:02 GMT) Full text and rfc822 format available.Message #32 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH v3 1/2] gnu: Add dhcpcd. Date: Tue, 13 Feb 2024 13:50:42 +0100
From: Sören Tempel <soeren <at> soeren-tempel.net>
* gnu/packages/admin.scm (dhcpcd): new procedure.
Signed-off-by: Sören Tempel <soeren <at> soeren-tempel.net>
---
gnu/packages/admin.scm | 56 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 13 Feb 2024 12:52:02 GMT) Full text and rfc822 format available.Message #35 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: soeren <at> soeren-tempel.net To: 68675 <at> debbugs.gnu.org Subject: [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation. Date: Tue, 13 Feb 2024 13:50:43 +0100
From: Sören Tempel <soeren <at> 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 <at> soeren-tempel.net>
---
gnu/services/networking.scm | 92 +++++++++++++++++++++++++++----------
1 file changed, 67 insertions(+), 25 deletions(-)
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
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Tue, 13 Feb 2024 13:00:02 GMT) Full text and rfc822 format available.Message #38 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Sören Tempel <soeren <at> soeren-tempel.net> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation. Date: Tue, 13 Feb 2024 13:52:15 +0100
Hi Ludovic, Thanks a lot for the feedback, really truly helpful! More comments below. Ludovic Courtès <ludo <at> 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 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. > 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. > 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. > 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 [1]: https://github.com/NetworkConfiguration/dhcpcd/blob/v10.0.6/src/dhcpcd.c#L2144 [2]: https://github.com/nmeum/guix-channel/blob/b1b80697a9d35ca015ce56ccf3031f91f2bf554f/src/nmeum/services/networking.scm#L130-L132
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Wed, 28 Feb 2024 20:48:02 GMT) Full text and rfc822 format available.Message #41 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: Sören Tempel <soeren <at> soeren-tempel.net> Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v2] services: dhcp: Support the dhcpcd implementation. Date: Wed, 28 Feb 2024 21:46:16 +0100
Hi Sören,
Apologies for the late reply.
Sören Tempel <soeren <at> soeren-tempel.net> skribis:
> Ludovic Courtès <ludo <at> 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?
>> 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’.
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Wed, 28 Feb 2024 20:56:01 GMT) Full text and rfc822 format available.Message #44 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: soeren <at> soeren-tempel.net Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation. Date: Wed, 28 Feb 2024 21:53:51 +0100
Hello,
soeren <at> soeren-tempel.net skribis:
> * 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).
[...]
> + ;; 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. :-))
> + (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’.
guix-patches <at> gnu.org:bug#68675; Package guix-patches.
(Mon, 11 Mar 2024 09:12:02 GMT) Full text and rfc822 format available.Message #47 received at 68675 <at> debbugs.gnu.org (full text, mbox):
From: Sören Tempel <soeren <at> soeren-tempel.net> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 68675 <at> debbugs.gnu.org Subject: Re: [bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation. Date: Mon, 11 Mar 2024 10:10:24 +0100
Hi, Ludovic Courtès <ludo <at> gnu.org> wrote: > Apologies for the late reply. No worries, I understand that you have a lot of other things to do :) > 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? > 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). > 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. > 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 [1]: https://github.com/NetworkConfiguration/dhcpcd/blob/v10.0.6/src/dhcpcd.c#L2144 [2]: https://github.com/nmeum/guix-channel/blob/b1b80697a9d35ca015ce56ccf3031f91f2bf554f/src/nmeum/services/networking.scm#L209-L211
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.