From 10537699fba124dadad6305cc18bf7452ed1ba8f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Jan 2015 14:50:47 +0100 Subject: [PATCH 1/7] default-route: improve logging format for default route entries The previous syntax (s/S for synced, n/N for never-default) is confusing. Indicate 'never-default' by '0', vs. '1'. Indicated synced/non-synced as '+sync' and '-sync'. (cherry picked from commit df9533d0455eb3e31f7da1ad4728802f7d8dd879) --- src/nm-default-route-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 82cc463518..e2a20df603 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -87,14 +87,14 @@ NM_DEFINE_SINGLETON_GETTER (NMDefaultRouteManager, nm_default_route_manager_get, #define _LOGW(addr_family, ...) _LOG (LOGL_WARN , addr_family, __VA_ARGS__) #define _LOGE(addr_family, ...) _LOG (LOGL_ERR , addr_family, __VA_ARGS__) -#define LOG_ENTRY_FMT "entry[%u/%s:%p:%s:%c%c]" +#define LOG_ENTRY_FMT "entry[%u/%s:%p:%s:%c:%csync]" #define LOG_ENTRY_ARGS(entry_idx, entry) \ (entry_idx), \ NM_IS_DEVICE ((entry)->source.pointer) ? "dev" : "vpn", \ (entry)->source.pointer, \ NM_IS_DEVICE ((entry)->source.pointer) ? nm_device_get_iface ((entry)->source.device) : nm_vpn_connection_get_connection_id ((entry)->source.vpn), \ - ((entry)->never_default ? 'N' : 'n'), \ - ((entry)->synced ? 'S' : 's') + ((entry)->never_default ? '0' : '1'), \ + ((entry)->synced ? '+' : '-') /***********************************************************************************/ From a7742aeeeb940d611634e450540627e99bb376f6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 17 Feb 2015 07:09:57 +0100 Subject: [PATCH 2/7] trivial: fix spelling in comment (cherry picked from commit 9d6b67012c2bd0cbdcb8af715b5388b17e45d8a2) --- src/nm-default-route-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index e2a20df603..8d58e00147 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -127,7 +127,7 @@ typedef struct { * to indicate that the ifindex is managed but has no default-route. * Missing entries also indicate that a certain ifindex has no default-route. * The difference is that missing entries are considered assumed while on - * (synced && never_default) entires the absence of the default route + * (synced && never_default) entries the absence of the default route * is enforced. NMDefaultRouteManager will actively remove any default * route on such ifindexes. * Also, for VPN sources in addition we track them so that a never-default @@ -301,7 +301,7 @@ _platform_route_sync_flush (const VTableIP *vtable, NMDefaultRouteManager *self, route = _vt_route_index (vtable, routes, i); - /* look at all entires and see if the route for this ifindex pair is + /* look at all entries and see if the route for this ifindex pair is * a known entry. */ for (j = 0; j < entries->len; j++) { Entry *e = g_ptr_array_index (entries, j); From a022cf7c76ca5c896c01e52aa4b3c3f468c532fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Fri, 17 Apr 2015 09:07:15 +0200 Subject: [PATCH 3/7] core: (trivial) fix spelling in comments (cherry picked from commit 8257940606b1ad3cd333974a0c5c52afd4281a8b) --- src/nm-default-route-manager.c | 8 ++++---- src/nm-policy.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 8d58e00147..de71f4a49f 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -540,7 +540,7 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c } else { if (!_vt_routes_has_entry (vtable, routes, entry)) { g_array_append_val (changed_metrics, entry->effective_metric); - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": readd route %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": read route %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) entry->effective_metric, (guint) entry->effective_metric); } @@ -1229,10 +1229,10 @@ _resync_idle_reschedule (NMDefaultRouteManager *self) { NMDefaultRouteManagerPrivate *priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self); - /* since we react on external changes and readd/remove default routes for - * the interfaces we manage, there could be the erronous situation where two applications + /* since we react on external changes and read/remove default routes for + * the interfaces we manage, there could be the erroneous situation where two applications * fight over a certain default route. - * Avoid this, by increasingly wait longer to touch the system (backoff wait time). */ + * Avoid this, by increasingly wait longer to touch tehe system (backoff wait time). */ if (priv->resync.backoff_wait_time_ms == 0) { /* for scheduling idle, always reschedule (to process all other events first) */ diff --git a/src/nm-policy.c b/src/nm-policy.c index 87fcdcc925..8a573c8b34 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -170,7 +170,7 @@ _set_hostname (NMPolicy *policy, * there was no valid hostname to start with. */ - /* Clear lookup adresses if we have a hostname, so that we don't + /* Clear lookup addresses if we have a hostname, so that we don't * restart the reverse lookup thread later. */ if (new_hostname) From 9bac91eb6919171ac0ef74226aab4afdd258f442 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 Apr 2015 11:27:49 +0200 Subject: [PATCH 4/7] trivial: fix spelling in comments (cherry picked from commit ccba1b1e2d1fb2a23dd9e86d8ba44f7a3287cafb) --- src/nm-default-route-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index de71f4a49f..4caf4672d7 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -540,7 +540,7 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c } else { if (!_vt_routes_has_entry (vtable, routes, entry)) { g_array_append_val (changed_metrics, entry->effective_metric); - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": read route %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": re-add route %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) entry->effective_metric, (guint) entry->effective_metric); } @@ -1229,10 +1229,10 @@ _resync_idle_reschedule (NMDefaultRouteManager *self) { NMDefaultRouteManagerPrivate *priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self); - /* since we react on external changes and read/remove default routes for + /* since we react on external changes and re-add/remove default routes for * the interfaces we manage, there could be the erroneous situation where two applications * fight over a certain default route. - * Avoid this, by increasingly wait longer to touch tehe system (backoff wait time). */ + * Avoid this, by increasingly wait longer to touch the system (backoff wait time). */ if (priv->resync.backoff_wait_time_ms == 0) { /* for scheduling idle, always reschedule (to process all other events first) */ From 76e5d55a9892adefa760469d6b23f2b03fe166cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 27 May 2015 11:34:31 +0200 Subject: [PATCH 5/7] 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(). (cherry picked from commit 49227a07f3e71033f7d10023cfe5a39334f335cb) --- src/devices/nm-device.c | 4 ++-- src/nm-default-route-manager.c | 36 ++++++++++++++++++++++------------ src/nm-default-route-manager.h | 4 ++-- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 625ad3ee33..398d9e05e4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3134,7 +3134,7 @@ ip4_config_merge_and_apply (NMDevice *self, * tweak the effective metric). */ if ( !assumed - && nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection)) { + && nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) { guint32 gateway = 0; priv->default_route.v4_is_assumed = FALSE; @@ -3697,7 +3697,7 @@ ip6_config_merge_and_apply (NMDevice *self, * tweak the effective metric). */ if ( !assumed - && nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection)) { + && nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection, NULL)) { const struct in6_addr *gateway = NULL; priv->default_route.v6_is_assumed = FALSE; 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 7be6d9644011918ec2f1cdc9f440890adb3f7c4b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Jun 2015 18:15:38 +0200 Subject: [PATCH 6/7] default-route-manager: disambiguate logging statements (cherry picked from commit 8da17c3a19d48b25eda7c2e9b31a2b1b962ae20f) --- src/nm-default-route-manager.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 1019807c45..9312d30522 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -524,23 +524,23 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c * or none. Hence, we only have to remember what is going to change. */ g_array_append_val (changed_metrics, expected_metric); if (old_entry) { - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": update %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": sync:update %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) old_entry->effective_metric, (guint) expected_metric); } else { - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": add %s (%u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": sync:add %s (%u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) expected_metric); } } else if (entry->effective_metric != expected_metric) { g_array_append_val (changed_metrics, entry->effective_metric); g_array_append_val (changed_metrics, expected_metric); - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": resync metric %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": sync:metric %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) entry->effective_metric, (guint) expected_metric); } else { if (!_vt_routes_has_entry (vtable, routes, entry)) { g_array_append_val (changed_metrics, entry->effective_metric); - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": re-add route %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": sync:re-add %s (%u -> %u)", LOG_ENTRY_ARGS (i, entry), vtable->vt->route_to_string (&entry->route), (guint) entry->effective_metric, (guint) entry->effective_metric); } @@ -605,10 +605,11 @@ _entry_at_idx_update (const VTableIP *vtable, NMDefaultRouteManager *self, guint if (!entry->synced && !entry->never_default) entry->effective_metric = entry->route.rx.metric; - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": %s %s", + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": %s %s (%"G_GUINT32_FORMAT")", LOG_ENTRY_ARGS (entry_idx, entry), - old_entry ? "update" : "add", - vtable->vt->route_to_string (&entry->route)); + old_entry ? "record:update" : "record:add ", + vtable->vt->route_to_string (&entry->route), + entry->effective_metric); g_ptr_array_sort_with_data (entries, _sort_entries_cmp, NULL); @@ -628,7 +629,7 @@ _entry_at_idx_remove (const VTableIP *vtable, NMDefaultRouteManager *self, guint entry = g_ptr_array_index (entries, entry_idx); - _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": remove %s (%u)", LOG_ENTRY_ARGS (entry_idx, entry), + _LOGD (vtable->vt->addr_family, LOG_ENTRY_FMT": record:remove %s (%u)", LOG_ENTRY_ARGS (entry_idx, entry), vtable->vt->route_to_string (&entry->route), (guint) entry->effective_metric); /* Remove the entry from the list (but don't free it yet) */ From 27edd58bd441e423c93380af5e88cd62533ca08f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jun 2015 17:45:01 +0200 Subject: [PATCH 7/7] default-route-manager: fix syncing routes to consider non-synced routes We already protected route-metrics that are configured as default-routes in platform. For most cases, that list is identical to our internal list of non-synced routes. But if for some reason that is not the case, we must also protect the metric of routs that we currently track as "non-synced". (cherry picked from commit 6849050ad914b046e55bcb4eaad826000fef620f) --- src/nm-default-route-manager.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 9312d30522..4beffcf7b1 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -411,6 +411,30 @@ _get_assumed_interface_metrics (const VTableIP *vtable, NMDefaultRouteManager *s g_hash_table_add (result, GUINT_TO_POINTER (vtable->vt->metric_normalize (route->metric))); } + /* also add all non-synced metrics from our entries list. We might have there some metrics that + * we track as non-synced but that are no longer part of platform routes. Anyway, for now + * we still want to treat them as assumed. */ + for (i = 0; i < entries->len; i++) { + gboolean ifindex_has_synced_entry = FALSE; + Entry *e_i = g_ptr_array_index (entries, i); + + if (e_i->synced) + continue; + + for (j = 0; j < entries->len; j++) { + Entry *e_j = g_ptr_array_index (entries, j); + + if ( j != i + && (e_j->synced && e_j->route.rx.ifindex == e_i->route.rx.ifindex)) { + ifindex_has_synced_entry = TRUE; + break; + } + } + + if (!ifindex_has_synced_entry) + g_hash_table_add (result, GUINT_TO_POINTER (vtable->vt->metric_normalize (e_i->route.rx.metric))); + } + return result; }