GNU bug report logs - #68675
[PATCH] Support dhcpcd in dhcp-client-service-type

Previous Next

Full log

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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.