Skip to content

Commit

Permalink
Generate udev rules files to rename devices
Browse files Browse the repository at this point in the history
Due to a systemd issue[1], using link files to rename interfaces
doesn't work as expected. Link files will not rename an interface
if it was already renamed, and interfaces are renamed in initrd, so
set-name will often not work as expected when rebooting.

However, rules files will cause a renaming, even if the interface
has been renamed in initrd.

So, while we sort out whether the systemd-udev behaviour is broken
or not, we can simply generate udev rules files with appropriate
renaming info in /run/udev/rules.d/70-netplan-<interface>.rules

A file will be created for non-virtual interfaces with both a
set-name and a driver or a mac address in the match stanza.
(Renaming from name to name doesn't work.)

This is at least a temporary fix to LP: #1770082

As far as testing goes, test successful set-name: generations, and
a few cases where we expect no files to be generated.

[1] systemd/systemd#9006

Signed-off-by: Daniel Axtens <dja@axtens.net>
  • Loading branch information
daxtens committed May 17, 2018
1 parent d158eb9 commit b0c51bf
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 1 deletion.
44 changes: 43 additions & 1 deletion src/networkd.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,47 @@ write_network_file(net_definition* def, const char* rootdir, const char* path)
g_string_free_to_file(s, rootdir, path, ".network");
}

static void
write_rules_file(net_definition* def, const char* rootdir)
{
GString* s = NULL;
g_autofree char* path = g_strjoin(NULL, "run/udev/rules.d/70-netplan-", def->id, ".rules", NULL);

/* do we need to write a .rules file?
* It's only required for reliably setting the name of a physical device
* until systemd issue #9006 is resolved. */
if (def->type >= ND_VIRTUAL)
return;

/* Matching by name does not work.
*
* As far as I can tell, if you match by the name coming out of
* initrd, systemd complains that a link file is matching on a
* renamed name. If you match by the unstable kernel name, the
* device no longer has that name when udevd reads the file, so
* the rule doesn't fire. So only support mac and driver. */
if (!def->set_name || (!def->match.mac && !def->match.driver))
return;

/* build file contents */
s = g_string_sized_new(200);

g_string_append(s, "SUBSYSTEM==\"net\", ACTION==\"add\", ");

if (def->match.driver) {
g_string_append_printf(s,"DRIVERS==\"%s\", ", def->match.driver);
} else {
g_string_append(s, "DRIVERS==\"?*\", ");
}

if (def->match.mac)
g_string_append_printf(s, "ATTR{address}==\"%s\", ", def->match.mac);

g_string_append_printf(s, "NAME=\"%s\"\n", def->set_name);

g_string_free_to_file(s, rootdir, path, NULL);
}

