Blob Blame History Raw
From 2d98ce90188fdf4d21c2b597ba848d249a2d4e09 Mon Sep 17 00:00:00 2001
From: Francesco Giudici <fgiudici@redhat.com>
Date: Mon, 15 Jan 2018 12:49:33 +0100
Subject: [PATCH 1/3] device: always consider both ip families when deciding to
 fail

Example: when dhcpv4 lease renewal fails, if ipv4.may-fail was "yes",
check also if we have a successful ipv6 conf: if not fail.
Previously we just ignored the other ip family status.

(cherry picked from commit da0fee4d9f16d0de6323ea709e459b79b5158dad)
---
 src/devices/nm-device.c | 53 ++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index f8651e2e1..345034185 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -3774,21 +3774,24 @@ get_ip_config_may_fail (NMDevice *self, int addr_family)
 /*
  * check_ip_state
  *
- * Transition the device from IP_CONFIG to the next state according to the
- * outcome of IPv4 and IPv6 configuration. @may_fail indicates that we are
- * called just after the initial configuration and thus IPv4/IPv6 are allowed to
- * fail if the ipvx.may-fail properties say so, because the IP methods couldn't
- * even be started.
+ * When @full_state_update is TRUE, transition the device from IP_CONFIG to the
+ * next state according to the outcome of IPv4 and IPv6 configuration. @may_fail
+ * indicates that we are called just after the initial configuration and thus
+ * IPv4/IPv6 are allowed to fail if the ipvx.may-fail properties say so, because
+ * the IP methods couldn't even be started.
+ * If @full_state_update is FALSE, just check if the connection should be failed
+ * due to the state of both ip families and the ipvx.may-fail settings.
  */
 static void
-check_ip_state (NMDevice *self, gboolean may_fail)
+check_ip_state (NMDevice *self, gboolean may_fail, gboolean full_state_update)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 	gboolean ip4_disabled = FALSE, ip6_ignore = FALSE;
 	NMSettingIPConfig *s_ip4, *s_ip6;
 	NMDeviceState state;
 
-	if (nm_device_get_state (self) != NM_DEVICE_STATE_IP_CONFIG)
+	if (   full_state_update
+	    && nm_device_get_state (self) != NM_DEVICE_STATE_IP_CONFIG)
 		return;
 
 	/* Don't progress into IP_CHECK or SECONDARIES if we're waiting for the
@@ -3835,9 +3838,12 @@ check_ip_state (NMDevice *self, gboolean may_fail)
 			state = NM_DEVICE_STATE_FAILED;
 		}
 
-		nm_device_state_changed (self,
-		                         state,
-		                         NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE);
+		if (   full_state_update
+		    || state == NM_DEVICE_STATE_FAILED) {
+			nm_device_state_changed (self,
+			                         state,
+			                         NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE);
+		}
 		return;
 	}
 
@@ -3850,7 +3856,8 @@ check_ip_state (NMDevice *self, gboolean may_fail)
 	/* If at least a method has completed, proceed with activation */
 	if (   (priv->ip4_state == IP_DONE && !ip4_disabled)
 	    || (priv->ip6_state == IP_DONE && !ip6_ignore)) {
-		nm_device_state_changed (self, NM_DEVICE_STATE_IP_CHECK, NM_DEVICE_STATE_REASON_NONE);
+		if (full_state_update)
+			nm_device_state_changed (self, NM_DEVICE_STATE_IP_CHECK, NM_DEVICE_STATE_REASON_NONE);
 		return;
 	}
 }
@@ -3892,7 +3899,7 @@ nm_device_slave_notify_enslave (NMDevice *self, gboolean success)
 
 	if (activating) {
 		if (success)
-			check_ip_state (self, FALSE);
+			check_ip_state (self, FALSE, TRUE);
 		else
 			nm_device_queue_state (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_UNKNOWN);
 	} else
@@ -5268,7 +5275,7 @@ nm_device_ip_method_failed (NMDevice *self,
 	_set_ip_state (self, addr_family, IP_FAIL);
 
 	if (get_ip_config_may_fail (self, addr_family))
-		check_ip_state (self, FALSE);
+		check_ip_state (self, FALSE, (nm_device_get_state (self) == NM_DEVICE_STATE_IP_CONFIG));
 	else
 		nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason);
 }
@@ -7997,7 +8004,7 @@ nm_device_activate_stage3_ip4_start (NMDevice *self)
 
 	if (nm_device_sys_iface_state_is_external (self)) {
 		_set_ip_state (self, AF_INET, IP_DONE);
-		check_ip_state (self, FALSE);
+		check_ip_state (self, FALSE, TRUE);
 		return TRUE;
 	}
 
