From 5849c97c0368d51ea35590736aaf40ee522fed0d 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. Don't replace the device route if we have any indication that a conflict could arise. https://bugzilla.gnome.org/show_bug.cgi?id=723178 --- src/platform/nm-fake-platform.c | 8 ++++++ src/platform/nm-linux-platform.c | 61 +++++++++++++++++++++++++++++++++------ src/platform/nm-platform.c | 62 ++++++++++++++++++++++++---------------- src/platform/nm-platform.h | 4 +++ 4 files changed, 102 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 533b7a5e4a..a5eb7d4ed3 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -982,6 +982,12 @@ ip6_address_exists (NMPlatform *platform, int ifindex, struct in6_addr addr, int return FALSE; } +static gboolean +ip4_check_reinstall_device_route (NMPlatform *platform, int ifindex, const NMPlatformIP4Address *address, guint32 device_route_metric) +{ + return FALSE; +} + /******************************************************************/ static GArray * @@ -1345,6 +1351,8 @@ nm_fake_platform_class_init (NMFakePlatformClass *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-linux-platform.c b/src/platform/nm-linux-platform.c index 954edec3b7..7ced0ba77b 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,53 @@ 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; + + 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 +3686,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 +3712,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 +3741,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 +3848,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 +4405,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 04ac4ce089..14fc44c5fa 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1732,6 +1732,25 @@ _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) +{ + g_return_val_if_fail (address, FALSE); + + if ( ifindex <= 0 + || address->plen <= 0 + || address->plen >= 32) + return FALSE; + + if (device_route_metric == NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE) { + /* The automatically added route would be already our desired priority. + * Nothing to do. */ + return FALSE; + } + + return klass->ip4_check_reinstall_device_route (platform, ifindex, address, device_route_metric); +} + /** * nm_platform_ip4_address_sync: * @ifindex: Interface index @@ -1771,40 +1790,33 @@ 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 (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, would replace the other. This is avoided + * by the above nm_platform_ip4_check_reinstall_device_route() check. + */ + network = nm_utils_ip4_address_clear_host_address (known_address->address, known_address->plen); + (void) nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_KERNEL, network, known_address->plen, + 0, known_address->address, device_route_metric, 0); + (void) 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); - (void) nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_KERNEL, network, known_address->plen, - 0, known_address->address, device_route_metric, 0); - (void) 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