[libvirt] [PATCH] 1/3 Add IPv4 and IPv6 parsing routines

[Daniel P. Berrange]

On Wed, Oct 14, 2009 at 12:51:32PM +0200, Daniel Veillard wrote:
> On Wed, Oct 14, 2009 at 10:49:20AM +0100, Daniel P. Berrange wrote:
> > On Tue, Oct 13, 2009 at 04:12:47PM +0200, Daniel Veillard wrote:
> > >     * src/util/util.h src/util/util.c: two new functions virParseIPv4
> > >       and virParseIPv6
> > 
> > I think this should just be a thin wrapper around getaddrinfo()
> > which already knows how to parse all possible address types.
> 
>   Are we allowing all possible address types in the XML ?

Well at this time we only allow IPv4 addreses in this, so we
would want to pass the AF_INET  flag to restrict  it. Other
areas of libvirt which could uses this code would be more
flexible.  We've got an open RFE for IPv6 support 

https://bugzilla.redhat.com/show_bug.cgi?id=514749

> I based that parsing routing in a large part as a syntactic check
> on the allowed set of addresses. For example I didn't allow
> 
>    ::ffff:12.34.56.78
> 
> kind of IPv6 syntax.

Why shouldn't we allow that - its valid IPv6 address syntax }

> > If we avoid a custom typedef, and just use 'struct sockaddr_storage'
> > this in turn makes it easy for callers to pass the result straight
> > into any socket API calls they might use. eg this could all be done
> > with a couple of lines of code
> > 
> > 
> > That would automatically cope with both IPv4 / 6 addresses. If
> > we want to restrict it we could add a 3rd argument, 'int family'
> > and use that to set  hints.ai_family field - the caller would
> > just pass AF_INET or AF_INET6 to specify a particular type, or
> > leave it at 0 to allow any type.
> 
>   Dunno, I find the getaddrinfo interface and the hints stuff
> fairly incomprehensible. When I see the OpenGroup definition of
> struct sockaddr_storage
> 
> struct sockaddr_storage {
>     sa_family_t  ss_family;  /* Address family. */
> /*
>  *  Following fields are implementation-defined.
>  */
>     char _ss_pad1[_SS_PAD1SIZE];
>         /* 6-byte pad; this is to make implementation-defined
>            pad up to alignment field that follows explicit in
>            the data structure. */
>     int64_t _ss_align;  /* Field to force desired structure
>                            storage alignment. */
>     char _ss_pad2[_SS_PAD2SIZE];
>         /* 112-byte pad to achieve desired size,
>            _SS_MAXSIZE value minus size of ss_family
>            __ss_pad1, __ss_align fields is 112. */
> };

NB it is not intended to use the internals of the sockaddr_storage
struct, with the exception of the ss_family field. It is provided
as a struct that is guarenteed large enough to store any type of
address. To use the data, you'd cast to the appropriate  struct
based on ss_family.

eg, if  ss_family == AF_INET, then you can cast to

   struct sockaddr_in

which has a 'struct in_addr sin_addr' field available to get
the raw address - in this case 'in_addr' is just an int32

For AF_INET6, you would cast to sockaddr_in6, which has a
'struct in6_addr sin6_addr' which gives you easy acess to
the 16 byte array of the address.

>  I'm not too enthusiastic about using this for internal APIs.
> And I don't see how I would check ranges for the IP addresses based
> on this. Actually I don't find it helps to calculate a range,
> and I prefer my good old arrays of well defined ints for that purpose.

The sockaddr_in/in6 structs both give you an array of bytes in exactly
the same manner

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|