[PATCH 1/2] save duid to lease file

2013-02-13 Thread Gene Czarcinski
get_duid() got a default-duid from the lease file, from
one of the system files, or from the machine-id but did
not save it back to the lease file so that dhclient
could use it.
Signed-off-by: Gene Czarcinski g...@czarc.net
---
 src/dhcp-manager/nm-dhcp-dhclient.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c 
b/src/dhcp-manager/nm-dhcp-dhclient.c
index d9f5135..ad41dcd 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient.c
@@ -643,7 +643,6 @@ get_duid (NMDHCPClient *client)
TRUE);
nm_log_dbg (LOGD_DHCP, Looking for DHCPv6 DUID in '%s'., leasefile);
duid = nm_dhcp_dhclient_read_duid (leasefile, error);
-   g_free (leasefile);
 
if (error) {
nm_log_warn (LOGD_DHCP, Failed to read leasefile '%s': (%d) 
%s,
@@ -666,8 +665,27 @@ get_duid (NMDHCPClient *client)
}
}
 
-   /* return our DUID, otherwise let the parent class make a default DUID 
*/
-   return duid ? duid : NM_DHCP_CLIENT_CLASS 
(nm_dhcp_dhclient_parent_class)-get_duid (client);
+   /* If no DUID, let the parent class make a default DUID */
+   if (!duid)
+   duid = NM_DHCP_CLIENT_CLASS 
(nm_dhcp_dhclient_parent_class)-get_duid (client);
+
+   nm_log_dbg (LOGD_DHCP, Attempting to write default-duid to leasefile 
'%s', leasefile);
+   if (duid) {
+   if (!nm_dhcp_dhclient_save_duid (leasefile, 
nm_dhcp_dhclient_escape_duid (duid), error)) {
+   nm_log_warn (LOGD_DHCP, Failed to write default-duid 
to leasefile '%s' :(%d) %s,
+   leasefile,
+   error ? error-code : -1,
+   error ? error-message : 
(unknown));
+   g_clear_error(error);
+   }
+   }
+   else {
+   nm_log_warn (LOGD_DHCP, Failed to get duid-UUID from parent);
+   }
+
+   g_free (leasefile);
+
+   return duid;
 }
 
 /***/
-- 
1.8.1.2

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH 0/2] Fix default-duid support

2013-02-13 Thread Gene Czarcinski
These two patches correct the code for default-duid support.

The first patch changes the code so that a default-duid is
gotten from the lease file, from one of the system files such as
/etc/dhclient6.leases, or a generate duid-UUID based on the
machine-id.

The second patch adds the routine machine_id_parse() and uses it
in place of uuid_parse().  The machine-id uuid is not compatable
with standard uuid.

Gene Czarcinski (2):
  save duid to lease file
  add special machine-id parse function

 src/dhcp-manager/nm-dhcp-client.c   | 30 +-
 src/dhcp-manager/nm-dhcp-dhclient.c | 24 +---
 2 files changed, 50 insertions(+), 4 deletions(-)

-- 
1.8.1.2

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH 2/2] add special machine-id parse function

2013-02-13 Thread Gene Czarcinski
The original used uuid_parse() but that function did not
work properly since the format of the machine-id is
not compatable with a real uuid.  This patch adds a new
machine_id_parse() routine to correctly convert the
character string of hex digits to a 16 byte binary string.
Signed-off-by: Gene Czarcinski g...@czarc.net
---
 src/dhcp-manager/nm-dhcp-client.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/dhcp-manager/nm-dhcp-client.c 
b/src/dhcp-manager/nm-dhcp-client.c
index 2cdd304..a74f2ad 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -20,6 +20,7 @@
 #include config.h
 #include glib.h
 #include string.h
+#include ctype.h
 #include sys/types.h
 #include sys/wait.h
 #include errno.h
@@ -323,6 +324,32 @@ nm_dhcp_client_start_ip4 (NMDHCPClient *self,
return priv-pid ? TRUE : FALSE;
 }
 
