From e02b1a86208df7c7f2ae1d79e65597f02d8f6d27 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Feb 2014 13:08:23 +0100
Subject: [PATCH 01/10] platform: refactor link_get() not to use auto_nl_object

The previous implementation called nl_object_get() and nl_object_put()
each time in link_get(). As nl_object_get() and nl_object_put()
causes debug logging in libnl, this clutters the output.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 81630fe60b..8177f230f0 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -1487,7 +1487,7 @@ static struct rtnl_link *
 link_get (NMPlatform *platform, int ifindex)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	auto_nl_object struct rtnl_link *rtnllink = rtnl_link_get (priv->link_cache, ifindex);
+	struct rtnl_link *rtnllink = rtnl_link_get (priv->link_cache, ifindex);
 
 	if (!rtnllink) {
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
@@ -1497,10 +1497,10 @@ link_get (NMPlatform *platform, int ifindex)
 	/* physical interfaces must be found by udev before they can be used */
 	if (!link_is_announceable (platform, rtnllink)) {
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
+		rtnl_link_put (rtnllink);
 		return NULL;
 	}
 
-	nl_object_get ((struct nl_object *) rtnllink);
 	return rtnllink;
 }
 

From e54a3ccaf8a5a95c6bf9ca2da1a0ee98db0b6541 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 14:42:26 +0100
Subject: [PATCH 02/10] platform: add debug logging when adding/deleting routes

Also, change the logging of nm_platform_ip._address_delete()
to log what we are about to do, *before* checking for existing
addresses.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-platform.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index bc10d7c49b..8bdfaf42f1 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -1281,13 +1281,14 @@ nm_platform_ip4_address_delete (int ifindex, in_addr_t address, int plen)
 	g_return_val_if_fail (plen > 0, FALSE);
 	g_return_val_if_fail (klass->ip4_address_delete, FALSE);
 
+	debug ("address: deleting IPv4 address %s/%d", nm_utils_inet4_ntop (address, NULL), plen);
+
 	if (!nm_platform_ip4_address_exists (ifindex, address, plen)) {
 		debug ("address doesn't exists");
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
 		return FALSE;
 	}
 
-	debug ("address: deleting IPv4 address %s/%d", nm_utils_inet4_ntop (address, NULL), plen);
 	return klass->ip4_address_delete (platform, ifindex, address, plen);
 }
 
@@ -1300,13 +1301,14 @@ nm_platform_ip6_address_delete (int ifindex, struct in6_addr address, int plen)
 	g_return_val_if_fail (plen > 0, FALSE);
 	g_return_val_if_fail (klass->ip6_address_delete, FALSE);
 
+	debug ("address: deleting IPv6 address %s/%d", nm_utils_inet6_ntop (&address, NULL), plen);
+
 	if (!nm_platform_ip6_address_exists (ifindex, address, plen)) {
 		debug ("address doesn't exists");
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
 		return FALSE;
 	}
 
-	debug ("address: deleting IPv6 address %s/%d", nm_utils_inet6_ntop (&address, NULL), plen);
 	return klass->ip6_address_delete (platform, ifindex, address, plen);
 }
 
@@ -1530,6 +1532,18 @@ nm_platform_ip4_route_add (int ifindex,
 	g_return_val_if_fail (mss >= 0, FALSE);
 	g_return_val_if_fail (klass->ip4_route_add, FALSE);
 
+	if (nm_logging_enabled (LOGL_DEBUG, LOGD_PLATFORM)) {
+		NMPlatformIP4Route route = { 0 };
+
+		route.ifindex = ifindex;
+		route.network = network;
+		route.plen = plen;
+		route.gateway = gateway;
+		route.metric = metric;
+		route.mss = mss;
+
+		debug ("route: adding or updating IPv4 route: %s", nm_platform_ip4_route_to_string (&route));
+	}
 	return klass->ip4_route_add (platform, ifindex, network, plen, gateway, metric, mss);
 }
 
@@ -1543,6 +1557,18 @@ nm_platform_ip6_route_add (int ifindex,
 	g_return_val_if_fail (mss >= 0, FALSE);
 	g_return_val_if_fail (klass->ip6_route_add, FALSE);
 
+	if (nm_logging_enabled (LOGL_DEBUG, LOGD_PLATFORM)) {
+		NMPlatformIP6Route route = { 0 };
+
+		route.ifindex = ifindex;
+		route.network = network;
+		route.plen = plen;
+		route.gateway = gateway;
+		route.metric = metric;
+		route.mss = mss;
+
+		debug ("route: adding or updating IPv6 route: %s", nm_platform_ip6_route_to_string (&route));
+	}
 	return klass->ip6_route_add (platform, ifindex, network, plen, gateway, metric, mss);
 }
 
