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.

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; }

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