From 3a390124b7948f42972316f303489776e3a8659f Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Thu, 19 Dec 2019 13:14:42 -0500 Subject: [PATCH 1/7] network: rename linux_configure_after_setting_mtu() to linux_configure_continue() This is a simple rename to make the function name more generic; the next commit expands its usage to more than only the mtu handler. --- src/network/networkd-link.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index f353833e0d..732fd91e3c 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1308,7 +1308,7 @@ static int link_set_proxy_arp(Link *link) { return 0; } -static int link_configure_after_setting_mtu(Link *link); +static int link_configure_continue(Link *link); static int set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { int r; @@ -1329,7 +1329,7 @@ static int set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) log_link_debug(link, "Setting MTU done."); if (link->state == LINK_STATE_INITIALIZED) { - r = link_configure_after_setting_mtu(link); + r = link_configure_continue(link); if (r < 0) link_enter_failed(link); } @@ -2785,10 +2785,22 @@ static int link_configure(Link *link) { if (r < 0) return r; - return link_configure_after_setting_mtu(link); + return link_configure_continue(link); } -static int link_configure_after_setting_mtu(Link *link) { +/* The configuration continues in this separate function, instead of + * including this in the above link_configure() function, for two + * reasons: + * 1) some devices reset the link when the mtu is set, which caused + * an infinite loop here in networkd; see: + * https://github.com/systemd/systemd/issues/6593 + * https://github.com/systemd/systemd/issues/9831 + * 2) if ipv6ll is disabled, then bringing the interface up must be + * delayed until after we get confirmation from the kernel that + * the addr_gen_mode parameter has been set (via netlink), see: + * https://github.com/systemd/systemd/issues/13882 + */ +static int link_configure_continue(Link *link) { int r; assert(link); From 9524014ee63826368ff0e18f5ac9d3483c458f5c Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Thu, 19 Dec 2019 13:17:45 -0500 Subject: [PATCH 2/7] network: add link->setting_genmode flag Similar to the link->setting_mtu flag, this delays continued configuration until after the genmode has been successfully set; this is important because we do not want networkd to raise the interface before the genmode has been set, as if we're disabling the genmode, raising the interface before we have successfully disabled the genmode will result in the kernel creating a ipv6ll address, even though we don't want that. --- src/network/networkd-link.c | 15 +++++++++++++-- src/network/networkd-link.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 732fd91e3c..2913bf946d 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1598,12 +1598,22 @@ static int link_address_genmode_handler(sd_netlink *rtnl, sd_netlink_message *m, assert(link); + link->setting_genmode = false; + if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; r = sd_netlink_message_get_errno(m); if (r < 0) log_link_message_warning_errno(link, m, r, "Could not set address genmode for interface, ignoring"); + else + log_link_debug(link, "Setting address genmode done."); + + if (link->state == LINK_STATE_INITIALIZED) { + r = link_configure_continue(link); + if (r < 0) + link_enter_failed(link); + } return 1; } @@ -1618,7 +1628,7 @@ static int link_configure_addrgen_mode(Link *link) { assert(link->manager); assert(link->manager->rtnl); - if (!socket_ipv6_is_supported()) + if (!socket_ipv6_is_supported() || link->setting_genmode) return 0; log_link_debug(link, "Setting address genmode for link"); @@ -1662,6 +1672,7 @@ static int link_configure_addrgen_mode(Link *link) { return log_link_error_errno(link, r, "Could not send rtnetlink message: %m"); link_ref(link); + link->setting_genmode = true; return 0; } @@ -2807,7 +2818,7 @@ static int link_configure_continue(Link *link) { assert(link->network); assert(link->state == LINK_STATE_INITIALIZED); - if (link->setting_mtu) + if (link->setting_mtu || link->setting_genmode) return 0; /* The kernel resets ipv6 mtu after changing device mtu; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 33be661828..59a5107311 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -118,6 +118,7 @@ typedef struct Link { bool routing_policy_rules_configured:1; bool qdiscs_configured:1; bool setting_mtu:1; + bool setting_genmode:1; bool ipv6_mtu_set:1; LIST_HEAD(Address, pool_addresses); From 0917a2717810d3c6b5b91e9fb057b2520e3ad733 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Sat, 4 Jan 2020 18:41:18 -0500 Subject: [PATCH 3/7] network: if ipv6ll is disabled, enumerate tentative ipv6 addrs before dropping foreign addrs The kernel will create an ipv6ll tentative address immediately when an interface is raised if addr_gen_mode is not disabled; and, the kernel does not notify netlink listeners about any tentative addresses. So it's possible for an interface to contain tentative ipv6 link-local address(es) that networkd doesn't know about when all foreign addresses are dropped. In this case, networkd is later notified about the new ipv6ll address(es) after they finish DAD and are no longer tentative; but since that's after networkd has already dropped foreign addresses, they are incorrectly left on the interface. --- src/network/networkd-link.c | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 2913bf946d..fa6ac4b8f7 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2506,6 +2506,48 @@ static bool link_address_is_dynamic(Link *link, Address *address) { return false; } +static int link_enumerate_ipv6_tentative_addresses(Link *link) { + _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL; + sd_netlink_message *addr; + int r; + + assert(link); + assert(link->manager); + assert(link->manager->rtnl); + + r = sd_rtnl_message_new_addr(link->manager->rtnl, &req, RTM_GETADDR, 0, AF_INET6); + if (r < 0) + return r; + + r = sd_netlink_call(link->manager->rtnl, req, 0, &reply); + if (r < 0) + return r; + + for (addr = reply; addr; addr = sd_netlink_message_next(addr)) { + unsigned char flags; + int ifindex; + + r = sd_rtnl_message_addr_get_ifindex(addr, &ifindex); + if (r < 0) { + log_link_warning_errno(link, r, "rtnl: invalid ifindex, ignoring: %m"); + continue; + } else if (link->ifindex != ifindex) + continue; + + r = sd_rtnl_message_addr_get_flags(addr, &flags); + if (r < 0) { + log_link_warning_errno(link, r, "rtnl: received address message with invalid flags, ignoring: %m"); + continue; + } else if (!(flags & IFA_F_TENTATIVE)) + continue; + + log_link_debug(link, "Found tentative ipv6 link-local address"); + (void) manager_rtnl_process_address(link->manager->rtnl, addr, link->manager); + } + + return 0; +} + static int link_drop_foreign_config(Link *link) { Address *address; Neighbor *neighbor; @@ -2513,6 +2555,14 @@ static int link_drop_foreign_config(Link *link) { Iterator i; int r; + /* The kernel doesn't notify us about tentative addresses; + * so if ipv6ll is disabled, we need to enumerate them now so we can drop them below */ + if (!link_ipv6ll_enabled(link)) { + r = link_enumerate_ipv6_tentative_addresses(link); + if (r < 0) + return r; + } + SET_FOREACH(address, link->addresses_foreign, i) { /* we consider IPv6LL addresses to be managed by the kernel */ if (address->family == AF_INET6 && in_addr_is_link_local(AF_INET6, &address->in_addr) == 1 && link_ipv6ll_enabled(link)) From 4c64965257637c055a9f12e5d1f820bbc72fcf19 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Mon, 6 Jan 2020 16:35:28 -0500 Subject: [PATCH 4/7] network: drop foreign config after addr_gen_mode has been set Interfaces may come up at any time, even during our initialization of them, for various reasons; e.g. the kernel will raise VLAN when its parent is raised; or we will raise an interface if configured with BindCarrier and its associated interfaces come up. When LinkLocalAddressing has been disabled for ipv6, we disable addr_gen_mode in the kernel, so it will not automatically create a ipv6ll address when the interface is raised. However, we currently drop all foreign addresses before disabling addr_gen_mode. If the link has been up for a long time, then its kernel-created ipv6ll address will be correctly dropped. If the link is down, and stays down until we raise it after finishing configuration, the addr_gen_mode setting will be disabled when the interface is raised and the kernel will not create any ipv6ll address. However, if the interface is raised after dropping foreign config, but before we have disabled addr_gen_mode, the kernel will create a ipv6ll tentative address that will eventually finish DAD and become a working ipv6ll address, even though we have been configured to disable ipv6ll. Moving our call to drop foreign addresses to after we have successfully set addr_gen_mode closes this window; after we disable addr_gen_mode, we can safely remove foreign ipv6ll addresses (including tentative ones) and be sure that the kernel will not create any more. Fixes: #13882. --- src/network/networkd-link.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index fa6ac4b8f7..d9a695b3a4 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2734,15 +2734,6 @@ static int link_configure(Link *link) { if (link->iftype == ARPHRD_CAN) return link_configure_can(link); - /* Drop foreign config, but ignore loopback or critical devices. - * We do not want to remove loopback address or addresses used for root NFS. */ - if (!(link->flags & IFF_LOOPBACK) && - link->network->keep_configuration != KEEP_CONFIGURATION_YES) { - r = link_drop_foreign_config(link); - if (r < 0) - return r; - } - /* If IPv6 configured that is static IPv6 address and IPv6LL autoconfiguration is enabled * for this interface, then enable IPv6 */ (void) link_update_ipv6_sysctl(link); @@ -2871,6 +2862,15 @@ static int link_configure_continue(Link *link) { if (link->setting_mtu || link->setting_genmode) return 0; + /* Drop foreign config, but ignore loopback or critical devices. + * We do not want to remove loopback address or addresses used for root NFS. */ + if (!(link->flags & IFF_LOOPBACK) && + link->network->keep_configuration != KEEP_CONFIGURATION_YES) { + r = link_drop_foreign_config(link); + if (r < 0) + return r; + } + /* The kernel resets ipv6 mtu after changing device mtu; * we must set this here, after we've set device mtu */ r = link_set_ipv6_mtu(link); From 19cf3143cf9f69e0c3c86f7ea16773b734c55a92 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 8 Jan 2020 07:25:16 -0500 Subject: [PATCH 5/7] test-network: rename check_operstate() to wait_operstate() Rename only, no functional change; this makes following patches easier to read. --- test/test-network/systemd-networkd-tests.py | 52 ++++++++++----------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 43349ff207..ae591219d9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -381,7 +381,7 @@ class Utilities(): def check_link_exists(self, link): self.assertTrue(link_exists(link)) - def check_operstate(self, link, expected, show_status=True, setup_state='configured'): + def wait_operstate(self, link, expected, show_status=True, setup_state='configured'): self.assertRegex(get_operstate(link, show_status, setup_state), expected) def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured', setup_timeout=5): @@ -492,7 +492,7 @@ class NetworkctlTests(unittest.TestCase, Utilities): check_output(*networkctl_cmd, 'reload', env=env) time.sleep(3) self.check_link_exists('test1') - self.check_operstate('test1', 'off', setup_state='unmanaged') + self.wait_operstate('test1', 'off', setup_state='unmanaged') copy_unit_to_networkd_unit_path('11-dummy.network') check_output(*networkctl_cmd, 'reload', env=env) @@ -501,15 +501,15 @@ class NetworkctlTests(unittest.TestCase, Utilities): remove_unit_from_networkd_path(['11-dummy.network']) check_output(*networkctl_cmd, 'reload', env=env) time.sleep(1) - self.check_operstate('test1', 'degraded', setup_state='unmanaged') + self.wait_operstate('test1', 'degraded', setup_state='unmanaged') remove_unit_from_networkd_path(['11-dummy.netdev']) check_output(*networkctl_cmd, 'reload', env=env) - self.check_operstate('test1', 'degraded', setup_state='unmanaged') + self.wait_operstate('test1', 'degraded', setup_state='unmanaged') copy_unit_to_networkd_unit_path('11-dummy.netdev', '11-dummy.network') check_output(*networkctl_cmd, 'reload', env=env) - self.check_operstate('test1', 'degraded') + self.wait_operstate('test1', 'degraded') def test_glob(self): copy_unit_to_networkd_unit_path('11-dummy.netdev', '11-dummy.network') @@ -807,8 +807,8 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.wait_online(['bridge99', 'test1:degraded'], bool_any=True) - self.check_operstate('bridge99', '(off|no-carrier)', setup_state='configuring') - self.check_operstate('test1', 'degraded') + self.wait_operstate('bridge99', '(off|no-carrier)', setup_state='configuring') + self.wait_operstate('test1', 'degraded') def test_bridge(self): copy_unit_to_networkd_unit_path('25-bridge.netdev', '25-bridge-configure-without-carrier.network') @@ -1885,7 +1885,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.check_link_exists('dummy98') - self.check_operstate('dummy98', 'off', setup_state='unmanaged') + self.wait_operstate('dummy98', 'off', setup_state='unmanaged') def test_ipv6_address_label(self): copy_unit_to_networkd_unit_path('25-ipv6-address-label-section.network', '12-dummy.netdev') @@ -2081,7 +2081,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'UP,LOWER_UP') self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1') - self.check_operstate('test1', 'routable') + self.wait_operstate('test1', 'routable') check_output('ip link add dummy99 type dummy') check_output('ip link set dummy99 up') @@ -2090,7 +2090,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'UP,LOWER_UP') self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1') - self.check_operstate('test1', 'routable') + self.wait_operstate('test1', 'routable') check_output('ip link del dummy98') time.sleep(2) @@ -2098,7 +2098,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'UP,LOWER_UP') self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1') - self.check_operstate('test1', 'routable') + self.wait_operstate('test1', 'routable') check_output('ip link set dummy99 down') time.sleep(2) @@ -2107,7 +2107,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.assertNotRegex(output, 'UP,LOWER_UP') self.assertRegex(output, 'DOWN') self.assertNotRegex(output, '192.168.10') - self.check_operstate('test1', 'off') + self.wait_operstate('test1', 'off') check_output('ip link set dummy99 up') time.sleep(2) @@ -2115,7 +2115,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'UP,LOWER_UP') self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1') - self.check_operstate('test1', 'routable') + self.wait_operstate('test1', 'routable') def test_domain(self): copy_unit_to_networkd_unit_path('12-dummy.netdev', '24-search-domain.network') @@ -2348,30 +2348,30 @@ class NetworkdBondTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'MASTER,UP,LOWER_UP') - self.check_operstate('dummy98', 'enslaved') - self.check_operstate('test1', 'enslaved') - self.check_operstate('bond99', 'routable') + self.wait_operstate('dummy98', 'enslaved') + self.wait_operstate('test1', 'enslaved') + self.wait_operstate('bond99', 'routable') check_output('ip link set dummy98 down') time.sleep(2) - self.check_operstate('dummy98', 'off') - self.check_operstate('test1', 'enslaved') - self.check_operstate('bond99', 'degraded-carrier') + self.wait_operstate('dummy98', 'off') + self.wait_operstate('test1', 'enslaved') + self.wait_operstate('bond99', 'degraded-carrier') check_output('ip link set dummy98 up') time.sleep(2) - self.check_operstate('dummy98', 'enslaved') - self.check_operstate('test1', 'enslaved') - self.check_operstate('bond99', 'routable') + self.wait_operstate('dummy98', 'enslaved') + self.wait_operstate('test1', 'enslaved') + self.wait_operstate('bond99', 'routable') check_output('ip link set dummy98 down') check_output('ip link set test1 down') time.sleep(2) - self.check_operstate('dummy98', 'off') - self.check_operstate('test1', 'off') + self.wait_operstate('dummy98', 'off') + self.wait_operstate('test1', 'off') for trial in range(30): if trial > 0: @@ -2493,12 +2493,12 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): self.assertEqual(call('ip link del test1'), 0) time.sleep(3) - self.check_operstate('bridge99', 'degraded-carrier') + self.wait_operstate('bridge99', 'degraded-carrier') check_output('ip link del dummy98') time.sleep(3) - self.check_operstate('bridge99', 'no-carrier') + self.wait_operstate('bridge99', 'no-carrier') output = check_output('ip address show bridge99') print(output) From a4632dc7d131a4ec41b6d59b8ab5e592ad72f346 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 8 Jan 2020 07:28:29 -0500 Subject: [PATCH 6/7] test-network: convert wait_operstate() to recheck condition for timeout seconds Failing after a single check leads to extra sleeps scattered through test cases, and can also lead to false failures. Instead perform a recheck for a number of seconds until the state matches, and fail only if the timeout is exceeded. This allows removing all the manual sleeps in the testcases. --- test/test-network/systemd-networkd-tests.py | 57 +++++++++++---------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index ae591219d9..7a796d479e 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -368,21 +368,41 @@ def restart_networkd(sleep_sec=0, show_logs=True, remove_state_files=True): stop_networkd(show_logs, remove_state_files) start_networkd(sleep_sec) -def get_operstate(link, show_status=True, setup_state='configured'): - output = check_output(*networkctl_cmd, '-n', '0', 'status', link, env=env) - if show_status: - print(output) - for line in output.splitlines(): - if 'State:' in line and (not setup_state or setup_state in line): - return line.split()[1] - return None class Utilities(): def check_link_exists(self, link): self.assertTrue(link_exists(link)) - def wait_operstate(self, link, expected, show_status=True, setup_state='configured'): - self.assertRegex(get_operstate(link, show_status, setup_state), expected) + def wait_operstate(self, link, operstate='degraded', setup_state='configured', setup_timeout=5, fail_assert=True): + """Wait for the link to reach the specified operstate and/or setup state. + + Specify None or '' for either operstate or setup_state to ignore that state. + This will recheck until the state conditions are met or the timeout expires. + + If the link successfully matches the requested state, this returns True. + If this times out waiting for the link to match, the behavior depends on the + 'fail_assert' parameter; if True, this causes a test assertion failure, + otherwise this returns False. The default is to cause assertion failure. + + Note that this function matches on *exactly* the given operstate and setup_state. + To wait for a link to reach *or exceed* a given operstate, use wait_online(). + """ + if not operstate: + operstate = r'\S+' + if not setup_state: + setup_state = r'\S+' + + for secs in range(setup_timeout + 1): + output = check_output(*networkctl_cmd, '-n', '0', 'status', link, env=env) + print(output) + if re.search(rf'(?m)^\s*State:\s+{operstate}\s+\({setup_state}\)\s*$', output): + return True + # don't bother sleeping if time is up + if secs < setup_timeout: + time.sleep(1) + if fail_assert: + self.fail(f'Timed out waiting for {link} to reach state {operstate}/{setup_state}') + return False def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured', setup_timeout=5): args = wait_online_cmd + [f'--timeout={timeout}'] + [f'--interface={link}' for link in links_with_operstate] @@ -490,8 +510,6 @@ class NetworkctlTests(unittest.TestCase, Utilities): copy_unit_to_networkd_unit_path('11-dummy.netdev') check_output(*networkctl_cmd, 'reload', env=env) - time.sleep(3) - self.check_link_exists('test1') self.wait_operstate('test1', 'off', setup_state='unmanaged') copy_unit_to_networkd_unit_path('11-dummy.network') @@ -500,7 +518,6 @@ class NetworkctlTests(unittest.TestCase, Utilities): remove_unit_from_networkd_path(['11-dummy.network']) check_output(*networkctl_cmd, 'reload', env=env) - time.sleep(1) self.wait_operstate('test1', 'degraded', setup_state='unmanaged') remove_unit_from_networkd_path(['11-dummy.netdev']) @@ -2353,14 +2370,12 @@ class NetworkdBondTests(unittest.TestCase, Utilities): self.wait_operstate('bond99', 'routable') check_output('ip link set dummy98 down') - time.sleep(2) self.wait_operstate('dummy98', 'off') self.wait_operstate('test1', 'enslaved') self.wait_operstate('bond99', 'degraded-carrier') check_output('ip link set dummy98 up') - time.sleep(2) self.wait_operstate('dummy98', 'enslaved') self.wait_operstate('test1', 'enslaved') @@ -2368,19 +2383,11 @@ class NetworkdBondTests(unittest.TestCase, Utilities): check_output('ip link set dummy98 down') check_output('ip link set test1 down') - time.sleep(2) self.wait_operstate('dummy98', 'off') self.wait_operstate('test1', 'off') - for trial in range(30): - if trial > 0: - time.sleep(1) - output = check_output('ip address show bond99') - print(output) - if get_operstate('bond99') == 'no-carrier': - break - else: + if not self.wait_operstate('bond99', 'no-carrier', setup_timeout=30, fail_assert=False): # Huh? Kernel does not recognize that all slave interfaces are down? # Let's confirm that networkd's operstate is consistent with ip's result. self.assertNotRegex(output, 'NO-CARRIER') @@ -2491,12 +2498,10 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): self.assertRegex(output, 'ff00::/8 table local metric 256 pref medium') self.assertEqual(call('ip link del test1'), 0) - time.sleep(3) self.wait_operstate('bridge99', 'degraded-carrier') check_output('ip link del dummy98') - time.sleep(3) self.wait_operstate('bridge99', 'no-carrier') From 0c020321c83e6f09834ae5be57a2a3c3acf1427a Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 8 Jan 2020 07:32:19 -0500 Subject: [PATCH 7/7] test-network: simplify wait_online() by calling wait_operstate() The wait_operstate() function now rechecks the condition for a timeout, so the wait_online() function can simply call it to check for the setup_state. --- test/test-network/systemd-networkd-tests.py | 37 ++++++++++++--------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 7a796d479e..f45e948b68 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -405,6 +405,25 @@ class Utilities(): return False def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured', setup_timeout=5): + """Wait for the link(s) to reach the specified operstate and/or setup state. + + This is similar to wait_operstate() but can be used for multiple links, + and it also calls systemd-networkd-wait-online to wait for the given operstate. + The operstate should be specified in the link name, like 'eth0:degraded'. + If just a link name is provided, wait-online's default operstate to wait for is degraded. + + The 'timeout' parameter controls the systemd-networkd-wait-online timeout, and the + 'setup_timeout' controls the per-link timeout waiting for the setup_state. + + Set 'bool_any' to True to wait for any (instead of all) of the given links. + If this is set, no setup_state checks are done. + + Note that this function waits for the link(s) to reach *or exceed* the given operstate. + However, the setup_state, if specified, must be matched *exactly*. + + This returns if the link(s) reached the requested operstate/setup_state; otherwise it + raises CalledProcessError or fails test assertion. + """ args = wait_online_cmd + [f'--timeout={timeout}'] + [f'--interface={link}' for link in links_with_operstate] if bool_any: args += ['--any'] @@ -416,22 +435,8 @@ class Utilities(): print(output) raise if not bool_any and setup_state: - # check at least once now, then once per sec for setup_timeout secs - for secs in range(setup_timeout + 1): - for link in links_with_operstate: - output = check_output(*networkctl_cmd, '-n', '0', 'status', link.split(':')[0]) - print(output) - if not re.search(rf'(?m)^\s*State:.*({setup_state}).*$', output): - # this link isn't in the right state; break into the sleep below - break - else: - # all the links were in the right state; break to exit the timer loop - break - # don't bother sleeping if time is up - if secs < setup_timeout: - time.sleep(1) - else: - self.fail(f'link {link} state does not match {setup_state}') + for link in links_with_operstate: + self.wait_operstate(link.split(':')[0], None, setup_state, setup_timeout) def wait_address(self, link, address_regex, scope='global', ipv='', timeout_sec=100): for i in range(timeout_sec):