@@ -1554,6 +1580,8 @@ nm_platform_ip4_route_delete (int ifindex, in_addr_t network, int plen, int metr
 	g_return_val_if_fail (platform, FALSE);
 	g_return_val_if_fail (klass->ip4_route_delete, FALSE);
 
+	debug ("route: deleting IPv4 route %s/%d, metric=%d", nm_utils_inet4_ntop (network, NULL), plen, metric);
+
 	if (!nm_platform_ip4_route_exists (ifindex, network, plen, metric)) {
 		debug ("route not found");
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
@@ -1572,6 +1600,8 @@ nm_platform_ip6_route_delete (int ifindex,
 	g_return_val_if_fail (platform, FALSE);
 	g_return_val_if_fail (klass->ip6_route_delete, FALSE);
 
+	debug ("route: deleting IPv6 route %s/%d, metric=%d", nm_utils_inet6_ntop (&network, NULL), plen, metric);
+
 	if (!nm_platform_ip6_route_exists (ifindex, network, plen, metric)) {
 		debug ("route not found");
 		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;

From dc54b2e3b2565b9af6478f98e004eaa478809e51 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Feb 2014 22:10:05 +0100
Subject: [PATCH 03/10] platform: cleanup object_type_from_nl_object()

- change object_type_from_nl_object() to accept unknown object
  types.
- replace g_assert_not_reached() with g_return_if_fail().

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 49 ++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 8177f230f0..bf7b859b11 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -141,12 +141,13 @@ nm_rtnl_addr_set_prefixlen (struct rtnl_addr *rtnladdr, int plen)
 #define rtnl_addr_set_prefixlen nm_rtnl_addr_set_prefixlen
 
 typedef enum {
+	UNKNOWN_OBJECT_TYPE,
 	LINK,
 	IP4_ADDRESS,
 	IP6_ADDRESS,
 	IP4_ROUTE,
 	IP6_ROUTE,
-	N_TYPES
+	N_TYPES,
 } ObjectType;
 
 typedef enum {
@@ -159,9 +160,10 @@ typedef enum {
 static ObjectType
 object_type_from_nl_object (const struct nl_object *object)
 {
-	const char *type_str = nl_object_get_type (object);
+	const char *type_str;
 
-	g_assert (object);
+	if (!object || !(type_str = nl_object_get_type (object)))
+		return UNKNOWN_OBJECT_TYPE;
 
 	if (!strcmp (type_str, "route/link"))
 		return LINK;
@@ -172,7 +174,7 @@ object_type_from_nl_object (const struct nl_object *object)
 		case AF_INET6:
 			return IP6_ADDRESS;
 		default:
-			g_assert_not_reached ();
+			return UNKNOWN_OBJECT_TYPE;
 		}
 	} else if (!strcmp (type_str, "route/route")) {
 		switch (rtnl_route_get_family ((struct rtnl_route *) object)) {
@@ -181,10 +183,10 @@ object_type_from_nl_object (const struct nl_object *object)
 		case AF_INET6:
 			return IP6_ROUTE;
 		default:
-			g_assert_not_reached ();
+			return UNKNOWN_OBJECT_TYPE;
 		}
 	} else
-		g_assert_not_reached ();
+		return UNKNOWN_OBJECT_TYPE;
 }
 
 /* libnl inclues LINK_ATTR_FAMILY in oo_id_attrs of link_obj_ops and thus
@@ -231,7 +233,10 @@ get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
 				return NULL;
 			}
 		}
-	default:
+	case IP4_ADDRESS:
+	case IP6_ADDRESS:
+	case IP4_ROUTE:
+	case IP6_ROUTE:
 		/* Fallback to a one-time cache allocation. */
 		{
 			struct nl_cache *cache;
@@ -247,6 +252,9 @@ get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
 			nl_cache_free (cache);
 			return object;
 		}
+	default:
+		g_return_val_if_reached (NULL);
+		return NULL;
 	}
 }
 
@@ -264,7 +272,8 @@ add_kernel_object (struct nl_sock *sock, struct nl_object *object)
 	case IP6_ROUTE:
 		return rtnl_route_add (sock, (struct rtnl_route *) object, NLM_F_CREATE | NLM_F_REPLACE);
 	default:
-		g_assert_not_reached ();
+		g_return_val_if_reached (-NLE_INVAL);
+		return -NLE_INVAL;
 	}
 }
 
@@ -282,7 +291,8 @@ delete_kernel_object (struct nl_sock *sock, struct nl_object *object)
 	case IP6_ROUTE:
 		return rtnl_route_delete (sock, (struct rtnl_route *) object, 0);
 	default:
-		g_assert_not_reached ();
+		g_return_val_if_reached (-NLE_INVAL);
+		return -NLE_INVAL;
 	}
 }
 
@@ -931,11 +941,11 @@ init_ip6_route (NMPlatformIP6Route *route, struct rtnl_route *rtnlroute)
 /* Object and cache manipulation */
 
 static const char *signal_by_type_and_status[N_TYPES][N_STATUSES] = {
-	{ NM_PLATFORM_LINK_ADDED, NM_PLATFORM_LINK_CHANGED, NM_PLATFORM_LINK_REMOVED },
-	{ NM_PLATFORM_IP4_ADDRESS_ADDED, NM_PLATFORM_IP4_ADDRESS_CHANGED, NM_PLATFORM_IP4_ADDRESS_REMOVED },
-	{ NM_PLATFORM_IP6_ADDRESS_ADDED, NM_PLATFORM_IP6_ADDRESS_CHANGED, NM_PLATFORM_IP6_ADDRESS_REMOVED },
-	{ NM_PLATFORM_IP4_ROUTE_ADDED, NM_PLATFORM_IP4_ROUTE_CHANGED, NM_PLATFORM_IP4_ROUTE_REMOVED },
-	{ NM_PLATFORM_IP6_ROUTE_ADDED, NM_PLATFORM_IP6_ROUTE_CHANGED, NM_PLATFORM_IP6_ROUTE_REMOVED }
+	[LINK]        = { NM_PLATFORM_LINK_ADDED,        NM_PLATFORM_LINK_CHANGED,        NM_PLATFORM_LINK_REMOVED },
+	[IP4_ADDRESS] = { NM_PLATFORM_IP4_ADDRESS_ADDED, NM_PLATFORM_IP4_ADDRESS_CHANGED, NM_PLATFORM_IP4_ADDRESS_REMOVED },
+	[IP6_ADDRESS] = { NM_PLATFORM_IP6_ADDRESS_ADDED, NM_PLATFORM_IP6_ADDRESS_CHANGED, NM_PLATFORM_IP6_ADDRESS_REMOVED },
+	[IP4_ROUTE]   = { NM_PLATFORM_IP4_ROUTE_ADDED,   NM_PLATFORM_IP4_ROUTE_CHANGED,   NM_PLATFORM_IP4_ROUTE_REMOVED },
+	[IP6_ROUTE]   = { NM_PLATFORM_IP6_ROUTE_ADDED,   NM_PLATFORM_IP6_ROUTE_CHANGED,   NM_PLATFORM_IP6_ROUTE_REMOVED }
 };
 
 static struct nl_cache *
@@ -953,7 +963,8 @@ choose_cache (NMPlatform *platform, struct nl_object *object)
 	case IP6_ROUTE:
 		return priv->route_cache;
 	default:
-		g_assert_not_reached ();
+		g_return_val_if_reached (NULL);
+		return NULL;
 	}
 }
 
@@ -1095,7 +1106,7 @@ announce_object (NMPlatform *platform, const struct nl_object *object, ObjectSta
 		}
 		return;
 	default:
-		error ("Announcing object: object type unknown: %d", object_type);
+		g_return_if_reached ();
 	}
 }
 
@@ -1951,7 +1962,8 @@ master_category (NMPlatform *platform, int master)
 	case NM_LINK_TYPE_BOND:
 		return "bonding";
 	default:
-		g_assert_not_reached ();
+		g_return_val_if_reached (NULL);
+		return NULL;
 	}
 }
 
@@ -1969,7 +1981,8 @@ slave_category (NMPlatform *platform, int slave)
 	case NM_LINK_TYPE_BRIDGE:
 		return "brport";
 	default:
-		g_assert_not_reached ();
+		g_return_val_if_reached (NULL);
+		return NULL;
 	}
 }
 

