[libvirt] [PATCH 1/3] Virtual network cleanup/bugfixes
The first two patches remedy a bug found by Stefan Berger in the new brAddInetInterface() function that resulted in the broadcast address not being properly set. This is a regression from previous releases, so it should be pushed prior to the upcoming release. The third patch improves the API for a couple of functions that were recently added, but does not change any behavior. I'd be happy pushing it now or after the release.
These functions work only for IPv4, becasue IPv6 doesn't have the same concept of "broadcast address" as IPv4. They merely OR the inverse of the netmask with the given host address, thus turning on all the host bits. --- src/libvirt_private.syms | 2 + src/util/network.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 6 +++++ 3 files changed, 63 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a959ad9..19e581c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -558,6 +558,8 @@ virShrinkN; # network.h +virSocketAddrBroadcast; +virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/network.c b/src/util/network.c index 61044fc..f58986e 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -341,6 +341,61 @@ virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) } /** + * virSocketAddrBroadcast: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @netmask: the netmask address + * @broadcast: virSocketAddr to recieve the broadcast address + * + * Mask ON the host bits of @addr according to @netmask, turning it + * into a broadcast address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast) +{ + if ((addr->data.stor.ss_family != AF_INET) || + (netmask->data.stor.ss_family != AF_INET)) { + broadcast->data.stor.ss_family = AF_UNSPEC; + return -1; + } + + broadcast->data.stor.ss_family = AF_INET; + broadcast->len = addr->len; + broadcast->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr + | ~netmask->data.inet4.sin_addr.s_addr); + return 0; +} + +/** + * virSocketAddrBroadcastByPrefix: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @prefix: prefix (# of 1 bits) of netmask to apply + * @broadcast: virSocketAddr to recieve the broadcast address + * + * 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 +virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix, &netmask, + addr->data.stor.ss_family) < 0) + return -1; + + return virSocketAddrBroadcast(addr, &netmask, broadcast); +} + +/** * virSocketCheckNetmask: * @addr1: a first network address * @addr2: a second network address diff --git a/src/util/network.h b/src/util/network.h index 2fcee43..bcbc607 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -77,6 +77,12 @@ int virSocketAddrMask (virSocketAddrPtr addr, const virSocketAddrPtr netmask); int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix); +int virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast); +int virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast); int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask); int virSocketAddrPrefixToNetmask(unsigned int prefix, -- 1.7.3.4
On 12/31/2010 06:33 AM, Laine Stump wrote:
These functions work only for IPv4, becasue IPv6 doesn't have the same concept of "broadcast address" as IPv4. They merely OR the inverse of the netmask with the given host address, thus turning on all the host bits. --- src/libvirt_private.syms | 2 + src/util/network.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 6 +++++ 3 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a959ad9..19e581c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -558,6 +558,8 @@ virShrinkN;
# network.h +virSocketAddrBroadcast; +virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/network.c b/src/util/network.c index 61044fc..f58986e 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -341,6 +341,61 @@ virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) }
/** + * virSocketAddrBroadcast: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @netmask: the netmask address + * @broadcast: virSocketAddr to recieve the broadcast address + * + * Mask ON the host bits of @addr according to @netmask, turning it + * into a broadcast address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast) +{ + if ((addr->data.stor.ss_family != AF_INET) || + (netmask->data.stor.ss_family != AF_INET)) { + broadcast->data.stor.ss_family = AF_UNSPEC; + return -1; + } + + broadcast->data.stor.ss_family = AF_INET; + broadcast->len = addr->len; + broadcast->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr + | ~netmask->data.inet4.sin_addr.s_addr); + return 0; +} + +/** + * virSocketAddrBroadcastByPrefix: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @prefix: prefix (# of 1 bits) of netmask to apply + * @broadcast: virSocketAddr to recieve the broadcast address + * + * 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 +virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix,&netmask, + addr->data.stor.ss_family)< 0) + return -1; + + return virSocketAddrBroadcast(addr,&netmask, broadcast); +} + +/** * virSocketCheckNetmask: * @addr1: a first network address * @addr2: a second network address diff --git a/src/util/network.h b/src/util/network.h index 2fcee43..bcbc607 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -77,6 +77,12 @@ int virSocketAddrMask (virSocketAddrPtr addr, const virSocketAddrPtr netmask); int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix); +int virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast); +int virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast);
int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask); int virSocketAddrPrefixToNetmask(unsigned int prefix, ACK.
Stefan
On 12/31/2010 08:08 AM, Stefan Berger wrote:
On 12/31/2010 06:33 AM, Laine Stump wrote:
These functions work only for IPv4, becasue IPv6 doesn't have the same concept of "broadcast address" as IPv4. They merely OR the inverse of the netmask with the given host address, thus turning on all the host bits. --- src/libvirt_private.syms | 2 + src/util/network.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 6 +++++ 3 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a959ad9..19e581c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -558,6 +558,8 @@ virShrinkN;
# network.h +virSocketAddrBroadcast; +virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/network.c b/src/util/network.c index 61044fc..f58986e 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -341,6 +341,61 @@ virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) }
/** + * virSocketAddrBroadcast: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @netmask: the netmask address + * @broadcast: virSocketAddr to recieve the broadcast address + * + * Mask ON the host bits of @addr according to @netmask, turning it + * into a broadcast address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast) +{ + if ((addr->data.stor.ss_family != AF_INET) || + (netmask->data.stor.ss_family != AF_INET)) { + broadcast->data.stor.ss_family = AF_UNSPEC; + return -1; + } + + broadcast->data.stor.ss_family = AF_INET; + broadcast->len = addr->len; + broadcast->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr + | ~netmask->data.inet4.sin_addr.s_addr); + return 0; +} + +/** + * virSocketAddrBroadcastByPrefix: + * @addr: address that needs to be turned into broadcast address (IPv4 only) + * @prefix: prefix (# of 1 bits) of netmask to apply + * @broadcast: virSocketAddr to recieve the broadcast address + * + * 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 +virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix,&netmask, + addr->data.stor.ss_family)< 0) + return -1; + + return virSocketAddrBroadcast(addr,&netmask, broadcast); +} + +/** * virSocketCheckNetmask: * @addr1: a first network address * @addr2: a second network address diff --git a/src/util/network.h b/src/util/network.h index 2fcee43..bcbc607 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -77,6 +77,12 @@ int virSocketAddrMask (virSocketAddrPtr addr, const virSocketAddrPtr netmask); int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix); +int virSocketAddrBroadcast(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr broadcast); +int virSocketAddrBroadcastByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr broadcast);
int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask); int virSocketAddrPrefixToNetmask(unsigned int prefix, ACK.
Thanks, Stefan. Pushed.
Previously we used ioctl() to set the IP address and netmask of the bridges used for virtual networks, and apparently the SIOCSIFNETMASK ioctl implicitly set the broadcast address for the interface. The new method of using the "ip" command requires broadcast address to be explicitly specified though. --- src/util/bridge.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/util/bridge.c b/src/util/bridge.c index 81a043c..e53fce5 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -677,14 +677,23 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, unsigned int prefix) { virCommandPtr cmd = NULL; - char *addrstr; + char *addrstr = NULL, *bcaststr = NULL; + virSocketAddr broadcast; int ret = -1; if (!(addrstr = virSocketFormatAddr(addr))) goto cleanup; + /* format up a broadcast address if this is IPv4 */ + if ((VIR_SOCKET_IS_FAMILY(addr, AF_INET)) && + ((virSocketAddrBroadcastByPrefix(addr, prefix, &broadcast) < 0) || + !(bcaststr = virSocketFormatAddr(&broadcast)))) { + goto cleanup; + } cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "addr", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); + if (bcaststr) + virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArgList(cmd, "dev", ifname, NULL); if (virCommandRun(cmd, NULL) < 0) @@ -693,6 +702,7 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, ret = 0; cleanup: VIR_FREE(addrstr); + VIR_FREE(bcaststr); virCommandFree(cmd); return ret; } -- 1.7.3.4
On 12/31/2010 06:33 AM, Laine Stump wrote:
Previously we used ioctl() to set the IP address and netmask of the bridges used for virtual networks, and apparently the SIOCSIFNETMASK ioctl implicitly set the broadcast address for the interface. The new method of using the "ip" command requires broadcast address to be explicitly specified though. --- src/util/bridge.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/util/bridge.c b/src/util/bridge.c index 81a043c..e53fce5 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -677,14 +677,23 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, unsigned int prefix) { virCommandPtr cmd = NULL; - char *addrstr; + char *addrstr = NULL, *bcaststr = NULL; + virSocketAddr broadcast; int ret = -1;
if (!(addrstr = virSocketFormatAddr(addr))) goto cleanup; + /* format up a broadcast address if this is IPv4 */ + if ((VIR_SOCKET_IS_FAMILY(addr, AF_INET))&& + ((virSocketAddrBroadcastByPrefix(addr, prefix,&broadcast)< 0) || + !(bcaststr = virSocketFormatAddr(&broadcast)))) { + goto cleanup; + } cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "addr", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); + if (bcaststr) + virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArgList(cmd, "dev", ifname, NULL);
if (virCommandRun(cmd, NULL)< 0) @@ -693,6 +702,7 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, ret = 0; cleanup: VIR_FREE(addrstr); + VIR_FREE(bcaststr); virCommandFree(cmd); return ret; }
ACK. Stefan
On 12/31/2010 08:10 AM, Stefan Berger wrote:
On 12/31/2010 06:33 AM, Laine Stump wrote:
Previously we used ioctl() to set the IP address and netmask of the bridges used for virtual networks, and apparently the SIOCSIFNETMASK ioctl implicitly set the broadcast address for the interface. The new method of using the "ip" command requires broadcast address to be explicitly specified though. --- src/util/bridge.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/util/bridge.c b/src/util/bridge.c index 81a043c..e53fce5 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -677,14 +677,23 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, unsigned int prefix) { virCommandPtr cmd = NULL; - char *addrstr; + char *addrstr = NULL, *bcaststr = NULL; + virSocketAddr broadcast; int ret = -1;
if (!(addrstr = virSocketFormatAddr(addr))) goto cleanup; + /* format up a broadcast address if this is IPv4 */ + if ((VIR_SOCKET_IS_FAMILY(addr, AF_INET))&& + ((virSocketAddrBroadcastByPrefix(addr, prefix,&broadcast)< 0) || + !(bcaststr = virSocketFormatAddr(&broadcast)))) { + goto cleanup; + } cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "addr", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); + if (bcaststr) + virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArgList(cmd, "dev", ifname, NULL);
if (virCommandRun(cmd, NULL)< 0) @@ -693,6 +702,7 @@ brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, ret = 0; cleanup: VIR_FREE(addrstr); + VIR_FREE(bcaststr); virCommandFree(cmd); return ret; }
ACK. Stefan
Thanks (for the review and for finding the bug!). Pushed.
The original version of these functions would modify the address sent in, meaning that the caller would usually need to copy the address first. This change makes the original a const, and puts the resulting masked address into a new arg (which could point to the same virSocketAddr as the original, if the caller really wants to modify it). This also makes the API consistent with virSocketAddrBroadcast[ByPrefix]. --- src/util/iptables.c | 3 +-- src/util/network.c | 36 ++++++++++++++++++++++++++---------- src/util/network.h | 10 ++++++---- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/util/iptables.c b/src/util/iptables.c index 647e7ae..6770fe0 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -298,8 +298,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, return NULL; } - network = *netaddr; - if (virSocketAddrMaskByPrefix(&network, prefix) < 0) { + if (virSocketAddrMaskByPrefix(netaddr, prefix, &network) < 0) { iptablesError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failure to mask address")); return NULL; diff --git a/src/util/network.c b/src/util/network.c index f58986e..a7e7423 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -298,23 +298,35 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) +virSocketAddrMask(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network) { - if (addr->data.stor.ss_family != netmask->data.stor.ss_family) + if (addr->data.stor.ss_family != netmask->data.stor.ss_family) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + } if (addr->data.stor.ss_family == AF_INET) { - addr->data.inet4.sin_addr.s_addr - &= netmask->data.inet4.sin_addr.s_addr; + network->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr + & netmask->data.inet4.sin_addr.s_addr); + network->data.stor.ss_family = AF_INET; + network->len = addr->len; return 0; } if (addr->data.stor.ss_family == AF_INET6) { int ii; - for (ii = 0; ii < 16; ii++) - addr->data.inet6.sin6_addr.s6_addr[ii] - &= netmask->data.inet6.sin6_addr.s6_addr[ii]; + for (ii = 0; ii < 16; ii++) { + network->data.inet6.sin6_addr.s6_addr[ii] + = (addr->data.inet6.sin6_addr.s6_addr[ii] + & netmask->data.inet6.sin6_addr.s6_addr[ii]); + } + network->data.stor.ss_family = AF_INET6; + network->len = addr->len; return 0; } + network->data.stor.ss_family = AF_UNSPEC; return -1; } @@ -329,15 +341,19 @@ virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) +virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network) { virSocketAddr netmask; if (virSocketAddrPrefixToNetmask(prefix, &netmask, - addr->data.stor.ss_family) < 0) + addr->data.stor.ss_family) < 0) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + } - return virSocketAddrMask(addr, &netmask); + return virSocketAddrMask(addr, &netmask, network); } /** diff --git a/src/util/network.h b/src/util/network.h index bcbc607..0b43bf6 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -73,10 +73,12 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask); int virSocketCheckNetmask (virSocketAddrPtr addr1, virSocketAddrPtr addr2, virSocketAddrPtr netmask); -int virSocketAddrMask (virSocketAddrPtr addr, - const virSocketAddrPtr netmask); -int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, - unsigned int prefix); +int virSocketAddrMask (const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network); +int virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network); int virSocketAddrBroadcast(const virSocketAddrPtr addr, const virSocketAddrPtr netmask, virSocketAddrPtr broadcast); -- 1.7.3.4
On 12/31/2010 06:33 AM, Laine Stump wrote:
The original version of these functions would modify the address sent in, meaning that the caller would usually need to copy the address first. This change makes the original a const, and puts the resulting masked address into a new arg (which could point to the same virSocketAddr as the original, if the caller really wants to modify it).
This also makes the API consistent with virSocketAddrBroadcast[ByPrefix]. --- src/util/iptables.c | 3 +-- src/util/network.c | 36 ++++++++++++++++++++++++++---------- src/util/network.h | 10 ++++++---- 3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/src/util/iptables.c b/src/util/iptables.c index 647e7ae..6770fe0 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -298,8 +298,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, return NULL; }
- network = *netaddr; - if (virSocketAddrMaskByPrefix(&network, prefix)< 0) { + if (virSocketAddrMaskByPrefix(netaddr, prefix,&network)< 0) { iptablesError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failure to mask address")); return NULL; diff --git a/src/util/network.c b/src/util/network.c index f58986e..a7e7423 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -298,23 +298,35 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) +virSocketAddrMask(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network) { - if (addr->data.stor.ss_family != netmask->data.stor.ss_family) + if (addr->data.stor.ss_family != netmask->data.stor.ss_family) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + }
if (addr->data.stor.ss_family == AF_INET) { - addr->data.inet4.sin_addr.s_addr -&= netmask->data.inet4.sin_addr.s_addr; + network->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr +& netmask->data.inet4.sin_addr.s_addr); + network->data.stor.ss_family = AF_INET; + network->len = addr->len; return 0; } if (addr->data.stor.ss_family == AF_INET6) { int ii; - for (ii = 0; ii< 16; ii++) - addr->data.inet6.sin6_addr.s6_addr[ii] -&= netmask->data.inet6.sin6_addr.s6_addr[ii]; + for (ii = 0; ii< 16; ii++) { + network->data.inet6.sin6_addr.s6_addr[ii] + = (addr->data.inet6.sin6_addr.s6_addr[ii] +& netmask->data.inet6.sin6_addr.s6_addr[ii]); + } + network->data.stor.ss_family = AF_INET6; + network->len = addr->len; return 0; } + network->data.stor.ss_family = AF_UNSPEC; return -1; }
@@ -329,15 +341,19 @@ virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) +virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network) { virSocketAddr netmask;
if (virSocketAddrPrefixToNetmask(prefix,&netmask, - addr->data.stor.ss_family)< 0) + addr->data.stor.ss_family)< 0) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + }
- return virSocketAddrMask(addr,&netmask); + return virSocketAddrMask(addr,&netmask, network); }
/** diff --git a/src/util/network.h b/src/util/network.h index bcbc607..0b43bf6 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -73,10 +73,12 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask); int virSocketCheckNetmask (virSocketAddrPtr addr1, virSocketAddrPtr addr2, virSocketAddrPtr netmask); -int virSocketAddrMask (virSocketAddrPtr addr, - const virSocketAddrPtr netmask); -int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, - unsigned int prefix); +int virSocketAddrMask (const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network); +int virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network); int virSocketAddrBroadcast(const virSocketAddrPtr addr, const virSocketAddrPtr netmask, virSocketAddrPtr broadcast); Looks good to me. ACK. Stefan
On 12/31/2010 08:13 AM, Stefan Berger wrote:
On 12/31/2010 06:33 AM, Laine Stump wrote:
The original version of these functions would modify the address sent in, meaning that the caller would usually need to copy the address first. This change makes the original a const, and puts the resulting masked address into a new arg (which could point to the same virSocketAddr as the original, if the caller really wants to modify it).
This also makes the API consistent with virSocketAddrBroadcast[ByPrefix]. --- src/util/iptables.c | 3 +-- src/util/network.c | 36 ++++++++++++++++++++++++++---------- src/util/network.h | 10 ++++++---- 3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/src/util/iptables.c b/src/util/iptables.c index 647e7ae..6770fe0 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -298,8 +298,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, return NULL; }
- network = *netaddr; - if (virSocketAddrMaskByPrefix(&network, prefix)< 0) { + if (virSocketAddrMaskByPrefix(netaddr, prefix,&network)< 0) { iptablesError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failure to mask address")); return NULL; diff --git a/src/util/network.c b/src/util/network.c index f58986e..a7e7423 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -298,23 +298,35 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) +virSocketAddrMask(const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network) { - if (addr->data.stor.ss_family != netmask->data.stor.ss_family) + if (addr->data.stor.ss_family != netmask->data.stor.ss_family) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + }
if (addr->data.stor.ss_family == AF_INET) { - addr->data.inet4.sin_addr.s_addr -&= netmask->data.inet4.sin_addr.s_addr; + network->data.inet4.sin_addr.s_addr + = (addr->data.inet4.sin_addr.s_addr +& netmask->data.inet4.sin_addr.s_addr); + network->data.stor.ss_family = AF_INET; + network->len = addr->len; return 0; } if (addr->data.stor.ss_family == AF_INET6) { int ii; - for (ii = 0; ii< 16; ii++) - addr->data.inet6.sin6_addr.s6_addr[ii] -&= netmask->data.inet6.sin6_addr.s6_addr[ii]; + for (ii = 0; ii< 16; ii++) { + network->data.inet6.sin6_addr.s6_addr[ii] + = (addr->data.inet6.sin6_addr.s6_addr[ii] +& netmask->data.inet6.sin6_addr.s6_addr[ii]); + } + network->data.stor.ss_family = AF_INET6; + network->len = addr->len; return 0; } + network->data.stor.ss_family = AF_UNSPEC; return -1; }
@@ -329,15 +341,19 @@ virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) * Returns 0 in case of success, or -1 on error. */ int -virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix) +virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network) { virSocketAddr netmask;
if (virSocketAddrPrefixToNetmask(prefix,&netmask, - addr->data.stor.ss_family)< 0) + addr->data.stor.ss_family)< 0) { + network->data.stor.ss_family = AF_UNSPEC; return -1; + }
- return virSocketAddrMask(addr,&netmask); + return virSocketAddrMask(addr,&netmask, network); }
/** diff --git a/src/util/network.h b/src/util/network.h index bcbc607..0b43bf6 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -73,10 +73,12 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask); int virSocketCheckNetmask (virSocketAddrPtr addr1, virSocketAddrPtr addr2, virSocketAddrPtr netmask); -int virSocketAddrMask (virSocketAddrPtr addr, - const virSocketAddrPtr netmask); -int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, - unsigned int prefix); +int virSocketAddrMask (const virSocketAddrPtr addr, + const virSocketAddrPtr netmask, + virSocketAddrPtr network); +int virSocketAddrMaskByPrefix(const virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr network); int virSocketAddrBroadcast(const virSocketAddrPtr addr, const virSocketAddrPtr netmask, virSocketAddrPtr broadcast); Looks good to me. ACK. Stefan
Since this is just a change to new code since the last release, and testing shows no regressions, I've pushed it so it doesn't get lost.
participants (2)
-
Laine Stump -
Stefan Berger