platform: avoid conflicts when reinstalling the device-routeth/bgo723178_device_route_metric_v2

Since f32075d2fc11252e5661166b2f46c18c017929e9, we remove the kernel
added IPv4 device route, and re-add it with appropriate metric.

This could potentially replace existing, conflicting routes. Be more
careful and only take any action when we don't have a conflicting
route and when we add the address for the first time.

The motivation for this was libreswan which might install a VPN route
for a subnet that we also have configured on an interface. But the route
conflict could happen easily for other reasons, for example if you
configure a conflicting route manually.

This is all avoided by not doing anything if we have any indication
that some conflicts could arise.

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

3 files changed, 96 insertions, 31 deletions

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 954edec3b7..07a56c583c 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -103,7 +103,7 @@ G_DEFINE_TYPE (NMLinuxPlatform, nm_linux_platform, NM_TYPE_PLATFORM)
 
 static const char *to_string_object (NMPlatform *platform, struct nl_object *obj);
 static gboolean _address_match (struct rtnl_addr *addr, int family, int ifindex);
-static gboolean _route_match (struct rtnl_route *rtnlroute, int family, int ifindex);
+static gboolean _route_match (struct rtnl_route *rtnlroute, int family, int ifindex, gboolean include_proto_kernel);
 
 void
 nm_linux_platform_setup (void)