From a5f3fcae295b2bc9e39cf37b95c7e87a45eec76a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 16:08:36 +0100
Subject: [PATCH 04/10] platform: add function choose_cache_by_type()

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index bf7b859b11..81cd41458d 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -949,11 +949,11 @@ static const char *signal_by_type_and_status[N_TYPES][N_STATUSES] = {
 };
 
 static struct nl_cache *
-choose_cache (NMPlatform *platform, struct nl_object *object)
+choose_cache_by_type (NMPlatform *platform, ObjectType object_type)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 
-	switch (object_type_from_nl_object (object)) {
+	switch (object_type) {
 	case LINK:
 		return priv->link_cache;
 	case IP4_ADDRESS:
@@ -968,6 +968,12 @@ choose_cache (NMPlatform *platform, struct nl_object *object)
 	}
 }
 
+static struct nl_cache *
+choose_cache (NMPlatform *platform, struct nl_object *object)
+{
+	return choose_cache_by_type (platform, object_type_from_nl_object (object));
+}
+
 static gboolean
 object_has_ifindex (struct nl_object *object, int ifindex)
 {

From a6f92665555c39dd609db42b4c350b1691149d17 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Feb 2014 20:16:03 +0100
Subject: [PATCH 05/10] platform: fix caching for link types

This bug was present since a long time, however libnl3-v3.2.23
(commit fdd1ba220dd7b780400e9d0652cde80e59f63572) changed the returned
family of bridge link objects, which breaks NetworkManager.

This resulted in error messages such as:

  DBG<4>            object.c:207  nl_object_get: New reference to object 0x19c34b0, total 2
  DBG<5>        route/link.c:895  link_keygen: link 0x19c34b0 key (dev 9 fam 7) keysz 8, hash 0x2b2
  DBG<2>         hashtable.c:127  nl_hash_table_add: Warning: Add to hashtable found duplicate...
  DBG<4>            object.c:221  nl_object_put: Returned object reference 0x19c34b0, 1 remaining
  NetworkManager[17745]: <error> [1392114373.475432] [platform/nm-linux-platform.c:1328] event_notification(): netlink cache error: Object exists

Even before the change of libnl, I saw the following error lines
 <debug> [...] [platform/nm-linux-platform.c:1216] event_notification(): netlink event (type 16) for link: virbr0 (4)
 <error> [...] [platform/nm-linux-platform.c:1265] event_notification(): netlink cache error: Object exists
Hence, the caching mechanism for libnl objects already had a bug.

For rtnl link objects, the identifier consists of family and ifindex.
Since in upper layers, we don't easily know the family, we need a way to find
the objects inside the cache. We do this, by only caching links of family
AF_UNSPEC.

Objects that we receive via event_notification() are never cached. They are only used
to trigger refetching the kernel_object. Their family is irrelevant, we
only need to know, that something about this ifindex changed.

For objects retrieved via get_kernel_object(), we only get link objects of
family AF_UNSPEC or AF_BRIDGE. In any case, we reset (coerce) their family
before caching. This way, inside the link cache, there are only objects with
(coerced) family AF_UNSPEC. We loose the information, which family the
link had, however we don't need it anyway.

https://bugzilla.gnome.org/show_bug.cgi?id=719905
https://bugzilla.redhat.com/show_bug.cgi?id=1063290

Duplicates:
https://bugzilla.gnome.org/show_bug.cgi?id=724225
https://bugzilla.redhat.com/show_bug.cgi?id=1063800

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 52 +++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 81cd41458d..1c5808d518 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -189,26 +189,47 @@ object_type_from_nl_object (const struct nl_object *object)
 		return UNKNOWN_OBJECT_TYPE;
 }
 
-/* libnl inclues LINK_ATTR_FAMILY in oo_id_attrs of link_obj_ops and thus
- * refuses to search for items that lack this attribute. I believe this is a
- * bug or a bad design at the least. Address family is not an identifying
- * attribute of a network interface and IMO is not an attribute of a network
- * interface at all.
- */
+static void
+_nl_link_family_unset (struct nl_object *obj, int *family)
+{
+	if (!obj || object_type_from_nl_object (obj) != LINK)
+		*family = AF_UNSPEC;
+	else {
+		*family = rtnl_link_get_family ((struct rtnl_link *) obj);
+
+		/* Always explicitly set the family to AF_UNSPEC, even if rtnl_link_get_family() might
+		 * already return %AF_UNSPEC. The reason is, that %AF_UNSPEC is the default family
+		 * and libnl nl_object_identical() function will only succeed, if the family is
+		 * explicitly set (which we cannot be sure, unless setting it). */
+		rtnl_link_set_family ((struct rtnl_link *) obj, AF_UNSPEC);
+	}
+}
+
+/* In our link cache, we coerce the family of all link objects to AF_UNSPEC.
+ * Thus, before searching for an object, we fixup @needle to have the right
+ * id (by resetting the family). */
 static struct nl_object *
 nm_nl_cache_search (struct nl_cache *cache, struct nl_object *needle)
 {
-	if (object_type_from_nl_object (needle) == LINK)
-		rtnl_link_set_family ((struct rtnl_link *) needle, AF_UNSPEC);
+	int family;
+	struct nl_object *obj;
 
-	return nl_cache_search (cache, needle);
+	_nl_link_family_unset (needle, &family);
+	obj = nl_cache_search (cache, needle);
+	if (family != AF_UNSPEC) {
+		/* restore the family of the @needle instance. If the family was
+		 * unset before, we cannot make it unset again. Thus, in that case
+		 * we cannot undo _nl_link_family_unset() entirely. */
+		rtnl_link_set_family ((struct rtnl_link *) needle, family);
+	}
+
+	return obj;
 }
-#define nl_cache_search nm_nl_cache_search
 
 /* Ask the kernel for an object identical (as in nl_cache_identical) to the
  * needle argument. This is a kernel counterpart for nl_cache_search.
  *
- * libnl 3.2 doesn't seem to provide such functionality.
+ * The returned object must be freed by the caller with nl_object_put().
  */
 static struct nl_object *
 get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
