From ed3bab7a0e9097e443599297bf5a916315a8d574 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 8 Nov 2024 20:38:42 +0900 Subject: [PATCH 1/8] netwrok: call link_drop_unmanaged_config() earlier in link_configure() Otherwise, even if a link enters the configuring state at the beginning of link_configure(), link_check_ready() may be called before link_drop_unmanaged_config() is called, and the link may enter the configured state. Fixes #35092. --- src/network/networkd-link.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 080e6beff3..ab74260adb 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1148,6 +1148,10 @@ static int link_configure(Link *link) { link_set_state(link, LINK_STATE_CONFIGURING); + r = link_drop_unmanaged_config(link); + if (r < 0) + return r; + r = link_new_bound_to_list(link); if (r < 0) return r; @@ -1253,10 +1257,6 @@ static int link_configure(Link *link) { if (r < 0) return r; - r = link_drop_unmanaged_config(link); - if (r < 0) - return r; - r = link_request_static_configs(link); if (r < 0) return r; From 525a582ae860cd3afed3b7d64ed7b80b2a841909 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 12:00:38 +0900 Subject: [PATCH 2/8] network: drop unnecessary size specifier It does not save any memory usage but increase code complexity. --- src/network/networkd-link.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index ad8bac7ad6..848e67c843 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -179,7 +179,7 @@ typedef struct Link { Set *ndisc_dnr; uint32_t ndisc_mtu; unsigned ndisc_messages; - bool ndisc_configured:1; + bool ndisc_configured; sd_radv *radv; From 52f46b77d7bafb7ce44f016e1793862873a3fc73 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 09:52:03 +0900 Subject: [PATCH 3/8] network: reset 'configured' flags even if we keep DHCP lease and friends on reconfigure Follow-up for 451c2baf30f50b95d73e648058c7c2348dbf0c31. With the commits, reloading .network files does not release previously acquired DHCP lease and friends if possible. If previously a DHCP client was configured as not requesting DNS servers or so, then the previously acquired lease might not contain any DNS servers. In that case, if the new .network file enables UseDNS=, then the interface should enter the configured state after a new lease is acquired. To achieve that, we need to reset the flags. With this change, the workaround applied to the test by the commit 451c2baf30f50b95d73e648058c7c2348dbf0c31 can be dropped. --- src/network/networkd-link.c | 9 +++++++++ test/test-network/systemd-networkd-tests.py | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index ab74260adb..f9047103c0 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1136,6 +1136,15 @@ static int link_drop_dynamic_config(Link *link, Network *network) { link->lldp_rx = sd_lldp_rx_unref(link->lldp_rx); /* TODO: keep the received neighbors. */ link->lldp_tx = sd_lldp_tx_unref(link->lldp_tx); + /* Even if we do not release DHCP lease or so, reset 'configured' flags. Otherwise, e.g. if + * previously UseDNS= was disabled but is now enabled, link will enter configured state before + * expected DNS servers being acquired. */ + link->ipv4ll_address_configured = false; + link->dhcp4_configured = false; + link->dhcp6_configured = false; + link->dhcp_pd_configured = false; + link->ndisc_configured = false; + return r; } diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 8ee94a0294..59503d31a1 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -6705,7 +6705,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): '--dhcp-option=option6:ntp-server,[2600::ff]') networkctl_reload() - networkctl_reconfigure('veth99') # Release previously acquired lease and start new DHCPv6 handshake. self.wait_online('veth99:routable', 'veth-peer:routable') # checking address From 130d66956ff7577233574fc26108dbdc4ce42d8c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 09:52:03 +0900 Subject: [PATCH 4/8] test-network: reconfigure interface cleanly to drop previous DHCP lease and friends Follow-up for 451c2baf30f50b95d73e648058c7c2348dbf0c31. With the commits, reloading .network files does not release previously acquired DHCP lease and friends if possible. On graceful reconfigure triggered by the reload, the interface may acquire a new DHCPv4 lease earlier than DHCPv6 lease. In that case, the check will fail as it is done with the new DHCPv4 lease and old DHCPv6 lease, which does not contain any IPv6 DNS servers or so. So, when switching from no -> yes, we need to wait a new lease with DNS servers or so. To achieve that, we need to clean reconfigure the interface. --- test/test-network/systemd-networkd-tests.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 59503d31a1..111da949ab 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -7447,7 +7447,7 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): self.assertIn('inet 169.254.133.11/16 metric 2048 brd 169.254.255.255 scope link', output) def test_dhcp_client_use_dns(self): - def check(self, ipv4, ipv6): + def check(self, ipv4, ipv6, needs_reconfigure=False): os.makedirs(os.path.join(network_unit_dir, '25-dhcp-client.network.d'), exist_ok=True) with open(os.path.join(network_unit_dir, '25-dhcp-client.network.d/override.conf'), mode='w', encoding='utf-8') as f: f.write('[DHCPv4]\nUseDNS=') @@ -7457,6 +7457,8 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): f.write('\n[IPv6AcceptRA]\nUseDNS=no') networkctl_reload() + if needs_reconfigure: + networkctl_reconfigure('veth99') self.wait_online('veth99:routable') # link becomes 'routable' when at least one protocol provide an valid address. Hence, we need to explicitly wait for both addresses. @@ -7488,7 +7490,7 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): check(self, True, True) check(self, True, False) - check(self, False, True) + check(self, False, True, needs_reconfigure=True) check(self, False, False) def test_dhcp_client_default_use_domains(self): @@ -7540,7 +7542,7 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): check(self, False, False, False) def test_dhcp_client_use_dnr(self): - def check(self, ipv4, ipv6): + def check(self, ipv4, ipv6, needs_reconfigure=False): os.makedirs(os.path.join(network_unit_dir, '25-dhcp-client.network.d'), exist_ok=True) with open(os.path.join(network_unit_dir, '25-dhcp-client.network.d/override.conf'), mode='w', encoding='utf-8') as f: f.write('[DHCPv4]\nUseDNS=') @@ -7550,6 +7552,8 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): f.write('\n[IPv6AcceptRA]\nUseDNS=no') networkctl_reload() + if needs_reconfigure: + networkctl_reconfigure('veth99') self.wait_online('veth99:routable') # link becomes 'routable' when at least one protocol provide an valid address. Hence, we need to explicitly wait for both addresses. @@ -7586,11 +7590,11 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): check(self, True, True) check(self, True, False) - check(self, False, True) + check(self, False, True, needs_reconfigure=True) check(self, False, False) def test_dhcp_client_use_captive_portal(self): - def check(self, ipv4, ipv6): + def check(self, ipv4, ipv6, needs_reconfigure=False): os.makedirs(os.path.join(network_unit_dir, '25-dhcp-client.network.d'), exist_ok=True) with open(os.path.join(network_unit_dir, '25-dhcp-client.network.d/override.conf'), mode='w', encoding='utf-8') as f: f.write('[DHCPv4]\nUseCaptivePortal=') @@ -7600,6 +7604,8 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): f.write('\n[IPv6AcceptRA]\nUseCaptivePortal=no') networkctl_reload() + if needs_reconfigure: + networkctl_reconfigure('veth99') self.wait_online('veth99:routable') # link becomes 'routable' when at least one protocol provide an valid address. Hence, we need to explicitly wait for both addresses. @@ -7624,7 +7630,7 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): check(self, True, True) check(self, True, False) - check(self, False, True) + check(self, False, True, needs_reconfigure=True) check(self, False, False) def test_dhcp_client_reject_captive_portal(self): From 42152390dab7c01c181fd1ee008973aa16ec47ce Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 10:13:37 +0900 Subject: [PATCH 5/8] network: reorder dropping dynamic configuration Follow-up for 451c2baf30f50b95d73e648058c7c2348dbf0c31. --- src/network/networkd-link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index f9047103c0..990203b528 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1127,11 +1127,11 @@ static int link_drop_dynamic_config(Link *link, Network *network) { * previously DHCP=yes and now DHCP=no, but keep DHCP lease when DHCP setting is unchanged. */ r = link_drop_ndisc_config(link, network); - RET_GATHER(r, link_drop_radv_config(link, network)); + RET_GATHER(r, link_drop_radv_config(link, network)); /* Stop before dropping DHCP-PD prefixes. */ + RET_GATHER(r, link_drop_ipv4ll_config(link, network)); /* Stop before DHCPv4 client. */ RET_GATHER(r, link_drop_dhcp4_config(link, network)); RET_GATHER(r, link_drop_dhcp6_config(link, network)); RET_GATHER(r, link_drop_dhcp_pd_config(link, network)); - RET_GATHER(r, link_drop_ipv4ll_config(link, network)); link->dhcp_server = sd_dhcp_server_unref(link->dhcp_server); link->lldp_rx = sd_lldp_rx_unref(link->lldp_rx); /* TODO: keep the received neighbors. */ link->lldp_tx = sd_lldp_tx_unref(link->lldp_tx); From 4e76c57c7f86c46ef2ec3d3f7c25e049cf357214 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 09:16:06 +0900 Subject: [PATCH 6/8] network/dhcp-pd: do not remove unreachable route when reconfiguring non-upstream interface Unreachable routes are not owned by any interfaces, and its ifindex is zero. Previously, if a non-upstream interface is reconfigured, all routes including unreachable routes configured by the upstream interface are removed. This makes unreachable routes are always handled by the upstream interface, and only removed when the delegated prefixes are changed or lost. --- src/network/networkd-dhcp-prefix-delegation.c | 76 ++++++++++++++----- src/network/networkd-dhcp-prefix-delegation.h | 2 +- src/network/networkd-dhcp4.c | 2 +- src/network/networkd-dhcp6.c | 5 +- 4 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 0819ed54f1..7a5d0e0415 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -613,8 +613,49 @@ static int dhcp_pd_finalize(Link *link) { return 0; } -void dhcp_pd_prefix_lost(Link *uplink) { +static void dhcp_pd_mark_unreachable_route(Manager *manager, NetworkConfigSource source) { + assert(manager); + Route *route; + SET_FOREACH(route, manager->routes) { + if (route->source != source) + continue; + if (route->family != AF_INET6) + continue; + if (route->nexthop.ifindex != 0) /* IPv6 unreachable has 0 ifindex. */ + continue; + if (!route_type_is_reject(route->type)) + continue; + + route_mark(route); + } +} + +static int dhcp_pd_remove_unreachable_route(Manager *manager, NetworkConfigSource source, bool only_marked) { + int ret = 0; + + assert(manager); + + Route *route; + SET_FOREACH(route, manager->routes) { + if (route->source != source) + continue; + if (route->family != AF_INET6) + continue; + if (route->nexthop.ifindex != 0) /* IPv6 unreachable has 0 ifindex. */ + continue; + if (!route_type_is_reject(route->type)) + continue; + if (only_marked && !route_is_marked(route)) + continue; + + RET_GATHER(ret, route_remove_and_cancel(route, manager)); + } + + return ret; +} + +static void dhcp_pd_prefix_lost(Link *uplink, NetworkConfigSource source) { Link *link; int r; @@ -630,22 +671,7 @@ void dhcp_pd_prefix_lost(Link *uplink) { link_enter_failed(link); } - SET_FOREACH(route, uplink->manager->routes) { - if (!IN_SET(route->source, NETWORK_CONFIG_SOURCE_DHCP4, NETWORK_CONFIG_SOURCE_DHCP6)) - continue; - if (route->family != AF_INET6) - continue; - if (route->type != RTN_UNREACHABLE) - continue; - if (!set_contains(uplink->dhcp_pd_prefixes, - &(struct in_addr_prefix) { - .family = AF_INET6, - .prefixlen = route->dst_prefixlen, - .address = route->dst })) - continue; - - (void) route_remove_and_cancel(route, uplink->manager); - } + (void) dhcp_pd_remove_unreachable_route(uplink->manager, source, /* only_marked = */ false); set_clear(uplink->dhcp_pd_prefixes); } @@ -653,13 +679,20 @@ void dhcp_pd_prefix_lost(Link *uplink) { void dhcp4_pd_prefix_lost(Link *uplink) { Link *tunnel; - dhcp_pd_prefix_lost(uplink); + assert(uplink); + assert(uplink->manager); + + dhcp_pd_prefix_lost(uplink, NETWORK_CONFIG_SOURCE_DHCP4); if (uplink->dhcp4_6rd_tunnel_name && link_get_by_name(uplink->manager, uplink->dhcp4_6rd_tunnel_name, &tunnel) >= 0) (void) link_remove(tunnel); } +void dhcp6_pd_prefix_lost(Link *uplink) { + dhcp_pd_prefix_lost(uplink, NETWORK_CONFIG_SOURCE_DHCP6); +} + static int dhcp4_unreachable_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Route *route) { int r; @@ -1005,9 +1038,11 @@ int dhcp4_pd_prefix_acquired(Link *uplink) { return r; /* Request unreachable route */ + dhcp_pd_mark_unreachable_route(uplink->manager, NETWORK_CONFIG_SOURCE_DHCP4); r = dhcp4_request_unreachable_route(uplink, &pd_prefix, pd_prefixlen, lifetime_usec, &server_address); if (r < 0) return r; + (void) dhcp_pd_remove_unreachable_route(uplink->manager, NETWORK_CONFIG_SOURCE_DHCP4, /* only_marked = */ true); /* Create or update 6rd SIT tunnel device. */ r = dhcp4_pd_create_6rd_tunnel(uplink, dhcp4_pd_6rd_tunnel_create_handler); @@ -1085,11 +1120,14 @@ int dhcp6_pd_prefix_acquired(Link *uplink) { assert(uplink); assert(uplink->dhcp6_lease); + assert(uplink->manager); r = sd_dhcp6_lease_get_server_address(uplink->dhcp6_lease, &server_address.in6); if (r < 0) return log_link_warning_errno(uplink, r, "Failed to get server address of DHCPv6 lease: %m"); + dhcp_pd_mark_unreachable_route(uplink->manager, NETWORK_CONFIG_SOURCE_DHCP6); + /* First, logs acquired prefixes and request unreachable routes. */ FOREACH_DHCP6_PD_PREFIX(uplink->dhcp6_lease) { usec_t lifetime_valid_usec; @@ -1120,6 +1158,8 @@ int dhcp6_pd_prefix_acquired(Link *uplink) { return r; } + (void) dhcp_pd_remove_unreachable_route(uplink->manager, NETWORK_CONFIG_SOURCE_DHCP6, /* only_marked = */ true); + /* Then, assign subnet prefixes. */ HASHMAP_FOREACH(link, uplink->manager->links_by_index) { if (!dhcp_pd_is_uplink(link, uplink, /* accept_auto = */ true)) diff --git a/src/network/networkd-dhcp-prefix-delegation.h b/src/network/networkd-dhcp-prefix-delegation.h index 099e1444a5..27e920e1cf 100644 --- a/src/network/networkd-dhcp-prefix-delegation.h +++ b/src/network/networkd-dhcp-prefix-delegation.h @@ -20,8 +20,8 @@ int dhcp_request_prefix_delegation(Link *link); int link_drop_dhcp_pd_config(Link *link, Network *network); int dhcp4_pd_prefix_acquired(Link *uplink); int dhcp6_pd_prefix_acquired(Link *uplink); -void dhcp_pd_prefix_lost(Link *uplink); void dhcp4_pd_prefix_lost(Link *uplink); +void dhcp6_pd_prefix_lost(Link *uplink); int dhcp_pd_reconfigure_address(Address *address, Link *link); CONFIG_PARSER_PROTOTYPE(config_parse_dhcp_pd_subnet_id); diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index ca4e2a5926..dae1a7b7f4 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -247,7 +247,7 @@ static int dhcp4_remove_address_and_routes(Link *link, bool only_marked) { SET_FOREACH(route, link->manager->routes) { if (route->source != NETWORK_CONFIG_SOURCE_DHCP4) continue; - if (route->nexthop.ifindex != 0 && route->nexthop.ifindex != link->ifindex) + if (route->nexthop.ifindex != link->ifindex) continue; if (only_marked && !route_is_marked(route)) continue; diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 88e7c0a263..a6808fdb75 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -307,7 +307,6 @@ static int dhcp6_lease_ip_acquired(sd_dhcp6_client *client, Link *link) { int r; link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP6); - manager_mark_routes(link->manager, NULL, NETWORK_CONFIG_SOURCE_DHCP6); r = sd_dhcp6_client_get_lease(client, &lease); if (r < 0) @@ -330,7 +329,7 @@ static int dhcp6_lease_ip_acquired(sd_dhcp6_client *client, Link *link) { return r; } else if (sd_dhcp6_lease_has_pd_prefix(lease_old)) /* When we had PD prefixes but not now, we need to remove them. */ - dhcp_pd_prefix_lost(link); + dhcp6_pd_prefix_lost(link); if (link->dhcp6_messages == 0) { link->dhcp6_configured = true; @@ -377,7 +376,7 @@ static int dhcp6_lease_lost(Link *link) { log_link_info(link, "DHCPv6 lease lost"); if (sd_dhcp6_lease_has_pd_prefix(link->dhcp6_lease)) - dhcp_pd_prefix_lost(link); + dhcp6_pd_prefix_lost(link); link->dhcp6_lease = sd_dhcp6_lease_unref(link->dhcp6_lease); From e8da735cebb14936c529a46a04635472c9bcbe84 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 03:02:54 +0900 Subject: [PATCH 7/8] network: drop static configs later Follow-up for dd6d53a8dc58c5e6e310b09ba7f7a22600a87ba9. Unnecessary static configs will be anyway dropped later in link_configure() -> link_drop_unmanaged_config(). Hence, even if we are reconfiguring an interface cleanly, it is not necessary to drop static configs here. --- 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 990203b528..72765eebd8 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1397,13 +1397,6 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) { /* Dropping old .network file */ if (FLAGS_SET(flags, LINK_RECONFIGURE_CLEANLY)) { - /* Remove all static configurations. Note, dynamic configurations are dropped by - * link_stop_engines(), and foreign configurations will be removed later by - * link_configure() -> link_drop_unmanaged_config(). */ - r = link_drop_static_config(link); - if (r < 0) - return r; - /* Stop DHCP client and friends, and drop dynamic configurations like DHCP address. */ r = link_stop_engines(link, /* may_keep_dhcp = */ false); if (r < 0) From 82df2e0f0496834064f32e4f09cc32728c1cbddd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 7 Nov 2024 03:26:39 +0900 Subject: [PATCH 8/8] network: make 'networkctl reconfigure' work safely even when KeepConfiguration=dhcp or yes Previously, even if KeepConfiguration=dhcp or yes is specified in the new .network file, dynamic configurations like DHCP address and routes were dropped when 'networkctl reconfigure INTERFACE' is invoked. If the setting is specified, let's gracefully handle the dynamic configurations. Then, 'networkctl reconfigure' can be also used for an interface that has critical connections. --- src/network/networkd-dhcp-prefix-delegation.c | 17 +++++--- src/network/networkd-dhcp4.c | 17 +++++--- src/network/networkd-dhcp6.c | 14 ++++--- src/network/networkd-ipv4ll.c | 28 ++++++++----- src/network/networkd-link.c | 41 +++++++++++-------- src/network/networkd-ndisc.c | 33 ++++++++++----- src/network/networkd-radv.c | 6 +-- 7 files changed, 99 insertions(+), 57 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 7a5d0e0415..16426de981 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -1294,14 +1294,21 @@ int dhcp_request_prefix_delegation(Link *link) { int link_drop_dhcp_pd_config(Link *link, Network *network) { assert(link); - assert(network); + assert(link->network); - if (!link_dhcp_pd_is_enabled(link)) + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */ + + if (!link_dhcp_pd_is_enabled(link)) /* Disabled now, drop all configs. */ + return dhcp_pd_remove(link, /* only_marked = */ false); + + /* If previously explicitly disabled, then there is nothing we need to drop. + * If this is called on start up, we do not know the previous settings, assume nothing changed. */ + if (!network || !network->dhcp_pd) return 0; - /* If will be disabled or at least one config is changed, then drop all configurations. */ - if (!network->dhcp_pd || - link->network->dhcp_pd_assign != network->dhcp_pd_assign || + /* If at least one setting is changed, then drop all configurations. */ + if (link->network->dhcp_pd_assign != network->dhcp_pd_assign || (link->network->dhcp_pd_assign && (link->network->dhcp_pd_manage_temporary_address != network->dhcp_pd_manage_temporary_address || !set_equal(link->network->dhcp_pd_tokens, network->dhcp_pd_tokens))) || diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index dae1a7b7f4..684580f5b6 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1837,15 +1837,22 @@ int link_drop_dhcp4_config(Link *link, Network *network) { int ret = 0; assert(link); - assert(network); + assert(link->network); - if (!link_dhcp4_enabled(link)) - return 0; /* Currently DHCPv4 client is not enabled, there is nothing we need to drop. */ + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */ - if (!FLAGS_SET(network->dhcp, ADDRESS_FAMILY_IPV4)) - /* Currently enabled but will be disabled. Stop the client and drop the lease. */ + if (!link_dhcp4_enabled(link)) { + /* DHCP client is disabled. Stop the client if it is running and drop the lease. */ ret = sd_dhcp_client_stop(link->dhcp_client); + /* Also explicitly drop DHCPv4 address and routes. Why? This is for the case when the DHCPv4 + * client was enabled on the previous invocation of networkd, but when it is restarted, a new + * .network file may match to the interface, and DHCPv4 client may be disabled. In that case, + * the DHCPv4 client is not running, hence sd_dhcp_client_stop() in the above does nothing. */ + RET_GATHER(ret, dhcp4_remove_address_and_routes(link, /* only_marked = */ false)); + } + /* Even if the client is currently enabled and also enabled in the new .network file, detailed * settings for the client may be different. Let's unref() the client. But do not unref() the lease. * it will be unref()ed later when a new lease is acquired. */ diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index a6808fdb75..31a3fbc8e8 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -844,15 +844,19 @@ int link_drop_dhcp6_config(Link *link, Network *network) { int ret = 0; assert(link); - assert(network); + assert(link->network); - if (!link_dhcp6_enabled(link)) - return 0; /* Currently DHCPv6 client is not enabled, there is nothing we need to drop. */ + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */ - if (!FLAGS_SET(network->dhcp, ADDRESS_FAMILY_IPV6)) - /* Currently enabled but will be disabled. Stop the client and drop the lease. */ + if (!link_dhcp6_enabled(link)) { + /* DHCPv6 client is disabled. Stop the client if it is running and drop the lease. */ ret = sd_dhcp6_client_stop(link->dhcp6_client); + /* Also explicitly drop DHCPv6 addresses and routes. See also link_drop_dhcp4_config(). */ + RET_GATHER(ret, dhcp6_remove(link, /* only_marked = */ false)); + } + /* Even if the client is currently enabled and also enabled in the new .network file, detailed * settings for the client may be different. Let's unref() the client. But do not unref() the lease. * it will be unref()ed later when a new lease is acquired. */ diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 094cee6a14..7398cefe77 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -206,7 +206,7 @@ int ipv4ll_configure(Link *link) { return 0; if (link->ipv4ll) - return -EBUSY; + return 0; r = sd_ipv4ll_new(&link->ipv4ll); if (r < 0) @@ -246,19 +246,27 @@ int link_drop_ipv4ll_config(Link *link, Network *network) { int ret = 0; assert(link); - assert(network); + assert(link->network); - if (!link_ipv4ll_enabled(link)) - return 0; + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */ - Network *saved = link->network; - link->network = network; - bool enabled = link_ipv4ll_enabled(link); - link->network = saved; - - if (!enabled) + if (!link_ipv4ll_enabled(link)) { + /* The client is disabled. Stop if it is running, and drop the address. */ ret = sd_ipv4ll_stop(link->ipv4ll); + /* Also, explicitly drop the address for the case that this is called on start up. + * See also comments in link_drop_dhcp4_config(). */ + Address *a; + SET_FOREACH(a, link->addresses) { + if (a->source != NETWORK_CONFIG_SOURCE_IPV4LL) + continue; + + assert(a->family == AF_INET); + RET_GATHER(ret, address_remove_and_cancel(a, link)); + } + } + link->ipv4ll = sd_ipv4ll_unref(link->ipv4ll); return ret; } diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 72765eebd8..ef314975de 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1121,7 +1121,7 @@ static int link_drop_dynamic_config(Link *link, Network *network) { int r; assert(link); - assert(network); + assert(link->network); /* Drop unnecessary dynamic configurations gracefully, e.g. drop DHCP lease in the case that * previously DHCP=yes and now DHCP=no, but keep DHCP lease when DHCP setting is unchanged. */ @@ -1394,22 +1394,7 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) { joined, isempty(joined) ? "" : ")"); - /* Dropping old .network file */ - - if (FLAGS_SET(flags, LINK_RECONFIGURE_CLEANLY)) { - /* Stop DHCP client and friends, and drop dynamic configurations like DHCP address. */ - r = link_stop_engines(link, /* may_keep_dhcp = */ false); - if (r < 0) - return r; - - /* Free DHCP client and friends. */ - link_free_engines(link); - } else { - r = link_drop_dynamic_config(link, network); - if (r < 0) - return r; - } - + /* Dropping configurations based on the old .network file. */ r = link_drop_requests(link); if (r < 0) return r; @@ -1420,10 +1405,30 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) { * map here, as it depends on .network files assigned to other links. */ link_free_bound_to_list(link); - link->network = network_unref(link->network); + _cleanup_(network_unrefp) Network *old_network = TAKE_PTR(link->network); /* Then, apply new .network file */ link->network = network_ref(network); + + if (FLAGS_SET(network->keep_configuration, KEEP_CONFIGURATION_DHCP) || + !FLAGS_SET(flags, LINK_RECONFIGURE_CLEANLY)) { + /* To make 'networkctl reconfigure INTERFACE' work safely for an interface whose new .network + * file has KeepConfiguration=dhcp or yes, even if a clean reconfiguration is requested, + * drop only unnecessary or possibly being changed dynamic configurations here. */ + r = link_drop_dynamic_config(link, old_network); + if (r < 0) + return r; + } else { + /* Otherwise, stop DHCP client and friends unconditionally, and drop all dynamic + * configurations like DHCP address and routes. */ + r = link_stop_engines(link, /* may_keep_dhcp = */ false); + if (r < 0) + return r; + + /* Free DHCP client and friends. */ + link_free_engines(link); + } + link_update_operstate(link, true); link_dirty(link); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 5ab9c881f2..c1d2373f65 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -2672,28 +2672,39 @@ int link_drop_ndisc_config(Link *link, Network *network) { int r, ret = 0; assert(link); - assert(network); + assert(link->network); - if (!link_ndisc_enabled(link)) - return 0; /* Currently DHCPv4 client is not enabled, there is nothing we need to drop. */ + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */ - Network *current = link->network; - link->network = network; - bool enabled = link_ndisc_enabled(link); - link->network = current; - - if (!enabled) { - /* Currently enabled but will be disabled. Stop the client and flush configs. */ + if (!link_ndisc_enabled(link)) { + /* NDisc is disabled. Stop the client if it is running and flush configs. */ ret = ndisc_stop(link); ndisc_flush(link); link->ndisc = sd_ndisc_unref(link->ndisc); return ret; } - /* Even if the client is currently enabled and also enabled in the new .network file, detailed + /* Even if the client was previously enabled and also enabled in the new .network file, detailed * settings for the client may be different. Let's unref() the client. */ link->ndisc = sd_ndisc_unref(link->ndisc); + /* Get if NDisc was enabled or not. */ + Network *current = link->network; + link->network = network; + bool enabled = link_ndisc_enabled(link); + link->network = current; + + /* If previously explicitly disabled, there should be nothing to drop. + * If we do not know the previous setting of the client, e.g. when networkd is restarted, in that + * case we do not have the previous .network file assigned to the interface, then let's assume no + * detailed configuration is changed. Hopefully, unmatching configurations will be dropped after + * their lifetime. */ + if (!enabled) + return 0; + + assert(network); + /* Redirect messages will be ignored. Drop configurations based on the previously received redirect * messages. */ if (!network->ndisc_use_redirect) diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index a284e2c47e..deb37c4850 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -649,10 +649,10 @@ int link_drop_radv_config(Link *link, Network *network) { int ret = 0; assert(link); - assert(network); + assert(link->network); - if (!link_radv_enabled(link)) - return 0; + if (link->network == network) + return 0; /* .network file is unchanged. It is not necessary to reconfigure the server. */ // FIXME: check detailed settings and do not stop if nothing changed. // FIXME: save dynamic prefixes acquired by DHCP-PD.