Skip to content

Commit

Permalink
Do not try to match/rename duplicated MAC of vlan (LP: #1888726) (#166)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@slyon
slyon committed Oct 27, 2020
1 parent 449957e commit 83c1c1c
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 73 deletions.
21 changes: 10 additions & 11 deletions src/networkd.c
View file
Expand Up @@ -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");
Expand Down
15 changes: 9 additions & 6 deletions tests/generator/base.py
View file
Expand Up @@ -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\
Expand All @@ -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):
Expand Down
103 changes: 47 additions & 56 deletions tests/generator/test_vlans.py
View file
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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,''')
Expand All @@ -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)


Expand Down

0 comments on commit 83c1c1c

Please sign in to comment.