From bd63d39252eab03ea2c3f02b9c9b7d3ab338ef30 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 7 May 2018 16:57:48 +0200
Subject: [PATCH] dhcp: fix handling of failure events

DHCPv4 can fail for two reasons:

 (a) the client failed to contact server and to get an initial lease

 (b) the client failed to renew the lease after it was successfully
     acquired

For (a) the client generates a TIMEOUT event, for (b) an EXPIRED
event.  Currently we fail the IP method immediately after (a), but
this doesn't work well when the carrier flickers and we restart the
client because if the server goes temporarily down, the IP method
fails and DHCP is never restarted.

Let's change this, and determine whether to fail IP configuration only
by looking at the current IP state: when it's IP_CONF then we are
getting the initial lease and a failure means that IP configuration
must fail; otherwise any other state means that the lease expired or
could not be renewed and thus we keep the client running for the grace
period.

https://bugzilla.redhat.com/show_bug.cgi?id=1573780
---
 src/devices/nm-device.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index cf6baf47a1..7c46f1d608 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -6830,11 +6830,12 @@ dhcp4_grace_period_expired (gpointer user_data)
 }
 
 static void
-dhcp4_fail (NMDevice *self, gboolean timeout)
+dhcp4_fail (NMDevice *self)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	_LOGD (LOGD_DHCP4, "DHCPv4 failed%s", timeout ? " (timeout)" : "");
+	_LOGD (LOGD_DHCP4, "DHCPv4 failed (ip_state %s)",
+	       _ip_state_to_string (priv->ip4_state));
 
 	/* Keep client running if there are static addresses configured
 	 * on the interface.
@@ -6848,7 +6849,7 @@ dhcp4_fail (NMDevice *self, gboolean timeout)
 	 * configuration.
 	 */
 	if (   !priv->dhcp4.was_active
-	    && (timeout || priv->ip4_state == IP_CONF)) {
+	    && priv->ip4_state == IP_CONF) {
 		dhcp4_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE);
 		nm_device_activate_schedule_ip4_config_timeout (self);
 		return;
@@ -6911,7 +6912,7 @@ dhcp4_state_changed (NMDhcpClient *client,
 	case NM_DHCP_STATE_BOUND:
 		if (!ip4_config) {
 			_LOGW (LOGD_DHCP4, "failed to get IPv4 config in response to DHCP event.");
-			dhcp4_fail (self, FALSE);
+			dhcp4_fail (self);
 			break;
 		}
 
@@ -6950,11 +6951,11 @@ dhcp4_state_changed (NMDhcpClient *client,
 			if (dhcp4_lease_change (self, ip4_config))
 				nm_device_update_metered (self);
 			else
-				dhcp4_fail (self, FALSE);
+				dhcp4_fail (self);
 		}
 		break;
 	case NM_DHCP_STATE_TIMEOUT:
-		dhcp4_fail (self, TRUE);
+		dhcp4_fail (self);
 		break;
 	case NM_DHCP_STATE_EXPIRE:
 		/* Ignore expiry before we even have a lease (NAK, old lease, etc) */
@@ -6963,7 +6964,7 @@ dhcp4_state_changed (NMDhcpClient *client,
 		/* fall through */
 	case NM_DHCP_STATE_DONE:
 	case NM_DHCP_STATE_FAIL:
-		dhcp4_fail (self, FALSE);
+		dhcp4_fail (self);
 		break;
 	default:
 		break;