[bug#71561] [PATCH] services: networking: Allow dhcp-client to use a config file

[bug#71561] [PATCH] services: networking: Allow dhcp-client to use a config file

[Richard Sent]

* gnu/services/networking (dhcp-client-configuration): Create
dhcp-client-configuration-config-file.
(dhcp-client-shepherd-service): Use the config file when invoking
dhclient if supplied.
* doc/guix.texi: Document it.

Change-Id: I286de4ddf59c5e606bf1fe0a7510570869e62b1a
---
Hi all.

This patch adds support for providing dhcp-client a configuration
file, which is a file of the form
https://man.archlinux.org/man/dhclient.conf.5. I believe this can be
used to resolve
https://lists.gnu.org/archive/html/help-guix/2024-06/msg00071.html
using the instructions at https://unix.stackexchange.com/a/399694,
although I haven't confirmed that. Either way, adding more flexibility
to the service is a good thing in my eyes.

 doc/guix.texi               |  3 +++
 gnu/services/networking.scm | 15 +++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0102fd0fad..ad67a436ea 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21035,6 +21035,9 @@ Networking Setup
 non-loopback interfaces that can be activated.  Otherwise the DHCP
 client listens only on the specified interfaces.
 
+@item @code{config-file} (default: @code{#f})
+The configuration file for the DHCP client.
+
 @item @code{shepherd-requirement} (default: @code{'()})
 @itemx @code{shepherd-provision} (default: @code{'(networking)})
 This option can be used to provide a list of symbols naming Shepherd services
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 378e117a86..12d8934e43 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -91,6 +91,7 @@ (define-module (gnu services networking)
             dhcp-client-configuration?
             dhcp-client-configuration-package
             dhcp-client-configuration-interfaces
+            dhcp-client-configuration-config-file
             dhcp-client-configuration-shepherd-provision
             dhcp-client-configuration-shepherd-requirement
 
@@ -319,6 +320,8 @@ (define-record-type* <dhcp-client-configuration>
                         (default '()))
   (shepherd-provision   dhcp-client-configuration-shepherd-provision
                         (default '(networking)))
+  (config-file dhcp-client-configuration-config-file
+               (default #f))
   (interfaces   dhcp-client-configuration-interfaces
                 (default 'all)))                  ;'all | list of strings
 
@@ -329,6 +332,7 @@ (define dhcp-client-shepherd-service
            (requirement (dhcp-client-configuration-shepherd-requirement config))
            (provision (dhcp-client-configuration-shepherd-provision config))
            (interfaces (dhcp-client-configuration-interfaces config))
+           (config-file (dhcp-client-configuration-config-file config))
            (pid-file "/var/run/dhclient.pid"))
        (list (shepherd-service
               (documentation "Set up networking via DHCP.")
@@ -364,6 +368,11 @@ (define dhcp-client-shepherd-service
                                        (_
                                         #~'#$interfaces))))
 
+                         (define config-file-args
+                           (if #$config-file
+                               (list "-cf" #$config-file)
+                               '()))
+
                          (false-if-exception (delete-file #$pid-file))
                          (let ((pid (fork+exec-command
                                      ;; By default dhclient uses a
@@ -371,8 +380,10 @@ (define dhcp-client-shepherd-service
                                      ;; 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))))
+                                     `(,dhclient "-nw" "-I"
+                                                 "-pf" ,#$pid-file
+                                                 ,@config-file-args
+                                                 ,@ifaces))))
                            (and (zero? (cdr (waitpid pid)))
                                 (read-pid-file #$pid-file)))))
               (stop #~(make-kill-destructor))))))

base-commit: 7770e4140cddd5dc9c9879cb2bfcbd6cd4de59a0
-- 
2.45.1

[bug#71561] [PATCH] services: networking: Allow dhcp-client to use a config file

[Maxim Cournoyer]

Hi Richard,

Richard Sent <richard@freakingpenguin.com> writes:

> * gnu/services/networking (dhcp-client-configuration): Create

Nitpick: The file name should be complete, including the extension.
Pressing 'C' on the diff's hunk while authoring a commit message in
Magit (Emacs) creates a ChangeLog entry automatically, if that helps.

> dhcp-client-configuration-config-file.

I'd perhaps reword this to:

(dhcp-client-configuration) [config-file]: New field.
(dhcp-client-configuration-config-file): New accessor.

> (dhcp-client-shepherd-service): Use the config file when invoking
> dhclient if supplied.
> * doc/guix.texi: Document it.
>
> Change-Id: I286de4ddf59c5e606bf1fe0a7510570869e62b1a
> ---
> Hi all.
>
> This patch adds support for providing dhcp-client a configuration
> file, which is a file of the form
> https://man.archlinux.org/man/dhclient.conf.5. I believe this can be
> used to resolve
> https://lists.gnu.org/archive/html/help-guix/2024-06/msg00071.html
> using the instructions at https://unix.stackexchange.com/a/399694,
> although I haven't confirmed that. Either way, adding more flexibility
> to the service is a good thing in my eyes.

I think it's a good thing to have the 'escape hatch' implemented by many
services!

>  doc/guix.texi               |  3 +++
>  gnu/services/networking.scm | 15 +++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 0102fd0fad..ad67a436ea 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -21035,6 +21035,9 @@ Networking Setup
>  non-loopback interfaces that can be activated.  Otherwise the DHCP
>  client listens only on the specified interfaces.
>  
> +@item @code{config-file} (default: @code{#f})
> +The configuration file for the DHCP client.
> +
>  @item @code{shepherd-requirement} (default: @code{'()})
>  @itemx @code{shepherd-provision} (default: @code{'(networking)})
>  This option can be used to provide a list of symbols naming Shepherd services
> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
> index 378e117a86..12d8934e43 100644
> --- a/gnu/services/networking.scm
> +++ b/gnu/services/networking.scm
> @@ -91,6 +91,7 @@ (define-module (gnu services networking)
>              dhcp-client-configuration?
>              dhcp-client-configuration-package
>              dhcp-client-configuration-interfaces
> +            dhcp-client-configuration-config-file
>              dhcp-client-configuration-shepherd-provision
>              dhcp-client-configuration-shepherd-requirement
>  
> @@ -319,6 +320,8 @@ (define-record-type* <dhcp-client-configuration>
>                          (default '()))
>    (shepherd-provision   dhcp-client-configuration-shepherd-provision
>                          (default '(networking)))
> +  (config-file dhcp-client-configuration-config-file
> +               (default #f))
>    (interfaces   dhcp-client-configuration-interfaces
>                  (default 'all)))                  ;'all | list of strings
>  
> @@ -329,6 +332,7 @@ (define dhcp-client-shepherd-service
>             (requirement (dhcp-client-configuration-shepherd-requirement config))
>             (provision (dhcp-client-configuration-shepherd-provision config))
>             (interfaces (dhcp-client-configuration-interfaces config))
> +           (config-file (dhcp-client-configuration-config-file config))
>             (pid-file "/var/run/dhclient.pid"))
>         (list (shepherd-service
>                (documentation "Set up networking via DHCP.")
> @@ -364,6 +368,11 @@ (define dhcp-client-shepherd-service
>                                         (_
>                                          #~'#$interfaces))))
>  
> +                         (define config-file-args
> +                           (if #$config-file
> +                               (list "-cf" #$config-file)
> +                               '()))
> +
>                           (false-if-exception (delete-file #$pid-file))
>                           (let ((pid (fork+exec-command
>                                       ;; By default dhclient uses a
> @@ -371,8 +380,10 @@ (define dhcp-client-shepherd-service
>                                       ;; 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))))
> +                                     `(,dhclient "-nw" "-I"
> +                                                 "-pf" ,#$pid-file
> +                                                 ,@config-file-args
> +                                                 ,@ifaces))))
>                             (and (zero? (cdr (waitpid pid)))
>                                  (read-pid-file #$pid-file)))))
>                (stop #~(make-kill-destructor))))))

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

Thanks,
Maxim

[bug#71561] [PATCH] services: networking: Allow dhcp-client to use a config file

[Richard Sent]

Hi Maxim,

> Nitpick: The file name should be complete, including the extension.
> Pressing 'C' on the diff's hunk while authoring a commit message in
> Magit (Emacs) creates a ChangeLog entry automatically, if that helps.

Huh, I didn't know that. Good to know, thanks.

> I'd perhaps reword this to:
>
> (dhcp-client-configuration) [config-file]: New field.
> (dhcp-client-configuration-config-file): New accessor.

I believe these comments have been addressed in v2. I also added another
commit to prepend "ISC" in references to DHCP client. This way it should
be clearer this service is specific to the ISC's implementation.

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

[bug#71561] [PATCH 1/2] services: networking: Allow dhcp-client to use a config file

[Richard Sent]

* gnu/services/networking.scm (dhcp-client-configuration) [config-file]: New
field.
(dhcp-client-configuration-config-file): New accessor.
(dhcp-client-shepherd-service): Use the config file when invoking
dhclient if supplied.
* doc/guix.texi: Document it.

Change-Id: I286de4ddf59c5e606bf1fe0a7510570869e62b1a
---
 doc/guix.texi               |  3 +++
 gnu/services/networking.scm | 15 +++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0102fd0fad..ad67a436ea 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21035,6 +21035,9 @@ Networking Setup
 non-loopback interfaces that can be activated.  Otherwise the DHCP
 client listens only on the specified interfaces.
 
+@item @code{config-file} (default: @code{#f})
+The configuration file for the DHCP client.
+
 @item @code{shepherd-requirement} (default: @code{'()})
 @itemx @code{shepherd-provision} (default: @code{'(networking)})
 This option can be used to provide a list of symbols naming Shepherd services
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 378e117a86..12d8934e43 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -91,6 +91,7 @@ (define-module (gnu services networking)
             dhcp-client-configuration?
             dhcp-client-configuration-package
             dhcp-client-configuration-interfaces
+            dhcp-client-configuration-config-file
             dhcp-client-configuration-shepherd-provision
             dhcp-client-configuration-shepherd-requirement
 
@@ -319,6 +320,8 @@ (define-record-type* <dhcp-client-configuration>
                         (default '()))
   (shepherd-provision   dhcp-client-configuration-shepherd-provision
                         (default '(networking)))
+  (config-file dhcp-client-configuration-config-file
+               (default #f))
   (interfaces   dhcp-client-configuration-interfaces
                 (default 'all)))                  ;'all | list of strings
 
@@ -329,6 +332,7 @@ (define dhcp-client-shepherd-service
            (requirement (dhcp-client-configuration-shepherd-requirement config))
            (provision (dhcp-client-configuration-shepherd-provision config))
            (interfaces (dhcp-client-configuration-interfaces config))
+           (config-file (dhcp-client-configuration-config-file config))
            (pid-file "/var/run/dhclient.pid"))
        (list (shepherd-service
               (documentation "Set up networking via DHCP.")
@@ -364,6 +368,11 @@ (define dhcp-client-shepherd-service
                                        (_
                                         #~'#$interfaces))))
 
+                         (define config-file-args
+                           (if #$config-file
+                               (list "-cf" #$config-file)
+                               '()))
+
                          (false-if-exception (delete-file #$pid-file))
                          (let ((pid (fork+exec-command
                                      ;; By default dhclient uses a
@@ -371,8 +380,10 @@ (define dhcp-client-shepherd-service
                                      ;; 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))))
+                                     `(,dhclient "-nw" "-I"
+                                                 "-pf" ,#$pid-file
+                                                 ,@config-file-args
+                                                 ,@ifaces))))
                            (and (zero? (cdr (waitpid pid)))
                                 (read-pid-file #$pid-file)))))
               (stop #~(make-kill-destructor))))))

base-commit: b993f4735d41e690dbafb8ee2e17fce996a8cf20
-- 
2.45.1

[bug#71561] [PATCH 2/2] doc: Prepend ISC to DHCP client references.

[Richard Sent]

This makes it clearer that this service uses the ISC's dhclient implementation
and does not support DHCP clients that have different command line interfaces.

* doc/guix.texi (Networking Setup): Prepend ISC to all DHCP client references.

Change-Id: I750ef2ffb4b23445c4a2b97aaa44eba56458f430
---
 doc/guix.texi | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ad67a436ea..8dd3bee5ca 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21016,27 +21016,27 @@ Networking Setup
 
 @cindex DHCP, networking service
 @defvar dhcp-client-service-type
-This is the type of services that run @var{dhcp}, a Dynamic Host Configuration
-Protocol (DHCP) client.
+This is the type of services that run @var{dhclient}, the ISC Dynamic
+Host Configuration Protocol (DHCP) client.
 @end defvar
 
 @deftp {Data Type} dhcp-client-configuration
-Data type representing the configuration of the DHCP client service.
+Data type representing the configuration of the ISC DHCP client service.
 
 @table @asis
 @item @code{package} (default: @code{isc-dhcp})
-DHCP client package to use.
+The ISC DHCP client package to use.
 
 @item @code{interfaces} (default: @code{'all})
-Either @code{'all} or the list of interface names that the DHCP client
-should listen on---e.g., @code{'("eno1")}.
+Either @code{'all} or the list of interface names that the ISC DHCP
+client should listen on---e.g., @code{'("eno1")}.
 
-When set to @code{'all}, the DHCP client listens on all the available
-non-loopback interfaces that can be activated.  Otherwise the DHCP
-client listens only on the specified interfaces.
+When set to @code{'all}, the ISC DHCP client listens on all the
+available non-loopback interfaces that can be activated.  Otherwise the
+ISC DHCP client listens only on the specified interfaces.
 
 @item @code{config-file} (default: @code{#f})
-The configuration file for the DHCP client.
+The configuration file for the ISC DHCP client.
 
 @item @code{shepherd-requirement} (default: @code{'()})
 @itemx @code{shepherd-provision} (default: @code{'(networking)})
@@ -21047,7 +21047,7 @@ Networking Setup
 
 Likewise, @code{shepherd-provision} is a list of Shepherd service names
 (symbols) provided by this service.  You might want to change the
-default value if you intend to run several DHCP clients, only one of
+default value if you intend to run several ISC DHCP clients, only one of
 which provides the @code{networking} Shepherd service.
 @end table
 @end deftp
-- 
2.45.1

[bug#71561] [PATCH 2/2] doc: Prepend ISC to DHCP client references.

[Maxim Cournoyer]

Richard Sent <richard@freakingpenguin.com> writes:

> This makes it clearer that this service uses the ISC's dhclient implementation
> and does not support DHCP clients that have different command line interfaces.
>
> * doc/guix.texi (Networking Setup): Prepend ISC to all DHCP client references.
>
> Change-Id: I750ef2ffb4b23445c4a2b97aaa44eba56458f430

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

-- 
Thanks,
Maxim

[bug#71561] [PATCH 1/2] services: networking: Allow dhcp-client to use a config file

[Maxim Cournoyer]

Richard Sent <richard@freakingpenguin.com> writes:

> * gnu/services/networking.scm (dhcp-client-configuration) [config-file]: New
> field.
> (dhcp-client-configuration-config-file): New accessor.
> (dhcp-client-shepherd-service): Use the config file when invoking
> dhclient if supplied.
> * doc/guix.texi: Document it.
>
> Change-Id: I286de4ddf59c5e606bf1fe0a7510570869e62b1a

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

-- 
Thanks,
Maxim

bug#71561: [PATCH] services: networking: Allow dhcp-client to use a config file

[Maxim Cournoyer]

Hi,

Richard Sent <richard@freakingpenguin.com> writes:

> * gnu/services/networking (dhcp-client-configuration): Create
> dhcp-client-configuration-config-file.
> (dhcp-client-shepherd-service): Use the config file when invoking
> dhclient if supplied.
> * doc/guix.texi: Document it.
>
> Change-Id: I286de4ddf59c5e606bf1fe0a7510570869e62b1a

Applied, thanks.

-- 
Maxim