@@ -225,6 +246,7 @@ get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
 			nle = rtnl_link_get_kernel (sock, ifindex, name, (struct rtnl_link **) &kernel_object);
 			switch (nle) {
 			case -NLE_SUCCESS:
+				_nl_link_family_unset (kernel_object, &nle);
 				return kernel_object;
 			case -NLE_NODEV:
 				return NULL;
@@ -1128,7 +1150,7 @@ refresh_object (NMPlatform *platform, struct nl_object *object, gboolean removed
 	int nle;
 
 	cache = choose_cache (platform, object);
-	cached_object = nl_cache_search (choose_cache (platform, object), object);
+	cached_object = nm_nl_cache_search (cache, object);
 	kernel_object = get_kernel_object (priv->nlh, object);
 
 	if (removed) {
@@ -1223,7 +1245,7 @@ delete_object (NMPlatform *platform, struct nl_object *obj)
 	 * for delete_kernel_object() and we need to search the cache first. If
 	 * that problem is fixed, we can use 'object' directly.
 	 */
-	cached_object = nl_cache_search (choose_cache (platform, object), object);
+	cached_object = nm_nl_cache_search (choose_cache (platform, object), object);
 	g_return_val_if_fail (cached_object, FALSE);
 
 	nle = delete_kernel_object (priv->nlh, cached_object);
@@ -1285,7 +1307,7 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 	g_return_val_if_fail (object, NL_OK);
 
 	cache = choose_cache (platform, object);
-	cached_object = nl_cache_search (cache, object);
+	cached_object = nm_nl_cache_search (cache, object);
 	kernel_object = get_kernel_object (priv->nlh, object);
 
 	/* Just for debugging */
@@ -1321,7 +1343,7 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 		 * already removed and announced.
 		 */
 		if (event == RTM_DELLINK) {
-			if (!link_is_announceable (platform, (struct rtnl_link *) object))
+			if (!link_is_announceable (platform, (struct rtnl_link *) cached_object))
 				return NL_OK;
 		}
 		announce_object (platform, cached_object, REMOVED, NM_PLATFORM_REASON_EXTERNAL);

From ebbd6575ffc78f500062303fe18ecb908b2e29ea Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Feb 2014 20:12:12 +0100
Subject: [PATCH 06/10] platform: log the link family in event_notification()
 and get_kernel_object()

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 58 ++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 1c5808d518..e4df5be042 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -234,24 +234,36 @@ nm_nl_cache_search (struct nl_cache *cache, struct nl_object *needle)
 static struct nl_object *
 get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
 {
+	struct nl_object *object = NULL;
+	ObjectType type = object_type_from_nl_object (needle);
 
-	switch (object_type_from_nl_object (needle)) {
+	switch (type) {
 	case LINK:
 		{
-			struct nl_object *kernel_object;
 			int ifindex = rtnl_link_get_ifindex ((struct rtnl_link *) needle);
 			const char *name = rtnl_link_get_name ((struct rtnl_link *) needle);
 			int nle;
 
-			nle = rtnl_link_get_kernel (sock, ifindex, name, (struct rtnl_link **) &kernel_object);
+			nle = rtnl_link_get_kernel (sock, ifindex, name, (struct rtnl_link **) &object);
 			switch (nle) {
 			case -NLE_SUCCESS:
-				_nl_link_family_unset (kernel_object, &nle);
-				return kernel_object;
+				if (nm_logging_enabled (LOGL_DEBUG, LOGD_PLATFORM)) {
+					name = rtnl_link_get_name ((struct rtnl_link *) object);
+					debug ("get_kernel_object for link: %s (%d, family %d)",
+					       name ? name : "(unknown)",
+					       rtnl_link_get_ifindex ((struct rtnl_link *) object),
+					       rtnl_link_get_family ((struct rtnl_link *) object));
+				}
+
+				_nl_link_family_unset (object, &nle);
+				return object;
 			case -NLE_NODEV:
+				debug ("get_kernel_object for link %s (%d) had no result",
+				       name ? name : "(unknown)", ifindex);
 				return NULL;
 			default:
-				error ("Netlink error: %s", nl_geterror (nle));
+				error ("get_kernel_object for link %s (%d) failed: %s (%d)",
+				       name ? name : "(unknown)", ifindex, nl_geterror (nle), nle);
 				return NULL;
 			}
 		}
@@ -262,16 +274,25 @@ get_kernel_object (struct nl_sock *sock, struct nl_object *needle)
 		/* Fallback to a one-time cache allocation. */
 		{
 			struct nl_cache *cache;
-			struct nl_object *object;
 			int nle;
 
 			nle = nl_cache_alloc_and_fill (
 					nl_cache_ops_lookup (nl_object_get_type (needle)),
 					sock, &cache);
-			g_return_val_if_fail (!nle, NULL);
+			if (nle) {
+				error ("get_kernel_object for type %d failed: %s (%d)",
+				       type, nl_geterror (nle), nle);
+				return NULL;
+			}
+
 			object = nl_cache_search (cache, needle);
 
 			nl_cache_free (cache);
+
+			if (object)
+				debug ("get_kernel_object for type %d returned %p", type, object);
+			else
+				debug ("get_kernel_object for type %d had no result", type);
 			return object;
 		}
 	default:
@@ -1306,19 +1327,22 @@ event_notification (struct nl_msg *msg, gpointer user_data)
 	nl_msg_parse (msg, ref_object, &object);
 	g_return_val_if_fail (object, NL_OK);
 
+	if (nm_logging_enabled (LOGL_DEBUG, LOGD_PLATFORM)) {
+		if (object_type_from_nl_object (object) == LINK) {
+			const char *name = rtnl_link_get_name ((struct rtnl_link *) object);
+
+			debug ("netlink event (type %d) for link: %s (%d, family %d)",
+			       event, name ? name : "(unknown)",
+			       rtnl_link_get_ifindex ((struct rtnl_link *) object),
+			       rtnl_link_get_family ((struct rtnl_link *) object));
+		} else
+			debug ("netlink event (type %d)", event);
+	}
+
 	cache = choose_cache (platform, object);
 	cached_object = nm_nl_cache_search (cache, object);
 	kernel_object = get_kernel_object (priv->nlh, object);
 
-	/* Just for debugging */
-	if (object_type_from_nl_object (object) == LINK) {
-		int ifindex = rtnl_link_get_ifindex ((struct rtnl_link *) object);
-		const char *name = rtnl_link_get_name ((struct rtnl_link *) object);
-		debug ("netlink event (type %d) for link: %s (%d)",
-		       event, name ? name : "(unknown)", ifindex);
-	} else
-		debug ("netlink event (type %d)", event);
-
 	hack_empty_master_iff_lower_up (platform, kernel_object);
 
 	/* Removed object */

From e8775dd9fc7b4867b2a541a163d14c630bf1085b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 18:12:41 +0100
Subject: [PATCH 07/10] core: add function
 nm_utils_ip6_address_clear_host_address()

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/NetworkManagerUtils.c | 44 ++++++++++++++++++++++++
 src/NetworkManagerUtils.h |  3 ++
 src/rdisc/nm-lndp-rdisc.c | 27 ++-------------
 src/tests/test-general.c  | 71 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 25 deletions(-)

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index f7f2b986f8..956a6062d6 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -75,6 +75,50 @@ nm_ethernet_address_is_valid (const struct ether_addr *test_addr)
 }
 
 
+/* nm_utils_ip4_address_clear_host_address:
+ * @addr: source ip6 address
+ * @plen: prefix length of network
+ *
+ * returns: the input address, with the host address set to 0.
+ */
+in_addr_t
+nm_utils_ip4_address_clear_host_address (in_addr_t addr, guint8 plen)
+{
+	return addr & nm_utils_ip4_prefix_to_netmask (plen);
+}
+
+	/* nm_utils_ip6_address_clear_host_address:
+ * @dst: destination output buffer, will contain the network part of the @src address
+ * @src: source ip6 address
+ * @plen: prefix length of network
+ *
+ * Note: this function is self assignment save, to update @src inplace, set both
+ * @dst and @src to the same destination.
+ */
+void
+nm_utils_ip6_address_clear_host_address (struct in6_addr *dst, const struct in6_addr *src, guint8 plen)
+{
+	g_return_if_fail (plen <= 128);
+	g_return_if_fail (src);
+	g_return_if_fail (dst);
+
+	if (plen < 128) {
+		guint nbytes = plen / 8;
+		guint nbits = plen % 8;
+
+		if (nbytes && dst != src)
+			memcpy (dst, src, nbytes);
+		if (nbits) {
+			dst->s6_addr[nbytes] = (src->s6_addr[nbytes] & (0xFF << (8 - nbits)));
+			nbytes++;
+		}
+		if (nbytes <= 15)
+			memset (&dst->s6_addr[nbytes], 0, 16 - nbytes);
+	} else if (src != dst)
+		*dst = *src;
+}
+
+
 int
 nm_spawn_process (const char *args)
 {
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index d33ceda507..ee234dda80 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -35,6 +35,9 @@
 
 gboolean nm_ethernet_address_is_valid (const struct ether_addr *test_addr);
 
+in_addr_t nm_utils_ip4_address_clear_host_address (in_addr_t addr, guint8 plen);
+void nm_utils_ip6_address_clear_host_address (struct in6_addr *dst, const struct in6_addr *src, guint8 plen);
+
 int nm_spawn_process (const char *args);
 
 gboolean nm_match_spec_string (const GSList *specs, const char *string);
diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c
index 5fb28778fc..3b0b0365b4 100644
--- a/src/rdisc/nm-lndp-rdisc.c
+++ b/src/rdisc/nm-lndp-rdisc.c
@@ -428,29 +428,6 @@ fill_address_from_mac (struct in6_addr *address, const char *mac)
 	memcpy (identifier + 5, mac + 3, 3);
 }
 
-/* Ensure the given address is masked with its prefix and that all host
- * bits are set to zero.  Some IPv6 router advertisement daemons (eg, radvd)
- * don't enforce this in their configuration.
- */
-static void
-set_address_masked (struct in6_addr *dst, struct in6_addr *src, guint8 plen)
-{
-	guint nbytes = plen / 8;
-	guint nbits = plen % 8;
-
-	g_return_if_fail (plen <= 128);
-	g_assert (src);
-	g_assert (dst);
-
-	if (plen >= 128)
-		*dst = *src;
-	else {
-		memset (dst, 0, sizeof (*dst));
-		memcpy (dst, src, nbytes);
-		dst->s6_addr[nbytes] = (src->s6_addr[nbytes] & (0xFF << (8 - nbits)));
-	}
-}
-
 static int
 receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 {
@@ -529,7 +506,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 		/* Device route */
 		memset (&route, 0, sizeof (route));
 		route.plen = ndp_msg_opt_prefix_len (msg, offset);
-		set_address_masked (&route.network, ndp_msg_opt_prefix (msg, offset), route.plen);
+		nm_utils_ip6_address_clear_host_address (&route.network, ndp_msg_opt_prefix (msg, offset), route.plen);
 		route.timestamp = now;
 		if (ndp_msg_opt_prefix_flag_on_link (msg, offset)) {
 			route.lifetime = ndp_msg_opt_prefix_valid_time (msg, offset);
@@ -560,7 +537,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 		memset (&route, 0, sizeof (route));
 		route.gateway = gateway.address;
 		route.plen = ndp_msg_opt_route_prefix_len (msg, offset);
-		set_address_masked (&route.network, ndp_msg_opt_route_prefix (msg, offset), route.plen);
+		nm_utils_ip6_address_clear_host_address (&route.network, ndp_msg_opt_route_prefix (msg, offset), route.plen);
 		route.timestamp = now;
 		route.lifetime = ndp_msg_opt_route_lifetime (msg, offset);
 		route.preference = translate_preference (ndp_msg_opt_route_preference (msg, offset));
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index 649653c75e..3fd7115cb0 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -60,6 +60,76 @@ test_nm_utils_ascii_str_to_int64 (void)
 	test_nm_utils_ascii_str_to_int64_do ("\r\n\t10000\t\n\t\n", 10, 0, 10000, -1, 0, 10000);
 }
 
+/* Reference implementation for nm_utils_ip6_address_clear_host_address.
+ * Taken originally from set_address_masked(), src/rdisc/nm-lndp-rdisc.c
+ **/
+static void
+ip6_address_clear_host_address_reference (struct in6_addr *dst, struct in6_addr *src, guint8 plen)
+{
+	guint nbytes = plen / 8;
+	guint nbits = plen % 8;
+
+	g_return_if_fail (plen <= 128);
+	g_assert (src);
+	g_assert (dst);
+
+	if (plen >= 128)
+		*dst = *src;
+	else {
+		memset (dst, 0, sizeof (*dst));
+		memcpy (dst, src, nbytes);
+		dst->s6_addr[nbytes] = (src->s6_addr[nbytes] & (0xFF << (8 - nbits)));
+	}
+}
+
+static void
+_randomize_in6_addr (struct in6_addr *addr, GRand *rand)
+{
+	int i;
+
+	for (i=0; i < 4; i++)
+		((guint32 *)addr)[i] = g_rand_int (rand);
+}
+
+static void
+test_nm_utils_ip6_address_clear_host_address (void)
+{
+	GRand *rand = g_rand_new ();
+	int plen, i;
+
+	g_rand_set_seed (rand, 0);
+
+	for (plen = 0; plen <= 128; plen++) {
+		for (i =0; i<50; i++) {
+			struct in6_addr addr_src, addr_ref;
+			struct in6_addr addr1, addr2;
+
+			_randomize_in6_addr (&addr_src, rand);
+			_randomize_in6_addr (&addr_ref, rand);
+			_randomize_in6_addr (&addr1, rand);
+			_randomize_in6_addr (&addr2, rand);
+
+			addr1 = addr_src;
+			ip6_address_clear_host_address_reference (&addr_ref, &addr1, plen);
+
+			_randomize_in6_addr (&addr1, rand);
+			_randomize_in6_addr (&addr2, rand);
+			addr1 = addr_src;
+			nm_utils_ip6_address_clear_host_address (&addr2, &addr1, plen);
+			g_assert_cmpint (memcmp (&addr1, &addr_src, sizeof (struct in6_addr)), ==, 0);
+			g_assert_cmpint (memcmp (&addr2, &addr_ref, sizeof (struct in6_addr)), ==, 0);
+
+			/* test for self assignment/inplace update. */
+			_randomize_in6_addr (&addr1, rand);
+			addr1 = addr_src;
+			nm_utils_ip6_address_clear_host_address (&addr1, &addr1, plen);
+			g_assert_cmpint (memcmp (&addr1, &addr_ref, sizeof (struct in6_addr)), ==, 0);
+		}
+	}
+
+	g_rand_free (rand);
+}
+
 /*******************************************/
 
 int
@@ -70,6 +140,7 @@ main (int argc, char **argv)
 	g_type_init ();
 
 	g_test_add_func ("/general/nm_utils_ascii_str_to_int64", test_nm_utils_ascii_str_to_int64);
+	g_test_add_func ("/general/nm_utils_ip6_address_clear_host_address", test_nm_utils_ip6_address_clear_host_address);
 
 	return g_test_run ();
 }

From d6add4de5c5ae934a355bb72a14ab4d5d4b2472f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 19:47:04 +0100
Subject: [PATCH 08/10] platform: clear host identifier before adding a route

Adding IPv4 routes, with a non-zero host identifer fails with an
error message. Adding IPv6 addresses, does not return an error,
but it seems to have no effect.

Thus we have to make sure that the host part of routes
is always zero.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index e4df5be042..33bba2c499 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -2582,16 +2582,42 @@ ip6_route_get_all (NMPlatform *platform, int ifindex, gboolean include_default)
 	return routes;
 }
 
+static void
+clear_host_address (int family, const void *network, int plen, void *dst)
+{
+	g_return_if_fail (plen == (guint8)plen);
+	g_return_if_fail (network);
+
+	switch (family) {
+	case AF_INET:
+		*((in_addr_t *) dst) = nm_utils_ip4_address_clear_host_address (*((in_addr_t *) network), plen);
+		break;
+	case AF_INET6:
+		nm_utils_ip6_address_clear_host_address ((struct in6_addr *) dst, (const struct in6_addr *) network, plen);
+		break;
+	default:
+		g_assert_not_reached ();
+	}
+}
+
 static struct nl_object *
 build_rtnl_route (int family, int ifindex, gconstpointer network, int plen, gconstpointer gateway, int metric, int mss)
 {
+	guint32 network_clean[4];
 	struct rtnl_route *rtnlroute = rtnl_route_alloc ();
 	struct rtnl_nexthop *nexthop = rtnl_route_nh_alloc ();
 	int addrlen = (family == AF_INET) ? sizeof (in_addr_t) : sizeof (struct in6_addr);
 	/* Workaround a libnl bug by using zero destination address length for default routes */
-	auto_nl_addr struct nl_addr *dst = nl_addr_build (family, network, plen ? addrlen : 0);
+	auto_nl_addr struct nl_addr *dst = NULL;
 	auto_nl_addr struct nl_addr *gw = gateway ? nl_addr_build (family, gateway, addrlen) : NULL;
 
+	/* There seem to be problems adding a route with non-zero host identifier.
+	 * Adding IPv6 routes is simply ignored, without error message.
+	 * In the IPv4 case, we got an error. Thus, we have to make sure, that
+	 * the address is sane. */
+	clear_host_address (family, network, plen, network_clean);
+	dst = nl_addr_build (family, network_clean, plen ? addrlen : 0);
+
 	g_assert (rtnlroute && dst && nexthop);
 
 	nl_addr_set_prefixlen (dst, plen);
@@ -2635,7 +2661,7 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen
 static gboolean
 ip6_route_delete (NMPlatform *platform, int ifindex, struct in6_addr network, int plen, int metric)
 {
-	struct in6_addr gateway = in6addr_any;
+	struct in6_addr gateway = IN6ADDR_ANY_INIT;
 
 	return delete_object (platform, build_rtnl_route (AF_INET6, ifindex, &network, plen, &gateway, metric, 0));
 }

From 5f5c7284d16c5f24200bbbd40c5e9a83f84516cd Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 15:11:05 +0100
Subject: [PATCH 09/10] platform: refactor delete_object() and allow deletion
 of objects that are not cached

- refactor delete_object() by merging with delete_kernel_object()

- allow deletion of object that we cannot find in the cache
  currently. The kernel might have such an address, even if we don't
  have it currently cached. In this case, fall back to @obj.

  Also try to work around an issue, that we cannot delete an IPv4 route without
  knowing its scope.

- suppress logging error message for NLE_NOADDR, which is a common
  failure when deleting an address. But at the same time, add some more
  debug logging, for NLE_NOADDR and NLE_OBJ_NOTFOUND.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-linux-platform.c | 90 ++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 32 deletions(-)

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 33bba2c499..dde01c2019 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -320,25 +320,6 @@ add_kernel_object (struct nl_sock *sock, struct nl_object *object)
 	}
 }
 
-/* libnl 3.2 doesn't seem to provide such a generic way to delete libnl-route objects. */
-static int
-delete_kernel_object (struct nl_sock *sock, struct nl_object *object)
-{
-	switch (object_type_from_nl_object (object)) {
-	case LINK:
-		return rtnl_link_delete (sock, (struct rtnl_link *) object);
-	case IP4_ADDRESS:
-	case IP6_ADDRESS:
-		return rtnl_addr_delete (sock, (struct rtnl_addr *) object, 0);
-	case IP4_ROUTE:
-	case IP6_ROUTE:
-		return rtnl_route_delete (sock, (struct rtnl_route *) object, 0);
-	default:
-		g_return_val_if_reached (-NLE_INVAL);
-		return -NLE_INVAL;
-	}
-}
-
 /* nm_rtnl_link_parse_info_data(): Re-fetches a link from the kernel
  * and parses its IFLA_INFO_DATA using a caller-provided parser.
  *
@@ -1234,6 +1215,8 @@ add_object (NMPlatform *platform, struct nl_object *obj)
 		.dp_fd = stderr,
 	};
 
+	g_return_val_if_fail (object, FALSE);
+
 	nle = add_kernel_object (priv->nlh, object);
 
 	/* NLE_EXIST is considered equivalent to success to avoid race conditions. You
@@ -1258,26 +1241,48 @@ static gboolean
 delete_object (NMPlatform *platform, struct nl_object *obj)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
-	auto_nl_object struct nl_object *object = obj;
+	auto_nl_object struct nl_object *obj_cleanup = obj;
 	auto_nl_object struct nl_object *cached_object = NULL;
+	struct nl_object *object;
+	int object_type;
 	int nle;
 
-	/* FIXME: For some reason the result of build_rtnl_route() is not suitable
-	 * for delete_kernel_object() and we need to search the cache first. If
-	 * that problem is fixed, we can use 'object' directly.
-	 */
-	cached_object = nm_nl_cache_search (choose_cache (platform, object), object);
-	g_return_val_if_fail (cached_object, FALSE);
+	object_type = object_type_from_nl_object (obj);
+	g_return_val_if_fail (object_type != UNKNOWN_OBJECT_TYPE, FALSE);
 
-	nle = delete_kernel_object (priv->nlh, cached_object);
+	cached_object = nm_nl_cache_search (choose_cache_by_type (platform, object_type), obj);
+	object = cached_object ? cached_object : obj;
+
+	switch (object_type) {
+	case LINK:
+		nle = rtnl_link_delete (priv->nlh, (struct rtnl_link *) object);
+		break;
+	case IP4_ADDRESS:
+	case IP6_ADDRESS:
+		nle = rtnl_addr_delete (priv->nlh, (struct rtnl_addr *) object, 0);
+		break;
+	case IP4_ROUTE:
+	case IP6_ROUTE:
+		nle = rtnl_route_delete (priv->nlh, (struct rtnl_route *) object, 0);
+		break;
+	default:
+		g_assert_not_reached ();
+	}
 
-	/* NLE_OBJ_NOTFOUND is considered equivalent to success to avoid race conditions. You
-	 * never know when something deletes the same object just before NetworkManager.
-	 */
 	switch (nle) {
 	case -NLE_SUCCESS:
-	case -NLE_OBJ_NOTFOUND:
 		break;
+	case -NLE_OBJ_NOTFOUND:
+		debug("delete_object failed with \"%s\" (%d), meaning the object was already removed",
+		      nl_geterror (nle), nle);
+		break;
+	case -NLE_NOADDR:
+		if (object_type == IP4_ADDRESS || object_type == IP6_ADDRESS) {
+			debug("delete_object for address failed with \"%s\" (%d), meaning the address was already removed",
+			      nl_geterror (nle), nle);
+			break;
+		}
+		/* fall-through to error, because we only expect this for addresses. */
 	default:
 		error ("Netlink error: %s", nl_geterror (nle));
 		return FALSE;
@@ -2626,6 +2631,7 @@ build_rtnl_route (int family, int ifindex, gconstpointer network, int plen, gcon
 	rtnl_route_set_tos (rtnlroute, 0);
 	rtnl_route_set_dst (rtnlroute, dst);
 	rtnl_route_set_priority (rtnlroute, metric);
+	rtnl_route_set_family (rtnlroute, family);
 
 	rtnl_route_nh_set_ifindex (nexthop, ifindex);
 	if (gw && !nl_addr_iszero (gw))
@@ -2654,8 +2660,28 @@ static gboolean
 ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen, int metric)
 {
 	in_addr_t gateway = 0;
+	struct nl_object *route = build_rtnl_route (AF_INET, ifindex, &network, plen, &gateway, metric, 0);
 
-	return delete_object (platform, build_rtnl_route (AF_INET, ifindex, &network, plen, &gateway, metric, 0));
+	g_return_val_if_fail (route, FALSE);
+
+	/* When searching for a matching IPv4 route to delete, the kernel
+	 * searches for a matching scope, unless the RTM_DELROUTE message
+	 * specifies RT_SCOPE_NOWHERE (see fib_table_delete()).
+	 *
+	 * However, if we set the scope of @rtnlroute to RT_SCOPE_NOWHERE (or
+	 * leave it unset), rtnl_route_build_msg() will reset the scope to
+	 * rtnl_route_guess_scope() -- which might be the wrong scope.
+	 *
+	 * As a workaround, we set the scope to RT_SCOPE_UNIVERSE, so libnl
+	 * will not overwrite it. But this only works if we guess correctly.
+	 *
+	 * As a better workaround, we don't use @rtnlroute as argument for
+	 * rtnl_route_delete(), but we look into our cache, if we already have
+	 * this route ready.
+	 **/
+	rtnl_route_set_scope ((struct rtnl_route *) route, RT_SCOPE_UNIVERSE);
+
+	return delete_object (platform, route);
 }
 
 static gboolean

From 2bc90a5f2d3dabc84a6ce3a8eb6a0e2582c1c9f2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 13 Feb 2014 14:56:38 +0100
Subject: [PATCH 10/10] platform: do not check for _exists() before deleting
 addresses and routes

Before, nm_platform_ip4_address_exists(), et al. look into the cache to see
whether the address/route already exists and returned an error if it
did.

Change the semantic of the delete functions, to return success in case of
"nothing to delete". Also always try to delete the object in the
kernel. The reason is, that the cache might be out of date and the
caller really wants to delete it. So, to be sure, we always delete.

In most cases the object is actually in the cache (because that is
how the caller came to know that such an object might exist).
In those cases, the lookup was not useful either, because the object
was actually cached.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/platform/nm-fake-platform.c   | 24 ++++++++++++------------
 src/platform/nm-platform.c        | 28 ----------------------------
 src/platform/tests/test-address.c |  8 ++++----
 src/platform/tests/test-route.c   |  8 ++++----
 4 files changed, 20 insertions(+), 48 deletions(-)

diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c
index 43944721cd..d0127c4733 100644
--- a/src/platform/nm-fake-platform.c
+++ b/src/platform/nm-fake-platform.c
@@ -827,7 +827,7 @@ ip4_address_delete (NMPlatform *platform, int ifindex, in_addr_t addr, int plen)
 		}
 	}
 
-	g_assert_not_reached ();
+	return TRUE;
 }
 
 static gboolean
@@ -850,7 +850,7 @@ ip6_address_delete (NMPlatform *platform, int ifindex, struct in6_addr addr, int
 		}
 	}
 
