Subject: [bug#64616] [PATCH 0/1] services: static-networking: Add support for bonds and vlans

From: Ludovic Courtès <[email-masked]>
Date: Sat, 12 Aug 2023 22:28:59 +0200

Hi Alexey,

Alexey Abramov <[email-masked]> skribis:

> * gnu/services/base.scm (<network-link-by-macaddress>,
> <network-link-by-name>): Provide records to match *existing*
> interfaces and amend them.
> * gnu/services/base.scm (network-set-up/linux,
> network-tear-down/linux): Add support to change settings of existing
> interfaces. Move address cleanup above links cleanup.
> * doc/guix.texi (Networking Setup): Document it.
> * gnu/tests/networking.scm (run-static-networking-advanced-test): Add tests

Sounds like a great addition!

Not being a networking expert, I’d like to have someone else comment on
it (Cc’ing Julien in case they’re around), but I can make some
preliminary comments:

>  @deftp {Data Type} network-link
>  Data type for a network link (@pxref{Link,,, guile-netlink,
> -Guile-Netlink Manual}).
> +Guile-Netlink Manual}).  A new interface with settings, specified in
> +arguments will be created.

I don’t understand this sentence, especially since creating a
<network-link> record will not create a new interface.

> +@deftp {Data Type} network-link-by-macaddress
> +Data type for a network link with a specific MAC address. Arguments will
> +be applied to existing link matching the MAC.
> +
> +@table @code
> +@item macaddress
> +The MAC address to match a link.

[...]

> +@deftp {Data Type} network-link-by-name
> +Data type for a network link with a specific name. Arguments will be
> +applied to existing link mathing the name.
> +
> +@table @code
> +@item name
> +The name of the link.

[...]

> +(static-networking
> + (links (list (network-link
> +               (name "bond0")
> +               (type "bond")
> +               (arguments '((mode . "802.3ad")
> +                            (miimon . 100)
> +                            (lacp-active . "on")
> +                            (lacp-rate . "fast"))))
> +
> +              (network-link-by-macaddress
> +               (macaddress "98:11:22:33:44:55")
> +               (arguments '((master . "bond0"))))
> +
> +              (network-link-by-macaddress
> +               (macaddress "98:11:22:33:44:56")
> +               (arguments '((master . "bond0"))))

My first reaction is that a “network link matched by MAC address” is
still “a network link”.  IOW, I would find it more natural to have a
single data type; it would also mirror the data types used by the
RTnetlink layer.

To do that, what would you think of keeping just the <network-link>
record, but adding two new fields: ‘for-mac-address’ and
‘for-device’?

(As an aside, please don’t abbreviate; so ‘mac-address’ rather than
‘macaddress’.)

> +Here is another example for more advance configuration with bonds and
> +vlans.  The following snippet will create a bond out of two interfaces,
> +rename the slaves and create a vlan 1055 on top of it.

Could you (1) explain in one or two sentences what bonds and VLANs are,
ideally with a cross-reference to learn more about it, and (2) explain
the example in a bit more detail?  I would also encourage you to use the
“upstream” and “downstream” rather than “master” and “slave”, due to
their obvious connotation, though I realize that Guile-Netlink and
presumably Linux/RTnetlink itself use that terminology.

> +(define (run-static-networking-advanced-test vm)

This is awesome!

Thank you!

Ludo’.