+/* uuid_parse does not work for machine-id, so we use our own converter */
+static int
+machine_id_parse (const char *in, uuid_t uu)
+{
+   const char *cp;
+   int i;
+   unsigned char buf[3];
+
+   if (strlen(in) != 32)
+   return -1;
+   for (i = 0, cp = in; i  32; i++, cp++)
+   {
+   if (!isxdigit(*cp))
+   return -1;
+   }
+   buf[2] = 0;
+   cp = in;
+   for (i = 0; i  16; i++)
+   {
+   buf[0] = *cp++;
+   buf[1] = *cp++;
+   uu[i] = strtoul (buf, NULL, 16);
+   }
+   return 0;
+}
+
 static GByteArray *
 generate_duid_from_machine_id (void)
 {
@@ -348,7 +375,8 @@ generate_duid_from_machine_id (void)
}
 
contents = g_strstrip (contents);
-   ret = uuid_parse (contents, uuid);
+   nm_log_dbg (LOGD_DHCP6, /etc/machine-id len= %d, value= '%s', 
strlen(contents), contents);
+   ret = machine_id_parse (contents, uuid);
g_free (contents);
 
if (ret != 0) {
-- 
1.8.1.2

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH 0/2] Fix default-duid support

2013-02-13 Thread Gene Czarcinski

On 02/13/2013 03:01 AM, Gene Czarcinski wrote:

These two patches correct the code for default-duid support.

The first patch changes the code so that a default-duid is
gotten from the lease file, from one of the system files such as
/etc/dhclient6.leases, or a generate duid-UUID based on the
machine-id.

The second patch adds the routine machine_id_parse() and uses it
in place of uuid_parse().  The machine-id uuid is not compatable
with standard uuid.

Gene Czarcinski (2):
   save duid to lease file
   add special machine-id parse function

  src/dhcp-manager/nm-dhcp-client.c   | 30 +-
  src/dhcp-manager/nm-dhcp-dhclient.c | 24 +---
  2 files changed, 50 insertions(+), 4 deletions(-)


This is based on git 0.9.7.997 but appears to apply to master also.

Besides duid-UUID now working, I tested getting a sysadmin specified 
default-duid
from /etc/dhclient6.leases, /var/lib/dhcp/dhclient6.leases, 
/var/lib/dhclient/dhclient6.leases,

or from the NetworkManger connection-specific lease file.

I suggest this be applied before releasing 0.9.8.0.

I rebased to nm-0-9-8 with no problems or corrections needed.

Gene
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH 2/5] connectivity: Add libxml2 as a dependency

2013-02-13 Thread Jonh Wendell
Hi Dan.

2013/2/11 Dan Williams d...@redhat.com

 On Mon, 2013-02-11 at 15:30 -0200, Jonh Wendell wrote:
  In these patches I want to fix the 511-http-status. As it's something
  new, of course most hotspots don't use that (including my employer).
 
  Almost all of them rely on 30X Moved with help of the Wispr
  'pseudo-protocol'. It's on my TODO list to work on those scenarios.
  That would touch only code in NMConnectivity object.
 
 
  Indeed, for that cases, we would use xml parsing as wispr is xml.

 Thanks for the clarification.  In any case, would a regex be possible
 here instead of the XML for now?  It might be less code and would
 certainly be less error-prone when checking broken HTML which is quite
 common.


I'm not sure if parsing a HTML with regex is less error-prone. There are
lots of traps in mal-formed HTML, or even if legitimate HTML structs like

img title=displays  src=big.gif

So, I'd prefer to rely on libxml which is supposed to be more smart than a
simple regex.
In the patch, I'm using the flags 'HTML_PARSE_NOERROR |
HTML_PARSE_NOWARNING' so that libxml doesn't complain about broken HTML. In
fact, I've tested here some broken HTML and it was parsed successfully by
libxml.

Plus, as I stated earlier, libxml will be required to handle Wispr
responses (and hotspot 2.0), which are legitimate XML trees. So, why not
just add it as a dependency right now?


 Dan

 
 
  2013/2/11 Dan Williams d...@redhat.com
  On Mon, 2013-02-11 at 17:10 +0100, Bastien Nocera wrote:
   On Mon, 2013-02-11 at 10:06 -0600, Dan Williams wrote:
