device: merge branch 'th/device-dad-handling-rh1593210'

While this fixes rh#1593210 and CI test "ipv6_keep_external_routes",
it breaks CI test "ipv6_add_static_address_manually_not_active". See
related bug rh#1585078. It seems in the short term, the fix is more
important than the new issue, hence merging.

https://bugzilla.redhat.com/show_bug.cgi?id=1593210
https://github.com/NetworkManager/NetworkManager/pull/144

src/devices/nm-device.c

@@ -280,11 +280,6 @@ typedef struct _NMDevicePrivate {
 
 	bool          real:1;
 
-	/* there was a IP config change, but no idle action was scheduled because device
-	 * is still not platform-init */
-	bool queued_ip4_config_pending:1;
-	bool queued_ip6_config_pending:1;
-
 	bool update_ip_config_completed_v4:1;
 	bool update_ip_config_completed_v6:1;
 
@@ -4104,9 +4099,6 @@ realize_start_setup (NMDevice *self,
 		_notify (self, PROP_UDI);
 	}
 
-	priv->queued_ip4_config_pending = TRUE;
-	priv->queued_ip6_config_pending = TRUE;
-
 	nm_device_update_hw_address (self);
 	nm_device_update_initial_hw_address (self);
 	nm_device_update_permanent_hw_address (self, FALSE);
@@ -10263,13 +10255,13 @@ _cleanup_ip_pre (NMDevice *self, int addr_family, CleanupType cleanup_type)
 	}
 
 	if (IS_IPv4) {
-		priv->queued_ip4_config_pending = FALSE;
 		dhcp4_cleanup (self, cleanup_type, FALSE);
 		arp_cleanup (self);
 		dnsmasq_cleanup (self);
 		ipv4ll_cleanup (self);
 	} else {
-		priv->queued_ip6_config_pending = FALSE;
+		g_slist_free_full (priv->dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
+		priv->dad6_failed_addrs = NULL;
 		g_clear_object (&priv->dad6_ip6_config);
 		dhcp6_cleanup (self, cleanup_type, FALSE);
 		nm_clear_g_source (&priv->linklocal6_timeout_id);
@@ -12270,22 +12262,16 @@ static gboolean
 queued_ip_config_change (NMDevice *self, int addr_family)
 {
 	NMDevicePrivate *priv;
-	gboolean need_ipv6ll = FALSE;
 	const gboolean IS_IPv4 = (addr_family == AF_INET);
-	NMPlatform *platform;
 
 	g_return_val_if_fail (NM_IS_DEVICE (self), G_SOURCE_REMOVE);
 
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
-	nm_assert (IS_IPv4 ? !priv->queued_ip4_config_pending : !priv->queued_ip6_config_pending);
-
 	/* Wait for any queued state changes */
 	if (priv->queued_state.id)
 		return G_SOURCE_CONTINUE;
 
-	priv->queued_ip_config_id_x[IS_IPv4] = 0;
-
 	/* If a commit is scheduled, this function would potentially interfere with
 	 * it changing IP configurations before they are applied. Postpone the
 	 * update in such case.
@@ -12294,38 +12280,38 @@ queued_ip_config_change (NMDevice *self, int addr_family)
 	                                    IS_IPv4
 	                                      ? activate_stage5_ip4_config_result
 	                                      : activate_stage5_ip6_config_commit,
-	                                    addr_family)) {
+	                                    addr_family))
-		if (IS_IPv4) {
+		return G_SOURCE_CONTINUE;
-			priv->queued_ip4_config_pending = FALSE;
-			priv->queued_ip_config_id_4 = g_idle_add (queued_ip4_config_change, self);
-		} else {
-			priv->queued_ip6_config_pending = FALSE;
-			priv->queued_ip_config_id_6 = g_idle_add (queued_ip6_config_change, self);
-		}
-		_LOGT (LOGD_DEVICE, "IP%c update was postponed",
-		       nm_utils_addr_family_to_char (addr_family));
-	} else {
-		update_ip_config (self, addr_family);
 
-		if (!IS_IPv4) {
+	priv->queued_ip_config_id_x[IS_IPv4] = 0;
-			/* Check whether we need to complete waiting for link-local.
+
-			 * We are also called from an idle handler, so no problem doing state transitions
+	update_ip_config (self, addr_family);
-			 * now. */
+
-			linklocal6_check_complete (self);
+	if (!IS_IPv4) {
-		}
+		/* Check whether we need to complete waiting for link-local.
+		 * We are also called from an idle handler, so no problem doing state transitions
+		 * now. */
+		linklocal6_check_complete (self);
 	}
 
 	if (!IS_IPv4) {
-		if (   priv->state < NM_DEVICE_STATE_DEACTIVATING
+		NMPlatform *platform;
+		GSList *dad6_failed_addrs, *iter;
+
+		dad6_failed_addrs = g_steal_pointer (&priv->dad6_failed_addrs);
+
+		if (   priv->state > NM_DEVICE_STATE_DISCONNECTED
+		    && priv->state < NM_DEVICE_STATE_DEACTIVATING
+		    && !nm_device_sys_iface_state_is_external (self)
 		    && (platform = nm_device_get_platform (self))
 		    && nm_platform_link_get (platform, priv->ifindex)) {
-			/* Handle DAD failures */
+			gboolean need_ipv6ll = FALSE;
-			while (priv->dad6_failed_addrs) {
+			NMNDiscConfigMap ndisc_config_changed = NM_NDISC_CONFIG_NONE;
-				nm_auto_nmpobj const NMPObject *obj = NULL;
-				const NMPlatformIP6Address *addr;
 
-				obj = priv->dad6_failed_addrs->data;
+			/* Handle DAD failures */
-				priv->dad6_failed_addrs = g_slist_delete_link (priv->dad6_failed_addrs, priv->dad6_failed_addrs);
+			for (iter = dad6_failed_addrs; iter; iter = iter->next) {
+				const NMPObject *obj = iter->data;
+				const NMPlatformIP6Address *addr;
 
 				if (!nm_ndisc_dad_addr_is_fail_candidate (platform, obj))
 					continue;
@@ -12338,9 +12324,12 @@ queued_ip_config_change (NMDevice *self, int addr_family)
 				if (IN6_IS_ADDR_LINKLOCAL (&addr->address))
 					need_ipv6ll = TRUE;
 				else if (priv->ndisc)
-					nm_ndisc_dad_failed (priv->ndisc, &addr->address);
+					ndisc_config_changed |= nm_ndisc_dad_failed (priv->ndisc, &addr->address, FALSE);
 			}
 
+			if (ndisc_config_changed != NM_NDISC_CONFIG_NONE)
+				nm_ndisc_emit_config_change (priv->ndisc, ndisc_config_changed);
+
 			/* If no IPv6 link-local address exists but other addresses do then we
 			 * must add the LL address to remain conformant with RFC 3513 chapter 2.1
 			 * ("Addressing Model"): "All interfaces are required to have at least
@@ -12349,14 +12338,14 @@ queued_ip_config_change (NMDevice *self, int addr_family)
 			if (   priv->ip_config_6
 			    && nm_ip6_config_get_num_addresses (priv->ip_config_6))
 				need_ipv6ll = TRUE;
-
 			if (need_ipv6ll)
 				check_and_add_ipv6ll_addr (self);
-		} else {
-			g_slist_free_full (priv->dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
-			priv->dad6_failed_addrs = NULL;
 		}
 
+		g_slist_free_full (dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
+	}
+
+	if (!IS_IPv4) {
 		/* Check if DAD is still pending */
 		if (   priv->ip6_state == IP_CONF
 		    && priv->dad6_ip6_config
@@ -12412,16 +12401,20 @@ device_ipx_changed (NMPlatform *platform,
 	if (nm_device_get_ip_ifindex (self) != ifindex)
 		return;
 
+	if (!nm_device_is_real (self))
+		return;
+
+	if (nm_device_get_unmanaged_flags (self, NM_UNMANAGED_PLATFORM_INIT)) {
+		/* ignore all platform signals until the link is initialized in platform. */
+		return;
+	}
+
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
 	switch (obj_type) {
 	case NMP_OBJECT_TYPE_IP4_ADDRESS:
 	case NMP_OBJECT_TYPE_IP4_ROUTE:
-		if (nm_device_get_unmanaged_flags (self, NM_UNMANAGED_PLATFORM_INIT)) {
+		if (!priv->queued_ip_config_id_4) {
-			priv->queued_ip4_config_pending = TRUE;
-			nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_4));
-		} else if (!priv->queued_ip_config_id_4) {
-			priv->queued_ip4_config_pending = FALSE;
 			priv->queued_ip_config_id_4 = g_idle_add (queued_ip4_config_change, self);
 			_LOGD (LOGD_DEVICE, "queued IP4 config change");
 		}
@@ -12435,13 +12428,10 @@ device_ipx_changed (NMPlatform *platform,
 			priv->dad6_failed_addrs = g_slist_prepend (priv->dad6_failed_addrs,
 			                                           (gpointer) nmp_object_ref (NMP_OBJECT_UP_CAST (addr)));
 		}
+
 		/* fall through */
 	case NMP_OBJECT_TYPE_IP6_ROUTE:
-		if (nm_device_get_unmanaged_flags (self, NM_UNMANAGED_PLATFORM_INIT)) {
+		if (!priv->queued_ip_config_id_6) {
-			priv->queued_ip6_config_pending = TRUE;
-			nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_6));
-		} else if (!priv->queued_ip_config_id_6) {
-			priv->queued_ip6_config_pending = FALSE;
 			priv->queued_ip_config_id_6 = g_idle_add (queued_ip6_config_change, self);
 			_LOGD (LOGD_DEVICE, "queued IP6 config change");
 		}
@@ -12708,17 +12698,11 @@ _set_unmanaged_flags (NMDevice *self,
 			                               !!unmanaged);
 		}
 
-		if (priv->queued_ip4_config_pending) {
+		/* trigger an initial update of IP configuration. */
-			priv->queued_ip4_config_pending = FALSE;
+		nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_4));
-			nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_4));
+		nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_6));
-			priv->queued_ip_config_id_4 = g_idle_add (queued_ip4_config_change, self);
+		priv->queued_ip_config_id_4 = g_idle_add (queued_ip4_config_change, self);
-		}
+		priv->queued_ip_config_id_6 = g_idle_add (queued_ip6_config_change, self);
-
-		if (priv->queued_ip6_config_pending) {
-			priv->queued_ip6_config_pending = FALSE;
-			nm_assert_se (!nm_clear_g_source (&priv->queued_ip_config_id_6));
-			priv->queued_ip_config_id_6 = g_idle_add (queued_ip6_config_change, self);
-		}
 
 		if (!priv->pending_actions) {
 			do_notify_has_pending_actions = TRUE;

src/ndisc/nm-ndisc.c

@@ -231,8 +231,8 @@ _data_complete (NMNDiscDataInternal *data)
 	return &data->public;
 }
 
