From cd3597b4864935a8dc978b711a4847a6132b8a2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Netix=20=28Espinet=20Franc=CC=A7ois=29?= Date: Sat, 29 Dec 2018 11:16:51 +0100 Subject: [PATCH] openvswitch: better integration with systemd Systemd dependencies for scripted mode were refactored according to analysis in #34586. networking.vswitches can now be used with systemd-networkd, although they are not supported by the daemon, a nixos receipe creates the switch and attached required interfaces (just like the scripted version). Vlans and internal interfaces are implemented following the template format i.e. each interface is described using an attributeSet (vlan and type at the moment). If vlan is present, then interface is added to the vswitch with given tag (access mode). Type internal enabled vswitch to create interfaces (see openvswitch docs). Added configuration for configuring supported openFlow version on the vswitch This commit is a split from the original PR #35127. --- nixos/modules/services/networking/dhcpcd.nix | 2 +- .../tasks/network-interfaces-scripted.nix | 49 ++++++++----- .../tasks/network-interfaces-systemd.nix | 69 +++++++++++++++++-- nixos/modules/tasks/network-interfaces.nix | 56 +++++++++++++-- nixos/modules/virtualisation/libvirtd.nix | 2 +- nixos/modules/virtualisation/openvswitch.nix | 7 +- 6 files changed, 151 insertions(+), 34 deletions(-) diff --git a/nixos/modules/services/networking/dhcpcd.nix b/nixos/modules/services/networking/dhcpcd.nix index 7b2786034552..4b572df10b43 100644 --- a/nixos/modules/services/networking/dhcpcd.nix +++ b/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; diff --git a/nixos/modules/tasks/network-interfaces-scripted.nix b/nixos/modules/tasks/network-interfaces-scripted.nix index 1726d05115ea..4d25137c5dfc 100644 --- a/nixos/modules/tasks/network-interfaces-scripted.nix +++ b/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; - bindsTo = [ "vswitchd.service" (subsystemDevice n) ] ++ deps; - partOf = [ "network-setup.service" "vswitchd.service" ]; - after = [ "network-pre.target" "vswitchd.service" ] ++ deps; - before = [ "network-setup.service" ]; + wantedBy = [ "network-setup.service" (subsystemDevice n) ] ++ internalConfigs; + # before = [ "network-setup.service" ]; + # should work without internalConfigs dependencies because address/link configuration depends + # on the device, which is created by ovs-vswitchd with type=internal, but it does not... + 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}..." - ovs-vsctl --if-exists del-br ${n} - - echo "Adding Open vSwitch ${n}..." - ovs-vsctl -- add-br ${n} ${concatMapStrings (i: " -- add-port ${n} ${i}") v.interfaces} \ + 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 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 - ovs-vsctl --if-exists del-br ${n} + 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 ''; }); @@ -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 - # interface the logical interface is dependent upon is not up yet we will + + # 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 # 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 diff --git a/nixos/modules/tasks/network-interfaces-systemd.nix b/nixos/modules/tasks/network-interfaces-systemd.nix index 9ffa1089ee69..37b42c010a60 100644 --- a/nixos/modules/tasks/network-interfaces-systemd.nix +++ b/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" ]; + }; + }; }; } diff --git a/nixos/modules/tasks/network-interfaces.nix b/nixos/modules/tasks/network-interfaces.nix index 31e2ed1cd1ea..fc80e6fcc3ef 100644 --- a/nixos/modules/tasks/network-interfaces.nix +++ b/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); @@ -310,6 +310,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)); @@ -460,8 +486,8 @@ in networking.vswitches = mkOption { default = { }; example = - { vs0.interfaces = [ "eth0" "eth1" ]; - vs1.interfaces = [ "eth2" "wlan0" ]; + { vs0.interfaces = { eth0 = { }; lo1 = { type="internal"; }; }; + vs1.interfaces = [ { name = "eth2"; } { name = "lo2"; type="internal"; } ]; }; description = '' @@ -478,9 +504,8 @@ in interfaces = mkOption { example = [ "eth0" "eth1" ]; - type = types.listOf types.str; - description = - "The physical network interfaces connected by the vSwitch."; + description = "The physical network interfaces connected by the vSwitch."; + type = with types; loaOf (submodule vswitchInterfaceOpts); }; controllers = mkOption { @@ -504,6 +529,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 openFlowRules). + ''; + }; + extraOvsctlCmds = mkOption { type = types.lines; default = ""; diff --git a/nixos/modules/virtualisation/libvirtd.nix b/nixos/modules/virtualisation/libvirtd.nix index 52d852894ce5..9f7bac480e38 100644 --- a/nixos/modules/virtualisation/libvirtd.nix +++ b/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}''; diff --git a/nixos/modules/virtualisation/openvswitch.nix b/nixos/modules/virtualisation/openvswitch.nix index 6b8ad83661fe..c6a3ceddc3e0 100644 --- a/nixos/modules/virtualisation/openvswitch.nix +++ b/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 ]; + }