Merge pull request #53033 from netixx/openvswitch-improved-systemd

openvswitch: better integration with systemd

nixos/modules/services/networking/dhcpcd.nix

@@ -19,7 +19,7 @@ let
     map (i: i.name) (filter (i: if i.useDHCP != null then !i.useDHCP else i.ipv4.addresses != [ ]) interfaces)
     ++ mapAttrsToList (i: _: i) config.networking.sits
     ++ concatLists (attrValues (mapAttrs (n: v: v.interfaces) config.networking.bridges))
-    ++ concatLists (attrValues (mapAttrs (n: v: v.interfaces) config.networking.vswitches))
+    ++ flatten (concatMap (i: attrNames (filterAttrs (_: config: config.type != "internal") i.interfaces)) (attrValues config.networking.vswitches))
     ++ concatLists (attrValues (mapAttrs (n: v: v.interfaces) config.networking.bonds))
     ++ config.networking.dhcpcd.denyInterfaces;
 

nixos/modules/tasks/network-interfaces-scripted.nix

@@ -10,7 +10,7 @@ let
 
   slaves = concatMap (i: i.interfaces) (attrValues cfg.bonds)
     ++ concatMap (i: i.interfaces) (attrValues cfg.bridges)
-    ++ concatMap (i: i.interfaces) (attrValues cfg.vswitches)
+    ++ concatMap (i: attrNames (filterAttrs (_: config: config.type != "internal") i.interfaces)) (attrValues cfg.vswitches)
     ++ concatMap (i: [i.interface]) (attrValues cfg.macvlans)
     ++ concatMap (i: [i.interface]) (attrValues cfg.vlans);
 
@@ -336,34 +336,47 @@ let
 
         createVswitchDevice = n: v: nameValuePair "${n}-netdev"
           (let
-            deps = concatLists (map deviceDependency v.interfaces);
+            deps = concatLists (map deviceDependency (attrNames (filterAttrs (_: config: config.type != "internal") v.interfaces)));
+            internalConfigs = concatMap (i: ["network-link-${i}.service" "network-addresses-${i}.service"]) (attrNames (filterAttrs (_: config: config.type == "internal") v.interfaces));
             ofRules = pkgs.writeText "vswitch-${n}-openFlowRules" v.openFlowRules;
           in
           { description = "Open vSwitch Interface ${n}";
-            wantedBy = [ "network-setup.service" "vswitchd.service" ] ++ deps;
+            wantedBy = [ "network-setup.service" (subsystemDevice n) ] ++ internalConfigs;
-            bindsTo =  [ "vswitchd.service" (subsystemDevice n) ] ++ deps;
+            # before = [ "network-setup.service" ];
-            partOf = [ "network-setup.service" "vswitchd.service" ];
+            # should work without internalConfigs dependencies because address/link configuration depends
-            after = [ "network-pre.target" "vswitchd.service" ] ++ deps;
+            # on the device, which is created by ovs-vswitchd with type=internal, but it does not...
-            before = [ "network-setup.service" ];
+            before = [ "network-setup.service" ] ++ internalConfigs;
+            partOf = [ "network-setup.service" ]; # shutdown the bridge when network is shutdown
+            bindsTo = [ "ovs-vswitchd.service" ]; # requires ovs-vswitchd to be alive at all times
+            after = [ "network-pre.target" "ovs-vswitchd.service" ] ++ deps; # start switch after physical interfaces and vswitch daemon
+            wants = deps; # if one or more interface fails, the switch should continue to run
             serviceConfig.Type = "oneshot";
             serviceConfig.RemainAfterExit = true;
             path = [ pkgs.iproute config.virtualisation.vswitch.package ];
+            preStart = ''
+              echo "Resetting Open vSwitch ${n}..."
+              ovs-vsctl --if-exists del-br ${n} -- add-br ${n} \
+                        -- set bridge ${n} protocols=${concatStringsSep "," v.supportedOpenFlowVersions}
+            '';
             script = ''
-              echo "Removing old Open vSwitch ${n}..."
+              echo "Configuring Open vSwitch ${n}..."
-              ovs-vsctl --if-exists del-br ${n}
+              ovs-vsctl ${concatStrings (mapAttrsToList (name: config: " -- add-port ${n} ${name}" + optionalString (config.vlan != null) " tag=${toString config.vlan}") v.interfaces)} \
-
+                ${concatStrings (mapAttrsToList (name: config: optionalString (config.type != null) " -- set interface ${name} type=${config.type}") v.interfaces)} \
-              echo "Adding Open vSwitch ${n}..."
-              ovs-vsctl -- add-br ${n} ${concatMapStrings (i: " -- add-port ${n} ${i}") v.interfaces} \
                 ${concatMapStrings (x: " -- set-controller ${n} " + x)  v.controllers} \
                 ${concatMapStrings (x: " -- " + x) (splitString "\n" v.extraOvsctlCmds)}
 
+
               echo "Adding OpenFlow rules for Open vSwitch ${n}..."
-              ovs-ofctl add-flows ${n} ${ofRules}
+              ovs-ofctl --protocols=${v.openFlowVersion} add-flows ${n} ${ofRules}
             '';
             postStop = ''
+              echo "Cleaning Open vSwitch ${n}"
+              echo "Shuting down internal ${n} interface"
               ip link set ${n} down || true
-              ovs-ofctl del-flows ${n} || true
+              echo "Deleting flows for ${n}"
-              ovs-vsctl --if-exists del-br ${n}
+              ovs-ofctl --protocols=${v.openFlowVersion} del-flows ${n} || true
+              echo "Deleting Open vSwitch ${n}"
+              ovs-vsctl --if-exists del-br ${n} || true
             '';
           });
 
