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 --- V2 changes: * Simplify virSocketAddrMaskByPrefix by calling virSocketAddrMask instead of duplicating code. * do bitwise operations arithmetically rather than in loops * prefix is now unsigned when it is a function arg. src/libvirt_private.syms | 3 + src/util/network.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 10 ++++ 3 files changed, 127 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..12b56ea 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -288,6 +288,59 @@ 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 < 16; ii++) + addr->data.inet6.sin6_addr.s6_addr[ii] + &= netmask->data.inet6.sin6_addr.s6_addr[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, unsigned int prefix) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix, &netmask, + addr->data.stor.ss_family) < 0) + return -1; + + return virSocketAddrMask(addr, &netmask); +} + +/** * virSocketCheckNetmask: * @addr1: a first network address * @addr2: a second network address @@ -486,3 +539,64 @@ 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(unsigned int prefix, + virSocketAddrPtr netmask, + int family) +{ + int result = -1; + + netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */ + + if (family == AF_INET) { + int ip; + + if (prefix > 32) + goto error; + + ip = prefix ? ~((1 << (32 - prefix)) - 1) : 0; + 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; + } + if (prefix > 0) { + /* final partial byte */ + netmask->data.inet6.sin6_addr.s6_addr[ii++] + = ~((1 << (8 - prefix)) -1); + } + 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..2fcee43 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, + unsigned int prefix); int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask); +int virSocketAddrPrefixToNetmask(unsigned int prefix, + virSocketAddrPtr netmask, + int family); #endif /* __VIR_NETWORK_H__ */ -- 1.7.3.4

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) --- V2 Changes: * expose the nitty-gritty of detecting class A-C networks, rather than using IN_CLASSx() macros (which aren't guaranteed to be on all platforms) src/conf/network_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/network_conf.h | 3 +++ src/libvirt_private.syms | 2 ++ 3 files changed, 51 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b469269..427b338 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -207,6 +207,52 @@ 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. + * On Linux we could use the IN_CLASSx() macros, but those + * aren't guaranteed on all platforms, so we just deal with + * the bits ourselves. + */ + const unsigned char *octets + = (const unsigned char *)(&def->ipAddress.data.inet4.sin_addr.s_addr); + if ((octets[0] & 0x80) == 0) { + /* Class A network */ + return 8; + } else if ((octets[0] & 0xC0) == 0x80) { + /* Class B network */ + return 16; + } else if ((octets[0] & 0xE0) == 0xC0) { + /* Class C network */ + 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

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. --- No Changes from V1. 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 b0834ae..81a58a9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -585,28 +585,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; } @@ -634,38 +634,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; } @@ -702,28 +702,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; } @@ -743,69 +743,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; } @@ -828,7 +827,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

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. --- No changes from V1. 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 81a58a9..23c39e2 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); } @@ -669,7 +669,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, goto masqerr5; } - return 1; + return 0; masqerr5: iptablesRemoveForwardMasquerade(driver->iptables, @@ -696,7 +696,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, network->def->bridge, network->def->forwardDev); masqerr1: - return 0; + return -1; } static int @@ -727,7 +727,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, goto routeerr2; } - return 1; + return 0; routeerr2: @@ -737,7 +737,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, network->def->bridge, network->def->forwardDev); routeerr1: - return 0; + return -1; } static int @@ -811,11 +811,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 @@ -833,7 +833,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, @@ -857,7 +857,7 @@ networkAddIptablesRules(struct network_driver *driver, err2: iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); err1: - return 0; + return -1; } static void @@ -926,7 +926,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 */ } } @@ -1141,7 +1141,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

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.) --- V2 Changes: * Any prefix arg is now unsigned int rather than int. 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 23c39e2..41c8478 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -585,11 +585,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, @@ -601,7 +609,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, @@ -636,7 +644,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, @@ -648,7 +656,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, @@ -660,7 +668,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, @@ -674,25 +682,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: @@ -702,11 +710,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, @@ -718,7 +734,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, @@ -733,7 +749,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, routeerr2: iptablesRemoveForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); routeerr1: @@ -869,40 +885,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); @@ -1006,17 +1032,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) @@ -1069,7 +1101,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); @@ -1125,9 +1157,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..f7b692c 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) + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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, + unsigned 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..982acf1 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, + unsigned int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowOut (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *iface, const char *physdev); int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *iface, const char *physdev); int iptablesAddForwardAllowIn (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowIn (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *iface, const char *physdev); @@ -93,12 +93,12 @@ int iptablesRemoveForwardRejectIn (iptablesContext *ctx, int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *physdev, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + unsigned int prefix, const char *physdev, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, -- 1.7.3.4

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. --- V2 Changes: * Any prefix arg is now unsigned int rather than int. 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 427b338..b17c708 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -470,19 +470,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; @@ -493,18 +488,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

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? --- V2 Changes: * Any prefix arg is now unsigned int rather than int. 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 73c1176..4db613a 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 41c8478..ba9ff96 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1142,37 +1142,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..dcd3f39 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 IP addresses - 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, + unsigned 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/%u", 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, + unsigned 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/%u", 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..e8e7eca 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, + unsigned int prefix); +int brDelInetAddress (brControl *ctl, const char *ifname, - virSocketAddr *addr); + virSocketAddr *addr, + unsigned int prefix); int brSetForwardDelay (brControl *ctl, const char *bridge, -- 1.7.3.4

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'/> --- No changes from V1. 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

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. --- V2 Changes: * prefix in virNetworkIpDef struct is now unsigned int rather than int. * caught a memory leak in an error patch of virNetworkIPParseXML(). docs/schemas/network.rng | 41 +++- src/conf/network_conf.c | 453 +++++++++++++++++++++---------- 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, 590 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 b17c708..86442ce 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,21 +218,48 @@ 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. * On Linux we could use the IN_CLASSx() macros, but those * aren't guaranteed on all platforms, so we just deal with * the bits ourselves. */ const unsigned char *octets - = (const unsigned char *)(&def->ipAddress.data.inet4.sin_addr.s_addr); + = (const unsigned char *)(&def->address.data.inet4.sin_addr.s_addr); if ((octets[0] & 0x80) == 0) { /* Class A network */ return 8; @@ -233,6 +271,8 @@ int virNetworkDefPrefix(const virNetworkDefPtr def) return 24; } return -1; + } else if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET6)) { + return 64; } return -1; } @@ -241,22 +281,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; @@ -390,33 +431,147 @@ 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 = NULL, *netmask = NULL; + 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='%u' 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); + VIR_FREE(netmask); + + ctxt->node = save; + return result; } static virNetworkDefPtr @@ -424,8 +579,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) { virNetworkDefPtr def; char *tmp; - char *ipAddress; - char *netmask; + xmlNodePtr *ipNodes = NULL; + int nIps; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -469,44 +624,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) { @@ -585,11 +728,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='%u'", 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); @@ -621,81 +854,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..a51794d 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). + */ + unsigned 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 ba9ff96..0f8b050 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) { goto cleanup; } @@ -584,8 +583,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, @@ -596,7 +597,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) { @@ -608,7 +609,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) { @@ -643,7 +644,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) { @@ -655,7 +656,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) { @@ -667,7 +668,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) { @@ -681,25 +682,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); @@ -709,8 +710,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, @@ -721,7 +723,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) { @@ -733,7 +735,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) { @@ -748,7 +750,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, routeerr2: iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); @@ -758,7 +760,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) { @@ -791,7 +794,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'"), @@ -824,29 +827,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; @@ -861,7 +865,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: @@ -878,14 +882,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, @@ -896,34 +902,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); @@ -932,7 +939,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); @@ -951,10 +958,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]); @@ -1028,7 +1043,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; @@ -1036,18 +1052,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 */ @@ -1121,6 +1137,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver, { int err; virErrorPtr save_err; + virNetworkIpDefPtr ipv4def; if (virNetworkObjIsActive(network)) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -1128,8 +1145,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))) { @@ -1158,8 +1178,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, @@ -1169,7 +1189,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); @@ -1184,7 +1204,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 && @@ -1194,12 +1214,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; @@ -1217,7 +1238,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); @@ -1246,6 +1268,7 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver, { int err; char *stateFile; + virNetworkIpDefPtr ipv4def; VIR_INFO(_("Shutting down network '%s'"), network->def->name); @@ -1262,7 +1285,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))) { @@ -1526,6 +1552,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; @@ -1556,12 +1583,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); } @@ -1577,7 +1607,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); @@ -1600,7 +1631,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

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. --- No changes from V1. 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 4db613a..76652f4 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 03afe6c..e0d5aaf 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -842,14 +842,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); @@ -871,14 +873,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); @@ -887,7 +891,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); @@ -896,14 +901,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); @@ -911,7 +918,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); @@ -922,21 +930,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; } @@ -955,20 +963,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 f7b692c..647e7ae 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 982acf1..572d612 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

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. --- No changes from V1. 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 e0d5aaf..d3adc32 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -824,6 +824,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) @@ -926,6 +985,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 */ @@ -956,6 +1020,8 @@ networkRemoveGeneralIptablesRules(struct network_driver *driver, int ii; virNetworkIpDefPtr ipv4def; + networkRemoveGeneralIp6tablesRules(driver, network); + for (ii = 0; (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii)); ii++) { @@ -984,12 +1050,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; } @@ -999,10 +1071,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 */ @@ -1084,30 +1160,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; } @@ -1119,7 +1211,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; } @@ -1261,7 +1357,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; @@ -1300,8 +1396,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 */ @@ -1313,6 +1411,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) { @@ -1707,9 +1807,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++) { @@ -1723,14 +1821,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) { @@ -1755,7 +1845,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); @@ -1780,12 +1871,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