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, 70 insertions, 49 deletions

diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 11c2e965b2..885b066b17 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -304,6 +304,16 @@ _support_user_ipv6ll_detect (struct nlattr **tb)
  ******************************************************************/
 
 static void
+_ip6_route_normalize_metric (const NMPlatformIP6Route **route, NMPlatformIP6Route *r_storage)
+{
+	if ((*route)->metric != nm_utils_ip6_route_metric_normalize ((*route)->metric)) {
+		*r_storage = **route;
+		r_storage->metric = nm_utils_ip6_route_metric_normalize ((*route)->metric);
+		*route = r_storage;
+	}
+}
+
+static void
 clear_host_address (int family, const void *network, guint8 plen, void *dst)
 {
 	g_return_if_fail (network);
@@ -3876,6 +3886,16 @@ do_add_addrroute (NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *
 
 	event_handler_read_netlink (platform, FALSE);
 
+	if (NM_IN_SET (NMP_OBJECT_GET_TYPE (obj_id), NMP_OBJECT_TYPE_IP4_ROUTE, NMP_OBJECT_TYPE_IP6_ROUTE)) {
+		/* for routes, we set the flags to NLM_F_CREATE | NLM_F_APPEND. This will fail with EEXIST
+		 * if the route already exists, which is just splendind.
+		 *
+		 * So, first check the cache whether we expect the route to exist, and if so, set the
+		 * flags to REPLACE. */
+		if (nmp_cache_lookup_obj (priv->cache, obj_id))
+			nlmsg_hdr (nlmsg)->nlmsg_flags |= NLM_F_REPLACE;
+	}
+
 	nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result, NULL);
 	if (nle < 0) {
 		_LOGE ("do-add-%s[%s]: failure sending netlink request \"%s\" (%d)",
@@ -5548,65 +5568,64 @@ ip6_route_get_all (NMPlatform *platform, int ifindex, NMPlatformGetRouteFlags fl
 }
 
 static gboolean
-ip4_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source,
-               in_addr_t network, guint8 plen, in_addr_t gateway,
-               in_addr_t pref_src, guint32 metric, guint32 mss)
+ip4_route_add (NMPlatform *platform, const NMPlatformIP4Route *route)
 {
 	NMPObject obj_id;
 	nm_auto_nlmsg struct nl_msg *nlmsg = NULL;
 
 	nlmsg = _nl_msg_new_route (RTM_NEWROUTE,
-	                           NLM_F_CREATE | NLM_F_REPLACE,
+	                           NLM_F_CREATE | NLM_F_APPEND,
 	                           AF_INET,
-	                           ifindex,
-	                           source,
-	                           gateway ? RT_SCOPE_UNIVERSE : RT_SCOPE_LINK,
-	                           &network,
-	                           plen,
-	                           &gateway,
-	                           metric,
-	                           mss,
-	                           pref_src ? &pref_src : NULL);
-
-	nmp_object_stackinit_id_ip4_route (&obj_id, ifindex, network, plen, metric);
+	                           route->ifindex,
+	                           route->rt_source,
+	                           route->gateway ? RT_SCOPE_UNIVERSE : RT_SCOPE_LINK,
+	                           &route->network,
+	                           route->plen,
+	                           &route->gateway,
+	                           route->metric,
+	                           route->mss,
+	                           route->pref_src ? &route->pref_src : NULL);
+
+	nmp_object_stackinit_id_ip4_route (&obj_id, route);
 	return do_add_addrroute (platform, &obj_id, nlmsg);
 }
 
 static gboolean
-ip6_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source,
-               struct in6_addr network, guint8 plen, struct in6_addr gateway,
-               guint32 metric, guint32 mss)
+ip6_route_add (NMPlatform *platform, const NMPlatformIP6Route *route)
 {
 	NMPObject obj_id;
 	nm_auto_nlmsg struct nl_msg *nlmsg = NULL;
+	NMPlatformIP6Route r_storage;
+
+	_ip6_route_normalize_metric (&route, &r_storage);
 
 	nlmsg = _nl_msg_new_route (RTM_NEWROUTE,
-	                           NLM_F_CREATE | NLM_F_REPLACE,
+	                           NLM_F_CREATE | NLM_F_APPEND,
 	                           AF_INET6,
-	                           ifindex,
-	                           source,
-	                           !IN6_IS_ADDR_UNSPECIFIED (&gateway) ? RT_SCOPE_UNIVERSE : RT_SCOPE_LINK,
-	                           &network,
-	                           plen,
-	                           &gateway,
-	                           metric,
-	                           mss,
+	                           route->ifindex,
+	                           route->rt_source,
+	                           !IN6_IS_ADDR_UNSPECIFIED (&route->gateway) ? RT_SCOPE_UNIVERSE : RT_SCOPE_LINK,
+	                           &route->network,
+	                           route->plen,
+	                           &route->gateway,
+	                           route->metric,
+	                           route->mss,
 	                           NULL);
 
-	nmp_object_stackinit_id_ip6_route (&obj_id, ifindex, &network, plen, metric);
+	nmp_object_stackinit_id_ip6_route (&obj_id, route);
 	return do_add_addrroute (platform, &obj_id, nlmsg);
 }
 
 static gboolean
-ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, guint8 plen, guint32 metric)
+ip4_route_delete (NMPlatform *platform, const NMPlatformIP4Route *route)
 {
 	NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform);
 	nm_auto_nlmsg struct nl_msg *nlmsg = NULL;
 	NMPObject obj_id;
 
-	nmp_object_stackinit_id_ip4_route (&obj_id, ifindex, network, plen, metric);
+	nmp_object_stackinit_id_ip4_route (&obj_id, route);
 
-	if (metric == 0) {
+	if (route->metric == 0) {
 		/* Deleting an IPv4 route with metric 0 does not only delete an exectly matching route.
 		 * If no route with metric 0 exists, it might delete another route to the same destination.
 		 * For nm_platform_ip4_route_delete() we don't want this semantic.
@@ -5638,13 +5657,13 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, guint8 p
 	nlmsg = _nl_msg_new_route (RTM_DELROUTE,
 	                           0,
 	                           AF_INET,
-	                           ifindex,
+	                           route->ifindex,
 	                           NM_IP_CONFIG_SOURCE_UNKNOWN,
 	                           RT_SCOPE_NOWHERE,
-	                           &network,
-	                           plen,
-	                           NULL,
-	                           metric,
+	                           &route->network,
+	                           route->plen,
+	                           &route->gateway,
+	                           route->metric,
 	                           0,
 	                           NULL);
 	if (!nlmsg)
@@ -5654,40 +5673,41 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, guint8 p
 }
 
 static gboolean
-ip6_route_delete (NMPlatform *platform, int ifindex, struct in6_addr network, guint8 plen, guint32 metric)
+ip6_route_delete (NMPlatform *platform, const NMPlatformIP6Route *route)
 {
 	nm_auto_nlmsg struct nl_msg *nlmsg = NULL;
 	NMPObject obj_id;
+	NMPlatformIP6Route r_storage;
 
-	metric = nm_utils_ip6_route_metric_normalize (metric);
+	_ip6_route_normalize_metric (&route, &r_storage);
 
 	nlmsg = _nl_msg_new_route (RTM_DELROUTE,
 	                           0,
 	                           AF_INET6,
-	                           ifindex,
+	                           route->ifindex,
 	                           NM_IP_CONFIG_SOURCE_UNKNOWN,
 	                           RT_SCOPE_NOWHERE,
-	                           &network,
-	                           plen,
-	                           NULL,
-	                           metric,
+	                           &route->network,
+	                           route->plen,
+	                           &route->gateway,
+	                           route->metric,
 	                           0,
 	                           NULL);
 	if (!nlmsg)
 		return FALSE;
 
-	nmp_object_stackinit_id_ip6_route (&obj_id, ifindex, &network, plen, metric);
+	nmp_object_stackinit_id_ip6_route (&obj_id, route);
 
 	return do_delete_object (platform, &obj_id, nlmsg);
 }
 
 static const NMPlatformIP4Route *
-ip4_route_get (NMPlatform *platform, int ifindex, in_addr_t network, guint8 plen, guint32 metric)
+ip4_route_get (NMPlatform *platform, const NMPlatformIP4Route *route)
 {
 	NMPObject obj_id;
 	const NMPObject *obj;
 
-	nmp_object_stackinit_id_ip4_route (&obj_id, ifindex, network, plen, metric);
+	nmp_object_stackinit_id_ip4_route (&obj_id, route);
 	obj = nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, &obj_id);
 	if (nmp_object_is_visible (obj))
 		return &obj->ip4_route;
@@ -5695,14 +5715,15 @@ ip4_route_get (NMPlatform *platform, int ifindex, in_addr_t network, guint8 plen
 }
 
 static const NMPlatformIP6Route *
-ip6_route_get (NMPlatform *platform, int ifindex, struct in6_addr network, guint8 plen, guint32 metric)
+ip6_route_get (NMPlatform *platform, const NMPlatformIP6Route *route)
 {
 	NMPObject obj_id;
 	const NMPObject *obj;
+	NMPlatformIP6Route r_storage;
 
-	metric = nm_utils_ip6_route_metric_normalize (metric);
+	_ip6_route_normalize_metric (&route, &r_storage);
 
-	nmp_object_stackinit_id_ip6_route (&obj_id, ifindex, &network, plen, metric);
+	nmp_object_stackinit_id_ip6_route (&obj_id, route);
 	obj = nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, &obj_id);
 	if (nmp_object_is_visible (obj))
 		return &obj->ip6_route;