From 687d0dd95ed933859f6a6f0a594ef6757be86130 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 21 Aug 2019 09:12:08 +0200 Subject: n-dhcp4: arm timers in bound state Arm timers when the bound state is reached, otherwise the lease is never renewed. https://github.com/nettools/n-dhcp4/pull/4 --- shared/n-dhcp4/src/n-dhcp4-c-probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-probe.c b/shared/n-dhcp4/src/n-dhcp4-c-probe.c index f1ac3db746..107c18bbad 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-probe.c @@ -1009,7 +1009,7 @@ int n_dhcp4_client_probe_transition_accept(NDhcp4ClientProbe *probe, NDhcp4Incom probe->state = N_DHCP4_CLIENT_PROBE_STATE_BOUND; - /* XXX: trigger timers */ + n_dhcp4_client_arm_timer (probe->client); break; -- cgit v1.2.1 From 8b5bf6e4d1f739d6ce9725d30e39a4e54a527b01 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 21 Aug 2019 08:54:03 +0200 Subject: device: accept lease only after addresses are configured In the accept() callback, the nettools client creates a UDP socket with the received address as source, so the address must be already configured on the interface. Also, handle errors returned by nm_dhcp_client_accept(). Fixes: 401fee7c2040 ('dhcp: support notifying the client of the result of DAD') --- src/devices/nm-device.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 5ea94c7959..81e8d934f9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7787,8 +7787,8 @@ dhcp4_dad_cb (NMDevice *self, NMIP4Config **configs, gboolean success) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); if (success) { - nm_dhcp_client_accept (priv->dhcp4.client, NULL); - nm_device_activate_schedule_ip_config_result (self, AF_INET, NM_IP_CONFIG_CAST (configs[1])); + nm_device_activate_schedule_ip_config_result (self, AF_INET, + NM_IP_CONFIG_CAST (configs[1])); } else { nm_dhcp_client_decline (priv->dhcp4.client, "Address conflict detected", NULL); nm_device_ip_method_failed (self, AF_INET, @@ -10774,6 +10774,18 @@ activate_stage5_ip_config_result_4 (NMDevice *self) } } + if (priv->dhcp4.client) { + gs_free_error GError *error = NULL; + + if (!nm_dhcp_client_accept (priv->dhcp4.client, &error)) { + _LOGW (LOGD_DHCP4, + "Activation: Stage 5 of 5 (IPv4 Commit) error accepting lease: %s", + error->message); + nm_device_ip_method_failed (self, AF_INET, NM_DEVICE_STATE_REASON_DHCP_ERROR); + return; + } + } + /* If IPv4 wasn't the first to complete, and DHCP was used, then ensure * dispatcher scripts get the DHCP lease information. */ -- cgit v1.2.1 From 9c123cdd3f6133bae8fb9776c715d70ea4c959f1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 14 Aug 2019 10:55:44 +0200 Subject: device: keep client running after activation failure If DHCPv4 fails but IPv6 succeeds it makes sense to continue trying DHCP so that we will eventually be able to get an address if the DHCP server comes back. Always keep the client running; it will be only terminated when the connection is brought down. https://bugzilla.redhat.com/show_bug.cgi?id=1688329 --- src/devices/nm-device.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 81e8d934f9..02963b52c2 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7736,7 +7736,13 @@ dhcp4_fail (NMDevice *self, NMDhcpState dhcp_state) _ip_state_to_string (priv->ip_state_4), priv->dhcp4.was_active); - /* Keep client running if there are static addresses configured + /* The client is always left running after a failure. */ + + /* Nothing to do if we failed before... */ + if (priv->ip_state_4 == NM_DEVICE_IP_STATE_FAIL) + goto clear_config; + + /* ... and also if there are static addresses configured * on the interface. */ if ( priv->ip_state_4 == NM_DEVICE_IP_STATE_DONE @@ -7752,14 +7758,12 @@ dhcp4_fail (NMDevice *self, NMDhcpState dhcp_state) */ if ( dhcp_state == NM_DHCP_STATE_TERMINATED || (!priv->dhcp4.was_active && priv->ip_state_4 == NM_DEVICE_IP_STATE_CONF)) { - dhcp4_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE); nm_device_activate_schedule_ip_config_timeout (self, AF_INET); return; } /* In any other case (expired lease, assumed connection, etc.), - * start a grace period in which we keep the client running, - * hoping that it will regain a lease. + * wait for some time before failing the IP method. */ if (!priv->dhcp4.grace_id) { priv->dhcp4.grace_id = g_timeout_add_seconds (DHCP_GRACE_PERIOD_SEC, @@ -8381,10 +8385,16 @@ dhcp6_fail (NMDevice *self, NMDhcpState dhcp_state) _ip_state_to_string (priv->ip_state_6), priv->dhcp6.was_active); + /* The client is always left running after a failure. */ + + /* Nothing to do if we failed before... */ + if (priv->ip_state_6 == NM_DEVICE_IP_STATE_FAIL) + goto clear_config; + is_dhcp_managed = (priv->dhcp6.mode == NM_NDISC_DHCP_LEVEL_MANAGED); if (is_dhcp_managed) { - /* Keep client running if there are static addresses configured + /* ... and also if there are static addresses configured * on the interface. */ if ( priv->ip_state_6 == NM_DEVICE_IP_STATE_DONE @@ -8400,14 +8410,12 @@ dhcp6_fail (NMDevice *self, NMDhcpState dhcp_state) */ if ( dhcp_state == NM_DHCP_STATE_TERMINATED || (!priv->dhcp6.was_active && priv->ip_state_6 == NM_DEVICE_IP_STATE_CONF)) { - dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE); nm_device_activate_schedule_ip_config_timeout (self, AF_INET6); return; } /* In any other case (expired lease, assumed connection, etc.), - * start a grace period in which we keep the client running, - * hoping that it will regain a lease. + * wait for some time before failing the IP method. */ if (!priv->dhcp6.grace_id) { priv->dhcp6.grace_id = g_timeout_add_seconds (DHCP_GRACE_PERIOD_SEC, -- cgit v1.2.1