-static void
+void
-_emit_config_change (NMNDisc *self, NMNDiscConfigMap changed)
+nm_ndisc_emit_config_change (NMNDisc *self, NMNDiscConfigMap changed)
 {
 	_config_changed_log (self, changed);
 	g_signal_emit (self, signals[CONFIG_RECEIVED], 0,
@@ -743,7 +743,7 @@ nm_ndisc_set_iid (NMNDisc *ndisc, const NMUtilsIPv6IfaceId iid)
 		if (rdata->addresses->len) {
 			_LOGD ("IPv6 interface identifier changed, flushing addresses");
 			g_array_remove_range (rdata->addresses, 0, rdata->addresses->len);
-			_emit_config_change (ndisc, NM_NDISC_CONFIG_ADDRESSES);
+			nm_ndisc_emit_config_change (ndisc, NM_NDISC_CONFIG_ADDRESSES);
 			solicit_routers (ndisc);
 		}
 		return TRUE;
@@ -796,8 +796,8 @@ nm_ndisc_start (NMNDisc *ndisc)
 	}
 }
 
-void
+NMNDiscConfigMap
-nm_ndisc_dad_failed (NMNDisc *ndisc, const struct in6_addr *address)
+nm_ndisc_dad_failed (NMNDisc *ndisc, const struct in6_addr *address, gboolean emit_changed_signal)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -819,8 +819,10 @@ nm_ndisc_dad_failed (NMNDisc *ndisc, const struct in6_addr *address)
 		i++;
 	}
 
