platform: properly handle same routes in platform cacheth/platform-route-gateway

From kernels point of view, you can add a new routes as long as
there is no existing route that is exactly identical. That effectively
means, that every field of the route is part of the ID.

Currently, we handle that wrong, thus when kernel notifies platform
about two such routes, we would wrongly merge them together.
For example:

    ip link add dev0 type dummy
    ip link add dev1 type dummy
    ip link set dev0 up
    ip link set dev1 up
    ip addr add 192.168.200.4/24 dev dev0
    ip addr add 192.168.201.4/24 dev dev1

    ip route add    10.132.5.0/24 dev dev0
    ip route append 10.132.5.0/24 via 192.168.212.1 dev dev0
    ip route append 10.132.5.0/24 via 192.168.212.1 dev dev0 proto 5
    ip route show dev dev0

    ip route add    1:2:3:4:5::/64 via fe80::1:a dev dev0
    ip route append 1:2:3:4:5::/64 via fe80::1:b dev dev0
    ip route append 1:2:3:4:5::/64 via fe80::1:b dev dev0 proto 5
    ip -6 route show dev dev0

Note the difference here are the netlink flags:
  `ip route append`  (NLM_F_CREATE|NLM_F_APPEND)
  `ip route prepend` (NLM_F_CREATE)
  `ip route add`     (NLM_F_CREATE|NLM_F_EXCL)
  `ip route change`  (NLM_F_REPLACE)
  `ip route replace` (NLM_F_CREATE|NLM_F_REPLACE)

Extend platform to consider every property of a route to be part
of the ID. Also update the API like route_add(), route_get() and
route_delete() to accept full route structures as arguments.
For delete, that means you can only delete a route that you know
about. But that isn't really actually a problem.

1 files changed, 98 insertions, 35 deletions

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index afbe5f8263..e382370446 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -2908,45 +2908,49 @@ nm_platform_ip6_route_add (NMPlatform *self,
 }
 
 gboolean
-nm_platform_ip4_route_delete (NMPlatform *self, int ifindex, in_addr_t network, guint8 plen, guint32 metric)
+nm_platform_ip4_route_delete (NMPlatform *self, int ifindex, in_addr_t network, guint8 plen, guint32 metric, guint32 gateway)
 {
 	char str_dev[TO_STRING_DEV_BUF_SIZE];
+	char str_gateway[NM_UTILS_INET_ADDRSTRLEN];
 
 	_CHECK_SELF (self, klass, FALSE);
 
-	_LOGD ("route: deleting IPv4 route %s/%d, metric=%"G_GUINT32_FORMAT", ifindex %d%s",
+	_LOGD ("route: deleting IPv4 route %s/%d, metric=%"G_GUINT32_FORMAT", ifindex %d%s, gateway=%s",
 	       nm_utils_inet4_ntop (network, NULL), plen, metric, ifindex,
-	       _to_string_dev (self, ifindex, str_dev, sizeof (str_dev)));
-	return klass->ip4_route_delete (self, ifindex, network, plen, metric);
+	       _to_string_dev (self, ifindex, str_dev, sizeof (str_dev)),
+	       nm_utils_inet4_ntop (gateway, str_gateway));
+	return klass->ip4_route_delete (self, ifindex, network, plen, metric, gateway);
 }
 
 gboolean
-nm_platform_ip6_route_delete (NMPlatform *self, int ifindex, struct in6_addr network, guint8 plen, guint32 metric)
+nm_platform_ip6_route_delete (NMPlatform *self, int ifindex, struct in6_addr network, guint8 plen, guint32 metric, const struct in6_addr *gateway)
 {
 	char str_dev[TO_STRING_DEV_BUF_SIZE];
+	char str_gateway[NM_UTILS_INET_ADDRSTRLEN];
 
 	_CHECK_SELF (self, klass, FALSE);
 
-	_LOGD ("route: deleting IPv6 route %s/%d, metric=%"G_GUINT32_FORMAT", ifindex %d%s",
+	_LOGD ("route: deleting IPv6 route %s/%d, metric=%"G_GUINT32_FORMAT", ifindex %d%s, gateway=%s",
 	       nm_utils_inet6_ntop (&network, NULL), plen, metric, ifindex,
-	       _to_string_dev (self, ifindex, str_dev, sizeof (str_dev)));
-	return klass->ip6_route_delete (self, ifindex, network, plen, metric);
+	       _to_string_dev (self, ifindex, str_dev, sizeof (str_dev)),
+	       nm_utils_inet6_ntop (gateway ?: &nm_ip_addr_zero.addr6, str_gateway));
+	return klass->ip6_route_delete (self, ifindex, network, plen, metric, gateway);
 }
 
 const NMPlatformIP4Route *