-	g_assert_not_reached ();
+	return TRUE;
 }
 
 static gboolean
@@ -1064,11 +1064,11 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen
 	NMPlatformIP4Route *route = ip4_route_get (platform, ifindex, network, plen, metric);
 	NMPlatformIP4Route deleted_route;
 
-	g_assert (route);
-
-	memcpy (&deleted_route, route, sizeof (deleted_route));
-	memset (route, 0, sizeof (*route));
-	g_signal_emit_by_name (platform, NM_PLATFORM_IP4_ROUTE_REMOVED, ifindex, &deleted_route, NM_PLATFORM_REASON_INTERNAL);
+	if (route) {
+		memcpy (&deleted_route, route, sizeof (deleted_route));
+		memset (route, 0, sizeof (*route));
+		g_signal_emit_by_name (platform, NM_PLATFORM_IP4_ROUTE_REMOVED, ifindex, &deleted_route, NM_PLATFORM_REASON_INTERNAL);
+	}
 
 	return TRUE;
 }
@@ -1079,11 +1079,11 @@ ip6_route_delete (NMPlatform *platform, int ifindex, struct in6_addr network, in
 	NMPlatformIP6Route *route = ip6_route_get (platform, ifindex, network, plen, metric);
 	NMPlatformIP6Route deleted_route;
 
-	g_assert (route);
-
-	memcpy (&deleted_route, route, sizeof (deleted_route));
-	memset (route, 0, sizeof (*route));
-	g_signal_emit_by_name (platform, NM_PLATFORM_IP6_ROUTE_REMOVED, ifindex, &deleted_route, NM_PLATFORM_REASON_INTERNAL);
+	if (route) {
+		memcpy (&deleted_route, route, sizeof (deleted_route));
+		memset (route, 0, sizeof (*route));
+		g_signal_emit_by_name (platform, NM_PLATFORM_IP6_ROUTE_REMOVED, ifindex, &deleted_route, NM_PLATFORM_REASON_INTERNAL);
+	}
 
 	return TRUE;
 }
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 8bdfaf42f1..8485e74ebe 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -1282,13 +1282,6 @@ nm_platform_ip4_address_delete (int ifindex, in_addr_t address, int plen)
 	g_return_val_if_fail (klass->ip4_address_delete, FALSE);
 
 	debug ("address: deleting IPv4 address %s/%d", nm_utils_inet4_ntop (address, NULL), plen);
