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