device: don't schedule grace timeout if dhcp-timeout is infinity

It feels wrong to schedule a timeout with G_MAXUINT32, if we actually
disabled the timeout. Of course, in practice there should be little
difference.

3 files changed, 67 insertions, 33 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index fdad064759..4cfc914508 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -76,8 +76,21 @@ _LOG_DECLARE_SELF (NMDevice);
 /*****************************************************************************/
 
 #define DEFAULT_AUTOCONNECT    TRUE
+
+static guint32
+dhcp_grace_period_from_timeout (guint32 timeout)
+{
 #define DHCP_GRACE_PERIOD_MULTIPLIER 2U
 
+	nm_assert (timeout > 0);
+	nm_assert (timeout < G_MAXINT32);
+
+	if (timeout < G_MAXUINT32 / DHCP_GRACE_PERIOD_MULTIPLIER)
+		return timeout * DHCP_GRACE_PERIOD_MULTIPLIER;
+
+	return G_MAXUINT32;
+}
+
 #define CARRIER_WAIT_TIME_MS 6000
 #define CARRIER_WAIT_TIME_AFTER_MTU_MS 10000
 
@@ -463,12 +476,13 @@ typedef struct _NMDevicePrivate {
 	/* DHCPv4 tracking */
 	struct {
 		NMDhcpClient *  client;
-		gulong          state_sigid;
 		NMDhcp4Config * config;
 		char *          pac_url;
 		char *          root_path;
-		bool            was_active;
+		gulong          state_sigid;
 		guint           grace_id;
+		bool            was_active:1;
+		bool            grace_pending:1;
 	} dhcp4;
 
 	struct {
@@ -533,17 +547,18 @@ typedef struct _NMDevicePrivate {
 
 	struct {
 		NMDhcpClient *   client;
-		NMNDiscDHCPLevel mode;
-		gulong           state_sigid;
-		gulong           prefix_sigid;
 		NMDhcp6Config *  config;
 		/* IP6 config from DHCP */
 		AppliedConfig    ip6_config;
 		/* Event ID of the current IP6 config from DHCP */
 		char *           event_id;
+		gulong           state_sigid;
+		gulong           prefix_sigid;
+		NMNDiscDHCPLevel mode;
 		guint            needed_prefixes;
-		bool             was_active;
 		guint            grace_id;
+		bool             was_active:1;
+		bool             grace_pending:1;
 	} dhcp6;
 
 	gboolean needs_ip6_subnet;
@@ -7533,7 +7548,7 @@ get_dhcp_timeout (NMDevice *self, int addr_family)
 {
 	NMDeviceClass *klass;
 	NMConnection *connection;
-	NMSettingIPConfig *s_ip;
+	int timeout_i;
 	guint32 timeout;
 
 	nm_assert (NM_IS_DEVICE (self));
@@ -7541,11 +7556,12 @@ get_dhcp_timeout (NMDevice *self, int addr_family)
 
 	connection = nm_device_get_applied_connection (self);
 
-	s_ip = nm_connection_get_setting_ip_config (connection, addr_family);
+	timeout_i  = nm_setting_ip_config_get_dhcp_timeout (nm_connection_get_setting_ip_config (connection, addr_family));
+	nm_assert (timeout_i >= 0 && timeout_i <= G_MAXINT32);
 
-	timeout = nm_setting_ip_config_get_dhcp_timeout (s_ip);
+	timeout = (guint32) timeout_i;
 	if (timeout)
-		return timeout;
+		goto out;
 
 	timeout = nm_config_data_get_connection_default_int64 (NM_CONFIG_GET_DATA,
 	                                                       addr_family == AF_INET
@@ -7554,13 +7570,22 @@ get_dhcp_timeout (NMDevice *self, int addr_family)
 	                                                       self,
 	                                                       0, G_MAXINT32, 0);
 	if (timeout)
-		return timeout;
+		goto out;
 
 	klass = NM_DEVICE_GET_CLASS (self);
-	if (klass->get_dhcp_timeout_for_device)
+	if (klass->get_dhcp_timeout_for_device) {
 		timeout = klass->get_dhcp_timeout_for_device (self, addr_family);
+		if (timeout)
+			goto out;
+	}
 
-	return timeout ?: NM_DHCP_TIMEOUT_DEFAULT;
+	timeout = NM_DHCP_TIMEOUT_DEFAULT;
+
+out:
+	G_STATIC_ASSERT_EXPR (G_MAXINT32 == NM_DHCP_TIMEOUT_INFINITY);
+	nm_assert (timeout > 0);
+	nm_assert (timeout <= G_MAXINT32);
+	return timeout;
 }
 
 static void
@@ -7570,6 +7595,7 @@ dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release)
 
 	priv->dhcp4.was_active = FALSE;
 	nm_clear_g_source (&priv->dhcp4.grace_id);
+	priv->dhcp4.grace_pending = FALSE;
 	g_clear_pointer (&priv->dhcp4.pac_url, g_free);
 	g_clear_pointer (&priv->dhcp4.root_path, g_free);
 
@@ -7850,6 +7876,7 @@ dhcp4_grace_period_expired (gpointer user_data)
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
 	priv->dhcp4.grace_id = 0;
+	priv->dhcp4.grace_pending = FALSE;
 	_LOGI (LOGD_DHCP4, "DHCPv4: grace period expired");
 
 	nm_device_ip_method_failed (self, AF_INET,
@@ -7897,25 +7924,26 @@ dhcp4_fail (NMDevice *self, NMDhcpState dhcp_state)
 	/* In any other case (expired lease, assumed connection, etc.),
 	 * wait for some time before failing the IP method.
 	 */
-	if (!priv->dhcp4.grace_id) {
+	if (!priv->dhcp4.grace_pending) {
 		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;
+		if (timeout == NM_DHCP_TIMEOUT_INFINITY) {
+			_LOGI (LOGD_DHCP4, "DHCPv4: trying to acquire a new lease");
+		} else {
+			timeout = dhcp_grace_period_from_timeout (timeout);
 			_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");
+			nm_assert (!priv->dhcp4.grace_id);
+			priv->dhcp4.grace_id = g_timeout_add_seconds (timeout,
+			                                              dhcp4_grace_period_expired,
+			                                              self);
 		}
 
-		priv->dhcp4.grace_id = g_timeout_add_seconds (timeout,
-		                                              dhcp4_grace_period_expired,
-		                                              self);
+		priv->dhcp4.grace_pending = TRUE;
 		goto clear_config;
 	}
 	return;
@@ -7972,6 +8000,7 @@ dhcp4_state_changed (NMDhcpClient *client,
 		}
 
 		nm_clear_g_source (&priv->dhcp4.grace_id);
+		priv->dhcp4.grace_pending = FALSE;
 
 		/* After some failures, we have been able to renew the lease:
 		 * update the ip state
@@ -8598,6 +8627,7 @@ dhcp6_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release)
 	applied_config_clear (&priv->dhcp6.ip6_config);
 	g_clear_pointer (&priv->dhcp6.event_id, g_free);
 	nm_clear_g_source (&priv->dhcp6.grace_id);
+	priv->dhcp6.grace_pending = FALSE;
 
 	if (priv->dhcp6.client) {
 		nm_clear_g_signal_handler (priv->dhcp6.client, &priv->dhcp6.state_sigid);
@@ -8653,6 +8683,7 @@ dhcp6_grace_period_expired (gpointer user_data)
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
 	priv->dhcp6.grace_id = 0;
+	priv->dhcp6.grace_pending = FALSE;
 	_LOGI (LOGD_DHCP6, "DHCPv6: grace period expired");
 
 	nm_device_ip_method_failed (self, AF_INET6,
@@ -8704,25 +8735,26 @@ dhcp6_fail (NMDevice *self, NMDhcpState dhcp_state)
 		/* In any other case (expired lease, assumed connection, etc.),
 		 * wait for some time before failing the IP method.
 		 */
-		if (!priv->dhcp6.grace_id) {
+		if (!priv->dhcp6.grace_pending) {
 			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;
+			if (timeout == NM_DHCP_TIMEOUT_INFINITY)
+				_LOGI (LOGD_DHCP6, "DHCPv6: trying to acquire a new lease");
+			else {
+				timeout = dhcp_grace_period_from_timeout (timeout);
 				_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");
+				nm_assert (!priv->dhcp6.grace_id);
+				priv->dhcp6.grace_id = g_timeout_add_seconds (timeout,
+				                                              dhcp6_grace_period_expired,
+				                                              self);
 			}
 
-			priv->dhcp6.grace_id = g_timeout_add_seconds (timeout,
-			                                              dhcp6_grace_period_expired,
-			                                              self);
+			priv->dhcp6.grace_pending = TRUE;
 			goto clear_config;
 		}
 	} else {
@@ -8762,6 +8794,7 @@ dhcp6_state_changed (NMDhcpClient *client,
 	case NM_DHCP_STATE_BOUND:
 	case NM_DHCP_STATE_EXTENDED:
 		nm_clear_g_source (&priv->dhcp6.grace_id);
+		priv->dhcp6.grace_pending = FALSE;
 		/* If the server sends multiple IPv6 addresses, we receive a state
 		 * changed event for each of them. Use the event ID to merge IPv6
 		 * addresses from the same transaction into a single configuration.
diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c
index e7bc8831eb..5cc0472c00 100644
--- a/src/dhcp/nm-dhcp-client.c
+++ b/src/dhcp/nm-dhcp-client.c
@@ -1203,6 +1203,7 @@ nm_dhcp_client_class_init (NMDhcpClientClass *client_class)
 	                       G_PARAM_READWRITE |
 	                       G_PARAM_STATIC_STRINGS);
 
+	G_STATIC_ASSERT_EXPR (G_MAXINT32 == NM_DHCP_TIMEOUT_INFINITY);
 	obj_properties[PROP_TIMEOUT] =
 	    g_param_spec_uint (NM_DHCP_CLIENT_TIMEOUT, "", "",
 	                       1, G_MAXINT32, NM_DHCP_TIMEOUT_DEFAULT,
diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h
index 884de85040..1ff09067e8 100644
--- a/src/dhcp/nm-dhcp-client.h
+++ b/src/dhcp/nm-dhcp-client.h
@@ -12,8 +12,8 @@
 #include "nm-ip6-config.h"
 #include "nm-dhcp-utils.h"
 
-#define NM_DHCP_TIMEOUT_DEFAULT ((guint32) 45) /* default DHCP timeout, in seconds */
-#define NM_DHCP_TIMEOUT_INFINITY G_MAXINT32
+#define NM_DHCP_TIMEOUT_DEFAULT  ((guint32) 45) /* default DHCP timeout, in seconds */
+#define NM_DHCP_TIMEOUT_INFINITY ((guint32) G_MAXINT32)
 
 #define NM_TYPE_DHCP_CLIENT            (nm_dhcp_client_get_type ())
 #define NM_DHCP_CLIENT(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_DHCP_CLIENT, NMDhcpClient))