From bea089c35d11e18136465fe800aa5a1cd3d299c5 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. (cherry picked from commit 19133b08daf97fedeeb3bd9d76bb8e58f87951a4) --- 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 9eb8b5b0c3..8a6d678eaf 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1080,7 +1080,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 7a343cc981c08ea8f4e9652303bbe007739a6fcd 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 (cherry picked from commit 641e8b00fea2503acbcd5c8c942ed751a6607469) --- 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 8a6d678eaf..1d12eec9eb 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1468,10 +1468,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); } @@ -1605,10 +1610,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 90dca9e0c9bb28c5cdce09a7fe0bebc263943c02 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 (cherry picked from commit a5d1db08f8a6d422d6dcb382fb0ca93f018299a9) --- shared/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-macros-internal.h b/shared/nm-macros-internal.h index ffa3ace086..fe27b761a0 100644 --- a/shared/nm-macros-internal.h +++ b/shared/nm-macros-internal.h @@ -46,6 +46,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 762757cfa10ad318d1869ee26d2c76b13d35841b 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. (cherry picked from commit df48628a483f286f6e52e2d7d3cffe4fa31c4299) --- 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 1d12eec9eb..cb8474aac0 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1023,12 +1023,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); } }