static void
write_wpa_conf(net_definition* def, const char* rootdir)
{
Expand Down Expand Up @@ -443,9 +484,10 @@ write_networkd_conf(net_definition* def, const char* rootdir)
{
g_autofree char* path_base = g_strjoin(NULL, "run/systemd/network/10-netplan-", def->id, NULL);

/* We want this for all backends when renaming, as *.link files are
/* We want this for all backends when renaming, as *.link and *.rules files are
* evaluated by udev, not networkd itself or NetworkManager. */
write_link_file(def, rootdir, path_base);
write_rules_file(def, rootdir);

if (def->backend != BACKEND_NETWORKD) {
g_debug("networkd: definition %s is not for us (backend %i)", def->id, def->backend);
Expand Down
38 changes: 38 additions & 0 deletions tests/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
ND_WIFI_DHCP4 = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv4\n\n[DHCP]\nUseMTU=true\nRouteMetric=600\n'
ND_DHCP6 = '[Match]\nName=%s\n\n[Network]\nDHCP=ipv6\n\n[DHCP]\nUseMTU=true\nRouteMetric=100\n'
ND_DHCPYES = '[Match]\nName=%s\n\n[Network]\nDHCP=yes\n\n[DHCP]\nUseMTU=true\nRouteMetric=100\n'
UDEV_MAC_RULE = 'SUBSYSTEM=="net", ACTION=="add", DRIVERS=="%s", ATTR{address}=="%s", NAME="%s"\n'
UDEV_NO_MAC_RULE = 'SUBSYSTEM=="net", ACTION=="add", DRIVERS=="%s", NAME="%s"\n'


class TestBase(unittest.TestCase):
Expand Down Expand Up @@ -90,6 +92,20 @@ def assert_networkd(self, file_contents_map):
with open(os.path.join(networkd_dir, '10-netplan-' + fname)) as f:
self.assertEqual(f.read(), contents)

def assert_networkd_udev(self, file_contents_map):
udev_dir = os.path.join(self.workdir.name, 'run', 'udev', 'rules.d')
if not file_contents_map:
# it can either not exist, or can only contain 90-netplan.rules
self.assertTrue((not os.path.exists(udev_dir)) or
(os.listdir(udev_dir) == ['90-netplan.rules']))
return

self.assertEqual(set(os.listdir(udev_dir)) - set(['90-netplan.rules']),
{'70-netplan-' + f for f in file_contents_map})
for fname, contents in file_contents_map.items():
with open(os.path.join(udev_dir, '70-netplan-' + fname)) as f:
self.assertEqual(f.read(), contents)

def assert_nm(self, connections_map=None, conf=None):
# check config
conf_path = os.path.join(self.workdir.name, 'run', 'NetworkManager', 'conf.d', 'netplan.conf')
Expand Down Expand Up @@ -296,6 +312,7 @@ def test_eth_optional(self):
UseMTU=true
RouteMetric=100
'''})
self.assert_networkd_udev(None)

def test_eth_wol(self):
self.generate('''network:
Expand All @@ -306,6 +323,7 @@ def test_eth_wol(self):
dhcp4: n''')

self.assert_networkd({'eth0.link': '[Match]\nOriginalName=eth0\n\n[Link]\nWakeOnLan=magic\n'})
self.assert_networkd_udev(None)
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=interface-name:eth0,''')
Expand All @@ -322,6 +340,7 @@ def test_eth_mtu(self):
dhcp4: n''')

self.assert_networkd({'eth1.link': '[Match]\nOriginalName=eth1\n\n[Link]\nWakeOnLan=off\nMTUBytes=1280\n'})
self.assert_networkd_udev(None)

def test_mtu_all(self):
self.generate(textwrap.dedent("""
Expand All @@ -347,6 +366,7 @@ def test_mtu_all(self):
'eth1.link': '[Match]\nOriginalName=eth1\n\n[Link]\nWakeOnLan=off\nMTUBytes=1280\n',
'eth1.network': '[Match]\nName=eth1\n\n[Network]\nBond=bond0\nLinkLocalAddressing=no\n'
})
self.assert_networkd_udev(None)

def test_eth_match_by_driver_rename(self):
self.generate('''network:
Expand All @@ -358,6 +378,7 @@ def test_eth_match_by_driver_rename(self):
set-name: lom1''')

self.assert_networkd({'def1.link': '[Match]\nDriver=ixgbe\n\n[Link]\nName=lom1\nWakeOnLan=off\n'})
self.assert_networkd_udev({'def1.rules': (UDEV_NO_MAC_RULE % ('ixgbe', 'lom1'))})
# NM cannot match by driver, so blacklisting needs to happen via udev
self.assert_nm(None, None)
self.assert_nm_udev('ACTION=="add|change", SUBSYSTEM=="net", ENV{ID_NET_DRIVER}=="ixgbe", ENV{NM_UNMANAGED}="1"\n')
Expand All @@ -372,6 +393,7 @@ def test_eth_match_by_mac_rename(self):
set-name: lom1''')

self.assert_networkd({'def1.link': '[Match]\nMACAddress=11:22:33:44:55:66\n\n[Link]\nName=lom1\nWakeOnLan=off\n'})
self.assert_networkd_udev({'def1.rules': (UDEV_MAC_RULE % ('?*', '11:22:33:44:55:66', 'lom1'))})
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=mac:11:22:33:44:55:66,''')
Expand All @@ -385,6 +407,7 @@ def test_eth_implicit_name_match_dhcp4(self):
dhcp4: y''')

