platform: fix udev/kernel interface race conditions

A network interface is only exposed if it's recognized by both netlink
cache and udev.

src/platform/nm-linux-platform.c

@@ -754,13 +754,6 @@ announce_object (NMPlatform *platform, const struct nl_object *object, ObjectSta
 	ObjectType object_type = object_type_from_nl_object (object);
 	const char *sig = signal_by_type_and_status[object_type][status];
 
-	if (object_type == LINK && status == ADDED) {
-		/* We have to wait until udev has registered the device; we'll
-		 * emit NM_PLATFORM_LINK_ADDED from udev_device_added().
-		 */
-		return;
-	}
-
 	switch (object_type) {
 	case LINK:
 		{
@@ -768,6 +761,22 @@ announce_object (NMPlatform *platform, const struct nl_object *object, ObjectSta
 
 			link_init (platform, &device, (struct rtnl_link *) object);
 
+			/* Skip devices not yet discovered by udev. They will be announced
+			 * by udev_device_added(). This doesn't apply to removed devices, as
+			 * those come either from udev_device_removed(), event_notification()
+			 * or link_delete() which block the announcment themselves when
+			 * appropriate.
+			 */
+			switch (status) {
+			case ADDED:
+			case CHANGED:
+				if (!device.driver)
+					return;
+				break;
+			default:
+				break;
+			}
+
 			/* In some cases, the link action is followed by address and/or
 			 * route action. Kernel silently removes routes when interface
 			 * goes !IFF_UP and we also need to handle addresses and routes
@@ -922,7 +931,6 @@ delete_object (NMPlatform *platform, struct nl_object *obj)
 	}
 
 	nl_cache_remove (cached_object);
-
 	announce_object (platform, cached_object, REMOVED);
 
 	return TRUE;
@@ -984,6 +992,16 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 			return NL_OK;
 
 		nl_cache_remove (cached_object);
+		/* Don't announced removed interfaces that are not recognized by
+		 * udev. They were either not yet discovered or they have been
+		 * already removed and announced.
+		 */
+		if (event == RTM_DELLINK) {
+			int ifindex = rtnl_link_get_ifindex ((struct rtnl_link *) object);
+
+			if (!g_hash_table_lookup (priv->udev_devices, GINT_TO_POINTER (ifindex)))
+				return NL_OK;
+		}
 		announce_object (platform, cached_object, REMOVED);
 
 		return NL_OK;
@@ -1164,8 +1182,10 @@ link_get (NMPlatform *platform, int ifindex)
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	struct rtnl_link *rtnllink = rtnl_link_get (priv->link_cache, ifindex);
 
-	if (!rtnllink)
+	if (!rtnllink || !g_hash_table_lookup (priv->udev_devices, GINT_TO_POINTER (ifindex))) {
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
+		return NULL;
+	}
 
 	return rtnllink;
 }
@@ -1198,6 +1218,13 @@ link_change (NMPlatform *platform, int ifindex, struct rtnl_link *change)
 static gboolean
 link_delete (NMPlatform *platform, int ifindex)
 {
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+
+	if (!g_hash_table_lookup (priv->udev_devices, GINT_TO_POINTER (ifindex))) {
+		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
+		return FALSE;
+	}
+
 	return delete_object (platform, build_rtnl_link (ifindex, NULL, NM_LINK_TYPE_NONE));
 }
 
@@ -2252,7 +2279,6 @@ udev_device_added (NMPlatform *platform,
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	auto_nl_object struct rtnl_link *rtnllink = NULL;
 	const char *ifname, *devtype;
-	NMPlatformLink link;
 	int ifindex;
 
 	ifname = g_udev_device_get_name (udev_device);
@@ -2286,17 +2312,17 @@ udev_device_added (NMPlatform *platform,
 		return;
 	}
 
-	rtnllink = link_get (platform, ifindex);
+	g_hash_table_insert (priv->udev_devices, GINT_TO_POINTER (ifindex),
+	                     g_object_ref (udev_device));
+
+	/* Don't announce devices that have not yet been discovered via Netlink. */
+	rtnllink = rtnl_link_get (priv->link_cache, ifindex);
 	if (!rtnllink) {
 		debug ("%s: not found in link cache, ignoring...", ifname);
 		return;
 	}
 
-	g_hash_table_insert (priv->udev_devices, GINT_TO_POINTER (ifindex),
+	announce_object (platform, (struct nl_object *) rtnllink, ADDED);
-	                     g_object_ref (udev_device));
-
-	link_init (platform, &link, rtnllink);
-	g_signal_emit_by_name (platform, NM_PLATFORM_LINK_ADDED, ifindex, &link);
 }
 
 static void
@@ -2304,7 +2330,7 @@ udev_device_removed (NMPlatform *platform,
                      GUdevDevice *udev_device)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	int ifindex;
+	int ifindex = 0;
 
 	if (g_udev_device_get_sysfs_attr (udev_device, "ifindex")) {
 		ifindex = g_udev_device_get_sysfs_attr_as_int (udev_device, "ifindex");
@@ -2324,6 +2350,14 @@ udev_device_removed (NMPlatform *platform,
 			}
 		}
 	}
+
+    /* Announce device removal if it's still in the Netlink cache. */
+	if (ifindex) {
+		struct rtnl_link *device = rtnl_link_get (priv->link_cache, ifindex);
+
+		if (device)
+			announce_object (platform, (struct nl_object *) device, REMOVED);
+	}
 }
 
 static void

src/platform/tests/test-address.c

@@ -221,9 +221,13 @@ test_ip6_address_external (void)
 void
 setup_tests (void)
 {
+	SignalData *link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME);
+
 	nm_platform_link_delete (nm_platform_link_get_ifindex (DEVICE_NAME));
 	g_assert (!nm_platform_link_exists (DEVICE_NAME));
-	nm_platform_dummy_add (DEVICE_NAME);
+	g_assert (nm_platform_dummy_add (DEVICE_NAME));
+	wait_signal (link_added);
+	free_signal (link_added);
 
 	g_test_add_func ("/address/internal/ip4", test_ip4_address);
 	g_test_add_func ("/address/internal/ip6", test_ip6_address);

src/platform/tests/test-cleanup.c

@@ -5,6 +5,7 @@
 static void
 test_cleanup_internal ()
 {
+	SignalData *link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME);
 	int ifindex;
 	GArray *addresses4;
 	GArray *addresses6;
@@ -28,11 +29,13 @@ test_cleanup_internal ()
 	inet_pton (AF_INET6, "2001:db8:c:d:0:0:0:0", &network6);
 	inet_pton (AF_INET6, "2001:db8:e:f:1:2:3:4", &gateway6);
 
-	/* Create device */
+	/* Create and set up device */
 	g_assert (nm_platform_dummy_add (DEVICE_NAME));
+	wait_signal (link_added);
+	free_signal (link_added);
+	g_assert (nm_platform_link_set_up (nm_platform_link_get_ifindex (DEVICE_NAME)));
 	ifindex = nm_platform_link_get_ifindex (DEVICE_NAME);
 	g_assert (ifindex > 0);
-	g_assert (nm_platform_link_set_up (ifindex));
 
 	/* Add routes and addresses */
 	g_assert (nm_platform_ip4_address_add (ifindex, addr4, plen4));

src/platform/tests/test-common.c

@@ -46,6 +46,53 @@ free_signal (SignalData *data)
 	g_free (data);
 }
 
+void
+link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, SignalData *data)
+{
+	
+	GArray *links;
+	NMPlatformLink *cached;
+	int i;
+
+	g_assert (received);
+	g_assert_cmpint (received->ifindex, ==, ifindex);
+
+	if (data->ifindex && data->ifindex != received->ifindex)
+		return;
+	if (data->ifname && g_strcmp0 (data->ifname, nm_platform_link_get_name (ifindex)) != 0)
+		return;
+
+	if (data->loop) {
+		debug ("Quitting main loop.");
+		g_main_loop_quit (data->loop);
+	}
+
+	if (data->received)
+		g_error ("Received signal '%s' a second time.", data->name);
+
+	debug ("Recieved signal '%s' ifindex %d ifname '%s'.", data->name, ifindex, received->name);
+	data->received = TRUE;
+
+	/* Check the data */
+	g_assert (received->ifindex > 0);
+	links = nm_platform_link_get_all ();
+	for (i = 0; i < links->len; i++) {
+		cached = &g_array_index (links, NMPlatformLink, i);
+		if (cached->ifindex == received->ifindex) {
+			g_assert (!memcmp (cached, received, sizeof (*cached)));
+			if (!g_strcmp0 (data->name, NM_PLATFORM_LINK_REMOVED)) {
+				g_error ("Deleted link still found in the local cache.");
+			}
+			g_array_unref (links);
+			return;
+		}
+	}
+	g_array_unref (links);
+
+	if (g_strcmp0 (data->name, NM_PLATFORM_LINK_REMOVED))
+		g_error ("Added/changed link not found in the local cache.");
+}
+
 void
 run_command (const char *format, ...)
 {

src/platform/tests/test-common.h

@@ -31,6 +31,8 @@ void accept_signal (SignalData *data);
 void wait_signal (SignalData *data);
 void free_signal (SignalData *data);
 
+void link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, SignalData *data);
+
 void run_command (const char *format, ...);
 
 void setup_tests (void);

