2:03 a.m.
Finally!
Most of this patchset is setup for patch 09/13, which updates the
network XML parser to support IPv6, and 12/13, which turns on IPv6 in
the bridge driver.
In order to have each patch individually pass make check and
(otherwise function properly in case someone is doing a bisect), there
is a bit of extra code churn (lines that are changed in one patch,
only to be changed again in a later patch); I tried to minimize this
as much as possible.
For for IPv6 to work correctly, target *and build* systems will
now need to have ip6tables and radvd available. The way I added
ip6tables into autoconfigure.ac is identical to existing iptables, and
the way I added radvd is identical to dnsmasq. Unfortunately this
doesn't communicate the requirement downstream in a programmatic
fashion, so I guess we need to make sure that this is adequately
reported in the next set of release notes.
There is a remaining problem, outlined in PATCH 13/13, which I would
like some input on - there is a race between radvd writing its pidfile
and libvirtd reading it, and a couple possible ways to fix it, neither
of which is optimal. If anyone has a different idea, I'd like to hear
it (I'm leaning towards option 2 right now - it creates an extra file,
but behavior is 100% correct).
2:03 a.m.
New subject: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
virSocketPrefixToNetmask: Given a 'prefix', which is the number of 1
bits in a netmask, fill in a virSocketAddr object with a netmask as an
IP address (IPv6 or IPv4).
virSocketAddrMask: Mask off the host bits in one virSocketAddr
according to the netmask in another virSocketAddr.
virSocketAddrMaskByPrefix, Mask off the host bits in a virSocketAddr
according to a prefix (number of 1 bits in netmask).
VIR_SOCKET_FAMILY: return the family of a virSocketAddr
---
src/libvirt_private.syms | 3 +
src/util/network.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/network.h | 10 ++++
3 files changed, 143 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0e3033d..19841ef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -559,6 +559,9 @@ virShrinkN;
# network.h
virSocketAddrIsNetmask;
+virSocketAddrMask;
+virSocketAddrMaskByPrefix;
+virSocketAddrPrefixToNetmask;
virSocketCheckNetmask;
virSocketFormatAddr;
virSocketFormatAddrFull;
diff --git a/src/util/network.c b/src/util/network.c
index 1abe78b..e4791b9 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) {
}
/**
+ * virSocketAddrMask:
+ * @addr: address that needs to be masked
+ * @netmask: the netmask address
+ *
+ * Mask off the host bits of @addr according to @netmask, turning it
+ * into a network address.
+ *
+ * Returns 0 in case of success, or -1 on error.
+ */
+int
+virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask)
+{
+ if (addr->data.stor.ss_family != netmask->data.stor.ss_family)
+ return -1;
+
+ if (addr->data.stor.ss_family == AF_INET) {
+ addr->data.inet4.sin_addr.s_addr
+ &= netmask->data.inet4.sin_addr.s_addr;
+ return 0;
+ }
+ if (addr->data.stor.ss_family == AF_INET6) {
+ int ii;
+ for (ii = 0; ii < 4; ii++)
+ addr->data.inet6.sin6_addr.s6_addr32[ii]
+ &= netmask->data.inet6.sin6_addr.s6_addr32[ii];
+ return 0;
+ }
+ return -1;
+}
+
+/**
+ * virSocketAddrMaskByPrefix:
+ * @addr: address that needs to be masked
+ * @prefix: prefix (# of 1 bits) of netmask to apply
+ *
+ * Mask off the host bits of @addr according to @prefix, turning it
+ * into a network address.
+ *
+ * Returns 0 in case of success, or -1 on error.
+ */
+int
+virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
+{
+ virSocketAddr netmask;
+
+ if (virSocketAddrPrefixToNetmask(prefix, &netmask,
+ addr->data.stor.ss_family) < 0)
+ return -1;
+
+ if (addr->data.stor.ss_family == AF_INET) {
+
+ addr->data.inet4.sin_addr.s_addr
+ &= netmask.data.inet4.sin_addr.s_addr;
+ return 0;
+ }
+ if (addr->data.stor.ss_family == AF_INET6) {
+ int ii;
+
+ for (ii = 0; ii < 4; ii++)
+ addr->data.inet6.sin6_addr.s6_addr32[ii]
+ &= netmask.data.inet6.sin6_addr.s6_addr32[ii];
+ return 0;
+ }
+ return -1;
+}
+
+/**
* virSocketCheckNetmask:
* @addr1: a first network address
* @addr2: a second network address
@@ -486,3 +553,66 @@ int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask)
}
return -1;
}
+
+/**
+ * virSocketPrefixToNetmask:
+ * @prefix: number of 1 bits to put in the netmask
+ * @netmask: address to fill in with the desired netmask
+ * @family: family of the address (AF_INET or AF_INET6 only)
+ *
+ * given @prefix and @family, fill in @netmask with a netmask
+ * (eg 255.255.255.0).
+ *
+ * Returns 0 on success or -1 on error.
+ */
+
+int virSocketAddrPrefixToNetmask(int prefix,
+ virSocketAddrPtr netmask,
+ int family)
+{
+ int result = -1;
+
+ netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */
+
+ if (family == AF_INET) {
+ int ip = 0;
+
+ if (prefix < 0 || prefix > 32)
+ goto error;
+
+ while (prefix-- > 0) {
+ ip >>= 1;
+ ip |= 0x80000000;
+ }
+ netmask->data.inet4.sin_addr.s_addr = htonl(ip);
+ netmask->data.stor.ss_family = AF_INET;
+ result = 0;
+
+ } else if (family == AF_INET6) {
+ int ii = 0;
+
+ if (prefix > 128)
+ goto error;
+
+ while (prefix >= 8) {
+ /* do as much as possible an entire byte at a time */
+ netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0xff;
+ prefix -= 8;
+ }
+ while (prefix-- > 0) {
+ /* final partial byte one bit at a time */
+ netmask->data.inet6.sin6_addr.s6_addr[ii] >>= 1;
+ netmask->data.inet6.sin6_addr.s6_addr[ii] |= 0x80;
+ }
+ ii++;
+ while (ii < 16) {
+ /* zerofill remainder in case it wasn't initialized */
+ netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0;
+ }
+ netmask->data.stor.ss_family = AF_INET6;
+ result = 0;
+ }
+
+error:
+ return result;
+}
diff --git a/src/util/network.h b/src/util/network.h
index 521f466..6e1394f 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -41,6 +41,9 @@ typedef struct {
# define VIR_SOCKET_IS_FAMILY(s, f) \
((s)->data.sa.sa_family == f)
+# define VIR_SOCKET_FAMILY(s) \
+ ((s)->data.sa.sa_family)
+
typedef virSocketAddr *virSocketAddrPtr;
int virSocketParseAddr (const char *val,
@@ -70,7 +73,14 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask);
int virSocketCheckNetmask (virSocketAddrPtr addr1,
virSocketAddrPtr addr2,
virSocketAddrPtr netmask);
+int virSocketAddrMask (virSocketAddrPtr addr,
+ const virSocketAddrPtr netmask);
+int virSocketAddrMaskByPrefix(virSocketAddrPtr addr,
+ int prefix);
int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask);
+int virSocketAddrPrefixToNetmask(int prefix,
+ virSocketAddrPtr netmask,
+ int family);
#endif /* __VIR_NETWORK_H__ */
--
1.7.3.4
4:45 p.m.
New subject: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On Mon, Dec 20, 2010 at 09:03, Laine Stump <laine(a)laine.org> wrote:
diff --git a/src/util/network.c b/src/util/network.c
index 1abe78b..e4791b9 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) {
}
/**
+ * virSocketAddrMask:
+ * @addr: address that needs to be masked
+ * @netmask: the netmask address
+ *
+ * Mask off the host bits of @addr according to @netmask, turning it
+ * into a network address.
+ *
+ * Returns 0 in case of success, or -1 on error.
+ */
+int
+virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask)
+{
+ if (addr->data.stor.ss_family != netmask->data.stor.ss_family)
+ return -1;
+
+ if (addr->data.stor.ss_family == AF_INET) {
+ addr->data.inet4.sin_addr.s_addr
+ &= netmask->data.inet4.sin_addr.s_addr;
+ return 0;
+ }
+ if (addr->data.stor.ss_family == AF_INET6) {
+ int ii;
+ for (ii = 0; ii < 4; ii++)
+ addr->data.inet6.sin6_addr.s6_addr32[ii]
+ &= netmask->data.inet6.sin6_addr.s6_addr32[ii];
+ return 0;
+ }
+ return -1;
+}
+
+/**
+ * virSocketAddrMaskByPrefix:
+ * @addr: address that needs to be masked
+ * @prefix: prefix (# of 1 bits) of netmask to apply
+ *
+ * Mask off the host bits of @addr according to @prefix, turning it
+ * into a network address.
+ *
+ * Returns 0 in case of success, or -1 on error.
+ */
+int
+virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
+{
+ virSocketAddr netmask;
+
+ if (virSocketAddrPrefixToNetmask(prefix, &netmask,
+ addr->data.stor.ss_family) < 0)
+ return -1;
why not replace following:
+ if (addr->data.stor.ss_family == AF_INET) {
+
+ addr->data.inet4.sin_addr.s_addr
+ &= netmask.data.inet4.sin_addr.s_addr;
+ return 0;
+ }
+ if (addr->data.stor.ss_family == AF_INET6) {
+ int ii;
+
+ for (ii = 0; ii < 4; ii++)
+ addr->data.inet6.sin6_addr.s6_addr32[ii]
+ &= netmask.data.inet6.sin6_addr.s6_addr32[ii];
+ return 0;
+ }
+ return -1;
+}
with simple:
return virSocketAddrMask(addr, netmask);
With cost of checking ss_family for addr and netmask we achieve clearer code.
+/**
+ * virSocketPrefixToNetmask:
+ * @prefix: number of 1 bits to put in the netmask
+ * @netmask: address to fill in with the desired netmask
+ * @family: family of the address (AF_INET or AF_INET6 only)
+ *
+ * given @prefix and @family, fill in @netmask with a netmask
+ * (eg 255.255.255.0).
+ *
+ * Returns 0 on success or -1 on error.
+ */
+
+int virSocketAddrPrefixToNetmask(int prefix,
+ virSocketAddrPtr netmask,
+ int family)
+{
+ int result = -1;
+
+ netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */
You don't check if (prefix < 0) for AF_INET6. So my suggestion is to
check it here:
if (prefix < 0)
goto error;
+ if (family == AF_INET) {
+ int ip = 0;
+
and remove this check from here:
+ if (prefix < 0 || prefix > 32)
+ goto error;
+
btw: does prefix really needs to be signed int?
--
Pawel
3:35 p.m.
New subject: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On 12/20/2010 05:45 PM, Paweł Krześniak wrote:
On Mon, Dec 20, 2010 at 09:03, Laine Stump<laine(a)laine.org>
wrote:
> diff --git a/src/util/network.c b/src/util/network.c
> index 1abe78b..e4791b9 100644
> --- a/src/util/network.c
> +++ b/src/util/network.c
> @@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) {
> }
>
> /**
> + * virSocketAddrMask:
> + * @addr: address that needs to be masked
> + * @netmask: the netmask address
> + *
> + * Mask off the host bits of @addr according to @netmask, turning it
> + * into a network address.
> + *
> + * Returns 0 in case of success, or -1 on error.
> + */
> +int
> +virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask)
> +{
> + if (addr->data.stor.ss_family != netmask->data.stor.ss_family)
> + return -1;
> +
> + if (addr->data.stor.ss_family == AF_INET) {
> + addr->data.inet4.sin_addr.s_addr
> +&= netmask->data.inet4.sin_addr.s_addr;
> + return 0;
> + }
> + if (addr->data.stor.ss_family == AF_INET6) {
> + int ii;
> + for (ii = 0; ii< 4; ii++)
> + addr->data.inet6.sin6_addr.s6_addr32[ii]
> +&= netmask->data.inet6.sin6_addr.s6_addr32[ii];
> + return 0;
> + }
> + return -1;
> +}
> +
> +/**
> + * virSocketAddrMaskByPrefix:
> + * @addr: address that needs to be masked
> + * @prefix: prefix (# of 1 bits) of netmask to apply
> + *
> + * Mask off the host bits of @addr according to @prefix, turning it
> + * into a network address.
> + *
> + * Returns 0 in case of success, or -1 on error.
> + */
> +int
> +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
> +{
> + virSocketAddr netmask;
> +
> + if (virSocketAddrPrefixToNetmask(prefix,&netmask,
> + addr->data.stor.ss_family)< 0)
> + return -1;
why not replace following:
> + if (addr->data.stor.ss_family == AF_INET) {
> +
> + addr->data.inet4.sin_addr.s_addr
> +&= netmask.data.inet4.sin_addr.s_addr;
> + return 0;
> + }
> + if (addr->data.stor.ss_family == AF_INET6) {
> + int ii;
> +
> + for (ii = 0; ii< 4; ii++)
> + addr->data.inet6.sin6_addr.s6_addr32[ii]
> +&= netmask.data.inet6.sin6_addr.s6_addr32[ii];
> + return 0;
> + }
> + return -1;
> +}
with simple:
return virSocketAddrMask(addr, netmask);
With cost of checking ss_family for addr and netmask we achieve clearer code.
Right. Eric also suggested this, and I can't even tell you why I didn't
do it that way to begin with. :-P
> +/**
> + * virSocketPrefixToNetmask:
> + * @prefix: number of 1 bits to put in the netmask
> + * @netmask: address to fill in with the desired netmask
> + * @family: family of the address (AF_INET or AF_INET6 only)
> + *
> + * given @prefix and @family, fill in @netmask with a netmask
> + * (eg 255.255.255.0).
> + *
> + * Returns 0 on success or -1 on error.
> + */
> +
> +int virSocketAddrPrefixToNetmask(int prefix,
> + virSocketAddrPtr netmask,
> + int family)
> +{
> + int result = -1;
> +
> + netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */
You don't check if (prefix< 0) for AF_INET6. So my suggestion is to
check it here:
if (prefix< 0)
goto error;
> + if (family == AF_INET) {
> + int ip = 0;
> +
and remove this check from here:
> + if (prefix< 0 || prefix> 32)
> + goto error;
> +
btw: does prefix really needs to be signed int?
No. I had it that way for consistency with prefix return values (which
are ints so we can return -1 for an error), but after your and Eric's
comments, I'm changing them to unsigned int (but leaving the return
values as ints).
v2 is coming up. Thanks for the comments!
5:40 p.m.
New subject: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On 12/20/2010 01:03 AM, Laine Stump wrote:
virSocketPrefixToNetmask: Given a 'prefix', which is the
number of 1
bits in a netmask, fill in a virSocketAddr object with a netmask as an
IP address (IPv6 or IPv4).
virSocketAddrMask: Mask off the host bits in one virSocketAddr
according to the netmask in another virSocketAddr.
virSocketAddrMaskByPrefix, Mask off the host bits in a virSocketAddr
according to a prefix (number of 1 bits in netmask).
VIR_SOCKET_FAMILY: return the family of a virSocketAddr
All sound quite useful.
+ if (addr->data.stor.ss_family == AF_INET6) {
+ int ii;
+ for (ii = 0; ii < 4; ii++)
+ addr->data.inet6.sin6_addr.s6_addr32[ii]
+ &= netmask->data.inet6.sin6_addr.s6_addr32[ii];
Not portable. Nothing standardizes the existence of s6_addr32[4], and
not all platforms provide this convenience alias. Instead, you'll have
to iterate over s6_addr[16] bytes.
+int
+virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
s/int prefix/unsigned &/
+{
+ virSocketAddr netmask;
+
+ if (virSocketAddrPrefixToNetmask(prefix, &netmask,
+ addr->data.stor.ss_family) < 0)
+ return -1;
Avoid code duplication; use virSocketAddrMask to do the masking.
+int virSocketAddrPrefixToNetmask(int prefix,
For consistency with the rest of the file, put a newline after the
return type and start virSocketAddrPrefixToNetmask in the first column.
Again, use unsigned int for prefix.
+ if (family == AF_INET) {
+ int ip = 0;
+
+ if (prefix < 0 || prefix > 32)
+ goto error;
+
+ while (prefix-- > 0) {
+ ip >>= 1;
+ ip |= 0x80000000;
Bit-wise loops are slow compared to direct computation. Why not just:
if (prefix == 0)
ip = 0;
else
ip = ~((1 << (32 - prefix)) - 1);
+ } else if (family == AF_INET6) {
+ int ii = 0;
+
+ if (prefix > 128)
Technically, we could use (CHAR_BIT * sizeof
netmask->data.inet6.sin6_addr.s6_addr) instead of 128, and so forth, but
the magic numbers used in this function aren't too hard to see without
having to hide them behind named constants, so I'm fine with keeping 128.
+ goto error;
Another argument to make prefix unsigned, since you didn't check for
negative values.
+
+ while (prefix >= 8) {
+ /* do as much as possible an entire byte at a time */
+ netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0xff;
+ prefix -= 8;
+ }
okay.
+ while (prefix-- > 0) {
+ /* final partial byte one bit at a time */
+ netmask->data.inet6.sin6_addr.s6_addr[ii] >>= 1;
+ netmask->data.inet6.sin6_addr.s6_addr[ii] |= 0x80;
+ }
+ ii++;
Given your assumption that you are not starting from an initialized
value, this loop ends up putting garbage in the low half of the byte.
Fix that, and avoid the bitwise loop at the same time, by replacing
those six lines with:
if (prefix > 0) {
netmask->data.inet6.sin6_addr.s6_addr[ii++] =
~((1 << (8 - prefix)) - 1);
}
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:42 p.m.
New subject: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On 12/20/2010 06:40 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
> virSocketPrefixToNetmask: Given a 'prefix', which is the number of 1
> bits in a netmask, fill in a virSocketAddr object with a netmask as an
> IP address (IPv6 or IPv4).
>
> virSocketAddrMask: Mask off the host bits in one virSocketAddr
> according to the netmask in another virSocketAddr.
>
> virSocketAddrMaskByPrefix, Mask off the host bits in a virSocketAddr
> according to a prefix (number of 1 bits in netmask).
>
> VIR_SOCKET_FAMILY: return the family of a virSocketAddr
All sound quite useful.
> + if (addr->data.stor.ss_family == AF_INET6) {
> + int ii;
> + for (ii = 0; ii< 4; ii++)
> + addr->data.inet6.sin6_addr.s6_addr32[ii]
> +&= netmask->data.inet6.sin6_addr.s6_addr32[ii];
Not portable. Nothing standardizes the existence of s6_addr32[4], and
not all platforms provide this convenience alias. Instead, you'll have
to iterate over s6_addr[16] bytes.
Fixed.
> +int
> +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
s/int prefix/unsigned&/
Okay, I've changed all of these (except for return values which are
prefix - those will remain int so that -1 can be returned for error).
> +{
> + virSocketAddr netmask;
> +
> + if (virSocketAddrPrefixToNetmask(prefix,&netmask,
> + addr->data.stor.ss_family)< 0)
> + return -1;
Avoid code duplication; use virSocketAddrMask to do the masking.
Right. Not sure why I didn't do that to begin with.
> +int virSocketAddrPrefixToNetmask(int prefix,
For consistency with the rest of the file, put a newline after the
return type and start virSocketAddrPrefixToNetmask in the first column.
Again, use unsigned int for prefix.
Yep. I meant to put the return type on a separate line everywhere, but
this one slipped.
> + if (family == AF_INET) {
> + int ip = 0;
> +
> + if (prefix< 0 || prefix> 32)
> + goto error;
> +
> + while (prefix--> 0) {
> + ip>>= 1;
> + ip |= 0x80000000;
Bit-wise loops are slow compared to direct computation. Why not just:
if (prefix == 0)
ip = 0;
else
ip = ~((1<< (32 - prefix)) - 1);
Or
ip = prefix ? ~((1 << (32 - prefix)) - 1) : 0;
:-)
> + } else if (family == AF_INET6) {
> + int ii = 0;
> +
> + if (prefix> 128)
Technically, we could use (CHAR_BIT * sizeof
netmask->data.inet6.sin6_addr.s6_addr) instead of 128, and so forth, but
the magic numbers used in this function aren't too hard to see without
having to hide them behind named constants, so I'm fine with keeping 128.
> + goto error;
Another argument to make prefix unsigned, since you didn't check for
negative values.
> +
> + while (prefix>= 8) {
> + /* do as much as possible an entire byte at a time */
> + netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0xff;
> + prefix -= 8;
> + }
okay.
> + while (prefix--> 0) {
> + /* final partial byte one bit at a time */
> + netmask->data.inet6.sin6_addr.s6_addr[ii]>>= 1;
> + netmask->data.inet6.sin6_addr.s6_addr[ii] |= 0x80;
> + }
> + ii++;
Given your assumption that you are not starting from an initialized
value, this loop ends up putting garbage in the low half of the byte.
Fix that, and avoid the bitwise loop at the same time, by replacing
those six lines with:
if (prefix> 0) {
netmask->data.inet6.sin6_addr.s6_addr[ii++] =
~((1<< (8 - prefix)) - 1);
}
Done.
Thanks for the thorough review. v2 coming up!
2:03 a.m.
New subject: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
Later patches will add the possibility to define a network's netmask
as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to
deal with definition of both kinds (prefix or netmask), add two new
functions:
virNetworkDefNetmask: return a copy of the netmask into a
virSocketAddr. If no netmask was specified in the XML, create a
default netmask based on the network class of the virNetworkDef's IP
address.
virNetworkDefPrefix: return the netmask as numeric prefix (or the
default prefix for the network class of the virNetworkDef's IP
address, if no netmask was specified in the XML)
---
src/conf/network_conf.c | 37 +++++++++++++++++++++++++++++++++++++
src/conf/network_conf.h | 3 +++
src/libvirt_private.syms | 2 ++
3 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b469269..b6ac4e3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -207,6 +207,43 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
}
}
+/* return number of 1 bits in netmask for the network's ipAddress,
+ * or -1 on error
+ */
+int virNetworkDefPrefix(const virNetworkDefPtr def)
+{
+ if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
+ return virSocketGetNumNetmaskBits(&def->netmask);
+ } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) {
+ /* return the natural prefix for the network's ip address */
+ int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr);
+ if (IN_CLASSA(addr))
+ return 8;
+ if (IN_CLASSB(addr))
+ return 16;
+ if (IN_CLASSC(addr))
+ return 24;
+ return -1;
+ }
+ return -1;
+}
+
+/* Fill in a virSocketAddr with the proper netmask for this
+ * definition, based on either the definition's netmask, or its
+ * prefix. Return -1 on error (and set the netmask family to AF_UNSPEC)
+ */
+int virNetworkDefNetmask(const virNetworkDefPtr def,
+ virSocketAddrPtr netmask)
+{
+ if (VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
+ *netmask = def->netmask;
+ return 0;
+ }
+
+ return virSocketAddrPrefixToNetmask(virNetworkDefPrefix(def), netmask,
+ VIR_SOCKET_FAMILY(&def->ipAddress));
+}
+
static int
virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 7d31693..a922d28 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -133,6 +133,9 @@ virNetworkDefPtr virNetworkDefParseNode(xmlDocPtr xml,
char *virNetworkDefFormat(const virNetworkDefPtr def);
+int virNetworkDefPrefix(const virNetworkDefPtr def);
+int virNetworkDefNetmask(const virNetworkDefPtr def,
+ virSocketAddrPtr netmask);
int virNetworkSaveXML(const char *configDir,
virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 19841ef..d9ba7b1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -578,9 +578,11 @@ virNetworkAssignDef;
virNetworkConfigFile;
virNetworkDefFormat;
virNetworkDefFree;
+virNetworkDefNetmask;
virNetworkDefParseFile;
virNetworkDefParseNode;
virNetworkDefParseString;
+virNetworkDefPrefix;
virNetworkDeleteConfig;
virNetworkFindByName;
virNetworkFindByUUID;
--
1.7.3.4
5:52 p.m.
New subject: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
On 12/20/2010 01:03 AM, Laine Stump wrote:
Later patches will add the possibility to define a network's
netmask
as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to
deal with definition of both kinds (prefix or netmask), add two new
functions:
virNetworkDefNetmask: return a copy of the netmask into a
virSocketAddr. If no netmask was specified in the XML, create a
default netmask based on the network class of the virNetworkDef's IP
address.
virNetworkDefPrefix: return the netmask as numeric prefix (or the
default prefix for the network class of the virNetworkDef's IP
address, if no netmask was specified in the XML)
What happens if the user specifies a netmask in the XML that is
non-contiguous (bad practice, but some routers do allow it)?
+/* return number of 1 bits in netmask for the network's ipAddress,
+ * or -1 on error
+ */
+int virNetworkDefPrefix(const virNetworkDefPtr def)
Should this return different values, such as -1 if network class not
determined, and -2 if netmask was specified but non-contiguous? Or do
callers not care?
+{
+ if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
+ return virSocketGetNumNetmaskBits(&def->netmask);
[hmm, back to my theme of preferring direct operations over bitwise
loops, I notice that the existing GetNumNetmaksBits implementation could
be independently optimized]
+ } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress,
AF_INET)) {
+ /* return the natural prefix for the network's ip address */
+ int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr);
+ if (IN_CLASSA(addr))
Where is this defined? Oh, I found it - in <netinet/in.h>, but only as
a Linux extension. Since POSIX doesn't require it to exist, you'll need
to take care to provide fallback definitions for this macro and its friends.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:52 p.m.
New subject: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
On 12/20/2010 06:52 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
> Later patches will add the possibility to define a network's netmask
> as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to
> deal with definition of both kinds (prefix or netmask), add two new
> functions:
>
> virNetworkDefNetmask: return a copy of the netmask into a
> virSocketAddr. If no netmask was specified in the XML, create a
> default netmask based on the network class of the virNetworkDef's IP
> address.
>
> virNetworkDefPrefix: return the netmask as numeric prefix (or the
> default prefix for the network class of the virNetworkDef's IP
> address, if no netmask was specified in the XML)
What happens if the user specifies a netmask in the XML that is
non-contiguous (bad practice, but some routers do allow it)?
If that's the case,
>
> +/* return number of 1 bits in netmask for the network's ipAddress,
> + * or -1 on error
> + */
> +int virNetworkDefPrefix(const virNetworkDefPtr def)
Should this return different values, such as -1 if network class not
determined, and -2 if netmask was specified but non-contiguous? Or do
callers not care?
> +{
> + if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
> + return virSocketGetNumNetmaskBits(&def->netmask);
[hmm, back to my theme of preferring direct operations over bitwise
loops, I notice that the existing GetNumNetmaksBits implementation could
be independently optimized]
> + } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) {
> + /* return the natural prefix for the network's ip address */
> + int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr);
> + if (IN_CLASSA(addr))
Where is this defined? Oh, I found it - in<netinet/in.h>, but only as
a Linux extension. Since POSIX doesn't require it to exist, you'll need
to take care to provide fallback definitions for this macro and its friends.
If the raw numbers have to be in there anyway, I'd rather just do it by
hand for all platforms.
5:25 p.m.
New subject: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
On 12/21/2010 04:52 PM, Laine Stump wrote:
On 12/20/2010 06:52 PM, Eric Blake wrote:
> On 12/20/2010 01:03 AM, Laine Stump wrote:
>> Later patches will add the possibility to define a network's netmask
>> as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to
>> deal with definition of both kinds (prefix or netmask), add two new
>> functions:
>>
>> virNetworkDefNetmask: return a copy of the netmask into a
>> virSocketAddr. If no netmask was specified in the XML, create a
>> default netmask based on the network class of the virNetworkDef's IP
>> address.
>>
>> virNetworkDefPrefix: return the netmask as numeric prefix (or the
>> default prefix for the network class of the virNetworkDef's IP
>> address, if no netmask was specified in the XML)
> What happens if the user specifies a netmask in the XML that is
> non-contiguous (bad practice, but some routers do allow it)?
If that's the case,
Sorry, I accidentally hit send before I finished my thought...
If someone wants to use a non-contiguous netmask, they can't use
libvirt's virtual networks anyway, because iptables doesn't work with
non-contiguous netmasks (netmask is specified as a prefix in all
iptables commands - a later patch in this series takes advantage of that
to simplify things).
So I don't think it's something we need to be concerned about.
2:03 a.m.
New subject: [libvirt] [PATCH 03/13] Fix logging of failed iptables commands
The functions in iptables.c all return -1 on failure, but all their
callers (which all happen to be in bridge_driver.c) assume that they
are returning an errno, and the logging is done accordingly. This
patch fixes all the error checking and logging to assume < 0 is an
error, and nothing else.
---
src/network/bridge_driver.c | 183 +++++++++++++++++++++----------------------
1 files changed, 91 insertions(+), 92 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c3f32d7..5186511 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -586,28 +586,28 @@ cleanup:
static int
networkAddMasqueradingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network) {
- int err;
+
/* allow forwarding packets from the bridge interface */
- if ((err = iptablesAddForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->bridge,
- network->def->forwardDev))) {
- virReportSystemError(err,
- _("failed to add iptables rule to allow forwarding from
'%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowOut(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->bridge,
+ network->def->forwardDev) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow forwarding from
'%s'"),
+ network->def->bridge);
goto masqerr1;
}
/* allow forwarding packets to the bridge interface if they are part of an existing
connection */
- if ((err = iptablesAddForwardAllowRelatedIn(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->bridge,
- network->def->forwardDev))) {
- virReportSystemError(err,
- _("failed to add iptables rule to allow forwarding to
'%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowRelatedIn(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->bridge,
+ network->def->forwardDev) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow forwarding to
'%s'"),
+ network->def->bridge);
goto masqerr2;
}
@@ -635,38 +635,38 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
*/
/* First the generic masquerade rule for other protocols */
- if ((err = iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->forwardDev,
- NULL))) {
- virReportSystemError(err,
- _("failed to add iptables rule to enable masquerading
to '%s'"),
- network->def->forwardDev ?
network->def->forwardDev : NULL);
+ if (iptablesAddForwardMasquerade(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->forwardDev,
+ NULL) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to enable masquerading to
'%s'"),
+ network->def->forwardDev ?
network->def->forwardDev : NULL);
goto masqerr3;
}
/* UDP with a source port restriction */
- if ((err = iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->forwardDev,
- "udp"))) {
- virReportSystemError(err,
- _("failed to add iptables rule to enable UDP
masquerading to '%s'"),
- network->def->forwardDev ?
network->def->forwardDev : NULL);
+ if (iptablesAddForwardMasquerade(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->forwardDev,
+ "udp") < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to enable UDP masquerading
to '%s'"),
+ network->def->forwardDev ?
network->def->forwardDev : NULL);
goto masqerr4;
}
/* TCP with a source port restriction */
- if ((err = iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->forwardDev,
- "tcp"))) {
- virReportSystemError(err,
- _("failed to add iptables rule to enable TCP
masquerading to '%s'"),
- network->def->forwardDev ?
network->def->forwardDev : NULL);
+ if (iptablesAddForwardMasquerade(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->forwardDev,
+ "tcp") < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to enable TCP masquerading
to '%s'"),
+ network->def->forwardDev ?
network->def->forwardDev : NULL);
goto masqerr5;
}
@@ -703,28 +703,28 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
static int
networkAddRoutingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network) {
- int err;
+
/* allow routing packets from the bridge interface */
- if ((err = iptablesAddForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->bridge,
- network->def->forwardDev))) {
- virReportSystemError(err,
- _("failed to add iptables rule to allow routing from
'%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowOut(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->bridge,
+ network->def->forwardDev) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow routing from
'%s'"),
+ network->def->bridge);
goto routeerr1;
}
/* allow routing packets to the bridge interface */
- if ((err = iptablesAddForwardAllowIn(driver->iptables,
- &network->def->ipAddress,
- &network->def->netmask,
- network->def->bridge,
- network->def->forwardDev))) {
- virReportSystemError(err,
- _("failed to add iptables rule to allow routing to
'%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowIn(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->bridge,
+ network->def->forwardDev) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow routing to
'%s'"),
+ network->def->bridge);
goto routeerr2;
}
@@ -744,69 +744,68 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
static int
networkAddIptablesRules(struct network_driver *driver,
virNetworkObjPtr network) {
- int err;
/* allow DHCP requests through to dnsmasq */
- if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 67)))
{
- virReportSystemError(err,
- _("failed to add iptables rule to allow DHCP requests
from '%s'"),
- network->def->bridge);
+ if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0)
{
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow DHCP requests
from '%s'"),
+ network->def->bridge);
goto err1;
}
- if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 67)))
{
- virReportSystemError(err,
- _("failed to add iptables rule to allow DHCP requests
from '%s'"),
- network->def->bridge);
+ if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 67) < 0)
{
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow DHCP requests
from '%s'"),
+ network->def->bridge);
goto err2;
}
/* allow DNS requests through to dnsmasq */
- if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 53)))
{
- virReportSystemError(err,
- _("failed to add iptables rule to allow DNS requests
from '%s'"),
- network->def->bridge);
+ if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 53) < 0)
{
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow DNS requests from
'%s'"),
+ network->def->bridge);
goto err3;
}
- if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 53)))
{
- virReportSystemError(err,
- _("failed to add iptables rule to allow DNS requests
from '%s'"),
- network->def->bridge);
+ if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 53) < 0)
{
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow DNS requests from
'%s'"),
+ network->def->bridge);
goto err4;
}
/* allow TFTP requests through to dnsmasq */
if (network->def->tftproot &&
- (err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 69)))
{
- virReportSystemError(err,
- _("failed to add iptables rule to allow TFTP requests
from '%s'"),
- network->def->bridge);
+ iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0)
{
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow TFTP requests
from '%s'"),
+ network->def->bridge);
goto err4tftp;
}
/* Catch all rules to block forwarding to/from bridges */
- if ((err = iptablesAddForwardRejectOut(driver->iptables,
network->def->bridge))) {
- virReportSystemError(err,
- _("failed to add iptables rule to block outbound
traffic from '%s'"),
- network->def->bridge);
+ if (iptablesAddForwardRejectOut(driver->iptables, network->def->bridge) <
0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to block outbound traffic
from '%s'"),
+ network->def->bridge);
goto err5;
}
- if ((err = iptablesAddForwardRejectIn(driver->iptables,
network->def->bridge))) {
- virReportSystemError(err,
- _("failed to add iptables rule to block inbound traffic
to '%s'"),
- network->def->bridge);
+ if (iptablesAddForwardRejectIn(driver->iptables, network->def->bridge) <
0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to block inbound traffic
to '%s'"),
+ network->def->bridge);
goto err6;
}
/* Allow traffic between guests on the same bridge */
- if ((err = iptablesAddForwardAllowCross(driver->iptables,
network->def->bridge))) {
- virReportSystemError(err,
- _("failed to add iptables rule to allow cross bridge
traffic on '%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowCross(driver->iptables, network->def->bridge)
< 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow cross bridge
traffic on '%s'"),
+ network->def->bridge);
goto err7;
}
@@ -829,7 +828,7 @@ networkAddIptablesRules(struct network_driver *driver,
if ((VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) ||
network->def->nranges) &&
(iptablesAddOutputFixUdpChecksum(driver->iptables,
- network->def->bridge, 68) != 0)) {
+ network->def->bridge, 68) < 0)) {
VIR_WARN("Could not add rule to fixup DHCP response checksums "
"on network '%s'.", network->def->name);
VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
--
1.7.3.4
5:57 p.m.
New subject: [libvirt] [PATCH 03/13] Fix logging of failed iptables commands
On 12/20/2010 01:03 AM, Laine Stump wrote:
The functions in iptables.c all return -1 on failure, but all their
callers (which all happen to be in bridge_driver.c) assume that they
are returning an errno, and the logging is done accordingly. This
patch fixes all the error checking and logging to assume < 0 is an
error, and nothing else.
---
src/network/bridge_driver.c | 183 +++++++++++++++++++++----------------------
1 files changed, 91 insertions(+), 92 deletions(-)
Do any of the iptables.c functions guarantee that errno is a sane value
when returning -1?
- virReportSystemError(err,
- _("failed to add iptables rule to allow forwarding
from '%s'"),
- network->def->bridge);
+ if (iptablesAddForwardAllowOut(driver->iptables,
+ &network->def->ipAddress,
+ &network->def->netmask,
+ network->def->bridge,
+ network->def->forwardDev) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to allow forwarding from
'%s'"),
+ network->def->bridge);
or are we okay with this slightly less-informative message, if we happen
to be ignoring a valid errno? Then again, given that the old code was
using strerror(-1), which doesn't convey any information, we aren't
really losing anything in practice from the old code by not displaying
errno, even if iptables guaranteed that errno was useful. Therefore:
ACK as-is.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5:52 p.m.
New subject: [libvirt] [PATCH 03/13] Fix logging of failed iptables commands
On 12/20/2010 06:57 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
> The functions in iptables.c all return -1 on failure, but all their
> callers (which all happen to be in bridge_driver.c) assume that they
> are returning an errno, and the logging is done accordingly. This
> patch fixes all the error checking and logging to assume< 0 is an
> error, and nothing else.
> ---
> src/network/bridge_driver.c | 183 +++++++++++++++++++++----------------------
> 1 files changed, 91 insertions(+), 92 deletions(-)
Do any of the iptables.c functions guarantee that errno is a sane value
when returning -1?
There are only two possible reasons for any of the functions in
iptables.c to fail: 1) out of memory, or 2) the exec of the iptables
command fails or returns a non-0 status.
In all OOM cases, a message has already been logged before returning,
and it's likely the act of doing that reporting will trash errno (I
haven't checked). (And by the time that happens, you're so hosed that
it's not going to matter that a later error message overwrites that
message or whatever).
In the case of a problem exec'ing iptables, there could be a valid
errno, or not (if the command just returned a non-0 exit code.
At any rate, nobody has been looking at errno on return from the
iptables functions; they've been attempting to interpret a -1 return
code (placed into the local "err") as an errno-like value, which simply
isn't correct. This patch fixes that misconception.
> - virReportSystemError(err,
> - _("failed to add iptables rule to allow forwarding
from '%s'"),
> - network->def->bridge);
> + if (iptablesAddForwardAllowOut(driver->iptables,
> +&network->def->ipAddress,
> +&network->def->netmask,
> + network->def->bridge,
> + network->def->forwardDev)< 0) {
> + networkReportError(VIR_ERR_SYSTEM_ERROR,
> + _("failed to add iptables rule to allow forwarding
from '%s'"),
> + network->def->bridge);
or are we okay with this slightly less-informative message, if we happen
to be ignoring a valid errno? Then again, given that the old code was
using strerror(-1), which doesn't convey any information, we aren't
really losing anything in practice from the old code by not displaying
errno, even if iptables guaranteed that errno was useful. Therefore:
ACK as-is.
2:03 a.m.
New subject: [libvirt] [PATCH 04/13] Consistently return 0 on success, -1 on failure in bridge_driver.c
Some functions in this file were returning 1 on success and 0 on
failure, and others were returning 0 on success and -1 on
failure. Switch them all to return the libvirt-preferred 0/-1.
---
src/network/bridge_driver.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5186511..1ae3924 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -373,7 +373,7 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
unsigned int i;
if (! force && virFileExists(dctx->hostsfile->path))
- return 1;
+ return 0;
for (i = 0 ; i < network->def->nhosts ; i++) {
virNetworkDHCPHostDefPtr host = &(network->def->hosts[i]);
@@ -382,9 +382,9 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
}
if (dnsmasqSave(dctx) < 0)
- return 0;
+ return -1;
- return 1;
+ return 0;
}
@@ -487,7 +487,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
goto cleanup;
}
- if (networkSaveDnsmasqHostsfile(network, dctx, false)) {
+ if (networkSaveDnsmasqHostsfile(network, dctx, false) < 0) {
virCommandAddArgPair(cmd, "--dhcp-hostsfile",
dctx->hostsfile->path);
}
@@ -670,7 +670,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
goto masqerr5;
}
- return 1;
+ return 0;
masqerr5:
iptablesRemoveForwardMasquerade(driver->iptables,
@@ -697,7 +697,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
network->def->bridge,
network->def->forwardDev);
masqerr1:
- return 0;
+ return -1;
}
static int
@@ -728,7 +728,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
goto routeerr2;
}
- return 1;
+ return 0;
routeerr2:
@@ -738,7 +738,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
network->def->bridge,
network->def->forwardDev);
routeerr1:
- return 0;
+ return -1;
}
static int
@@ -812,11 +812,11 @@ networkAddIptablesRules(struct network_driver *driver,
/* If masquerading is enabled, set up the rules*/
if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
- !networkAddMasqueradingIptablesRules(driver, network))
+ networkAddMasqueradingIptablesRules(driver, network) < 0)
goto err8;
/* else if routing is enabled, set up the rules*/
else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
- !networkAddRoutingIptablesRules(driver, network))
+ networkAddRoutingIptablesRules(driver, network) < 0)
goto err8;
/* If we are doing local DHCP service on this network, attempt to
@@ -834,7 +834,7 @@ networkAddIptablesRules(struct network_driver *driver,
VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
}
- return 1;
+ return 0;
err8:
iptablesRemoveForwardAllowCross(driver->iptables,
@@ -858,7 +858,7 @@ networkAddIptablesRules(struct network_driver *driver,
err2:
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67);
err1:
- return 0;
+ return -1;
}
static void
@@ -927,7 +927,7 @@ networkReloadIptablesRules(struct network_driver *driver)
if (virNetworkObjIsActive(driver->networks.objs[i])) {
networkRemoveIptablesRules(driver, driver->networks.objs[i]);
- if (!networkAddIptablesRules(driver, driver->networks.objs[i])) {
+ if (networkAddIptablesRules(driver, driver->networks.objs[i]) < 0) {
/* failed to add but already logged */
}
}
@@ -1142,7 +1142,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
goto err_delbr;
}
- if (!networkAddIptablesRules(driver, network))
+ if (networkAddIptablesRules(driver, network) < 0)
goto err_delbr1;
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
--
1.7.3.4
5:59 p.m.
New subject: [libvirt] [PATCH 04/13] Consistently return 0 on success, -1 on failure in bridge_driver.c
On 12/20/2010 01:03 AM, Laine Stump wrote:
Some functions in this file were returning 1 on success and 0 on
failure, and others were returning 0 on success and -1 on
failure. Switch them all to return the libvirt-preferred 0/-1.
---
src/network/bridge_driver.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:03 a.m.
New subject: [libvirt] [PATCH 05/13] Pass prefix rather than netmask into iptables functions
IPv6 will use prefix exclusively, and IPv4 will also optionally be
able to use it, and the iptables functions really need a prefix
anyway, so use the new virNetworkDefPrefix() function to send prefixes
into iptables functions instead of netmasks.
Also, in a couple places where a netmask is actually needed, use the
new private API function for it rather than getting it directly. This
will allow for cases where no netmask or prefix is specified (it
returns the default for the current class of network.)
---
src/network/bridge_driver.c | 91 +++++++++++++++++++++++++++++++------------
src/util/iptables.c | 63 +++++++++++++++---------------
src/util/iptables.h | 16 ++++----
3 files changed, 105 insertions(+), 65 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1ae3924..b592840 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -586,11 +586,19 @@ cleanup:
static int
networkAddMasqueradingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network) {
+ int prefix = virNetworkDefPrefix(network->def);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid prefix or netmask for '%s'"),
+ network->def->bridge);
+ goto masqerr1;
+ }
/* allow forwarding packets from the bridge interface */
if (iptablesAddForwardAllowOut(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -602,7 +610,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* allow forwarding packets to the bridge interface if they are part of an existing
connection */
if (iptablesAddForwardAllowRelatedIn(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -637,7 +645,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* First the generic masquerade rule for other protocols */
if (iptablesAddForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
NULL) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -649,7 +657,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* UDP with a source port restriction */
if (iptablesAddForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
"udp") < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -661,7 +669,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* TCP with a source port restriction */
if (iptablesAddForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
"tcp") < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -675,25 +683,25 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
masqerr5:
iptablesRemoveForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
"udp");
masqerr4:
iptablesRemoveForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
NULL);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
masqerr2:
iptablesRemoveForwardAllowOut(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
masqerr1:
@@ -703,11 +711,19 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
static int
networkAddRoutingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network) {
+ int prefix = virNetworkDefPrefix(network->def);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid prefix or netmask for '%s'"),
+ network->def->bridge);
+ goto routeerr1;
+ }
/* allow routing packets from the bridge interface */
if (iptablesAddForwardAllowOut(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -719,7 +735,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
/* allow routing packets to the bridge interface */
if (iptablesAddForwardAllowIn(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -734,7 +750,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
routeerr2:
iptablesRemoveForwardAllowOut(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
routeerr1:
@@ -870,40 +886,50 @@ networkRemoveIptablesRules(struct network_driver *driver,
network->def->bridge, 68);
}
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
+ int prefix = virNetworkDefPrefix(network->def);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid prefix or netmask for
'%s'"),
+ network->def->bridge);
+ goto error;
+ }
+
if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
iptablesRemoveForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
"tcp");
iptablesRemoveForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
"udp");
iptablesRemoveForwardMasquerade(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->forwardDev,
NULL);
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
} else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)
iptablesRemoveForwardAllowIn(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
iptablesRemoveForwardAllowOut(driver->iptables,
&network->def->ipAddress,
- &network->def->netmask,
+ prefix,
network->def->bridge,
network->def->forwardDev);
}
+error:
iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
@@ -1007,17 +1033,23 @@ static int networkCheckRouteCollision(virNetworkObjPtr network)
{
int ret = -1, len;
unsigned int net_dest;
+ virSocketAddr netmask;
char *cur, *buf = NULL;
enum {MAX_ROUTE_SIZE = 1024*64};
- if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET) ||
- !VIR_SOCKET_IS_FAMILY(&network->def->netmask, AF_INET)) {
+ if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) {
/* Only support collision check for IPv4 */
return 0;
}
+ if (virNetworkDefNetmask(network->def, &netmask) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get netmask of '%s'"),
+ network->def->bridge);
+ }
+
net_dest = (network->def->ipAddress.data.inet4.sin_addr.s_addr &
- network->def->netmask.data.inet4.sin_addr.s_addr);
+ netmask.data.inet4.sin_addr.s_addr);
/* Read whole routing table into memory */
if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0)
@@ -1070,7 +1102,7 @@ static int networkCheckRouteCollision(virNetworkObjPtr network)
addr_val &= mask_val;
if ((net_dest == addr_val) &&
- (network->def->netmask.data.inet4.sin_addr.s_addr == mask_val)) {
+ (netmask.data.inet4.sin_addr.s_addr == mask_val)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("Network is already in use by interface %s"),
iface);
@@ -1126,9 +1158,18 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
goto err_delbr;
}
- if (VIR_SOCKET_HAS_ADDR(&network->def->netmask) &&
- (err = brSetInetNetmask(driver->brctl, network->def->bridge,
- &network->def->netmask))) {
+ virSocketAddr netmask;
+
+ if (virNetworkDefNetmask(network->def, &netmask) < 0) {
+
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP
address"),
+ network->def->bridge);
+ goto err_delbr;
+ }
+
+ if ((err = brSetInetNetmask(driver->brctl, network->def->bridge,
+ &netmask))) {
virReportSystemError(err,
_("cannot set netmask on bridge '%s'"),
network->def->bridge);
diff --git a/src/util/iptables.c b/src/util/iptables.c
index effd81b..89e05b7 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -276,25 +276,24 @@ iptablesRemoveUdpInput(iptablesContext *ctx,
static char *iptablesFormatNetwork(virSocketAddr *netaddr,
- virSocketAddr *netmask)
+ int prefix)
{
virSocketAddr network;
- int prefix;
char *netstr;
char *ret;
- if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET) ||
- !VIR_SOCKET_IS_FAMILY(netmask, AF_INET)) {
+ if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET)) {
iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Only IPv4 addresses can be used with iptables"));
return NULL;
}
network = *netaddr;
- network.data.inet4.sin_addr.s_addr &=
- netmask->data.inet4.sin_addr.s_addr;
-
- prefix = virSocketGetNumNetmaskBits(netmask);
+ if (virSocketAddrMaskByPrefix(&network, prefix) < 0) {
+ iptablesError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failure to mask address"));
+ return NULL;
+ }
netstr = virSocketFormatAddr(&network);
@@ -315,7 +314,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr,
static int
iptablesForwardAllowOut(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev,
int action)
@@ -323,7 +322,7 @@ iptablesForwardAllowOut(iptablesContext *ctx,
int ret;
char *networkstr;
- if (!(networkstr = iptablesFormatNetwork(netaddr, netmask)))
+ if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
return -1;
if (physdev && physdev[0]) {
@@ -362,11 +361,11 @@ iptablesForwardAllowOut(iptablesContext *ctx,
int
iptablesAddForwardAllowOut(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, ADD);
+ return iptablesForwardAllowOut(ctx, netaddr, prefix, iface, physdev, ADD);
}
/**
@@ -385,11 +384,11 @@ iptablesAddForwardAllowOut(iptablesContext *ctx,
int
iptablesRemoveForwardAllowOut(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, REMOVE);
+ return iptablesForwardAllowOut(ctx, netaddr, prefix, iface, physdev, REMOVE);
}
@@ -399,7 +398,7 @@ iptablesRemoveForwardAllowOut(iptablesContext *ctx,
static int
iptablesForwardAllowRelatedIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev,
int action)
@@ -407,7 +406,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx,
int ret;
char *networkstr;
- if (!(networkstr = iptablesFormatNetwork(netaddr, netmask)))
+ if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
return -1;
if (physdev && physdev[0]) {
@@ -450,11 +449,11 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx,
int
iptablesAddForwardAllowRelatedIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, ADD);
+ return iptablesForwardAllowRelatedIn(ctx, netaddr, prefix, iface, physdev, ADD);
}
/**
@@ -473,11 +472,11 @@ iptablesAddForwardAllowRelatedIn(iptablesContext *ctx,
int
iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, REMOVE);
+ return iptablesForwardAllowRelatedIn(ctx, netaddr, prefix, iface, physdev, REMOVE);
}
/* Allow all traffic destined to the bridge, with a valid network address
@@ -485,7 +484,7 @@ iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx,
static int
iptablesForwardAllowIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev,
int action)
@@ -493,7 +492,7 @@ iptablesForwardAllowIn(iptablesContext *ctx,
int ret;
char *networkstr;
- if (!(networkstr = iptablesFormatNetwork(netaddr, netmask)))
+ if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
return -1;
if (physdev && physdev[0]) {
@@ -532,11 +531,11 @@ iptablesForwardAllowIn(iptablesContext *ctx,
int
iptablesAddForwardAllowIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, ADD);
+ return iptablesForwardAllowIn(ctx, netaddr, prefix, iface, physdev, ADD);
}
/**
@@ -555,11 +554,11 @@ iptablesAddForwardAllowIn(iptablesContext *ctx,
int
iptablesRemoveForwardAllowIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev)
{
- return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, REMOVE);
+ return iptablesForwardAllowIn(ctx, netaddr, prefix, iface, physdev, REMOVE);
}
@@ -722,7 +721,7 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx,
static int
iptablesForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *physdev,
const char *protocol,
int action)
@@ -730,7 +729,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
int ret;
char *networkstr;
- if (!(networkstr = iptablesFormatNetwork(netaddr, netmask)))
+ if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
return -1;
if (protocol && protocol[0]) {
@@ -792,11 +791,11 @@ iptablesForwardMasquerade(iptablesContext *ctx,
int
iptablesAddForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *physdev,
const char *protocol)
{
- return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, ADD);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD);
}
/**
@@ -815,11 +814,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
int
iptablesRemoveForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *physdev,
const char *protocol)
{
- return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, REMOVE);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE);
}
diff --git a/src/util/iptables.h b/src/util/iptables.h
index ed843ec..6f59b92 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -45,34 +45,34 @@ int iptablesRemoveUdpInput (iptablesContext
*ctx,
int iptablesAddForwardAllowOut (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
int iptablesRemoveForwardAllowOut (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
int iptablesAddForwardAllowIn (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
int iptablesRemoveForwardAllowIn (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *iface,
const char *physdev);
@@ -93,12 +93,12 @@ int iptablesRemoveForwardRejectIn (iptablesContext
*ctx,
int iptablesAddForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *physdev,
const char *protocol);
int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
- virSocketAddr *netmask,
+ int prefix,
const char *physdev,
const char *protocol);
int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
--
1.7.3.4
6:06 p.m.
New subject: [libvirt] [PATCH 05/13] Pass prefix rather than netmask into iptables functions
On 12/20/2010 01:03 AM, Laine Stump wrote:
IPv6 will use prefix exclusively, and IPv4 will also optionally be
able to use it, and the iptables functions really need a prefix
anyway, so use the new virNetworkDefPrefix() function to send prefixes
into iptables functions instead of netmasks.
Also, in a couple places where a netmask is actually needed, use the
new private API function for it rather than getting it directly. This
will allow for cases where no netmask or prefix is specified (it
returns the default for the current class of network.)
+++ b/src/util/iptables.c
@@ -276,25 +276,24 @@ iptablesRemoveUdpInput(iptablesContext *ctx,
static char *iptablesFormatNetwork(virSocketAddr *netaddr,
- virSocketAddr *netmask)
+ int prefix)
Depending on the resolution to 1/13, you probably want these to all be
unsigned int prefix as well.
But that's a mechanical change, and I didn't see anything else wrong, so
conditional ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:03 a.m.
New subject: [libvirt] [PATCH 06/13] Make virtual network netmasks optional
When a netmask isn't specified for an IPv4 address, one can be implied
based on what network class range the address is in. The
virNetworkDefPrefix function does this for us, so netmask isn't
required.
---
src/conf/network_conf.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b6ac4e3..09566ca 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->delay = 0;
ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
- netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
- if (ipAddress &&
- netmask) {
+ if (ipAddress) {
xmlNodePtr ip;
if (virSocketParseAddr(ipAddress, &def->ipAddress, AF_UNSPEC) < 0)
goto error;
- if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0)
- goto error;
/* XXX someday we want IPv6, so will need to relax this */
- if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET) ||
- !VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
+ if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) {
virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("Only IPv4 addresses are
supported"));
goto error;
@@ -484,18 +479,25 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
VIR_FREE(ipAddress);
- VIR_FREE(netmask);
+ netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
+ if (netmask) {
- /* IPv4 forwarding setup */
- if (virXPathBoolean("count(./forward) > 0", ctxt)) {
- if (def->ipAddress.data.sa.sa_family != AF_INET ||
- def->netmask.data.sa.sa_family != AF_INET) {
- virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Forwarding requested, but no
IPv4 address/netmask provided"));
+ if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0)
+ goto error;
+
+ /* XXX someday we want IPv6, so will need to relax this */
+ if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("Only IPv4 addresses are
supported"));
goto error;
}
+ }
+ VIR_FREE(netmask);
+
+ /* IPv4 forwarding setup */
+ if (virXPathBoolean("count(./forward) > 0", ctxt)) {
tmp = virXPathString("string(./forward[1]/@mode)", ctxt);
if (tmp) {
if ((def->forwardType = virNetworkForwardTypeFromString(tmp)) < 0) {
--
1.7.3.4
6:13 p.m.
New subject: [libvirt] [PATCH 06/13] Make virtual network netmasks optional
On 12/20/2010 01:03 AM, Laine Stump wrote:
When a netmask isn't specified for an IPv4 address, one can be
implied
based on what network class range the address is in. The
virNetworkDefPrefix function does this for us, so netmask isn't
required.
---
src/conf/network_conf.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b6ac4e3..09566ca 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->delay = 0;
ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
- netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
- if (ipAddress &&
- netmask) {
+ if (ipAddress) {
Before this patch, if you specified one but not the other, then neither
was recognized. Now, if you specify ipAddress but not netmask,
ipAddress is still recognized (good), but if you specify netmask but not
ipAddress, you've caused the def file to be in a state that wasn't
possible before (potentially bad). Should you also add a sanity check
that declares the configuration invalid if netmask is specified without
ipAddress, so you don't have to worry about it in the rest of the code base?
If the answer to that question is that netmask without ipAddress doesn't
hurt, then conditional ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:34 a.m.
New subject: [libvirt] [PATCH 06/13] Make virtual network netmasks optional
On 12/20/2010 07:13 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
> When a netmask isn't specified for an IPv4 address, one can be implied
> based on what network class range the address is in. The
> virNetworkDefPrefix function does this for us, so netmask isn't
> required.
> ---
> src/conf/network_conf.c | 30 ++++++++++++++++--------------
> 1 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index b6ac4e3..09566ca 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> def->delay = 0;
>
> ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
> - netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
> - if (ipAddress&&
> - netmask) {
> + if (ipAddress) {
Before this patch, if you specified one but not the other, then neither
was recognized. Now, if you specify ipAddress but not netmask,
ipAddress is still recognized (good), but if you specify netmask but not
ipAddress, you've caused the def file to be in a state that wasn't
possible before (potentially bad). Should you also add a sanity check
that declares the configuration invalid if netmask is specified without
ipAddress, so you don't have to worry about it in the rest of the code base?
Yes, good idea. I changed it to give an error if netmask (or prefix, in
later patches) is provided without an IP address.
I also noticed a pre-existing memory leak during error exits, and am
adding a fix for that to the series.
10:44 a.m.
New subject: [libvirt] [PATCH 06/13] Make virtual network netmasks optional
On 12/22/2010 11:34 AM, Laine Stump wrote:
On 12/20/2010 07:13 PM, Eric Blake wrote:
> On 12/20/2010 01:03 AM, Laine Stump wrote:
>> When a netmask isn't specified for an IPv4 address, one can be implied
>> based on what network class range the address is in. The
>> virNetworkDefPrefix function does this for us, so netmask isn't
>> required.
>> ---
>> src/conf/network_conf.c | 30 ++++++++++++++++--------------
>> 1 files changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
>> index b6ac4e3..09566ca 100644
>> --- a/src/conf/network_conf.c
>> +++ b/src/conf/network_conf.c
>> @@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>> def->delay = 0;
>>
>> ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
>> - netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
>> - if (ipAddress&&
>> - netmask) {
>> + if (ipAddress) {
> Before this patch, if you specified one but not the other, then neither
> was recognized. Now, if you specify ipAddress but not netmask,
> ipAddress is still recognized (good), but if you specify netmask but not
> ipAddress, you've caused the def file to be in a state that wasn't
> possible before (potentially bad). Should you also add a sanity check
> that declares the configuration invalid if netmask is specified without
> ipAddress, so you don't have to worry about it in the rest of the
> code base?
Yes, good idea. I changed it to give an error if netmask (or prefix,
in later patches) is provided without an IP address.
I also noticed a pre-existing memory leak during error exits, and am
adding a fix for that to the series.
Heh. Getting ahead of myself...
Continuing the git rebase after making those changes, I noticed that 1)
all this code moves into a separate function in patch 09/13, and 2) that
patch already checks for netmask w/o an IP address and gives a proper
error message. For those reasons, I'm going to leave this patch as-is
(since it will only be there to satisfy bisects).
2:03 a.m.
New subject: [libvirt] [PATCH 07/13] Replace brSetInetAddress/brSetInetNetmask with brAddInetAddress
brSetInetAddress can only set a single IP address on the bridge, and
uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace
it and brSetInetNetmask with a single function that uses the external
"ip addr add" command to add an address/prefix to the interface - this
supports IPv6, and allows adding multiple addresses to the interface.
Although it isn't currently used in the code, we also add a
brDelInetAddress for completeness' sake.
Also, while we're modifying bridge.c, we change brSetForwardDelay and
brSetEnableSTP to use the new virCommand API rather than the
deprecated virRun, and also log an error message in bridge_driver.c if
either of those fail (previously the failure would be completely
silent).
NB: it's a bit bothersome that there is no difference in error
reporting between being unable to run one of these commands, and the
command returning a non-0 exit status, but there is no precedent in
bridge.c for logging error messages locally rather than pushing them
up the call chain, and what's pushed up is only 'success' or
'failure'; no exit codes. Perhaps a non-0 exit could be passed up
directly as the return, other errors could return -1, and callers
could check for ret != 0 rather than ret < 0?
---
configure.ac | 3 +
src/libvirt_bridge.syms | 3 +-
src/network/bridge_driver.c | 50 ++++++++-------
src/util/bridge.c | 141 +++++++++++++++++++++++++------------------
src/util/bridge.h | 10 ++-
5 files changed, 118 insertions(+), 89 deletions(-)
diff --git a/configure.ac b/configure.ac
index 558bb77..357d59b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,6 +306,9 @@ if test x"$with_rhel5_api" = x"yes"; then
AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API])
fi
+AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH])
+AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary])
+
AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables
binary])
diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms
index 2658291..c3773bd 100644
--- a/src/libvirt_bridge.syms
+++ b/src/libvirt_bridge.syms
@@ -6,15 +6,16 @@
# bridge.h
brAddBridge;
+brAddInetAddress;
brAddInterface;
brAddTap;
brDeleteTap;
brDeleteBridge;
+brDelInetAddress;
brHasBridge;
brInit;
brSetEnableSTP;
brSetForwardDelay;
-brSetInetAddress;
brSetInetNetmask;
brSetInterfaceUp;
brShutdown;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b592840..b8132ba 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1143,37 +1143,39 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
if (networkDisableIPV6(network) < 0)
goto err_delbr;
- if (brSetForwardDelay(driver->brctl, network->def->bridge,
network->def->delay) < 0)
- goto err_delbr;
-
- if (brSetEnableSTP(driver->brctl, network->def->bridge,
network->def->stp ? 1 : 0) < 0)
- goto err_delbr;
-
- if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) &&
- (err = brSetInetAddress(driver->brctl, network->def->bridge,
- &network->def->ipAddress))) {
- virReportSystemError(err,
- _("cannot set IP address on bridge
'%s'"),
- network->def->bridge);
+ if ((err = brSetForwardDelay(driver->brctl, network->def->bridge,
+ network->def->delay))) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot set forward delay on bridge
'%s'"),
+ network->def->bridge);
goto err_delbr;
}
- virSocketAddr netmask;
-
- if (virNetworkDefNetmask(network->def, &netmask) < 0) {
-
+ if ((err = brSetEnableSTP(driver->brctl, network->def->bridge,
+ network->def->stp ? 1 : 0))) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("bridge '%s' has an invalid netmask or IP
address"),
- network->def->bridge);
+ _("cannot set STP '%s' on bridge
'%s'"),
+ network->def->stp ? "on" : "off",
network->def->bridge);
goto err_delbr;
}
- if ((err = brSetInetNetmask(driver->brctl, network->def->bridge,
- &netmask))) {
- virReportSystemError(err,
- _("cannot set netmask on bridge '%s'"),
- network->def->bridge);
- goto err_delbr;
+ if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) {
+ int prefix = virNetworkDefPrefix(network->def);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP
address"),
+ network->def->bridge);
+ goto err_delbr;
+ }
+
+ if ((err = brAddInetAddress(driver->brctl, network->def->bridge,
+ &network->def->ipAddress, prefix))) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot set IP address on bridge
'%s'"),
+ network->def->bridge);
+ goto err_delbr;
+ }
}
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) {
diff --git a/src/util/bridge.c b/src/util/bridge.c
index 97cf73f..0cf9301 100644
--- a/src/util/bridge.c
+++ b/src/util/bridge.c
@@ -46,6 +46,7 @@
# include <net/if_arp.h> /* ARPHRD_ETHER */
# include "internal.h"
+# include "command.h"
# include "memory.h"
# include "util.h"
# include "logging.h"
@@ -655,73 +656,84 @@ brGetInterfaceUp(brControl *ctl,
return 0;
}
-static int
-brSetInetAddr(brControl *ctl,
- const char *ifname,
- int cmd,
- virSocketAddr *addr)
-{
- struct ifreq ifr;
-
- if (!ctl || !ctl->fd || !ifname || !addr)
- return EINVAL;
-
- memset(&ifr, 0, sizeof(struct ifreq));
-
- if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
- return EINVAL;
-
- if (!VIR_SOCKET_IS_FAMILY(addr, AF_INET))
- return EINVAL;
-
- ifr.ifr_addr = addr->data.sa;
-
- if (ioctl(ctl->fd, cmd, &ifr) < 0)
- return errno;
-
- return 0;
-}
-
/**
- * brSetInetAddress:
+ * brAddInetAddress:
* @ctl: bridge control pointer
* @ifname: the interface name
- * @addr: the string representation of the IP address
+ * @addr: the IP address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
*
- * Function to bind the interface to an IP address, it should handle
- * IPV4 and IPv6. The string for addr would be of the form
- * "ddd.ddd.ddd.ddd" assuming the common IPv4 format.
+ * Add an IP address to an interface. This function *does not* remove
+ * any previously added interfaces - that must be done separately with
+ * brDelInetAddress.
*
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 in case of error.
*/
int
-brSetInetAddress(brControl *ctl,
+brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED,
const char *ifname,
- virSocketAddr *addr)
+ virSocketAddr *addr,
+ int prefix)
{
- return brSetInetAddr(ctl, ifname, SIOCSIFADDR, addr);
+ virCommandPtr cmd;
+ char *addrstr;
+ int ret = -1;
+
+ if (!(addrstr = virSocketFormatAddr(addr)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "addr", "add", NULL);
+ virCommandAddArgFormat(cmd, "%s/%d", addrstr, prefix);
+ virCommandAddArgList(cmd, "dev", ifname, NULL);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ virCommandFree(cmd);
+ return ret;
}
/**
- * brSetInetNetmask:
+ * brDelInetAddress:
* @ctl: bridge control pointer
* @ifname: the interface name
- * @addr: the string representation of the netmask
+ * @addr: the IP address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
*
- * Function to set the netmask of an interface, it should handle
- * IPV4 and IPv6 forms. The string for addr would be of the form
- * "ddd.ddd.ddd.ddd" assuming the common IPv4 format.
+ * Delete an IP address from an interface.
*
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 in case of error.
*/
int
-brSetInetNetmask(brControl *ctl,
+brDelInetAddress(brControl *ctl ATTRIBUTE_UNUSED,
const char *ifname,
- virSocketAddr *addr)
+ virSocketAddr *addr,
+ int prefix)
{
- return brSetInetAddr(ctl, ifname, SIOCSIFNETMASK, addr);
+ virCommandPtr cmd;
+ char *addrstr;
+ int ret = -1;
+
+ if (!(addrstr = virSocketFormatAddr(addr)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "addr", "del", NULL);
+ virCommandAddArgFormat(cmd, "%s/%d", addrstr, prefix);
+ virCommandAddArgList(cmd, "dev", ifname, NULL);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ virCommandFree(cmd);
+ return ret;
}
/**
@@ -740,17 +752,20 @@ brSetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED,
const char *bridge,
int delay)
{
- char delayStr[30];
- const char *const progargv[] = {
- BRCTL, "setfd", bridge, delayStr, NULL
- };
+ virCommandPtr cmd;
+ int ret = -1;
- snprintf(delayStr, sizeof(delayStr), "%d", delay);
+ cmd = virCommandNew(BRCTL);
+ virCommandAddArgList(cmd, "setfd", bridge, NULL);
+ virCommandAddArgFormat(cmd, "%d", delay);
- if (virRun(progargv, NULL) < 0)
- return -1;
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
- return 0;
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
}
/**
@@ -769,15 +784,21 @@ brSetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED,
const char *bridge,
int enable)
{
- const char *setting = enable ? "on" : "off";
- const char *const progargv[] = {
- BRCTL, "stp", bridge, setting, NULL
- };
+ virCommandPtr cmd;
+ int ret = -1;
- if (virRun(progargv, NULL) < 0)
- return -1;
+ cmd = virCommandNew(BRCTL);
+ virCommandAddArgList(cmd, "stp", bridge,
+ enable ? "on" : "off",
+ NULL);
- return 0;
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
}
#endif /* WITH_BRIDGE */
diff --git a/src/util/bridge.h b/src/util/bridge.h
index 3ef38d9..8132d35 100644
--- a/src/util/bridge.h
+++ b/src/util/bridge.h
@@ -83,12 +83,14 @@ int brGetInterfaceUp (brControl *ctl,
const char *ifname,
int *up);
-int brSetInetAddress (brControl *ctl,
+int brAddInetAddress (brControl *ctl,
const char *ifname,
- virSocketAddr *addr);
-int brSetInetNetmask (brControl *ctl,
+ virSocketAddr *addr,
+ int prefix);
+int brDelInetAddress (brControl *ctl,
const char *ifname,
- virSocketAddr *addr);
+ virSocketAddr *addr,
+ int prefix);
int brSetForwardDelay (brControl *ctl,
const char *bridge,
--
1.7.3.4
4:46 p.m.
New subject: [libvirt] [PATCH 07/13] Replace brSetInetAddress/brSetInetNetmask with brAddInetAddress
On Mon, Dec 20, 2010 at 09:03, Laine Stump <laine(a)laine.org> wrote:
brSetInetAddress can only set a single IP address on the bridge, and
uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace
it and brSetInetNetmask with a single function that uses the external
"ip addr add" command to add an address/prefix to the interface - this
supports IPv6, and allows adding multiple addresses to the interface.
ioctl(SIOCSIFADDR) on AF_INET socket works for IPv4, on AF_INET6
socket works for IPv6 space.
You need to create fd6=socket(AF_INET6, SOCK_STREAM, 0) in brInit()
and do ioctl() on fd6.
--
Pawel
11:21 a.m.
New subject: [libvirt] [PATCH 07/13] Replace brSetInetAddress/brSetInetNetmask with brAddInetAddress
On 12/20/2010 05:46 PM, Paweł Krześniak wrote:
On Mon, Dec 20, 2010 at 09:03, Laine Stump<laine(a)laine.org>
wrote:
> brSetInetAddress can only set a single IP address on the bridge, and
> uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace
> it and brSetInetNetmask with a single function that uses the external
> "ip addr add" command to add an address/prefix to the interface - this
> supports IPv6, and allows adding multiple addresses to the interface.
ioctl(SIOCSIFADDR) on AF_INET socket works for IPv4, on AF_INET6
socket works for IPv6 space.
You need to create fd6=socket(AF_INET6, SOCK_STREAM, 0) in brInit()
and do ioctl() on fd6.
Interesting information. I still prefer to use the ip command, though,
as it allows me to add multiple IP addresses to an interface.
2:03 a.m.
New subject: [libvirt] [PATCH 08/13] make the <dhcp> element optional in network.rng
In practice this has always been optional, but the RNG has shown it as
mandatory, and since all the examples for make check had it, it was
never noticed. One of the existing test cases has been changed to
check for this.
I also noticed that the dhcp/host/ip was still defined as <text/>,
but should really be <ref name='ipv4-addr'/>
---
docs/schemas/network.rng | 52 ++++++++++++++-------------
tests/networkxml2xmlin/routed-network.xml | 3 --
tests/networkxml2xmlout/routed-network.xml | 3 --
3 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 1daa30e..ac13af2 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -95,31 +95,33 @@
<attribute name="root"><text/></attribute>
</element>
</optional>
- <!-- Define the range(s) of IP addresses that the DHCP
- server should hand out -->
- <element name="dhcp">
- <zeroOrMore>
- <element name="range">
- <attribute name="start"><ref
name="ipv4-addr"/></attribute>
- <attribute name="end"><ref
name="ipv4-addr"/></attribute>
- </element>
- </zeroOrMore>
- <zeroOrMore>
- <element name="host">
- <attribute name="mac"><ref
name="mac-addr"/></attribute>
- <attribute name="name"><text/></attribute>
- <attribute name="ip"><text/></attribute>
- </element>
- </zeroOrMore>
- <optional>
- <element name="bootp">
- <attribute name="file"><text/></attribute>
- <optional>
- <attribute
name="server"><text/></attribute>
- </optional>
- </element>
- </optional>
- </element>
+ <optional>
+ <!-- Define the range(s) of IP addresses that the DHCP
+ server should hand out -->
+ <element name="dhcp">
+ <zeroOrMore>
+ <element name="range">
+ <attribute name="start"><ref
name="ipv4-addr"/></attribute>
+ <attribute name="end"><ref
name="ipv4-addr"/></attribute>
+ </element>
+ </zeroOrMore>
+ <zeroOrMore>
+ <element name="host">
+ <attribute name="mac"><ref
name="mac-addr"/></attribute>
+ <attribute
name="name"><text/></attribute>
+ <attribute name="ip"><ref
name="ipv4-addr"/></attribute>
+ </element>
+ </zeroOrMore>
+ <optional>
+ <element name="bootp">
+ <attribute
name="file"><text/></attribute>
+ <optional>
+ <attribute
name="server"><text/></attribute>
+ </optional>
+ </element>
+ </optional>
+ </element>
+ </optional>
</element>
</optional>
</interleave>
diff --git a/tests/networkxml2xmlin/routed-network.xml
b/tests/networkxml2xmlin/routed-network.xml
index 824ad75..6634ee8 100644
--- a/tests/networkxml2xmlin/routed-network.xml
+++ b/tests/networkxml2xmlin/routed-network.xml
@@ -4,8 +4,5 @@
<bridge name="virbr1" />
<forward mode="route" dev="eth1"/>
<ip address="192.168.122.1" netmask="255.255.255.0">
- <dhcp>
- <range start="192.168.122.2" end="192.168.122.254" />
- </dhcp>
</ip>
</network>
diff --git a/tests/networkxml2xmlout/routed-network.xml
b/tests/networkxml2xmlout/routed-network.xml
index fa36c08..8f11166 100644
--- a/tests/networkxml2xmlout/routed-network.xml
+++ b/tests/networkxml2xmlout/routed-network.xml
@@ -4,8 +4,5 @@
<forward dev='eth1' mode='route'/>
<bridge name='virbr1' stp='on' delay='0' />
<ip address='192.168.122.1' netmask='255.255.255.0'>
- <dhcp>
- <range start='192.168.122.2' end='192.168.122.254' />
- </dhcp>
</ip>
</network>
--
1.7.3.4
2:03 a.m.
New subject: [libvirt] [PATCH 09/13] Change virtual network XML parsing/formatting to support IPv6
This commit adds support for IPv6 parsing and formatting to the
virtual network XML parser, including moving around data definitions
to allow for multiple <ip> elements on a single network, but only
changes the consumers of this API to accomodate for the changes in
API/structure, not to add any actual IPv6 functionality. That will
come in a later patch - this patch attempts to maintain the same final
functionality in both drivers that use the network XML parser - vbox
and "bridge" (the Linux bridge-based driver used by the qemu
hypervisor driver).
* src/libvirt_private.syms: add new private API functions
* src/conf/network_conf.[ch]: change C data structure and parsing/formatting
* src/network/bridge_driver.c: update to use new parser/formatter
* src/vbox/vbox_tmpl.c: update to use new parser/formatter
* docs/schemas/network.rng: changes to the schema -
* there can now be more than one <ip> element.
* ip address is now an ip-addr (ipv4 or ipv6) rather than ipv4-addr
* new optional "prefix" attribute that can be used in place of
"netmask"
* new optional "family" attribute - "ipv4" or "ipv6" (will
default to ipv4)
* define data types for the above
* tests/networkxml2xml(in|out)/nat-network.xml: add multiple <ip> elements
(including IPv6) to a single network definition to verify they are being
correctly parsed and formatted.
---
docs/schemas/network.rng | 41 +++-
src/conf/network_conf.c | 452 +++++++++++++++++++++----------
src/conf/network_conf.h | 50 +++-
src/libvirt_private.syms | 5 +-
src/network/bridge_driver.c | 248 ++++++++++--------
src/vbox/vbox_tmpl.c | 81 ++++--
tests/networkxml2xmlin/nat-network.xml | 8 +
tests/networkxml2xmlout/nat-network.xml | 8 +
8 files changed, 589 insertions(+), 304 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index ac13af2..bc34ddc 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -80,15 +80,21 @@
</optional>
<!-- <ip> element -->
- <optional>
+ <zeroOrMore>
<!-- The IP element sets up NAT'ing and an optional DHCP server
local to the host. -->
<element name="ip">
<optional>
- <attribute name="address"><ref
name="ipv4-addr"/></attribute>
+ <attribute name="address"><ref
name="ip-addr"/></attribute>
+ </optional>
+ <optional>
+ <choice>
+ <attribute name="netmask"><ref
name="ipv4-addr"/></attribute>
+ <attribute name="prefix"><ref
name="ip-prefix"/></attribute>
+ </choice>
</optional>
<optional>
- <attribute name="netmask"><ref
name="ipv4-addr"/></attribute>
+ <attribute name="family"><ref
name="addr-family"/></attribute>
</optional>
<optional>
<element name="tftp">
@@ -123,7 +129,7 @@
</element>
</optional>
</element>
- </optional>
+ </zeroOrMore>
</interleave>
</element>
</define>
@@ -135,6 +141,33 @@
</data>
</define>
+ <!-- Based on http://blog.mes-stats.fr/2008/10/09/regex-ipv4-et-ipv6 -->
+ <define name='ipv6-addr'>
+ <data type='string'>
+ <!-- To understand this better, take apart the toplevel '|'s -->
+ <param
name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|(([0-9A-Fa-f]{1,4}:){0,5}:((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|(::([0-9A-Fa-f]{1,4}:){0,5}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)</param>
+ </data>
+ </define>
+
+ <define name='ip-addr'>
+ <choice>
+ <ref name='ipv4-addr'/>
+ <ref name='ipv6-addr'/>
+ </choice>
+ </define>
+
+ <define name='ip-prefix'>
+ <data type='unsignedInt'>
+ <param name="maxInclusive">128</param>
+ </data>
+ </define>
+
+ <define name='addr-family'>
+ <data type='string'>
+ <param name="pattern">(ipv4)|(ipv6)</param>
+ </data>
+ </define>
+
<!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC"
-->
<define name='mac-addr'>
<data type='string'>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 09566ca..4486a5b 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -87,9 +87,26 @@ virNetworkObjPtr virNetworkFindByName(const virNetworkObjListPtr nets,
}
+static void virNetworkIpDefClear(virNetworkIpDefPtr def)
+{
+ int ii;
+
+ VIR_FREE(def->family);
+ VIR_FREE(def->ranges);
+
+ for (ii = 0 ; ii < def->nhosts && def->hosts ; ii++) {
+ VIR_FREE(def->hosts[ii].mac);
+ VIR_FREE(def->hosts[ii].name);
+ }
+
+ VIR_FREE(def->hosts);
+ VIR_FREE(def->tftproot);
+ VIR_FREE(def->bootfile);
+}
+
void virNetworkDefFree(virNetworkDefPtr def)
{
- int i;
+ int ii;
if (!def)
return;
@@ -99,16 +116,10 @@ void virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def->forwardDev);
VIR_FREE(def->domain);
- VIR_FREE(def->ranges);
-
- for (i = 0 ; i < def->nhosts && def->hosts ; i++) {
- VIR_FREE(def->hosts[i].mac);
- VIR_FREE(def->hosts[i].name);
+ for (ii = 0 ; ii < def->nips && def->ips ; ii++) {
+ virNetworkIpDefClear(&def->ips[ii]);
}
- VIR_FREE(def->hosts);
-
- VIR_FREE(def->tftproot);
- VIR_FREE(def->bootfile);
+ VIR_FREE(def->ips);
VIR_FREE(def);
}
@@ -207,16 +218,43 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
}
}
+/* return ips[index], or NULL if there aren't enough ips */
+virNetworkIpDefPtr
+virNetworkDefGetIpByIndex(const virNetworkDefPtr def,
+ int family, int n)
+{
+ int ii;
+
+ if (!def->ips || n >= def->nips)
+ return NULL;
+
+ if (family == AF_UNSPEC) {
+ return &def->ips[n];
+ }
+
+ /* find the nth ip of type "family" */
+ for (ii = 0; ii < def->nips; ii++) {
+ if (VIR_SOCKET_IS_FAMILY(&def->ips[ii].address, family)
+ && (n-- <= 0)) {
+ return &def->ips[ii];
+ }
+ }
+ /* failed to find enough of the right family */
+ return NULL;
+}
+
/* return number of 1 bits in netmask for the network's ipAddress,
* or -1 on error
*/
-int virNetworkDefPrefix(const virNetworkDefPtr def)
+int virNetworkIpDefPrefix(const virNetworkIpDefPtr def)
{
- if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
+ if (def->prefix > 0) {
+ return def->prefix;
+ } else if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
return virSocketGetNumNetmaskBits(&def->netmask);
- } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) {
+ } else if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) {
/* return the natural prefix for the network's ip address */
- int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr);
+ int addr = ntohl(def->address.data.inet4.sin_addr.s_addr);
if (IN_CLASSA(addr))
return 8;
if (IN_CLASSB(addr))
@@ -224,6 +262,8 @@ int virNetworkDefPrefix(const virNetworkDefPtr def)
if (IN_CLASSC(addr))
return 24;
return -1;
+ } else if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET6)) {
+ return 64;
}
return -1;
}
@@ -232,22 +272,23 @@ int virNetworkDefPrefix(const virNetworkDefPtr def)
* definition, based on either the definition's netmask, or its
* prefix. Return -1 on error (and set the netmask family to AF_UNSPEC)
*/
-int virNetworkDefNetmask(const virNetworkDefPtr def,
- virSocketAddrPtr netmask)
+int virNetworkIpDefNetmask(const virNetworkIpDefPtr def,
+ virSocketAddrPtr netmask)
{
if (VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
*netmask = def->netmask;
return 0;
}
- return virSocketAddrPrefixToNetmask(virNetworkDefPrefix(def), netmask,
- VIR_SOCKET_FAMILY(&def->ipAddress));
+ return virSocketAddrPrefixToNetmask(virNetworkIpDefPrefix(def), netmask,
+ VIR_SOCKET_FAMILY(&def->address));
}
static int
-virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
- xmlNodePtr node) {
+virNetworkDHCPRangeDefParseXML(virNetworkIpDefPtr def,
+ xmlNodePtr node)
+{
xmlNodePtr cur;
@@ -381,33 +422,146 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
}
static int
-virNetworkIPParseXML(virNetworkDefPtr def,
- xmlNodePtr node) {
- xmlNodePtr cur;
+virNetworkIPParseXML(const char *networkName,
+ virNetworkIpDefPtr def,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
+{
+ /*
+ * virNetworkIpDef object is already allocated as part of an array.
+ * On failure clear it out, but don't free it.
+ */
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE &&
- xmlStrEqual(cur->name, BAD_CAST "dhcp")) {
- int result = virNetworkDHCPRangeDefParseXML(def, cur);
- if (result)
- return result;
+ xmlNodePtr cur, save;
+ char *address, *netmask;
+ unsigned long prefix;
+ int result = -1;
+
+ save = ctxt->node;
+ ctxt->node = node;
+
+ /* grab raw data from XML */
+ def->family = virXPathString("string(./@family)", ctxt);
+ address = virXPathString("string(./@address)", ctxt);
+ if (virXPathULong("string(./@prefix)", ctxt, &prefix) < 0)
+ def->prefix = 0;
+ else
+ def->prefix = prefix;
+
+ netmask = virXPathString("string(./@netmask)", ctxt);
+
+ if (address) {
+ if (virSocketParseAddr(address, &def->address, AF_UNSPEC) < 0) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("Bad address '%s' in definition of
network '%s'"),
+ address, networkName);
+ goto error;
+ }
- } else if (cur->type == XML_ELEMENT_NODE &&
- xmlStrEqual(cur->name, BAD_CAST "tftp")) {
- char *root;
+ }
- if (!(root = virXMLPropString(cur, "root"))) {
- cur = cur->next;
- continue;
+ /* validate family vs. address */
+ if (def->family == NULL) {
+ if (!(VIR_SOCKET_IS_FAMILY(&def->address, AF_INET) ||
+ VIR_SOCKET_IS_FAMILY(&def->address, AF_UNSPEC))) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("no family specified for non-IPv4 address
address '%s' in network '%s'"),
+ address, networkName);
+ goto error;
+ }
+ } else if (STREQ(def->family, "ipv4")) {
+ if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("family 'ipv4' specified for non-IPv4
address '%s' in network '%s'"),
+ address, networkName);
+ goto error;
+ }
+ } else if (STREQ(def->family, "ipv6")) {
+ if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET6)) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("family 'ipv6' specified for non-IPv6
address '%s' in network '%s'"),
+ address, networkName);
+ goto error;
+ }
+ } else {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("Unrecognized family '%s' in definition of
network '%s'"),
+ def->family, networkName);
+ goto error;
+ }
+
+ /* parse/validate netmask */
+ if (netmask) {
+ if (address == NULL) {
+ /* netmask is meaningless without an address */
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("netmask specified without address in network
'%s'"),
+ networkName);
+ goto error;
+ }
+
+ if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("netmask not supported for address '%s'
in network '%s' (IPv4 only)"),
+ address, networkName);
+ goto error;
+ }
+
+ if (def->prefix > 0) {
+ /* can't have both netmask and prefix at the same time */
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network '%s' has prefix='%d'
but no address"),
+ networkName, def->prefix);
+ goto error;
+ }
+
+ if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0)
+ goto error;
+
+ if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
+ virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network '%s' has invalid netmask
'%s' for address '%s' (both must be IPv4)"),
+ networkName, netmask, address);
+ goto error;
+ }
+ }
+
+ if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) {
+ /* parse IPv4-related info */
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "dhcp")) {
+ result = virNetworkDHCPRangeDefParseXML(def, cur);
+ if (result)
+ goto error;
+
+ } else if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "tftp")) {
+ char *root;
+
+ if (!(root = virXMLPropString(cur, "root"))) {
+ cur = cur->next;
+ continue;
+ }
+
+ def->tftproot = (char *)root;
}
- def->tftproot = root;
+ cur = cur->next;
}
+ }
- cur = cur->next;
+ result = 0;
+
+error:
+ if (result < 0) {
+ virNetworkIpDefClear(def);
}
- return 0;
+ VIR_FREE(address);
+
+ ctxt->node = save;
+ return result;
}
static virNetworkDefPtr
@@ -415,8 +569,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
{
virNetworkDefPtr def;
char *tmp;
- char *ipAddress;
- char *netmask;
+ xmlNodePtr *ipNodes = NULL;
+ int nIps;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -460,44 +614,32 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
if (virXPathULong("string(./bridge[1]/@delay)", ctxt, &def->delay)
< 0)
def->delay = 0;
- ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
- if (ipAddress) {
- xmlNodePtr ip;
-
- if (virSocketParseAddr(ipAddress, &def->ipAddress, AF_UNSPEC) < 0)
- goto error;
+ nIps = virXPathNodeSet("./ip", ctxt, &ipNodes);
+ if (nIps > 0) {
+ int ii;
- /* XXX someday we want IPv6, so will need to relax this */
- if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) {
- virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%s", _("Only IPv4 addresses are
supported"));
+ /* allocate array to hold all the addrs */
+ if (VIR_ALLOC_N(def->ips, nIps) < 0) {
+ virReportOOMError();
goto error;
}
-
- if ((ip = virXPathNode("./ip[1]", ctxt)) &&
- virNetworkIPParseXML(def, ip) < 0)
- goto error;
- }
- VIR_FREE(ipAddress);
-
- netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
- if (netmask) {
-
- if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0)
- goto error;
-
- /* XXX someday we want IPv6, so will need to relax this */
- if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) {
- virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%s", _("Only IPv4 addresses are
supported"));
- goto error;
+ /* parse each addr */
+ for (ii = 0; ii < nIps; ii++) {
+ int ret = virNetworkIPParseXML(def->name, &def->ips[ii],
+ ipNodes[ii], ctxt);
+ if (ret < 0)
+ goto error;
+ def->nips++;
}
}
- VIR_FREE(netmask);
-
/* IPv4 forwarding setup */
if (virXPathBoolean("count(./forward) > 0", ctxt)) {
+ if (def->nips == 0) {
+ virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Forwarding requested, but no IP
address provided"));
+ goto error;
+ }
tmp = virXPathString("string(./forward[1]/@mode)", ctxt);
if (tmp) {
if ((def->forwardType = virNetworkForwardTypeFromString(tmp)) < 0) {
@@ -576,11 +718,101 @@ cleanup:
return def;
}
+static int
+virNetworkIpDefFormat(virBufferPtr buf,
+ const virNetworkIpDefPtr def)
+{
+ int result = -1;
+
+ virBufferAddLit(buf, " <ip");
+
+ if (def->family) {
+ virBufferVSprintf(buf, " family='%s'", def->family);
+ }
+ if (VIR_SOCKET_HAS_ADDR(&def->address)) {
+ char *addr = virSocketFormatAddr(&def->address);
+ if (!addr)
+ goto error;
+ virBufferVSprintf(buf, " address='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
+ char *addr = virSocketFormatAddr(&def->netmask);
+ if (!addr)
+ goto error;
+ virBufferVSprintf(buf, " netmask='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (def->prefix > 0) {
+ virBufferVSprintf(buf," prefix='%d'", def->prefix);
+ }
+ virBufferAddLit(buf, ">\n");
+
+ if (def->tftproot) {
+ virBufferEscapeString(buf, " <tftp root='%s' />\n",
+ def->tftproot);
+ }
+ if ((def->nranges || def->nhosts)) {
+ int ii;
+ virBufferAddLit(buf, " <dhcp>\n");
+ for (ii = 0 ; ii < def->nranges ; ii++) {
+ char *saddr = virSocketFormatAddr(&def->ranges[ii].start);
+ if (!saddr)
+ goto error;
+ char *eaddr = virSocketFormatAddr(&def->ranges[ii].end);
+ if (!eaddr) {
+ VIR_FREE(saddr);
+ goto error;
+ }
+ virBufferVSprintf(buf, " <range start='%s'
end='%s' />\n",
+ saddr, eaddr);
+ VIR_FREE(saddr);
+ VIR_FREE(eaddr);
+ }
+ for (ii = 0 ; ii < def->nhosts ; ii++) {
+ virBufferAddLit(buf, " <host ");
+ if (def->hosts[ii].mac)
+ virBufferVSprintf(buf, "mac='%s' ",
def->hosts[ii].mac);
+ if (def->hosts[ii].name)
+ virBufferVSprintf(buf, "name='%s' ",
def->hosts[ii].name);
+ if (VIR_SOCKET_HAS_ADDR(&def->hosts[ii].ip)) {
+ char *ipaddr = virSocketFormatAddr(&def->hosts[ii].ip);
+ if (!ipaddr)
+ goto error;
+ virBufferVSprintf(buf, "ip='%s' ", ipaddr);
+ VIR_FREE(ipaddr);
+ }
+ virBufferAddLit(buf, "/>\n");
+ }
+ if (def->bootfile) {
+ virBufferEscapeString(buf, " <bootp file='%s' ",
+ def->bootfile);
+ if (VIR_SOCKET_HAS_ADDR(&def->bootserver)) {
+ char *ipaddr = virSocketFormatAddr(&def->bootserver);
+ if (!ipaddr)
+ goto error;
+ virBufferEscapeString(buf, "server='%s' ", ipaddr);
+ VIR_FREE(ipaddr);
+ }
+ virBufferAddLit(buf, "/>\n");
+ }
+
+ virBufferAddLit(buf, " </dhcp>\n");
+ }
+
+ virBufferAddLit(buf, " </ip>\n");
+
+ result = 0;
+error:
+ return result;
+}
+
char *virNetworkDefFormat(const virNetworkDefPtr def)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
unsigned char *uuid;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ int ii;
virBufferAddLit(&buf, "<network>\n");
virBufferEscapeString(&buf, " <name>%s</name>\n",
def->name);
@@ -612,81 +844,9 @@ char *virNetworkDefFormat(const virNetworkDefPtr def)
if (def->domain)
virBufferVSprintf(&buf, " <domain name='%s'/>\n",
def->domain);
- if (VIR_SOCKET_HAS_ADDR(&def->ipAddress) ||
- VIR_SOCKET_HAS_ADDR(&def->netmask)) {
- virBufferAddLit(&buf, " <ip");
-
- if (VIR_SOCKET_HAS_ADDR(&def->ipAddress)) {
- char *addr = virSocketFormatAddr(&def->ipAddress);
- if (!addr)
- goto error;
- virBufferVSprintf(&buf, " address='%s'", addr);
- VIR_FREE(addr);
- }
-
- if (VIR_SOCKET_HAS_ADDR(&def->netmask)) {
- char *addr = virSocketFormatAddr(&def->netmask);
- if (!addr)
- goto error;
- virBufferVSprintf(&buf, " netmask='%s'", addr);
- VIR_FREE(addr);
- }
-
- virBufferAddLit(&buf, ">\n");
-
- if (def->tftproot) {
- virBufferEscapeString(&buf, " <tftp root='%s'
/>\n",
- def->tftproot);
- }
- if ((def->nranges || def->nhosts)) {
- int i;
- virBufferAddLit(&buf, " <dhcp>\n");
- for (i = 0 ; i < def->nranges ; i++) {
- char *saddr = virSocketFormatAddr(&def->ranges[i].start);
- if (!saddr)
- goto error;
- char *eaddr = virSocketFormatAddr(&def->ranges[i].end);
- if (!eaddr) {
- VIR_FREE(saddr);
- goto error;
- }
- virBufferVSprintf(&buf, " <range start='%s'
end='%s' />\n",
- saddr, eaddr);
- VIR_FREE(saddr);
- VIR_FREE(eaddr);
- }
- for (i = 0 ; i < def->nhosts ; i++) {
- virBufferAddLit(&buf, " <host ");
- if (def->hosts[i].mac)
- virBufferVSprintf(&buf, "mac='%s' ",
def->hosts[i].mac);
- if (def->hosts[i].name)
- virBufferVSprintf(&buf, "name='%s' ",
def->hosts[i].name);
- if (VIR_SOCKET_HAS_ADDR(&def->hosts[i].ip)) {
- char *ipaddr = virSocketFormatAddr(&def->hosts[i].ip);
- if (!ipaddr)
- goto error;
- virBufferVSprintf(&buf, "ip='%s' ", ipaddr);
- VIR_FREE(ipaddr);
- }
- virBufferAddLit(&buf, "/>\n");
- }
- if (def->bootfile) {
- virBufferEscapeString(&buf, " <bootp file='%s'
",
- def->bootfile);
- if (VIR_SOCKET_HAS_ADDR(&def->bootserver)) {
- char *ipaddr = virSocketFormatAddr(&def->bootserver);
- if (!ipaddr)
- goto error;
- virBufferEscapeString(&buf, "server='%s' ",
ipaddr);
- VIR_FREE(ipaddr);
- }
- virBufferAddLit(&buf, "/>\n");
- }
-
- virBufferAddLit(&buf, " </dhcp>\n");
- }
-
- virBufferAddLit(&buf, " </ip>\n");
+ for (ii = 0; ii < def->nips; ii++) {
+ if (virNetworkIpDefFormat(&buf, &def->ips[ii]) < 0)
+ goto error;
}
virBufferAddLit(&buf, "</network>\n");
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index a922d28..47f0408 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -56,6 +56,33 @@ struct _virNetworkDHCPHostDef {
virSocketAddr ip;
};
+typedef struct _virNetworkIpDef virNetworkIpDef;
+typedef virNetworkIpDef *virNetworkIpDefPtr;
+struct _virNetworkIpDef {
+ char *family; /* ipv4 or ipv6 - default is ipv4 */
+ virSocketAddr address; /* Bridge IP address */
+
+ /* The first two items below are taken directly from the XML (one
+ * or the other is given, but not both) and the 3rd is derived
+ * from the first two. When formatting XML, always use netMasktStr
+ * if it's non-NULL, but when using netmask in other code, use
+ * netmask, as it will automatically take into account prefix or
+ * natural netmasks (when no netmask or prefix is specified).
+ */
+ int prefix; /* ipv6 - only prefix allowed */
+ virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */
+
+ unsigned int nranges; /* Zero or more dhcp ranges */
+ virNetworkDHCPRangeDefPtr ranges;
+
+ unsigned int nhosts; /* Zero or more dhcp hosts */
+ virNetworkDHCPHostDefPtr hosts;
+
+ char *tftproot;
+ char *bootfile;
+ virSocketAddr bootserver;
+ };
+
typedef struct _virNetworkDef virNetworkDef;
typedef virNetworkDef *virNetworkDefPtr;
struct _virNetworkDef {
@@ -70,18 +97,8 @@ struct _virNetworkDef {
int forwardType; /* One of virNetworkForwardType constants */
char *forwardDev; /* Destination device for forwarding */
- virSocketAddr ipAddress; /* Bridge IP address */
- virSocketAddr netmask;
-
- unsigned int nranges; /* Zero or more dhcp ranges */
- virNetworkDHCPRangeDefPtr ranges;
-
- unsigned int nhosts; /* Zero or more dhcp hosts */
- virNetworkDHCPHostDefPtr hosts;
-
- char *tftproot;
- char *bootfile;
- virSocketAddr bootserver;
+ int nips;
+ virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */
};
typedef struct _virNetworkObj virNetworkObj;
@@ -133,9 +150,12 @@ virNetworkDefPtr virNetworkDefParseNode(xmlDocPtr xml,
char *virNetworkDefFormat(const virNetworkDefPtr def);
-int virNetworkDefPrefix(const virNetworkDefPtr def);
-int virNetworkDefNetmask(const virNetworkDefPtr def,
- virSocketAddrPtr netmask);
+virNetworkIpDefPtr
+virNetworkDefGetIpByIndex(const virNetworkDefPtr def,
+ int family, int n);
+int virNetworkIpDefPrefix(const virNetworkIpDefPtr def);
+int virNetworkIpDefNetmask(const virNetworkIpDefPtr def,
+ virSocketAddrPtr netmask);
int virNetworkSaveXML(const char *configDir,
virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9ba7b1..6cb8270 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -578,15 +578,16 @@ virNetworkAssignDef;
virNetworkConfigFile;
virNetworkDefFormat;
virNetworkDefFree;
-virNetworkDefNetmask;
+virNetworkDefGetIpByIndex;
virNetworkDefParseFile;
virNetworkDefParseNode;
virNetworkDefParseString;
-virNetworkDefPrefix;
virNetworkDeleteConfig;
virNetworkFindByName;
virNetworkFindByUUID;
virNetworkLoadAllConfigs;
+virNetworkIpDefNetmask;
+virNetworkIpDefPrefix;
virNetworkObjIsDuplicate;
virNetworkObjListFree;
virNetworkObjLock;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b8132ba..fe336a4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -145,8 +145,7 @@ networkFindActiveConfigs(struct network_driver *driver) {
obj->active = 1;
/* Finally try and read dnsmasq pid if any */
- if ((VIR_SOCKET_HAS_ADDR(&obj->def->ipAddress) ||
- obj->def->nranges) &&
+ if (obj->def->ips && (obj->def->nips > 0) &&
virFileReadPid(NETWORK_PID_DIR, obj->def->name,
&obj->dnsmasqPid) == 0) {
@@ -366,7 +365,7 @@ networkShutdown(void) {
static int
-networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
+networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
dnsmasqContext *dctx,
bool force)
{
@@ -375,8 +374,8 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
if (! force && virFileExists(dctx->hostsfile->path))
return 0;
- for (i = 0 ; i < network->def->nhosts ; i++) {
- virNetworkDHCPHostDefPtr host = &(network->def->hosts[i]);
+ for (i = 0; i < ipdef->nhosts; i++) {
+ virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
if ((host->mac) && VIR_SOCKET_HAS_ADDR(&host->ip))
dnsmasqAddDhcpHost(dctx, host->mac, &host->ip, host->name);
}
@@ -390,13 +389,14 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
static int
networkBuildDnsmasqArgv(virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef,
const char *pidfile,
virCommandPtr cmd) {
int r, ret = -1;
int nbleases = 0;
char *bridgeaddr;
- if (!(bridgeaddr = virSocketFormatAddr(&network->def->ipAddress)))
+ if (!(bridgeaddr = virSocketFormatAddr(&ipdef->address)))
goto cleanup;
/*
* NB, be careful about syntax for dnsmasq options in long format.
@@ -438,18 +438,18 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* clearly not practical
*
* virCommandAddArg(cmd, "--interface");
- * virCommandAddArg(cmd, network->def->bridge);
+ * virCommandAddArg(cmd, ipdef->bridge);
*/
virCommandAddArgList(cmd,
"--listen-address", bridgeaddr,
"--except-interface", "lo",
NULL);
- for (r = 0 ; r < network->def->nranges ; r++) {
- char *saddr = virSocketFormatAddr(&network->def->ranges[r].start);
+ for (r = 0 ; r < ipdef->nranges ; r++) {
+ char *saddr = virSocketFormatAddr(&ipdef->ranges[r].start);
if (!saddr)
goto cleanup;
- char *eaddr = virSocketFormatAddr(&network->def->ranges[r].end);
+ char *eaddr = virSocketFormatAddr(&ipdef->ranges[r].end);
if (!eaddr) {
VIR_FREE(saddr);
goto cleanup;
@@ -458,8 +458,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr);
VIR_FREE(saddr);
VIR_FREE(eaddr);
- nbleases += virSocketGetRange(&network->def->ranges[r].start,
- &network->def->ranges[r].end);
+ nbleases += virSocketGetRange(&ipdef->ranges[r].start,
+ &ipdef->ranges[r].end);
}
/*
@@ -467,19 +467,19 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* we have to add a special --dhcp-range option to enable the service in
* dnsmasq.
*/
- if (!network->def->nranges && network->def->nhosts) {
+ if (!ipdef->nranges && ipdef->nhosts) {
virCommandAddArg(cmd, "--dhcp-range");
virCommandAddArgFormat(cmd, "%s,static", bridgeaddr);
}
- if (network->def->nranges > 0) {
+ if (ipdef->nranges > 0) {
virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases);
}
- if (network->def->nranges || network->def->nhosts)
+ if (ipdef->nranges || ipdef->nhosts)
virCommandAddArg(cmd, "--dhcp-no-override");
- if (network->def->nhosts > 0) {
+ if (ipdef->nhosts > 0) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL) {
@@ -487,31 +487,30 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
goto cleanup;
}
- if (networkSaveDnsmasqHostsfile(network, dctx, false) < 0) {
+ if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) < 0) {
virCommandAddArgPair(cmd, "--dhcp-hostsfile",
dctx->hostsfile->path);
}
dnsmasqContextFree(dctx);
}
- if (network->def->tftproot) {
+ if (ipdef->tftproot) {
virCommandAddArgList(cmd, "--enable-tftp",
- "--tftp-root", network->def->tftproot,
+ "--tftp-root", ipdef->tftproot,
NULL);
}
- if (network->def->bootfile) {
-
+ if (ipdef->bootfile) {
virCommandAddArg(cmd, "--dhcp-boot");
- if (VIR_SOCKET_HAS_ADDR(&network->def->bootserver)) {
- char *bootserver = virSocketFormatAddr(&network->def->bootserver);
+ if (VIR_SOCKET_HAS_ADDR(&ipdef->bootserver)) {
+ char *bootserver = virSocketFormatAddr(&ipdef->bootserver);
if (!bootserver)
goto cleanup;
virCommandAddArgFormat(cmd, "%s%s%s",
- network->def->bootfile, ",,",
bootserver);
+ ipdef->bootfile, ",,", bootserver);
VIR_FREE(bootserver);
} else {
- virCommandAddArg(cmd, network->def->bootfile);
+ virCommandAddArg(cmd, ipdef->bootfile);
}
}
@@ -521,9 +520,9 @@ cleanup:
return ret;
}
-
static int
-dhcpStartDhcpDaemon(virNetworkObjPtr network)
+dhcpStartDhcpDaemon(virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
{
virCommandPtr cmd = NULL;
char *pidfile = NULL;
@@ -531,7 +530,7 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network)
network->dnsmasqPid = -1;
- if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) {
+ if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot start dhcp daemon without IPv4
address for server"));
goto cleanup;
@@ -556,7 +555,7 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network)
}
cmd = virCommandNew(DNSMASQ);
- if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) {
+ if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd) < 0) {
VIR_FREE(pidfile);
goto cleanup;
}
@@ -585,8 +584,10 @@ cleanup:
static int
networkAddMasqueradingIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network) {
- int prefix = virNetworkDefPrefix(network->def);
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
+{
+ int prefix = virNetworkIpDefPrefix(ipdef);
if (prefix < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -597,7 +598,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* allow forwarding packets from the bridge interface */
if (iptablesAddForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
@@ -609,7 +610,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* allow forwarding packets to the bridge interface if they are part of an existing
connection */
if (iptablesAddForwardAllowRelatedIn(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
@@ -644,7 +645,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* First the generic masquerade rule for other protocols */
if (iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
NULL) < 0) {
@@ -656,7 +657,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* UDP with a source port restriction */
if (iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
"udp") < 0) {
@@ -668,7 +669,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
/* TCP with a source port restriction */
if (iptablesAddForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
"tcp") < 0) {
@@ -682,25 +683,25 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
masqerr5:
iptablesRemoveForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
"udp");
masqerr4:
iptablesRemoveForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
NULL);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
masqerr2:
iptablesRemoveForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
@@ -710,8 +711,9 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
static int
networkAddRoutingIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network) {
- int prefix = virNetworkDefPrefix(network->def);
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef) {
+ int prefix = virNetworkIpDefPrefix(ipdef);
if (prefix < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -722,7 +724,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
/* allow routing packets from the bridge interface */
if (iptablesAddForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
@@ -734,7 +736,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
/* allow routing packets to the bridge interface */
if (iptablesAddForwardAllowIn(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev) < 0) {
@@ -749,7 +751,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
routeerr2:
iptablesRemoveForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
@@ -759,7 +761,8 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
static int
networkAddIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network) {
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef) {
/* allow DHCP requests through to dnsmasq */
if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0)
{
@@ -792,7 +795,7 @@ networkAddIptablesRules(struct network_driver *driver,
}
/* allow TFTP requests through to dnsmasq */
- if (network->def->tftproot &&
+ if (ipdef && ipdef->tftproot &&
iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0)
{
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow TFTP requests
from '%s'"),
@@ -825,29 +828,30 @@ networkAddIptablesRules(struct network_driver *driver,
goto err7;
}
-
- /* If masquerading is enabled, set up the rules*/
- if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
- networkAddMasqueradingIptablesRules(driver, network) < 0)
- goto err8;
- /* else if routing is enabled, set up the rules*/
- else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
- networkAddRoutingIptablesRules(driver, network) < 0)
- goto err8;
-
- /* If we are doing local DHCP service on this network, attempt to
- * add a rule that will fixup the checksum of DHCP response
- * packets back to the guests (but report failure without
- * aborting, since not all iptables implementations support it).
- */
-
- if ((VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) ||
- network->def->nranges) &&
- (iptablesAddOutputFixUdpChecksum(driver->iptables,
- network->def->bridge, 68) < 0)) {
- VIR_WARN("Could not add rule to fixup DHCP response checksums "
- "on network '%s'.", network->def->name);
- VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
+ if (ipdef) {
+ /* If masquerading is enabled, set up the rules*/
+ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
+ networkAddMasqueradingIptablesRules(driver, network, ipdef) < 0)
+ goto err8;
+ /* else if routing is enabled, set up the rules*/
+ else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
+ networkAddRoutingIptablesRules(driver, network, ipdef) < 0)
+ goto err8;
+
+ /* If we are doing local DHCP service on this network, attempt to
+ * add a rule that will fixup the checksum of DHCP response
+ * packets back to the guests (but report failure without
+ * aborting, since not all iptables implementations support it).
+ */
+
+ if (ipdef && (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET) ||
+ ipdef->nranges) &&
+ (iptablesAddOutputFixUdpChecksum(driver->iptables,
+ network->def->bridge, 68) < 0)) {
+ VIR_WARN("Could not add rule to fixup DHCP response checksums "
+ "on network '%s'.", network->def->name);
+ VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
+ }
}
return 0;
@@ -862,7 +866,7 @@ networkAddIptablesRules(struct network_driver *driver,
iptablesRemoveForwardRejectOut(driver->iptables,
network->def->bridge);
err5:
- if (network->def->tftproot) {
+ if (ipdef && ipdef->tftproot) {
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
}
err4tftp:
@@ -879,14 +883,16 @@ networkAddIptablesRules(struct network_driver *driver,
static void
networkRemoveIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network) {
- if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) ||
- network->def->nranges) {
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef) {
+
+ if (ipdef && (VIR_SOCKET_HAS_ADDR(&ipdef->address) ||
+ ipdef->nranges)) {
iptablesRemoveOutputFixUdpChecksum(driver->iptables,
network->def->bridge, 68);
}
- if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
- int prefix = virNetworkDefPrefix(network->def);
+ if (ipdef && network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
+ int prefix = virNetworkIpDefPrefix(ipdef);
if (prefix < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -897,34 +903,35 @@ networkRemoveIptablesRules(struct network_driver *driver,
if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
iptablesRemoveForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
"tcp");
iptablesRemoveForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
"udp");
iptablesRemoveForwardMasquerade(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->forwardDev,
NULL);
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
- } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)
+ } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) {
iptablesRemoveForwardAllowIn(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
+ }
iptablesRemoveForwardAllowOut(driver->iptables,
- &network->def->ipAddress,
+ &ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
@@ -933,7 +940,7 @@ error:
iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
- if (network->def->tftproot)
+ if (ipdef && ipdef->tftproot)
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53);
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53);
@@ -952,10 +959,18 @@ networkReloadIptablesRules(struct network_driver *driver)
virNetworkObjLock(driver->networks.objs[i]);
if (virNetworkObjIsActive(driver->networks.objs[i])) {
- networkRemoveIptablesRules(driver, driver->networks.objs[i]);
- if (networkAddIptablesRules(driver, driver->networks.objs[i]) < 0) {
- /* failed to add but already logged */
- }
+ virNetworkIpDefPtr ipv4def;
+
+ /* Find the one allowed IPv4 ip address in the definition */
+ /* Even if none is found, we still call the functions below */
+ ipv4def = virNetworkDefGetIpByIndex(driver->networks.objs[i]->def,
+ AF_INET, 0);
+ networkRemoveIptablesRules(driver, driver->networks.objs[i],
+ ipv4def);
+ if (networkAddIptablesRules(driver, driver->networks.objs[i],
+ ipv4def) < 0) {
+ /* failed to add but already logged */
+ }
}
virNetworkObjUnlock(driver->networks.objs[i]);
@@ -1029,7 +1044,8 @@ cleanup:
* other scenarios where we can ruin host network connectivity.
* XXX: Using a proper library is preferred over parsing /proc
*/
-static int networkCheckRouteCollision(virNetworkObjPtr network)
+static int networkCheckRouteCollision(virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
{
int ret = -1, len;
unsigned int net_dest;
@@ -1037,18 +1053,18 @@ static int networkCheckRouteCollision(virNetworkObjPtr network)
char *cur, *buf = NULL;
enum {MAX_ROUTE_SIZE = 1024*64};
- if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) {
+ if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
/* Only support collision check for IPv4 */
return 0;
}
- if (virNetworkDefNetmask(network->def, &netmask) < 0) {
+ if (virNetworkIpDefNetmask(ipdef, &netmask) < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to get netmask of '%s'"),
network->def->bridge);
}
- net_dest = (network->def->ipAddress.data.inet4.sin_addr.s_addr &
+ net_dest = (ipdef->address.data.inet4.sin_addr.s_addr &
netmask.data.inet4.sin_addr.s_addr);
/* Read whole routing table into memory */
@@ -1122,6 +1138,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
{
int err;
virErrorPtr save_err;
+ virNetworkIpDefPtr ipv4def;
if (virNetworkObjIsActive(network)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1129,8 +1146,11 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
return -1;
}
+ /* find the one allowed IPv4 ip address in the definition */
+ ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
+
/* Check to see if network collides with an existing route */
- if (networkCheckRouteCollision(network) < 0)
+ if (ipv4def && networkCheckRouteCollision(network, ipv4def) < 0)
return -1;
if ((err = brAddBridge(driver->brctl, network->def->bridge))) {
@@ -1159,8 +1179,8 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
goto err_delbr;
}
- if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) {
- int prefix = virNetworkDefPrefix(network->def);
+ if (ipv4def) {
+ int prefix = virNetworkIpDefPrefix(ipv4def);
if (prefix < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1170,7 +1190,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
}
if ((err = brAddInetAddress(driver->brctl, network->def->bridge,
- &network->def->ipAddress, prefix))) {
+ &ipv4def->address, prefix))) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set IP address on bridge
'%s'"),
network->def->bridge);
@@ -1185,7 +1205,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
goto err_delbr;
}
- if (networkAddIptablesRules(driver, network) < 0)
+ if (ipv4def && networkAddIptablesRules(driver, network, ipv4def) < 0)
goto err_delbr1;
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
@@ -1195,12 +1215,13 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
goto err_delbr2;
}
- if ((VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) ||
- network->def->nranges) &&
- dhcpStartDhcpDaemon(network) < 0)
+ /*
+ * Start the dhcp daemon for the 1st (and only supported) ipv4
+ * address.
+ */
+ if (ipv4def && dhcpStartDhcpDaemon(network, ipv4def) < 0)
goto err_delbr2;
-
/* Persist the live configuration now we have bridge info */
if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) {
goto err_kill;
@@ -1218,7 +1239,8 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
err_delbr2:
save_err = virSaveLastError();
- networkRemoveIptablesRules(driver, network);
+ if (ipv4def)
+ networkRemoveIptablesRules(driver, network, ipv4def);
if (save_err) {
virSetError(save_err);
virFreeError(save_err);
@@ -1247,6 +1269,7 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
{
int err;
char *stateFile;
+ virNetworkIpDefPtr ipv4def;
VIR_INFO(_("Shutting down network '%s'"),
network->def->name);
@@ -1263,7 +1286,10 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
if (network->dnsmasqPid > 0)
kill(network->dnsmasqPid, SIGTERM);
- networkRemoveIptablesRules(driver, network);
+ /* find the one allowed IPv4 ip address in the definition */
+ ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
+ if (ipv4def)
+ networkRemoveIptablesRules(driver, network, ipv4def);
char ebuf[1024];
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
@@ -1527,6 +1553,7 @@ cleanup:
static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
struct network_driver *driver = conn->networkPrivateData;
+ virNetworkIpDefPtr ipv4def;
virNetworkDefPtr def;
virNetworkObjPtr network = NULL;
virNetworkPtr ret = NULL;
@@ -1557,12 +1584,15 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
goto cleanup;
}
- if (network->def->nhosts > 0) {
+ /* we only support dhcp on one IPv4 address per defined network */
+ ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
+
+ if (ipv4def && ipv4def->nhosts > 0) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL)
goto cleanup;
- networkSaveDnsmasqHostsfile(network, dctx, true);
+ networkSaveDnsmasqHostsfile(ipv4def, dctx, true);
dnsmasqContextFree(dctx);
}
@@ -1578,7 +1608,8 @@ cleanup:
static int networkUndefine(virNetworkPtr net) {
struct network_driver *driver = net->conn->networkPrivateData;
- virNetworkObjPtr network = NULL;
+ virNetworkObjPtr network;
+ virNetworkIpDefPtr ipv4def;
int ret = -1;
networkDriverLock(driver);
@@ -1601,7 +1632,10 @@ static int networkUndefine(virNetworkPtr net) {
network) < 0)
goto cleanup;
- if (network->def->nhosts > 0) {
+ /* find the one allowed IPv4 ip address in the definition */
+ ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
+
+ if (ipv4def && ipv4def->nhosts > 0) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL)
goto cleanup;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 728c501..03defb5 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -7038,9 +7038,23 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn,
const char *
IHostNetworkInterface *networkInterface = NULL;
virNetworkDefPtr def = virNetworkDefParseString(xml);
+ virNetworkIpDefPtr ipdef;
+ virSocketAddr netmask;
if ( (!def)
- || (def->forwardType != VIR_NETWORK_FORWARD_NONE))
+ || (def->forwardType != VIR_NETWORK_FORWARD_NONE)
+ || (def->nips == 0 || !def->ips))
+ goto cleanup;
+
+ /* Look for the first IPv4 IP address definition and use that.
+ * If there weren't any IPv4 addresses, ignore the network (since it's
+ * required below to have an IPv4 address)
+ */
+ ipdef = virNetworkDefGetIpByIndex(def, AF_INET, 0);
+ if (!ipdef)
+ goto cleanup;
+
+ if (virNetworkIpDefNetmask(ipdef, &netmask) < 0)
goto cleanup;
/* the current limitation of hostonly network is that you can't
@@ -7096,9 +7110,9 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn,
const char *
/* Currently support only one dhcp server per network
* with contigious address space from start to end
*/
- if ((def->nranges >= 1) &&
- VIR_SOCKET_HAS_ADDR(&def->ranges[0].start) &&
- VIR_SOCKET_HAS_ADDR(&def->ranges[0].end)) {
+ if ((ipdef->nranges >= 1) &&
+ VIR_SOCKET_HAS_ADDR(&ipdef->ranges[0].start) &&
+ VIR_SOCKET_HAS_ADDR(&ipdef->ranges[0].end)) {
IDHCPServer *dhcpServer = NULL;
data->vboxObj->vtbl->FindDHCPServerByNetworkName(data->vboxObj,
@@ -7118,10 +7132,10 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr
conn, const char *
PRUnichar *toIPAddressUtf16 = NULL;
PRUnichar *trunkTypeUtf16 = NULL;
- ipAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&def->ipAddress);
- networkMaskUtf16 = vboxSocketFormatAddrUtf16(data,
&def->netmask);
- fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&def->ranges[0].start);
- toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&def->ranges[0].end);
+ ipAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&ipdef->address);
+ networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask);
+ fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&ipdef->ranges[0].start);
+ toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&ipdef->ranges[0].end);
if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL ||
fromIPAddressUtf16 == NULL || toIPAddressUtf16 == NULL) {
@@ -7158,13 +7172,13 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr
conn, const char *
}
}
- if ((def->nhosts >= 1) &&
- VIR_SOCKET_HAS_ADDR(&def->hosts[0].ip)) {
+ if ((ipdef->nhosts >= 1) &&
+ VIR_SOCKET_HAS_ADDR(&ipdef->hosts[0].ip)) {
PRUnichar *ipAddressUtf16 = NULL;
PRUnichar *networkMaskUtf16 = NULL;
- ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &def->hosts[0].ip);
- networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &def->netmask);
+ ipAddressUtf16 = vboxSocketFormatAddrUtf16(data,
&ipdef->hosts[0].ip);
+ networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask);
if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL) {
VBOX_UTF16_FREE(ipAddressUtf16);
@@ -7385,12 +7399,19 @@ static int vboxNetworkDestroy(virNetworkPtr network) {
static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSED) {
VBOX_OBJECT_HOST_CHECK(network->conn, char *, NULL);
virNetworkDefPtr def = NULL;
+ virNetworkIpDefPtr ipdef = NULL;
char *networkNameUtf8 = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
goto cleanup;
}
+ if (VIR_ALLOC(ipdef) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ def->ips = ipdef;
+ def->nips = 1;
if (virAsprintf(&networkNameUtf8, "HostInterfaceNetworking-%s",
network->name) < 0) {
virReportOOMError();
@@ -7427,8 +7448,8 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
networkNameUtf16,
&dhcpServer);
if (dhcpServer) {
- def->nranges = 1;
- if (VIR_ALLOC_N(def->ranges, def->nranges) >=0 ) {
+ ipdef->nranges = 1;
+ if (VIR_ALLOC_N(ipdef->ranges, ipdef->nranges) >=0 ) {
PRUnichar *ipAddressUtf16 = NULL;
PRUnichar *networkMaskUtf16 = NULL;
PRUnichar *fromIPAddressUtf16 = NULL;
@@ -7443,13 +7464,13 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
* with contigious address space from start to end
*/
if (vboxSocketParseAddrUtf16(data, ipAddressUtf16,
- &def->ipAddress) < 0 ||
+ &ipdef->address) < 0 ||
vboxSocketParseAddrUtf16(data, networkMaskUtf16,
- &def->netmask) < 0 ||
+ &ipdef->netmask) < 0 ||
vboxSocketParseAddrUtf16(data, fromIPAddressUtf16,
- &def->ranges[0].start) < 0
||
+ &ipdef->ranges[0].start) <
0 ||
vboxSocketParseAddrUtf16(data, toIPAddressUtf16,
- &def->ranges[0].end) < 0)
{
+ &ipdef->ranges[0].end) <
0) {
errorOccurred = true;
}
@@ -7462,16 +7483,16 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
goto cleanup;
}
} else {
- def->nranges = 0;
+ ipdef->nranges = 0;
virReportOOMError();
}
- def->nhosts = 1;
- if (VIR_ALLOC_N(def->hosts, def->nhosts) >=0 ) {
- def->hosts[0].name = strdup(network->name);
- if (def->hosts[0].name == NULL) {
- VIR_FREE(def->hosts);
- def->nhosts = 0;
+ ipdef->nhosts = 1;
+ if (VIR_ALLOC_N(ipdef->hosts, ipdef->nhosts) >=0 ) {
+ ipdef->hosts[0].name = strdup(network->name);
+ if (ipdef->hosts[0].name == NULL) {
+ VIR_FREE(ipdef->hosts);
+ ipdef->nhosts = 0;
virReportOOMError();
} else {
PRUnichar *macAddressUtf16 = NULL;
@@ -7481,10 +7502,10 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
networkInterface->vtbl->GetHardwareAddress(networkInterface, &macAddressUtf16);
networkInterface->vtbl->GetIPAddress(networkInterface,
&ipAddressUtf16);
- VBOX_UTF16_TO_UTF8(macAddressUtf16,
&def->hosts[0].mac);
+ VBOX_UTF16_TO_UTF8(macAddressUtf16,
&ipdef->hosts[0].mac);
if (vboxSocketParseAddrUtf16(data, ipAddressUtf16,
- &def->hosts[0].ip) <
0) {
+ &ipdef->hosts[0].ip) <
0) {
errorOccurred = true;
}
@@ -7496,7 +7517,7 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
}
}
} else {
- def->nhosts = 0;
+ ipdef->nhosts = 0;
}
VBOX_RELEASE(dhcpServer);
@@ -7509,9 +7530,9 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags
ATTRIBUTE_UNUSE
networkInterface->vtbl->GetIPAddress(networkInterface,
&ipAddressUtf16);
if (vboxSocketParseAddrUtf16(data, networkMaskUtf16,
- &def->netmask) < 0 ||
+ &ipdef->netmask) < 0 ||
vboxSocketParseAddrUtf16(data, ipAddressUtf16,
- &def->ipAddress) < 0) {
+ &ipdef->address) < 0) {
errorOccurred = true;
}
diff --git a/tests/networkxml2xmlin/nat-network.xml
b/tests/networkxml2xmlin/nat-network.xml
index 93ab186..23f7fcb 100644
--- a/tests/networkxml2xmlin/nat-network.xml
+++ b/tests/networkxml2xmlin/nat-network.xml
@@ -10,4 +10,12 @@
<host mac="00:16:3e:3e:a9:1a" name="b.example.com"
ip="192.168.122.11" />
</dhcp>
</ip>
+ <ip family="ipv4" address="192.168.123.1"
netmask="255.255.255.0">
+ </ip>
+ <ip family="ipv6" address="2001:db8:ac10:fe01::1"
prefix="64">
+ </ip>
+ <ip family="ipv6" address="2001:db8:ac10:fd01::1"
prefix="64">
+ </ip>
+ <ip family="ipv4" address="10.24.10.1">
+ </ip>
</network>
diff --git a/tests/networkxml2xmlout/nat-network.xml
b/tests/networkxml2xmlout/nat-network.xml
index 036d4fb..eb71d9e 100644
--- a/tests/networkxml2xmlout/nat-network.xml
+++ b/tests/networkxml2xmlout/nat-network.xml
@@ -10,4 +10,12 @@
<host mac='00:16:3e:3e:a9:1a' name='b.example.com'
ip='192.168.122.11' />
</dhcp>
</ip>
+ <ip family='ipv4' address='192.168.123.1'
netmask='255.255.255.0'>
+ </ip>
+ <ip family='ipv6' address='2001:db8:ac10:fe01::1'
prefix='64'>
+ </ip>
+ <ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
+ </ip>
+ <ip family='ipv4' address='10.24.10.1'>
+ </ip>
</network>
--
1.7.3.4
2:03 a.m.
New subject: [libvirt] [PATCH 10/13] Support multiple IP addresses on one network in bridge_driver.c
This patch reorganizes the code in bridge_driver.c to account for the
concept of a single network with multiple IP addresses, without adding
in the extra variable of IPv6. A small bit of code has been
temporarily added that checks all given addresses to verify they are
IPv4 - this will be removed when full IPv6 support is turned on.
---
src/network/bridge_driver.c | 603 ++++++++++++++++++++++++++----------------
1 files changed, 373 insertions(+), 230 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fe336a4..606dde3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -521,20 +521,25 @@ cleanup:
}
static int
-dhcpStartDhcpDaemon(virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef)
+networkStartDhcpDaemon(virNetworkObjPtr network)
{
virCommandPtr cmd = NULL;
char *pidfile = NULL;
- int ret = -1, err;
+ int ret = -1, err, ii;
+ virNetworkIpDefPtr ipdef;
network->dnsmasqPid = -1;
- if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot start dhcp daemon without IPv4
address for server"));
- goto cleanup;
+ /* Look for first IPv4 address that has dhcp defined. */
+ /* We support dhcp config on 1 IPv4 interface only. */
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ ii++) {
+ if (ipdef->nranges || ipdef->nhosts)
+ break;
}
+ if (!ipdef)
+ return 0;
if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) {
virReportSystemError(err,
@@ -584,8 +589,8 @@ cleanup:
static int
networkAddMasqueradingIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef)
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
{
int prefix = virNetworkIpDefPrefix(ipdef);
@@ -608,7 +613,9 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
goto masqerr1;
}
- /* allow forwarding packets to the bridge interface if they are part of an existing
connection */
+ /* allow forwarding packets to the bridge interface if they are
+ * part of an existing connection
+ */
if (iptablesAddForwardAllowRelatedIn(driver->iptables,
&ipdef->address,
prefix,
@@ -709,10 +716,48 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
return -1;
}
+static void
+networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
+{
+ int prefix = virNetworkIpDefPrefix(ipdef);
+
+ if (prefix >= 0) {
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->forwardDev,
+ "tcp");
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->forwardDev,
+ "udp");
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->forwardDev,
+ NULL);
+
+ iptablesRemoveForwardAllowRelatedIn(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->bridge,
+ network->def->forwardDev);
+ iptablesRemoveForwardAllowOut(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->bridge,
+ network->def->forwardDev);
+ }
+}
+
static int
networkAddRoutingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef) {
+ virNetworkIpDefPtr ipdef)
+{
int prefix = virNetworkIpDefPrefix(ipdef);
if (prefix < 0) {
@@ -748,23 +793,56 @@ networkAddRoutingIptablesRules(struct network_driver *driver,
return 0;
-
- routeerr2:
+routeerr2:
iptablesRemoveForwardAllowOut(driver->iptables,
&ipdef->address,
prefix,
network->def->bridge,
network->def->forwardDev);
- routeerr1:
+routeerr1:
return -1;
}
+static void
+networkRemoveRoutingIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
+{
+ int prefix = virNetworkIpDefPrefix(ipdef);
+
+ if (prefix >= 0) {
+ iptablesRemoveForwardAllowIn(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->bridge,
+ network->def->forwardDev);
+
+ iptablesRemoveForwardAllowOut(driver->iptables,
+ &ipdef->address,
+ prefix,
+ network->def->bridge,
+ network->def->forwardDev);
+ }
+}
+
static int
-networkAddIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef) {
+networkAddGeneralIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ int ii;
+ virNetworkIpDefPtr ipv4def;
+
+ /* First look for first IPv4 address that has dhcp or tftpboot defined. */
+ /* We support dhcp config on 1 IPv4 interface only. */
+ for (ii = 0;
+ (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ ii++) {
+ if (ipv4def->nranges || ipv4def->nhosts || ipv4def->tftproot)
+ break;
+ }
/* allow DHCP requests through to dnsmasq */
+
if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0)
{
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow DHCP requests
from '%s'"),
@@ -779,6 +857,20 @@ networkAddIptablesRules(struct network_driver *driver,
goto err2;
}
+ /* If we are doing local DHCP service on this network, attempt to
+ * add a rule that will fixup the checksum of DHCP response
+ * packets back to the guests (but report failure without
+ * aborting, since not all iptables implementations support it).
+ */
+
+ if (ipv4def && (ipv4def->nranges || ipv4def->nhosts) &&
+ (iptablesAddOutputFixUdpChecksum(driver->iptables,
+ network->def->bridge, 68) < 0)) {
+ VIR_WARN("Could not add rule to fixup DHCP response checksums "
+ "on network '%s'.", network->def->name);
+ VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
+ }
+
/* allow DNS requests through to dnsmasq */
if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 53) < 0)
{
networkReportError(VIR_ERR_SYSTEM_ERROR,
@@ -794,30 +886,29 @@ networkAddIptablesRules(struct network_driver *driver,
goto err4;
}
- /* allow TFTP requests through to dnsmasq */
- if (ipdef && ipdef->tftproot &&
+ /* allow TFTP requests through to dnsmasq if necessary */
+ if (ipv4def && ipv4def->tftproot &&
iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0)
{
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow TFTP requests
from '%s'"),
network->def->bridge);
- goto err4tftp;
+ goto err5;
}
-
/* Catch all rules to block forwarding to/from bridges */
if (iptablesAddForwardRejectOut(driver->iptables, network->def->bridge) <
0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to block outbound traffic
from '%s'"),
network->def->bridge);
- goto err5;
+ goto err6;
}
if (iptablesAddForwardRejectIn(driver->iptables, network->def->bridge) <
0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to block inbound traffic
to '%s'"),
network->def->bridge);
- goto err6;
+ goto err7;
}
/* Allow traffic between guests on the same bridge */
@@ -825,129 +916,139 @@ networkAddIptablesRules(struct network_driver *driver,
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow cross bridge
traffic on '%s'"),
network->def->bridge);
- goto err7;
- }
-
- if (ipdef) {
- /* If masquerading is enabled, set up the rules*/
- if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
- networkAddMasqueradingIptablesRules(driver, network, ipdef) < 0)
- goto err8;
- /* else if routing is enabled, set up the rules*/
- else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
- networkAddRoutingIptablesRules(driver, network, ipdef) < 0)
- goto err8;
-
- /* If we are doing local DHCP service on this network, attempt to
- * add a rule that will fixup the checksum of DHCP response
- * packets back to the guests (but report failure without
- * aborting, since not all iptables implementations support it).
- */
-
- if (ipdef && (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET) ||
- ipdef->nranges) &&
- (iptablesAddOutputFixUdpChecksum(driver->iptables,
- network->def->bridge, 68) < 0)) {
- VIR_WARN("Could not add rule to fixup DHCP response checksums "
- "on network '%s'.", network->def->name);
- VIR_WARN0("May need to update iptables package & kernel to support
CHECKSUM rule.");
- }
+ goto err8;
}
return 0;
- err8:
- iptablesRemoveForwardAllowCross(driver->iptables,
- network->def->bridge);
- err7:
- iptablesRemoveForwardRejectIn(driver->iptables,
- network->def->bridge);
- err6:
- iptablesRemoveForwardRejectOut(driver->iptables,
- network->def->bridge);
- err5:
- if (ipdef && ipdef->tftproot) {
+ /* unwind in reverse order from the point of failure */
+err8:
+ iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
+err7:
+ iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
+err6:
+ if (ipv4def && ipv4def->tftproot) {
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
}
- err4tftp:
+err5:
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53);
- err4:
+err4:
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53);
- err3:
+err3:
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67);
- err2:
+err2:
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67);
- err1:
+err1:
return -1;
}
static void
-networkRemoveIptablesRules(struct network_driver *driver,
- virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef) {
+networkRemoveGeneralIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ int ii;
+ virNetworkIpDefPtr ipv4def;
- if (ipdef && (VIR_SOCKET_HAS_ADDR(&ipdef->address) ||
- ipdef->nranges)) {
- iptablesRemoveOutputFixUdpChecksum(driver->iptables,
- network->def->bridge, 68);
+ for (ii = 0;
+ (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ ii++) {
+ if (ipv4def->nranges || ipv4def->nhosts || ipv4def->tftproot)
+ break;
}
- if (ipdef && network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
- int prefix = virNetworkIpDefPrefix(ipdef);
-
- if (prefix < 0) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Invalid prefix or netmask for
'%s'"),
- network->def->bridge);
- goto error;
- }
-
- if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
- iptablesRemoveForwardMasquerade(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->forwardDev,
- "tcp");
- iptablesRemoveForwardMasquerade(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->forwardDev,
- "udp");
- iptablesRemoveForwardMasquerade(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->forwardDev,
- NULL);
- iptablesRemoveForwardAllowRelatedIn(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->bridge,
- network->def->forwardDev);
- } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) {
- iptablesRemoveForwardAllowIn(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->bridge,
- network->def->forwardDev);
- }
- iptablesRemoveForwardAllowOut(driver->iptables,
- &ipdef->address,
- prefix,
- network->def->bridge,
- network->def->forwardDev);
- }
-error:
iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
- if (ipdef && ipdef->tftproot)
+ if (ipv4def && ipv4def->tftproot) {
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
+ }
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53);
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53);
+ if (ipv4def && (ipv4def->nranges || ipv4def->nhosts)) {
+ iptablesRemoveOutputFixUdpChecksum(driver->iptables,
+ network->def->bridge, 68);
+ }
iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67);
iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67);
}
+static int
+networkAddIpSpecificIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
+{
+ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
+ networkAddMasqueradingIptablesRules(driver, network, ipdef) < 0)
+ return -1;
+ else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
+ networkAddRoutingIptablesRules(driver, network, ipdef) < 0)
+ return -1;
+
+ return 0;
+}
+
+static void
+networkRemoveIpSpecificIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
+{
+ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT)
+ networkRemoveMasqueradingIptablesRules(driver, network, ipdef);
+ else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)
+ networkRemoveRoutingIptablesRules(driver, network, ipdef);
+}
+
+/* Add all rules for all ip addresses (and general rules) on a network */
+static int
+networkAddIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ int ii;
+ virNetworkIpDefPtr ipdef;
+
+ /* Add "once per network" rules */
+ if (networkAddGeneralIptablesRules(driver, network) < 0)
+ return -1;
+
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
+ ii++) {
+ /* Add address-specific iptables rules */
+ if (networkAddIpSpecificIptablesRules(driver, network, ipdef) < 0) {
+ goto err;
+ }
+ }
+ return 0;
+
+err:
+ /* The final failed call to networkAddIpSpecificIptablesRules will
+ * have removed any rules it created, but we need to remove those
+ * added for previous IP addresses.
+ */
+ while ((--ii >= 0) &&
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii))) {
+ networkRemoveIpSpecificIptablesRules(driver, network, ipdef);
+ }
+ networkRemoveGeneralIptablesRules(driver, network);
+ return -1;
+}
+
+/* Remove all rules for all ip addresses (and general rules) on a network */
+static void
+networkRemoveIptablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ int ii;
+ virNetworkIpDefPtr ipdef;
+
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
+ ii++) {
+ networkRemoveIpSpecificIptablesRules(driver, network, ipdef);
+ }
+ networkRemoveGeneralIptablesRules(driver, network);
+}
+
static void
networkReloadIptablesRules(struct network_driver *driver)
{
@@ -957,22 +1058,12 @@ networkReloadIptablesRules(struct network_driver *driver)
for (i = 0 ; i < driver->networks.count ; i++) {
virNetworkObjLock(driver->networks.objs[i]);
-
if (virNetworkObjIsActive(driver->networks.objs[i])) {
- virNetworkIpDefPtr ipv4def;
-
- /* Find the one allowed IPv4 ip address in the definition */
- /* Even if none is found, we still call the functions below */
- ipv4def = virNetworkDefGetIpByIndex(driver->networks.objs[i]->def,
- AF_INET, 0);
- networkRemoveIptablesRules(driver, driver->networks.objs[i],
- ipv4def);
- if (networkAddIptablesRules(driver, driver->networks.objs[i],
- ipv4def) < 0) {
- /* failed to add but already logged */
- }
+ networkRemoveIptablesRules(driver, driver->networks.objs[i]);
+ if (networkAddIptablesRules(driver, driver->networks.objs[i]) < 0) {
+ /* failed to add but already logged */
+ }
}
-
virNetworkObjUnlock(driver->networks.objs[i]);
}
}
@@ -1044,32 +1135,16 @@ cleanup:
* other scenarios where we can ruin host network connectivity.
* XXX: Using a proper library is preferred over parsing /proc
*/
-static int networkCheckRouteCollision(virNetworkObjPtr network,
- virNetworkIpDefPtr ipdef)
+static int
+networkCheckRouteCollision(virNetworkObjPtr network)
{
- int ret = -1, len;
- unsigned int net_dest;
- virSocketAddr netmask;
+ int ret = 0, len;
char *cur, *buf = NULL;
enum {MAX_ROUTE_SIZE = 1024*64};
- if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
- /* Only support collision check for IPv4 */
- return 0;
- }
-
- if (virNetworkIpDefNetmask(ipdef, &netmask) < 0) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to get netmask of '%s'"),
- network->def->bridge);
- }
-
- net_dest = (ipdef->address.data.inet4.sin_addr.s_addr &
- netmask.data.inet4.sin_addr.s_addr);
-
/* Read whole routing table into memory */
if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0)
- goto error;
+ goto out;
/* Dropping the last character shouldn't hurt */
if (len > 0)
@@ -1088,7 +1163,8 @@ static int networkCheckRouteCollision(virNetworkObjPtr network,
while (cur) {
char iface[17], dest[128], mask[128];
unsigned int addr_val, mask_val;
- int num;
+ virNetworkIpDefPtr ipdef;
+ int num, ii;
/* NUL-terminate the line, so sscanf doesn't go beyond a newline. */
char *nl = strchr(cur, '\n');
@@ -1117,28 +1193,70 @@ static int networkCheckRouteCollision(virNetworkObjPtr network,
addr_val &= mask_val;
- if ((net_dest == addr_val) &&
- (netmask.data.inet4.sin_addr.s_addr == mask_val)) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Network is already in use by interface %s"),
- iface);
- goto error;
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ ii++) {
+
+ unsigned int net_dest;
+ virSocketAddr netmask;
+
+ if (virNetworkIpDefNetmask(ipdef, &netmask) < 0) {
+ VIR_WARN("Failed to get netmask of '%s'",
+ network->def->bridge);
+ continue;
+ }
+
+ net_dest = (ipdef->address.data.inet4.sin_addr.s_addr &
+ netmask.data.inet4.sin_addr.s_addr);
+
+ if ((net_dest == addr_val) &&
+ (netmask.data.inet4.sin_addr.s_addr == mask_val)) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Network is already in use by interface
%s"),
+ iface);
+ ret = -1;
+ goto out;
+ }
}
}
out:
- ret = 0;
-error:
VIR_FREE(buf);
return ret;
}
-static int networkStartNetworkDaemon(struct network_driver *driver,
- virNetworkObjPtr network)
+static int
+networkAddAddrToBridge(struct network_driver *driver,
+ virNetworkObjPtr network,
+ virNetworkIpDefPtr ipdef)
{
- int err;
- virErrorPtr save_err;
- virNetworkIpDefPtr ipv4def;
+ int prefix = virNetworkIpDefPrefix(ipdef);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP
address"),
+ network->def->bridge);
+ return -1;
+ }
+
+ if (brAddInetAddress(driver->brctl, network->def->bridge,
+ &ipdef->address, prefix) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot set IP address on bridge '%s'"),
+ network->def->bridge);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+networkStartNetworkDaemon(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ int ii, err, v4present = 0;
+ virErrorPtr save_err = NULL;
+ virNetworkIpDefPtr ipdef;
if (virNetworkObjIsActive(network)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1146,13 +1264,11 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
return -1;
}
- /* find the one allowed IPv4 ip address in the definition */
- ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
-
- /* Check to see if network collides with an existing route */
- if (ipv4def && networkCheckRouteCollision(network, ipv4def) < 0)
+ /* Check to see if any network IP collides with an existing route */
+ if (networkCheckRouteCollision(network) < 0)
return -1;
+ /* Create and configure the bridge device */
if ((err = brAddBridge(driver->brctl, network->def->bridge))) {
virReportSystemError(err,
_("cannot create bridge '%s'"),
@@ -1160,15 +1276,13 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
return -1;
}
- if (networkDisableIPV6(network) < 0)
- goto err_delbr;
-
+ /* Set bridge options */
if ((err = brSetForwardDelay(driver->brctl, network->def->bridge,
network->def->delay))) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set forward delay on bridge
'%s'"),
network->def->bridge);
- goto err_delbr;
+ goto err1;
}
if ((err = brSetEnableSTP(driver->brctl, network->def->bridge,
@@ -1176,90 +1290,95 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set STP '%s' on bridge
'%s'"),
network->def->stp ? "on" : "off",
network->def->bridge);
- goto err_delbr;
+ goto err1;
}
- if (ipv4def) {
- int prefix = virNetworkIpDefPrefix(ipv4def);
+ /* Disable IPv6 on the bridge */
+ if (networkDisableIPV6(network) < 0)
+ goto err1;
- if (prefix < 0) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("bridge '%s' has an invalid netmask or IP
address"),
- network->def->bridge);
- goto err_delbr;
- }
+ /* Add "once per network" rules */
+ if (networkAddIptablesRules(driver, network) < 0)
+ goto err1;
+
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
+ ii++) {
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET))
+ v4present = 1;
- if ((err = brAddInetAddress(driver->brctl, network->def->bridge,
- &ipv4def->address, prefix))) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot set IP address on bridge
'%s'"),
- network->def->bridge);
- goto err_delbr;
+ /* Add the IP address/netmask to the bridge */
+ if (networkAddAddrToBridge(driver, network, ipdef) < 0) {
+ goto err2;
}
}
+ /* Bring up the bridge interface */
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) {
virReportSystemError(err,
_("failed to bring the bridge '%s' up"),
network->def->bridge);
- goto err_delbr;
+ goto err2;
}
- if (ipv4def && networkAddIptablesRules(driver, network, ipv4def) < 0)
- goto err_delbr1;
-
+ /* If forwardType != NONE, turn on global IP forwarding */
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
networkEnableIpForwarding() < 0) {
virReportSystemError(errno, "%s",
_("failed to enable IP forwarding"));
- goto err_delbr2;
+ goto err3;
}
- /*
- * Start the dhcp daemon for the 1st (and only supported) ipv4
- * address.
- */
- if (ipv4def && dhcpStartDhcpDaemon(network, ipv4def) < 0)
- goto err_delbr2;
+
+ /* start dnsmasq if there are any IPv4 addresses */
+ if (v4present && networkStartDhcpDaemon(network) < 0)
+ goto err3;
/* Persist the live configuration now we have bridge info */
if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) {
- goto err_kill;
+ goto err4;
}
network->active = 1;
return 0;
- err_kill:
+ err4:
+ if (!save_err)
+ save_err = virSaveLastError();
+
if (network->dnsmasqPid > 0) {
kill(network->dnsmasqPid, SIGTERM);
network->dnsmasqPid = -1;
}
- err_delbr2:
- save_err = virSaveLastError();
- if (ipv4def)
- networkRemoveIptablesRules(driver, network, ipv4def);
- if (save_err) {
- virSetError(save_err);
- virFreeError(save_err);
- }
-
- err_delbr1:
+ err3:
+ if (!save_err)
+ save_err = virSaveLastError();
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
char ebuf[1024];
VIR_WARN("Failed to bring down bridge '%s' : %s",
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
- err_delbr:
+ err2:
+ if (!save_err)
+ save_err = virSaveLastError();
+ networkRemoveIptablesRules(driver, network);
+
+ err1:
+ if (!save_err)
+ save_err = virSaveLastError();
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
char ebuf[1024];
VIR_WARN("Failed to delete bridge '%s' : %s",
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
+ if (save_err) {
+ virSetError(save_err);
+ virFreeError(save_err);
+ }
return -1;
}
@@ -1269,7 +1388,6 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
{
int err;
char *stateFile;
- virNetworkIpDefPtr ipv4def;
VIR_INFO(_("Shutting down network '%s'"),
network->def->name);
@@ -1286,17 +1404,14 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
if (network->dnsmasqPid > 0)
kill(network->dnsmasqPid, SIGTERM);
- /* find the one allowed IPv4 ip address in the definition */
- ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
- if (ipv4def)
- networkRemoveIptablesRules(driver, network, ipv4def);
-
char ebuf[1024];
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
VIR_WARN("Failed to bring down bridge '%s' : %s",
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
+ networkRemoveIptablesRules(driver, network);
+
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
VIR_WARN("Failed to delete bridge '%s' : %s",
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
@@ -1553,10 +1668,11 @@ cleanup:
static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
struct network_driver *driver = conn->networkPrivateData;
- virNetworkIpDefPtr ipv4def;
+ virNetworkIpDefPtr ipdef, ipv4def = NULL;
virNetworkDefPtr def;
virNetworkObjPtr network = NULL;
virNetworkPtr ret = NULL;
+ int ii;
networkDriverLock(driver);
@@ -1584,10 +1700,33 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
goto cleanup;
}
- /* we only support dhcp on one IPv4 address per defined network */
- ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
+ /* we only support dhcp on one IPv4 address per defined network, and currently
+ * don't support IPv6.
+ */
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
+ ii++) {
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
+ if (ipdef->nranges || ipdef->nhosts) {
+ if (ipv4def) {
+ networkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("Multiple dhcp sections
found. dhcp is supported only for a single IPv4 address on each network"));
+ goto cleanup;
+ } else {
+ ipv4def = ipdef;
+ }
+ }
+ } else {
+ /* we currently only support IPv4 */
+ networkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported address family '%s' (%d) in
network definition"),
+ ipdef->family ? ipdef->family :
"unspecified",
+ VIR_SOCKET_FAMILY(&ipdef->address));
+ goto cleanup;
- if (ipv4def && ipv4def->nhosts > 0) {
+ }
+ }
+ if (ipv4def) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL)
goto cleanup;
@@ -1610,7 +1749,7 @@ static int networkUndefine(virNetworkPtr net) {
struct network_driver *driver = net->conn->networkPrivateData;
virNetworkObjPtr network;
virNetworkIpDefPtr ipv4def;
- int ret = -1;
+ int ret = -1, ii;
networkDriverLock(driver);
@@ -1632,10 +1771,14 @@ static int networkUndefine(virNetworkPtr net) {
network) < 0)
goto cleanup;
- /* find the one allowed IPv4 ip address in the definition */
- ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
-
- if (ipv4def && ipv4def->nhosts > 0) {
+ /* we only support dhcp on one IPv4 address per defined network */
+ for (ii = 0;
+ (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ ii++) {
+ if (ipv4def->nranges || ipv4def->nhosts)
+ break;
+ }
+ if (ipv4def) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL)
goto cleanup;
--
1.7.3.4
2:03 a.m.
New subject: [libvirt] [PATCH 11/13] Update iptables.c to also support ip6tables.
All of the iptables functions eventually call down to a single
bottom-level function, and fortunately, ip6tables syntax (for all the
args that we use) is identical to iptables format (except the
addresses), so all we need to do is:
1) Get an address family down to the lowest level function in each
case, either implied through an address, or explicitly when no
address is in the parameter list, and
2) At the lowest level, just decide whether to call "iptables" or
"ip6tables" based on the family.
The location of the ip6tables binary is determined at build time by
autoconf. If a particular target system happens to not have ip6tables
installed, any attempts to run it will generate an error, but that
won't happen unless someone tries to define an IPv6 address for a
network. This is identical behavior to IPv4 addresses and iptables.
---
configure.ac | 3 ++
src/network/bridge_driver.c | 54 ++++++++++++++++++-------------
src/util/iptables.c | 75 +++++++++++++++++++++++++++++++-----------
src/util/iptables.h | 10 ++++++
4 files changed, 99 insertions(+), 43 deletions(-)
diff --git a/configure.ac b/configure.ac
index 357d59b..279ccd2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -312,6 +312,9 @@ AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip
binary])
AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables
binary])
+AC_PATH_PROG([IP6TABLES_PATH], [ip6tables], /sbin/ip6tables, [/usr/sbin:$PATH])
+AC_DEFINE_UNQUOTED([IP6TABLES_PATH], "$IP6TABLES_PATH", [path to ip6tables
binary])
+
AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH])
AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables
binary])
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 606dde3..e405429 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -843,14 +843,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
/* allow DHCP requests through to dnsmasq */
- if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0)
{
+ if (iptablesAddTcpInput(driver->iptables, AF_INET,
+ network->def->bridge, 67) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow DHCP requests
from '%s'"),
network->def->bridge);
goto err1;
}
- if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 67) < 0)
{
+ if (iptablesAddUdpInput(driver->iptables, AF_INET,
+ network->def->bridge, 67) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow DHCP requests
from '%s'"),
network->def->bridge);
@@ -872,14 +874,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
}
/* allow DNS requests through to dnsmasq */
- if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 53) < 0)
{
+ if (iptablesAddTcpInput(driver->iptables, AF_INET,
+ network->def->bridge, 53) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow DNS requests from
'%s'"),
network->def->bridge);
goto err3;
}
- if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 53) < 0)
{
+ if (iptablesAddUdpInput(driver->iptables, AF_INET,
+ network->def->bridge, 53) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow DNS requests from
'%s'"),
network->def->bridge);
@@ -888,7 +892,8 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
/* allow TFTP requests through to dnsmasq if necessary */
if (ipv4def && ipv4def->tftproot &&
- iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0)
{
+ iptablesAddUdpInput(driver->iptables, AF_INET,
+ network->def->bridge, 69) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow TFTP requests
from '%s'"),
network->def->bridge);
@@ -897,14 +902,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
/* Catch all rules to block forwarding to/from bridges */
- if (iptablesAddForwardRejectOut(driver->iptables, network->def->bridge) <
0) {
+ if (iptablesAddForwardRejectOut(driver->iptables, AF_INET,
+ network->def->bridge) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to block outbound traffic
from '%s'"),
network->def->bridge);
goto err6;
}
- if (iptablesAddForwardRejectIn(driver->iptables, network->def->bridge) <
0) {
+ if (iptablesAddForwardRejectIn(driver->iptables, AF_INET,
+ network->def->bridge) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to block inbound traffic
to '%s'"),
network->def->bridge);
@@ -912,7 +919,8 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
}
/* Allow traffic between guests on the same bridge */
- if (iptablesAddForwardAllowCross(driver->iptables, network->def->bridge)
< 0) {
+ if (iptablesAddForwardAllowCross(driver->iptables, AF_INET,
+ network->def->bridge) < 0) {
networkReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to add iptables rule to allow cross bridge
traffic on '%s'"),
network->def->bridge);
@@ -923,21 +931,21 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
/* unwind in reverse order from the point of failure */
err8:
- iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
+ iptablesRemoveForwardRejectIn(driver->iptables, AF_INET,
network->def->bridge);
err7:
- iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
+ iptablesRemoveForwardRejectOut(driver->iptables, AF_INET,
network->def->bridge);
err6:
if (ipv4def && ipv4def->tftproot) {
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
69);
}
err5:
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
53);
err4:
- iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53);
+ iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge,
53);
err3:
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
67);
err2:
- iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67);
+ iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge,
67);
err1:
return -1;
}
@@ -956,20 +964,20 @@ networkRemoveGeneralIptablesRules(struct network_driver *driver,
break;
}
- iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge);
- iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge);
- iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge);
+ iptablesRemoveForwardAllowCross(driver->iptables, AF_INET,
network->def->bridge);
+ iptablesRemoveForwardRejectIn(driver->iptables, AF_INET,
network->def->bridge);
+ iptablesRemoveForwardRejectOut(driver->iptables, AF_INET,
network->def->bridge);
if (ipv4def && ipv4def->tftproot) {
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
69);
}
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53);
- iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
53);
+ iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge,
53);
if (ipv4def && (ipv4def->nranges || ipv4def->nhosts)) {
iptablesRemoveOutputFixUdpChecksum(driver->iptables,
network->def->bridge, 68);
}
- iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67);
- iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67);
+ iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge,
67);
+ iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge,
67);
}
static int
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 89e05b7..c3e18cb 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -99,14 +99,17 @@ iptRulesNew(const char *table,
}
static int ATTRIBUTE_SENTINEL
-iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...)
+iptablesAddRemoveRule(iptRules *rules, int family, int action,
+ const char *arg, ...)
{
va_list args;
int ret;
virCommandPtr cmd;
const char *s;
- cmd = virCommandNew(IPTABLES_PATH);
+ cmd = virCommandNew((family == AF_INET6)
+ ? IP6TABLES_PATH : IPTABLES_PATH);
+
virCommandAddArgList(cmd, "--table", rules->table,
action == ADD ? "--insert" : "--delete",
rules->chain, arg, NULL);
@@ -177,6 +180,7 @@ iptablesContextFree(iptablesContext *ctx)
static int
iptablesInput(iptablesContext *ctx,
+ int family,
const char *iface,
int port,
int action,
@@ -188,6 +192,7 @@ iptablesInput(iptablesContext *ctx,
portstr[sizeof(portstr) - 1] = '\0';
return iptablesAddRemoveRule(ctx->input_filter,
+ family,
action,
"--in-interface", iface,
"--protocol", tcp ? "tcp" :
"udp",
@@ -210,10 +215,11 @@ iptablesInput(iptablesContext *ctx,
int
iptablesAddTcpInput(iptablesContext *ctx,
+ int family,
const char *iface,
int port)
{
- return iptablesInput(ctx, iface, port, ADD, 1);
+ return iptablesInput(ctx, family, iface, port, ADD, 1);
}
/**
@@ -229,10 +235,11 @@ iptablesAddTcpInput(iptablesContext *ctx,
*/
int
iptablesRemoveTcpInput(iptablesContext *ctx,
+ int family,
const char *iface,
int port)
{
- return iptablesInput(ctx, iface, port, REMOVE, 1);
+ return iptablesInput(ctx, family, iface, port, REMOVE, 1);
}
/**
@@ -249,10 +256,11 @@ iptablesRemoveTcpInput(iptablesContext *ctx,
int
iptablesAddUdpInput(iptablesContext *ctx,
+ int family,
const char *iface,
int port)
{
- return iptablesInput(ctx, iface, port, ADD, 0);
+ return iptablesInput(ctx, family, iface, port, ADD, 0);
}
/**
@@ -268,10 +276,11 @@ iptablesAddUdpInput(iptablesContext *ctx,
*/
int
iptablesRemoveUdpInput(iptablesContext *ctx,
+ int family,
const char *iface,
int port)
{
- return iptablesInput(ctx, iface, port, REMOVE, 0);
+ return iptablesInput(ctx, family, iface, port, REMOVE, 0);
}
@@ -282,9 +291,10 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr,
char *netstr;
char *ret;
- if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET)) {
+ if (!(VIR_SOCKET_IS_FAMILY(netaddr, AF_INET) ||
+ VIR_SOCKET_IS_FAMILY(netaddr, AF_INET6))) {
iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only IPv4 addresses can be used with iptables"));
+ _("Only IPv4 or IPv6 addresses can be used with
iptables"));
return NULL;
}
@@ -327,6 +337,7 @@ iptablesForwardAllowOut(iptablesContext *ctx,
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"--in-interface", iface,
@@ -335,6 +346,7 @@ iptablesForwardAllowOut(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"--in-interface", iface,
@@ -411,6 +423,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx,
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--destination", networkstr,
"--in-interface", physdev,
@@ -421,6 +434,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--destination", networkstr,
"--out-interface", iface,
@@ -497,6 +511,7 @@ iptablesForwardAllowIn(iptablesContext *ctx,
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--destination", networkstr,
"--in-interface", physdev,
@@ -505,6 +520,7 @@ iptablesForwardAllowIn(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->forward_filter,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--destination", networkstr,
"--out-interface", iface,
@@ -567,10 +583,12 @@ iptablesRemoveForwardAllowIn(iptablesContext *ctx,
*/
static int
iptablesForwardAllowCross(iptablesContext *ctx,
+ int family,
const char *iface,
int action)
{
return iptablesAddRemoveRule(ctx->forward_filter,
+ family,
action,
"--in-interface", iface,
"--out-interface", iface,
@@ -591,8 +609,10 @@ iptablesForwardAllowCross(iptablesContext *ctx,
*/
int
iptablesAddForwardAllowCross(iptablesContext *ctx,
- const char *iface) {
- return iptablesForwardAllowCross(ctx, iface, ADD);
+ int family,
+ const char *iface)
+{
+ return iptablesForwardAllowCross(ctx, family, iface, ADD);
}
/**
@@ -608,8 +628,10 @@ iptablesAddForwardAllowCross(iptablesContext *ctx,
*/
int
iptablesRemoveForwardAllowCross(iptablesContext *ctx,
- const char *iface) {
- return iptablesForwardAllowCross(ctx, iface, REMOVE);
+ int family,
+ const char *iface)
+{
+ return iptablesForwardAllowCross(ctx, family, iface, REMOVE);
}
@@ -618,14 +640,16 @@ iptablesRemoveForwardAllowCross(iptablesContext *ctx,
*/
static int
iptablesForwardRejectOut(iptablesContext *ctx,
+ int family,
const char *iface,
int action)
{
return iptablesAddRemoveRule(ctx->forward_filter,
- action,
- "--in-interface", iface,
- "--jump", "REJECT",
- NULL);
+ family,
+ action,
+ "--in-interface", iface,
+ "--jump", "REJECT",
+ NULL);
}
/**
@@ -640,9 +664,10 @@ iptablesForwardRejectOut(iptablesContext *ctx,
*/
int
iptablesAddForwardRejectOut(iptablesContext *ctx,
+ int family,
const char *iface)
{
- return iptablesForwardRejectOut(ctx, iface, ADD);
+ return iptablesForwardRejectOut(ctx, family, iface, ADD);
}
/**
@@ -657,9 +682,10 @@ iptablesAddForwardRejectOut(iptablesContext *ctx,
*/
int
iptablesRemoveForwardRejectOut(iptablesContext *ctx,
+ int family,
const char *iface)
{
- return iptablesForwardRejectOut(ctx, iface, REMOVE);
+ return iptablesForwardRejectOut(ctx, family, iface, REMOVE);
}
@@ -670,10 +696,12 @@ iptablesRemoveForwardRejectOut(iptablesContext *ctx,
*/
static int
iptablesForwardRejectIn(iptablesContext *ctx,
+ int family,
const char *iface,
int action)
{
return iptablesAddRemoveRule(ctx->forward_filter,
+ family,
action,
"--out-interface", iface,
"--jump", "REJECT",
@@ -692,9 +720,10 @@ iptablesForwardRejectIn(iptablesContext *ctx,
*/
int
iptablesAddForwardRejectIn(iptablesContext *ctx,
+ int family,
const char *iface)
{
- return iptablesForwardRejectIn(ctx, iface, ADD);
+ return iptablesForwardRejectIn(ctx, family, iface, ADD);
}
/**
@@ -709,9 +738,10 @@ iptablesAddForwardRejectIn(iptablesContext *ctx,
*/
int
iptablesRemoveForwardRejectIn(iptablesContext *ctx,
+ int family,
const char *iface)
{
- return iptablesForwardRejectIn(ctx, iface, REMOVE);
+ return iptablesForwardRejectIn(ctx, family, iface, REMOVE);
}
@@ -735,6 +765,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
if (protocol && protocol[0]) {
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"-p", protocol,
@@ -745,6 +776,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"-p", protocol,
@@ -756,6 +788,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
} else {
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"!", "--destination",
networkstr,
@@ -764,6 +797,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"!", "--destination",
networkstr,
@@ -834,6 +868,7 @@ iptablesOutputFixUdpChecksum(iptablesContext *ctx,
portstr[sizeof(portstr) - 1] = '\0';
return iptablesAddRemoveRule(ctx->mangle_postrouting,
+ AF_INET,
action,
"--out-interface", iface,
"--protocol", "udp",
diff --git a/src/util/iptables.h b/src/util/iptables.h
index 6f59b92..8088c8e 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -30,16 +30,20 @@ iptablesContext *iptablesContextNew (void);
void iptablesContextFree (iptablesContext *ctx);
int iptablesAddTcpInput (iptablesContext *ctx,
+ int family,
const char *iface,
int port);
int iptablesRemoveTcpInput (iptablesContext *ctx,
+ int family,
const char *iface,
int port);
int iptablesAddUdpInput (iptablesContext *ctx,
+ int family,
const char *iface,
int port);
int iptablesRemoveUdpInput (iptablesContext *ctx,
+ int family,
const char *iface,
int port);
@@ -77,18 +81,24 @@ int iptablesRemoveForwardAllowIn (iptablesContext
*ctx,
const char *physdev);
int iptablesAddForwardAllowCross (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesRemoveForwardAllowCross (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesAddForwardRejectOut (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesRemoveForwardRejectOut (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesAddForwardRejectIn (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesRemoveForwardRejectIn (iptablesContext *ctx,
+ int family,
const char *iface);
int iptablesAddForwardMasquerade (iptablesContext *ctx,
--
1.7.3.4
9:48 a.m.
New subject: [libvirt] [PATCH 11/13] Update iptables.c to also support ip6tables.
On Mon, Dec 20, 2010 at 03:03:23AM -0500, Laine Stump wrote:
All of the iptables functions eventually call down to a single
bottom-level function, and fortunately, ip6tables syntax (for all the
args that we use) is identical to iptables format (except the
addresses), so all we need to do is:
1) Get an address family down to the lowest level function in each
case, either implied through an address, or explicitly when no
address is in the parameter list, and
2) At the lowest level, just decide whether to call "iptables" or
"ip6tables" based on the family.
The location of the ip6tables binary is determined at build time by
autoconf. If a particular target system happens to not have ip6tables
installed, any attempts to run it will generate an error, but that
won't happen unless someone tries to define an IPv6 address for a
network. This is identical behavior to IPv4 addresses and iptables.
@@ -735,6 +765,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
if (protocol && protocol[0]) {
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"-p", protocol,
@@ -745,6 +776,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"-p", protocol,
@@ -756,6 +788,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
} else {
if (physdev && physdev[0]) {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"!", "--destination",
networkstr,
@@ -764,6 +797,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
NULL);
} else {
ret = iptablesAddRemoveRule(ctx->nat_postrouting,
+ VIR_SOCKET_FAMILY(netaddr),
action,
"--source", networkstr,
"!", "--destination",
networkstr,
Masquerading doesn't exist in IPv6 world, so technically we should be
raising an error for AF_INET6 in these 4 cases as a sanity check.
Daniel
11:19 a.m.
New subject: [libvirt] [PATCH 11/13] Update iptables.c to also support ip6tables.
On 01/04/2011 10:48 AM, Daniel P. Berrange wrote:
> @@ -735,6 +765,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
> if (protocol&& protocol[0]) {
> if (physdev&& physdev[0]) {
> ret = iptablesAddRemoveRule(ctx->nat_postrouting,
> + VIR_SOCKET_FAMILY(netaddr),
> action,
> "--source", networkstr,
> "-p", protocol,
> @@ -745,6 +776,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
> NULL);
> } else {
> ret = iptablesAddRemoveRule(ctx->nat_postrouting,
> + VIR_SOCKET_FAMILY(netaddr),
> action,
> "--source", networkstr,
> "-p", protocol,
> @@ -756,6 +788,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
> } else {
> if (physdev&& physdev[0]) {
> ret = iptablesAddRemoveRule(ctx->nat_postrouting,
> + VIR_SOCKET_FAMILY(netaddr),
> action,
> "--source", networkstr,
> "!", "--destination",
networkstr,
> @@ -764,6 +797,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
> NULL);
> } else {
> ret = iptablesAddRemoveRule(ctx->nat_postrouting,
> + VIR_SOCKET_FAMILY(netaddr),
> action,
> "--source", networkstr,
> "!", "--destination",
networkstr,
Masquerading doesn't exist in IPv6 world, so technically we should be
raising an error for AF_INET6 in these 4 cases as a sanity check.
Good point. I was just absent-mindedly following the form of the other
changes, relying on the fact that we never call it that way. :-)
I'll make an appropriate patch that gives an error if someone tries to
call it with an IPv6 address (I guess it should be an internal error,
since the higher level code is currently already assuring that we don't
do that).
2:03 a.m.
New subject: [libvirt] [PATCH 12/13] Turn on IPv6 support in the bridge_driver.c virtual network driver
At this point everything is already in place to make IPv6 happen, we just
need to add a few rules, remove some checks for IPv4-only, and document
the changes to the XML on the website.
---
docs/formatnetwork.html.in | 35 +++++++--
src/network/bridge_driver.c | 186 ++++++++++++++++++++++++++++++++----------
2 files changed, 169 insertions(+), 52 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 7a24518..b1b0485 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -85,8 +85,13 @@
</dd>
<dt><code>forward</code></dt>
<dd>Inclusion of the <code>forward</code> element indicates that
- the virtual network is to be connected to the physical LAN. If
- no attributes are set, NAT forwarding will be used for connectivity.
+ the virtual network is to be connected to the physical
+ LAN. the <code>mode</code> attribute determines the method of
+ forwarding; possible selections are 'nat' and 'route'. If mode
+ is not specified, NAT forwarding will be used for
+ connectivity. If a network has any IPv6 addresses defined,
+ even if <code>mode</code> is given as 'nat', the IPv6
traffic
+ will be forwarded using routing, since IPv6 has no concept of NAT.
Firewall rules will allow forwarding to any other network device whether
ethernet, wireless, dialup, or VPN. If the <code>dev</code>
attribute
is set, the firewall rules will restrict forwarding to the named
@@ -118,21 +123,37 @@
<dl>
<dt><code>ip</code></dt>
<dd>The <code>address</code> attribute defines an IPv4 address
in
- dotted-decimal format, that will be configured on the bridge
+ dotted-decimal format, or an IPv6 address in standard
+ colon-separated hexadecimal format, that will be configured on
+ the bridge
device associated with the virtual network. To the guests this
- address will be their default route. The <code>netmask</code>
+ address will be their default route. For IPv4 addresses, the
<code>netmask</code>
attribute defines the significant bits of the network address,
- again specified in dotted-decimal format. <span
class="since">Since 0.3.0</span>
+ again specified in dotted-decimal format. For IPv6 addresses,
+ and as an alternate method for IPv4 addresses, you can specify
+ the significant bits of the network address with the
<code>prefix</code>
+ attribute, which is an integer (for example,
<code>netmask='255.255.255.0'</code>
+ could also be given as <code>prefix='24'</code>. The
<code>family</code>
+ attribute is used to specify the type of address - 'ipv4' or
'ipv6'; if no
+ <code>family</code> is given, 'ipv4' is assumed. A network
can have more than
+ one of each family of address defined, but only a single address can have a
+ <code>dhcp</code> or <code>tftp</code> element. <span
class="since">Since 0.3.0;
+ IPv6, multiple addresses on a single network, <code>family</code>,
and
+ <code>prefix</code> since 0.8.7</span>
</dd><dt><code>tftp</code></dt><dd>Immediately
within
the <code>ip</code> element there is an optional
<code>tftp</code>
element. The presence of this element and of its attribute
<code>root</code> enables TFTP services. The attribute specifies
- the path to the root directory served via TFTP.
+ the path to the root directory served via TFTP. <code>tftp</code> is
not
+ supported for IPv6 addresses, can only be specified on a single IPv4 address
+ per network.
<span class="since">Since 0.7.1</span>
</dd><dt><code>dhcp</code></dt><dd>Also within
the <code>ip</code> element there is an
optional <code>dhcp</code> element. The presence of this element
enables DHCP services on the virtual network. It will further
- contain one or more <code>range</code> elements.
+ contain one or more <code>range</code> elements. The
+ <code>dhcp</code> element is not supported for IPv6, and
+ is only supported on a single IP address per network for IPv4.
<span class="since">Since 0.3.0</span>
</dd>
<dt><code>range</code></dt>
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e405429..665a13f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -825,6 +825,65 @@ networkRemoveRoutingIptablesRules(struct network_driver *driver,
}
}
+/* Add all once/network rules required for IPv6 (if any IPv6 addresses are defined) */
+static int
+networkAddGeneralIp6tablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+
+ if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
+ return 0;
+
+ /* Catch all rules to block forwarding to/from bridges */
+
+ if (iptablesAddForwardRejectOut(driver->iptables, AF_INET6,
+ network->def->bridge) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add ip6tables rule to block outbound traffic
from '%s'"),
+ network->def->bridge);
+ goto err1;
+ }
+
+ if (iptablesAddForwardRejectIn(driver->iptables, AF_INET6,
+ network->def->bridge) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add ip6tables rule to block inbound traffic
to '%s'"),
+ network->def->bridge);
+ goto err2;
+ }
+
+ /* Allow traffic between guests on the same bridge */
+ if (iptablesAddForwardAllowCross(driver->iptables, AF_INET6,
+ network->def->bridge) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add ip6tables rule to allow cross bridge
traffic on '%s'"),
+ network->def->bridge);
+ goto err3;
+ }
+
+ return 0;
+
+ /* unwind in reverse order from the point of failure */
+err3:
+ iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6,
network->def->bridge);
+err2:
+ iptablesRemoveForwardRejectOut(driver->iptables, AF_INET6,
network->def->bridge);
+err1:
+ return -1;
+}
+
+static void
+networkRemoveGeneralIp6tablesRules(struct network_driver *driver,
+ virNetworkObjPtr network)
+{
+ if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
+ return;
+
+ iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6,
network->def->bridge);
+ iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6,
network->def->bridge);
+ iptablesRemoveForwardRejectOut(driver->iptables, AF_INET6,
network->def->bridge);
+}
+
static int
networkAddGeneralIptablesRules(struct network_driver *driver,
virNetworkObjPtr network)
@@ -927,6 +986,11 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
goto err8;
}
+ /* add IPv6 general rules, if needed */
+ if (networkAddGeneralIp6tablesRules(driver, network) < 0) {
+ goto err8;
+ }
+
return 0;
/* unwind in reverse order from the point of failure */
@@ -957,6 +1021,8 @@ networkRemoveGeneralIptablesRules(struct network_driver *driver,
int ii;
virNetworkIpDefPtr ipv4def;
+ networkRemoveGeneralIp6tablesRules(driver, network);
+
for (ii = 0;
(ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
ii++) {
@@ -985,12 +1051,18 @@ networkAddIpSpecificIptablesRules(struct network_driver *driver,
virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
{
- if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT &&
- networkAddMasqueradingIptablesRules(driver, network, ipdef) < 0)
- return -1;
- else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE &&
- networkAddRoutingIptablesRules(driver, network, ipdef) < 0)
- return -1;
+ /* NB: in the case of IPv6, routing rules are added when the
+ * forward mode is NAT. This is because IPv6 has no NAT.
+ */
+
+ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET))
+ return networkAddMasqueradingIptablesRules(driver, network, ipdef);
+ else if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET6))
+ return networkAddRoutingIptablesRules(driver, network, ipdef);
+ } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) {
+ return networkAddRoutingIptablesRules(driver, network, ipdef);
+ }
return 0;
}
@@ -1000,10 +1072,14 @@ networkRemoveIpSpecificIptablesRules(struct network_driver
*driver,
virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
{
- if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT)
- networkRemoveMasqueradingIptablesRules(driver, network, ipdef);
- else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)
+ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET))
+ networkRemoveMasqueradingIptablesRules(driver, network, ipdef);
+ else if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET6))
+ networkRemoveRoutingIptablesRules(driver, network, ipdef);
+ } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) {
networkRemoveRoutingIptablesRules(driver, network, ipdef);
+ }
}
/* Add all rules for all ip addresses (and general rules) on a network */
@@ -1085,30 +1161,46 @@ networkEnableIpForwarding(void)
#define SYSCTL_PATH "/proc/sys"
-static int networkDisableIPV6(virNetworkObjPtr network)
+static int
+networkSetIPv6Sysctls(virNetworkObjPtr network)
{
char *field = NULL;
int ret = -1;
- if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/disable_ipv6",
network->def->bridge) < 0) {
- virReportOOMError();
- goto cleanup;
- }
+ if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0)) {
+ /* Only set disable_ipv6 if there are no ipv6 addresses defined for
+ * the network.
+ */
+ if (virAsprintf(&field, SYSCTL_PATH
"/net/ipv6/conf/%s/disable_ipv6",
+ network->def->bridge) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
- if (access(field, W_OK) < 0 && errno == ENOENT) {
- VIR_DEBUG("ipv6 appears to already be disabled on %s",
network->def->bridge);
- ret = 0;
- goto cleanup;
- }
+ if (access(field, W_OK) < 0 && errno == ENOENT) {
+ VIR_DEBUG("ipv6 appears to already be disabled on %s",
+ network->def->bridge);
+ ret = 0;
+ goto cleanup;
+ }
- if (virFileWriteStr(field, "1", 0) < 0) {
- virReportSystemError(errno,
- _("cannot enable %s"), field);
- goto cleanup;
+ if (virFileWriteStr(field, "1", 0) < 0) {
+ virReportSystemError(errno,
+ _("cannot enable %s"), field);
+ goto cleanup;
+ }
+ VIR_FREE(field);
}
- VIR_FREE(field);
- if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra",
network->def->bridge) < 0) {
+ /* The rest of the ipv6 sysctl tunables should always be set,
+ * whether or not we're using ipv6 on this bridge.
+ */
+
+ /* Prevent guests from hijacking the host network by sending out
+ * their own router advertisements.
+ */
+ if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra",
+ network->def->bridge) < 0) {
virReportOOMError();
goto cleanup;
}
@@ -1120,7 +1212,11 @@ static int networkDisableIPV6(virNetworkObjPtr network)
}
VIR_FREE(field);
- if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/autoconf",
network->def->bridge) < 0) {
+ /* All interfaces used as a gateway (which is what this is, by
+ * definition), must always have autoconf=0.
+ */
+ if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/autoconf",
+ network->def->bridge) < 0) {
virReportOOMError();
goto cleanup;
}
@@ -1262,7 +1358,7 @@ static int
networkStartNetworkDaemon(struct network_driver *driver,
virNetworkObjPtr network)
{
- int ii, err, v4present = 0;
+ int ii, err, v4present = 0, v6present = 0;
virErrorPtr save_err = NULL;
virNetworkIpDefPtr ipdef;
@@ -1301,8 +1397,10 @@ networkStartNetworkDaemon(struct network_driver *driver,
goto err1;
}
- /* Disable IPv6 on the bridge */
- if (networkDisableIPV6(network) < 0)
+ /* Disable IPv6 on the bridge if there are no IPv6 addresses
+ * defined, and set other IPv6 sysctl tunables appropriately.
+ */
+ if (networkSetIPv6Sysctls(network) < 0)
goto err1;
/* Add "once per network" rules */
@@ -1314,6 +1412,8 @@ networkStartNetworkDaemon(struct network_driver *driver,
ii++) {
if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET))
v4present = 1;
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET6))
+ v6present = 1;
/* Add the IP address/netmask to the bridge */
if (networkAddAddrToBridge(driver, network, ipdef) < 0) {
@@ -1708,9 +1808,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
goto cleanup;
}
- /* we only support dhcp on one IPv4 address per defined network, and currently
- * don't support IPv6.
- */
+ /* We only support dhcp on one IPv4 address per defined network */
for (ii = 0;
(ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
ii++) {
@@ -1724,14 +1822,6 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
ipv4def = ipdef;
}
}
- } else {
- /* we currently only support IPv4 */
- networkReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported address family '%s' (%d) in
network definition"),
- ipdef->family ? ipdef->family :
"unspecified",
- VIR_SOCKET_FAMILY(&ipdef->address));
- goto cleanup;
-
}
}
if (ipv4def) {
@@ -1756,7 +1846,8 @@ cleanup:
static int networkUndefine(virNetworkPtr net) {
struct network_driver *driver = net->conn->networkPrivateData;
virNetworkObjPtr network;
- virNetworkIpDefPtr ipv4def;
+ virNetworkIpDefPtr ipdef;
+ int v4present = 0, v6present = 0;
int ret = -1, ii;
networkDriverLock(driver);
@@ -1781,12 +1872,17 @@ static int networkUndefine(virNetworkPtr net) {
/* we only support dhcp on one IPv4 address per defined network */
for (ii = 0;
- (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
ii++) {
- if (ipv4def->nranges || ipv4def->nhosts)
- break;
+ if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
+ if (ipdef->nranges || ipdef->nhosts)
+ v4present = 1;
+ } else if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET6)) {
+ v6present = 1;
+ }
}
- if (ipv4def) {
+
+ if (v4present) {
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
if (dctx == NULL)
goto cleanup;
--
1.7.3.4
2:03 a.m.
New subject: [libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses
Running an instance of the router advertisement daemon (radvd) allows
guests using the virtual network to automatically acquire and IPv6
address and default route. Note that acquiring an address only works
for networks with a prefix length of exactly 64 - radvd is still run
in other circumstances, and still advertises routes, but autoconf will
not work because it requires exactly 64 bits of address info from the
network prefix.
This patch should not be committed as-is, because there is a race
condition with the pidfile that I still need to remedy. Unlike
dnsmasq, radvd does not guarantee that its pidfile has been written
before its original process exits; instead it uses daemon(3) to fork
and exit the parent process in a single step, then writes the pidfile
later. The result is that libvirtd tries to read the pidfile before
it's been created, and ends up not having the proper pid to use when
it later tries to kill radvd.
There are two possible solutions for this:
1) Don't attempt to immediately read the pidfile and store the pid in
memory. Instead, just read the pidfile later when we want to kill
radvd. (This could still lead to a race if networkStart and
networkDestroy were called in tight sequence by some application)
2) Don't allow radvd to daemonize itself. Instead, run it with "-d 1"
(setting a debuglevel > 0 implies that it should not daemonize),
and use virCommand's own pidfile/daemonize functions. (The problem
here is that there's no way to tell radvd to not write a pidfile
itself, so we would end up with 2 pidfiles for each instance. This
isn't a functional problem, just cosmetic).
Another unresolved(?) problem is that the location of the radvd binary
is found by autoconf during the build, so if it's not present on the
build machine. Should this be handled by specfiles on specific distros
adding a BuildRequires for the radvd package, or can/should more be
done in the libvirt tree? (Current behavior is identical to dnsmasq,
so I guess it's acceptable, as long as all the downstream builders are
made aware of the need for radvd for proper IPv6 support).
---
configure.ac | 4 +
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 232 +++++++++++++++++++++++++++++++++++++++----
3 files changed, 216 insertions(+), 21 deletions(-)
diff --git a/configure.ac b/configure.ac
index 279ccd2..231d0c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -135,6 +135,8 @@ dnl detect them, in which case we'll search for the program
dnl along the $PATH at runtime and fail if it's not there.
AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([RADVD], [radvd], [radvd],
+ [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([BRCTL], [brctl], [brctl],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([UDEVADM], [udevadm], [],
@@ -146,6 +148,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [],
AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"],
[Location or name of the dnsmasq program])
+AC_DEFINE_UNQUOTED([RADVD],["$RADVD"],
+ [Location or name of the radvd program])
AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"],
[Location or name of the brctl program (see bridge-utils)])
if test -n "$UDEVADM"; then
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 47f0408..ca5784e 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -107,6 +107,7 @@ struct _virNetworkObj {
virMutex lock;
pid_t dnsmasqPid;
+ pid_t radvdPid;
unsigned int active : 1;
unsigned int autostart : 1;
unsigned int persistent : 1;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 665a13f..37f0eed 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -65,6 +65,7 @@
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
#define DNSMASQ_STATE_DIR LOCALSTATEDIR "/lib/libvirt/dnsmasq"
+#define RADVD_STATE_DIR LOCALSTATEDIR "/lib/libvirt/radvd"
#define VIR_FROM_THIS VIR_FROM_NETWORK
@@ -107,6 +108,25 @@ static void networkReloadIptablesRules(struct network_driver
*driver);
static struct network_driver *driverState = NULL;
+static char *
+networkRadvdPidfileBasename(const char *netname)
+{
+ /* this is simple but we want to be sure it's consistently done */
+ char *pidfilebase;
+
+ virAsprintf(&pidfilebase, "%s-radvd", netname);
+ return pidfilebase;
+}
+
+static char *
+networkRadvdConfigFileName(const char *netname)
+{
+ char *configfile;
+
+ virAsprintf(&configfile, "%s/%s-radvd.conf",
+ RADVD_STATE_DIR, netname);
+ return configfile;
+}
static void
networkFindActiveConfigs(struct network_driver *driver) {
@@ -144,26 +164,43 @@ networkFindActiveConfigs(struct network_driver *driver) {
brHasBridge(driver->brctl, obj->def->bridge) == 0) {
obj->active = 1;
- /* Finally try and read dnsmasq pid if any */
- if (obj->def->ips && (obj->def->nips > 0) &&
- virFileReadPid(NETWORK_PID_DIR, obj->def->name,
- &obj->dnsmasqPid) == 0) {
-
- /* Check its still alive */
- if (kill(obj->dnsmasqPid, 0) != 0)
- obj->dnsmasqPid = -1;
-
-#ifdef __linux__
- char *pidpath;
+ /* Try and read dnsmasq/radvd pids if any */
+ if (obj->def->ips && (obj->def->nips > 0)) {
+ char *pidpath, *radvdpidbase;
+
+ if (virFileReadPid(NETWORK_PID_DIR, obj->def->name,
+ &obj->dnsmasqPid) == 0) {
+ /* Check that it's still alive */
+ if (kill(obj->dnsmasqPid, 0) != 0)
+ obj->dnsmasqPid = -1;
+ if (virAsprintf(&pidpath, "/proc/%d/exe",
obj->dnsmasqPid) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0)
+ obj->dnsmasqPid = -1;
+ VIR_FREE(pidpath);
+ }
- if (virAsprintf(&pidpath, "/proc/%d/exe",
obj->dnsmasqPid) < 0) {
+ if (!(radvdpidbase = networkRadvdPidfileBasename(obj->def->name)))
{
virReportOOMError();
goto cleanup;
}
- if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0)
- obj->dnsmasqPid = -1;
- VIR_FREE(pidpath);
-#endif
+ if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase,
+ &obj->radvdPid) == 0) {
+ /* Check that it's still alive */
+ if (kill(obj->radvdPid, 0) != 0)
+ obj->radvdPid = -1;
+ if (virAsprintf(&pidpath, "/proc/%d/exe",
obj->radvdPid) < 0) {
+ virReportOOMError();
+ VIR_FREE(radvdpidbase);
+ goto cleanup;
+ }
+ if (virFileLinkPointsTo(pidpath, RADVD) == 0)
+ obj->radvdPid = -1;
+ VIR_FREE(pidpath);
+ }
+ VIR_FREE(radvdpidbase);
}
}
@@ -588,6 +625,123 @@ cleanup:
}
static int
+networkStartRadvd(virNetworkObjPtr network)
+{
+ char *pidfile = NULL;
+ char *radvdpidbase = NULL;
+ virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
+ char *configstr = NULL;
+ char *configfile = NULL;
+ virCommandPtr cmd = NULL;
+ int ret = -1, err, ii;
+ virNetworkIpDefPtr ipdef;
+
+ network->radvdPid = -1;
+
+ if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) {
+ virReportSystemError(err,
+ _("cannot create directory %s"),
+ NETWORK_PID_DIR);
+ goto cleanup;
+ }
+ if ((err = virFileMakePath(RADVD_STATE_DIR)) != 0) {
+ virReportSystemError(err,
+ _("cannot create directory %s"),
+ RADVD_STATE_DIR);
+ goto cleanup;
+ }
+
+ /* construct pidfile name */
+ if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (!(pidfile = virFilePid(NETWORK_PID_DIR, radvdpidbase))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ /* create radvd config file appropriate for this network */
+ virBufferVSprintf(&configbuf, "interface %s\n"
+ "{\n"
+ " AdvSendAdvert on;\n"
+ " AdvManagedFlag off;\n"
+ " AdvOtherConfigFlag off;\n"
+ "\n",
+ network->def->bridge);
+ for (ii = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii));
+ ii++) {
+ int prefix;
+ char *netaddr;
+
+ prefix = virNetworkIpDefPrefix(ipdef);
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid
prefix"),
+ network->def->bridge);
+ goto cleanup;
+ }
+ if (!(netaddr = virSocketFormatAddr(&ipdef->address)))
+ goto cleanup;
+ virBufferVSprintf(&configbuf,
+ " prefix %s/%d\n"
+ " {\n"
+ " AdvOnLink on;\n"
+ " AdvAutonomous on;\n"
+ " AdvRouterAddr off;\n"
+ " };\n",
+ netaddr, prefix);
+ VIR_FREE(netaddr);
+ }
+
+ virBufferAddLit(&configbuf, "};\n");
+
+ if (virBufferError(&configbuf)) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (!(configstr = virBufferContentAndReset(&configbuf))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ /* construct the filename */
+ if (!(configfile = networkRadvdConfigFileName(network->def->name))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ /* write the file */
+ if (virFileWriteStr(configfile, configstr, 0600) < 0) {
+ virReportSystemError(errno,
+ _("couldn't write radvd config file
'%s'"),
+ configfile);
+ goto cleanup;
+ }
+
+ cmd = virCommandNew(RADVD);
+ virCommandAddArgList(cmd, "--config", configfile,
+ "--pidfile", pidfile,
+ NULL);
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase,
+ &network->radvdPid) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ VIR_FREE(configfile);
+ VIR_FREE(configstr);
+ virBufferFreeAndReset(&configbuf);
+ VIR_FREE(radvdpidbase);
+ VIR_FREE(pidfile);
+ return ret;
+}
+
+static int
networkAddMasqueradingIptablesRules(struct network_driver *driver,
virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
@@ -1154,9 +1308,14 @@ networkReloadIptablesRules(struct network_driver *driver)
/* Enable IP Forwarding. Return 0 for success, -1 for failure. */
static int
-networkEnableIpForwarding(void)
+networkEnableIpForwarding(int enableIPv4, int enableIPv6)
{
- return virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n",
0);
+ int ret = 0;
+ if (enableIPv4)
+ ret = virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n",
0);
+ if (enableIPv6 && ret == 0)
+ ret = virFileWriteStr("/proc/sys/net/ipv6/conf/all/forwarding",
"1\n", 0);
+ return ret;
}
#define SYSCTL_PATH "/proc/sys"
@@ -1431,7 +1590,7 @@ networkStartNetworkDaemon(struct network_driver *driver,
/* If forwardType != NONE, turn on global IP forwarding */
if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
- networkEnableIpForwarding() < 0) {
+ networkEnableIpForwarding(v4present, v6present) < 0) {
virReportSystemError(errno, "%s",
_("failed to enable IP forwarding"));
goto err3;
@@ -1442,15 +1601,28 @@ networkStartNetworkDaemon(struct network_driver *driver,
if (v4present && networkStartDhcpDaemon(network) < 0)
goto err3;
+ /* start radvd if there are any ipv6 addresses */
+ if (v6present && networkStartRadvd(network) < 0)
+ goto err4;
+
/* Persist the live configuration now we have bridge info */
if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) {
- goto err4;
+ goto err5;
}
network->active = 1;
return 0;
+ err5:
+ if (!save_err)
+ save_err = virSaveLastError();
+
+ if (network->radvdPid > 0) {
+ kill(network->radvdPid, SIGTERM);
+ network->radvdPid = -1;
+ }
+
err4:
if (!save_err)
save_err = virSaveLastError();
@@ -1509,6 +1681,9 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
unlink(stateFile);
VIR_FREE(stateFile);
+ if (network->radvdPid > 0)
+ kill(network->radvdPid, SIGTERM);
+
if (network->dnsmasqPid > 0)
kill(network->dnsmasqPid, SIGTERM);
@@ -1529,8 +1704,13 @@ static int networkShutdownNetworkDaemon(struct network_driver
*driver,
if (network->dnsmasqPid > 0 &&
(kill(network->dnsmasqPid, 0) == 0))
kill(network->dnsmasqPid, SIGKILL);
-
network->dnsmasqPid = -1;
+
+ if (network->radvdPid > 0 &&
+ (kill(network->radvdPid, 0) == 0))
+ kill(network->radvdPid, SIGKILL);
+ network->radvdPid = -1;
+
network->active = 0;
if (network->newDef) {
@@ -1891,6 +2071,16 @@ static int networkUndefine(virNetworkPtr net) {
dnsmasqContextFree(dctx);
}
+ if (v6present) {
+ char *configfile = networkRadvdConfigFileName(network->def->name);
+
+ if (!configfile) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ unlink(configfile);
+ }
+
virNetworkRemoveInactive(&driver->networks,
network);
network = NULL;
--
1.7.3.4
4:46 p.m.
New subject: [libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses
On Mon, Dec 20, 2010 at 09:03, Laine Stump <laine(a)laine.org> wrote:
There are two possible solutions for this:
1) Don't attempt to immediately read the pidfile and store the pid in
memory. Instead, just read the pidfile later when we want to kill
radvd. (This could still lead to a race if networkStart and
networkDestroy were called in tight sequence by some application)
In such tight sequence you can detect that radvd has been just
started, because radvd truncates pidfile before daemon(). In such
situation you can wait for pid awhile (e.g. separate thread could use
inotify for this)
Some general questions:
- radvd is optional for ipv6 network. How one can configure to not to
spawn radvd daemon
- did you thought about running only one instance of radvd and sending
SIGHUP to daemon after adding/removing interfaces to conffile
--
Pawel
12:01 p.m.
New subject: [libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses
On 12/20/2010 05:46 PM, Paweł Krześniak wrote:
On Mon, Dec 20, 2010 at 09:03, Laine Stump<laine(a)laine.org>
wrote:
> There are two possible solutions for this:
>
> 1) Don't attempt to immediately read the pidfile and store the pid in
> memory. Instead, just read the pidfile later when we want to kill
> radvd. (This could still lead to a race if networkStart and
> networkDestroy were called in tight sequence by some application)
In such tight sequence you can detect that radvd has been just
started, because radvd truncates pidfile before daemon(). In such
situation you can wait for pid awhile (e.g. separate thread could use
inotify for this)
It works out reasonably well to manually daemonize radvd, and create the
pidfile ourselves, so that's the way I'm doing it.
Some general questions:
- radvd is optional for ipv6 network. How one can configure to not to
spawn radvd daemon
We decided back when we were discussing this feature that we wanted
radvd to always be run. The logic is that we want the virtual networks
to cover the needs of 95% of installations, and be as simple as possible
for all of those. The other 5% will need to setup their bridge manually.
(Keep in mind that even when radvd is running, it's totally acceptable
for a guest to ignore it and manually configure their interfaces, so
even most people who don't want/need radvd can live with its presence).
Here's the original discussion thread:
https://www.redhat.com/archives/libvir-list/2010-November/msg00146.html
- did you thought about running only one instance of radvd and
sending
SIGHUP to daemon after adding/removing interfaces to conffile
Yes, we talked about that as well, and decided that we wanted libvirt's
use of radvd to be as segregated from the rest of the system's use as
possible - less potential for a problem in one place to leak over into
another, and no headaches trying to cleanly/atomically edit the config file.
5:22 p.m.
New subject: [libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses
On 12/20/2010 01:03 AM, Laine Stump wrote:
1) Don't attempt to immediately read the pidfile and store the pid in
memory. Instead, just read the pidfile later when we want to kill
radvd. (This could still lead to a race if networkStart and
networkDestroy were called in tight sequence by some application)
Still racy unless you go with the extra complexity of using inotify() to
determine when the radvd process has completed writing to the pidfile.
Seems rather complex, but since we're already assuming Linux, it doesn't
seem to hard to assumen inotify().
2) Don't allow radvd to daemonize itself. Instead, run it with "-d 1"
(setting a debuglevel > 0 implies that it should not daemonize),
and use virCommand's own pidfile/daemonize functions. (The problem
here is that there's no way to tell radvd to not write a pidfile
itself, so we would end up with 2 pidfiles for each instance. This
isn't a functional problem, just cosmetic).
Looks like you can use radvd -p /dev/null to make radvd avoid the second
pidfile, and just go with virCommand's pidfile. How much extra output
does radvd -d 1 cause in comparison to radvd -d 0?
I'm leaning towards option 2; it's better to avoid the race.
Another unresolved(?) problem is that the location of the radvd binary
is found by autoconf during the build, so if it's not present on the
build machine. Should this be handled by specfiles on specific distros
adding a BuildRequires for the radvd package, or can/should more be
done in the libvirt tree? (Current behavior is identical to dnsmasq,
so I guess it's acceptable, as long as all the downstream builders are
made aware of the need for radvd for proper IPv6 support).
specfiles sound like the way to go to me.
+static char *
+networkRadvdConfigFileName(const char *netname)
+{
+ char *configfile;
+
+ virAsprintf(&configfile, "%s/%s-radvd.conf",
+ RADVD_STATE_DIR, netname);
It's slightly more efficient to do
virAsprintf(&configfile, RADVD_STATE_DIR "/%s-radvd.conf",
netname);
but it doesn't hurt my feelings to leave it as is.
+ /* Try and read dnsmasq/radvd pids if any */
+ if (obj->def->ips && (obj->def->nips > 0)) {
+ char *pidpath, *radvdpidbase;
+
+ if (virFileReadPid(NETWORK_PID_DIR, obj->def->name,
+ &obj->dnsmasqPid) == 0) {
Thinking out loud here - can virFileReadPid be taught how to inspect
whether other processes still have the pidfile open for writing, and
block until those other files have closed?
Another thought - abuse fcntl(F_SETLK), to pass the radvd process an
open and locked fd pointing to the same pid file that the radvd process
will separately open. When the radvd process then close()s the pidfile,
that will nuke the fcntl lock, and the parent process can use that to
tell when things are done. Unfortunately, reading the man page further,
this doesn't sound too good (that is, inotify() is more robust), because
fcntl locks are not preserved across fork(), but radvd calls daemon()
which calls fork(), so the lock would be lost before the daemon process
starts.
+
+ cmd = virCommandNew(RADVD);
+ virCommandAddArgList(cmd, "--config", configfile,
+ "--pidfile", pidfile,
+ NULL);
You could combine these:
cmd = virCommandNewArgList(RADVD, "--config", configfile,
"--pidfile", pidfile, NULL);
+ if (v6present) {
+ char *configfile = networkRadvdConfigFileName(network->def->name);
+
+ if (!configfile) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ unlink(configfile);
+ }
Where do you VIR_FREE(configfile)?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:54 a.m.
New subject: [libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses
On 12/20/2010 06:22 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
> 1) Don't attempt to immediately read the pidfile and store the pid in
> memory. Instead, just read the pidfile later when we want to kill
> radvd. (This could still lead to a race if networkStart and
> networkDestroy were called in tight sequence by some application)
Still racy unless you go with the extra complexity of using inotify() to
determine when the radvd process has completed writing to the pidfile.
Seems rather complex, but since we're already assuming Linux, it doesn't
seem to hard to assumen inotify().
> 2) Don't allow radvd to daemonize itself. Instead, run it with "-d 1"
> (setting a debuglevel> 0 implies that it should not daemonize),
> and use virCommand's own pidfile/daemonize functions. (The problem
> here is that there's no way to tell radvd to not write a pidfile
> itself, so we would end up with 2 pidfiles for each instance. This
> isn't a functional problem, just cosmetic).
Looks like you can use radvd -p /dev/null to make radvd avoid the second
pidfile, and just go with virCommand's pidfile. How much extra output
does radvd -d 1 cause in comparison to radvd -d 0?
Very little.
I'm leaning towards option 2; it's better to avoid the race.
And it works! I've folded that into the v2 patch.
> +static char *
> +networkRadvdConfigFileName(const char *netname)
> +{
> + char *configfile;
> +
> + virAsprintf(&configfile, "%s/%s-radvd.conf",
> + RADVD_STATE_DIR, netname);
It's slightly more efficient to do
virAsprintf(&configfile, RADVD_STATE_DIR "/%s-radvd.conf",
netname);
but it doesn't hurt my feelings to leave it as is.
Changed anyway.
> +
> + cmd = virCommandNew(RADVD);
> + virCommandAddArgList(cmd, "--config", configfile,
> + "--pidfile", pidfile,
> + NULL);
You could combine these:
cmd = virCommandNewArgList(RADVD, "--config", configfile,
"--pidfile", pidfile, NULL);
Nice! I didn't see that function.
>
> + if (v6present) {
> + char *configfile = networkRadvdConfigFileName(network->def->name);
> +
> + if (!configfile) {
> + virReportOOMError();
> + goto cleanup;
> + }
> + unlink(configfile);
> + }
Where do you VIR_FREE(configfile)?
(eep) Er, I didn't :-/ Now I do. Thanks for catching that!
5070
days inactive
5085
days old
35 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump -
Paweł Krześniak