From 49227a07f3e71033f7d10023cfe5a39334f335cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 27 May 2015 11:34:31 +0200 Subject: [PATCH 1/3] default-route: add @out_is_never_default argument to has_default_route() Also accept a NULL connection in nm_default_route_manager_ip4_connection_has_default_route() and nm_default_route_manager_ip6_connection_has_default_route(). --- src/devices/nm-device.c | 6 ++---- src/nm-default-route-manager.c | 36 ++++++++++++++++++++++------------ src/nm-default-route-manager.h | 4 ++-- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 043d6dbe87..0a17637e68 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3127,8 +3127,7 @@ ip4_config_merge_and_apply (NMDevice *self, * configured. */ priv->default_route.v4_is_assumed = FALSE; - if ( !connection - || !nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection)) + if (!nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) goto END_ADD_DEFAULT_ROUTE; if (!nm_ip4_config_get_num_addresses (composite)) { @@ -3709,8 +3708,7 @@ ip6_config_merge_and_apply (NMDevice *self, * configured. */ priv->default_route.v6_is_assumed = FALSE; - if ( !connection - || !nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection)) + if (!nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) goto END_ADD_DEFAULT_ROUTE; if (!nm_ip6_config_get_num_addresses (composite)) { diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 4caf4672d7..1019807c45 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -832,48 +832,60 @@ nm_default_route_manager_ip6_update_default_route (NMDefaultRouteManager *self, /***********************************************************************************/ static gboolean -_ipx_connection_has_default_route (const VTableIP *vtable, NMDefaultRouteManager *self, NMConnection *connection) +_ipx_connection_has_default_route (const VTableIP *vtable, NMDefaultRouteManager *self, NMConnection *connection, gboolean *out_is_never_default) { const char *method; NMSettingIPConfig *s_ip; + gboolean is_never_default = FALSE; + gboolean has_default_route = FALSE; g_return_val_if_fail (NM_IS_DEFAULT_ROUTE_MANAGER (self), FALSE); - g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); + + if (!connection) + goto out; if (vtable->vt->is_ip4) s_ip = nm_connection_get_setting_ip4_config (connection); else s_ip = nm_connection_get_setting_ip6_config (connection); - if (!s_ip || nm_setting_ip_config_get_never_default (s_ip)) - return FALSE; + if (!s_ip) + goto out; + if (nm_setting_ip_config_get_never_default (s_ip)) { + is_never_default = TRUE; + goto out; + } if (vtable->vt->is_ip4) { method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP4_CONFIG); if ( !method || !strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) || !strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL)) - return FALSE; + goto out; } else { method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG); if ( !method || !strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) || !strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL)) - return FALSE; + goto out; } - return TRUE; + has_default_route = TRUE; +out: + if (out_is_never_default) + *out_is_never_default = is_never_default; + return has_default_route; } gboolean -nm_default_route_manager_ip4_connection_has_default_route (NMDefaultRouteManager *self, NMConnection *connection) +nm_default_route_manager_ip4_connection_has_default_route (NMDefaultRouteManager *self, NMConnection *connection, gboolean *out_is_never_default) { - return _ipx_connection_has_default_route (&vtable_ip4, self, connection); + return _ipx_connection_has_default_route (&vtable_ip4, self, connection, out_is_never_default); } gboolean -nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager *self, NMConnection *connection) +nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager *self, NMConnection *connection, gboolean *out_is_never_default) { - return _ipx_connection_has_default_route (&vtable_ip6, self, connection); + return _ipx_connection_has_default_route (&vtable_ip6, self, connection, out_is_never_default); } /***********************************************************************************/ @@ -972,7 +984,7 @@ _ipx_get_best_activating_device (const VTableIP *vtable, NMDefaultRouteManager * || state >= NM_DEVICE_STATE_DEACTIVATING) continue; - if (!_ipx_connection_has_default_route (vtable, self, nm_device_get_connection (device))) + if (!_ipx_connection_has_default_route (vtable, self, nm_device_get_connection (device), NULL)) continue; prio = nm_device_get_ip4_route_metric (device); diff --git a/src/nm-default-route-manager.h b/src/nm-default-route-manager.h index d8e422735e..7fc27bbd1c 100644 --- a/src/nm-default-route-manager.h +++ b/src/nm-default-route-manager.h @@ -51,8 +51,8 @@ NMDefaultRouteManager *nm_default_route_manager_get (void); void nm_default_route_manager_ip4_update_default_route (NMDefaultRouteManager *manager, gpointer source); void nm_default_route_manager_ip6_update_default_route (NMDefaultRouteManager *manager, gpointer source); -gboolean nm_default_route_manager_ip4_connection_has_default_route (NMDefaultRouteManager *manager, NMConnection *connection); -gboolean nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager *manager, NMConnection *connection); +gboolean nm_default_route_manager_ip4_connection_has_default_route (NMDefaultRouteManager *manager, NMConnection *connection, gboolean *out_is_never_default); +gboolean nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager *manager, NMConnection *connection, gboolean *out_is_never_default); NMDevice *nm_default_route_manager_ip4_get_best_device (NMDefaultRouteManager *manager, const GSList *devices, gboolean fully_activated, NMDevice *preferred_device); NMDevice *nm_default_route_manager_ip6_get_best_device (NMDefaultRouteManager *manager, const GSList *devices, gboolean fully_activated, NMDevice *preferred_device); From 98e50e358bde29a78d7a9547a21736aa143cd04b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2015 21:21:49 +0200 Subject: [PATCH 2/3] default-route: for devices with 'never-default' enforce the default-route only once Since da708059dabb2854d11eed1a403398327b31535b, we would pickup the default-route as configured externally, except at those moments when NM re-applys the IP configuration of the interface, such as during a DHCP lease. That allows the user to add/remove the default-route externally (iproute). But still, at random times (DHCP lease), we will revert those external changes. Extend this, that if the connection is explicitly configured as 'never-default=yes', that it tells NM not to interfere with externally added default-routes on this device. That means, NM will only remove any preexisting default-routes when configuring the device a first time. On any later attempts, NM will assume whatever is configured there. That makes sense because the user indicated not wanting NM to manage a default-route on that device, so if something externally added a default-route, assume that is what the user wants. This only affects non-assumed connections, with 'never-default=yes'. https://bugzilla.redhat.com/show_bug.cgi?id=1205405 --- src/devices/nm-device.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 0a17637e68..f8fcf70ed4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -257,9 +257,11 @@ typedef struct { struct { gboolean v4_has; gboolean v4_is_assumed; + gboolean v4_configure_first_time; NMPlatformIP4Route v4; gboolean v6_has; gboolean v6_is_assumed; + gboolean v6_configure_first_time; NMPlatformIP6Route v6; } default_route; @@ -3065,6 +3067,7 @@ ip4_config_merge_and_apply (NMDevice *self, gboolean has_direct_route; const guint32 default_route_metric = nm_device_get_ip4_route_metric (self); guint32 gateway; + gboolean connection_has_default_route, connection_is_never_default; /* Merge all the configs into the composite config */ if (config) { @@ -3122,12 +3125,24 @@ ip4_config_merge_and_apply (NMDevice *self, if (nm_device_uses_assumed_connection (self)) goto END_ADD_DEFAULT_ROUTE; + connection_has_default_route + = nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), + connection, &connection_is_never_default); + + if ( !priv->default_route.v4_configure_first_time + && connection_is_never_default) { + /* If the connection is explicitly configured as never-default, we enforce the (absense of the) + * default-route only once. That allows the user to configure a connection as never-default, + * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + goto END_ADD_DEFAULT_ROUTE; + } /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ + priv->default_route.v4_configure_first_time = FALSE; priv->default_route.v4_is_assumed = FALSE; - if (!nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) + if (!connection_has_default_route) goto END_ADD_DEFAULT_ROUTE; if (!nm_ip4_config_get_num_addresses (composite)) { @@ -3647,6 +3662,7 @@ ip6_config_merge_and_apply (NMDevice *self, NMIP6Config *composite; gboolean has_direct_route; const struct in6_addr *gateway; + gboolean connection_has_default_route, connection_is_never_default; /* If no config was passed in, create a new one */ composite = nm_ip6_config_new (nm_device_get_ip_ifindex (self)); @@ -3703,12 +3719,24 @@ ip6_config_merge_and_apply (NMDevice *self, if (nm_device_uses_assumed_connection (self)) goto END_ADD_DEFAULT_ROUTE; + connection_has_default_route + = nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), + connection, &connection_is_never_default); + + if ( !priv->default_route.v6_configure_first_time + && connection_is_never_default) { + /* If the connection is explicitly configured as never-default, we enforce the (absence of the) + * default-route only once. That allows the user to configure a connection as never-default, + * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + goto END_ADD_DEFAULT_ROUTE; + } /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ + priv->default_route.v6_configure_first_time = FALSE; priv->default_route.v6_is_assumed = FALSE; - if (!nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) + if (!connection_has_default_route) goto END_ADD_DEFAULT_ROUTE; if (!nm_ip6_config_get_num_addresses (composite)) { @@ -7539,8 +7567,10 @@ _cleanup_generic_post (NMDevice *self, gboolean deconfigure) priv->default_route.v4_has = FALSE; priv->default_route.v4_is_assumed = TRUE; + priv->default_route.v4_configure_first_time = TRUE; priv->default_route.v6_has = FALSE; priv->default_route.v6_is_assumed = TRUE; + priv->default_route.v6_configure_first_time = TRUE; nm_default_route_manager_ip4_update_default_route (nm_default_route_manager_get (), self); nm_default_route_manager_ip6_update_default_route (nm_default_route_manager_get (), self); @@ -8490,7 +8520,9 @@ nm_device_init (NMDevice *self) priv->ip6_saved_properties = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free); priv->default_route.v4_is_assumed = TRUE; + priv->default_route.v4_configure_first_time = TRUE; priv->default_route.v6_is_assumed = TRUE; + priv->default_route.v6_configure_first_time = TRUE; } static GObject* From d51975ed921a5876b76e081b8f3df4e2ca1f1ca9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 27 May 2015 11:52:39 +0200 Subject: [PATCH 3/3] default-route: also configure default-routes for assumed connections Previously for assumed connections we would never configure a default route. That has serious problems for example in the following two scenarios: - the default-route might have a limited lifetime from a previous SLAAC/accept_ra setting. In this case, once we assume the connection we must also ensure that we extend the lifetime of the default route. - the gateway could be received via DHCP/RA and it might change. If we ignore default-routes for assumed connection we miss that change. The problem is that the notion of "assumed connection" wrongly combines two conflicting goals (related bug bgo#746440): a) have an external device that is entirely unmanged by NM. b) do a seamless takeover of a previously managed connection at start, but still fully manage. This patch changes the handling of default-routes towards meaning b). https://bugzilla.redhat.com/show_bug.cgi?id=1224291 --- src/devices/nm-device.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f8fcf70ed4..35418ccb17 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3093,7 +3093,7 @@ ip4_config_merge_and_apply (NMDevice *self, if (priv->wwan_ip4_config) nm_ip4_config_merge (composite, priv->wwan_ip4_config); - /* Merge user overrides into the composite config. For assumed connection, + /* Merge user overrides into the composite config. For assumed connections, * con_ip4_config is empty. */ if (priv->con_ip4_config) nm_ip4_config_merge (composite, priv->con_ip4_config); @@ -3122,14 +3122,12 @@ ip4_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } - if (nm_device_uses_assumed_connection (self)) - goto END_ADD_DEFAULT_ROUTE; - connection_has_default_route = nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection, &connection_is_never_default); if ( !priv->default_route.v4_configure_first_time + && !nm_device_uses_assumed_connection (self) && connection_is_never_default) { /* If the connection is explicitly configured as never-default, we enforce the (absense of the) * default-route only once. That allows the user to configure a connection as never-default, @@ -3137,6 +3135,11 @@ ip4_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } + /* At this point, we treat assumed and non-assumed connections alike. + * For assumed connections we do that because we still manage RA and DHCP + * leases for them, so we must extend/update the default route on commits. + */ + /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ priv->default_route.v4_configure_first_time = FALSE; @@ -3716,14 +3719,12 @@ ip6_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } - if (nm_device_uses_assumed_connection (self)) - goto END_ADD_DEFAULT_ROUTE; - connection_has_default_route = nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection, &connection_is_never_default); if ( !priv->default_route.v6_configure_first_time + && !nm_device_uses_assumed_connection (self) && connection_is_never_default) { /* If the connection is explicitly configured as never-default, we enforce the (absence of the) * default-route only once. That allows the user to configure a connection as never-default, @@ -3731,6 +3732,11 @@ ip6_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } + /* At this point, we treat assumed and non-assumed connections alike. + * For assumed connections we do that because we still manage RA and DHCP + * leases for them, so we must extend/update the default route on commits. + */ + /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ priv->default_route.v6_configure_first_time = FALSE;