[libvirt] [PATCH] network utilities

Revamping my previous patches to parse IPv4/6 numeric addresses, adding range computation and netmask checking entries too. As suggested I used an union for the socket address type: typedef union { struct sockaddr_storage stor; struct sockaddr_in inet4; struct sockaddr_in6 inet6; } virSocketAddr; typedef virSocketAddr *virSocketAddrPtr; But I wonder if it's not better to make it a struct and add the lenght of the address as this is usually needed when passing one of the struct sockaddr_* and that lenght is returned as part of parsing, keeping both together is probably more useful. A macro to get the type AF_INET/AF_INET6 out of a virSocketAddr might be useful instead of having the users manually guess that ->stor.ss_family is teh filed to look at. Also I wonder how useful the Ipv6 code for netmask and ranges really is, it seems that IPv6 mostly go with prefix and never use netmasks, maybe that's just dead code, maybe that's just wrong, anyway here it is. I named the header and file network.[ch] as I think other network related utilities like validation or generation of MAC addresses could well be moved in that file too. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

(AHA! Just figured out how to get a patch sent as an attached file into the reply when using Thunderbird - 1) make sure Thunderbird is configured to display text attachments inline, 2) select everything in the original message (with ctrl-A), *then* 3) hit the reply button) On 10/22/2009 10:46 AM, Daniel Veillard wrote:
Revamping my previous patches to parse IPv4/6 numeric addresses, adding range computation and netmask checking entries too.
As suggested I used an union for the socket address type:
typedef union { struct sockaddr_storage stor; struct sockaddr_in inet4; struct sockaddr_in6 inet6; } virSocketAddr; typedef virSocketAddr *virSocketAddrPtr;
But I wonder if it's not better to make it a struct and add the lenght of the address as this is usually needed when passing one of the struct sockaddr_* and that lenght is returned as part of parsing, keeping both together is probably more useful. A macro to get the type AF_INET/AF_INET6 out of a virSocketAddr might be useful instead of having the users manually guess that ->stor.ss_family is teh filed to look at.
Also I wonder how useful the Ipv6 code for netmask and ranges really is, it seems that IPv6 mostly go with prefix and never use netmasks, maybe that's just dead code, maybe that's just wrong, anyway here it is.
I named the header and file network.[ch] as I think other network related utilities like validation or generation of MAC addresses could well be moved in that file too.
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ network_util.patch
commit d5ab523c72242d76184838d8215aa0ebe720c08e Author: Daniel Veillard<veillard@redhat.com> Date: Thu Oct 22 16:34:43 2009 +0200
Set of new network related utilities
* src/util/network.h src/util/network.c: utilities to parse network addresses, check netmask and compute ranges
diff --git a/src/Makefile.am b/src/Makefile.am index 8e27ea7..8fbfb5f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -55,6 +55,7 @@ UTIL_SOURCES = \ util/memory.c util/memory.h \ util/pci.c util/pci.h \ util/hostusb.c util/hostusb.h \ + util/network.c util/network.h \ util/qparams.c util/qparams.h \ util/stats_linux.c util/stats_linux.h \ util/storage_file.c util/storage_file.h \ diff --git a/src/util/network.c b/src/util/network.c new file mode 100644 index 0000000..7093dce --- /dev/null +++ b/src/util/network.c @@ -0,0 +1,324 @@ +/* + * network.c: network helper APIs for libvirt + * + * Copyright (C) 2009-2009 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software + * + * Daniel Veillard<veillard@redhat.com> + */ + +#include<config.h> + +#include "memory.h" +#include "network.h" + +/* + * Helpers to extract the IP arrays from the virSocketAddrPtr + * That part is the less portable of the module + */ +typedef unsigned char virIPv4Addr[4]; +typedef virIPv4Addr *virIPv4AddrPtr; +typedef unsigned short virIPv6Addr[8];
Are you defining this an an array of short for efficiency? It's much easier to deal with if you just look at it as a byte array - you don't need to worry about network vs. host ordering when doing comparisons and bit shifts/masks (IPv6 addresses, like IPv4 addresses, are *always* in network order, even when being passed around in applications - see section 3.2 of http://tools.ietf.org/html/rfc3493.)
+typedef virIPv6Addr *virIPv6AddrPtr; + +static int getIPv4Addr(virSocketAddrPtr addr, virIPv4AddrPtr tab) { + unsigned long val; + int i; + + if ((addr == NULL) || (tab == NULL) || (addr->stor.ss_family != AF_INET)) + return(-1); + + val = ntohl(addr->inet4.sin_addr.s_addr); + + for (i = 0;i< 4;i++) { + (*tab)[i] = val& 0xFF; + val>>= 8; + } + + return(0); +}
I get the sense you don't like typecasts ;-) Since the IP address is just a byte array, and the bytes are always in the same order whether on the network or on a little or big endian host, why not just do something like this: return *((int *)&addr->inet4.sin_addr.s_addr); (it's only the port that you need to worry about swapping)
+ +static int getIPv6Addr(virSocketAddrPtr addr, virIPv6AddrPtr tab) { + virIPv6AddrPtr val; + int i; + + if ((addr == NULL) || (tab == NULL) || (addr->stor.ss_family != AF_INET6)) + return(-1); + + val = (virIPv6AddrPtr)&(addr->inet6.sin6_addr.__in6_u.__u6_addr16); + + for (i = 0;i< 8;i++) { + (*tab)[i] = ntohs((*val)[i]); + } + + return(0); +}
Likewise with this - the address bytes in the sockaddr_in6 are in network order, so the extra work of swapping the bytes is going to mess things up. You could do a loop copying each element of __u6_addr8, __u6_addr16, or __u6_addr32, or you could just take advantage of struct assignment and do it with a single line of code: *val = *((virIPv6AddrPtr) &(addr->inet6.sin6_addr.in6_u)); (I think I got the *'s and &'s right ;-)
+/** + * virSocketParseAddr: + * @val: a numeric network address IPv4 or IPv6 + * @addr: where to store the return value. + * @hint: optional hint to pass down to getaddrinfo + * + * Mostly a wrapper for getaddrinfo() extracting the address storage + * from the numeric string like 1.2.3.4 or 2001:db8:85a3:0:0:8a2e:370:7334 + * + * Returns the lenght of the network address or -1 in case of error.
(Too bad getaddrinfo doesn't return a pointer just past the last byte it counted as part of the address, or at least the number of characters it ate - that would be a much more useful return value, since the length of the address is already implied in the address family that's set in the sockaddr.)
+ */ +int +virSocketParseAddr(const char *val, virSocketAddrPtr addr, int hint) { + int len; + struct addrinfo hints; + struct addrinfo *res = NULL; + + if ((val == NULL) || (addr == NULL)) + return(-1); + + memset(&hints, 0, sizeof(hints)); + hints.ai_flags = AI_NUMERICHOST | hint; + if ((getaddrinfo(val, NULL,&hints,&res) != 0) || (res == NULL)) { + return(-1); + } + + len = res->ai_addrlen; + memcpy(&addr->stor, res->ai_addr, len); + + freeaddrinfo(res); + return(len); +} + +/* + * virSocketParseIpv4Addr: + * @val: an IPv4 numeric address + * @addr: the loacation to store the result + * + * Extract the address storage from an IPv4 numeric address + * + * Returns the lenght of the network address or -1 in case of error. + */ +int +virSocketParseIpv4Addr(const char *val, virSocketAddrPtr addr) { + return(virSocketParseAddr(val, addr, AF_INET)); +} + +/* + * virSocketParseIpv6Addr: + * @val: an IPv6 numeric address + * @addr: the loacation to store the result + * + * Extract the address storage from an IPv6 numeric address + * + * Returns the lenght of the network address or -1 in case of error. + */ +int +virSocketParseIpv6Addr(const char *val, virSocketAddrPtr addr) { + return(virSocketParseAddr(val, addr, AF_INET6)); +} +
(I didn't check over the bit arithmetic in the netmask stuff, because it will all change anyway if you store the IPv6 addresses as an array[16] of bytes rather than an array[8] of host order shorts).)
+/** + * virSocketAddrIsNetmask: + * @netmask: the netmask address + * + * Check that @netmask is a proper network mask + * + * Returns 0 in case of success and -1 in case of error + */ +int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { + int i; + + if (netmask == NULL) + return(-1); + + if (netmask->stor.ss_family == AF_INET) { + virIPv4Addr tm; + unsigned char tmp; + int ok = 0; + + if (getIPv4Addr(netmask,&tm)< 0) + return(-1); + + for (i = 0;i< 4;i++) { + if (tm[i] != 0) + break; + } + + if (i>= 4) + return(0); + + tmp = 0xFF; + do { + if (tm[i] == tmp) { + ok = 1; + break; + } + tmp<<= 1; + } while (tmp != 0); + if (ok == 0) + return(-1); + i++; + + if (i>= 4) + return(0); + + for (;i< 4;i++) { + if (tm[i] != 0xFF) + return(-1); + } + } else if (netmask->stor.ss_family == AF_INET6) { + virIPv6Addr tm; + unsigned short tmp; + int ok = 0; + + /* + * Hum, on IPv6 people use prefixes instead of netmask + */ + if (getIPv6Addr(netmask,&tm)< 0) + return(-1); + + for (i = 0;i< 8;i++) { + if (tm[i] != 0) + break; + } + + if (i>= 8) + return(0); + + tmp = 0xFFFF; + do { + if (tm[i] == tmp) { + ok = 1; + break; + } + tmp<<= 1; + } while (tmp != 0); + if (ok == 0) + return(-1); + i++; + + if (i>= 8) + return(0); + + for (;i< 8;i++) { + if (tm[i] != 0xFFFF) + return(-1); + } + } else { + return(-1); + } + return(0); +} + +/** + * virSocketCheckNetmask: + * @addr1: a first network address + * @addr2: a second network address + * @netmask: the netmask address + * + * Check that @addr1 and @addr2 pertain to the same @netmask address + * range and returns the size of the range + * + * Returns 1 in case of success and 0 in case of failure and + * -1 in case of error + */ +int virSocketCheckNetmask(virSocketAddrPtr addr1, virSocketAddrPtr addr2, + virSocketAddrPtr netmask) { + int i; + + if ((addr1 == NULL) || (addr2 == NULL) || (netmask == NULL)) + return(-1); + if ((addr1->stor.ss_family != addr2->stor.ss_family) || + (addr1->stor.ss_family != netmask->stor.ss_family)) + return(-1); + + if (virSocketAddrIsNetmask(netmask) != 0) + return(-1); + + if (addr1->stor.ss_family == AF_INET) { + virIPv4Addr t1, t2, tm; + + if ((getIPv4Addr(addr1,&t1)< 0) || + (getIPv4Addr(addr2,&t2)< 0) || + (getIPv4Addr(netmask,&tm)< 0)) + return(-1); + + for (i = 0;i< 4;i++) { + if ((t1[i]& tm[i]) != (t2[i]& tm[i])) + return(0); + } + + } else if (addr1->stor.ss_family == AF_INET) { + virIPv6Addr t1, t2, tm; + + if ((getIPv6Addr(addr1,&t1)< 0) || + (getIPv6Addr(addr2,&t2)< 0) || + (getIPv6Addr(netmask,&tm)< 0)) + return(-1); + + for (i = 0;i< 8;i++) { + if ((t1[i]& tm[i]) != (t2[i]& tm[i])) + return(0); + } + + } else { + return(-1); + } + return(1); +} + +/** + * virSocketGetRange: + * @start: start of an IP range + * @end: end of an IP range + * + * Check the order of the 2 addresses and compute the range, this + * will return 1 for identical addresses. Errors can come from incompatible + * addresses type, excessive range (>= 2^^16) where the two addresses are + * unrelated or inverted start and end. + * + * Returns the size of the range or -1 in case of failure + */ +int virSocketGetRange(virSocketAddrPtr start, virSocketAddrPtr end) { + int ret = 0, i; + + if ((start == NULL) || (end == NULL)) + return(-1); + if (start->stor.ss_family != end->stor.ss_family) + return(-1); + + if (start->stor.ss_family == AF_INET) { + virIPv4Addr t1, t2; + + if ((getIPv4Addr(start,&t1)< 0) || + (getIPv4Addr(end,&t2)< 0)) + return(-1); + + for (i = 0;i< 2;i++) { + if (t1[i] != t2[i]) + return(-1); + } + ret = (t2[2] - t1[2]) * 256 + (t2[3] - t1[3]); + if (ret< 0) + return(-1); + ret++; + } else if (start->stor.ss_family == AF_INET6) { + virIPv6Addr t1, t2; + + if ((getIPv6Addr(start,&t1)< 0) || + (getIPv6Addr(end,&t2)< 0)) + return(-1); + + for (i = 0;i< 7;i++) { + if (t1[i] != t2[i]) + return(-1); + } + ret = t2[7] - t1[7]; + if (ret< 0) + return(-1); + ret++; + } else { + return(-1); + } + return(ret); +} + diff --git a/src/util/network.h b/src/util/network.h new file mode 100644 index 0000000..e590747 --- /dev/null +++ b/src/util/network.h @@ -0,0 +1,49 @@ +/* + * network.h: network helper APIs for libvirt + * + * Copyright (C) 2009-2009 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software + * + * Daniel Veillard<veillard@redhat.com> + */ + +#ifndef __VIR_NETWORK_H__ +#define __VIR_NETWORK_H__ + +#include "internal.h" + +#include<sys/types.h> +#include<sys/socket.h> +#include<netdb.h> + +typedef union { + struct sockaddr_storage stor; + struct sockaddr_in inet4; + struct sockaddr_in6 inet6; +} virSocketAddr; +typedef virSocketAddr *virSocketAddrPtr; + +int virSocketParseAddr (const char *val, + virSocketAddrPtr addr, + int hint); + +int virSocketParseIpv4Addr(const char *val, + virSocketAddrPtr addr); + +int virSocketParseIpv6Addr(const char *val, + virSocketAddrPtr addr); + +int virSocketAddrInNetwork(virSocketAddrPtr addr1, + virSocketAddrPtr addr2, + virSocketAddrPtr netmask); + +int virSocketGetRange (virSocketAddrPtr start, + virSocketAddrPtr end); + +int virSocketAddrIsNetmask(virSocketAddrPtr netmask); + +int virSocketCheckNetmask (virSocketAddrPtr addr1, + virSocketAddrPtr addr2, + virSocketAddrPtr netmask); +#endif /* __VIR_NETWORK_H__ */
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Oct 22, 2009 at 04:42:20PM -0400, Laine Stump wrote:
+/* + * Helpers to extract the IP arrays from the virSocketAddrPtr + * That part is the less portable of the module + */ +typedef unsigned char virIPv4Addr[4]; +typedef virIPv4Addr *virIPv4AddrPtr; +typedef unsigned short virIPv6Addr[8];
Are you defining this an an array of short for efficiency? It's much easier to deal with if you just look at it as a byte array - you don't need to worry about network vs. host ordering when doing comparisons and bit shifts/masks (IPv6 addresses, like IPv4 addresses, are *always* in network order, even when being passed around in applications - see section 3.2 of http://tools.ietf.org/html/rfc3493.)
No. Those routines and structs are purely internal, and I choose to adopt a structure which match the way addresses are serialized in writing. So if you see 192.168.0.1 and look at the parsed array (in code or in gdb) you see the same thing. This is mostly to make the code understandable. For Ipv6 since in writing people use blocks of values 0000-FFFF is was making sense for me to match that and use an array of short. htonx and ntohx are just here to make code and debug easier to understand. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 10/28/2009 10:35 AM, Daniel Veillard wrote:
On Thu, Oct 22, 2009 at 04:42:20PM -0400, Laine Stump wrote:
+/* + * Helpers to extract the IP arrays from the virSocketAddrPtr + * That part is the less portable of the module + */ +typedef unsigned char virIPv4Addr[4]; +typedef virIPv4Addr *virIPv4AddrPtr; +typedef unsigned short virIPv6Addr[8];
Are you defining this an an array of short for efficiency? It's much easier to deal with if you just look at it as a byte array - you don't need to worry about network vs. host ordering when doing comparisons and bit shifts/masks (IPv6 addresses, like IPv4 addresses, are *always* in network order, even when being passed around in applications - see section 3.2 of http://tools.ietf.org/html/rfc3493.)
No. Those routines and structs are purely internal, and I choose to adopt a structure which match the way addresses are serialized in writing. So if you see 192.168.0.1 and look at the parsed array (in code or in gdb) you see the same thing. This is mostly to make the code understandable. For Ipv6 since in writing people use blocks of values 0000-FFFF is was making sense for me to match that and use an array of short. htonx and ntohx are just here to make code and debug easier to understand.
Okay, now I see that 1) you only have the conversion in one direction, so obviously it's not going to be used in any API, and 2) all the netmask functions are operating from the virSocketAddr rather than the viIPvxAddr. My concern was caused by thinking it would be used in more general ways. Personally I've just gotten used to using /x, and reading the number backwards ;-) (and if it's IPv6, I've only ever got a couple addresses I'm dealing with, and I make sure they're different enough to be easily recognizeable. That will of course change sometime in the next 20 years or so, when IPv6 is actually in common use on production networks :-P) (seriously, I should be using it on my home network. Heck, Red Hat should be routing it through the VPN so we can all globally drink the koolaid!)

On Thu, Oct 22, 2009 at 04:46:51PM +0200, Daniel Veillard wrote:
Revamping my previous patches to parse IPv4/6 numeric addresses, adding range computation and netmask checking entries too.
As suggested I used an union for the socket address type:
typedef union { struct sockaddr_storage stor; struct sockaddr_in inet4; struct sockaddr_in6 inet6; } virSocketAddr; typedef virSocketAddr *virSocketAddrPtr;
But I wonder if it's not better to make it a struct and add the lenght of the address as this is usually needed when passing one of the struct sockaddr_* and that lenght is returned as part of parsing, keeping both together is probably more useful. A macro to get the type AF_INET/AF_INET6 out of a virSocketAddr might be useful instead of having the users manually guess that ->stor.ss_family is teh filed to look at.
Also I wonder how useful the Ipv6 code for netmask and ranges really is, it seems that IPv6 mostly go with prefix and never use netmasks, maybe that's just dead code, maybe that's just wrong, anyway here it is.
I named the header and file network.[ch] as I think other network related utilities like validation or generation of MAC addresses could well be moved in that file too. [..] commit d5ab523c72242d76184838d8215aa0ebe720c08e Author: Daniel Veillard <veillard@redhat.com> Date: Thu Oct 22 16:34:43 2009 +0200
Set of new network related utilities
* src/util/network.h src/util/network.c: utilities to parse network addresses, check netmask and compute ranges
Okay I have pushed this, I will need to update the internal symbols list and adapt my patch 2 and 3 to solve the original bug on dnsmasq that I was aiming at, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

* src/libvirt_private.syms: Add symbols added in 24c8fc5d --- src/libvirt_private.syms | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3997704..96c2b3b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,6 +285,16 @@ virReallocN; virFree; +# network.h +virSocketParseAddr; +virSocketParseIpv4Addr; +virSocketParseIpv6Addr; +virSocketAddrInNetwork; +virSocketGetRange; +virSocketAddrIsNetmask; +virSocketCheckNetmask; + + # network_conf.h virNetworkAssignDef; virNetworkConfigFile; -- 1.6.2.5

--- src/util/network.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/network.c b/src/util/network.c index 8581cdc..abd866c 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -91,11 +91,11 @@ virSocketParseAddr(const char *val, virSocketAddrPtr addr, int hint) { /* * virSocketParseIpv4Addr: * @val: an IPv4 numeric address - * @addr: the loacation to store the result + * @addr: the location to store the result * * Extract the address storage from an IPv4 numeric address * - * Returns the lenght of the network address or -1 in case of error. + * Returns the length of the network address or -1 in case of error. */ int virSocketParseIpv4Addr(const char *val, virSocketAddrPtr addr) { @@ -105,11 +105,11 @@ virSocketParseIpv4Addr(const char *val, virSocketAddrPtr addr) { /* * virSocketParseIpv6Addr: * @val: an IPv6 numeric address - * @addr: the loacation to store the result + * @addr: the location to store the result * * Extract the address storage from an IPv6 numeric address * - * Returns the lenght of the network address or -1 in case of error. + * Returns the length of the network address or -1 in case of error. */ int virSocketParseIpv6Addr(const char *val, virSocketAddrPtr addr) { -- 1.6.2.5

On Fri, Oct 30, 2009 at 03:24:28PM +0000, Matthew Booth wrote:
--- src/util/network.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/network.c b/src/util/network.c
Ah, yup, nice :-) applied ! thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Fri, Oct 30, 2009 at 03:24:27PM +0000, Matthew Booth wrote:
* src/libvirt_private.syms: Add symbols added in 24c8fc5d --- src/libvirt_private.syms | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3997704..96c2b3b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,6 +285,16 @@ virReallocN; virFree;
+# network.h +virSocketParseAddr; +virSocketParseIpv4Addr; +virSocketParseIpv6Addr; +virSocketAddrInNetwork; +virSocketGetRange; +virSocketAddrIsNetmask; +virSocketCheckNetmask; + + # network_conf.h virNetworkAssignDef; virNetworkConfigFile;
Ah, I had that in my tree, sounds like an ACK :-) I just prefer mine because it's sorted alphabetically ;-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard
-
Laine Stump
-
Matthew Booth