From 61e1027cc78318d83b840f712bdc95a45bb83e74 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Tue, 1 Nov 2022 15:29:05 -0400 Subject: [PATCH] device: preserve the DHCP lease during reapply When the connection setting changes at the first place, then calling the device reapply, the ip address got temporarily removed when DHCP restarted. To avoid the ip address got temporarily removed, we should preserve the previous lease and keep using it until the new lease comes along. --- src/core/devices/nm-device.c | 67 ++++++++++++++++++++-------------- src/core/dhcp/nm-dhcp-client.c | 47 ++++++++++++++++++++++++ src/core/dhcp/nm-dhcp-client.h | 18 ++------- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 4f7e863f26..c6232374d3 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -849,7 +849,8 @@ static void _dev_ipshared4_spawn_dnsmasq(NMDevice *self); static void _dev_ipshared6_start(NMDevice *self); -static void _cleanup_ip_pre(NMDevice *self, int addr_family, CleanupType cleanup_type); +static void +_cleanup_ip_pre(NMDevice *self, int addr_family, CleanupType cleanup_type, gboolean preserve_dhcp); static void concheck_update_state(NMDevice *self, int addr_family, @@ -4278,8 +4279,8 @@ _set_ifindex(NMDevice *self, int ifindex, gboolean is_ip_ifindex) } if (!priv->l3cfg) { - _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_KEEP); - _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_KEEP); + _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_KEEP, FALSE); + _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_KEEP, FALSE); } _LOGD(LOGD_DEVICE, @@ -10461,6 +10462,7 @@ _dev_ipdhcpx_start(NMDevice *self, int addr_family) .request_broadcast = request_broadcast, .acd_timeout_msec = _prop_get_ipv4_dad_timeout(self), }, + .previous_lease = priv->l3cds[L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4)].d, }; priv->ipdhcp_data_4.client = @@ -10511,16 +10513,16 @@ _dev_ipdhcpx_start(NMDevice *self, int addr_family) G_CALLBACK(_dev_ipdhcpx_notify), self); - /* FIXME(l3cfg:dhcp:previous-lease): take the NML3ConfigData from the previous lease (if any) - * and pass it on to NMDhcpClient. This is a fake lease that we use initially (until - * NMDhcpClient got a real lease). Note that NMDhcpClient needs to check whether the - * lease already expired. */ - + /* Take the NML3ConfigData from the previous lease (if any) that was passed to the NMDhcpClient. + * This may be the old lease only used during the duration of a reapply until we get the + * new lease. */ previous_lease = nm_dhcp_client_get_lease(priv->ipdhcp_data_x[IS_IPv4].client); + if (!priv->ipdhcp_data_x[IS_IPv4].config) { priv->ipdhcp_data_x[IS_IPv4].config = nm_dhcp_config_new(addr_family, previous_lease); _notify(self, PROP_DHCPX_CONFIG(IS_IPv4)); } + if (previous_lease) { nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, previous_lease); _dev_l3_register_l3cds_set_one_full(self, @@ -12108,13 +12110,34 @@ activate_stage3_ip_config(NMDevice *self) ifindex = nm_device_get_ip_ifindex(self); + ipv4_method = nm_device_get_effective_ip_config_method(self, AF_INET); + if (nm_streq(ipv4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO)) { + /* "auto" usually means DHCPv4 or autoconf6, but it doesn't have to be. Subclasses + * can overwrite it. For example, you cannot run DHCPv4 on PPP/WireGuard links. */ + ipv4_method = klass->get_ip_method_auto(self, AF_INET); + } + + ipv6_method = nm_device_get_effective_ip_config_method(self, AF_INET6); + + if (nm_streq(ipv6_method, NM_SETTING_IP6_CONFIG_METHOD_AUTO)) { + ipv6_method = klass->get_ip_method_auto(self, AF_INET6); + } + if (priv->ip_data_4.do_reapply) { _LOGD_ip(AF_INET, "reapply..."); - _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_KEEP_REAPPLY); + priv->ip_data_4.do_reapply = FALSE; + _cleanup_ip_pre(self, + AF_INET, + CLEANUP_TYPE_KEEP_REAPPLY, + nm_streq(ipv4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO)); } if (priv->ip_data_6.do_reapply) { _LOGD_ip(AF_INET6, "reapply..."); - _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_KEEP_REAPPLY); + priv->ip_data_6.do_reapply = FALSE; + _cleanup_ip_pre(self, + AF_INET6, + CLEANUP_TYPE_KEEP_REAPPLY, + nm_streq(ipv6_method, NM_SETTING_IP6_CONFIG_METHOD_AUTO)); } /* Add the interface to the specified firewall zone */ @@ -12166,18 +12189,6 @@ activate_stage3_ip_config(NMDevice *self) * let's do it! */ _commit_mtu(self); - ipv4_method = nm_device_get_effective_ip_config_method(self, AF_INET); - if (nm_streq(ipv4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO)) { - /* "auto" usually means DHCPv4 or autoconf6, but it doesn't have to be. Subclasses - * can overwrite it. For example, you cannot run DHCPv4 on PPP/WireGuard links. */ - ipv4_method = klass->get_ip_method_auto(self, AF_INET); - } - - ipv6_method = nm_device_get_effective_ip_config_method(self, AF_INET6); - if (nm_streq(ipv6_method, NM_SETTING_IP6_CONFIG_METHOD_AUTO)) { - ipv6_method = klass->get_ip_method_auto(self, AF_INET6); - } - if (!nm_device_sys_iface_state_is_external(self) && (!klass->ready_for_ip_config || klass->ready_for_ip_config(self, TRUE))) { if (priv->ipmanual_data.state_6 == NM_DEVICE_IP_STATE_NONE @@ -12630,7 +12641,7 @@ delete_on_deactivate_check_and_schedule(NMDevice *self) } static void -_cleanup_ip_pre(NMDevice *self, int addr_family, CleanupType cleanup_type) +_cleanup_ip_pre(NMDevice *self, int addr_family, CleanupType cleanup_type, gboolean preserve_dhcp) { const int IS_IPv4 = NM_IS_IPv4(addr_family); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); @@ -12641,7 +12652,7 @@ _cleanup_ip_pre(NMDevice *self, int addr_family, CleanupType cleanup_type) _dev_ipdev_cleanup(self, AF_UNSPEC); _dev_ipdev_cleanup(self, addr_family); - _dev_ipdhcpx_cleanup(self, addr_family, TRUE, FALSE); + _dev_ipdhcpx_cleanup(self, addr_family, !preserve_dhcp || !keep_reapply, FALSE); if (!IS_IPv4) _dev_ipac6_cleanup(self); @@ -15406,8 +15417,8 @@ _cleanup_generic_pre(NMDevice *self, CleanupType cleanup_type) for (i = 0; i < 2; i++) nm_clear_pointer(&priv->hostname_resolver_x[i], _hostname_resolver_free); - _cleanup_ip_pre(self, AF_INET, cleanup_type); - _cleanup_ip_pre(self, AF_INET6, cleanup_type); + _cleanup_ip_pre(self, AF_INET, cleanup_type, FALSE); + _cleanup_ip_pre(self, AF_INET6, cleanup_type, FALSE); _dev_ip_state_req_timeout_cancel(self, AF_UNSPEC); } @@ -15920,8 +15931,8 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, /* Clean up any half-done IP operations if the device's layer2 * finds out it needs authentication during IP config. */ - _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_DECONFIGURE); - _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_DECONFIGURE); + _cleanup_ip_pre(self, AF_INET, CLEANUP_TYPE_DECONFIGURE, FALSE); + _cleanup_ip_pre(self, AF_INET6, CLEANUP_TYPE_DECONFIGURE, FALSE); } break; default: diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 897f17a3f6..2ab02c7796 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -84,6 +84,7 @@ typedef struct _NMDhcpClientPrivate { * and is set from l3cd_next. */ const NML3ConfigData *l3cd_curr; + GSource *previous_lease_timeout_source; GSource *no_lease_timeout_source; GSource *watch_source; GBytes *effective_client_id; @@ -269,6 +270,12 @@ nm_dhcp_client_create_options_dict(NMDhcpClient *self, gboolean static_keys) return options; } +const NML3ConfigData * +nm_dhcp_client_get_lease(NMDhcpClient *self) +{ + return NM_DHCP_CLIENT_GET_PRIVATE(self)->l3cd_curr; +} + /*****************************************************************************/ gboolean @@ -863,6 +870,9 @@ _nm_dhcp_client_notify(NMDhcpClient *self, return; } + if (priv->l3cd_next) + nm_clear_g_source_inst(&priv->previous_lease_timeout_source); + nm_l3_config_data_reset(&priv->l3cd_curr, priv->l3cd_next); if (client_event_type == NM_DHCP_CLIENT_EVENT_TYPE_BOUND && priv->l3cd_curr @@ -1328,6 +1338,19 @@ wait_dhcp_commit_done: } } +static gboolean +_previous_lease_timeout_cb(gpointer user_data) +{ + NMDhcpClient *self = user_data; + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + + nm_clear_g_source_inst(&priv->previous_lease_timeout_source); + + _nm_dhcp_client_notify(self, NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT, NULL); + + return G_SOURCE_CONTINUE; +} + gboolean nm_dhcp_client_start(NMDhcpClient *self, GError **error) { @@ -1359,6 +1382,23 @@ nm_dhcp_client_start(NMDhcpClient *self, GError **error) _no_lease_timeout_schedule(self); + if (priv->config.previous_lease) { + /* We got passed a previous lease (during a reapply). For a few seconds, we + * will pretend that this is current lease. */ + priv->l3cd_curr = g_steal_pointer(&priv->config.previous_lease); + + /* Schedule a timeout for when we give up using this lease. Note + * that then we will emit a NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE event + * and the lease is gone. Note that NMDevice ignores that and will + * keep using the lease. + * + * At the same time, we have _no_lease_timeout_schedule() ticking, when + * that expires, we will emit a NM_DHCP_CLIENT_NOTIFY_TYPE_NO_LEASE_TIMEOUT + * signal, which causes NMDevice to clear the lease. */ + priv->previous_lease_timeout_source = + nm_g_timeout_add_seconds_source(15, _previous_lease_timeout_cb, self); + } + if (IS_IPv4) return NM_DHCP_CLIENT_GET_CLASS(self)->ip4_start(self, error); @@ -1439,6 +1479,8 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release) if (priv->is_stopped) return; + nm_clear_g_source_inst(&priv->previous_lease_timeout_source); + priv->is_stopped = TRUE; if (priv->invocation) { @@ -1784,6 +1826,8 @@ config_init(NMDhcpClientConfig *config, const NMDhcpClientConfig *src) g_object_ref(config->l3cfg); + nm_l3_config_data_ref_and_seal(config->previous_lease); + nm_g_bytes_ref(config->hwaddr); nm_g_bytes_ref(config->bcast_hwaddr); nm_g_bytes_ref(config->vendor_class_identifier); @@ -1844,6 +1888,8 @@ config_clear(NMDhcpClientConfig *config) { g_object_unref(config->l3cfg); + nm_clear_l3cd(&config->previous_lease); + nm_clear_pointer(&config->hwaddr, g_bytes_unref); nm_clear_pointer(&config->bcast_hwaddr, g_bytes_unref); nm_clear_pointer(&config->vendor_class_identifier, g_bytes_unref); @@ -1920,6 +1966,7 @@ dispose(GObject *object) watch_cleanup(self); + nm_clear_g_source_inst(&priv->previous_lease_timeout_source); nm_clear_g_source_inst(&priv->no_lease_timeout_source); if (!NM_IS_IPv4(priv->config.addr_family)) { diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h index 6d07a14f0d..f43770a07b 100644 --- a/src/core/dhcp/nm-dhcp-client.h +++ b/src/core/dhcp/nm-dhcp-client.h @@ -92,9 +92,8 @@ typedef struct { * NMDhcpClient is supposed to run. */ NML3Cfg *l3cfg; - /* FIXME(l3cfg:dhcp:previous-lease): most parameters of NMDhcpClient are immutable, - * so to change them (during reapply), we need to create and start - * a new NMDhcpClient instance. + /* Most parameters of NMDhcpClient are immutable, so to change them (during + * reapply), we need to create and start a new NMDhcpClient instance. * * However, while the restart happens, we want to stick to the previous * lease (if any). Allow the caller to provide such a previous lease, @@ -225,18 +224,7 @@ const NMDhcpClientConfig *nm_dhcp_client_get_config(NMDhcpClient *self); pid_t nm_dhcp_client_get_pid(NMDhcpClient *self); -static inline const NML3ConfigData * -nm_dhcp_client_get_lease(NMDhcpClient *self) -{ - /* FIXME(l3cfg:dhcp:previous-lease): this function returns the currently - * valid, exposed lease. - * - * Note that NMDhcpClient should accept as construct argument a *previous* lease, - * and (if that lease is still valid), pretend that it's good to use. The point is - * so that during reapply we keep using the current address, until a new lease - * was received. */ - return NULL; -} +const NML3ConfigData *nm_dhcp_client_get_lease(NMDhcpClient *self); void nm_dhcp_client_stop(NMDhcpClient *self, gboolean release);