-	if (changed)
+	if (emit_changed_signal && changed)
-		_emit_config_change (ndisc, NM_NDISC_CONFIG_ADDRESSES);
+		nm_ndisc_emit_config_change (ndisc, NM_NDISC_CONFIG_ADDRESSES);
+
+	return changed ? NM_NDISC_CONFIG_ADDRESSES : NM_NDISC_CONFIG_NONE;
 }
 
 #define CONFIG_MAP_MAX_STR 7
@@ -1131,7 +1133,7 @@ check_timestamps (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 	clean_dns_domains (ndisc, now, &changed, &nextevent);
 
 	if (changed)
-		_emit_config_change (ndisc, changed);
+		nm_ndisc_emit_config_change (ndisc, changed);
 
 	if (nextevent != G_MAXINT32) {
 		if (nextevent <= now)

src/ndisc/nm-ndisc.h

@@ -100,6 +100,7 @@ typedef struct {
 } NMNDiscDNSDomain;
 
 typedef enum {
+	NM_NDISC_CONFIG_NONE                                = 0,
 	NM_NDISC_CONFIG_DHCP_LEVEL                          = 1 << 0,
 	NM_NDISC_CONFIG_GATEWAYS                            = 1 << 1,
 	NM_NDISC_CONFIG_ADDRESSES                           = 1 << 2,
@@ -171,13 +172,17 @@ typedef struct {
 
 GType nm_ndisc_get_type (void);
 
+void nm_ndisc_emit_config_change (NMNDisc *self, NMNDiscConfigMap changed);
+
 int nm_ndisc_get_ifindex (NMNDisc *self);
 const char *nm_ndisc_get_ifname (NMNDisc *self);
 NMNDiscNodeType nm_ndisc_get_node_type (NMNDisc *self);
 
 gboolean nm_ndisc_set_iid (NMNDisc *ndisc, const NMUtilsIPv6IfaceId iid);
 void nm_ndisc_start (NMNDisc *ndisc);
-void nm_ndisc_dad_failed (NMNDisc *ndisc, const struct in6_addr *address);
+NMNDiscConfigMap nm_ndisc_dad_failed (NMNDisc *ndisc,
+                                      const struct in6_addr *address,
+                                      gboolean emit_changed_signal);
 void nm_ndisc_set_config (NMNDisc *ndisc,
                           const GArray *addresses,
                           const GArray *dns_servers,

src/nm-iface-helper.c

@@ -339,7 +339,8 @@ dad_failed_handle_idle (gpointer user_data)
 
 		if (nm_ndisc_dad_addr_is_fail_candidate (data->platform, obj)) {
 			nm_ndisc_dad_failed (data->ndisc,
-			                     &NMP_OBJECT_CAST_IP6_ADDRESS (obj)->address);
+			                     &NMP_OBJECT_CAST_IP6_ADDRESS (obj)->address,
+			                     TRUE);
 		}
 	}