Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
Hi Dan, Dan Williams writes: >> cat /var/run/NetworkManager/dnsmasq.conf >> server=192.168.1.1 >> server=fe80::4e60:deff:fed8:d7c5@eth0 >> server=192.168.1.1 >> server=fe80::4e60:deff:fed8:d7c5@wlan0 >> >> I suppose it doesn’t really hurt, at least not in my case, but wouldn’t >> it be cleaner if entry was only listed once? :-) > > Yeah, probably. I assume it would be sufficient to just not list the > IPv4 nameserver twice? Correct. Anything else is actually wrong — the “same” IPv6 link-local address with different interface identifiers HAS to be treated as a different address. -- Best regards, Michael ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
On Wed, 2013-02-13 at 21:11 +0100, Michael Stapelberg wrote: > Hi Dan, > > Sorry for replying late and thanks for taking care of this. > > Dan Williams writes: > > So I'll propose a different solution: check the last patch in the > > dcbw/dns-iface git branch, and let me know if that works for you? > > > > http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=dcbw/dns-iface > This branch no longer exists, but I presume you have merged it and it > contained the commit > http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=778d1cf2e8af334e8f1c922ed405c351f38b020a > > I have upgraded to NetworkManager git 08f0446 and can confirm that the > symptom I have been reporting is now fixed. > > Do note that my DNS configuration now does contain a duplicate entry, > though: > > cat /var/run/NetworkManager/dnsmasq.conf > server=192.168.1.1 > server=fe80::4e60:deff:fed8:d7c5@eth0 > server=192.168.1.1 > server=fe80::4e60:deff:fed8:d7c5@wlan0 > > I suppose it doesn’t really hurt, at least not in my case, but wouldn’t > it be cleaner if entry was only listed once? :-) Yeah, probably. I assume it would be sufficient to just not list the IPv4 nameserver twice? Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
Hi Dan, Sorry for replying late and thanks for taking care of this. Dan Williams writes: > So I'll propose a different solution: check the last patch in the > dcbw/dns-iface git branch, and let me know if that works for you? > > http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=dcbw/dns-iface This branch no longer exists, but I presume you have merged it and it contained the commit http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=778d1cf2e8af334e8f1c922ed405c351f38b020a I have upgraded to NetworkManager git 08f0446 and can confirm that the symptom I have been reporting is now fixed. Do note that my DNS configuration now does contain a duplicate entry, though: cat /var/run/NetworkManager/dnsmasq.conf server=192.168.1.1 server=fe80::4e60:deff:fed8:d7c5@eth0 server=192.168.1.1 server=fe80::4e60:deff:fed8:d7c5@wlan0 I suppose it doesn’t really hurt, at least not in my case, but wouldn’t it be cleaner if entry was only listed once? :-) -- Best regards, Michael ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
On Tue, 2013-02-05 at 23:00 +0100, Michael Stapelberg wrote: > Hi Dan, > > Dan Williams writes: > > Sending the interface name is a hack anyway just to make netconfig and > > resolvconf happy, even though prioritizing DNS information based on > > interface name is bogus. NM merges and prioritizes the DNS > > configuration before sending to resolvconf/netconfig, so whatever > > interface we happen to send to them is already quite wrong, since the > > data may come from multiple interfaces. > > > > Instead, we should rip out all the interface name stuff and simply send > > "NetworkManager". Then people can do whatever they want with NM's data > > in the resolvconf priority, and I have no idea what netconfig does with > > INTERFACE=xxx but it's still going to be wrong, whatever it is. > > > > (you simply can't prioritize DNS data based on interface names, because > > interfaces service many different networks. Sometimes your eth0 is > > connected to a lower-priority network than your wlan0, sometimes it's > > higher. That is a *per-network* decision, not a per-interface one, but > > netconfig/resolvconf still seem to think it's per-interface...) > This might be correct for the case you have in mind. > > However, note that I was talking about interface-specific nameserver > addresses (link-local IPv6), e.g. fe80::4e60:deff:fed8:d7c5%wlan0. Since > it is that code path in which the segfault is triggered, we clearly need > to send the _correct_ interface, not just “NetworkManager”, otherwise we > will end up with server=fe80::4e60:deff:fed8:d7c5%NetworkManager in the > dnsmasq config :-). Yes, though the solution there is to attach the interface name to the NMIP4Config and NMIP6Config objects so we can regenerate the right DNS information no matter when we're asked to update it, since some parts of it are not interface specific (eg, hostname, as you've found). So I'll propose a different solution: check the last patch in the dcbw/dns-iface git branch, and let me know if that works for you? http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=dcbw/dns-iface Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
Hi Dan, Dan Williams writes: > Sending the interface name is a hack anyway just to make netconfig and > resolvconf happy, even though prioritizing DNS information based on > interface name is bogus. NM merges and prioritizes the DNS > configuration before sending to resolvconf/netconfig, so whatever > interface we happen to send to them is already quite wrong, since the > data may come from multiple interfaces. > > Instead, we should rip out all the interface name stuff and simply send > "NetworkManager". Then people can do whatever they want with NM's data > in the resolvconf priority, and I have no idea what netconfig does with > INTERFACE=xxx but it's still going to be wrong, whatever it is. > > (you simply can't prioritize DNS data based on interface names, because > interfaces service many different networks. Sometimes your eth0 is > connected to a lower-priority network than your wlan0, sometimes it's > higher. That is a *per-network* decision, not a per-interface one, but > netconfig/resolvconf still seem to think it's per-interface...) This might be correct for the case you have in mind. However, note that I was talking about interface-specific nameserver addresses (link-local IPv6), e.g. fe80::4e60:deff:fed8:d7c5%wlan0. Since it is that code path in which the segfault is triggered, we clearly need to send the _correct_ interface, not just “NetworkManager”, otherwise we will end up with server=fe80::4e60:deff:fed8:d7c5%NetworkManager in the dnsmasq config :-). -- Best regards, Michael ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] dns: store priv->last_iface even when no actual updates are performed
On Tue, 2013-02-05 at 19:12 +0100, Michael Stapelberg wrote: > Hi, > > the attached patch fixes a segmentation fault with n-m >= 0.9.6.4 (I > upgraded from 0.9.4.0, so it might be introduced earlier). > > Here is the commit message: > > dns: store priv->last_iface even when no actual updates are performed > > Otherwise, with DNS batch updating (commit f76aa4f), we might end up in > the situation where priv->last_iface is NULL when adding a link-local > IPv6 DNS server (e.g. fe80::4e60:deff:fed8:d7c5%wlan0), leading to a > segmentation fault. Sending the interface name is a hack anyway just to make netconfig and resolvconf happy, even though prioritizing DNS information based on interface name is bogus. NM merges and prioritizes the DNS configuration before sending to resolvconf/netconfig, so whatever interface we happen to send to them is already quite wrong, since the data may come from multiple interfaces. Instead, we should rip out all the interface name stuff and simply send "NetworkManager". Then people can do whatever they want with NM's data in the resolvconf priority, and I have no idea what netconfig does with INTERFACE=xxx but it's still going to be wrong, whatever it is. (you simply can't prioritize DNS data based on interface names, because interfaces service many different networks. Sometimes your eth0 is connected to a lower-priority network than your wlan0, sometimes it's higher. That is a *per-network* decision, not a per-interface one, but netconfig/resolvconf still seem to think it's per-interface...) Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] dns: store priv->last_iface even when no actual updates are performed
Hi,
the attached patch fixes a segmentation fault with n-m >= 0.9.6.4 (I
upgraded from 0.9.4.0, so it might be introduced earlier).
Here is the commit message:
dns: store priv->last_iface even when no actual updates are performed
Otherwise, with DNS batch updating (commit f76aa4f), we might end up in
the situation where priv->last_iface is NULL when adding a link-local
IPv6 DNS server (e.g. fe80::4e60:deff:fed8:d7c5%wlan0), leading to a
segmentation fault.
--
Best regards,
Michael
>From 48d5f34f709dbf00ea0bffeabb4e5db136500a32 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg
Date: Tue, 5 Feb 2013 19:02:12 +0100
Subject: [PATCH] dns: store priv->last_iface even when no actual updates are
performed
Otherwise, with DNS batch updating (commit f76aa4f), we might end up in
the situation where priv->last_iface is NULL when adding a link-local
IPv6 DNS server (e.g. fe80::4e60:deff:fed8:d7c5%wlan0), leading to a
segmentation fault.
---
src/dns-manager/nm-dns-manager.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c
index 38691f3..3bab4cb 100644
--- a/src/dns-manager/nm-dns-manager.c
+++ b/src/dns-manager/nm-dns-manager.c
@@ -91,6 +91,15 @@ typedef struct {
} NMResolvConfData;
static void
+update_last_iface (NMDnsManagerPrivate *priv, const char *iface)
+{
+ if (iface && (iface != priv->last_iface)) {
+ g_free (priv->last_iface);
+ priv->last_iface = g_strdup (iface);
+ }
+}
+
+static void
add_string_item (GPtrArray *array, const char *str)
{
int i;
@@ -821,6 +830,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));
+ update_last_iface(priv, iface);
if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) {
nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
error ? error->code : -1,
@@ -858,6 +868,7 @@ nm_dns_manager_remove_ip4_config (NMDnsManager *mgr,
g_object_unref (config);
+ update_last_iface(priv, iface);
if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) {
nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
error ? error->code : -1,
@@ -898,6 +909,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));
+ update_last_iface(priv, iface);
if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) {
nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
error ? error->code : -1,
@@ -935,6 +947,7 @@ nm_dns_manager_remove_ip6_config (NMDnsManager *mgr,
g_object_unref (config);
+ update_last_iface(priv, iface);
if (!priv->updates_queue && !update_dns (mgr, iface, FALSE, &error)) {
nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
error ? error->code : -1,
--
1.7.10.4
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list
