From 822918df4cec0ee14a657efb86a08bf7f457e647 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sun, 17 May 2020 21:57:01 +0200 Subject: [PATCH] nixos/scripted-networking: use udev to configure link MACAddress and MTUBytes The `network-link-${i.name}` units raced with other things trying to configure the interface, or ran before the interface was available. Instead of running our own set of shell scripts on boot, and hoping they're executed at the right time, we can make use of udev to configure the interface *while they appear*, by providing `.link` files in /etc/systemd/network/*.link to set MACAddress and MTUBytes. This doesn't require networkd to be enabled, and is populated properly on non-networkd systems since https://github.com/NixOS/nixpkgs/pull/82941. This continues clean-up work done in https://github.com/NixOS/nixpkgs/pull/85170 for the scripted networking stack. The only leftover part of the `network-link-${i.name}` unit (bringing the interface up) is moved to the beginning of the `network-addresses-${i.name}` unit. Fixes: https://github.com/NixOS/nixpkgs/issues/74471 Closes: https://github.com/NixOS/nixpkgs/pull/87116 --- nixos/doc/manual/release-notes/rl-2009.xml | 15 ++++++ .../tasks/network-interfaces-scripted.nix | 53 ++++++------------- 2 files changed, 30 insertions(+), 38 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2009.xml b/nixos/doc/manual/release-notes/rl-2009.xml index a4c2719d0445..5d2ffd262e04 100644 --- a/nixos/doc/manual/release-notes/rl-2009.xml +++ b/nixos/doc/manual/release-notes/rl-2009.xml @@ -437,6 +437,21 @@ systemd.services.nginx.serviceConfig.ReadWritePaths = [ "/var/www" ]; Default algorithm for ZRAM swap was changed to zstd. + + + The scripted networking system now uses .link files in + /etc/systemd/network to configure mac address and link MTU, + instead of the sometimes buggy network-link-* units, which + have been removed. + Bringing the interface up has been moved to the beginning of the + network-addresses-* unit. + Note this doesn't require systemd-networkd - it's udev that + parses .link files. + Extra care needs to be taken in the presence of legacy udev rules + to rename interfaces, as MAC Address and MTU defined in these options can only match on the original link name. + In such cases, you most likely want to create a 10-*.link file through and set both name and MAC Address / MTU there. + + diff --git a/nixos/modules/tasks/network-interfaces-scripted.nix b/nixos/modules/tasks/network-interfaces-scripted.nix index f6fce3b1c8bb..d895c58bab03 100644 --- a/nixos/modules/tasks/network-interfaces-scripted.nix +++ b/nixos/modules/tasks/network-interfaces-scripted.nix @@ -54,7 +54,16 @@ let }; normalConfig = { - + systemd.network.links = let + createNetworkLink = i: nameValuePair "40-${i.name}" { + matchConfig.OriginalName = i.name; + linkConfig = optionalAttrs (i.macAddress != null) { + MACAddress = i.macAddress; + } // optionalAttrs (i.mtu != null) { + MTUBytes = toString i.mtu; + }; + }; + in listToAttrs (map createNetworkLink interfaces); systemd.services = let @@ -164,7 +173,6 @@ let { description = "Address configuration of ${i.name}"; wantedBy = [ "network-setup.service" - "network-link-${i.name}.service" "network.target" ]; # order before network-setup because the routes that are configured @@ -183,6 +191,8 @@ let state="/run/nixos/network/addresses/${i.name}" mkdir -p $(dirname "$state") + ip link set "${i.name}" up + ${flip concatMapStrings ips (ip: let cidr = "${ip.address}/${toString ip.prefixLength}"; @@ -237,38 +247,6 @@ let ''; }; - createNetworkLink = i: - let - deviceDependency = if (config.boot.isContainer || i.name == "lo") - then [] - else [ (subsystemDevice i.name) ]; - in - nameValuePair "network-link-${i.name}" - { description = "Link configuration of ${i.name}"; - wantedBy = [ "network-interfaces.target" ]; - before = [ "network-interfaces.target" ]; - bindsTo = deviceDependency; - after = [ "network-pre.target" ] ++ deviceDependency; - path = [ pkgs.iproute ]; - serviceConfig = { - Type = "oneshot"; - RemainAfterExit = true; - }; - script = - '' - echo "Configuring link..." - '' + optionalString (i.macAddress != null) '' - echo "setting MAC address to ${i.macAddress}..." - ip link set "${i.name}" address "${i.macAddress}" - '' + optionalString (i.mtu != null) '' - echo "setting MTU to ${toString i.mtu}..." - ip link set "${i.name}" mtu "${toString i.mtu}" - '' + '' - echo -n "bringing up interface... " - ip link set "${i.name}" up && echo "done" || (echo "failed"; exit 1) - ''; - }; - createTunDevice = i: nameValuePair "${i.name}-netdev" { description = "Virtual Network Interface ${i.name}"; bindsTo = [ "dev-net-tun.device" ]; @@ -298,7 +276,7 @@ let bindsTo = deps ++ optional v.rstp "mstpd.service"; partOf = [ "network-setup.service" ] ++ optional v.rstp "mstpd.service"; after = [ "network-pre.target" ] ++ deps ++ optional v.rstp "mstpd.service" - ++ concatMap (i: [ "network-addresses-${i}.service" "network-link-${i}.service" ]) v.interfaces; + ++ map (i: "network-addresses-${i}.service") v.interfaces; before = [ "network-setup.service" ]; serviceConfig.Type = "oneshot"; serviceConfig.RemainAfterExit = true; @@ -375,7 +353,7 @@ let createVswitchDevice = n: v: nameValuePair "${n}-netdev" (let 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)); + internalConfigs = map (i: "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}"; @@ -427,7 +405,7 @@ let bindsTo = deps; partOf = [ "network-setup.service" ]; after = [ "network-pre.target" ] ++ deps - ++ concatMap (i: [ "network-addresses-${i}.service" "network-link-${i}.service" ]) v.interfaces; + ++ map (i: "network-addresses-${i}.service") v.interfaces; before = [ "network-setup.service" ]; serviceConfig.Type = "oneshot"; serviceConfig.RemainAfterExit = true; @@ -540,7 +518,6 @@ let }); in listToAttrs ( - map createNetworkLink interfaces ++ map configureAddrs interfaces ++ map createTunDevice (filter (i: i.virtual) interfaces)) // mapAttrs' createBridgeDevice cfg.bridges