@@ -476,9 +489,9 @@ let
               # Remove Dead Interfaces
               ip link show "${n}" >/dev/null 2>&1 && ip link delete "${n}"
               ip link add link "${v.interface}" name "${n}" type vlan id "${toString v.id}"
-              
+
-              # We try to bring up the logical VLAN interface. If the master 
+              # We try to bring up the logical VLAN interface. If the master
-              # interface the logical interface is dependent upon is not up yet we will 
+              # interface the logical interface is dependent upon is not up yet we will
               # fail to immediately bring up the logical interface. The resulting logical
               # interface will brought up later when the master interface is up.
               ip link set "${n}" up || true

nixos/modules/tasks/network-interfaces-systemd.nix

@@ -1,4 +1,4 @@
-{ config, lib, utils, ... }:
+{ config, lib, utils, pkgs, ... }:
 
 with utils;
 with lib;
@@ -18,7 +18,10 @@ let
     concatLists (map (bond: bond.interfaces) (attrValues cfg.bonds))
     ++ concatLists (map (bridge: bridge.interfaces) (attrValues cfg.bridges))
     ++ map (sit: sit.dev) (attrValues cfg.sits)
-    ++ map (vlan: vlan.interface) (attrValues cfg.vlans);
+    ++ map (vlan: vlan.interface) (attrValues cfg.vlans)
+    # add dependency to physical or independently created vswitch member interface
+    # TODO: warn the user that any address configured on those interfaces will be useless
+    ++ concatMap (i: attrNames (filterAttrs (_: config: config.type != "internal") i.interfaces)) (attrValues cfg.vswitches);
 
 in
 
@@ -51,11 +54,6 @@ in
 
     networking.dhcpcd.enable = mkDefault false;
 
-    systemd.services.network-local-commands = {
-      after = [ "systemd-networkd.service" ];
-      bindsTo = [ "systemd-networkd.service" ];
-    };
-
     systemd.network =
       let
         domains = cfg.search ++ (optional (cfg.domain != null) cfg.domain);
@@ -233,6 +231,63 @@ in
     # This forces the network interface creator to initialize slaves.
     networking.interfaces = listToAttrs (map (i: nameValuePair i { }) slaves);
 
