From 778d1cf2e8af334e8f1c922ed405c351f38b020a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 5 Feb 2013 18:17:09 -0600 Subject: [PATCH] core: track which interface an IP config came from Various bits of code want the network interface which an IP config came from, for example when distinguishing which interface to send DNS requests to when the DNS servers are link-local. DNS plugins may also want this data for various reasons. So it makes sense to attach the interface name to the IP config object when the DNS manager gets it, so that later DNS updates that don't have any interface information (hostname changes, etc) can still generate correct DNS information. This also eliminates the "last_iface" hack, which was often inaccurate. It also now sends "NetworkManager" to SUSE netconfig as the interface name, because the DNS information being sent is already merged/prioritized and not specific to a network interface, so it's time to stop lying about where it came from. --- src/dns-manager/nm-dns-dnsmasq.c | 17 +++-- src/dns-manager/nm-dns-manager.c | 115 +++++++++++++------------------ src/dns-manager/nm-dns-manager.h | 8 +-- src/dns-manager/nm-dns-plugin.c | 6 +- src/dns-manager/nm-dns-plugin.h | 8 +-- src/nm-policy.c | 8 +-- 6 files changed, 70 insertions(+), 92 deletions(-) diff --git a/src/dns-manager/nm-dns-dnsmasq.c b/src/dns-manager/nm-dns-dnsmasq.c index e07240c565..f7e27da3fe 100644 --- a/src/dns-manager/nm-dns-dnsmasq.c +++ b/src/dns-manager/nm-dns-dnsmasq.c @@ -183,19 +183,23 @@ error: } static gboolean -add_ip6_config (GString *str, NMIP6Config *ip6, gboolean split, const char *iface) +add_ip6_config (GString *str, NMIP6Config *ip6, gboolean split) { const struct in6_addr *addr; char *buf; int n, i; gboolean added = FALSE; + const char *iface; + + iface = g_object_get_data (G_OBJECT (ip6), IP_CONFIG_IFACE_TAG); + g_assert (iface); if (split) { if (nm_ip6_config_get_num_nameservers (ip6) == 0) return FALSE; /* FIXME: it appears that dnsmasq can only handle one nameserver - * per domain (at the manpage seems to indicate that) so only use + * per domain (at least the manpage seems to indicate that) so only use * the first nameserver here. */ addr = nm_ip6_config_get_nameserver (ip6, 0); @@ -247,8 +251,7 @@ update (NMDnsPlugin *plugin, const GSList *vpn_configs, const GSList *dev_configs, const GSList *other_configs, - const char *hostname, - const char *iface) + const char *hostname) { NMDnsDnsmasq *self = NM_DNS_DNSMASQ (plugin); GString *conf; @@ -274,7 +277,7 @@ update (NMDnsPlugin *plugin, if (NM_IS_IP4_CONFIG (iter->data)) add_ip4_config (conf, NM_IP4_CONFIG (iter->data), TRUE); else if (NM_IS_IP6_CONFIG (iter->data)) - add_ip6_config (conf, NM_IP6_CONFIG (iter->data), TRUE, iface); + add_ip6_config (conf, NM_IP6_CONFIG (iter->data), TRUE); } /* Now add interface configs without split DNS */ @@ -282,7 +285,7 @@ update (NMDnsPlugin *plugin, if (NM_IS_IP4_CONFIG (iter->data)) add_ip4_config (conf, NM_IP4_CONFIG (iter->data), FALSE); else if (NM_IS_IP6_CONFIG (iter->data)) - add_ip6_config (conf, NM_IP6_CONFIG (iter->data), FALSE, iface); + add_ip6_config (conf, NM_IP6_CONFIG (iter->data), FALSE); } /* And any other random configs */ @@ -290,7 +293,7 @@ update (NMDnsPlugin *plugin, if (NM_IS_IP4_CONFIG (iter->data)) add_ip4_config (conf, NM_IP4_CONFIG (iter->data), FALSE); else if (NM_IS_IP6_CONFIG (iter->data)) - add_ip6_config (conf, NM_IP6_CONFIG (iter->data), FALSE, iface); + add_ip6_config (conf, NM_IP6_CONFIG (iter->data), FALSE); } /* Write out the config file */ diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 38691f37d6..b838e9f03d 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -74,11 +74,7 @@ typedef struct { GSList *plugins; - /* This is a hack because SUSE's netconfig always wants changes - * associated with a network interface, but sometimes a change isn't - * associated with a network interface (like hostnames). - */ - char *last_iface; + gboolean dns_touched; } NMDnsManagerPrivate; @@ -158,14 +154,19 @@ merge_one_ip4_config (NMResolvConfData *rc, NMIP4Config *src) } static void -merge_one_ip6_config (NMResolvConfData *rc, NMIP6Config *src, const char *iface) +merge_one_ip6_config (NMResolvConfData *rc, NMIP6Config *src) { guint32 num, i; + const char *iface; + + iface = g_object_get_data (G_OBJECT (src), IP_CONFIG_IFACE_TAG); + g_assert (iface); num = nm_ip6_config_get_num_nameservers (src); for (i = 0; i < num; i++) { const struct in6_addr *addr; char buf[INET6_ADDRSTRLEN]; + char *tmp; addr = nm_ip6_config_get_nameserver (src, i); @@ -176,7 +177,6 @@ merge_one_ip6_config (NMResolvConfData *rc, NMIP6Config *src, const char *iface) } else { if (inet_ntop (AF_INET6, addr, buf, INET6_ADDRSTRLEN) > 0) { if (IN6_IS_ADDR_LINKLOCAL (addr) && strchr (buf, '%') == NULL) { - char *tmp; tmp = g_strdup_printf ("%s%%%s", buf, iface); add_string_item (rc->nameservers, tmp); g_free (tmp); @@ -261,7 +261,6 @@ dispatch_netconfig (const char *domain, char **nameservers, const char *nis_domain, char **nis_servers, - const char *iface, GError **error) { char *str, *tmp; @@ -273,16 +272,10 @@ dispatch_netconfig (const char *domain, if (pid < 0) return FALSE; - // FIXME: this is wrong. We are not writing out the iface-specific - // resolv.conf data, we are writing out an already-fully-merged - // resolv.conf. Assuming netconfig works in the obvious way, then - // there are various failure modes, such as, eg, bringing up a VPN on - // eth0, then bringing up wlan0, then bringing down the VPN. Because - // NMDnsManager would have claimed that the VPN DNS server was also - // part of the wlan0 config, it will remain in resolv.conf after the - // VPN goes down, even though it is presumably no longer reachable - // at that point. - write_to_netconfig (fd, "INTERFACE", iface); + /* NM is writing already-merged DNS information to netconfig, so it + * does not apply to a specific network interface. + */ + write_to_netconfig (fd, "INTERFACE", "NetworkManager"); if (searches) { str = g_strjoinv (" ", searches); @@ -404,7 +397,6 @@ static gboolean dispatch_resolvconf (const char *domain, char **searches, char **nameservers, - const char *iface, GError **error) { char *cmd; @@ -416,7 +408,7 @@ dispatch_resolvconf (const char *domain, if (domain || searches || nameservers) { cmd = g_strconcat (RESOLVCONF_PATH, " -a ", "NetworkManager", NULL); - nm_log_info (LOGD_DNS, "(%s): writing resolv.conf to %s", iface, RESOLVCONF_PATH); + nm_log_info (LOGD_DNS, "Writing DNS information to %s", RESOLVCONF_PATH); if ((f = popen (cmd, "w")) == NULL) g_set_error (error, NM_DNS_MANAGER_ERROR, @@ -430,7 +422,7 @@ dispatch_resolvconf (const char *domain, } } else { cmd = g_strconcat (RESOLVCONF_PATH, " -d ", "NetworkManager", NULL); - nm_log_info (LOGD_DNS, "(%s): removing resolv.conf from %s", iface, RESOLVCONF_PATH); + nm_log_info (LOGD_DNS, "Removing DNS information from %s", RESOLVCONF_PATH); if (nm_spawn_process (cmd) == 0) retval = TRUE; } @@ -445,7 +437,6 @@ static gboolean update_resolv_conf (const char *domain, char **searches, char **nameservers, - const char *iface, GError **error) { char *tmp_resolv_conf; @@ -569,7 +560,6 @@ compute_hash (NMDnsManager *self, guint8 buffer[HASH_LEN]) static gboolean update_dns (NMDnsManager *self, - const char *iface, gboolean no_caching, GError **error) { @@ -589,12 +579,9 @@ update_dns (NMDnsManager *self, priv = NM_DNS_MANAGER_GET_PRIVATE (self); - nm_log_dbg (LOGD_DNS, "updating resolv.conf"); + priv->dns_touched = TRUE; - if (iface && (iface != priv->last_iface)) { - g_free (priv->last_iface); - priv->last_iface = g_strdup (iface); - } + nm_log_dbg (LOGD_DNS, "updating resolv.conf"); /* Update hash with config we're applying */ compute_hash (self, priv->hash); @@ -611,9 +598,9 @@ update_dns (NMDnsManager *self, merge_one_ip4_config (&rc, priv->ip4_device_config); if (priv->ip6_vpn_config) - merge_one_ip6_config (&rc, priv->ip6_vpn_config, iface); + merge_one_ip6_config (&rc, priv->ip6_vpn_config); if (priv->ip6_device_config) - merge_one_ip6_config (&rc, priv->ip6_device_config, iface); + merge_one_ip6_config (&rc, priv->ip6_device_config); for (iter = priv->configs; iter; iter = g_slist_next (iter)) { if ( (iter->data == priv->ip4_vpn_config) @@ -629,7 +616,7 @@ update_dns (NMDnsManager *self, } else if (NM_IS_IP6_CONFIG (iter->data)) { NMIP6Config *config = NM_IP6_CONFIG (iter->data); - merge_one_ip6_config (&rc, config, iface); + merge_one_ip6_config (&rc, config); } else g_assert_not_reached (); } @@ -721,8 +708,7 @@ update_dns (NMDnsManager *self, vpn_configs, dev_configs, other_configs, - priv->hostname, - iface)) { + priv->hostname)) { nm_log_warn (LOGD_DNS, "DNS: plugin %s update failed", plugin_name); /* If the plugin failed to update, we shouldn't write out a local @@ -747,19 +733,18 @@ update_dns (NMDnsManager *self, } #ifdef RESOLVCONF_PATH - success = dispatch_resolvconf (domain, searches, nameservers, iface, error); + success = dispatch_resolvconf (domain, searches, nameservers, error); #endif #ifdef NETCONFIG_PATH if (success == FALSE) { success = dispatch_netconfig (domain, searches, nameservers, - nis_domain, nis_servers, - iface, error); + nis_domain, nis_servers, error); } #endif if (success == FALSE) - success = update_resolv_conf (domain, searches, nameservers, iface, error); + success = update_resolv_conf (domain, searches, nameservers, error); if (searches) g_strfreev (searches); @@ -775,7 +760,6 @@ static void plugin_failed (NMDnsPlugin *plugin, gpointer user_data) { NMDnsManager *self = NM_DNS_MANAGER (user_data); - NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self); GError *error = NULL; /* Errors with non-caching plugins aren't fatal */ @@ -783,7 +767,7 @@ plugin_failed (NMDnsPlugin *plugin, gpointer user_data) return; /* Disable caching until the next DNS update */ - if (!update_dns (self, priv->last_iface, TRUE, &error)) { + if (!update_dns (self, TRUE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -806,6 +790,8 @@ nm_dns_manager_add_ip4_config (NMDnsManager *mgr, priv = NM_DNS_MANAGER_GET_PRIVATE (mgr); + g_object_set_data_full (G_OBJECT (config), IP_CONFIG_IFACE_TAG, g_strdup (iface), g_free); + switch (cfg_type) { case NM_DNS_IP_CONFIG_TYPE_VPN: priv->ip4_vpn_config = config; @@ -821,7 +807,7 @@ nm_dns_manager_add_ip4_config (NMDnsManager *mgr, if (!g_slist_find (priv->configs, config)) priv->configs = g_slist_append (priv->configs, g_object_ref (config)); - if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) { + if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -832,15 +818,12 @@ nm_dns_manager_add_ip4_config (NMDnsManager *mgr, } gboolean -nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, - const char *iface, - NMIP4Config *config) +nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, NMIP4Config *config) { NMDnsManagerPrivate *priv; GError *error = NULL; g_return_val_if_fail (mgr != NULL, FALSE); - g_return_val_if_fail (iface != NULL, FALSE); g_return_val_if_fail (config != NULL, FALSE); priv = NM_DNS_MANAGER_GET_PRIVATE (mgr); @@ -858,13 +841,15 @@ nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, g_object_unref (config); - if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) { + if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); g_clear_error (&error); } + g_object_set_data (G_OBJECT (config), IP_CONFIG_IFACE_TAG, NULL); + return TRUE; } @@ -883,6 +868,8 @@ nm_dns_manager_add_ip6_config (NMDnsManager *mgr, priv = NM_DNS_MANAGER_GET_PRIVATE (mgr); + g_object_set_data_full (G_OBJECT (config), IP_CONFIG_IFACE_TAG, g_strdup (iface), g_free); + switch (cfg_type) { case NM_DNS_IP_CONFIG_TYPE_VPN: priv->ip6_vpn_config = config; @@ -898,7 +885,7 @@ nm_dns_manager_add_ip6_config (NMDnsManager *mgr, if (!g_slist_find (priv->configs, config)) priv->configs = g_slist_append (priv->configs, g_object_ref (config)); - if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) { + if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -909,15 +896,12 @@ nm_dns_manager_add_ip6_config (NMDnsManager *mgr, } gboolean -nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, - const char *iface, - NMIP6Config *config) +nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, NMIP6Config *config) { NMDnsManagerPrivate *priv; GError *error = NULL; g_return_val_if_fail (mgr != NULL, FALSE); - g_return_val_if_fail (iface != NULL, FALSE); g_return_val_if_fail (config != NULL, FALSE); priv = NM_DNS_MANAGER_GET_PRIVATE (mgr); @@ -935,13 +919,15 @@ nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, g_object_unref (config); - if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) { + if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); g_clear_error (&error); } + g_object_set_data (G_OBJECT (config), IP_CONFIG_IFACE_TAG, NULL); + return TRUE; } @@ -973,7 +959,7 @@ nm_dns_manager_set_hostname (NMDnsManager *mgr, * wants one. But hostname changes are system-wide and *not* tied to a * specific interface, so netconfig can't really handle this. Fake it. */ - if (!priv->updates_queue && !update_dns (mgr, priv->last_iface, FALSE, &error)) { + if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -1023,7 +1009,7 @@ nm_dns_manager_end_updates (NMDnsManager *mgr, const char *func) /* Commit all the outstanding changes */ nm_log_dbg (LOGD_DNS, "(%s): committing DNS changes (%d)", func, priv->updates_queue); - if (!update_dns (mgr, priv->last_iface, FALSE, &error)) { + if (!update_dns (mgr, FALSE, &error)) { nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s", error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -1124,20 +1110,16 @@ dispose (GObject *object) g_slist_free (priv->plugins); priv->plugins = NULL; - /* If last_iface is NULL, this means we haven't done a DNS update before, - * so no reason to try and take down entries from resolv.conf. + /* If we're quitting leave a valid resolv.conf in place, not one + * pointing to 127.0.0.1 if any plugins were active. Thus update + * DNS after disposing of all plugins. But if we haven't done any + * DNS updates yet, there's no reason to touch resolv.conf on shutdown. */ - if (priv->last_iface != NULL) { - /* If we're quitting leave a valid resolv.conf in place, not one - * pointing to 127.0.0.1 if any plugins were active. Thus update - * DNS after disposing of all plugins. - */ - if (!update_dns (self, priv->last_iface, TRUE, &error)) { - nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); - g_clear_error (&error); - } + if (priv->dns_touched && !update_dns (self, TRUE, &error)) { + nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: (%d) %s", + error ? error->code : -1, + error && error->message ? error->message : "(unknown)"); + g_clear_error (&error); } g_slist_foreach (priv->configs, (GFunc) g_object_unref, NULL); @@ -1154,7 +1136,6 @@ finalize (GObject *object) NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (object); g_free (priv->hostname); - g_free (priv->last_iface); G_OBJECT_CLASS (nm_dns_manager_parent_class)->finalize (object); } diff --git a/src/dns-manager/nm-dns-manager.h b/src/dns-manager/nm-dns-manager.h index f559865b44..2349201206 100644 --- a/src/dns-manager/nm-dns-manager.h +++ b/src/dns-manager/nm-dns-manager.h @@ -75,18 +75,14 @@ gboolean nm_dns_manager_add_ip4_config (NMDnsManager *mgr, NMIP4Config *config, NMDnsIPConfigType cfg_type); -gboolean nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, - const char *iface, - NMIP4Config *config); +gboolean nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, NMIP4Config *config); gboolean nm_dns_manager_add_ip6_config (NMDnsManager *mgr, const char *iface, NMIP6Config *config, NMDnsIPConfigType cfg_type); -gboolean nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, - const char *iface, - NMIP6Config *config); +gboolean nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, NMIP6Config *config); void nm_dns_manager_set_hostname (NMDnsManager *mgr, const char *hostname); diff --git a/src/dns-manager/nm-dns-plugin.c b/src/dns-manager/nm-dns-plugin.c index b26f2b9460..e85b2a097a 100644 --- a/src/dns-manager/nm-dns-plugin.c +++ b/src/dns-manager/nm-dns-plugin.c @@ -56,8 +56,7 @@ nm_dns_plugin_update (NMDnsPlugin *self, const GSList *vpn_configs, const GSList *dev_configs, const GSList *other_configs, - const char *hostname, - const char *iface) + const char *hostname) { g_return_val_if_fail (NM_DNS_PLUGIN_GET_CLASS (self)->update != NULL, FALSE); @@ -65,8 +64,7 @@ nm_dns_plugin_update (NMDnsPlugin *self, vpn_configs, dev_configs, other_configs, - hostname, - iface); + hostname); } static gboolean diff --git a/src/dns-manager/nm-dns-plugin.h b/src/dns-manager/nm-dns-plugin.h index de4ad84e99..2a66557be2 100644 --- a/src/dns-manager/nm-dns-plugin.h +++ b/src/dns-manager/nm-dns-plugin.h @@ -32,6 +32,8 @@ #define NM_DNS_PLUGIN_FAILED "failed" #define NM_DNS_PLUGIN_CHILD_QUIT "child-quit" +#define IP_CONFIG_IFACE_TAG "dns-manager-iface" + typedef struct { GObject parent; } NMDnsPlugin; @@ -53,8 +55,7 @@ typedef struct { const GSList *vpn_configs, const GSList *dev_configs, const GSList *other_configs, - const char *hostname, - const char *iface); + const char *hostname); /* Subclasses should override and return TRUE if they start a local * caching nameserver that listens on localhost and would block any @@ -92,8 +93,7 @@ gboolean nm_dns_plugin_update (NMDnsPlugin *self, const GSList *vpn_configs, const GSList *dev_configs, const GSList *other_configs, - const char *hostname, - const char *iface); + const char *hostname); /* For subclasses/plugins */ diff --git a/src/nm-policy.c b/src/nm-policy.c index 420a941131..752cf44eb0 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1396,7 +1396,7 @@ device_ip4_config_changed (NMDevice *device, /* Old configs get removed immediately */ if (old_config) - nm_dns_manager_remove_ip4_config (dns_mgr, ip_iface, old_config); + nm_dns_manager_remove_ip4_config (dns_mgr, old_config); /* Ignore IP config changes while the device is activating, because we'll * catch all the changes when the device moves to ACTIVATED state. @@ -1433,7 +1433,7 @@ device_ip6_config_changed (NMDevice *device, /* Old configs get removed immediately */ if (old_config) - nm_dns_manager_remove_ip6_config (dns_mgr, ip_iface, old_config); + nm_dns_manager_remove_ip6_config (dns_mgr, old_config); /* Ignore IP config changes while the device is activating, because we'll * catch all the changes when the device moves to ACTIVATED state. @@ -1608,7 +1608,7 @@ vpn_connection_deactivated (NMPolicy *policy, NMVPNConnection *vpn) ip4_config = nm_vpn_connection_get_ip4_config (vpn); if (ip4_config) { /* Remove the VPN connection's IP4 config from DNS */ - nm_dns_manager_remove_ip4_config (mgr, ip_iface, ip4_config); + nm_dns_manager_remove_ip4_config (mgr, ip4_config); /* Re-apply routes and addresses of the VPN connection's parent interface, * which the VPN might have overridden. @@ -1629,7 +1629,7 @@ vpn_connection_deactivated (NMPolicy *policy, NMVPNConnection *vpn) ip6_config = nm_vpn_connection_get_ip6_config (vpn); if (ip6_config) { /* Remove the VPN connection's IP6 config from DNS */ - nm_dns_manager_remove_ip6_config (mgr, ip_iface, ip6_config); + nm_dns_manager_remove_ip6_config (mgr, ip6_config); /* Re-apply routes and addresses of the VPN connection's parent interface, * which the VPN might have overridden.