@@ -8010,7 +8017,7 @@ nm_device_activate_stage3_ip4_start (NMDevice *self)
 		g_object_unref (ip4_config);
 	} else if (ret == NM_ACT_STAGE_RETURN_IP_DONE) {
 		_set_ip_state (self, AF_INET, IP_DONE);
-		check_ip_state (self, FALSE);
+		check_ip_state (self, FALSE, TRUE);
 	} else if (ret == NM_ACT_STAGE_RETURN_FAILURE) {
 		nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason);
 		return FALSE;
@@ -8044,7 +8051,7 @@ nm_device_activate_stage3_ip6_start (NMDevice *self)
 
 	if (nm_device_sys_iface_state_is_external (self)) {
 		_set_ip_state (self, AF_INET6, IP_DONE);
-		check_ip_state (self, FALSE);
+		check_ip_state (self, FALSE, TRUE);
 		return TRUE;
 	}
 
@@ -8061,7 +8068,7 @@ nm_device_activate_stage3_ip6_start (NMDevice *self)
 		nm_device_activate_schedule_ip6_config_result (self);
 	} else if (ret == NM_ACT_STAGE_RETURN_IP_DONE) {
 		_set_ip_state (self, AF_INET6, IP_DONE);
-		check_ip_state (self, FALSE);
+		check_ip_state (self, FALSE, TRUE);
 	} else if (ret == NM_ACT_STAGE_RETURN_FAILURE) {
 		nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason);
 		return FALSE;
@@ -8111,7 +8118,7 @@ activate_stage3_ip_config_start (NMDevice *self)
 	/* Proxy */
 	nm_device_set_proxy_config (self, NULL);
 
-	check_ip_state (self, TRUE);
+	check_ip_state (self, TRUE, TRUE);
 }
 
 static void
@@ -8250,7 +8257,7 @@ activate_stage4_ip4_config_timeout (NMDevice *self)
 
 	_set_ip_state (self, AF_INET, IP_FAIL);
 
-	check_ip_state (self, FALSE);
+	check_ip_state (self, FALSE, TRUE);
 }
 
 /*
@@ -8306,7 +8313,7 @@ activate_stage4_ip6_config_timeout (NMDevice *self)
 
 	_set_ip_state (self, AF_INET6, IP_FAIL);
 
-	check_ip_state (self, FALSE);
+	check_ip_state (self, FALSE, TRUE);
 }
 
 /*
@@ -8554,7 +8561,7 @@ activate_stage5_ip4_config_result (NMDevice *self)
 
 	/* Enter the IP_CHECK state if this is the first method to complete */
 	_set_ip_state (self, AF_INET, IP_DONE);
-	check_ip_state (self, FALSE);
+	check_ip_state (self, FALSE, TRUE);
 }
 
 void
@@ -8709,7 +8716,7 @@ activate_stage5_ip6_config_commit (NMDevice *self)
 				_LOGD (LOGD_DEVICE | LOGD_IP6, "IPv6 DAD: awaiting termination");
 			} else {
 				_set_ip_state (self, AF_INET6, IP_DONE);
-				check_ip_state (self, FALSE);
+				check_ip_state (self, FALSE, TRUE);
 			}
 		}
 	} else {
@@ -11195,7 +11202,7 @@ queued_ip6_config_change (gpointer user_data)
 			_LOGD (LOGD_DEVICE | LOGD_IP6, "IPv6 DAD terminated");
 			g_clear_object (&priv->dad6_ip6_config);
 			_set_ip_state (self, AF_INET6, IP_DONE);
-			check_ip_state (self, FALSE);
+			check_ip_state (self, FALSE, TRUE);
 			if (priv->rt6_temporary_not_available)
 				nm_device_activate_schedule_ip6_config_result (self);
 		}
-- 
2.14.3


From 56353bfb82e6d744b64f117f346b25b18526bf2d Mon Sep 17 00:00:00 2001
From: Francesco Giudici <fgiudici@redhat.com>
Date: Mon, 15 Jan 2018 12:17:54 +0100
Subject: [PATCH 2/3] device: never stop trying renewing the lease

Always reschedule a lease renewal attempt: just clear the scheduled
renewal if the connection is really deactivated.

(cherry picked from commit 1a20ff86d585b826d1769d0aa0adeca3aa1a183a)
---
 src/devices/nm-device.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 345034185..f6d3f4c04 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -5900,15 +5900,23 @@ dhcp4_fail (NMDevice *self, gboolean timeout)
 	    && (timeout || (priv->ip4_state == IP_CONF))
 	    && !priv->dhcp4.was_active)
 		nm_device_activate_schedule_ip4_config_timeout (self);