-
-	if (!nm_platform_ip4_address_exists (ifindex, address, plen)) {
-		debug ("address doesn't exists");
-		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
-		return FALSE;
-	}
-
 	return klass->ip4_address_delete (platform, ifindex, address, plen);
 }
 
@@ -1302,13 +1295,6 @@ nm_platform_ip6_address_delete (int ifindex, struct in6_addr address, int plen)
 	g_return_val_if_fail (klass->ip6_address_delete, FALSE);
 
 	debug ("address: deleting IPv6 address %s/%d", nm_utils_inet6_ntop (&address, NULL), plen);
-
-	if (!nm_platform_ip6_address_exists (ifindex, address, plen)) {
-		debug ("address doesn't exists");
-		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
-		return FALSE;
-	}
-
 	return klass->ip6_address_delete (platform, ifindex, address, plen);
 }
 
@@ -1581,13 +1567,6 @@ nm_platform_ip4_route_delete (int ifindex, in_addr_t network, int plen, int metr
 	g_return_val_if_fail (klass->ip4_route_delete, FALSE);
 
 	debug ("route: deleting IPv4 route %s/%d, metric=%d", nm_utils_inet4_ntop (network, NULL), plen, metric);
-
-	if (!nm_platform_ip4_route_exists (ifindex, network, plen, metric)) {
-		debug ("route not found");
-		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
-		return FALSE;
-	}
-
 	return klass->ip4_route_delete (platform, ifindex, network, plen, metric);
 }
 
