From d09a179e6973fb1da4a0366c25d9d16679655f2d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Jun 2021 12:01:42 +0900 Subject: [PATCH 1/6] network: reconfigure link after coming back from sleep Previously, link was reconfigured with `link_carrier_reset()`, but it just re-request to configure static addresses, routes, etc, and restart engines (e.g. DHCP client). However, after coming back from sleep, several link information may be changed, especially the wifi access point may be different. So, we may need to reconfigure the interface. --- src/network/networkd-link.c | 107 +++++++++++++++++++++++---------- src/network/networkd-link.h | 2 +- src/network/networkd-manager.c | 4 +- 3 files changed, 78 insertions(+), 35 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 17fb932f81..2cf84b0f13 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1284,44 +1284,107 @@ static int link_reconfigure_impl(Link *link, bool force) { return 1; } -static int link_reconfigure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, bool force) { +static int link_reconfigure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, bool force, bool update_wifi) { + bool link_was_lower_up; int r; + assert(link); + + link_was_lower_up = link->flags & IFF_LOWER_UP; + r = link_getlink_handler_internal(rtnl, m, link, "Failed to update link state"); if (r <= 0) return r; + if (update_wifi && link_was_lower_up && link->flags & IFF_LOWER_UP) { + /* If the interface's L1 was not up, then wifi_get_info() is already called in + * link_update_flags(). So, it is not necessary to re-call here. */ + r = wifi_get_info(link); + if (r < 0) { + link_enter_failed(link); + return 0; + } + } + r = link_reconfigure_impl(link, force); - if (r < 0) + if (r < 0) { link_enter_failed(link); + return 0; + } + + return r; +} + +static int link_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { + return link_reconfigure_handler_internal(rtnl, m, link, /* force = */ false, /* update_wifi = */ false); +} + +static int link_force_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { + return link_reconfigure_handler_internal(rtnl, m, link, /* force = */ true, /* update_wifi = */ false); +} + +static int link_reconfigure_after_sleep_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { + int r; + + assert(link); + + r = link_reconfigure_handler_internal(rtnl, m, link, /* force = */ false, /* update_wifi = */ true); + if (r != 0) + return r; + + /* r == 0 means an error occurs, the link is unmanaged, or the matching network file is unchanged. */ + if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) + return 0; + + /* re-request static configs, and restart engines. */ + r = link_stop_engines(link, false); + if (r < 0) { + link_enter_failed(link); + return 0; + } + + r = link_acquire_dynamic_conf(link); + if (r < 0) { + link_enter_failed(link); + return 0; + } + + r = link_request_static_configs(link); + if (r < 0) { + link_enter_failed(link); + return 0; + } return 0; } -static int link_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return link_reconfigure_handler_internal(rtnl, m, link, false); -} - -static int link_force_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return link_reconfigure_handler_internal(rtnl, m, link, true); -} - -int link_reconfigure(Link *link, bool force) { +static int link_reconfigure_internal(Link *link, link_netlink_message_handler_t callback) { int r; + assert(link); + assert(callback); + /* When link in pending or initialized state, then link_configure() will be called. To prevent * the function from being called multiple times simultaneously, refuse to reconfigure the * interface in these cases. */ if (IN_SET(link->state, LINK_STATE_PENDING, LINK_STATE_INITIALIZED, LINK_STATE_LINGER)) return 0; /* 0 means no-op. */ - r = link_call_getlink(link, force ? link_force_reconfigure_handler : link_reconfigure_handler); + r = link_call_getlink(link, callback); if (r < 0) return r; return 1; /* 1 means the interface will be reconfigured. */ } +int link_reconfigure(Link *link, bool force) { + return link_reconfigure_internal(link, force ? link_force_reconfigure_handler : link_reconfigure_handler); +} + +int link_reconfigure_after_sleep(Link *link) { + return link_reconfigure_internal(link, link_reconfigure_after_sleep_handler); +} + static int link_initialized_and_synced(Link *link) { Network *network; int r; @@ -1570,26 +1633,6 @@ static int link_carrier_lost(Link *link) { return 0; } -int link_carrier_reset(Link *link) { - int r; - - assert(link); - - if (!link_has_carrier(link)) - return 0; - - r = link_carrier_lost(link); - if (r < 0) - return r; - - r = link_carrier_gained(link); - if (r < 0) - return r; - - log_link_info(link, "Reset carrier"); - return 0; -} - static int link_admin_state_up(Link *link) { int r; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 4077ccaf09..79400dee2e 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -232,7 +232,6 @@ void link_check_ready(Link *link); void link_update_operstate(Link *link, bool also_update_bond_master); -int link_carrier_reset(Link *link); bool link_has_carrier(Link *link); bool link_ipv6_enabled(Link *link); @@ -247,6 +246,7 @@ const char* link_state_to_string(LinkState s) _const_; LinkState link_state_from_string(const char *s) _pure_; int link_reconfigure(Link *link, bool force); +int link_reconfigure_after_sleep(Link *link); int manager_udev_process_link(sd_device_monitor *monitor, sd_device *device, void *userdata); int manager_rtnl_process_link(sd_netlink *rtnl, sd_netlink_message *message, Manager *m); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 374d27bef3..82aecf1c76 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -59,9 +59,9 @@ static int manager_reset_all(Manager *m) { assert(m); HASHMAP_FOREACH(link, m->links_by_index) { - r = link_carrier_reset(link); + r = link_reconfigure_after_sleep(link); if (r < 0) { - log_link_warning_errno(link, r, "Could not reset carrier: %m"); + log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); link_enter_failed(link); } } From f85499108f79011fa3ed6f18cc22a9dc50f6f9c2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Jun 2021 12:26:37 +0900 Subject: [PATCH 2/6] network: start/stop LLDP client on carrier gained/lost Then, link_acquire_dynamic_conf() matches link_stop_engines(). Previously, LLDP client is started on interface is up, and stopped on both down and carrier loss. --- src/network/networkd-link.c | 10 ++++++---- src/network/networkd-lldp-rx.c | 29 ----------------------------- src/network/networkd-lldp-rx.h | 1 - 3 files changed, 6 insertions(+), 34 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 2cf84b0f13..c232a8d6c5 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -690,6 +690,12 @@ static int link_acquire_dynamic_conf(Link *link) { if (r < 0) return log_link_warning_errno(link, r, "Failed to start LLDP transmission: %m"); + if (link->lldp) { + r = sd_lldp_start(link->lldp); + if (r < 0) + return log_link_warning_errno(link, r, "Failed to start LLDP client: %m"); + } + return 0; } @@ -1986,10 +1992,6 @@ static int link_update_flags(Link *link, sd_netlink_message *message) { return r; } - r = link_update_lldp(link); - if (r < 0) - return r; - if (!had_carrier && link_has_carrier(link)) { log_link_info(link, "Gained carrier"); diff --git a/src/network/networkd-lldp-rx.c b/src/network/networkd-lldp-rx.c index c30cc36b51..57c8446ade 100644 --- a/src/network/networkd-lldp-rx.c +++ b/src/network/networkd-lldp-rx.c @@ -103,38 +103,9 @@ int link_lldp_rx_configure(Link *link) { if (r < 0) return r; - r = link_update_lldp(link); - if (r < 0) - return r; - return 0; } -int link_update_lldp(Link *link) { - int r; - - assert(link); - - if (!link->lldp) - return 0; - - if (link->flags & IFF_UP) { - r = sd_lldp_start(link->lldp); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to start LLDP: %m"); - if (r > 0) - log_link_debug(link, "Started LLDP."); - } else { - r = sd_lldp_stop(link->lldp); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to stop LLDP: %m"); - if (r > 0) - log_link_debug(link, "Stopped LLDP."); - } - - return r; -} - int link_lldp_save(Link *link) { _cleanup_(unlink_and_freep) char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; diff --git a/src/network/networkd-lldp-rx.h b/src/network/networkd-lldp-rx.h index 49306eafd0..22f6602bd0 100644 --- a/src/network/networkd-lldp-rx.h +++ b/src/network/networkd-lldp-rx.h @@ -14,7 +14,6 @@ typedef enum LLDPMode { } LLDPMode; int link_lldp_rx_configure(Link *link); -int link_update_lldp(Link *link); int link_lldp_save(Link *link); const char* lldp_mode_to_string(LLDPMode m) _const_; From 6bfa06fc39655e0d43c87a54eccaa34227b61335 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 14 Jun 2021 07:04:18 +0900 Subject: [PATCH 3/6] network: do not drop foreign configs based on the previous .network file `link_drop_foreign_config()` will be called later in link_configure(). --- src/network/networkd-link.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index c232a8d6c5..bb5f60a828 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1259,13 +1259,6 @@ static int link_reconfigure_impl(Link *link, bool force) { if (r < 0) return r; - if (!IN_SET(link->state, LINK_STATE_UNMANAGED, LINK_STATE_PENDING, LINK_STATE_INITIALIZED)) { - log_link_debug(link, "State is %s, dropping foreign config", link_state_to_string(link->state)); - r = link_drop_foreign_config(link); - if (r < 0) - return r; - } - link_free_carrier_maps(link); link_free_engines(link); link->network = network_unref(link->network); From 5e3bb5da982b53bd71f1486fcd96f7ff06707b6d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 14 Jun 2021 07:13:58 +0900 Subject: [PATCH 4/6] network: do not drop foreign configs for loopback and critical interfaces on carrier lost --- src/network/networkd-link.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index bb5f60a828..f8dee666a6 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1005,6 +1005,16 @@ static int link_drop_foreign_config(Link *link) { assert(link); assert(link->manager); + /* Drop foreign config, but ignore unmanaged, loopback, or critical interfaces. We do not want + * to remove loopback address or addresses used for root NFS. */ + + if (IN_SET(link->state, LINK_STATE_UNMANAGED, LINK_STATE_PENDING, LINK_STATE_INITIALIZED)) + return 0; + if (FLAGS_SET(link->flags, IFF_LOOPBACK)) + return 0; + if (link->network->keep_configuration == KEEP_CONFIGURATION_YES) + return 0; + r = link_drop_foreign_routes(link); k = link_drop_foreign_nexthops(link); @@ -1157,14 +1167,9 @@ static int link_configure(Link *link) { if (r < 0) return r; - /* 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; - } + r = link_drop_foreign_config(link); + if (r < 0) + return r; r = link_request_static_configs(link); if (r < 0) @@ -1622,14 +1627,7 @@ static int link_carrier_lost(Link *link) { if (r < 0) return r; - if (!IN_SET(link->state, LINK_STATE_UNMANAGED, LINK_STATE_PENDING, LINK_STATE_INITIALIZED)) { - log_link_debug(link, "State is %s, dropping foreign config", link_state_to_string(link->state)); - r = link_drop_foreign_config(link); - if (r < 0) - return r; - } - - return 0; + return link_drop_foreign_config(link); } static int link_admin_state_up(Link *link) { From cb71e9c6066a430a6c7ff503d3d3787e3d3a9f0e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 14 Jun 2021 06:58:06 +0900 Subject: [PATCH 5/6] network: drop configurations in the previous .network file when entering unmanaged state When reconfiguring an interface and it will enter unmanaged state, let's drop configurations from the previously assigned .network file. --- src/network/networkd-link.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index f8dee666a6..8900534cd6 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1235,23 +1235,22 @@ static int link_get_network(Link *link, Network **ret) { } static int link_reconfigure_impl(Link *link, bool force) { - Network *network; + Network *network = NULL; int r; assert(link); r = link_get_network(link, &network); - if (r == -ENOENT) { - link_set_state(link, LINK_STATE_UNMANAGED); - return 0; - } - if (r < 0) + if (r < 0 && r != -ENOENT) return r; if (link->network == network && !force) return 0; - log_link_info(link, "Re-configuring with %s", network->filename); + if (network) + log_link_info(link, "Reconfiguring with %s.", network->filename); + else + log_link_info(link, "Unmanaging interface."); /* Dropping old .network file */ r = link_stop_engines(link, false); @@ -1269,6 +1268,11 @@ static int link_reconfigure_impl(Link *link, bool force) { link->network = network_unref(link->network); link_unref(set_remove(link->manager->links_requesting_uuid, link)); + if (!network) { + link_set_state(link, LINK_STATE_UNMANAGED); + return 0; + } + /* Then, apply new .network file */ link->network = network_ref(network); link_update_operstate(link, true); From 3bad5487e9a2fdf570c52c41208ea265d1b62df8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 15 Jun 2021 11:57:40 +0900 Subject: [PATCH 6/6] test-network: add a testcase to enter unmanaged state on reconfiguring --- test/test-network/systemd-networkd-tests.py | 33 +++++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index df9b6eadad..d3e3a212e7 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -674,9 +674,9 @@ class NetworkctlTests(unittest.TestCase, Utilities): output = check_output('ip -4 address show dev dummy98') print(output) - self.assertRegex(output, 'inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98') - self.assertRegex(output, 'inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98') - self.assertRegex(output, 'inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98') + self.assertIn('inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98', output) + self.assertIn('inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98', output) + self.assertIn('inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98', output) check_output('ip address del 10.1.2.3/16 dev dummy98') check_output('ip address del 10.1.2.4/16 dev dummy98') @@ -687,9 +687,30 @@ class NetworkctlTests(unittest.TestCase, Utilities): output = check_output('ip -4 address show dev dummy98') print(output) - self.assertRegex(output, 'inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98') - self.assertRegex(output, 'inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98') - self.assertRegex(output, 'inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98') + self.assertIn('inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98', output) + self.assertIn('inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98', output) + self.assertIn('inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98', output) + + remove_unit_from_networkd_path(['25-address-static.network']) + + check_output(*networkctl_cmd, 'reload', env=env) + self.wait_operstate('dummy98', 'degraded', setup_state='unmanaged') + + output = check_output('ip -4 address show dev dummy98') + print(output) + self.assertNotIn('inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98', output) + self.assertNotIn('inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98', output) + self.assertNotIn('inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98', output) + + copy_unit_to_networkd_unit_path('25-address-static.network') + check_output(*networkctl_cmd, 'reload', env=env) + self.wait_online(['dummy98:routable']) + + output = check_output('ip -4 address show dev dummy98') + print(output) + self.assertIn('inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98', output) + self.assertIn('inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98', output) + self.assertIn('inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98', output) def test_reload(self): start_networkd(3)