@@ -1668,7 +1668,7 @@ announce_object (NMPlatform *platform, const struct nl_object *object, NMPlatfor
 		{
 			NMPlatformIP4Route route;
 
-			if (!_route_match ((struct rtnl_route *) object, AF_INET, 0)) {
+			if (!_route_match ((struct rtnl_route *) object, AF_INET, 0, FALSE)) {
 				nm_log_dbg (LOGD_PLATFORM, "skip announce unmatching IP4 route %s", to_string_ip4_route ((struct rtnl_route *) object));
 				return;
 			}
@@ -1680,7 +1680,7 @@ announce_object (NMPlatform *platform, const struct nl_object *object, NMPlatfor
 		{
 			NMPlatformIP6Route route;
 
-			if (!_route_match ((struct rtnl_route *) object, AF_INET6, 0)) {
+			if (!_route_match ((struct rtnl_route *) object, AF_INET6, 0, FALSE)) {
 				nm_log_dbg (LOGD_PLATFORM, "skip announce unmatching IP6 route %s", to_string_ip6_route ((struct rtnl_route *) object));
 				return;
 			}
@@ -3632,10 +3632,60 @@ ip6_address_exists (NMPlatform *platform, int ifindex, struct in6_addr addr, int
 	return ip_address_exists (platform, AF_INET6, ifindex, &addr, plen);
 }
 
+static gboolean
+ip4_check_reinstall_device_route (NMPlatform *platform, int ifindex, const NMPlatformIP4Address *address, guint32 device_route_metric)
+{
+	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
+	NMPlatformIP4Address addr_candidate;
+	NMPlatformIP4Route route_candidate;
+	struct nl_object *object;
+	guint32 device_network;
+
+	g_return_val_if_fail (address, FALSE);
+
+	if (   ifindex <= 0
+	    || address->plen <= 0
+	    || address->plen >= 32)
+		return FALSE;
+
+	for (object = nl_cache_get_first (priv->address_cache); object; object = nl_cache_get_next (object)) {
+		if (_address_match ((struct rtnl_addr *) object, AF_INET, 0)) {
+			if (init_ip4_address (&addr_candidate, (struct rtnl_addr *) object))
+				if (   addr_candidate.plen == address->plen
+				    && addr_candidate.address == address->address) {
+					/* If we already have the same address installed on any interface,
+					 * we back off.
+					 * Perform this check first, as we expect to have significantly less
+					 * addresses to search. */
+					return FALSE;
+				}
+		}
+	}
+
+	device_network = nm_utils_ip4_address_clear_host_address (address->address, address->plen);
+
+	for (object = nl_cache_get_first (priv->route_cache); object; object = nl_cache_get_next (object)) {
+		if (_route_match ((struct rtnl_route *) object, AF_INET, 0, TRUE)) {
+			if (init_ip4_route (&route_candidate, (struct rtnl_route *) object)) {
+				if (   route_candidate.network == device_network
+				    && route_candidate.plen == address->plen
+				    && (   route_candidate.metric == 0
+				        || route_candidate.metric == device_route_metric)) {
+					/* There is already any route with metric 0 or the metric we want to install
+					 * for the same subnet. */
+					return FALSE;
+				}
+			}
+		}
+	}
+
+	return TRUE;
+}
+
 /******************************************************************/
 
 static gboolean
-_route_match (struct rtnl_route *rtnlroute, int family, int ifindex)
+_route_match (struct rtnl_route *rtnlroute, int family, int ifindex, gboolean include_proto_kernel)
 {
 	struct rtnl_nexthop *nexthop;
 
@@ -3643,7 +3693,7 @@ _route_match (struct rtnl_route *rtnlroute, int family, int ifindex)
 
 	if (rtnl_route_get_type (rtnlroute) != RTN_UNICAST ||
 	    rtnl_route_get_table (rtnlroute) != RT_TABLE_MAIN ||
-	    rtnl_route_get_protocol (rtnlroute) == RTPROT_KERNEL ||
+	    (!include_proto_kernel && rtnl_route_get_protocol (rtnlroute) == RTPROT_KERNEL) ||
 	    rtnl_route_get_family (rtnlroute) != family ||
 	    rtnl_route_get_nnexthops (rtnlroute) != 1 ||
 	    rtnl_route_get_flags (rtnlroute) & RTM_F_CLONED)
@@ -3669,7 +3719,7 @@ ip4_route_get_all (NMPlatform *platform, int ifindex, NMPlatformGetRouteMode mod
 	routes = g_array_new (FALSE, FALSE, sizeof (NMPlatformIP4Route));
 
 	for (object = nl_cache_get_first (priv->route_cache); object; object = nl_cache_get_next (object)) {
-		if (_route_match ((struct rtnl_route *) object, AF_INET, ifindex)) {
+		if (_route_match ((struct rtnl_route *) object, AF_INET, ifindex, FALSE)) {
 			if (_rtnl_route_is_default ((struct rtnl_route *) object)) {
 				if (mode == NM_PLATFORM_GET_ROUTE_MODE_NO_DEFAULT)
 					continue;
@@ -3698,7 +3748,7 @@ ip6_route_get_all (NMPlatform *platform, int ifindex, NMPlatformGetRouteMode mod
 	routes = g_array_new (FALSE, FALSE, sizeof (NMPlatformIP6Route));
 
 	for (object = nl_cache_get_first (priv->route_cache); object; object = nl_cache_get_next (object)) {
-		if (_route_match ((struct rtnl_route *) object, AF_INET6, ifindex)) {
+		if (_route_match ((struct rtnl_route *) object, AF_INET6, ifindex, FALSE)) {
 			if (_rtnl_route_is_default ((struct rtnl_route *) object)) {
 				if (mode == NM_PLATFORM_GET_ROUTE_MODE_NO_DEFAULT)
 					continue;
@@ -3805,7 +3855,7 @@ route_search_cache (struct nl_cache *cache, int family, int ifindex, const void
 		struct nl_addr *dst;
 		struct rtnl_route *rtnlroute = (struct rtnl_route *) object;
 
-		if (!_route_match (rtnlroute, family, ifindex))
+		if (!_route_match (rtnlroute, family, ifindex, FALSE))
 			continue;
 
 		if (metric && metric != rtnl_route_get_priority (rtnlroute))
@@ -4362,6 +4412,8 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass)
 	platform_class->ip4_address_exists = ip4_address_exists;
 	platform_class->ip6_address_exists = ip6_address_exists;
 
+	platform_class->ip4_check_reinstall_device_route = ip4_check_reinstall_device_route;
+
 	platform_class->ip4_route_get_all = ip4_route_get_all;
 	platform_class->ip6_route_get_all = ip6_route_get_all;
 	platform_class->ip4_route_add = ip4_route_add;
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 7433ccd186..355fc2d1d2 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -1732,6 +1732,14 @@ _address_get_lifetime (const NMPlatformIPAddress *address, guint32 now, guint32
 	return TRUE;
 }
 
+gboolean
+nm_platform_ip4_check_reinstall_device_route (int ifindex, const NMPlatformIP4Address *address, guint32 device_route_metric)
+{
+	if (klass->ip4_check_reinstall_device_route)
+		return klass->ip4_check_reinstall_device_route (platform, ifindex, address, device_route_metric);
+	return FALSE;
+}
+
 /**
  * nm_platform_ip4_address_sync:
  * @ifindex: Interface index
@@ -1771,38 +1779,39 @@ nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses, guint3
 		const NMPlatformIP4Address *known_address = &g_array_index (known_addresses, NMPlatformIP4Address, i);
 		guint32 lifetime, preferred;
 		guint32 network;
+		gboolean reinstall_device_route = FALSE;
 
 		/* add a padding of 5 seconds to avoid potential races. */
 		if (!_address_get_lifetime ((NMPlatformIPAddress *) known_address, now, 5, &lifetime, &preferred))
 			continue;
 
+		if (   known_address->plen > 0
+		    && known_address->plen < 32
+		    && device_route_metric != NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE
+		    && nm_platform_ip4_check_reinstall_device_route (ifindex, known_address, device_route_metric))
+			reinstall_device_route = TRUE;
+
 		if (!nm_platform_ip4_address_add (ifindex, known_address->address, known_address->peer_address, known_address->plen, lifetime, preferred, known_address->label))
 			return FALSE;
 
-		if (known_address->plen == 0 || known_address->plen == 32)
-			continue;
-
-		if (device_route_metric == NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE) {
-			/* Kernel already adds routes for us with this metric. */
-			continue;
+		if (reinstall_device_route) {
+			/* Kernel automatically adds a device route for us with metric 0. That is not what we want.
+			 * Remove it, and re-add it.
+			 *
+			 * In face of having the same subnets on two different interfaces with the same metric,
+			 * this is a problem. Surprisingly, kernel is able to add two routes for the same subnet/prefix,metric
+			 * to different interfaces. We cannot. Adding one, will replace the other. Indeed we will
+			 * toggle the routes between the interfaces.
+			 *
+			 * Indeed we have that problem for all our routes, that if another interface wants the same route
+			 * we don't coordinate them. See bug 740064.
+			 *
+			 * The workaround is to configure different device priorities via ipv4.route-metric. */
+
+			network = nm_utils_ip4_address_clear_host_address (known_address->address, known_address->plen);
+			nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_KERNEL, network, known_address->plen, 0, known_address->address, device_route_metric, 0);
+			nm_platform_ip4_route_delete (ifindex, network, known_address->plen, NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE);
 		}
-
-		/* Kernel automatically adds a device route for us with metric 0. That is not what we want.
-		 * Remove it, and re-add it.
-		 *
-		 * In face of having the same subnets on two different interfaces with the same metric,
-		 * this is a problem. Surprisingly, kernel is able to add two routes for the same subnet/prefix,metric
-		 * to different interfaces. We cannot. Adding one, will replace the other. Indeed we will
-		 * toggle the routes between the interfaces.
-		 *
-		 * Indeed we have that problem for all our routes, that if another interface wants the same route
-		 * we don't coordinate them. See bug 740064.
-		 *
-		 * The workaround is to configure different device priorities via ipv4.route-metric. */
-
-		network = nm_utils_ip4_address_clear_host_address (known_address->address, known_address->plen);
-		nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_KERNEL, network, known_address->plen, 0, known_address->address, device_route_metric, 0);
-		nm_platform_ip4_route_delete (ifindex, network, known_address->plen, NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE);
 	}
 
 	return TRUE;
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index 941cd0a028..848f78e428 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -423,6 +423,8 @@ typedef struct {
 	gboolean (*ip4_address_exists) (NMPlatform *, int ifindex, in_addr_t address, int plen);
 	gboolean (*ip6_address_exists) (NMPlatform *, int ifindex, struct in6_addr address, int plen);
 
+	gboolean (*ip4_check_reinstall_device_route) (NMPlatform *, int ifindex, const NMPlatformIP4Address *address, guint32 device_route_metric);
+
 	GArray * (*ip4_route_get_all) (NMPlatform *, int ifindex, NMPlatformGetRouteMode mode);
 	GArray * (*ip6_route_get_all) (NMPlatform *, int ifindex, NMPlatformGetRouteMode mode);
 	gboolean (*ip4_route_add) (NMPlatform *, int ifindex, NMIPConfigSource source,
@@ -570,6 +572,8 @@ gboolean nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresse
 gboolean nm_platform_ip6_address_sync (int ifindex, const GArray *known_addresses);
 gboolean nm_platform_address_flush (int ifindex);
 
+gboolean nm_platform_ip4_check_reinstall_device_route (int ifindex, const NMPlatformIP4Address *address, guint32 device_route_metric);
+
 GArray *nm_platform_ip4_route_get_all (int ifindex, NMPlatformGetRouteMode mode);
 GArray *nm_platform_ip6_route_get_all (int ifindex, NMPlatformGetRouteMode mode);
 gboolean nm_platform_ip4_route_add (int ifindex, NMIPConfigSource source,