-	else if (priv->ip4_state == IP_DONE || priv->dhcp4.was_active) {
+	else if (   priv->dhcp4.num_tries_left < DHCP_NUM_TRIES_MAX
+	         || priv->ip4_state == IP_DONE
+	         || priv->dhcp4.was_active) {
 		/* Don't fail immediately when the lease expires but try to
 		 * restart DHCP for a predefined number of times.
 		 */
 		if (priv->dhcp4.num_tries_left) {
 			priv->dhcp4.num_tries_left--;
 			dhcp_schedule_restart (self, AF_INET, "lease expired");
-		} else
+		} else {
 			nm_device_ip_method_failed (self, AF_INET, NM_DEVICE_STATE_REASON_IP_CONFIG_EXPIRED);
+			/* We failed the ipv4 method but schedule again the retries if the ipv6 method is
+			 * configured, keeping the connection up.
+			 */
+			if (nm_device_get_state (self) != NM_DEVICE_STATE_FAILED)
+				dhcp_schedule_restart (self, AF_INET, "renewal failed");
+		}
 	} else
 		g_warn_if_reached ();
 }
@@ -5950,6 +5958,12 @@ dhcp4_state_changed (NMDhcpClient *client,
 			break;
 		}
 
+		/* After some failures, we have been able to renew the lease:
+		 * update the ip state
+		 */
+		if (priv->ip4_state == IP_FAIL)
+			_set_ip_state (self, AF_INET, IP_CONF);
+
 		g_free (priv->dhcp4.pac_url);
 		priv->dhcp4.pac_url = g_strdup (g_hash_table_lookup (options, "wpad"));
 		nm_device_set_proxy_config (self, priv->dhcp4.pac_url);
@@ -6640,15 +6654,23 @@ dhcp6_fail (NMDevice *self, gboolean timeout)
 		    && (timeout || (priv->ip6_state == IP_CONF))
 		    && !priv->dhcp6.was_active)
 			nm_device_activate_schedule_ip6_config_timeout (self);
-		else if (priv->ip6_state == IP_DONE || priv->dhcp6.was_active) {
+		else if (   priv->dhcp6.num_tries_left < DHCP_NUM_TRIES_MAX
+		         || priv->ip6_state == IP_DONE
+		         || priv->dhcp6.was_active) {
 			/* Don't fail immediately when the lease expires but try to
 			 * restart DHCP for a predefined number of times.
 			 */
 			if (priv->dhcp6.num_tries_left) {
 				priv->dhcp6.num_tries_left--;
 				dhcp_schedule_restart (self, AF_INET6, "lease expired");
-			} else
+			} else {
 				nm_device_ip_method_failed (self, AF_INET6, NM_DEVICE_STATE_REASON_IP_CONFIG_EXPIRED);
+				/* We failed the ipv6 method but schedule again the retries if the ipv4 method is
+				 * configured, keeping the connection up.
+				 */
+				if (nm_device_get_state (self) != NM_DEVICE_STATE_FAILED)
+					dhcp_schedule_restart (self, AF_INET6, "renewal failed");
+			}
 		} else
 			g_warn_if_reached ();
 	} else {
@@ -6715,6 +6737,12 @@ dhcp6_state_changed (NMDhcpClient *client,
 			}
 		}
 
+		/* After long time we have been able to renew the lease:
+		 * update the ip state
+		 */
+		if (priv->ip6_state == IP_FAIL)
+			_set_ip_state (self, AF_INET6, IP_CONF);
+
 		priv->dhcp6.num_tries_left = DHCP_NUM_TRIES_MAX;
 
 		if (priv->ip6_state == IP_CONF) {
-- 
2.14.3


From b6d2ad3312eea21effbd8d6d5fe32056ccf89cb8 Mon Sep 17 00:00:00 2001
From: Francesco Giudici <fgiudici@redhat.com>
Date: Thu, 8 Feb 2018 18:28:10 +0100
Subject: [PATCH 3/3] device: enable DHCPv6 retries on lease renewal failure

https://bugzilla.gnome.org/show_bug.cgi?id=792745
(cherry picked from commit 1289450146e2b212cabca500cb8009f910651661)
---
 src/devices/nm-device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index f6d3f4c04..eb91dc257 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -6633,13 +6633,15 @@ static void
 dhcp6_fail (NMDevice *self, gboolean timeout)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	gboolean is_dhcp_managed;
 
 	_LOGD (LOGD_DHCP6, "DHCPv6 failed: timeout %d, num tries left %u",
            timeout, priv->dhcp6.num_tries_left);
 
+	is_dhcp_managed = (priv->dhcp6.mode == NM_NDISC_DHCP_LEVEL_MANAGED);
 	dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE);
 
-	if (priv->dhcp6.mode == NM_NDISC_DHCP_LEVEL_MANAGED) {
+	if (is_dhcp_managed || priv->dhcp6.num_tries_left < DHCP_NUM_TRIES_MAX) {
 		/* Don't fail if there are static addresses configured on
 		 * the device, instead retry after some time.
 		 */
-- 
2.14.3