Merge pull request #25192 from yuwata/wait-online-altname

wait-online: support alternative interface names

src/network/networkd-link.c

@@ -2238,7 +2238,7 @@ static int link_update_alternative_names(Link *link, sd_netlink_message *message
 
         r = sd_netlink_message_read_strv(message, IFLA_PROP_LIST, IFLA_ALT_IFNAME, &altnames);
         if (r == -ENODATA)
-                /* The message does not have IFLA_PROP_LIST container attribute. It does not means the
+                /* The message does not have IFLA_PROP_LIST container attribute. It does not mean the
                  * interface has no alternative name. */
                 return 0;
         if (r < 0)

src/network/wait-online/link.c

@@ -3,10 +3,12 @@
 #include "sd-network.h"
 
 #include "alloc-util.h"
+#include "format-util.h"
 #include "hashmap.h"
 #include "link.h"
 #include "manager.h"
 #include "string-util.h"
+#include "strv.h"
 
 int link_new(Manager *m, Link **ret, int ifindex, const char *ifname) {
         _cleanup_(link_freep) Link *l = NULL;
@@ -55,15 +57,95 @@ Link *link_free(Link *l) {
         if (l->manager) {
                 hashmap_remove(l->manager->links_by_index, INT_TO_PTR(l->ifindex));
                 hashmap_remove(l->manager->links_by_name, l->ifname);
+
+                STRV_FOREACH(n, l->altnames)
+                        hashmap_remove(l->manager->links_by_name, *n);
         }
 
         free(l->state);
         free(l->ifname);
+        strv_free(l->altnames);
         return mfree(l);
- }
+}
+
+static int link_update_name(Link *l, sd_netlink_message *m) {
+        char ifname_from_index[IF_NAMESIZE];
+        const char *ifname;
+        int r;
+
+        assert(l);
+        assert(l->manager);
+        assert(m);
+
+        r = sd_netlink_message_read_string(m, IFLA_IFNAME, &ifname);
+        if (r == -ENODATA)
+                /* Hmm? But ok. */
+                return 0;
+        if (r < 0)
+                return r;
+
+        if (streq(ifname, l->ifname))
+                return 0;
+
+        /* The kernel sometimes sends wrong ifname change. Let's confirm the received name. */
+        r = format_ifname(l->ifindex, ifname_from_index);
+        if (r < 0)
+                return r;
+
+        if (!streq(ifname, ifname_from_index)) {
+                log_link_debug(l, "New interface name '%s' received from the kernel does not correspond "
+                               "with the name currently configured on the actual interface '%s'. Ignoring.",
+                               ifname, ifname_from_index);
+                return 0;
+        }
+
+        hashmap_remove(l->manager->links_by_name, l->ifname);
+
+        r = free_and_strdup(&l->ifname, ifname);
+        if (r < 0)
+                return r;
+
+        r = hashmap_ensure_put(&l->manager->links_by_name, &string_hash_ops, l->ifname, l);
+        if (r < 0)
+                return r;
+
+        return 0;
+}
+
+static int link_update_altnames(Link *l, sd_netlink_message *m) {
+        _cleanup_strv_free_ char **altnames = NULL;
+        int r;
+
+        assert(l);
+        assert(l->manager);
+        assert(m);
+
+        r = sd_netlink_message_read_strv(m, IFLA_PROP_LIST, IFLA_ALT_IFNAME, &altnames);
+        if (r == -ENODATA)
+                /* The message does not have IFLA_PROP_LIST container attribute. It does not mean the
+                 * interface has no alternative name. */
+                return 0;
+        if (r < 0)
+                return r;
+
+        if (strv_equal(altnames, l->altnames))
+                return 0;
+
+        STRV_FOREACH(n, l->altnames)
+                hashmap_remove(l->manager->links_by_name, *n);
+
+        strv_free_and_replace(l->altnames, altnames);
+
+        STRV_FOREACH(n, l->altnames) {
+                r = hashmap_ensure_put(&l->manager->links_by_name, &string_hash_ops, *n, l);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
 
 int link_update_rtnl(Link *l, sd_netlink_message *m) {
-        const char *ifname;
         int r;
 
         assert(l);
@@ -74,24 +156,13 @@ int link_update_rtnl(Link *l, sd_netlink_message *m) {
         if (r < 0)
                 return r;
 
-        r = sd_netlink_message_read_string(m, IFLA_IFNAME, &ifname);
+        r = link_update_name(l, m);
         if (r < 0)
                 return r;
 
-        if (!streq(l->ifname, ifname)) {
+        r = link_update_altnames(l, m);
-                char *new_ifname;
+        if (r < 0)
-
+                return r;
-                new_ifname = strdup(ifname);
-                if (!new_ifname)
-                        return -ENOMEM;
-
-                assert_se(hashmap_remove(l->manager->links_by_name, l->ifname) == l);
-                free_and_replace(l->ifname, new_ifname);
-
-                r = hashmap_put(l->manager->links_by_name, l->ifname, l);
-                if (r < 0)
-                        return r;
-        }
 
         return 0;
 }

src/network/wait-online/link.h

@@ -14,6 +14,7 @@ struct Link {
 
         int ifindex;
         char *ifname;
+        char **altnames;
         unsigned flags;
 
         bool required_for_online;

src/network/wait-online/manager.c

@@ -12,6 +12,20 @@
 #include "time-util.h"
 #include "util.h"
 
+static bool link_in_command_line_interfaces(Link *link, Manager *m) {
+        assert(link);
+        assert(m);
+
+        if (hashmap_contains(m->command_line_interfaces_by_name, link->ifname))
+                return true;
+
+        STRV_FOREACH(n, link->altnames)
+                if (hashmap_contains(m->command_line_interfaces_by_name, *n))
+                        return true;
+
+        return false;
+}
+
 static bool manager_ignore_link(Manager *m, Link *link) {
         assert(m);
         assert(link);
@@ -22,14 +36,21 @@ static bool manager_ignore_link(Manager *m, Link *link) {
 
         /* if interfaces are given on the command line, ignore all others */
         if (m->command_line_interfaces_by_name &&
-            !hashmap_contains(m->command_line_interfaces_by_name, link->ifname))
+            !link_in_command_line_interfaces(link, m))
                 return true;
 
         if (!link->required_for_online)
                 return true;
 
         /* ignore interfaces we explicitly are asked to ignore */
-        return strv_fnmatch(m->ignored_interfaces, link->ifname);
+        if (strv_fnmatch(m->ignored_interfaces, link->ifname))
+                return true;
+
+        STRV_FOREACH(n, link->altnames)
+                if (strv_fnmatch(m->ignored_interfaces, *n))
+                        return true;
+
+        return false;
 }
 
 static int manager_link_is_online(Manager *m, Link *l, LinkOperationalStateRange s) {
@@ -58,7 +79,7 @@ static int manager_link_is_online(Manager *m, Link *l, LinkOperationalStateRange
         if (streq(l->state, "unmanaged")) {
                 /* If the link is in unmanaged state, then ignore the interface unless the interface is
                  * specified in '--interface/-i' option. */
-                if (!hashmap_contains(m->command_line_interfaces_by_name, l->ifname)) {
+                if (!link_in_command_line_interfaces(l, m)) {
                         log_link_debug(l, "link is not managed by networkd (yet?).");
                         return 0;
                 }
@@ -212,11 +233,13 @@ static int manager_process_link(sd_netlink *rtnl, sd_netlink_message *mm, void *
 
         case RTM_NEWLINK:
                 if (!l) {
-                        log_debug("Found link %i", ifindex);
+                        log_debug("Found link %s(%i)", ifname, ifindex);
 
                         r = link_new(m, &l, ifindex, ifname);
-                        if (r < 0)
+                        if (r < 0) {
-                                return log_error_errno(r, "Failed to create link object: %m");
+                                log_warning_errno(r, "Failed to create link object for %s(%i), ignoring: %m", ifname, ifindex);
+                                return 0;
+                        }
                 }
 
                 r = link_update_rtnl(l, mm);

test/test-network/conf/25-default.link

@@ -0,0 +1 @@
+../../../network/99-default.link

test/test-network/systemd-networkd-tests.py

@@ -181,16 +181,6 @@ def expectedFailureIfRoutingPolicyUIDRangeIsNotAvailable():
 
     return f
 
-def expectedFailureIfLinkFileFieldIsNotSet():
-    def f(func):
-        call_quiet('ip link add name dummy99 type dummy')
-        ret = run('udevadm info -w10s /sys/class/net/dummy99')
-        supported = ret.returncode == 0 and 'E: ID_NET_LINK_FILE=' in ret.stdout
-        remove_link('dummy99')
-        return func if supported else unittest.expectedFailure(func)
-
-    return f
-
 def expectedFailureIfNexthopIsNotAvailable():
     def f(func):
         rc = call_quiet('ip nexthop list')
@@ -236,12 +226,7 @@ def expectedFailureIfNetdevsimWithSRIOVIsNotAvailable():
         except OSError:
             return finalize(func, False)
 
-        if not os.path.exists('/sys/bus/netdevsim/devices/netdevsim99/sriov_numvfs'):
+        return finalize(func, os.path.exists('/sys/bus/netdevsim/devices/netdevsim99/sriov_numvfs'))
-            return finalize(func, False)
-
-        # Also checks if udevd supports .link files, as it seems disabled on CentOS CI (Arch).
-        rc = call_quiet('udevadm info -w10s /sys/class/net/eni99np1')
-        return finalize(func, rc == 0)
 
     return f
 
@@ -411,7 +396,10 @@ def create_service_dropin(service, command, reload_command=None, additional_sett
     create_unit_dropin(f'{service}.service', drop_in)
 
 def link_exists(link):
-    return os.path.exists(os.path.join('/sys/class/net', link, 'ifindex'))
+    return call_quiet(f'ip link show {link}') == 0
+
+def link_resolve(link):
+    return check_output(f'ip link show {link}').split(':')[1].strip()
 
 def remove_link(*links, protect=False):
     for link in links:
@@ -839,7 +827,6 @@ class Utilities():
             if re.search(rf'(?m)^\s*State:\s+{operstate}\s+\({setup_state}\)\s*$', output):
                 return True
 
-        print(output)
         if fail_assert:
             self.fail(f'Timed out waiting for {link} to reach state {operstate}/{setup_state}')
         return False
@@ -1062,15 +1049,14 @@ class NetworkctlTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'Type: loopback')
 
-    @expectedFailureIfLinkFileFieldIsNotSet()
     def test_udev_link_file(self):
-        copy_network_unit('11-dummy.netdev', '11-dummy.network')
+        copy_network_unit('11-dummy.netdev', '11-dummy.network', '25-default.link')
         start_networkd()
         self.wait_online(['test1:degraded'])
 
         output = check_output(*networkctl_cmd, '-n', '0', 'status', 'test1', env=env)
         print(output)
-        self.assertRegex(output, r'Link File: (/usr)?/lib/systemd/network/99-default.link')
+        self.assertRegex(output, r'Link File: /run/systemd/network/25-default.link')
         self.assertRegex(output, r'Network File: /run/systemd/network/11-dummy.network')
 
         output = check_output(*networkctl_cmd, '-n', '0', 'status', 'lo', env=env)
@@ -4229,6 +4215,8 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
 
     @expectedFailureIfNetdevsimWithSRIOVIsNotAvailable()
     def test_sriov(self):
+        copy_network_unit('25-default.link', '25-sriov.network')
+
         call('modprobe netdevsim')
 
         with open('/sys/bus/netdevsim/new_device', mode='w', encoding='utf-8') as f:
@@ -4237,7 +4225,6 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
         with open('/sys/bus/netdevsim/devices/netdevsim99/sriov_numvfs', mode='w', encoding='utf-8') as f:
             f.write('3')
 
-        copy_network_unit('25-sriov.network')
         start_networkd()
         self.wait_online(['eni99np1:routable'])
 
@@ -4261,6 +4248,9 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
         start_networkd()
         self.wait_online(['eni99np1:routable'])
 
+        # the name eni99np1 may be an alternative name.
+        ifname = link_resolve('eni99np1')
+
         output = check_output('ip link show dev eni99np1')
         print(output)
         self.assertRegex(output,
@@ -4275,7 +4265,7 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
             f.write('[Link]\nSR-IOVVirtualFunctions=4\n')
 
         udev_reload()
-        call(*udevadm_cmd, 'trigger', '--action=add', '--settle', '/sys/devices/netdevsim99/net/eni99np1')
+        check_output(*udevadm_cmd, 'trigger', '--action=add', '--settle', f'/sys/devices/netdevsim99/net/{ifname}')
 
         output = check_output('ip link show dev eni99np1')
         print(output)
@@ -4291,7 +4281,7 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
             f.write('[Link]\nSR-IOVVirtualFunctions=\n')
 
         udev_reload()
-        call(*udevadm_cmd, 'trigger', '--action=add', '--settle', '/sys/devices/netdevsim99/net/eni99np1')
+        check_output(*udevadm_cmd, 'trigger', '--action=add', '--settle', f'/sys/devices/netdevsim99/net/{ifname}')
 
         output = check_output('ip link show dev eni99np1')
         print(output)
@@ -4307,7 +4297,7 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
             f.write('[Link]\nSR-IOVVirtualFunctions=2\n')
 
         udev_reload()
-        call(*udevadm_cmd, 'trigger', '--action=add', '--settle', '/sys/devices/netdevsim99/net/eni99np1')
+        check_output(*udevadm_cmd, 'trigger', '--action=add', '--settle', f'/sys/devices/netdevsim99/net/{ifname}')
 
         output = check_output('ip link show dev eni99np1')
         print(output)
@@ -4323,7 +4313,7 @@ class NetworkdSRIOVTests(unittest.TestCase, Utilities):
             f.write('[Link]\nSR-IOVVirtualFunctions=\n')
 
         udev_reload()
-        call(*udevadm_cmd, 'trigger', '--action=add', '--settle', '/sys/devices/netdevsim99/net/eni99np1')
+        check_output(*udevadm_cmd, 'trigger', '--action=add', '--settle', f'/sys/devices/netdevsim99/net/{ifname}')
 
         output = check_output('ip link show dev eni99np1')
         print(output)