+    systemd.services = let
+      # We must escape interfaces due to the systemd interpretation
+      subsystemDevice = interface:
+        "sys-subsystem-net-devices-${escapeSystemdPath interface}.device";
+      # support for creating openvswitch switches
+      createVswitchDevice = n: v: nameValuePair "${n}-netdev"
+          (let
+            deps = map subsystemDevice (attrNames (filterAttrs (_: config: config.type != "internal") v.interfaces));
+            ofRules = pkgs.writeText "vswitch-${n}-openFlowRules" v.openFlowRules;
+          in
+          { description = "Open vSwitch Interface ${n}";
+            wantedBy = [ "network.target" (subsystemDevice n) ];
+            # and create bridge before systemd-networkd starts because it might create internal interfaces
+            before = [ "systemd-networkd.service" ];
+            # shutdown the bridge when network is shutdown
+            partOf = [ "network.target" ];
+            # requires ovs-vswitchd to be alive at all times
+            bindsTo = [ "ovs-vswitchd.service" ];
+            # start switch after physical interfaces and vswitch daemon
+            after = [ "network-pre.target" "ovs-vswitchd.service" ] ++ deps;
+            wants = deps; # if one or more interface fails, the switch should continue to run
+            serviceConfig.Type = "oneshot";
+            serviceConfig.RemainAfterExit = true;
+            path = [ pkgs.iproute config.virtualisation.vswitch.package ];
+            preStart = ''
+              echo "Resetting Open vSwitch ${n}..."
+              ovs-vsctl --if-exists del-br ${n} -- add-br ${n} \
+                        -- set bridge ${n} protocols=${concatStringsSep "," v.supportedOpenFlowVersions}
+            '';
+            script = ''
+              echo "Configuring Open vSwitch ${n}..."
+              ovs-vsctl ${concatStrings (mapAttrsToList (name: config: " -- add-port ${n} ${name}" + optionalString (config.vlan != null) " tag=${toString config.vlan}") v.interfaces)} \
+                ${concatStrings (mapAttrsToList (name: config: optionalString (config.type != null) " -- set interface ${name} type=${config.type}") v.interfaces)} \
+                ${concatMapStrings (x: " -- set-controller ${n} " + x)  v.controllers} \
+                ${concatMapStrings (x: " -- " + x) (splitString "\n" v.extraOvsctlCmds)}
+
+
+              echo "Adding OpenFlow rules for Open vSwitch ${n}..."
+              ovs-ofctl --protocols=${v.openFlowVersion} add-flows ${n} ${ofRules}
+            '';
+            postStop = ''
+              echo "Cleaning Open vSwitch ${n}"
+              echo "Shuting down internal ${n} interface"
+              ip link set ${n} down || true
+              echo "Deleting flows for ${n}"
+              ovs-ofctl --protocols=${v.openFlowVersion} del-flows ${n} || true
+              echo "Deleting Open vSwitch ${n}"
+              ovs-vsctl --if-exists del-br ${n} || true
+            '';
+          });
+    in mapAttrs' createVswitchDevice cfg.vswitches
+      // {
+            "network-local-commands" = {
+              after = [ "systemd-networkd.service" ];
+              bindsTo = [ "systemd-networkd.service" ];
+          };
+      };
   };
 
 }

nixos/modules/tasks/network-interfaces.nix

@@ -13,7 +13,7 @@ let
 
   slaves = concatMap (i: i.interfaces) (attrValues cfg.bonds)
     ++ concatMap (i: i.interfaces) (attrValues cfg.bridges)
-    ++ concatMap (i: i.interfaces) (attrValues cfg.vswitches);
+    ++ concatMap (i: attrNames (filterAttrs (name: config: ! (config.type == "internal" || hasAttr name cfg.interfaces)) i.interfaces)) (attrValues cfg.vswitches);
 
   slaveIfs = map (i: cfg.interfaces.${i}) (filter (i: cfg.interfaces ? ${i}) slaves);
 