-nm_platform_ip4_route_get (NMPlatform *self, int ifindex, in_addr_t network, guint8 plen, guint32 metric)
+nm_platform_ip4_route_get (NMPlatform *self, int ifindex, in_addr_t network, guint8 plen, guint32 metric, guint32 gateway)
 {
 	_CHECK_SELF (self, klass, FALSE);
 
-	return klass->ip4_route_get (self ,ifindex, network, plen, metric);
+	return klass->ip4_route_get (self ,ifindex, network, plen, metric, gateway);
 }
 
 const NMPlatformIP6Route *
-nm_platform_ip6_route_get (NMPlatform *self, int ifindex, struct in6_addr network, guint8 plen, guint32 metric)
+nm_platform_ip6_route_get (NMPlatform *self, int ifindex, struct in6_addr network, guint8 plen, guint32 metric, const struct in6_addr *gateway)
 {
 	_CHECK_SELF (self, klass, FALSE);
 
-	return klass->ip6_route_get (self, ifindex, network, plen, metric);
+	return klass->ip6_route_get (self, ifindex, network, plen, metric, gateway);
 }
 
 /******************************************************************/
@@ -3917,6 +3921,9 @@ int
 nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route *b)
 {
 	_CMP_SELF (a, b);
+
+	/* for nmp_object_id_equal(), all fields of the route are considered here. */
+
 	_CMP_FIELD (a, b, ifindex);
 	_CMP_FIELD (a, b, network);
 	_CMP_FIELD (a, b, plen);
@@ -3930,10 +3937,35 @@ nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route
 	return 0;
 }
 
+guint
+nm_platform_ip4_route_hash (const NMPlatformIP4Route *a)
+{
+	guint h = 0x8202baf9;
+
+	/* goes together with nm_platform_ip4_route_cmp(). */
+
+	if (a) {
+		h += (h * 33) + ((guint) a->ifindex);
+		h += (h * 33) + ((guint) a->network);
+		h += (h * 33) + ((guint) a->plen);
+		h += (h * 33) + ((guint) a->metric);
+		h += (h * 33) + ((guint) a->gateway);
+		h += (h * 33) + ((guint) a->rt_source);
+		h += (h * 33) + ((guint) a->mss);
+		h += (h * 33) + ((guint) a->scope_inv);
+		h += (h * 33) + ((guint) a->pref_src);
+		h += (h * 33) + ((guint) a->rt_cloned);
+	}
+	return h;
+}
+
 int
 nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route *b)
 {
 	_CMP_SELF (a, b);
+
+	/* for nmp_object_id_equal(), all fields of the route are considered here. */
+
 	_CMP_FIELD (a, b, ifindex);
 	_CMP_FIELD_MEMCMP (a, b, network);
 	_CMP_FIELD (a, b, plen);
@@ -3945,6 +3977,32 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route
 	return 0;
 }
 
