From 83c1c1c08426beb8c395a809dd4c1aa788223e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20M=C3=A4rdian?= Date: Tue, 27 Oct 2020 12:20:51 +0100 Subject: [PATCH] Do not try to match/rename duplicated MAC of vlan (LP: #1888726) (#166) Fix systemd-networkd misconfiguration; when matching on macaddr, if other interfaces in the system might have the same mac (e.g. bridge, vlan, etc), the match filtering must also filter which interface to match, e.g., besides the MACAddress= match it also needs (for example) one or more of: Type=!vlan Type=!bridge PS: also cleaning up the test_vlans.py file a bit, while working on it. --- src/networkd.c | 21 ++++--- tests/generator/base.py | 15 +++-- tests/generator/test_vlans.py | 103 ++++++++++++++++------------------ 3 files changed, 66 insertions(+), 73 deletions(-) diff --git a/src/networkd.c b/src/networkd.c index a1b31d836..48b0cbdc4 100644 --- a/src/networkd.c +++ b/src/networkd.c @@ -86,19 +86,18 @@ append_match_section(const NetplanNetDefinition* def, GString* s, gboolean match g_string_append_printf(s, "Name=%s\n", def->match.original_name); } - /* Workaround for bug LP: #1804861: something outputs netplan config - * that includes using the MAC of the first phy member of a bond as - * default value for the MAC of the bond device itself. This is - * evil, it's an optional field and networkd knows what to do if - * the MAC isn't specified; but work around this by adding an - * arbitrary additional match condition on Path= for the phys. - * This way, hopefully setting a MTU on the phy does not bleed over - * to bond/bridge and any further virtual devices (VLANs?) on top of - * it. + /* Workaround for bugs LP: #1804861 and LP: #1888726: something outputs + * netplan config that includes using the MAC of the first phy member of a + * bond as default value for the MAC of the bond device itself. This is + * evil, it's an optional field and networkd knows what to do if the MAC + * isn't specified; but work around this by adding an arbitrary additional + * match condition on Path= for the phys. This way, hopefully setting a MTU + * on the phy does not bleed over to bond/bridge and any further virtual + * devices (VLANs?) on top of it. * Make sure to add the extra match only if we're matching by MAC - * already and dealing with a bond or bridge. + * already and dealing with a bond, bridge or vlan. */ - if (def->bond || def->bridge) { + if (def->bond || def->bridge || def->has_vlans) { /* update if we support new device types */ if (def->match.mac) g_string_append(s, "Type=!vlan bond bridge\n"); diff --git a/tests/generator/base.py b/tests/generator/base.py index c869cf791..d4ca52378 100644 --- a/tests/generator/base.py +++ b/tests/generator/base.py @@ -39,13 +39,15 @@ # common patterns for expected output ND_EMPTY = '[Match]\nName=%s\n\n[Network]\nLinkLocalAddressing=%s\nConfigureWithoutCarrier=yes\n' ND_WITHIP = '[Match]\nName=%s\n\n[Network]\nLinkLocalAddressing=ipv6\nAddress=%s\nConfigureWithoutCarrier=yes\n' -ND_DHCP4 = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv4\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=true\n' -ND_DHCP4_NOMTU = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv4\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=false\n' ND_WIFI_DHCP4 = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv4\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=600\nUseMTU=true\n' -ND_DHCP6 = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv6\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=true\n' -ND_DHCP6_NOMTU = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv6\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=false\n' -ND_DHCPYES = '[Match]\nName=%s\n\n[Network]\nDHCP=yes\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=true\n' -ND_DHCPYES_NOMTU = '[Match]\nName=%s\n\n[Network]\nDHCP=yes\nLinkLocalAddressing=ipv6\n\n[DHCP]\nRouteMetric=100\nUseMTU=false\n' +ND_DHCP = '[Match]\nName=%s\n\n[Network]\nDHCP=%s\nLinkLocalAddressing=ipv6%s\n\n[DHCP]\nRouteMetric=100\nUseMTU=%s\n' +ND_DHCP4 = ND_DHCP % ('%s', 'ipv4', '', 'true') +ND_DHCP4_NOMTU = ND_DHCP % ('%s', 'ipv4', '', 'false') +ND_DHCP6 = ND_DHCP % ('%s', 'ipv6', '', 'true') +ND_DHCP6_NOMTU = ND_DHCP % ('%s', 'ipv6', '', 'false') +ND_DHCP6_WOCARRIER = ND_DHCP % ('%s', 'ipv6', '\nConfigureWithoutCarrier=yes', 'true') +ND_DHCPYES = ND_DHCP % ('%s', 'yes', '', 'true') +ND_DHCPYES_NOMTU = ND_DHCP % ('%s', 'yes', '', 'false') _OVS_BASE = '[Unit]\nDescription=OpenVSwitch configuration for %(iface)s\nDefaultDependencies=no\n\ Wants=ovsdb-server.service\nAfter=ovsdb-server.service\n' OVS_PHYSICAL = _OVS_BASE + 'Requires=sys-subsystem-net-devices-%(iface)s.device\nAfter=sys-subsystem-net-devices-%(iface)s\ @@ -69,6 +71,7 @@ \n\n[ipv4]\nmethod=manual\naddress1=15.15.15.15/24\ngateway=20.20.20.21\n\n[ipv6]\nmethod=manual\naddress1=\ 2001:de:ad:be:ef:ca:fe:1/128\n' ND_WG = '[NetDev]\nName=wg0\nKind=wireguard\n\n[WireGuard]\nPrivateKey%s\nListenPort=%s\n%s\n' +ND_VLAN = '[NetDev]\nName=%s\nKind=vlan\n\n[VLAN]\nId=%d\n' class TestBase(unittest.TestCase): diff --git a/tests/generator/test_vlans.py b/tests/generator/test_vlans.py index ddddd3745..606a0b1e9 100644 --- a/tests/generator/test_vlans.py +++ b/tests/generator/test_vlans.py @@ -20,7 +20,7 @@ import re import unittest -from .base import TestBase +from .base import TestBase, ND_VLAN, ND_EMPTY, ND_WITHIP, ND_DHCP6_WOCARRIER class TestNetworkd(TestBase): @@ -51,20 +51,8 @@ def test_vlan(self): # pragma: nocover VLAN=enred VLAN=engreen ''', - 'enblue.netdev': '''[NetDev] -Name=enblue -Kind=vlan - -[VLAN] -Id=1 -''', - 'engreen.netdev': '''[NetDev] -Name=engreen -Kind=vlan - -[VLAN] -Id=2 -''', + 'enblue.netdev': ND_VLAN % ('enblue', 1), + 'engreen.netdev': ND_VLAN % ('engreen', 2), 'enred.netdev': '''[NetDev] Name=enred MACAddress=aa:bb:cc:dd:ee:11 @@ -73,33 +61,10 @@ def test_vlan(self): # pragma: nocover [VLAN] Id=3 ''', - 'enblue.network': '''[Match] -Name=enblue + 'enblue.network': ND_WITHIP % ('enblue', '1.2.3.4/24'), + 'enred.network': ND_EMPTY % ('enred', 'ipv6'), + 'engreen.network': (ND_DHCP6_WOCARRIER % 'engreen')}) -[Network] -LinkLocalAddressing=ipv6 -Address=1.2.3.4/24 -ConfigureWithoutCarrier=yes -''', - 'enred.network': '''[Match] -Name=enred - -[Network] -LinkLocalAddressing=ipv6 -ConfigureWithoutCarrier=yes -''', - 'engreen.network': '''[Match] -Name=engreen - -[Network] -DHCP=ipv6 -LinkLocalAddressing=ipv6 -ConfigureWithoutCarrier=yes - -[DHCP] -RouteMetric=100 -UseMTU=true -'''}) self.assert_nm(None, '''[keyfile] # devices managed by networkd unmanaged-devices+=interface-name:en1,interface-name:enblue,interface-name:enred,interface-name:engreen,''') @@ -126,28 +91,54 @@ def test_vlan_sriov(self): LinkLocalAddressing=ipv6 VLAN=engreen ''', - 'engreen.netdev': '''[NetDev] -Name=engreen -Kind=vlan + 'engreen.netdev': ND_VLAN % ('engreen', 2), + 'engreen.network': (ND_DHCP6_WOCARRIER % 'engreen')}) -[VLAN] -Id=2 -''', - 'engreen.network': '''[Match] -Name=engreen + self.assert_nm(None, '''[keyfile] +# devices managed by networkd +unmanaged-devices+=interface-name:en1,interface-name:enblue,interface-name:engreen,''') + self.assert_nm_udev(None) + + # see LP: #1888726 + def test_vlan_parent_match(self): + self.generate('''network: + version: 2 + renderer: networkd + ethernets: + lan: + match: {macaddress: "11:22:33:44:55:66"} + set-name: lan + mtu: 9000 + vlans: + vlan20: {id: 20, link: lan}''') + + self.assert_networkd({'lan.network': '''[Match] +MACAddress=11:22:33:44:55:66 +Name=lan +Type=!vlan bond bridge + +[Link] +MTUBytes=9000 [Network] -DHCP=ipv6 LinkLocalAddressing=ipv6 -ConfigureWithoutCarrier=yes +VLAN=vlan20 +''', + 'lan.link': '''[Match] +MACAddress=11:22:33:44:55:66 +Type=!vlan bond bridge + +[Link] +Name=lan +WakeOnLan=off +MTUBytes=9000 +''', + 'vlan20.network': ND_EMPTY % ('vlan20', 'ipv6'), + 'vlan20.netdev': ND_VLAN % ('vlan20', 20)}) -[DHCP] -RouteMetric=100 -UseMTU=true -'''}) self.assert_nm(None, '''[keyfile] # devices managed by networkd -unmanaged-devices+=interface-name:en1,interface-name:enblue,interface-name:engreen,''') +unmanaged-devices+=mac:11:22:33:44:55:66,interface-name:vlan20,''') self.assert_nm_udev(None)