@@ -336,6 +336,32 @@ let
 
   };
 
+  vswitchInterfaceOpts = {name, ...}: {
+
+    options = {
+
+      name = mkOption {
+        description = "Name of the interface";
+        example = "eth0";
+        type = types.str;
+      };
+
+      vlan = mkOption {
+        description = "Vlan tag to apply to interface";
+        example = 10;
+        type = types.nullOr types.int;
+        default = null;
+      };
+
+      type = mkOption {
+        description = "Openvswitch type to assign to interface";
+        example = "internal";
+        type = types.nullOr types.str;
+        default = null;
+      };
+    };
+  };
+
   hexChars = stringToCharacters "0123456789abcdef";
 
   isHexString = s: all (c: elem c hexChars) (stringToCharacters (toLower s));
@@ -486,8 +512,8 @@ in
     networking.vswitches = mkOption {
       default = { };
       example =
-        { vs0.interfaces = [ "eth0" "eth1" ];
+        { vs0.interfaces = { eth0 = { }; lo1 = { type="internal"; }; };
-          vs1.interfaces = [ "eth2" "wlan0" ];
+          vs1.interfaces = [ { name = "eth2"; } { name = "lo2"; type="internal"; } ];
         };
       description =
         ''
@@ -504,9 +530,8 @@ in
 
           interfaces = mkOption {
             example = [ "eth0" "eth1" ];
-            type = types.listOf types.str;
+            description = "The physical network interfaces connected by the vSwitch.";
-            description =
+            type = with types; loaOf (submodule vswitchInterfaceOpts);
-              "The physical network interfaces connected by the vSwitch.";
           };
 
           controllers = mkOption {
@@ -530,6 +555,25 @@ in
             '';
           };
 
+          # TODO: custom "openflow version" type, with list from existing openflow protocols
+          supportedOpenFlowVersions = mkOption {
+            type = types.listOf types.str;
+            example = [ "OpenFlow10" "OpenFlow13" "OpenFlow14" ];
+            default = [ "OpenFlow13" ];
+            description = ''
+              Supported versions to enable on this switch.
+            '';
+          };
+
+          # TODO: use same type as elements from supportedOpenFlowVersions
+          openFlowVersion = mkOption {
+            type = types.str;
+            default = "OpenFlow13";
+            description = ''
+              Version of OpenFlow protocol to use when communicating with the switch internally (e.g. with <literal>openFlowRules</literal>).
+            '';
+          };
+
           extraOvsctlCmds = mkOption {
             type = types.lines;
             default = "";

nixos/modules/virtualisation/libvirtd.nix

@@ -219,7 +219,7 @@ in {
       wantedBy = [ "multi-user.target" ];
       requires = [ "libvirtd-config.service" ];
       after = [ "systemd-udev-settle.service" "libvirtd-config.service" ]
-              ++ optional vswitch.enable "vswitchd.service";
+              ++ optional vswitch.enable "ovs-vswitchd.service";
 
       environment.LIBVIRTD_ARGS = ''--config "${configFile}" ${concatStringsSep " " cfg.extraOptions}'';
 

nixos/modules/virtualisation/openvswitch.nix

@@ -124,7 +124,7 @@ in {
       '';
     };
 
-    systemd.services.vswitchd = {
+    systemd.services.ovs-vswitchd = {
       description = "Open_vSwitch Daemon";
       wantedBy = [ "multi-user.target" ];
       bindsTo = [ "ovsdb.service" ];
@@ -139,6 +139,8 @@ in {
         PIDFile = "/run/openvswitch/ovs-vswitchd.pid";
         # Use service type 'forking' to correctly determine when vswitchd is ready.
         Type = "forking";
+        Restart = "always";
+        RestartSec = 3;
       };
     };
 
@@ -182,4 +184,7 @@ in {
       '';
     };
   })]));
+
+  meta.maintainers = with maintainers; [ netixx ];
+
 }