diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index f353833e0d..d9a695b3a4 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); } @@ -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; } @@ -2495,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; @@ -2502,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)) @@ -2673,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); @@ -2785,19 +2837,40 @@ 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); assert(link->network); assert(link->state == LINK_STATE_INITIALIZED); - if (link->setting_mtu) + 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); 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); diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 43349ff207..f45e948b68 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -368,23 +368,62 @@ 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 check_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): + """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'] @@ -396,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): @@ -490,9 +515,7 @@ 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.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) @@ -500,16 +523,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 +829,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 +1907,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 +2103,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 +2112,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 +2120,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 +2129,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 +2137,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,39 +2370,29 @@ 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: - 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,14 +2503,12 @@ 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.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)