GNU bug report logs - #29117
Static networking vs Open vSwitch

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29117 in the body.
You can then email your comments to 29117 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: guix-patches <at> gnu.org
Subject: Static networking vs Open vSwitch
Date: Thu, 02 Nov 2017 20:13:05 +0100

[Message part 1 (text/plain, inline)]

Hello Guix,

When using static-networking-service with OpenvSwitch-managed
interfaces, shepherd will often attempt to start the static interface
configuration before the virtual interface has been created by OVS,
requiring manual intervention after boot.

This first patch allows arbitrary services to override 'requirement' in
<static-networking>; the second adds some tests for Open vSwitch,
including whether static-networking started successfully on a virtual
interface.

Marius Bakke (2):
  services: networking: Add a dependency override mechanism to
    <static-networking>.
  tests: networking: Add tests for OpenvSwitch.

 doc/guix.texi               |   5 +-
 gnu/services/networking.scm |  14 ++++--
 gnu/tests/networking.scm    | 109 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 121 insertions(+), 7 deletions(-)

-- 
2.15.0

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 29117 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: 29117 <at> debbugs.gnu.org
Cc: Marius Bakke <mbakke <at> fastmail.com>
Subject: [PATCH 1/2] services: networking: Add a dependency override mechanism
 to <static-networking>.
Date: Thu,  2 Nov 2017 20:14:45 +0100

* gnu/services/networking.scm (<static-networking>): Add 'requirement' record.
(static-networking-shepherd-service): Make UDEV the default requirement.
(static-networking-service): Expose 'requirement' parameter.
* doc/guix.texi (Networking Services): Document it.
---
 doc/guix.texi               |  5 ++++-
 gnu/services/networking.scm | 14 +++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 983d0e52e..c501c77eb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10288,9 +10288,12 @@ This is the type for statically-configured network interfaces.
 
 @deffn {Scheme Procedure} static-networking-service @var{interface} @var{ip} @
        [#:netmask #f] [#:gateway #f] [#:name-servers @code{'()}]
+       [#:requirement @code{'(udev)}]
 Return a service that starts @var{interface} with address @var{ip}.  If
 @var{netmask} is true, use it as the network mask.  If @var{gateway} is true,
-it must be a string specifying the default network gateway.
+it must be a string specifying the default network gateway.  @var{requirement}
+can be used to declare a dependency on another service before configuring the
+interface.
 
 This procedure can be called several times, one for each network
 interface of interest.  Behind the scenes what it does is extend
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index b0c23aafc..e7d9d9457 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -51,6 +51,7 @@
             static-networking-ip
             static-networking-netmask
             static-networking-gateway
+            static-networking-requirement
 
             static-networking-service
             static-networking-service-type
@@ -145,19 +146,21 @@ fe80::1%lo0 apps.facebook.com\n")
            (default #f))
   (provision static-networking-provision
              (default #f))
+  ;; Wait for udev to be up so that interfaces are actually usable.
+  (requirement static-networking-requirement
+               (default '(udev)))
   (name-servers static-networking-name-servers    ;FIXME: doesn't belong here
                 (default '())))
 
 (define static-networking-shepherd-service
   (match-lambda
     (($ <static-networking> interface ip netmask gateway provision
-                            name-servers)
+                            requirement name-servers)
      (let ((loopback? (and provision (memq 'loopback provision))))
        (shepherd-service
 
-        ;; Unless we're providing the loopback interface, wait for udev to be up
-        ;; and running so that INTERFACE is actually usable.
-        (requirement (if loopback? '() '(udev)))
+        ;; Force the loopback interface to start early.
+        (requirement (if loopback? '() requirement))
 
         (documentation
          "Bring up the networking interface using a static IP address.")
@@ -262,7 +265,7 @@ network interface.")))
 
 (define* (static-networking-service interface ip
                                     #:key
-                                    netmask gateway provision
+                                    netmask gateway provision requirement
                                     (name-servers '()))
   "Return a service that starts @var{interface} with address @var{ip}.  If
 @var{netmask} is true, use it as the network mask.  If @var{gateway} is true,
@@ -277,6 +280,7 @@ to handle."
                   (list (static-networking (interface interface) (ip ip)
                                            (netmask netmask) (gateway gateway)
                                            (provision provision)
+                                           (requirement requirement)
                                            (name-servers name-servers)))))
 
 (define dhcp-client-service-type
-- 
2.15.0

Message #11 received at 29117 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: 29117 <at> debbugs.gnu.org
Cc: Marius Bakke <mbakke <at> fastmail.com>
Subject: [PATCH 2/2] tests: networking: Add tests for OpenvSwitch.
Date: Thu,  2 Nov 2017 20:14:46 +0100

* gnu/tests/networking.scm (setup-openvswitch,
openvswitch-configuration-service, %openvswitch-os, run-openvswitch-test,
%test-openvswitch): New variables.
---
 gnu/tests/networking.scm | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/gnu/tests/networking.scm b/gnu/tests/networking.scm
index aeee105a1..750125ded 100644
--- a/gnu/tests/networking.scm
+++ b/gnu/tests/networking.scm
@@ -26,7 +26,9 @@
   #:use-module (guix store)
   #:use-module (guix monads)
   #:use-module (gnu packages bash)
-  #:export (%test-inetd))
+  #:use-module (gnu packages networking)
+  #:use-module (gnu services shepherd)
+  #:export (%test-inetd %test-openvswitch))
 
 (define %inetd-os
   ;; Operating system with 2 inetd services.
@@ -135,3 +137,108 @@ port 7, and a dict service on port 2628."
    (name "inetd")
    (description "Connect to a host with an INETD server.")
    (value (run-inetd-test))))
+
+
+;;;
+;;; Open vSwitch
+;;;
+
+(define setup-openvswitch
+  #~(let ((ovs-vsctl (lambda (str)
+                       (zero? (apply system*
+                                     #$(file-append openvswitch "/bin/ovs-vsctl")
+                                     (string-tokenize str)))))
+          (add-native-port (lambda (if)
+                             (string-append "--may-exist add-port br0 " if
+                                            " vlan_mode=native-untagged"
+                                            " -- set Interface " if
+                                            " type=internal"))))
+      (and (ovs-vsctl "--may-exist add-br br0")
+           ;; Connect eth0 as an "untagged" port (no VLANs).
+           (ovs-vsctl "--may-exist add-port br0 eth0 vlan_mode=native-untagged")
+           (ovs-vsctl (add-native-port "ovs0")))))
+
+(define openvswitch-configuration-service
+  (simple-service 'openvswitch-configuration shepherd-root-service-type
+                  (list (shepherd-service
+                         (provision '(openvswitch-configuration))
+                         (requirement '(vswitchd))
+                         (start #~(lambda ()
+                                    #$setup-openvswitch))
+                         (respawn? #f)))))
+
+(define %openvswitch-os
+  (simple-operating-system
+   (static-networking-service "ovs0" "10.1.1.1"
+                              #:netmask "255.255.255.252"
+                              #:requirement '(openvswitch-configuration))
+   (service openvswitch-service-type
+            (openvswitch-configuration
+             (package openvswitch)))
+   openvswitch-configuration-service))
+
+(define (run-openvswitch-test)
+  (define os
+    (marionette-operating-system %openvswitch-os
+                                 #:imported-modules '((gnu services herd))))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (gnu build marionette)
+                       (ice-9 popen)
+                       (ice-9 rdelim)
+                       (srfi srfi-64))
+
+          (define marionette
+            (make-marionette (list #$(virtual-machine os))))
+
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "openvswitch")
+
+          ;; Make sure the bridge is created.
+          (test-assert "br0 exists"
+            (marionette-eval
+             '(zero? (system* "ovs-vsctl" "br-exists" "br0"))
+             marionette))
+
+          ;; Make sure eth0 is connected to the bridge.
+          (test-equal "eth0 is connected to br0"
+            "br0"
+            (marionette-eval
+             '(begin
+                (use-modules (ice-9 popen) (ice-9 rdelim))
+                (let* ((port (open-pipe*
+                              OPEN_READ
+                              (string-append #$openvswitch "/bin/ovs-vsctl")
+                              "port-to-br" "eth0"))
+                       (output (read-line port)))
+                  (close-pipe port)
+                  output))
+             marionette))
+
+          ;; Make sure the virtual interface got a static IP.
+          (test-assert "networking has started on ovs0"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd)
+                             (srfi srfi-1))
+                (live-service-running
+                 (find (lambda (live)
+                         (memq 'networking-ovs0
+                               (live-service-provision live)))
+                       (current-services))))
+             marionette))
+
+          (test-end)
+          (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
+
+  (gexp->derivation "openvswitch-test" test))
+
+(define %test-openvswitch
+  (system-test
+   (name "openvswitch")
+   (description "Test a running OpenvSwitch configuration.")
+   (value (run-openvswitch-test))))
-- 
2.15.0

Message #16 received at 29117-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: 29117-done <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] services: networking: Add a dependency override
 mechanism to <static-networking>.
Date: Fri, 29 Dec 2017 14:12:35 +0100

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

> * gnu/services/networking.scm (<static-networking>): Add 'requirement' record.
> (static-networking-shepherd-service): Make UDEV the default requirement.
> (static-networking-service): Expose 'requirement' parameter.
> * doc/guix.texi (Networking Services): Document it.

I finally got around to properly test this patch on Real Systems.  There
was a problem with it: even though 'requirement' was added to the the
<static-networking> record with a default value, #:requirement became a
mandatory parameter when using (static-networking-service).

This fixup commit solves it:

[hmmm (text/x-patch, inline)]

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index dbfe15a31..c3ba0787c 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -147,9 +147,8 @@ fe80::1%lo0 apps.facebook.com\n")
            (default #f))
   (provision static-networking-provision
              (default #f))
-  ;; Wait for udev to be up so that interfaces are actually usable.
   (requirement static-networking-requirement
-               (default '(udev)))
+               (default '()))
   (name-servers static-networking-name-servers    ;FIXME: doesn't belong here
                 (default '())))
 
@@ -160,11 +159,9 @@ fe80::1%lo0 apps.facebook.com\n")
      (let ((loopback? (and provision (memq 'loopback provision))))
        (shepherd-service
 
-        ;; Force the loopback interface to start early.
-        (requirement (if loopback? '() requirement))
-
         (documentation
          "Bring up the networking interface using a static IP address.")
+        (requirement requirement)
         (provision (or provision
                        (list (symbol-append 'networking-
                                             (string->symbol interface)))))
@@ -266,7 +263,9 @@ network interface.")))
 
 (define* (static-networking-service interface ip
                                     #:key
-                                    netmask gateway provision requirement
+                                    netmask gateway provision
+                                    ;; Most interfaces require udev to be usable.
+                                    (requirement '(udev))
                                     (name-servers '()))
   "Return a service that starts @var{interface} with address @var{ip}.  If
 @var{netmask} is true, use it as the network mask.  If @var{gateway} is true,
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 7fc8f6aa7..f4681c804 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1986,6 +1986,7 @@ This service is not part of @var{%base-services}."
         (service static-networking-service-type
                  (list (static-networking (interface "lo")
                                           (ip "127.0.0.1")
+                                          (requirement '())
                                           (provision '(loopback)))))
         (syslog-service)
         (service urandom-seed-service-type)

[Message part 3 (text/plain, inline)]

<static-networking>[requirement] now defaults to the empty list, but
(static-network-service ...) adds udev if nothing is specified.

Since there were no complaints about the original patch, I assume the
intent was okay and went ahead and pushed this.

[signature.asc (application/pgp-signature, inline)]

Previous Next

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