From 6169cd570f855eec80819dd2ac63324c65cbc598 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 31 Aug 2018 17:02:21 +0200 Subject: [PATCH] core: nm-ip4-config: consider dns-related differences as relevant The DNS manager reacts to NM_DEVICE_IP4_CONFIG_CHANGED events, which are generated when there is a relevant change to an IP4 configuration. Until now, changes to the mdns,llmnr properties were not considered relevant (and neither minor, this is already a bug). Promote them to relevant so that the DNS manager is notified and will rewrite the DNS configuration when one of this properties changes. Note that the DNS priority should be considered relevant and added into the checksum as well, but is a problem right now because in the DNS manager we rely on the fact that an empty configuration (i.e. just created) has a zero checksum. This is needed to avoid rewriting resolv.conf when there is no change. The DNS priority initial value depends on the connection type (VPN or not), so it's a bit difficult to add it to checksum maintaining the assumption of checksum(empty)=0. This should be improved in the future. --- src/dns/nm-dns-manager.c | 2 ++ src/nm-ip4-config.c | 31 +++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 2a15bf5e46..6a59b41c7b 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -970,6 +970,8 @@ compute_hash (NMDnsManager *self, const NMGlobalDnsConfig *global, guint8 buffer else { const CList *head; + /* FIXME(ip-config-checksum): this relies on the fact that an IP + * configuration without DNS parameters gives a zero checksum. */ head = _ip_config_lst_head (self); c_list_for_each_entry (ip_data, head, ip_config_lst) nm_ip_config_hash (ip_data->ip_config, sum, TRUE); diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 5f1004130c..573c479a92 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -1885,8 +1885,15 @@ nm_ip4_config_replace (NMIP4Config *dst, const NMIP4Config *src, gboolean *relev has_relevant_changes = TRUE; } - dst_priv->mdns = src_priv->mdns; - dst_priv->llmnr = src_priv->llmnr; + if (src_priv->mdns != dst_priv->mdns) { + dst_priv->mdns = src_priv->mdns; + has_relevant_changes = TRUE; + } + + if (src_priv->llmnr != dst_priv->llmnr) { + dst_priv->llmnr = src_priv->llmnr; + has_relevant_changes = TRUE; + } /* DNS priority */ if (src_priv->dns_priority != dst_priv->dns_priority) { @@ -2877,6 +2884,7 @@ nm_ip4_config_hash (const NMIP4Config *self, GChecksum *sum, gboolean dns_only) NMDedupMultiIter ipconf_iter; const NMPlatformIP4Address *address; const NMPlatformIP4Route *route; + int val; g_return_if_fail (self); g_return_if_fail (sum); @@ -2923,6 +2931,25 @@ nm_ip4_config_hash (const NMIP4Config *self, GChecksum *sum, gboolean dns_only) s = nm_ip4_config_get_dns_option (self, i); g_checksum_update (sum, (const guint8 *) s, strlen (s)); } + + val = nm_ip4_config_mdns_get (self); + if (val != NM_SETTING_CONNECTION_MDNS_DEFAULT) + g_checksum_update (sum, (const guint8 *) &val, sizeof (val)); + + val = nm_ip4_config_llmnr_get (self); + if (val != NM_SETTING_CONNECTION_LLMNR_DEFAULT) + g_checksum_update (sum, (const guint8 *) &val, sizeof (val)); + + /* FIXME(ip-config-checksum): the DNS priority should be considered relevant + * and added into the checksum as well, but this can't be done right now + * because in the DNS manager we rely on the fact that an empty + * configuration (i.e. just created) has a zero checksum. This is needed to + * avoid rewriting resolv.conf when there is no change. + * + * The DNS priority initial value depends on the connection type (VPN or + * not), so it's a bit difficult to add it to checksum maintaining the + * assumption of checksum(empty)=0 + */ } /**