From 75faf5bb77706267feb0438803c2b318130c9916 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 16 Mar 2017 13:55:14 +0000 Subject: [PATCH 1/4] route-manager: add routine to query route shadowing for a link If a route is shadowed by another route to the same network it's a good indication we're multihoming and want to disable the Strict RP filtering. --- src/nm-route-manager.c | 27 +++++++++++++++++++++++++++ src/nm-route-manager.h | 1 + 2 files changed, 28 insertions(+) diff --git a/src/nm-route-manager.c b/src/nm-route-manager.c index a8778e2a7..13cb03272 100644 --- a/src/nm-route-manager.c +++ b/src/nm-route-manager.c @@ -967,6 +967,33 @@ nm_route_manager_route_flush (NMRouteManager *self, int ifindex) return success; } +/** + * nm_route_manager_ip4_routes_shadowed: + * @ifindex: Interface index + * + * Returns: %TRUE if some other link has a route to the same destination + * with a lower metric. + */ +gboolean +nm_route_manager_ip4_routes_shadowed (NMRouteManager *self, int ifindex) +{ + NMRouteManagerPrivate *priv = NM_ROUTE_MANAGER_GET_PRIVATE (self); + RouteIndex *index = priv->ip4_routes.index; + const NMPlatformIP4Route *route; + guint i; + + for (i = 1; i < index->len; i++) { + route = (const NMPlatformIP4Route *) index->entries[i]; + + if (route->ifindex != ifindex) + continue; + if (_v4_route_dest_cmp (route, (const NMPlatformIP4Route *) index->entries[i - 1]) == 0) + return TRUE; + } + + return FALSE; +} + /*****************************************************************************/ static gboolean diff --git a/src/nm-route-manager.h b/src/nm-route-manager.h index d12f02560..181d794ee 100644 --- a/src/nm-route-manager.h +++ b/src/nm-route-manager.h @@ -38,6 +38,7 @@ gboolean nm_route_manager_ip4_route_sync (NMRouteManager *self, int ifindex, con gboolean nm_route_manager_ip6_route_sync (NMRouteManager *self, int ifindex, const GArray *known_routes, gboolean ignore_kernel_routes, gboolean full_sync); gboolean nm_route_manager_route_flush (NMRouteManager *self, int ifindex); +gboolean nm_route_manager_ip4_routes_shadowed (NMRouteManager *self, int ifindex); void nm_route_manager_ip4_route_register_device_route_purge_list (NMRouteManager *self, GArray *device_route_purge_list); NMRouteManager *nm_route_manager_get (void); From 1b60b76871254464a9a95a7b9e526dac55bd6e9c Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 16 Mar 2017 14:01:42 +0000 Subject: [PATCH 2/4] route-manager: emit a signal when IPv4 routes change The devices will use this to reconsider their RP filtering decisions. --- src/nm-route-manager.c | 16 ++++++++++++++++ src/nm-route-manager.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/src/nm-route-manager.c b/src/nm-route-manager.c index 13cb03272..0a8f1f635 100644 --- a/src/nm-route-manager.c +++ b/src/nm-route-manager.c @@ -63,6 +63,12 @@ typedef struct { /*****************************************************************************/ +enum { + IP4_ROUTES_CHANGED, + LAST_SIGNAL, +}; +static guint signals[LAST_SIGNAL] = { 0 }; + NM_GOBJECT_PROPERTIES_DEFINE_BASE ( PROP_PLATFORM, ); @@ -904,6 +910,9 @@ next: } } + if (vtable->vt->is_ip4 && ipx_routes_changed) + g_signal_emit (self, signals[IP4_ROUTES_CHANGED], 0); + g_free (known_routes_idx); g_free (plat_routes_idx); g_array_unref (plat_routes); @@ -1285,4 +1294,11 @@ nm_route_manager_class_init (NMRouteManagerClass *klass) G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); + + signals[IP4_ROUTES_CHANGED] = + g_signal_new (NM_ROUTE_MANAGER_IP4_ROUTES_CHANGED, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, NULL, NULL, NULL, + G_TYPE_NONE, 0); } diff --git a/src/nm-route-manager.h b/src/nm-route-manager.h index 181d794ee..328fa8cba 100644 --- a/src/nm-route-manager.h +++ b/src/nm-route-manager.h @@ -30,6 +30,8 @@ #define NM_ROUTE_MANAGER_PLATFORM "platform" +#define NM_ROUTE_MANAGER_IP4_ROUTES_CHANGED "ip4-routes-changed" + typedef struct _NMRouteManagerClass NMRouteManagerClass; GType nm_route_manager_get_type (void); From 56e7e657b60407b575dc6bca12fc1417ce125b8a Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 16 Mar 2017 14:51:59 +0000 Subject: [PATCH 3/4] device: add convenience routines for IPv4 sysctls --- src/devices/nm-device.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index de9bf78bc..1ed280034 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -704,6 +704,38 @@ init_ip6_config_dns_priority (NMDevice *self, NMIP6Config *config) /*****************************************************************************/ +static gboolean +nm_device_ipv4_sysctl_set (NMDevice *self, const char *property, const char *value) +{ + NMPlatform *platform = NM_PLATFORM_GET; + gs_free char *value_to_free = NULL; + const char *value_to_set; + + if (value) { + value_to_set = value; + } else { + /* Set to a default value when we've got a NULL @value. */ + value_to_free = nm_platform_sysctl_get (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip4_property_path ("default", property))); + value_to_set = value_to_free; + } + + return nm_platform_sysctl_set (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip4_property_path (nm_device_get_ip_iface (self), property)), + value_to_set); +} + +static guint32 +nm_device_ipv4_sysctl_get_uint32 (NMDevice *self, const char *property, guint32 fallback) +{ + return nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip4_property_path (nm_device_get_ip_iface (self), property)), + 10, + 0, + G_MAXUINT32, + fallback); +} + gboolean nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value) { From cae3cef60fe6b37929e69d103663882274ad46bc Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 16 Mar 2017 14:27:03 +0000 Subject: [PATCH 4/4] device: apply a loose IPv4 rp_filter when it would interfere with multihoming The IPv4 Strict Reverse Path Forwarding filter (RFC 3704) drops legitimate traffic when the same route is present on multiple interfaces, which is a pretty common scenario for IPv4 hosts. In particular, if the traffic is routable via multiple interfaces it drops traffic incoming via the device that has lower metric on the route to the originating network. Among other things, this disrupts existing connection when the user connected to the Internet via Wi-Fi activates a Wired Ethernet connection that also has a default route. Also, the Strict filter (and Reverse Path filters in general) provide practically no value to hosts that have a default route. The solution this patch uses is to detect scenarios where Strict filter is known to interfere and switch to a saner RP filter on the affected links. Routes to the same network on multiple interfaces is a good indication the RP filter would drop the legitimate traffice from the link with a lower metric. This includes the default routes. In such cases, we switch to the Loose Reverse Path Forwarding. This addresses the problems the multihomed hosts face, at the cost of disabling filtering altogether when a default route is present. A Feasible Path Reverse Path Forwarding would address the main problems with the Strict filter, but it's not implemented by the Linux kernel. --- src/devices/nm-device.c | 49 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 1ed280034..017d55a3a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -339,6 +339,8 @@ typedef struct _NMDevicePrivate { NMPlatformIP4Route v4; NMPlatformIP6Route v6; } default_route; + bool v4_has_shadowed_routes; + const char *ip4_rp_filter; /* DHCPv4 tracking */ struct { @@ -2393,6 +2395,45 @@ link_changed_cb (NMPlatform *platform, } } +static void +ip4_rp_filter_update (NMDevice *self) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const char *ip4_rp_filter; + + if ( priv->v4_has_shadowed_routes + || priv->default_route.v4_has) { + if (nm_device_ipv4_sysctl_get_uint32 (self, "rp_filter", 0) != 1) { + /* Don't touch the rp_filter if it's not strict. */ + return; + } + /* Loose rp_filter */ + ip4_rp_filter = "2"; + } else { + /* Default rp_filter */ + ip4_rp_filter = NULL; + } + + if (ip4_rp_filter != priv->ip4_rp_filter) { + nm_device_ipv4_sysctl_set (self, "rp_filter", ip4_rp_filter); + priv->ip4_rp_filter = ip4_rp_filter; + } +} + +static void +ip4_routes_changed_changed_cb (NMRouteManager *route_manager, NMDevice *self) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + int ifindex = nm_device_get_ip_ifindex (self); + + if (nm_device_sys_iface_state_is_external_or_assume (self)) + return; + + priv->v4_has_shadowed_routes = nm_route_manager_ip4_routes_shadowed (route_manager, + ifindex); + ip4_rp_filter_update (self); +} + static void link_changed (NMDevice *self, const NMPlatformLink *pllink) { @@ -9442,6 +9483,8 @@ nm_device_set_ip4_config (NMDevice *self, } nm_default_route_manager_ip4_update_default_route (nm_default_route_manager_get (), self); + if (!nm_device_sys_iface_state_is_external_or_assume (self)) + ip4_rp_filter_update (self); if (has_changes) { NMSettingsConnection *settings_connection; @@ -13375,6 +13418,9 @@ constructed (GObject *object) g_signal_connect (platform, NM_PLATFORM_SIGNAL_IP6_ROUTE_CHANGED, G_CALLBACK (device_ipx_changed), self); g_signal_connect (platform, NM_PLATFORM_SIGNAL_LINK_CHANGED, G_CALLBACK (link_changed_cb), self); + g_signal_connect (nm_route_manager_get (), NM_ROUTE_MANAGER_IP4_ROUTES_CHANGED, + G_CALLBACK (ip4_routes_changed_changed_cb), self); + priv->settings = g_object_ref (NM_SETTINGS_GET); g_assert (priv->settings); @@ -13413,6 +13459,9 @@ dispose (GObject *object) g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (device_ipx_changed), self); g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (link_changed_cb), self); + g_signal_handlers_disconnect_by_func (nm_route_manager_get (), + G_CALLBACK (ip4_routes_changed_changed_cb), self); + g_slist_free_full (priv->arping.dad_list, (GDestroyNotify) nm_arping_manager_destroy); priv->arping.dad_list = NULL;