dhcp: merge branch 'bg/dhcp-timeout-rh1791378'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/404/diffs
https://bugzilla.redhat.com/show_bug.cgi?id=1791378

8 files changed, 116 insertions, 73 deletions

diff --git a/shared/n-dhcp4/src/n-dhcp4-c-probe.c b/shared/n-dhcp4/src/n-dhcp4-c-probe.c
index f4962d6995..bfead036bd 100644
--- a/shared/n-dhcp4/src/n-dhcp4-c-probe.c
+++ b/shared/n-dhcp4/src/n-dhcp4-c-probe.c
@@ -841,11 +841,10 @@ static int n_dhcp4_client_probe_transition_lifetime(NDhcp4ClientProbe *probe) {
                         return r;
 
                 c_assert(probe->client->current_probe == probe);
-                probe->client->current_probe = NULL;
-
-                n_dhcp4_c_connection_close(&probe->connection);
+                probe->current_lease = n_dhcp4_client_lease_unref(probe->current_lease);
 
-                probe->state = N_DHCP4_CLIENT_PROBE_STATE_EXPIRED;
+                probe->state = N_DHCP4_CLIENT_PROBE_STATE_INIT;
+                probe->ns_deferred =  n_dhcp4_gettime(CLOCK_BOOTTIME) + UINT64_C(1);
 
                 break;
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index dc23b3b9aa..0f410807a6 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -76,7 +76,7 @@ _LOG_DECLARE_SELF (NMDevice);
 /*****************************************************************************/
 
 #define DEFAULT_AUTOCONNECT    TRUE
-#define DHCP_GRACE_PERIOD_SEC  480
+#define DHCP_GRACE_PERIOD_MULTIPLIER 2U
 
 #define CARRIER_WAIT_TIME_MS 6000
 #define CARRIER_WAIT_TIME_AFTER_MTU_MS 10000
@@ -7500,6 +7500,41 @@ ensure_con_ip_config (NMDevice *self, int addr_family)
 /*****************************************************************************/
 /* DHCPv4 stuff */
 
+static guint32
+get_dhcp_timeout (NMDevice *self, int addr_family)
+{
+	NMDeviceClass *klass;
+	NMConnection *connection;
+	NMSettingIPConfig *s_ip;
+	guint32 timeout;
+
+	nm_assert (NM_IS_DEVICE (self));
+	nm_assert_addr_family (addr_family);
+
+	connection = nm_device_get_applied_connection (self);
+
+	s_ip = nm_connection_get_setting_ip_config (connection, addr_family);
+
+	timeout = nm_setting_ip_config_get_dhcp_timeout (s_ip);
+	if (timeout)
+		return timeout;
+
+	timeout = nm_config_data_get_connection_default_int64 (NM_CONFIG_GET_DATA,
+	                                                       addr_family == AF_INET
+	                                                         ? NM_CON_DEFAULT ("ipv4.dhcp-timeout")
+	                                                         : NM_CON_DEFAULT ("ipv6.dhcp-timeout"),
+	                                                       self,
+	                                                       0, G_MAXINT32, 0);
+	if (timeout)
+		return timeout;
+
+	klass = NM_DEVICE_GET_CLASS (self);
+	if (klass->get_dhcp_timeout)
+		timeout = klass->get_dhcp_timeout (self, addr_family);
+
+	return timeout ?: NM_DHCP_TIMEOUT_DEFAULT;
+}
+
 static void
 dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release)
 {
@@ -7749,9 +7784,10 @@ ip_config_merge_and_apply (NMDevice *self,
 }
 
 static gboolean
-dhcp4_lease_change (NMDevice *self, NMIP4Config *config)
+dhcp4_lease_change (NMDevice *self, NMIP4Config *config, gboolean bound)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	gs_free_error GError *error = NULL;
 
 	g_return_val_if_fail (config, FALSE);
 
@@ -7762,6 +7798,15 @@ dhcp4_lease_change (NMDevice *self, NMIP4Config *config)
 		return FALSE;
 	}
 
