From 19133b08daf97fedeeb3bd9d76bb8e58f87951a4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 23 Jun 2016 22:46:02 +0200 Subject: [PATCH 1/4] vpn: dispatch pre-up scripts only once If the plugin sends a new configuration when the connection is already activated the state should not go back to PRE_UP since this causes dispatcher scripts to run again. --- src/vpn-manager/nm-vpn-connection.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index b2aa8f6bc6..84021d3c28 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1082,7 +1082,8 @@ nm_vpn_connection_apply_config (NMVpnConnection *self) nm_default_route_manager_ip6_update_default_route (priv->default_route_manager, self); _LOGI ("VPN connection: (IP Config Get) complete"); - _set_vpn_state (self, STATE_PRE_UP, NM_VPN_CONNECTION_STATE_REASON_NONE, FALSE); + if (priv->vpn_state < STATE_PRE_UP) + _set_vpn_state (self, STATE_PRE_UP, NM_VPN_CONNECTION_STATE_REASON_NONE, FALSE); return TRUE; } From 641e8b00fea2503acbcd5c8c942ed751a6607469 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 23 Jun 2016 22:54:25 +0200 Subject: [PATCH 2/4] vpn: reuse existing ip_config objects when config gets updated Previously we created a new NMIP[46]Config object to replace the previous one every time the plugin reported a new IP configuration through the Ip[46]Config signal, but the old configuration was not removed from the DNS manager and would become stale. The interaction with DNS manager is handled by NMPolicy, which only reacts to changes in connection status, so it's not easy to have the configuration removed from DNS while keeping the connection state ACTIVATED. A cleaner solutions is to avoid creating a new IP configuration object and reuse the existing one if possible. The side effect is that the D-Bus path of the object will not change, which seems also positive. https://bugzilla.redhat.com/show_bug.cgi?id=1348901 --- src/vpn-manager/nm-vpn-connection.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 84021d3c28..dc8c0e8444 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1470,10 +1470,15 @@ nm_vpn_connection_ip4_config_get (NMVpnConnection *self, GVariant *dict) nm_connection_get_setting_ip4_config (_get_applied_connection (self)), route_metric); - nm_exported_object_clear_and_unexport (&priv->ip4_config); - priv->ip4_config = config; - nm_exported_object_export (NM_EXPORTED_OBJECT (config)); - g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP4_CONFIG); + if (priv->ip4_config) { + nm_ip4_config_replace (priv->ip4_config, config, NULL); + g_object_unref (config); + } else { + priv->ip4_config = config; + nm_exported_object_export (NM_EXPORTED_OBJECT (config)); + g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP4_CONFIG); + } + nm_vpn_connection_config_maybe_complete (self, TRUE); } @@ -1607,10 +1612,15 @@ next: nm_connection_get_setting_ip6_config (_get_applied_connection (self)), route_metric); - nm_exported_object_clear_and_unexport (&priv->ip6_config); - priv->ip6_config = config; - nm_exported_object_export (NM_EXPORTED_OBJECT (config)); - g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP6_CONFIG); + if (priv->ip6_config) { + nm_ip6_config_replace (priv->ip6_config, config, NULL); + g_object_unref (config); + } else { + priv->ip6_config = config; + nm_exported_object_export (NM_EXPORTED_OBJECT (config)); + g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_IP6_CONFIG); + } + nm_vpn_connection_config_maybe_complete (self, TRUE); } From a5d1db08f8a6d422d6dcb382fb0ca93f018299a9 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 24 Jun 2016 09:19:42 +0200 Subject: [PATCH 3/4] dns: log DNS servers at TRACE level Be more verbose at TRACE level and log the DNS servers associated to configurations. This will help to debug issues like [0]. [0] https://bugzilla.redhat.com/show_bug.cgi?id=1348887 --- shared/nm-utils/nm-macros-internal.h | 8 +++++ src/dns-manager/nm-dns-manager.c | 46 ++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 603689af01..81290a3fd2 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -51,6 +51,14 @@ _nm_auto_unset_gvalue_impl (GValue *v) } #define nm_auto_unset_gvalue nm_auto(_nm_auto_unset_gvalue_impl) +static inline void +_nm_auto_free_gstring_impl (GString **str) +{ + if (*str) + g_string_free (*str, TRUE); +} +#define nm_auto_free_gstring nm_auto(_nm_auto_free_gstring_impl) + /********************************************************/ /* http://stackoverflow.com/a/11172679 */ diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 6c29bd390f..3efd5ac1a8 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -940,6 +940,44 @@ merge_global_dns_config (NMResolvConfData *rc, NMGlobalDnsConfig *global_conf) return TRUE; } +static const char * +get_nameserver_list (void *config, GString **str) +{ + NMIP4Config *ip4; + NMIP6Config *ip6; + guint num, i; + + nm_assert (str); + + if (*str) + g_string_truncate (*str, 0); + else + *str = g_string_sized_new (64); + + if (NM_IS_IP4_CONFIG (config)) { + ip4 = (NMIP4Config *) config; + num = nm_ip4_config_get_num_nameservers (ip4); + for (i = 0; i < num; i++) { + g_string_append (*str, + nm_utils_inet4_ntop (nm_ip4_config_get_nameserver (ip4, i), + NULL)); + g_string_append_c (*str, ' '); + } + } else if (NM_IS_IP6_CONFIG (config)) { + ip6 = (NMIP6Config *) config; + num = nm_ip6_config_get_num_nameservers (ip6); + for (i = 0; i < num; i++) { + g_string_append (*str, + nm_utils_inet6_ntop (nm_ip6_config_get_nameserver (ip6, i), + NULL)); + g_string_append_c (*str, ' '); + } + } else + g_return_val_if_reached (NULL); + + return (*str)->str; +} + static gboolean update_dns (NMDnsManager *self, gboolean no_caching, @@ -959,6 +997,7 @@ update_dns (NMDnsManager *self, NMConfigData *data; NMGlobalDnsConfig *global_config; gs_free NMDnsIPConfigData **plugin_confs = NULL; + nm_auto_free_gstring GString *tmp_gstring = NULL; g_return_val_if_fail (!error || !*error, FALSE); @@ -1015,12 +1054,13 @@ update_dns (NMDnsManager *self, prev_prio = prio; - _LOGT ("config: %8d %-7s v%c %-16s %s", + _LOGT ("config: %8d %-7s v%c %-16s %s: %s", prio, _config_type_to_string (current->type), - v4 ? '4' : '6', + v4 ? '4' : '6', current->iface, - skip ? "" : ""); + skip ? "" : "", + get_nameserver_list (current->config, &tmp_gstring)); if (!skip) { merge_one_ip_config_data (self, &rc, current); From df48628a483f286f6e52e2d7d3cffe4fa31c4299 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 24 Jun 2016 09:25:51 +0200 Subject: [PATCH 4/4] vpn: don't merge DNS properties into parent device's configuration DNS properties should not be copied to parent device's configuration otherwise they will be applied twice, possibly with two different DNS priorities. --- src/vpn-manager/nm-vpn-connection.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index dc8c0e8444..bdbd4e6f6f 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1025,12 +1025,11 @@ apply_parent_device_config (NMVpnConnection *self) ifindex = nm_device_get_ip_ifindex (parent_dev); if (priv->ip4_config) { vpn4_parent_config = nm_ip4_config_new (ifindex); - nm_ip4_config_merge (vpn4_parent_config, priv->ip4_config, NM_IP_CONFIG_MERGE_DEFAULT); - nm_ip4_config_unset_gateway (vpn4_parent_config); + nm_ip4_config_merge (vpn4_parent_config, priv->ip4_config, NM_IP_CONFIG_MERGE_NO_DNS); } if (priv->ip6_config) { vpn6_parent_config = nm_ip6_config_new (ifindex); - nm_ip6_config_merge (vpn6_parent_config, priv->ip6_config, NM_IP_CONFIG_MERGE_DEFAULT); + nm_ip6_config_merge (vpn6_parent_config, priv->ip6_config, NM_IP_CONFIG_MERGE_NO_DNS); nm_ip6_config_set_gateway (vpn6_parent_config, NULL); } }