@@ -1601,13 +1580,6 @@ nm_platform_ip6_route_delete (int ifindex,
 	g_return_val_if_fail (klass->ip6_route_delete, FALSE);
 
 	debug ("route: deleting IPv6 route %s/%d, metric=%d", nm_utils_inet6_ntop (&network, NULL), plen, metric);
-
-	if (!nm_platform_ip6_route_exists (ifindex, network, plen, metric)) {
-		debug ("route not found");
-		platform->error = NM_PLATFORM_ERROR_NOT_FOUND;
-		return FALSE;
-	}
-
 	return klass->ip6_route_delete (platform, ifindex, network, plen, metric);
 }
 
diff --git a/src/platform/tests/test-address.c b/src/platform/tests/test-address.c
index 0f0ccc81eb..20f1bd9ed6 100644
--- a/src/platform/tests/test-address.c
+++ b/src/platform/tests/test-address.c
@@ -89,8 +89,8 @@ test_ip4_address (void)
 	accept_signal (address_removed);
 
 	/* Remove address again */
-	g_assert (!nm_platform_ip4_address_delete (ifindex, addr, IP4_PLEN));
-	error (NM_PLATFORM_ERROR_NOT_FOUND);
+	g_assert (nm_platform_ip4_address_delete (ifindex, addr, IP4_PLEN));
+	no_error ();
 
 	free_signal (address_added);
 	free_signal (address_changed);
