From e53f43fee81626c473569cb753029ab9efff5328 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 10:22:09 +0900 Subject: [PATCH 1/9] wait-online: ignore one more error in callback function --- src/network/wait-online/manager.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/network/wait-online/manager.c b/src/network/wait-online/manager.c index 3a2db101ed..260ccce2b6 100644 --- a/src/network/wait-online/manager.c +++ b/src/network/wait-online/manager.c @@ -212,11 +212,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) - return log_error_errno(r, "Failed to create link object: %m"); + if (r < 0) { + log_warning_errno(r, "Failed to create link object for %s(%i), ignoring: %m", ifname, ifindex); + return 0; + } } r = link_update_rtnl(l, mm); From 2401f920f351a4b15560f61b3008328d2f9b87fa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 10:14:30 +0900 Subject: [PATCH 2/9] wait-online: split out link_update_name() No functional changes, just refactoring and preparation for later commits. --- src/network/wait-online/link.c | 50 ++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/network/wait-online/link.c b/src/network/wait-online/link.c index e197e62e26..f379578355 100644 --- a/src/network/wait-online/link.c +++ b/src/network/wait-online/link.c @@ -3,6 +3,7 @@ #include "sd-network.h" #include "alloc-util.h" +#include "format-util.h" #include "hashmap.h" #include "link.h" #include "manager.h" @@ -62,7 +63,7 @@ Link *link_free(Link *l) { return mfree(l); } -int link_update_rtnl(Link *l, sd_netlink_message *m) { +static int link_update_name(Link *l, sd_netlink_message *m) { const char *ifname; int r; @@ -70,29 +71,44 @@ int link_update_rtnl(Link *l, sd_netlink_message *m) { 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; + + 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; +} + +int link_update_rtnl(Link *l, sd_netlink_message *m) { + int r; + + assert(l); + assert(l->manager); + assert(m); + r = sd_rtnl_message_link_get_flags(m, &l->flags); 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)) { - char *new_ifname; - - 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; } From 0d7e58038e3eaaeb28ff82b8b504d8780c736fe7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 10:25:50 +0900 Subject: [PATCH 3/9] wait-online: check received interface name --- src/network/wait-online/link.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/network/wait-online/link.c b/src/network/wait-online/link.c index f379578355..287a91648d 100644 --- a/src/network/wait-online/link.c +++ b/src/network/wait-online/link.c @@ -64,6 +64,7 @@ Link *link_free(Link *l) { } static int link_update_name(Link *l, sd_netlink_message *m) { + char ifname_from_index[IF_NAMESIZE]; const char *ifname; int r; @@ -81,6 +82,18 @@ static int link_update_name(Link *l, sd_netlink_message *m) { 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); From 8f7220fc5effb0823c4c3174a0123c4e990f158f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 10:19:43 +0900 Subject: [PATCH 4/9] wait-online: support alternative names --- src/network/wait-online/link.c | 44 ++++++++++++++++++++++++++++++- src/network/wait-online/link.h | 1 + src/network/wait-online/manager.c | 27 ++++++++++++++++--- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/network/wait-online/link.c b/src/network/wait-online/link.c index 287a91648d..a8ab7f53de 100644 --- a/src/network/wait-online/link.c +++ b/src/network/wait-online/link.c @@ -8,6 +8,7 @@ #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; @@ -56,12 +57,16 @@ 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]; @@ -107,6 +112,39 @@ static int link_update_name(Link *l, sd_netlink_message *m) { 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) { int r; @@ -122,6 +160,10 @@ int link_update_rtnl(Link *l, sd_netlink_message *m) { if (r < 0) return r; + r = link_update_altnames(l, m); + if (r < 0) + return r; + return 0; } diff --git a/src/network/wait-online/link.h b/src/network/wait-online/link.h index 3072a91e56..5dc26d9c40 100644 --- a/src/network/wait-online/link.h +++ b/src/network/wait-online/link.h @@ -14,6 +14,7 @@ struct Link { int ifindex; char *ifname; + char **altnames; unsigned flags; bool required_for_online; diff --git a/src/network/wait-online/manager.c b/src/network/wait-online/manager.c index 260ccce2b6..624029a812 100644 --- a/src/network/wait-online/manager.c +++ b/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; } From 2368ff812d86168dd52d342e4905f2bf7e78d0a2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Nov 2022 22:38:33 +0900 Subject: [PATCH 5/9] network: update comment --- src/network/networkd-link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 151346f198..d93ed71ae8 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2282,7 +2282,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) From dee6c26f3e33c51a05e98f9a418aa12801d1caf5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 12:12:26 +0900 Subject: [PATCH 6/9] test-network: make link_exists() support alternative names --- test/test-network/systemd-networkd-tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index dfef73f030..f1332cd916 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -411,7 +411,7 @@ 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 remove_link(*links, protect=False): for link in links: From d7ff72ec9cb356d2afc6143557e2800dddb5323b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 12:13:15 +0900 Subject: [PATCH 7/9] test-network: fix use of undeclared variable --- test/test-network/systemd-networkd-tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index f1332cd916..62bd1d2d1e 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -839,7 +839,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 From b95d35b5ed4d6d47ad68314f68cb7887546cb41d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 14:00:29 +0900 Subject: [PATCH 8/9] test-network: resolve interface name from alternative name --- test/test-network/systemd-networkd-tests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 62bd1d2d1e..b255eeba6d 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -413,6 +413,9 @@ def create_service_dropin(service, command, reload_command=None, additional_sett def link_exists(link): 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: if protect and link in protected_links: @@ -4236,6 +4239,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, @@ -4250,7 +4256,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) @@ -4266,7 +4272,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) @@ -4282,7 +4288,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) @@ -4298,7 +4304,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) From d8746f1620640b14058619e9fc08d33f2a2c60d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Oct 2022 10:40:31 +0900 Subject: [PATCH 9/9] test-network: explicitly prepare default.link On CentOS CI (Arch), 99-default.link is masked. Let's explicitly provide the same .link file with a different prefix number. --- test/test-network/conf/25-default.link | 1 + test/test-network/systemd-networkd-tests.py | 25 +++++---------------- 2 files changed, 6 insertions(+), 20 deletions(-) create mode 120000 test/test-network/conf/25-default.link diff --git a/test/test-network/conf/25-default.link b/test/test-network/conf/25-default.link new file mode 120000 index 0000000000..dee89415e5 --- /dev/null +++ b/test/test-network/conf/25-default.link @@ -0,0 +1 @@ +../../../network/99-default.link \ No newline at end of file diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index b255eeba6d..2e615ccdf7 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/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, 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 finalize(func, os.path.exists('/sys/bus/netdevsim/devices/netdevsim99/sriov_numvfs')) return f @@ -1064,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) @@ -4207,6 +4191,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: @@ -4215,7 +4201,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'])