+guint
+nm_platform_ip6_route_hash (const NMPlatformIP6Route *a)
+{
+	guint h = 0x0569b8f4;
+
+	/* goes together with nm_platform_ip6_route_cmp(). */
+
+	if (a) {
+		h += (h * 33) + ((guint) a->ifindex);
+		h += (h * 33) + ((guint) a->network.s6_addr32[0]);
+		h += (h * 33) + ((guint) a->network.s6_addr32[1]);
+		h += (h * 33) + ((guint) a->network.s6_addr32[2]);
+		h += (h * 33) + ((guint) a->network.s6_addr32[3]);
+		h += (h * 33) + ((guint) a->plen);
+		h += (h * 33) + ((guint) a->metric);
+		h += (h * 33) + ((guint) a->gateway.s6_addr32[0]);
+		h += (h * 33) + ((guint) a->gateway.s6_addr32[1]);
+		h += (h * 33) + ((guint) a->gateway.s6_addr32[2]);
+		h += (h * 33) + ((guint) a->gateway.s6_addr32[3]);
+		h += (h * 33) + ((guint) a->rt_source);
+		h += (h * 33) + ((guint) a->mss);
+		h += (h * 33) + ((guint) a->rt_cloned);
+	}
+	return h;
+}
+
 /**
  * nm_platform_ip_address_cmp_expiry:
  * @a: a NMPlatformIPAddress to compare
@@ -4072,28 +4130,31 @@ nm_platform_netns_push (NMPlatform *platform, NMPNetns **netns)
 static gboolean
 _vtr_v4_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route, gint64 metric)
 {
-	return nm_platform_ip4_route_add (self,
-	                                  ifindex > 0 ? ifindex : route->rx.ifindex,
-	                                  route->rx.rt_source,
-	                                  route->r4.network,
-	                                  route->rx.plen,
-	                                  route->r4.gateway,
-	                                  route->r4.pref_src,
-	                                  metric >= 0 ? (guint32) metric : route->rx.metric,
-	                                  route->rx.mss);
+	NMPlatformIPXRoute r_storage;
+
+	if (   ifindex > 0
+	    || (metric >= 0 && route->metric != metric)) {
+		r_storage.r4 = route->r4;
+		r_storage.rx.ifindex = ifindex;
+		r_storage.rx.metric = metric;
+		route = &r_storage;
+	}
+	return nm_platform_ip4_route_add (self, route);
 }
 
 static gboolean
 _vtr_v6_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route, gint64 metric)
 {
-	return nm_platform_ip6_route_add (self,
-	                                  ifindex > 0 ? ifindex : route->rx.ifindex,
-	                                  route->rx.rt_source,
-	                                  route->r6.network,
-	                                  route->rx.plen,
-	                                  route->r6.gateway,
-	                                  metric >= 0 ? (guint32) metric : route->rx.metric,
-	                                  route->rx.mss);
+	NMPlatformIPXRoute r_storage;
+
+	if (   ifindex > 0
+	    || (metric >= 0 && route->metric != metric)) {
+		r_storage.r6 = route->r6;
+		r_storage.rx.ifindex = ifindex;
+		r_storage.rx.metric = nm_utils_ip6_route_metric_normalize (metric);
+		route = &r_storage;
+	}
+	return nm_platform_ip6_route_add (self, route);
 }
 
 static gboolean
@@ -4103,7 +4164,8 @@ _vtr_v4_route_delete (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *r
 	                                     ifindex > 0 ? ifindex : route->rx.ifindex,
 	                                     route->r4.network,
 	                                     route->rx.plen,
-	                                     route->rx.metric);
+	                                     route->rx.metric,
+	                                     route->r4.gateway);
 }
 
 static gboolean
@@ -4113,7 +4175,8 @@ _vtr_v6_route_delete (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *r
 	                                     ifindex > 0 ? ifindex : route->rx.ifindex,
 	                                     route->r6.network,
 	                                     route->rx.plen,
-	                                     route->rx.metric);
+	                                     route->rx.metric,
+	                                     &route->r6.gateway);
 }
 
 static guint32
@@ -4123,15 +4186,15 @@ _vtr_v4_metric_normalize (guint32 metric)
 }
 
 static gboolean
-_vtr_v4_route_delete_default (NMPlatform *self, int ifindex, guint32 metric)
+_vtr_v4_route_delete_default (NMPlatform *self, int ifindex, guint32 metric, const NMIPAddr *gateway)
 {
-	return nm_platform_ip4_route_delete (self, ifindex, 0, 0, metric);
+	return nm_platform_ip4_route_delete (self, ifindex, 0, 0, metric, gateway ? gateway->addr4 : 0);
 }
 
 static gboolean
-_vtr_v6_route_delete_default (NMPlatform *self, int ifindex, guint32 metric)
+_vtr_v6_route_delete_default (NMPlatform *self, int ifindex, guint32 metric, const NMIPAddr *gateway)
 {
-	return nm_platform_ip6_route_delete (self, ifindex, in6addr_any, 0, metric);
+	return nm_platform_ip6_route_delete (self, ifindex, in6addr_any, 0, metric, gateway ? &gateway->addr6 : NULL);
 }
 
 /******************************************************************/