On Mon, 2013-02-11 at 12:09 -0200, Jonh Wendell wrote:
 From: Jonh Wendell jonh.wend...@oiwifi.com.br

 libsoup already depends on libxml2 but we need to
  explicitly link
 to it.
   
At least we already theoretically required it; though is
  it possible to
use GMarkup here instead of libxml2?  GMarkup would be
  somewhat simpler,
though it's only a subset.
  
   Given how it's used, there's probably little reason this
  couldn't be a
   regexp. Both GMarkup and libxml2 would choke on broken,
  slightly broken,
   and very very broken HTML files.
 
 
  Yeah, and I've heard that for example, some hotspots literally
  just
  append raw XML to the end of the HTTP request outside the
  XML.  I think
  we need to be somewhat more robust here and XML parsing may
  not be the
  way to get there?  Also, we may need to add special cases for
  various
  hotspots, which might require regex and not just XML parsing.
 
  Dan
 
 
 
 
  --
  Jonh Wendell
  http://www.bani.com.br
 





-- 
Jonh Wendell
http://www.bani.com.br
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH 1/2] save duid to lease file

2013-02-13 Thread Dan Williams
On Wed, 2013-02-13 at 03:01 -0500, Gene Czarcinski wrote:
 get_duid() got a default-duid from the lease file, from
 one of the system files, or from the machine-id but did
 not save it back to the lease file so that dhclient
 could use it.
 Signed-off-by: Gene Czarcinski g...@czarc.net

Instead of writing it when getting it, we should actually write it when
we're about to execute dhclient for IPv6.  So something like this
instead?  Good catch though, thanks for looking into this.

Dan

diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c 
b/src/dhcp-manager/nm-dhcp-dhclient.c
index d9f5135..c43ecf5 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient.c
@@ -455,8 +455,9 @@ dhclient_start (NMDHCPClient *client,
GError *error = NULL;
const char *iface, *uuid, *system_bus_address;
char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env 
= NULL;
-   gboolean ipv6;
+   gboolean ipv6, success;
guint log_domain;
+   char *escaped;
 
g_return_val_if_fail (priv-pid_file == NULL, -1);
 
@@ -497,6 +498,20 @@ dhclient_start (NMDHCPClient *client,
return -1;
}
 
+   /* Save the DUID to the leasefile dhclient will actually use */
+   if (ipv6) {
+   escaped = nm_dhcp_dhclient_escape_duid (duid);
+   success = nm_dhcp_dhclient_save_duid (priv-lease_file, 
escaped, error);
+   g_free (escaped);
+   if (!success) {
+   nm_log_warn (log_domain, (%s): failed to save DUID to 
%s: (%d) %s.,
+iface, priv-lease_file,
+error ? error-code : -1,
+error  error-message ? error-message : 
(unknown));
+   return -1;
+   }
+   }
+
argv = g_ptr_array_new ();
g_ptr_array_add (argv, (gpointer) priv-path);
 


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH 1/2] save duid to lease file

2013-02-13 Thread Gene Czarcinski

On 02/13/2013 10:43 AM, Dan Williams wrote:

On Wed, 2013-02-13 at 03:01 -0500, Gene Czarcinski wrote:

get_duid() got a default-duid from the lease file, from
one of the system files, or from the machine-id but did
not save it back to the lease file so that dhclient
could use it.
Signed-off-by: Gene Czarcinski g...@czarc.net

Instead of writing it when getting it, we should actually write it when
we're about to execute dhclient for IPv6.  So something like this
instead?  Good catch though, thanks for looking into this.

Dan
Works for me.  Once I turned on logging level=debug and realized that 
the DUID had the correct value but that it was never saved, the answer 
was obvious.


I believe you will find the other patch equally interesting.  There was 
no way to get duid-UUID with uuid_parse().


Gene



diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c 
b/src/dhcp-manager/nm-dhcp-dhclient.c
index d9f5135..c43ecf5 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient.c
@@ -455,8 +455,9 @@ dhclient_start (NMDHCPClient *client,
 GError *error = NULL;
 const char *iface, *uuid, *system_bus_address;
 char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env 
= NULL;
-   gboolean ipv6;
+   gboolean ipv6, success;
 guint log_domain;
+   char *escaped;
  
 g_return_val_if_fail (priv-pid_file == NULL, -1);
  
@@ -497,6 +498,20 @@ dhclient_start (NMDHCPClient *client,

 return -1;
 }
  
+   /* Save the DUID to the leasefile dhclient will actually use */

+   if (ipv6) {
+   escaped = nm_dhcp_dhclient_escape_duid (duid);
+   success = nm_dhcp_dhclient_save_duid (priv-lease_file, escaped, 
error);
+   g_free (escaped);
+   if (!success) {
+   nm_log_warn (log_domain, (%s): failed to save DUID to %s: 
(%d) %s.,
+iface, priv-lease_file,
+error ? error-code : -1,
+error  error-message ? error-message : 
(unknown));
+   return -1;
+   }
+   }
+
 argv = g_ptr_array_new ();
 g_ptr_array_add (argv, (gpointer) priv-path);
  






___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH 1/2] save duid to lease file

2013-02-13 Thread Gene Czarcinski

On 02/13/2013 11:36 AM, Dan Williams wrote:

On Wed, 2013-02-13 at 11:16 -0500, Gene Czarcinski wrote:

On 02/13/2013 10:43 AM, Dan Williams wrote:

On Wed, 2013-02-13 at 03:01 -0500, Gene Czarcinski wrote:

get_duid() got a default-duid from the lease file, from
one of the system files, or from the machine-id but did
not save it back to the lease file so that dhclient
could use it.
Signed-off-by: Gene Czarcinski g...@czarc.net

Instead of writing it when getting it, we should actually write it when
we're about to execute dhclient for IPv6.  So something like this
instead?  Good catch though, thanks for looking into this.

Dan

Works for me.  Once I turned on logging level=debug and realized that
the DUID had the correct value but that it was never saved, the answer
was obvious.

I believe you will find the other patch equally interesting.  There was
no way to get duid-UUID with uuid_parse().

Yeah, I'd looked at the code for libuuid already.  I saw that it was
ignoring '-' already, which obviously machine-id doesn't have, but I
mentally passed over the strlen() == 36 parts.  I've got that patch and
a cleanup in my tree waiting on some testing and then I'll push.

Thanks!
Dan
The code in patch 2 is a little paranoid about machine-id being the 
right length, format , etc. but I noticed that uuid_parse() is pretty 
paranoid too.  I would rather be too paranoid than having the software 
go off into the weeds with no indication what happened.


Gene
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: Possibility to determine state of a connection

2013-02-13 Thread Martin Steuer

Am 13.02.2013 01:17, schrieb Dan Williams:

On Tue, 2013-02-12 at 21:18 +0100, Martin Steuer wrote:

I started using NetworkManager on a fresh installation of mine and
noticed a problem regarding PPPoE connections: When establishing a PPPoE
connection via ethernet an existing IP connection is disconnected. Thus
loosing IP connectivity on the local network.


This has been an issue for a long time in NM, and it needs to be
addressed in a more complete manner.  Basically, we need to run both the
original eth connection and the PPPoE one in parallel.  In reality, this
looks a lot like a VPN, and perhaps we should treat it as such.  Or,
really, it's just a tunnel like a VPN is a tunnel, and we should have a
more generic framework for handling tunnel interfaces.



Thats what I was thinking, VPN seems simliar in nature.


That would imply a change in behavior of NM as D-Bus clients would see
it, so perhaps this would have to wait a bit until we support multiple
VPN connections.  Which is something I'm happy to provide some guidance
on if anyone wants to work on it?



I'd be willing to invest some time, how involved would that be?


Dan
snip


Martin
___
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

2013-02-13 Thread Michael Stapelberg
Hi Dan,

Sorry for replying late and thanks for taking care of this.

Dan Williams d...@redhat.com 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

2013-02-13 Thread Dan Williams
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 d...@redhat.com 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

2013-02-13 Thread Michael Stapelberg
Hi Dan,

Dan Williams d...@redhat.com 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