Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The interface detection/renaming logic is racy #48

Open
nh2 opened this issue Jan 2, 2022 · 1 comment
Open

The interface detection/renaming logic is racy #48

nh2 opened this issue Jan 2, 2022 · 1 comment

Comments

@nh2
Copy link
Contributor

nh2 commented Jan 2, 2022

As I reported in NixOS/nixpkgs#153134 (comment), the fact that nixops generates a udev rule to rename an interface based on a MAC address, to a name like eth0, will race with the interface detection of the kernel (which may change across kernel versions) and naming schemes of systemd (which may change across systemd versions).

This is done here:

def _get_udev_rule_for(self, interface):
"""
Get lines suitable for services.udev.extraRules for 'interface',
and thus essentially map the device name to a hardware address.
"""
cmd = "ip addr show \"{0}\" | sed -n -e 's|^.*link/ether *||p'"
cmd += " | cut -d' ' -f1"
mac_addr = self.run_command(cmd.format(interface), capture_stdout=True).strip()
rule = 'ACTION=="add", SUBSYSTEM=="net", ATTR{{address}}=="{0}", '
rule += 'NAME="{1}"'
return rule.format(mac_addr, interface)

As outlined there, I think nixops-hetzner could address it by:

  • Not using names eth0 as the target names as suggested, but something like internet0.

And further, given that udev-rule based renaming is now apparently deprecated:

  • Switching from the generated udev extraRules for renaming to a .link file.
@nh2 nh2 changed the title The interface The interface detection/renaming logic is racy Jan 2, 2022
@nh2
Copy link
Contributor Author

nh2 commented Jan 2, 2022

Not using names eth0 as the target names as suggested, but something like internet0.

I'm using this patch on NixOps 1.6 to address it:

diff --git a/nixops/backends/hetzner.py b/nixops/backends/hetzner.py
index 0ddf377..20faa8d 100644
--- a/nixops/backends/hetzner.py
+++ b/nixops/backends/hetzner.py
@@ -512,7 +512,7 @@ class HetznerState(MachineState):
         cmd = "ip addr show | sed -n -e 's/^[0-9]*: *//p' | cut -d: -f1"
         return self.run_command(cmd, capture_stdout=True).splitlines()
 
-    def _get_udev_rule_for(self, interface):
+    def _get_udev_rule_for(self, interface, stable_interface_name):
         """
         Get lines suitable for services.udev.extraRules for 'interface',
         and thus essentially map the device name to a hardware address.
@@ -524,7 +524,7 @@ class HetznerState(MachineState):
 
         rule = 'ACTION=="add", SUBSYSTEM=="net", ATTR{{address}}=="{0}", '
         rule += 'NAME="{1}"'
-        return rule.format(mac_addr, interface)
+        return rule.format(mac_addr, stable_interface_name)
 
     def _get_ipv4_addr_and_prefix_for(self, interface):
         """
@@ -586,11 +586,19 @@ class HetznerState(MachineState):
 
         server = self._get_server_by_ip(self.main_ipv4)
 
+        # Stable interface names:
+        # Based on the MAC address, we give each interface
+        # a stable name "net0", "net1", and so on.
+        # The "net" prefix does not collide with names that the kernel
+        # or systemd would assign.
+
         # Global networking options
         defgw_ip, defgw_dev = self._get_default_gw()
+        defgw_stable_iface_name = None  # assgined in loop below
         v6defgw = None
 
-        # Interface-specific networking options
+        # Interface-specific networking options.
+        stable_iface_index = 1
         for iface in self._get_ethernet_interfaces():
             if iface == "lo":
                 continue
@@ -599,10 +607,13 @@ class HetznerState(MachineState):
             if result is None:
                 continue
 
-            udev_rules.append(self._get_udev_rule_for(iface))
+            stable_iface_name = "net" + str(stable_iface_index)
+            stable_iface_index += 1
+
+            udev_rules.append(self._get_udev_rule_for(iface, stable_iface_name))
 
             ipv4addr, prefix = result
-            iface_attrs[iface] = {
+            iface_attrs[stable_iface_name] = {
                 'ipv4': {
                     'addresses': [
                         {'address': ipv4addr, 'prefixLength': int(prefix)},
@@ -622,13 +633,15 @@ class HetznerState(MachineState):
             #   https://github.com/NixOS/nixops/pull/1032#issuecomment-433741624
             if iface != defgw_dev:
                 net = self._calculate_ipv4_subnet(ipv4addr, int(prefix))
-                iface_attrs[iface]['ipv4'] = {
+                iface_attrs[stable_iface_name]['ipv4'] = {
                     'routes': [{
                         'address': net,
                         'prefixLength': int(prefix),
                         'via': defgw_ip,
                     }],
                 }
+            else:
+                defgw_stable_iface_name = stable_iface_name
 
             # IPv6 subnets only for eth0
             v6subnets = []
@@ -644,9 +657,9 @@ class HetznerState(MachineState):
                         v6defgw.get('address') == subnet.gateway)
                 v6defgw = {
                     'address': subnet.gateway,
-                    'interface': defgw_dev,
+                    'interface': defgw_stable_iface_name,
                 }
-            iface_attrs[iface]['ipv6'] = { 'addresses': v6subnets }
+            iface_attrs[stable_iface_name]['ipv6'] = { 'addresses': v6subnets }
 
         self.net_info = {
             'services': {
@@ -656,7 +669,7 @@ class HetznerState(MachineState):
                 'interfaces': iface_attrs,
                 'defaultGateway': {
                     'address': defgw_ip,
-                    'interface': defgw_dev,
+                    'interface': defgw_stable_iface_name,
                 },
                 'defaultGateway6': v6defgw,
                 'nameservers': self._get_nameservers(),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant