platform: use new platform caching

Switch platform caching implementation. Instead of caching libnl
objects, cache our own types.

Don't remove yet the now obsolete functions.

Advantage:

* Performance
  - as we now cache our native NMPlatformObject instances, we no longer
    have to convert libnl objects every time we access the platform
    cache.
  - for most cases, access is now O(1) because we can lookup the object
    in a hash table. Note that ip4_address_get_all() still has to
    create a copy of the result (O(n)), but as the caller is about to
    use those elements, he cannot do better then O(n) anyway.

* We cache our own native types and have full control over them. We
  cannot extend the libnl objects, which has many short-commings:
  - _rtnl_addr_hack_lifetimes_rel_to_abs() to convert the timestamps
    to absolute values (and back).
  - hack_empty_master_iff_lower_up() would modify the internal flag,
    but it looses the original value. That means, we can only hack
    the state before putting a link into the cache, but we cannot revert
    that change, when a slave in the cache changes state.
    That was previously solved by always refetching the master when
    a slave changed. Now we can re-evaluate the connected state
    (DELAYED_ACTION_TYPE_MASTER_CONNECTED).
  - we implement functions like equality, to-string as most suitable
    for us. Before we needed hacks like nm_nl_object_diff(),
    nm_nl_cache_search(), route_search_cache().
  - we can extend our objects with exactly those properties we care,
    and possibly additional properties that are not representable in
    the libnl objects.
  - we no longer cache RTM_F_CLONED routes and they get rejected early
    on as we receive them.
  - In the future, maybe it'd be interesting the make platform objects
    immutable (and ref-counted) and expose them directly.

* Previous implementation did not order the refresh of objects but
  called check_cache_items(). Now, those actions are delayed and
  combined in an attempt to reduce the overall number of reloads.
  Realize how expensive a check_cache_items() for addresses and routes
  was: it would iterate all addresses/routes and call refresh_object().
  The latter obtains a full dump of *all* objects again, and ignores
  all but the needle.
  Note that we probably still schedule some delayed actions that
  are not needed.
  Later we can optimize that further (related bug bgo #747985).

While some of these points could also have been implemented with
caching of libnl objects, that would have become hard to maintain.

https://bugzilla.gnome.org/show_bug.cgi?id=747981

src/platform/nm-fake-platform.c

@@ -27,6 +27,7 @@
 #include <linux/rtnetlink.h>
 
 #include "gsystem-local-alloc.h"
+#include "nm-utils.h"
 #include "NetworkManagerUtils.h"
 #include "nm-fake-platform.h"
 #include "nm-logging.h"
@@ -312,19 +313,23 @@ link_changed (NMPlatform *platform, NMFakePlatformLink *device)
 	g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, device->link.ifindex, &device->link, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL);
 
 	if (device->link.master) {
+		gboolean connected = FALSE;
+
 		NMFakePlatformLink *master = link_get (platform, device->link.master);
 
-		g_return_if_fail (master != device);
+		g_return_if_fail (master && master != device);
 
-		master->link.connected = FALSE;
 		for (i = 0; i < priv->links->len; i++) {
 			NMFakePlatformLink *slave = &g_array_index (priv->links, NMFakePlatformLink, i);
 
 			if (slave && slave->link.master == master->link.ifindex && slave->link.connected)
-				master->link.connected = TRUE;
+				connected = TRUE;
 		}
 
-		link_changed (platform, master);
+		if (master->link.connected != connected) {
+			master->link.connected = connected;
+			link_changed (platform, master);
+		}
 	}
 }
 
@@ -332,27 +337,33 @@ static gboolean
 link_set_up (NMPlatform *platform, int ifindex)
 {
 	NMFakePlatformLink *device = link_get (platform, ifindex);
+	gboolean up, connected;
 
 	if (!device)
 		return FALSE;
 
-	device->link.up = TRUE;
+	up = TRUE;
+	connected = TRUE;
 	switch (device->link.type) {
 	case NM_LINK_TYPE_DUMMY:
 	case NM_LINK_TYPE_VLAN:
-		device->link.connected = TRUE;
 		break;
 	case NM_LINK_TYPE_BRIDGE:
 	case NM_LINK_TYPE_BOND:
 	case NM_LINK_TYPE_TEAM:
-		device->link.connected = FALSE;
+		connected = FALSE;
 		break;
 	default:
-		device->link.connected = FALSE;
+		connected = FALSE;
 		g_error ("Unexpected device type: %d", device->link.type);
 	}
 
-	link_changed (platform, device);
+	if (   device->link.up != up
+	    || device->link.connected != connected) {
+		device->link.up = up;
+		device->link.connected = connected;
+		link_changed (platform, device);
+	}
 
 	return TRUE;
 }
@@ -365,10 +376,12 @@ link_set_down (NMPlatform *platform, int ifindex)
 	if (!device)
 		return FALSE;
 
-	device->link.up = FALSE;
-	device->link.connected = FALSE;
+	if (device->link.up || device->link.connected) {
+		device->link.up = FALSE;
+		device->link.connected = FALSE;
 
-	link_changed (platform, device);
+		link_changed (platform, device);
+	}
 
 	return TRUE;
 }
@@ -566,12 +579,21 @@ static gboolean
 link_enslave (NMPlatform *platform, int master, int slave)
 {
 	NMFakePlatformLink *device = link_get (platform, slave);
+	NMFakePlatformLink *master_device = link_get (platform, master);
 
 	g_return_val_if_fail (device, FALSE);
+	g_return_val_if_fail (master_device, FALSE);
 
-	device->link.master = master;
+	if (device->link.master != master) {
+		device->link.master = master;
 
-	link_changed (platform, device);
+		if (NM_IN_SET (master_device->link.type, NM_LINK_TYPE_BOND, NM_LINK_TYPE_TEAM)) {
+			device->link.up = TRUE;
+			device->link.connected = TRUE;
+		}
+
+		link_changed (platform, device);
+	}
 
 	return TRUE;
 }
@@ -1223,8 +1245,8 @@ ip4_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source,
 				break;
 		}
 		if (i == priv->ip4_routes->len) {
-			nm_log_warn (LOGD_PLATFORM, "Fake platform: error adding %s: Network Unreachable",
-			             nm_platform_ip4_route_to_string (&route));
+			nm_log_warn (LOGD_PLATFORM, "Fake platform: failure adding ip4-route '%d: %s/%d %d': Network Unreachable",
+			             route.ifindex, nm_utils_inet4_ntop (route.network, NULL), route.plen, route.metric);
 			return FALSE;
 		}
 	}
@@ -1290,8 +1312,8 @@ ip6_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source,
 				break;
 		}
 		if (i == priv->ip6_routes->len) {
-			nm_log_warn (LOGD_PLATFORM, "Fake platform: error adding %s: Network Unreachable",
-			             nm_platform_ip6_route_to_string (&route));
+			nm_log_warn (LOGD_PLATFORM, "Fake platform: failure adding ip6-route '%d: %s/%d %d': Network Unreachable",
+			             route.ifindex, nm_utils_inet6_ntop (&route.network, NULL), route.plen, route.metric);
 			return FALSE;
 		}
 	}

src/platform/tests/test-address.c

@@ -75,7 +75,7 @@ test_ip4_address (void)
 	/* Add address again (aka update) */
 	g_assert (nm_platform_ip4_address_add (NM_PLATFORM_GET, ifindex, addr, 0, IP4_PLEN, lifetime, preferred, NULL));
 	no_error ();
-	accept_signal (address_changed);
+	accept_signals (address_changed, 0, 1);
 
 	/* Test address listing */
 	addresses = nm_platform_ip4_address_get_all (NM_PLATFORM_GET, ifindex);
@@ -131,7 +131,7 @@ test_ip6_address (void)
 	/* Add address again (aka update) */
 	g_assert (nm_platform_ip6_address_add (NM_PLATFORM_GET, ifindex, addr, in6addr_any, IP6_PLEN, lifetime, preferred, flags));
 	no_error ();
-	accept_signal (address_changed);
+	accept_signals (address_changed, 0, 1);
 
 	/* Test address listing */
 	addresses = nm_platform_ip6_address_get_all (NM_PLATFORM_GET, ifindex);

src/platform/tests/test-link.c

@@ -120,10 +120,15 @@ software_add (NMLinkType link_type, const char *name)
 
 		{
 			int parent_ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, PARENT_NAME);
+			gboolean was_up = nm_platform_link_is_up (NM_PLATFORM_GET, parent_ifindex);
 
 			parent_changed = add_signal_ifindex (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_CHANGED, link_callback, parent_ifindex);
 			g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, parent_ifindex));
-			accept_signal (parent_changed);
+			if (was_up) {
+				/* when NM is running in the background, it will mess with addrgenmode which might cause additional signals. */
+				accept_signals (parent_changed, 0, 1);
+			} else
+				accept_signal (parent_changed);
 			free_signal (parent_changed);
 
 			return nm_platform_vlan_add (NM_PLATFORM_GET, name, parent_ifindex, VLAN_ID, 0, NULL);
@@ -142,6 +147,9 @@ test_slave (int master, int type, SignalData *master_changed)
 	SignalData *link_added = add_signal_ifname (NM_PLATFORM_SIGNAL_LINK_CHANGED, NM_PLATFORM_SIGNAL_ADDED, link_callback, SLAVE_NAME);
 	SignalData *link_changed, *link_removed;
 	char *value;
+	NMLinkType link_type = nm_platform_link_get_type (NM_PLATFORM_GET, master);
+
+	g_assert (NM_IN_SET (link_type, NM_LINK_TYPE_TEAM, NM_LINK_TYPE_BOND, NM_LINK_TYPE_BRIDGE));
 
 	g_assert (software_add (type, SLAVE_NAME));
 	ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, SLAVE_NAME);
@@ -154,19 +162,28 @@ test_slave (int master, int type, SignalData *master_changed)
 	 *
 	 * See https://bugzilla.redhat.com/show_bug.cgi?id=910348
 	 */
+	g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
 	g_assert (nm_platform_link_set_down (NM_PLATFORM_GET, ifindex));
 	g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
-	accept_signal (link_changed);
+	ensure_no_signal (link_changed);
 
 	/* Enslave */
 	link_changed->ifindex = ifindex;
 	g_assert (nm_platform_link_enslave (NM_PLATFORM_GET, master, ifindex)); no_error ();
 	g_assert_cmpint (nm_platform_link_get_master (NM_PLATFORM_GET, ifindex), ==, master); no_error ();
+
 	accept_signal (link_changed);
-	accept_signal (master_changed);
+	accept_signals (master_changed, 0, 1);
+
+	/* enslaveing brings put the slave */
+	if (NM_IN_SET (link_type, NM_LINK_TYPE_BOND, NM_LINK_TYPE_TEAM))
+		g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
+	else
+		g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
 
 	/* Set master up */
 	g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, master));
+	g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, master));
 	accept_signal (master_changed);
 
 	/* Master with a disconnected slave is disconnected
@@ -179,7 +196,7 @@ test_slave (int master, int type, SignalData *master_changed)
 	case NM_LINK_TYPE_TEAM:
 		g_assert (nm_platform_link_set_down (NM_PLATFORM_GET, ifindex));
 		accept_signal (link_changed);
-		accept_signal (master_changed);
+		accept_signals (master_changed, 0, 2);
 		break;
 	default:
 		break;
@@ -206,15 +223,16 @@ test_slave (int master, int type, SignalData *master_changed)
 	g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex));
 	g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, master));
 	accept_signal (link_changed);
-	accept_signal (master_changed);
+	/* NM running, can cause additional change of addrgenmode */
+	accept_signals (master_changed, 1, 2);
 
 	/* Enslave again
 	 *
 	 * Gracefully succeed if already enslaved.
 	 */
 	g_assert (nm_platform_link_enslave (NM_PLATFORM_GET, master, ifindex)); no_error ();
-	accept_signal (link_changed);
-	accept_signal (master_changed);
+	ensure_no_signal (link_changed);
+	ensure_no_signal (master_changed);
 
 	/* Set slave option */
 	switch (type) {
@@ -236,7 +254,10 @@ test_slave (int master, int type, SignalData *master_changed)
 	g_assert (nm_platform_link_release (NM_PLATFORM_GET, master, ifindex));
 	g_assert_cmpint (nm_platform_link_get_master (NM_PLATFORM_GET, ifindex), ==, 0); no_error ();
 	accept_signal (link_changed);
-	accept_signal (master_changed);
+	if (link_type != NM_LINK_TYPE_TEAM)
+		accept_signals (master_changed, 1, 2);
+	else
+		accept_signals (master_changed, 1, 1);
 
 	/* Release again */
 	g_assert (!nm_platform_link_release (NM_PLATFORM_GET, master, ifindex));
@@ -539,6 +560,7 @@ test_external (void)
 
 	run_command ("ip link del %s", DEVICE_NAME);
 	wait_signal (link_removed);
+	accept_signals (link_changed, 0, 1);
 	g_assert (!nm_platform_link_exists (NM_PLATFORM_GET, DEVICE_NAME));
 
 	free_signal (link_added);

src/platform/tests/test-route.c

@@ -153,7 +153,7 @@ test_ip4_route (void)
 	/* Add route again */
 	g_assert (nm_platform_ip4_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, network, plen, gateway, 0, metric, mss));
 	no_error ();
-	accept_signal (route_changed);
+	accept_signals (route_changed, 0, 1);
 
 	/* Add default route */
 	assert_ip4_route_exists (FALSE, DEVICE_NAME, 0, 0, metric);
@@ -167,7 +167,7 @@ test_ip4_route (void)
 	/* Add default route again */
 	g_assert (nm_platform_ip4_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, 0, 0, gateway, 0, metric, mss));
 	no_error ();
-	accept_signal (route_changed);
+	accept_signals (route_changed, 0, 1);
 
 	/* Test route listing */
 	routes = nm_platform_ip4_route_get_all (NM_PLATFORM_GET, ifindex, NM_PLATFORM_GET_ROUTE_MODE_ALL);
@@ -251,7 +251,7 @@ test_ip6_route (void)
 	/* Add route again */
 	g_assert (nm_platform_ip6_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, network, plen, gateway, metric, mss));
 	no_error ();
-	accept_signal (route_changed);
+	accept_signals (route_changed, 0, 1);
 
 	/* Add default route */
 	g_assert (!nm_platform_ip6_route_exists (NM_PLATFORM_GET, ifindex, in6addr_any, 0, metric));
@@ -265,7 +265,7 @@ test_ip6_route (void)
 	/* Add default route again */
 	g_assert (nm_platform_ip6_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, in6addr_any, 0, gateway, metric, mss));
 	no_error ();
-	accept_signal (route_changed);
+	accept_signals (route_changed, 0, 1);
 
 	/* Test route listing */
 	routes = nm_platform_ip6_route_get_all (NM_PLATFORM_GET, ifindex, NM_PLATFORM_GET_ROUTE_MODE_ALL);

src/tests/test-route-manager.c

@@ -250,7 +250,7 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 	};
 
 	setup_dev0_ip4 (fixture->ifindex0, 1000, 21021);
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*error adding 8.0.0.0/8 via 6.6.6.2 dev *");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip4-route '*: 8.0.0.0/8 22': *");
 	setup_dev1_ip4 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
@@ -262,7 +262,7 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 	nmtst_platform_ip4_routes_equal ((NMPlatformIP4Route *) routes->data, state1, routes->len, TRUE);
 	g_array_free (routes, TRUE);
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*error adding 8.0.0.0/8 via 6.6.6.2 dev *");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip4-route '*: 8.0.0.0/8 22': *");
 	setup_dev1_ip4 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
@@ -584,7 +584,7 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 	};
 
 	setup_dev0_ip6 (fixture->ifindex0);
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*error adding 2001:db8:d34d::/64 via 2001:db8:8086::2 dev *");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip6-route '*: 2001:db8:d34d::/64 20': *");
 	setup_dev1_ip6 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
@@ -598,7 +598,7 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 	g_array_free (routes, TRUE);
 
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*error adding 2001:db8:d34d::/64 via 2001:db8:8086::2 dev *");
+	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip6-route '*: 2001:db8:d34d::/64 20': *");
 	setup_dev1_ip6 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 	setup_dev0_ip6 (fixture->ifindex0);