device: fix delaying startup complete waiting for carrierth/device-carrier-wait-rh1450444

    platform: signal: link changed: 2: eth0 <DOWN;broadcast,multicast> mtu ...
    ...
    device[0x7f90c29c64d0] (eth0): bringing up device
    ...
    platform: signal: link changed: 2: eth0 <UP,LOWER_UP;broadcast,multicast,up,running,lowerup> mtu ...
    ...
    device (eth0): link connected
    ...
    device[0x7f90c29c64d0] (eth0): add_pending_action (2): 'carrier wait'

Note how we schedule the pending action 'carrier-wait', altough the device
already has carrier. That means, the pending action will not be removed
until timeout, 5 seconds later.

Avoid scheduling 'carrier-wait' if we already have carrier.

However, we don't just add the pending action 'carrier-wait' only in
nm_device_bring_up(). Instead, always schedule the carrier_wait timeout.
This is a grace period during which we keep setting 'carrier-wait' whenever
we have no carrier. This should prevent two cases:
  - during nm_device_bring_up() the platform state might not yet have
    caught up. If we don't add the pending action there, we will add
    it a moment later when carrier goes away.
  - bringing the interface up might cause carrier to get lost for a
    moment (flapping). If that happens within the timeout, consider
    add the pending action.

1 files changed, 29 insertions, 18 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 1ea051c918..4f70d75d66 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2290,19 +2290,23 @@ nm_device_set_carrier (NMDevice *self, gboolean carrier)
 		carrier_disconnected_action_cancel (self);
 		carrier_changed (self, TRUE);
 
-		if (nm_clear_g_source (&priv->carrier_wait_id)) {
-			nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE);
+		if (priv->carrier_wait_id) {
+			nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE);
 			_carrier_wait_check_queued_act_request (self);
 		}
-	} else if (   state <= NM_DEVICE_STATE_DISCONNECTED
-	           && !priv->queued_act_request) {
-		_LOGD (LOGD_DEVICE, "link disconnected");
-		carrier_changed (self, FALSE);
 	} else {
-		priv->carrier_defer_id = g_timeout_add_seconds (LINK_DISCONNECT_DELAY,
-		                                                carrier_disconnected_action_cb, self);
-		_LOGD (LOGD_DEVICE, "link disconnected (deferring action for %d seconds) (id=%u)",
-		       LINK_DISCONNECT_DELAY, priv->carrier_defer_id);
+		if (priv->carrier_wait_id)
+			nm_device_add_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE);
+		if (   state <= NM_DEVICE_STATE_DISCONNECTED
+		    && !priv->queued_act_request) {
+			_LOGD (LOGD_DEVICE, "link disconnected");
+			carrier_changed (self, FALSE);
+		} else {
+			priv->carrier_defer_id = g_timeout_add_seconds (LINK_DISCONNECT_DELAY,
+			                                                carrier_disconnected_action_cb, self);
+			_LOGD (LOGD_DEVICE, "link disconnected (deferring action for %d seconds) (id=%u)",
+			       LINK_DISCONNECT_DELAY, priv->carrier_defer_id);
+		}
 	}
 }
 
@@ -10241,12 +10245,12 @@ static gboolean
 carrier_wait_timeout (gpointer user_data)
 {
 	NMDevice *self = NM_DEVICE (user_data);
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	NM_DEVICE_GET_PRIVATE (self)->carrier_wait_id = 0;
-	nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE);
-
-	_carrier_wait_check_queued_act_request (self);
-
+	priv->carrier_wait_id = 0;
+	nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE);
+	if (!priv->carrier)
+		_carrier_wait_check_queued_act_request (self);
 	return G_SOURCE_REMOVE;
 }
 
@@ -10323,8 +10327,14 @@ nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware)
 	 * a timeout is reached.
 	 */
 	if (nm_device_has_capability (self, NM_DEVICE_CAP_CARRIER_DETECT)) {
-		if (!nm_clear_g_source (&priv->carrier_wait_id))
-			nm_device_add_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE);
+		/* we start a grace period of 5 seconds during which we will schedule
+		 * a pending action whenever we have no carrier.
+		 *
+		 * If during that time carrier goes away, we declare the interface
+		 * as not ready. */
+		nm_clear_g_source (&priv->carrier_wait_id);
+		if (!priv->carrier)
+			nm_device_add_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE);
 		priv->carrier_wait_id = g_timeout_add_seconds (5, carrier_wait_timeout, self);
 	}
 
@@ -13738,7 +13748,8 @@ dispose (GObject *object)
 
 	available_connections_del_all (self);
 
-	nm_clear_g_source (&priv->carrier_wait_id);
+	if (nm_clear_g_source (&priv->carrier_wait_id))
+		nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE);
 
 	_clear_queued_act_request (priv);