+	/* TODO: we should perform DAD again whenever we obtain a
+	 * new lease after an expiry. But what should we do if
+	 * a duplicate address is detected? Fail the connection;
+	 * restart DHCP; continue without an address? */
+	if (bound && !nm_dhcp_client_accept (priv->dhcp4.client, &error)) {
+		_LOGW (LOGD_DHCP4, "error accepting lease: %s", error->message);
+		return FALSE;
+	}
+
 	nm_dispatcher_call_device (NM_DISPATCHER_ACTION_DHCP4_CHANGE,
 	                           self,
 	                           NULL,
@@ -7825,12 +7870,24 @@ dhcp4_fail (NMDevice *self, NMDhcpState dhcp_state)
 	 * 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,
+		guint32 timeout;
+
+		/* Start a grace period equal to the DHCP timeout multiplied
+		 * by a constant factor. */
+		timeout = get_dhcp_timeout (self, AF_INET);
+		if (timeout < G_MAXUINT32 / DHCP_GRACE_PERIOD_MULTIPLIER) {
+			timeout *= DHCP_GRACE_PERIOD_MULTIPLIER;
+			_LOGI (LOGD_DHCP4,
+			       "DHCPv4: trying to acquire a new lease within %u seconds",
+			       timeout);
+		} else {
+			timeout = G_MAXUINT32;
+			_LOGI (LOGD_DHCP4, "DHCPv4: trying to acquire a new lease");
+		}
+
+		priv->dhcp4.grace_id = g_timeout_add_seconds (timeout,
 		                                              dhcp4_grace_period_expired,
 		                                              self);
-		_LOGI (LOGD_DHCP4,
-		       "DHCPv4: %u seconds grace period started",
-		       DHCP_GRACE_PERIOD_SEC);
 		goto clear_config;
 	}
 	return;