@@ -145,8 +145,8 @@ test_ip6_address (void)
 	accept_signal (address_removed);
 
 	/* Remove address again */
-	g_assert (!nm_platform_ip6_address_delete (ifindex, addr, IP6_PLEN));
-	error (NM_PLATFORM_ERROR_NOT_FOUND);
+	g_assert (nm_platform_ip6_address_delete (ifindex, addr, IP6_PLEN));
+	no_error ();
 
 	free_signal (address_added);
 	free_signal (address_changed);
diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c
index 6c764ad3c3..f333ffc3c1 100644
--- a/src/platform/tests/test-route.c
+++ b/src/platform/tests/test-route.c
@@ -113,8 +113,8 @@ test_ip4_route ()
 	accept_signal (route_removed);
 
 	/* Remove route again */
-	g_assert (!nm_platform_ip4_route_delete (ifindex, network, plen, metric));
-	error (NM_PLATFORM_ERROR_NOT_FOUND);
+	g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, metric));
+	no_error ();
 
 	free_signal (route_added);
 	free_signal (route_changed);
@@ -196,8 +196,8 @@ test_ip6_route ()
 	accept_signal (route_removed);
 
 	/* Remove route again */
-	g_assert (!nm_platform_ip6_route_delete (ifindex, network, plen, metric));
-	error (NM_PLATFORM_ERROR_NOT_FOUND);
+	g_assert (nm_platform_ip6_route_delete (ifindex, network, plen, metric));
+	no_error ();
 
 	free_signal (route_added);
 	free_signal (route_changed);