From 6647d4d2f54d0753b94b40a9c381e8a789a83540 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 26 Nov 2014 00:10:04 +0100 Subject: platform: avoid conflicts when reinstalling the device-route 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 --- src/platform/nm-linux-platform.c | 68 +++++++++++++++++++++++++++++++++++----- src/platform/nm-platform.c | 55 ++++++++++++++++++-------------- src/platform/nm-platform.h | 4 +++ 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, -- cgit v1.2.1