@@ -7879,6 +7936,7 @@ dhcp4_state_changed (NMDhcpClient *client,
 
 	switch (state) {
 	case NM_DHCP_STATE_BOUND:
+	case NM_DHCP_STATE_EXTENDED:
 		if (!ip4_config) {
 			_LOGW (LOGD_DHCP4, "failed to get IPv4 config in response to DHCP event.");
 			dhcp4_fail (self, state);
@@ -7921,7 +7979,8 @@ dhcp4_state_changed (NMDhcpClient *client,
 
 			ipv4_dad_start (self, configs, dhcp4_dad_cb);
 		} else if (priv->ip_state_4 == NM_DEVICE_IP_STATE_DONE) {
-			if (dhcp4_lease_change (self, ip4_config))
+			if (dhcp4_lease_change (self, ip4_config,
+			                        state == NM_DHCP_STATE_BOUND))
 				nm_device_update_metered (self);
 			else
 				dhcp4_fail (self, state);
@@ -7945,41 +8004,6 @@ dhcp4_state_changed (NMDhcpClient *client,
 	}
 }
 
-static int
-get_dhcp_timeout (NMDevice *self, int addr_family)
-{
-	NMDeviceClass *klass;
-	NMConnection *connection;
-	NMSettingIPConfig *s_ip;
-	guint32 timeout;
-
-	nm_assert (NM_IS_DEVICE (self));
-	nm_assert_addr_family (addr_family);
-
-	connection = nm_device_get_applied_connection (self);
-
-	s_ip = nm_connection_get_setting_ip_config (connection, addr_family);
-
-	timeout = nm_setting_ip_config_get_dhcp_timeout (s_ip);
-	if (timeout)
-		return timeout;
-
-	timeout = nm_config_data_get_connection_default_int64 (NM_CONFIG_GET_DATA,
-	                                                       addr_family == AF_INET
-	                                                         ? NM_CON_DEFAULT ("ipv4.dhcp-timeout")
-	                                                         : NM_CON_DEFAULT ("ipv6.dhcp-timeout"),
-	                                                       self,
-	                                                       0, G_MAXINT32, 0);
-	if (timeout)
-		return timeout;
-
-	klass = NM_DEVICE_GET_CLASS (self);
-	if (klass->get_dhcp_timeout)
-		timeout = klass->get_dhcp_timeout (self, addr_family);
-
-	return timeout ?: NM_DHCP_TIMEOUT_DEFAULT;
-}
-
 /**
  * dhcp_get_iaid:
  * @self: the #NMDevice
@@ -8653,12 +8677,24 @@ dhcp6_fail (NMDevice *self, NMDhcpState dhcp_state)
 		 * 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,
+			guint32 timeout;
+
+			/* Start a grace period equal to the DHCP timeout multiplied
+			 * by a constant factor. */
+			timeout = get_dhcp_timeout (self, AF_INET6);
+			if (timeout < G_MAXUINT32 / DHCP_GRACE_PERIOD_MULTIPLIER) {
+				timeout *= DHCP_GRACE_PERIOD_MULTIPLIER;
+				_LOGI (LOGD_DHCP6,
+				       "DHCPv6: trying to acquire a new lease within %u seconds",
+				       timeout);
+			} else {
+				timeout = G_MAXUINT32;
+				_LOGI (LOGD_DHCP6, "DHCPv6: trying to acquire a new lease");
+			}
+
+			priv->dhcp6.grace_id = g_timeout_add_seconds (timeout,
 			                                              dhcp6_grace_period_expired,
 			                                              self);
-			_LOGI (LOGD_DHCP6,
-			       "DHCPv6: %u seconds grace period started",
-			       DHCP_GRACE_PERIOD_SEC);
 			goto clear_config;
 		}
 	} else {
@@ -8696,6 +8732,7 @@ dhcp6_state_changed (NMDhcpClient *client,
 
 	switch (state) {
 	case NM_DHCP_STATE_BOUND:
+	case NM_DHCP_STATE_EXTENDED:
 		nm_clear_g_source (&priv->dhcp6.grace_id);
 		/* If the server sends multiple IPv6 addresses, we receive a state
 		 * changed event for each of them. Use the event ID to merge IPv6
diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c
index 0a07b26cb9..faac18e01d 100644
--- a/src/dhcp/nm-dhcp-client.c
+++ b/src/dhcp/nm-dhcp-client.c
@@ -317,6 +317,7 @@ nm_dhcp_client_get_use_fqdn (NMDhcpClient *self)
 static const char *state_table[NM_DHCP_STATE_MAX + 1] = {
 	[NM_DHCP_STATE_UNKNOWN]    = "unknown",
 	[NM_DHCP_STATE_BOUND]      = "bound",
+	[NM_DHCP_STATE_EXTENDED]   = "extended",
 	[NM_DHCP_STATE_TIMEOUT]    = "timeout",
 	[NM_DHCP_STATE_EXPIRE]     = "expire",
 	[NM_DHCP_STATE_DONE]       = "done",
@@ -336,13 +337,14 @@ static NMDhcpState
 reason_to_state (NMDhcpClient *self, const char *iface, const char *reason)
 {
 	if (g_ascii_strcasecmp (reason, "bound") == 0 ||
-	    g_ascii_strcasecmp (reason, "bound6") == 0 ||
-	    g_ascii_strcasecmp (reason, "renew") == 0 ||
-	    g_ascii_strcasecmp (reason, "renew6") == 0 ||
-	    g_ascii_strcasecmp (reason, "reboot") == 0 ||
-	    g_ascii_strcasecmp (reason, "rebind") == 0 ||
-	    g_ascii_strcasecmp (reason, "rebind6") == 0)
+	    g_ascii_strcasecmp (reason, "bound6") == 0)
 		return NM_DHCP_STATE_BOUND;
+	else if (g_ascii_strcasecmp (reason, "renew") == 0 ||
+	         g_ascii_strcasecmp (reason, "renew6") == 0 ||
+	         g_ascii_strcasecmp (reason, "reboot") == 0 ||
+	         g_ascii_strcasecmp (reason, "rebind") == 0 ||
+	         g_ascii_strcasecmp (reason, "rebind6") == 0)
+		return NM_DHCP_STATE_EXTENDED;
 	else if (g_ascii_strcasecmp (reason, "timeout") == 0)
 		return NM_DHCP_STATE_TIMEOUT;
 	else if (g_ascii_strcasecmp (reason, "nak") == 0 ||
@@ -415,7 +417,7 @@ nm_dhcp_client_set_state (NMDhcpClient *self,
 	NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
 	gs_free char *event_id = NULL;
 
-	if (new_state == NM_DHCP_STATE_BOUND) {
+	if (NM_IN_SET (new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) {
 		g_return_if_fail (NM_IS_IP_CONFIG (ip_config, priv->addr_family));
 		g_return_if_fail (options);
 	} else {
@@ -430,10 +432,11 @@ nm_dhcp_client_set_state (NMDhcpClient *self,
 
 	/* The client may send same-state transitions for RENEW/REBIND events and
 	 * the lease may have changed, so handle same-state transitions for the
-	 * BOUND state.  Ignore same-state transitions for other events since
-	 * the lease won't have changed and the state was already handled.
+	 * EXTENDED and BOUND states.  Ignore same-state transitions for other
+	 * events since the lease won't have changed and the state was already handled.
 	 */
-	if ((priv->state == new_state) && (new_state != NM_DHCP_STATE_BOUND))
+	if (   (priv->state == new_state)
+	    && !NM_IN_SET (new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED))
 		return;
 
 	if (_LOGI_ENABLED ()) {
@@ -448,7 +451,7 @@ nm_dhcp_client_set_state (NMDhcpClient *self,
 	}
 
 	if (   priv->addr_family == AF_INET6
-	    && new_state == NM_DHCP_STATE_BOUND) {
+	    && NM_IN_SET (new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) {
 		char *start, *iaid;
 
 		iaid = g_hash_table_lookup (options, "iaid");
@@ -877,7 +880,7 @@ nm_dhcp_client_handle_event (gpointer unused,
 	_LOGD ("DHCP state '%s' -> '%s' (reason: '%s')",
 	       state_to_string (old_state), state_to_string (new_state), reason);
 
-	if (new_state == NM_DHCP_STATE_BOUND) {
+	if (NM_IN_SET (new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) {
 		GVariantIter iter;
 		const char *name;
 		GVariant *value;
@@ -918,7 +921,7 @@ nm_dhcp_client_handle_event (gpointer unused,
 		nm_dhcp_client_emit_ipv6_prefix_delegated (self, &prefix);
 	} else {
 		/* Fail if no valid IP config was received */
-		if (   new_state == NM_DHCP_STATE_BOUND
+		if (   NM_IN_SET (new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)
 		    && !ip_config) {
 			_LOGW ("client bound but IP config not received");
 			new_state = NM_DHCP_STATE_FAIL;
diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h
index 6a431fa888..884de85040 100644
--- a/src/dhcp/nm-dhcp-client.h
+++ b/src/dhcp/nm-dhcp-client.h
@@ -43,7 +43,8 @@
 
 typedef enum {
 	NM_DHCP_STATE_UNKNOWN = 0,
-	NM_DHCP_STATE_BOUND,        /* new lease or lease changed */
+	NM_DHCP_STATE_BOUND,        /* new lease */
+	NM_DHCP_STATE_EXTENDED,     /* lease extended */
 	NM_DHCP_STATE_TIMEOUT,      /* timed out contacting server */
 	NM_DHCP_STATE_DONE,         /* client quit or stopped */
 	NM_DHCP_STATE_EXPIRE,       /* lease expired or NAKed */
diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c
index 1119b3bf89..da1d2598a8 100644
--- a/src/dhcp/nm-dhcp-nettools.c
+++ b/src/dhcp/nm-dhcp-nettools.c
@@ -976,7 +976,7 @@ lease_save (NMDhcpNettools *self, NDhcp4ClientLease *lease, const char *lease_fi
 }
 
 static void
-bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease)
+bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended)
 {
 	NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE (self);
 	const char *iface = nm_dhcp_client_get_iface (NM_DHCP_CLIENT (self));
@@ -984,7 +984,7 @@ bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease)
 	gs_unref_hashtable GHashTable *options = NULL;
 	GError *error = NULL;
 
-	_LOGT ("lease available");
+	_LOGT ("lease available (%s)", extended ? "extended" : "new");
 
 	ip4_config = lease_to_ip4_config (nm_dhcp_client_get_multi_idx (NM_DHCP_CLIENT (self)),
 	                                  iface,
@@ -1005,7 +1005,7 @@ bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease)
 	lease_save (self, lease, priv->lease_file);
 
 	nm_dhcp_client_set_state (NM_DHCP_CLIENT (self),
-	                          NM_DHCP_STATE_BOUND,
+	                          extended ? NM_DHCP_STATE_EXTENDED : NM_DHCP_STATE_BOUND,
 	                          NM_IP_CONFIG_CAST (ip4_config),
 	                          options);
 }
@@ -1036,10 +1036,10 @@ dhcp4_event_handle (NMDhcpNettools *self,
 		break;
 	case N_DHCP4_CLIENT_EVENT_GRANTED:
 		priv->lease = n_dhcp4_client_lease_ref (event->granted.lease);
-		bound4_handle (self, event->granted.lease);
+		bound4_handle (self, event->granted.lease, FALSE);
 		break;
 	case N_DHCP4_CLIENT_EVENT_EXTENDED:
-		bound4_handle (self, event->extended.lease);
+		bound4_handle (self, event->extended.lease, TRUE);
 		break;
 	case N_DHCP4_CLIENT_EVENT_DOWN:
 		/* ignore down events, they are purely informational */
diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c
index 3447ebf684..4f9b1113c1 100644
--- a/src/dhcp/nm-dhcp-systemd.c
+++ b/src/dhcp/nm-dhcp-systemd.c
@@ -477,7 +477,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx,
 /*****************************************************************************/
 
 static void
-bound4_handle (NMDhcpSystemd *self)
+bound4_handle (NMDhcpSystemd *self, gboolean extended)
 {
 	NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self);
 	const char *iface = nm_dhcp_client_get_iface (NM_DHCP_CLIENT (self));
@@ -514,7 +514,7 @@ bound4_handle (NMDhcpSystemd *self)
 	dhcp_lease_save (lease, priv->lease_file);
 
 	nm_dhcp_client_set_state (NM_DHCP_CLIENT (self),
-	                          NM_DHCP_STATE_BOUND,
+	                          extended ? NM_DHCP_STATE_EXTENDED : NM_DHCP_STATE_BOUND,
 	                          NM_IP_CONFIG_CAST (ip4_config),
 	                          options);
 }
@@ -538,8 +538,10 @@ dhcp_event_cb (sd_dhcp_client *client, int event, gpointer user_data)
 		break;
 	case SD_DHCP_CLIENT_EVENT_RENEW:
 	case SD_DHCP_CLIENT_EVENT_IP_CHANGE:
+		bound4_handle (self, TRUE);
+		break;
 	case SD_DHCP_CLIENT_EVENT_IP_ACQUIRE:
-		bound4_handle (self);
+		bound4_handle (self, FALSE);
 		break;
 	case SD_DHCP_CLIENT_EVENT_SELECTING:
 		break;
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index e6174a99b6..d9a6d55a85 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -923,7 +923,7 @@ nm_ndisc_start (NMNDisc *ndisc)
 	switch (priv->node_type) {
 	case NM_NDISC_NODE_TYPE_HOST:
 		ra_wait_secs = (((gint64) priv->router_solicitations) * priv->router_solicitation_interval) + 1;
-		ra_wait_secs = CLAMP (ra_wait_secs, 30, 120);
+		ra_wait_secs = MAX (ra_wait_secs, 30);
 		priv->ra_timeout_id = g_timeout_add_seconds (ra_wait_secs, ndisc_ra_timeout_cb, ndisc);
 		_LOGD ("scheduling RA timeout in %d seconds", (int) ra_wait_secs);
 		solicit_routers (ndisc);
diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c
index bfb7af578e..20a0b674b3 100644
--- a/src/nm-iface-helper.c
+++ b/src/nm-iface-helper.c
@@ -106,6 +106,7 @@ dhcp4_state_changed (NMDhcpClient *client,
 
 	switch (state) {
 	case NM_DHCP_STATE_BOUND:
+	case NM_DHCP_STATE_EXTENDED:
 		g_assert (ip4_config);
 		g_assert (nm_ip4_config_get_ifindex (ip4_config) == gl.ifindex);