From cddc7a28b825af49d6d1f8b6d34122e98d3f4c1e Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 15:18:59 +0200 Subject: [PATCH 1/6] nixos/networking: fix setting .macAddress and .mtu with networkd This needs to be set in the .linkConfig of a .network --- nixos/modules/tasks/network-interfaces-systemd.nix | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nixos/modules/tasks/network-interfaces-systemd.nix b/nixos/modules/tasks/network-interfaces-systemd.nix index 41deceb000e62..23e1e611a71e9 100644 --- a/nixos/modules/tasks/network-interfaces-systemd.nix +++ b/nixos/modules/tasks/network-interfaces-systemd.nix @@ -94,7 +94,12 @@ in address = forEach (interfaceIps i) (ip: "${ip.address}/${toString ip.prefixLength}"); networkConfig.IPv6PrivacyExtensions = "kernel"; - } ]; + linkConfig = optionalAttrs (i.macAddress != null) { + MACAddress = i.macAddress; + } // optionalAttrs (i.mtu != null) { + MTUBytes = toString i.mtu; + }; + }]; }))) (mkMerge (flip mapAttrsToList cfg.bridges (name: bridge: { netdevs."40-${name}" = { From ca391c8a4fcbdeae747fc1e0fe9858c651f47502 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 15:20:29 +0200 Subject: [PATCH 2/6] nixos/networking: add assertion catching setting mac addresses on tun devices Setting a MAC Address on a tun interface isn't supported, and invoking the corresponding command fails. --- nixos/modules/tasks/network-interfaces.nix | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nixos/modules/tasks/network-interfaces.nix b/nixos/modules/tasks/network-interfaces.nix index 63a79abd4eb3a..53430b0a9349f 100644 --- a/nixos/modules/tasks/network-interfaces.nix +++ b/nixos/modules/tasks/network-interfaces.nix @@ -1031,6 +1031,11 @@ in message = '' Temporary addresses are only needed when IPv6 is enabled. ''; + })) ++ (forEach interfaces (i: { + assertion = (i.virtual && i.virtualType == "tun") -> i.macAddress == null; + message = '' + Setting a MAC Address for tun device ${i.name} isn't supported. + ''; })) ++ [ { assertion = cfg.hostId == null || (stringLength cfg.hostId == 8 && isHexString cfg.hostId); From 532528190bdd739fda866645ba0078df28abc025 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 15:22:22 +0200 Subject: [PATCH 3/6] nixos/networking: move network-link-${i.name} to scripted networking The unit sets MTU and MAC Address even with networkd enabled, which isn't necessary anymore, as networkd handles this by itself. --- .../tasks/network-interfaces-scripted.nix | 33 +++++++++++++++++++ nixos/modules/tasks/network-interfaces.nix | 33 +------------------ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/nixos/modules/tasks/network-interfaces-scripted.nix b/nixos/modules/tasks/network-interfaces-scripted.nix index 98bae444df0d4..9720d90217c66 100644 --- a/nixos/modules/tasks/network-interfaces-scripted.nix +++ b/nixos/modules/tasks/network-interfaces-scripted.nix @@ -237,6 +237,38 @@ 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" ]; @@ -508,6 +540,7 @@ let }); in listToAttrs ( + map createNetworkLink interfaces ++ map configureAddrs interfaces ++ map createTunDevice (filter (i: i.virtual) interfaces)) // mapAttrs' createBridgeDevice cfg.bridges diff --git a/nixos/modules/tasks/network-interfaces.nix b/nixos/modules/tasks/network-interfaces.nix index 53430b0a9349f..44677d417ead0 100644 --- a/nixos/modules/tasks/network-interfaces.nix +++ b/nixos/modules/tasks/network-interfaces.nix @@ -1145,38 +1145,7 @@ in ${cfg.localCommands} ''; }; - } // (listToAttrs (forEach interfaces (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) - ''; - }))); - + }; services.mstpd = mkIf needsMstpd { enable = true; }; virtualisation.vswitch = mkIf (cfg.vswitches != { }) { enable = true; }; From 1e1945319c2017e0a40a2ea3023d64f282d2b538 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 19:16:12 +0200 Subject: [PATCH 4/6] nixosTests.networking: make routing table comparison more reliable This was whitespace-sensitive, kept fighting with my editor and broke the tests easily. To fix this, let python convert the output to individual lines, and strip whitespace from them before comparing. --- nixos/tests/networking.nix | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/nixos/tests/networking.nix b/nixos/tests/networking.nix index 0a6507d2dc884..5c4aa5c38e11b 100644 --- a/nixos/tests/networking.nix +++ b/nixos/tests/networking.nix @@ -602,17 +602,17 @@ let }; testScript = '' - targetIPv4Table = """ - 10.0.0.0/16 proto static scope link mtu 1500 - 192.168.1.0/24 proto kernel scope link src 192.168.1.2 - 192.168.2.0/24 via 192.168.1.1 proto static - """.strip() - - targetIPv6Table = """ - 2001:1470:fffd:2097::/64 proto kernel metric 256 pref medium - 2001:1470:fffd:2098::/64 via fdfd:b3f0::1 proto static metric 1024 pref medium - fdfd:b3f0::/48 proto static metric 1024 pref medium - """.strip() + targetIPv4Table = [ + "10.0.0.0/16 proto static scope link mtu 1500", + "192.168.1.0/24 proto kernel scope link src 192.168.1.2", + "192.168.2.0/24 via 192.168.1.1 proto static", + ] + + targetIPv6Table = [ + "2001:1470:fffd:2097::/64 proto kernel metric 256 pref medium", + "2001:1470:fffd:2098::/64 via fdfd:b3f0::1 proto static metric 1024 pref medium", + "fdfd:b3f0::/48 proto static metric 1024 pref medium", + ] machine.start() machine.wait_for_unit("network.target") @@ -620,9 +620,9 @@ let with subtest("test routing tables"): ipv4Table = machine.succeed("ip -4 route list dev eth0 | head -n3").strip() ipv6Table = machine.succeed("ip -6 route list dev eth0 | head -n3").strip() - assert ( - ipv4Table == targetIPv4Table - ), """ + assert [ + l.strip() for l in ipv4Table.splitlines() + ] == targetIPv4Table, """ The IPv4 routing table does not match the expected one: Result: {} @@ -631,9 +631,9 @@ let """.format( ipv4Table, targetIPv4Table ) - assert ( - ipv6Table == targetIPv6Table - ), """ + assert [ + l.strip() for l in ipv6Table.splitlines() + ] == targetIPv6Table, """ The IPv6 routing table does not match the expected one: Result: {} From 5150378c2f10d34a7ba4404c52f6c882284dd254 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 19:15:08 +0200 Subject: [PATCH 5/6] nixosTests.networking.virtual: fix with networkd We only need to wait for network.target to get up, and the network-addresses-${interfaceName} units are scripted networking only. --- nixos/tests/networking.nix | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nixos/tests/networking.nix b/nixos/tests/networking.nix index 5c4aa5c38e11b..5dde9046fc306 100644 --- a/nixos/tests/networking.nix +++ b/nixos/tests/networking.nix @@ -471,7 +471,7 @@ let with subtest("Wait for networking to come up"): machine.start() - machine.wait_for_unit("network-online.target") + machine.wait_for_unit("network.target") with subtest("Test interfaces set up"): list = machine.succeed("ip tuntap list | sort").strip() @@ -486,7 +486,8 @@ let """.format( list, targetList ) - + '' # network-addresses-* only exist in scripted networking + + optionalString (!networkd) '' with subtest("Test interfaces clean up"): machine.succeed("systemctl stop network-addresses-tap0") machine.sleep(10) From d1edd8b2f60e4a48be2d1f90f04fe91f2ba71035 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Apr 2020 19:17:56 +0200 Subject: [PATCH 6/6] nixosTests.networking: test setting MTU and MAC Address Both the scripted and networkd backend now support setting MTU and MAC Address, so do this in a test to ensure it doesn't break. --- nixos/tests/networking.nix | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nixos/tests/networking.nix b/nixos/tests/networking.nix index 5dde9046fc306..3d8ab761a4464 100644 --- a/nixos/tests/networking.nix +++ b/nixos/tests/networking.nix @@ -200,6 +200,7 @@ let useDHCP = false; interfaces.eth1 = { ipv4.addresses = mkOverride 0 [ ]; + mtu = 1343; useDHCP = true; }; interfaces.eth2.ipv4.addresses = mkOverride 0 [ ]; @@ -216,6 +217,9 @@ let with subtest("Wait until we have an ip address on each interface"): client.wait_until_succeeds("ip addr show dev eth1 | grep -q '192.168.1'") + with subtest("ensure MTU is set"): + assert "mtu 1343" in client.succeed("ip link show dev eth1") + with subtest("Test vlan 1"): client.wait_until_succeeds("ping -c 1 192.168.1.1") client.wait_until_succeeds("ping -c 1 192.168.1.2") @@ -455,11 +459,14 @@ let ipv4.addresses = [ { address = "192.168.1.1"; prefixLength = 24; } ]; ipv6.addresses = [ { address = "2001:1470:fffd:2096::"; prefixLength = 64; } ]; virtual = true; + mtu = 1342; + macAddress = "02:de:ad:be:ef:01"; }; networking.interfaces.tun0 = { ipv4.addresses = [ { address = "192.168.1.2"; prefixLength = 24; } ]; ipv6.addresses = [ { address = "2001:1470:fffd:2097::"; prefixLength = 64; } ]; virtual = true; + mtu = 1343; }; }; @@ -486,6 +493,10 @@ let """.format( list, targetList ) + with subtest("Test MTU and MAC Address are configured"): + assert "mtu 1342" in machine.succeed("ip link show dev tap0") + assert "mtu 1343" in machine.succeed("ip link show dev tun0") + assert "02:de:ad:be:ef:01" in machine.succeed("ip link show dev tap0") '' # network-addresses-* only exist in scripted networking + optionalString (!networkd) '' with subtest("Test interfaces clean up"):