src/platform/tests/test-link.c

@@ -14,53 +14,6 @@
 #define VLAN_FLAGS 0
 #define MTU 1357
 
-static void
-link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, SignalData *data)
-{
-	
-	GArray *links;
-	NMPlatformLink *cached;
-	int i;
-
-	g_assert (received);
-	g_assert_cmpint (received->ifindex, ==, ifindex);
-
-	if (data->ifindex && data->ifindex != received->ifindex)
-		return;
-	if (data->ifname && g_strcmp0 (data->ifname, nm_platform_link_get_name (ifindex)) != 0)
-		return;
-
-	if (data->loop) {
-		debug ("Quitting main loop.");
-		g_main_loop_quit (data->loop);
-	}
-
-	if (data->received)
-		g_error ("Received signal '%s' a second time.", data->name);
-
-	debug ("Recieved signal '%s' ifindex %d ifname '%s'.", data->name, ifindex, received->name);
-	data->received = TRUE;
-
-	/* Check the data */
-	g_assert (received->ifindex > 0);
-	links = nm_platform_link_get_all ();
-	for (i = 0; i < links->len; i++) {
-		cached = &g_array_index (links, NMPlatformLink, i);
-		if (cached->ifindex == received->ifindex) {
-			g_assert (!memcmp (cached, received, sizeof (*cached)));
-			if (!g_strcmp0 (data->name, NM_PLATFORM_LINK_REMOVED)) {
-				g_error ("Deleted link still found in the local cache.");
-			}
-			g_array_unref (links);
-			return;
-		}
-	}
-	g_array_unref (links);
-
-	if (g_strcmp0 (data->name, NM_PLATFORM_LINK_REMOVED))
-		g_error ("Added/changed link not found in the local cache.");
-}
-
 static void
 test_bogus(void)
 {
@@ -294,6 +247,7 @@ test_software (NMLinkType link_type, const char *link_typename)
 	link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME);
 	g_assert (software_add (link_type, DEVICE_NAME));
 	no_error ();
+	wait_signal (link_added);
 	g_assert (nm_platform_link_exists (DEVICE_NAME));
 	ifindex = nm_platform_link_get_ifindex (DEVICE_NAME);
 	g_assert (ifindex >= 0);
@@ -307,7 +261,6 @@ test_software (NMLinkType link_type, const char *link_typename)
 		g_assert_cmpint (vlan_id, ==, VLAN_ID);
 		no_error ();
 	}
-	wait_signal (link_added);
 
 	/* Add again */
 	g_assert (!software_add (link_type, DEVICE_NAME));

src/platform/tests/test-route.c

@@ -199,9 +199,14 @@ test_ip6_route ()
 void
 setup_tests (void)
 {
+	SignalData *link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME);
+
 	nm_platform_link_delete (nm_platform_link_get_ifindex (DEVICE_NAME));
 	g_assert (!nm_platform_link_exists (DEVICE_NAME));
 	g_assert (nm_platform_dummy_add (DEVICE_NAME));
+	wait_signal (link_added);
+	free_signal (link_added);
+
 	g_assert (nm_platform_link_set_up (nm_platform_link_get_ifindex (DEVICE_NAME)));
 
 	g_test_add_func ("/route/ip4", test_ip4_route);