self.assert_networkd({'engreen.network': ND_DHCP4 % 'engreen'})
self.assert_networkd_udev(None)

def test_eth_match_dhcp4(self):
self.generate('''network:
Expand All @@ -405,6 +428,7 @@ def test_eth_match_dhcp4(self):
UseMTU=true
RouteMetric=100
'''})
self.assert_networkd_udev(None)
self.assert_nm_udev('ACTION=="add|change", SUBSYSTEM=="net", ENV{ID_NET_DRIVER}=="ixgbe", ENV{NM_UNMANAGED}="1"\n')

def test_eth_match_name(self):
Expand All @@ -417,6 +441,7 @@ def test_eth_match_name(self):
dhcp4: true''')

self.assert_networkd({'def1.network': ND_DHCP4 % 'green'})
self.assert_networkd_udev(None)
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=interface-name:green,''')
Expand All @@ -435,6 +460,7 @@ def test_eth_set_mac(self):
self.assert_networkd({'def1.network': ND_DHCP4 % 'green',
'def1.link': '[Match]\nOriginalName=green\n\n[Link]\nWakeOnLan=off\nMACAddress=00:01:02:03:04:05\n'
})
self.assert_networkd_udev(None)

def test_eth_match_name_rename(self):
self.generate('''network:
Expand All @@ -449,6 +475,10 @@ def test_eth_match_name_rename(self):
# the .network needs to match on the renamed name
self.assert_networkd({'def1.link': '[Match]\nOriginalName=green\n\n[Link]\nName=blue\nWakeOnLan=off\n',
'def1.network': ND_DHCP4 % 'blue'})

# The udev rules engine does support renaming by name
self.assert_networkd_udev(None)

self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=interface-name:blue,''')
Expand All @@ -462,6 +492,7 @@ def test_eth_match_all_names(self):
dhcp4: true''')

self.assert_networkd({'def1.network': ND_DHCP4 % '*'})
self.assert_networkd_udev(None)
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=interface-name:*,''')
Expand All @@ -477,6 +508,7 @@ def test_eth_match_all(self):

self.assert_networkd({'def1.network': '[Match]\n\n[Network]\nDHCP=ipv4\n\n'
'[DHCP]\nUseMTU=true\nRouteMetric=100\n'})
self.assert_networkd_udev(None)
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=type:ethernet,''')
Expand Down Expand Up @@ -561,6 +593,7 @@ def test_eth_def_renderer(self):
dhcp4: true''')

self.assert_networkd({'eth0.network': ND_DHCP4 % 'eth0'})
self.assert_networkd_udev(None)
self.assert_nm(None, '''[keyfile]
# devices managed by networkd
unmanaged-devices+=interface-name:eth0,''')
Expand Down Expand Up @@ -1952,6 +1985,7 @@ def test_eth_match_by_driver_rename(self):
set-name: lom1''')

self.assert_networkd({'def1.link': '[Match]\nDriver=ixgbe\n\n[Link]\nName=lom1\nWakeOnLan=off\n'})
self.assert_networkd_udev({'def1.rules': (UDEV_NO_MAC_RULE % ('ixgbe', 'lom1'))})
self.assert_nm({'def1': '''[connection]
id=netplan-def1
type=ethernet
Expand Down Expand Up @@ -1979,6 +2013,7 @@ def test_eth_match_by_mac_rename(self):
set-name: lom1''')

self.assert_networkd({'def1.link': '[Match]\nMACAddress=11:22:33:44:55:66\n\n[Link]\nName=lom1\nWakeOnLan=off\n'})
self.assert_networkd_udev({'def1.rules': (UDEV_MAC_RULE % ('?*', '11:22:33:44:55:66', 'lom1'))})
self.assert_nm({'def1': '''[connection]
id=netplan-def1
type=ethernet
Expand Down Expand Up @@ -2085,6 +2120,9 @@ def test_eth_match_name_rename(self):
set-name: blue
dhcp4: true''')

# The udev rules engine does support renaming by name
self.assert_networkd_udev(None)

# NM needs to match on the renamed name
self.assert_nm({'def1': '''[connection]
id=netplan-def1
Expand Down

0 comments on commit b0c51bf

Please sign in to comment.