virSocketPrefixToNetmask: Given a 'prefix', which is the number of 1 bits in a netmask, fill in a virSocketAddr object with a netmask as an IP address (IPv6 or IPv4). virSocketAddrMask: Mask off the host bits in one virSocketAddr according to the netmask in another virSocketAddr. virSocketAddrMaskByPrefix, Mask off the host bits in a virSocketAddr according to a prefix (number of 1 bits in netmask). VIR_SOCKET_FAMILY: return the family of a virSocketAddr --- src/libvirt_private.syms | 3 + src/util/network.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 10 ++++ 3 files changed, 143 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0e3033d..19841ef 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -559,6 +559,9 @@ virShrinkN; # network.h virSocketAddrIsNetmask; +virSocketAddrMask; +virSocketAddrMaskByPrefix; +virSocketAddrPrefixToNetmask; virSocketCheckNetmask; virSocketFormatAddr; virSocketFormatAddrFull; diff --git a/src/util/network.c b/src/util/network.c index 1abe78b..e4791b9 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) { } /** + * virSocketAddrMask: + * @addr: address that needs to be masked + * @netmask: the netmask address + * + * Mask off the host bits of @addr according to @netmask, turning it + * into a network address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask) +{ + if (addr->data.stor.ss_family != netmask->data.stor.ss_family) + return -1; + + if (addr->data.stor.ss_family == AF_INET) { + addr->data.inet4.sin_addr.s_addr + &= netmask->data.inet4.sin_addr.s_addr; + return 0; + } + if (addr->data.stor.ss_family == AF_INET6) { + int ii; + for (ii = 0; ii < 4; ii++) + addr->data.inet6.sin6_addr.s6_addr32[ii] + &= netmask->data.inet6.sin6_addr.s6_addr32[ii]; + return 0; + } + return -1; +} + +/** + * virSocketAddrMaskByPrefix: + * @addr: address that needs to be masked + * @prefix: prefix (# of 1 bits) of netmask to apply + * + * Mask off the host bits of @addr according to @prefix, turning it + * into a network address. + * + * Returns 0 in case of success, or -1 on error. + */ +int +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix) +{ + virSocketAddr netmask; + + if (virSocketAddrPrefixToNetmask(prefix, &netmask, + addr->data.stor.ss_family) < 0) + return -1; + + if (addr->data.stor.ss_family == AF_INET) { + + addr->data.inet4.sin_addr.s_addr + &= netmask.data.inet4.sin_addr.s_addr; + return 0; + } + if (addr->data.stor.ss_family == AF_INET6) { + int ii; + + for (ii = 0; ii < 4; ii++) + addr->data.inet6.sin6_addr.s6_addr32[ii] + &= netmask.data.inet6.sin6_addr.s6_addr32[ii]; + return 0; + } + return -1; +} + +/** * virSocketCheckNetmask: * @addr1: a first network address * @addr2: a second network address @@ -486,3 +553,66 @@ int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask) } return -1; } + +/** + * virSocketPrefixToNetmask: + * @prefix: number of 1 bits to put in the netmask + * @netmask: address to fill in with the desired netmask + * @family: family of the address (AF_INET or AF_INET6 only) + * + * given @prefix and @family, fill in @netmask with a netmask + * (eg 255.255.255.0). + * + * Returns 0 on success or -1 on error. + */ + +int virSocketAddrPrefixToNetmask(int prefix, + virSocketAddrPtr netmask, + int family) +{ + int result = -1; + + netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */ + + if (family == AF_INET) { + int ip = 0; + + if (prefix < 0 || prefix > 32) + goto error; + + while (prefix-- > 0) { + ip >>= 1; + ip |= 0x80000000; + } + netmask->data.inet4.sin_addr.s_addr = htonl(ip); + netmask->data.stor.ss_family = AF_INET; + result = 0; + + } else if (family == AF_INET6) { + int ii = 0; + + if (prefix > 128) + goto error; + + while (prefix >= 8) { + /* do as much as possible an entire byte at a time */ + netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0xff; + prefix -= 8; + } + while (prefix-- > 0) { + /* final partial byte one bit at a time */ + netmask->data.inet6.sin6_addr.s6_addr[ii] >>= 1; + netmask->data.inet6.sin6_addr.s6_addr[ii] |= 0x80; + } + ii++; + while (ii < 16) { + /* zerofill remainder in case it wasn't initialized */ + netmask->data.inet6.sin6_addr.s6_addr[ii++] = 0; + } + netmask->data.stor.ss_family = AF_INET6; + result = 0; + } + +error: + return result; +} diff --git a/src/util/network.h b/src/util/network.h index 521f466..6e1394f 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -41,6 +41,9 @@ typedef struct { # define VIR_SOCKET_IS_FAMILY(s, f) \ ((s)->data.sa.sa_family == f) +# define VIR_SOCKET_FAMILY(s) \ + ((s)->data.sa.sa_family) + typedef virSocketAddr *virSocketAddrPtr; int virSocketParseAddr (const char *val, @@ -70,7 +73,14 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask); int virSocketCheckNetmask (virSocketAddrPtr addr1, virSocketAddrPtr addr2, virSocketAddrPtr netmask); +int virSocketAddrMask (virSocketAddrPtr addr, + const virSocketAddrPtr netmask); +int virSocketAddrMaskByPrefix(virSocketAddrPtr addr, + int prefix); int virSocketGetNumNetmaskBits(const virSocketAddrPtr netmask); +int virSocketAddrPrefixToNetmask(int prefix, + virSocketAddrPtr netmask, + int family); #endif /* __VIR_NETWORK_H__ */ -- 1.7.3.4

Some functions in this file were returning 1 on success and 0 on failure, and others were returning 0 on success and -1 on failure. Switch them all to return the libvirt-preferred 0/-1. --- src/network/bridge_driver.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5186511..1ae3924 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -373,7 +373,7 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, unsigned int i; if (! force && virFileExists(dctx->hostsfile->path)) - return 1; + return 0; for (i = 0 ; i < network->def->nhosts ; i++) { virNetworkDHCPHostDefPtr host = &(network->def->hosts[i]); @@ -382,9 +382,9 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, } if (dnsmasqSave(dctx) < 0) - return 0; + return -1; - return 1; + return 0; } @@ -487,7 +487,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, goto cleanup; } - if (networkSaveDnsmasqHostsfile(network, dctx, false)) { + if (networkSaveDnsmasqHostsfile(network, dctx, false) < 0) { virCommandAddArgPair(cmd, "--dhcp-hostsfile", dctx->hostsfile->path); } @@ -670,7 +670,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, goto masqerr5; } - return 1; + return 0; masqerr5: iptablesRemoveForwardMasquerade(driver->iptables, @@ -697,7 +697,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, network->def->bridge, network->def->forwardDev); masqerr1: - return 0; + return -1; } static int @@ -728,7 +728,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, goto routeerr2; } - return 1; + return 0; routeerr2: @@ -738,7 +738,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, network->def->bridge, network->def->forwardDev); routeerr1: - return 0; + return -1; } static int @@ -812,11 +812,11 @@ networkAddIptablesRules(struct network_driver *driver, /* If masquerading is enabled, set up the rules*/ if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT && - !networkAddMasqueradingIptablesRules(driver, network)) + networkAddMasqueradingIptablesRules(driver, network) < 0) goto err8; /* else if routing is enabled, set up the rules*/ else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE && - !networkAddRoutingIptablesRules(driver, network)) + networkAddRoutingIptablesRules(driver, network) < 0) goto err8; /* If we are doing local DHCP service on this network, attempt to @@ -834,7 +834,7 @@ networkAddIptablesRules(struct network_driver *driver, VIR_WARN0("May need to update iptables package & kernel to support CHECKSUM rule."); } - return 1; + return 0; err8: iptablesRemoveForwardAllowCross(driver->iptables, @@ -858,7 +858,7 @@ networkAddIptablesRules(struct network_driver *driver, err2: iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); err1: - return 0; + return -1; } static void @@ -927,7 +927,7 @@ networkReloadIptablesRules(struct network_driver *driver) if (virNetworkObjIsActive(driver->networks.objs[i])) { networkRemoveIptablesRules(driver, driver->networks.objs[i]); - if (!networkAddIptablesRules(driver, driver->networks.objs[i])) { + if (networkAddIptablesRules(driver, driver->networks.objs[i]) < 0) { /* failed to add but already logged */ } } @@ -1142,7 +1142,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver, goto err_delbr; } - if (!networkAddIptablesRules(driver, network)) + if (networkAddIptablesRules(driver, network) < 0) goto err_delbr1; if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && -- 1.7.3.4

IPv6 will use prefix exclusively, and IPv4 will also optionally be able to use it, and the iptables functions really need a prefix anyway, so use the new virNetworkDefPrefix() function to send prefixes into iptables functions instead of netmasks. Also, in a couple places where a netmask is actually needed, use the new private API function for it rather than getting it directly. This will allow for cases where no netmask or prefix is specified (it returns the default for the current class of network.) --- src/network/bridge_driver.c | 91 +++++++++++++++++++++++++++++++------------ src/util/iptables.c | 63 +++++++++++++++--------------- src/util/iptables.h | 16 ++++---- 3 files changed, 105 insertions(+), 65 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1ae3924..b592840 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -586,11 +586,19 @@ cleanup: static int networkAddMasqueradingIptablesRules(struct network_driver *driver, virNetworkObjPtr network) { + int prefix = virNetworkDefPrefix(network->def); + + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid prefix or netmask for '%s'"), + network->def->bridge); + goto masqerr1; + } /* allow forwarding packets from the bridge interface */ if (iptablesAddForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -602,7 +610,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* allow forwarding packets to the bridge interface if they are part of an existing connection */ if (iptablesAddForwardAllowRelatedIn(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -637,7 +645,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* First the generic masquerade rule for other protocols */ if (iptablesAddForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, NULL) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -649,7 +657,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* UDP with a source port restriction */ if (iptablesAddForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, "udp") < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -661,7 +669,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* TCP with a source port restriction */ if (iptablesAddForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, "tcp") < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -675,25 +683,25 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, masqerr5: iptablesRemoveForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); masqerr2: iptablesRemoveForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); masqerr1: @@ -703,11 +711,19 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, static int networkAddRoutingIptablesRules(struct network_driver *driver, virNetworkObjPtr network) { + int prefix = virNetworkDefPrefix(network->def); + + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid prefix or netmask for '%s'"), + network->def->bridge); + goto routeerr1; + } /* allow routing packets from the bridge interface */ if (iptablesAddForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -719,7 +735,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, /* allow routing packets to the bridge interface */ if (iptablesAddForwardAllowIn(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, @@ -734,7 +750,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, routeerr2: iptablesRemoveForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); routeerr1: @@ -870,40 +886,50 @@ networkRemoveIptablesRules(struct network_driver *driver, network->def->bridge, 68); } if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { + int prefix = virNetworkDefPrefix(network->def); + + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid prefix or netmask for '%s'"), + network->def->bridge); + goto error; + } + if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->forwardDev, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) iptablesRemoveForwardAllowIn(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); iptablesRemoveForwardAllowOut(driver->iptables, &network->def->ipAddress, - &network->def->netmask, + prefix, network->def->bridge, network->def->forwardDev); } +error: iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge); iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); @@ -1007,17 +1033,23 @@ static int networkCheckRouteCollision(virNetworkObjPtr network) { int ret = -1, len; unsigned int net_dest; + virSocketAddr netmask; char *cur, *buf = NULL; enum {MAX_ROUTE_SIZE = 1024*64}; - if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET) || - !VIR_SOCKET_IS_FAMILY(&network->def->netmask, AF_INET)) { + if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { /* Only support collision check for IPv4 */ return 0; } + if (virNetworkDefNetmask(network->def, &netmask) < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to get netmask of '%s'"), + network->def->bridge); + } + net_dest = (network->def->ipAddress.data.inet4.sin_addr.s_addr & - network->def->netmask.data.inet4.sin_addr.s_addr); + netmask.data.inet4.sin_addr.s_addr); /* Read whole routing table into memory */ if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0) @@ -1070,7 +1102,7 @@ static int networkCheckRouteCollision(virNetworkObjPtr network) addr_val &= mask_val; if ((net_dest == addr_val) && - (network->def->netmask.data.inet4.sin_addr.s_addr == mask_val)) { + (netmask.data.inet4.sin_addr.s_addr == mask_val)) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("Network is already in use by interface %s"), iface); @@ -1126,9 +1158,18 @@ static int networkStartNetworkDaemon(struct network_driver *driver, goto err_delbr; } - if (VIR_SOCKET_HAS_ADDR(&network->def->netmask) && - (err = brSetInetNetmask(driver->brctl, network->def->bridge, - &network->def->netmask))) { + virSocketAddr netmask; + + if (virNetworkDefNetmask(network->def, &netmask) < 0) { + + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("bridge '%s' has an invalid netmask or IP address"), + network->def->bridge); + goto err_delbr; + } + + if ((err = brSetInetNetmask(driver->brctl, network->def->bridge, + &netmask))) { virReportSystemError(err, _("cannot set netmask on bridge '%s'"), network->def->bridge); diff --git a/src/util/iptables.c b/src/util/iptables.c index effd81b..89e05b7 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -276,25 +276,24 @@ iptablesRemoveUdpInput(iptablesContext *ctx, static char *iptablesFormatNetwork(virSocketAddr *netaddr, - virSocketAddr *netmask) + int prefix) { virSocketAddr network; - int prefix; char *netstr; char *ret; - if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET) || - !VIR_SOCKET_IS_FAMILY(netmask, AF_INET)) { + if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET)) { iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only IPv4 addresses can be used with iptables")); return NULL; } network = *netaddr; - network.data.inet4.sin_addr.s_addr &= - netmask->data.inet4.sin_addr.s_addr; - - prefix = virSocketGetNumNetmaskBits(netmask); + if (virSocketAddrMaskByPrefix(&network, prefix) < 0) { + iptablesError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failure to mask address")); + return NULL; + } netstr = virSocketFormatAddr(&network); @@ -315,7 +314,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, static int iptablesForwardAllowOut(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev, int action) @@ -323,7 +322,7 @@ iptablesForwardAllowOut(iptablesContext *ctx, int ret; char *networkstr; - if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) + if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; if (physdev && physdev[0]) { @@ -362,11 +361,11 @@ iptablesForwardAllowOut(iptablesContext *ctx, int iptablesAddForwardAllowOut(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, ADD); + return iptablesForwardAllowOut(ctx, netaddr, prefix, iface, physdev, ADD); } /** @@ -385,11 +384,11 @@ iptablesAddForwardAllowOut(iptablesContext *ctx, int iptablesRemoveForwardAllowOut(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, REMOVE); + return iptablesForwardAllowOut(ctx, netaddr, prefix, iface, physdev, REMOVE); } @@ -399,7 +398,7 @@ iptablesRemoveForwardAllowOut(iptablesContext *ctx, static int iptablesForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev, int action) @@ -407,7 +406,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx, int ret; char *networkstr; - if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) + if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; if (physdev && physdev[0]) { @@ -450,11 +449,11 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx, int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, ADD); + return iptablesForwardAllowRelatedIn(ctx, netaddr, prefix, iface, physdev, ADD); } /** @@ -473,11 +472,11 @@ iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, REMOVE); + return iptablesForwardAllowRelatedIn(ctx, netaddr, prefix, iface, physdev, REMOVE); } /* Allow all traffic destined to the bridge, with a valid network address @@ -485,7 +484,7 @@ iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, static int iptablesForwardAllowIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev, int action) @@ -493,7 +492,7 @@ iptablesForwardAllowIn(iptablesContext *ctx, int ret; char *networkstr; - if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) + if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; if (physdev && physdev[0]) { @@ -532,11 +531,11 @@ iptablesForwardAllowIn(iptablesContext *ctx, int iptablesAddForwardAllowIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, ADD); + return iptablesForwardAllowIn(ctx, netaddr, prefix, iface, physdev, ADD); } /** @@ -555,11 +554,11 @@ iptablesAddForwardAllowIn(iptablesContext *ctx, int iptablesRemoveForwardAllowIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev) { - return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, REMOVE); + return iptablesForwardAllowIn(ctx, netaddr, prefix, iface, physdev, REMOVE); } @@ -722,7 +721,7 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx, static int iptablesForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *physdev, const char *protocol, int action) @@ -730,7 +729,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, int ret; char *networkstr; - if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) + if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; if (protocol && protocol[0]) { @@ -792,11 +791,11 @@ iptablesForwardMasquerade(iptablesContext *ctx, int iptablesAddForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *physdev, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD); } /** @@ -815,11 +814,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, int iptablesRemoveForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *physdev, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE); } diff --git a/src/util/iptables.h b/src/util/iptables.h index ed843ec..6f59b92 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -45,34 +45,34 @@ int iptablesRemoveUdpInput (iptablesContext *ctx, int iptablesAddForwardAllowOut (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowOut (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); int iptablesAddForwardAllowIn (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); int iptablesRemoveForwardAllowIn (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *iface, const char *physdev); @@ -93,12 +93,12 @@ int iptablesRemoveForwardRejectIn (iptablesContext *ctx, int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *physdev, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, - virSocketAddr *netmask, + int prefix, const char *physdev, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, -- 1.7.3.4

When a netmask isn't specified for an IPv4 address, one can be implied based on what network class range the address is in. The virNetworkDefPrefix function does this for us, so netmask isn't required. --- src/conf/network_conf.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b6ac4e3..09566ca 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) def->delay = 0; ipAddress = virXPathString("string(./ip[1]/@address)", ctxt); - netmask = virXPathString("string(./ip[1]/@netmask)", ctxt); - if (ipAddress && - netmask) { + if (ipAddress) { xmlNodePtr ip; if (virSocketParseAddr(ipAddress, &def->ipAddress, AF_UNSPEC) < 0) goto error; - if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0) - goto error; /* XXX someday we want IPv6, so will need to relax this */ - if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET) || - !VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) { + if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) { virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only IPv4 addresses are supported")); goto error; @@ -484,18 +479,25 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } VIR_FREE(ipAddress); - VIR_FREE(netmask); + netmask = virXPathString("string(./ip[1]/@netmask)", ctxt); + if (netmask) { - /* IPv4 forwarding setup */ - if (virXPathBoolean("count(./forward) > 0", ctxt)) { - if (def->ipAddress.data.sa.sa_family != AF_INET || - def->netmask.data.sa.sa_family != AF_INET) { - virNetworkReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Forwarding requested, but no IPv4 address/netmask provided")); + if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0) + goto error; + + /* XXX someday we want IPv6, so will need to relax this */ + if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("Only IPv4 addresses are supported")); goto error; } + } + VIR_FREE(netmask); + + /* IPv4 forwarding setup */ + if (virXPathBoolean("count(./forward) > 0", ctxt)) { tmp = virXPathString("string(./forward[1]/@mode)", ctxt); if (tmp) { if ((def->forwardType = virNetworkForwardTypeFromString(tmp)) < 0) { -- 1.7.3.4

brSetInetAddress can only set a single IP address on the bridge, and uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace it and brSetInetNetmask with a single function that uses the external "ip addr add" command to add an address/prefix to the interface - this supports IPv6, and allows adding multiple addresses to the interface. Although it isn't currently used in the code, we also add a brDelInetAddress for completeness' sake. Also, while we're modifying bridge.c, we change brSetForwardDelay and brSetEnableSTP to use the new virCommand API rather than the deprecated virRun, and also log an error message in bridge_driver.c if either of those fail (previously the failure would be completely silent). NB: it's a bit bothersome that there is no difference in error reporting between being unable to run one of these commands, and the command returning a non-0 exit status, but there is no precedent in bridge.c for logging error messages locally rather than pushing them up the call chain, and what's pushed up is only 'success' or 'failure'; no exit codes. Perhaps a non-0 exit could be passed up directly as the return, other errors could return -1, and callers could check for ret != 0 rather than ret < 0? --- configure.ac | 3 + src/libvirt_bridge.syms | 3 +- src/network/bridge_driver.c | 50 ++++++++------- src/util/bridge.c | 141 +++++++++++++++++++++++++------------------ src/util/bridge.h | 10 ++- 5 files changed, 118 insertions(+), 89 deletions(-) diff --git a/configure.ac b/configure.ac index 558bb77..357d59b 100644 --- a/configure.ac +++ b/configure.ac @@ -306,6 +306,9 @@ if test x"$with_rhel5_api" = x"yes"; then AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API]) fi +AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH]) +AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary]) + AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary]) diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms index 2658291..c3773bd 100644 --- a/src/libvirt_bridge.syms +++ b/src/libvirt_bridge.syms @@ -6,15 +6,16 @@ # bridge.h brAddBridge; +brAddInetAddress; brAddInterface; brAddTap; brDeleteTap; brDeleteBridge; +brDelInetAddress; brHasBridge; brInit; brSetEnableSTP; brSetForwardDelay; -brSetInetAddress; brSetInetNetmask; brSetInterfaceUp; brShutdown; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b592840..b8132ba 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1143,37 +1143,39 @@ static int networkStartNetworkDaemon(struct network_driver *driver, if (networkDisableIPV6(network) < 0) goto err_delbr; - if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0) - goto err_delbr; - - if (brSetEnableSTP(driver->brctl, network->def->bridge, network->def->stp ? 1 : 0) < 0) - goto err_delbr; - - if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) && - (err = brSetInetAddress(driver->brctl, network->def->bridge, - &network->def->ipAddress))) { - virReportSystemError(err, - _("cannot set IP address on bridge '%s'"), - network->def->bridge); + if ((err = brSetForwardDelay(driver->brctl, network->def->bridge, + network->def->delay))) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set forward delay on bridge '%s'"), + network->def->bridge); goto err_delbr; } - virSocketAddr netmask; - - if (virNetworkDefNetmask(network->def, &netmask) < 0) { - + if ((err = brSetEnableSTP(driver->brctl, network->def->bridge, + network->def->stp ? 1 : 0))) { networkReportError(VIR_ERR_INTERNAL_ERROR, - _("bridge '%s' has an invalid netmask or IP address"), - network->def->bridge); + _("cannot set STP '%s' on bridge '%s'"), + network->def->stp ? "on" : "off", network->def->bridge); goto err_delbr; } - if ((err = brSetInetNetmask(driver->brctl, network->def->bridge, - &netmask))) { - virReportSystemError(err, - _("cannot set netmask on bridge '%s'"), - network->def->bridge); - goto err_delbr; + if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) { + int prefix = virNetworkDefPrefix(network->def); + + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("bridge '%s' has an invalid netmask or IP address"), + network->def->bridge); + goto err_delbr; + } + + if ((err = brAddInetAddress(driver->brctl, network->def->bridge, + &network->def->ipAddress, prefix))) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set IP address on bridge '%s'"), + network->def->bridge); + goto err_delbr; + } } if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) { diff --git a/src/util/bridge.c b/src/util/bridge.c index 97cf73f..0cf9301 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -46,6 +46,7 @@ # include <net/if_arp.h> /* ARPHRD_ETHER */ # include "internal.h" +# include "command.h" # include "memory.h" # include "util.h" # include "logging.h" @@ -655,73 +656,84 @@ brGetInterfaceUp(brControl *ctl, return 0; } -static int -brSetInetAddr(brControl *ctl, - const char *ifname, - int cmd, - virSocketAddr *addr) -{ - struct ifreq ifr; - - if (!ctl || !ctl->fd || !ifname || !addr) - return EINVAL; - - memset(&ifr, 0, sizeof(struct ifreq)); - - if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) - return EINVAL; - - if (!VIR_SOCKET_IS_FAMILY(addr, AF_INET)) - return EINVAL; - - ifr.ifr_addr = addr->data.sa; - - if (ioctl(ctl->fd, cmd, &ifr) < 0) - return errno; - - return 0; -} - /** - * brSetInetAddress: + * brAddInetAddress: * @ctl: bridge control pointer * @ifname: the interface name - * @addr: the string representation of the IP address + * @addr: the IP address (IPv4 or IPv6) + * @prefix: number of 1 bits in the netmask * - * Function to bind the interface to an IP address, it should handle - * IPV4 and IPv6. The string for addr would be of the form - * "ddd.ddd.ddd.ddd" assuming the common IPv4 format. + * Add an IP address to an interface. This function *does not* remove + * any previously added interfaces - that must be done separately with + * brDelInetAddress. * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 in case of success or -1 in case of error. */ int -brSetInetAddress(brControl *ctl, +brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED, const char *ifname, - virSocketAddr *addr) + virSocketAddr *addr, + int prefix) { - return brSetInetAddr(ctl, ifname, SIOCSIFADDR, addr); + virCommandPtr cmd; + char *addrstr; + int ret = -1; + + if (!(addrstr = virSocketFormatAddr(addr))) + goto cleanup; + cmd = virCommandNew(IP_PATH); + virCommandAddArgList(cmd, "addr", "add", NULL); + virCommandAddArgFormat(cmd, "%s/%d", addrstr, prefix); + virCommandAddArgList(cmd, "dev", ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(addrstr); + virCommandFree(cmd); + return ret; } /** - * brSetInetNetmask: + * brDelInetAddress: * @ctl: bridge control pointer * @ifname: the interface name - * @addr: the string representation of the netmask + * @addr: the IP address (IPv4 or IPv6) + * @prefix: number of 1 bits in the netmask * - * Function to set the netmask of an interface, it should handle - * IPV4 and IPv6 forms. The string for addr would be of the form - * "ddd.ddd.ddd.ddd" assuming the common IPv4 format. + * Delete an IP address from an interface. * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 in case of success or -1 in case of error. */ int -brSetInetNetmask(brControl *ctl, +brDelInetAddress(brControl *ctl ATTRIBUTE_UNUSED, const char *ifname, - virSocketAddr *addr) + virSocketAddr *addr, + int prefix) { - return brSetInetAddr(ctl, ifname, SIOCSIFNETMASK, addr); + virCommandPtr cmd; + char *addrstr; + int ret = -1; + + if (!(addrstr = virSocketFormatAddr(addr))) + goto cleanup; + cmd = virCommandNew(IP_PATH); + virCommandAddArgList(cmd, "addr", "del", NULL); + virCommandAddArgFormat(cmd, "%s/%d", addrstr, prefix); + virCommandAddArgList(cmd, "dev", ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(addrstr); + virCommandFree(cmd); + return ret; } /** @@ -740,17 +752,20 @@ brSetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED, const char *bridge, int delay) { - char delayStr[30]; - const char *const progargv[] = { - BRCTL, "setfd", bridge, delayStr, NULL - }; + virCommandPtr cmd; + int ret = -1; - snprintf(delayStr, sizeof(delayStr), "%d", delay); + cmd = virCommandNew(BRCTL); + virCommandAddArgList(cmd, "setfd", bridge, NULL); + virCommandAddArgFormat(cmd, "%d", delay); - if (virRun(progargv, NULL) < 0) - return -1; + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; - return 0; + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; } /** @@ -769,15 +784,21 @@ brSetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED, const char *bridge, int enable) { - const char *setting = enable ? "on" : "off"; - const char *const progargv[] = { - BRCTL, "stp", bridge, setting, NULL - }; + virCommandPtr cmd; + int ret = -1; - if (virRun(progargv, NULL) < 0) - return -1; + cmd = virCommandNew(BRCTL); + virCommandAddArgList(cmd, "stp", bridge, + enable ? "on" : "off", + NULL); - return 0; + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; } #endif /* WITH_BRIDGE */ diff --git a/src/util/bridge.h b/src/util/bridge.h index 3ef38d9..8132d35 100644 --- a/src/util/bridge.h +++ b/src/util/bridge.h @@ -83,12 +83,14 @@ int brGetInterfaceUp (brControl *ctl, const char *ifname, int *up); -int brSetInetAddress (brControl *ctl, +int brAddInetAddress (brControl *ctl, const char *ifname, - virSocketAddr *addr); -int brSetInetNetmask (brControl *ctl, + virSocketAddr *addr, + int prefix); +int brDelInetAddress (brControl *ctl, const char *ifname, - virSocketAddr *addr); + virSocketAddr *addr, + int prefix); int brSetForwardDelay (brControl *ctl, const char *bridge, -- 1.7.3.4

This commit adds support for IPv6 parsing and formatting to the virtual network XML parser, including moving around data definitions to allow for multiple <ip> elements on a single network, but only changes the consumers of this API to accomodate for the changes in API/structure, not to add any actual IPv6 functionality. That will come in a later patch - this patch attempts to maintain the same final functionality in both drivers that use the network XML parser - vbox and "bridge" (the Linux bridge-based driver used by the qemu hypervisor driver). * src/libvirt_private.syms: add new private API functions * src/conf/network_conf.[ch]: change C data structure and parsing/formatting * src/network/bridge_driver.c: update to use new parser/formatter * src/vbox/vbox_tmpl.c: update to use new parser/formatter * docs/schemas/network.rng: changes to the schema - * there can now be more than one <ip> element. * ip address is now an ip-addr (ipv4 or ipv6) rather than ipv4-addr * new optional "prefix" attribute that can be used in place of "netmask" * new optional "family" attribute - "ipv4" or "ipv6" (will default to ipv4) * define data types for the above * tests/networkxml2xml(in|out)/nat-network.xml: add multiple <ip> elements (including IPv6) to a single network definition to verify they are being correctly parsed and formatted. --- docs/schemas/network.rng | 41 +++- src/conf/network_conf.c | 452 +++++++++++++++++++++---------- src/conf/network_conf.h | 50 +++- src/libvirt_private.syms | 5 +- src/network/bridge_driver.c | 248 ++++++++++-------- src/vbox/vbox_tmpl.c | 81 ++++-- tests/networkxml2xmlin/nat-network.xml | 8 + tests/networkxml2xmlout/nat-network.xml | 8 + 8 files changed, 589 insertions(+), 304 deletions(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index ac13af2..bc34ddc 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -80,15 +80,21 @@ </optional> <!-- <ip> element --> - <optional> + <zeroOrMore> <!-- The IP element sets up NAT'ing and an optional DHCP server local to the host. --> <element name="ip"> <optional> - <attribute name="address"><ref name="ipv4-addr"/></attribute> + <attribute name="address"><ref name="ip-addr"/></attribute> + </optional> + <optional> + <choice> + <attribute name="netmask"><ref name="ipv4-addr"/></attribute> + <attribute name="prefix"><ref name="ip-prefix"/></attribute> + </choice> </optional> <optional> - <attribute name="netmask"><ref name="ipv4-addr"/></attribute> + <attribute name="family"><ref name="addr-family"/></attribute> </optional> <optional> <element name="tftp"> @@ -123,7 +129,7 @@ </element> </optional> </element> - </optional> + </zeroOrMore> </interleave> </element> </define> @@ -135,6 +141,33 @@ </data> </define> + <!-- Based on http://blog.mes-stats.fr/2008/10/09/regex-ipv4-et-ipv6 --> + <define name='ipv6-addr'> + <data type='string'> + <!-- To understand this better, take apart the toplevel '|'s --> + <param name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|(([0-9A-Fa-f]{1,4}:){0,5}:((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|(::([0-9A-Fa-f]{1,4}:){0,5}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)</param> + </data> + </define> + + <define name='ip-addr'> + <choice> + <ref name='ipv4-addr'/> + <ref name='ipv6-addr'/> + </choice> + </define> + + <define name='ip-prefix'> + <data type='unsignedInt'> + <param name="maxInclusive">128</param> + </data> + </define> + + <define name='addr-family'> + <data type='string'> + <param name="pattern">(ipv4)|(ipv6)</param> + </data> + </define> + <!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" --> <define name='mac-addr'> <data type='string'> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 09566ca..4486a5b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -87,9 +87,26 @@ virNetworkObjPtr virNetworkFindByName(const virNetworkObjListPtr nets, } +static void virNetworkIpDefClear(virNetworkIpDefPtr def) +{ + int ii; + + VIR_FREE(def->family); + VIR_FREE(def->ranges); + + for (ii = 0 ; ii < def->nhosts && def->hosts ; ii++) { + VIR_FREE(def->hosts[ii].mac); + VIR_FREE(def->hosts[ii].name); + } + + VIR_FREE(def->hosts); + VIR_FREE(def->tftproot); + VIR_FREE(def->bootfile); +} + void virNetworkDefFree(virNetworkDefPtr def) { - int i; + int ii; if (!def) return; @@ -99,16 +116,10 @@ void virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def->forwardDev); VIR_FREE(def->domain); - VIR_FREE(def->ranges); - - for (i = 0 ; i < def->nhosts && def->hosts ; i++) { - VIR_FREE(def->hosts[i].mac); - VIR_FREE(def->hosts[i].name); + for (ii = 0 ; ii < def->nips && def->ips ; ii++) { + virNetworkIpDefClear(&def->ips[ii]); } - VIR_FREE(def->hosts); - - VIR_FREE(def->tftproot); - VIR_FREE(def->bootfile); + VIR_FREE(def->ips); VIR_FREE(def); } @@ -207,16 +218,43 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets, } } +/* return ips[index], or NULL if there aren't enough ips */ +virNetworkIpDefPtr +virNetworkDefGetIpByIndex(const virNetworkDefPtr def, + int family, int n) +{ + int ii; + + if (!def->ips || n >= def->nips) + return NULL; + + if (family == AF_UNSPEC) { + return &def->ips[n]; + } + + /* find the nth ip of type "family" */ + for (ii = 0; ii < def->nips; ii++) { + if (VIR_SOCKET_IS_FAMILY(&def->ips[ii].address, family) + && (n-- <= 0)) { + return &def->ips[ii]; + } + } + /* failed to find enough of the right family */ + return NULL; +} + /* return number of 1 bits in netmask for the network's ipAddress, * or -1 on error */ -int virNetworkDefPrefix(const virNetworkDefPtr def) +int virNetworkIpDefPrefix(const virNetworkIpDefPtr def) { - if (VIR_SOCKET_HAS_ADDR(&def->netmask)) { + if (def->prefix > 0) { + return def->prefix; + } else if (VIR_SOCKET_HAS_ADDR(&def->netmask)) { return virSocketGetNumNetmaskBits(&def->netmask); - } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) { + } else if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) { /* return the natural prefix for the network's ip address */ - int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr); + int addr = ntohl(def->address.data.inet4.sin_addr.s_addr); if (IN_CLASSA(addr)) return 8; if (IN_CLASSB(addr)) @@ -224,6 +262,8 @@ int virNetworkDefPrefix(const virNetworkDefPtr def) if (IN_CLASSC(addr)) return 24; return -1; + } else if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET6)) { + return 64; } return -1; } @@ -232,22 +272,23 @@ int virNetworkDefPrefix(const virNetworkDefPtr def) * definition, based on either the definition's netmask, or its * prefix. Return -1 on error (and set the netmask family to AF_UNSPEC) */ -int virNetworkDefNetmask(const virNetworkDefPtr def, - virSocketAddrPtr netmask) +int virNetworkIpDefNetmask(const virNetworkIpDefPtr def, + virSocketAddrPtr netmask) { if (VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) { *netmask = def->netmask; return 0; } - return virSocketAddrPrefixToNetmask(virNetworkDefPrefix(def), netmask, - VIR_SOCKET_FAMILY(&def->ipAddress)); + return virSocketAddrPrefixToNetmask(virNetworkIpDefPrefix(def), netmask, + VIR_SOCKET_FAMILY(&def->address)); } static int -virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def, - xmlNodePtr node) { +virNetworkDHCPRangeDefParseXML(virNetworkIpDefPtr def, + xmlNodePtr node) +{ xmlNodePtr cur; @@ -381,33 +422,146 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def, } static int -virNetworkIPParseXML(virNetworkDefPtr def, - xmlNodePtr node) { - xmlNodePtr cur; +virNetworkIPParseXML(const char *networkName, + virNetworkIpDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + /* + * virNetworkIpDef object is already allocated as part of an array. + * On failure clear it out, but don't free it. + */ - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "dhcp")) { - int result = virNetworkDHCPRangeDefParseXML(def, cur); - if (result) - return result; + xmlNodePtr cur, save; + char *address, *netmask; + unsigned long prefix; + int result = -1; + + save = ctxt->node; + ctxt->node = node; + + /* grab raw data from XML */ + def->family = virXPathString("string(./@family)", ctxt); + address = virXPathString("string(./@address)", ctxt); + if (virXPathULong("string(./@prefix)", ctxt, &prefix) < 0) + def->prefix = 0; + else + def->prefix = prefix; + + netmask = virXPathString("string(./@netmask)", ctxt); + + if (address) { + if (virSocketParseAddr(address, &def->address, AF_UNSPEC) < 0) { + virNetworkReportError(VIR_ERR_XML_ERROR, + _("Bad address '%s' in definition of network '%s'"), + address, networkName); + goto error; + } - } else if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "tftp")) { - char *root; + } - if (!(root = virXMLPropString(cur, "root"))) { - cur = cur->next; - continue; + /* validate family vs. address */ + if (def->family == NULL) { + if (!(VIR_SOCKET_IS_FAMILY(&def->address, AF_INET) || + VIR_SOCKET_IS_FAMILY(&def->address, AF_UNSPEC))) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("no family specified for non-IPv4 address address '%s' in network '%s'"), + address, networkName); + goto error; + } + } else if (STREQ(def->family, "ipv4")) { + if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("family 'ipv4' specified for non-IPv4 address '%s' in network '%s'"), + address, networkName); + goto error; + } + } else if (STREQ(def->family, "ipv6")) { + if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET6)) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("family 'ipv6' specified for non-IPv6 address '%s' in network '%s'"), + address, networkName); + goto error; + } + } else { + virNetworkReportError(VIR_ERR_XML_ERROR, + _("Unrecognized family '%s' in definition of network '%s'"), + def->family, networkName); + goto error; + } + + /* parse/validate netmask */ + if (netmask) { + if (address == NULL) { + /* netmask is meaningless without an address */ + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("netmask specified without address in network '%s'"), + networkName); + goto error; + } + + if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("netmask not supported for address '%s' in network '%s' (IPv4 only)"), + address, networkName); + goto error; + } + + if (def->prefix > 0) { + /* can't have both netmask and prefix at the same time */ + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("network '%s' has prefix='%d' but no address"), + networkName, def->prefix); + goto error; + } + + if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0) + goto error; + + if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) { + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("network '%s' has invalid netmask '%s' for address '%s' (both must be IPv4)"), + networkName, netmask, address); + goto error; + } + } + + if (VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) { + /* parse IPv4-related info */ + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "dhcp")) { + result = virNetworkDHCPRangeDefParseXML(def, cur); + if (result) + goto error; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "tftp")) { + char *root; + + if (!(root = virXMLPropString(cur, "root"))) { + cur = cur->next; + continue; + } + + def->tftproot = (char *)root; } - def->tftproot = root; + cur = cur->next; } + } - cur = cur->next; + result = 0; + +error: + if (result < 0) { + virNetworkIpDefClear(def); } - return 0; + VIR_FREE(address); + + ctxt->node = save; + return result; } static virNetworkDefPtr @@ -415,8 +569,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) { virNetworkDefPtr def; char *tmp; - char *ipAddress; - char *netmask; + xmlNodePtr *ipNodes = NULL; + int nIps; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -460,44 +614,32 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (virXPathULong("string(./bridge[1]/@delay)", ctxt, &def->delay) < 0) def->delay = 0; - ipAddress = virXPathString("string(./ip[1]/@address)", ctxt); - if (ipAddress) { - xmlNodePtr ip; - - if (virSocketParseAddr(ipAddress, &def->ipAddress, AF_UNSPEC) < 0) - goto error; + nIps = virXPathNodeSet("./ip", ctxt, &ipNodes); + if (nIps > 0) { + int ii; - /* XXX someday we want IPv6, so will need to relax this */ - if (!VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) { - virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("Only IPv4 addresses are supported")); + /* allocate array to hold all the addrs */ + if (VIR_ALLOC_N(def->ips, nIps) < 0) { + virReportOOMError(); goto error; } - - if ((ip = virXPathNode("./ip[1]", ctxt)) && - virNetworkIPParseXML(def, ip) < 0) - goto error; - } - VIR_FREE(ipAddress); - - netmask = virXPathString("string(./ip[1]/@netmask)", ctxt); - if (netmask) { - - if (virSocketParseAddr(netmask, &def->netmask, AF_UNSPEC) < 0) - goto error; - - /* XXX someday we want IPv6, so will need to relax this */ - if (!VIR_SOCKET_IS_FAMILY(&def->netmask, AF_INET)) { - virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("Only IPv4 addresses are supported")); - goto error; + /* parse each addr */ + for (ii = 0; ii < nIps; ii++) { + int ret = virNetworkIPParseXML(def->name, &def->ips[ii], + ipNodes[ii], ctxt); + if (ret < 0) + goto error; + def->nips++; } } - VIR_FREE(netmask); - /* IPv4 forwarding setup */ if (virXPathBoolean("count(./forward) > 0", ctxt)) { + if (def->nips == 0) { + virNetworkReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Forwarding requested, but no IP address provided")); + goto error; + } tmp = virXPathString("string(./forward[1]/@mode)", ctxt); if (tmp) { if ((def->forwardType = virNetworkForwardTypeFromString(tmp)) < 0) { @@ -576,11 +718,101 @@ cleanup: return def; } +static int +virNetworkIpDefFormat(virBufferPtr buf, + const virNetworkIpDefPtr def) +{ + int result = -1; + + virBufferAddLit(buf, " <ip"); + + if (def->family) { + virBufferVSprintf(buf, " family='%s'", def->family); + } + if (VIR_SOCKET_HAS_ADDR(&def->address)) { + char *addr = virSocketFormatAddr(&def->address); + if (!addr) + goto error; + virBufferVSprintf(buf, " address='%s'", addr); + VIR_FREE(addr); + } + if (VIR_SOCKET_HAS_ADDR(&def->netmask)) { + char *addr = virSocketFormatAddr(&def->netmask); + if (!addr) + goto error; + virBufferVSprintf(buf, " netmask='%s'", addr); + VIR_FREE(addr); + } + if (def->prefix > 0) { + virBufferVSprintf(buf," prefix='%d'", def->prefix); + } + virBufferAddLit(buf, ">\n"); + + if (def->tftproot) { + virBufferEscapeString(buf, " <tftp root='%s' />\n", + def->tftproot); + } + if ((def->nranges || def->nhosts)) { + int ii; + virBufferAddLit(buf, " <dhcp>\n"); + for (ii = 0 ; ii < def->nranges ; ii++) { + char *saddr = virSocketFormatAddr(&def->ranges[ii].start); + if (!saddr) + goto error; + char *eaddr = virSocketFormatAddr(&def->ranges[ii].end); + if (!eaddr) { + VIR_FREE(saddr); + goto error; + } + virBufferVSprintf(buf, " <range start='%s' end='%s' />\n", + saddr, eaddr); + VIR_FREE(saddr); + VIR_FREE(eaddr); + } + for (ii = 0 ; ii < def->nhosts ; ii++) { + virBufferAddLit(buf, " <host "); + if (def->hosts[ii].mac) + virBufferVSprintf(buf, "mac='%s' ", def->hosts[ii].mac); + if (def->hosts[ii].name) + virBufferVSprintf(buf, "name='%s' ", def->hosts[ii].name); + if (VIR_SOCKET_HAS_ADDR(&def->hosts[ii].ip)) { + char *ipaddr = virSocketFormatAddr(&def->hosts[ii].ip); + if (!ipaddr) + goto error; + virBufferVSprintf(buf, "ip='%s' ", ipaddr); + VIR_FREE(ipaddr); + } + virBufferAddLit(buf, "/>\n"); + } + if (def->bootfile) { + virBufferEscapeString(buf, " <bootp file='%s' ", + def->bootfile); + if (VIR_SOCKET_HAS_ADDR(&def->bootserver)) { + char *ipaddr = virSocketFormatAddr(&def->bootserver); + if (!ipaddr) + goto error; + virBufferEscapeString(buf, "server='%s' ", ipaddr); + VIR_FREE(ipaddr); + } + virBufferAddLit(buf, "/>\n"); + } + + virBufferAddLit(buf, " </dhcp>\n"); + } + + virBufferAddLit(buf, " </ip>\n"); + + result = 0; +error: + return result; +} + char *virNetworkDefFormat(const virNetworkDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; + int ii; virBufferAddLit(&buf, "<network>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); @@ -612,81 +844,9 @@ char *virNetworkDefFormat(const virNetworkDefPtr def) if (def->domain) virBufferVSprintf(&buf, " <domain name='%s'/>\n", def->domain); - if (VIR_SOCKET_HAS_ADDR(&def->ipAddress) || - VIR_SOCKET_HAS_ADDR(&def->netmask)) { - virBufferAddLit(&buf, " <ip"); - - if (VIR_SOCKET_HAS_ADDR(&def->ipAddress)) { - char *addr = virSocketFormatAddr(&def->ipAddress); - if (!addr) - goto error; - virBufferVSprintf(&buf, " address='%s'", addr); - VIR_FREE(addr); - } - - if (VIR_SOCKET_HAS_ADDR(&def->netmask)) { - char *addr = virSocketFormatAddr(&def->netmask); - if (!addr) - goto error; - virBufferVSprintf(&buf, " netmask='%s'", addr); - VIR_FREE(addr); - } - - virBufferAddLit(&buf, ">\n"); - - if (def->tftproot) { - virBufferEscapeString(&buf, " <tftp root='%s' />\n", - def->tftproot); - } - if ((def->nranges || def->nhosts)) { - int i; - virBufferAddLit(&buf, " <dhcp>\n"); - for (i = 0 ; i < def->nranges ; i++) { - char *saddr = virSocketFormatAddr(&def->ranges[i].start); - if (!saddr) - goto error; - char *eaddr = virSocketFormatAddr(&def->ranges[i].end); - if (!eaddr) { - VIR_FREE(saddr); - goto error; - } - virBufferVSprintf(&buf, " <range start='%s' end='%s' />\n", - saddr, eaddr); - VIR_FREE(saddr); - VIR_FREE(eaddr); - } - for (i = 0 ; i < def->nhosts ; i++) { - virBufferAddLit(&buf, " <host "); - if (def->hosts[i].mac) - virBufferVSprintf(&buf, "mac='%s' ", def->hosts[i].mac); - if (def->hosts[i].name) - virBufferVSprintf(&buf, "name='%s' ", def->hosts[i].name); - if (VIR_SOCKET_HAS_ADDR(&def->hosts[i].ip)) { - char *ipaddr = virSocketFormatAddr(&def->hosts[i].ip); - if (!ipaddr) - goto error; - virBufferVSprintf(&buf, "ip='%s' ", ipaddr); - VIR_FREE(ipaddr); - } - virBufferAddLit(&buf, "/>\n"); - } - if (def->bootfile) { - virBufferEscapeString(&buf, " <bootp file='%s' ", - def->bootfile); - if (VIR_SOCKET_HAS_ADDR(&def->bootserver)) { - char *ipaddr = virSocketFormatAddr(&def->bootserver); - if (!ipaddr) - goto error; - virBufferEscapeString(&buf, "server='%s' ", ipaddr); - VIR_FREE(ipaddr); - } - virBufferAddLit(&buf, "/>\n"); - } - - virBufferAddLit(&buf, " </dhcp>\n"); - } - - virBufferAddLit(&buf, " </ip>\n"); + for (ii = 0; ii < def->nips; ii++) { + if (virNetworkIpDefFormat(&buf, &def->ips[ii]) < 0) + goto error; } virBufferAddLit(&buf, "</network>\n"); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index a922d28..47f0408 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -56,6 +56,33 @@ struct _virNetworkDHCPHostDef { virSocketAddr ip; }; +typedef struct _virNetworkIpDef virNetworkIpDef; +typedef virNetworkIpDef *virNetworkIpDefPtr; +struct _virNetworkIpDef { + char *family; /* ipv4 or ipv6 - default is ipv4 */ + virSocketAddr address; /* Bridge IP address */ + + /* The first two items below are taken directly from the XML (one + * or the other is given, but not both) and the 3rd is derived + * from the first two. When formatting XML, always use netMasktStr + * if it's non-NULL, but when using netmask in other code, use + * netmask, as it will automatically take into account prefix or + * natural netmasks (when no netmask or prefix is specified). + */ + int prefix; /* ipv6 - only prefix allowed */ + virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */ + + unsigned int nranges; /* Zero or more dhcp ranges */ + virNetworkDHCPRangeDefPtr ranges; + + unsigned int nhosts; /* Zero or more dhcp hosts */ + virNetworkDHCPHostDefPtr hosts; + + char *tftproot; + char *bootfile; + virSocketAddr bootserver; + }; + typedef struct _virNetworkDef virNetworkDef; typedef virNetworkDef *virNetworkDefPtr; struct _virNetworkDef { @@ -70,18 +97,8 @@ struct _virNetworkDef { int forwardType; /* One of virNetworkForwardType constants */ char *forwardDev; /* Destination device for forwarding */ - virSocketAddr ipAddress; /* Bridge IP address */ - virSocketAddr netmask; - - unsigned int nranges; /* Zero or more dhcp ranges */ - virNetworkDHCPRangeDefPtr ranges; - - unsigned int nhosts; /* Zero or more dhcp hosts */ - virNetworkDHCPHostDefPtr hosts; - - char *tftproot; - char *bootfile; - virSocketAddr bootserver; + int nips; + virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */ }; typedef struct _virNetworkObj virNetworkObj; @@ -133,9 +150,12 @@ virNetworkDefPtr virNetworkDefParseNode(xmlDocPtr xml, char *virNetworkDefFormat(const virNetworkDefPtr def); -int virNetworkDefPrefix(const virNetworkDefPtr def); -int virNetworkDefNetmask(const virNetworkDefPtr def, - virSocketAddrPtr netmask); +virNetworkIpDefPtr +virNetworkDefGetIpByIndex(const virNetworkDefPtr def, + int family, int n); +int virNetworkIpDefPrefix(const virNetworkIpDefPtr def); +int virNetworkIpDefNetmask(const virNetworkIpDefPtr def, + virSocketAddrPtr netmask); int virNetworkSaveXML(const char *configDir, virNetworkDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9ba7b1..6cb8270 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -578,15 +578,16 @@ virNetworkAssignDef; virNetworkConfigFile; virNetworkDefFormat; virNetworkDefFree; -virNetworkDefNetmask; +virNetworkDefGetIpByIndex; virNetworkDefParseFile; virNetworkDefParseNode; virNetworkDefParseString; -virNetworkDefPrefix; virNetworkDeleteConfig; virNetworkFindByName; virNetworkFindByUUID; virNetworkLoadAllConfigs; +virNetworkIpDefNetmask; +virNetworkIpDefPrefix; virNetworkObjIsDuplicate; virNetworkObjListFree; virNetworkObjLock; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b8132ba..fe336a4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -145,8 +145,7 @@ networkFindActiveConfigs(struct network_driver *driver) { obj->active = 1; /* Finally try and read dnsmasq pid if any */ - if ((VIR_SOCKET_HAS_ADDR(&obj->def->ipAddress) || - obj->def->nranges) && + if (obj->def->ips && (obj->def->nips > 0) && virFileReadPid(NETWORK_PID_DIR, obj->def->name, &obj->dnsmasqPid) == 0) { @@ -366,7 +365,7 @@ networkShutdown(void) { static int -networkSaveDnsmasqHostsfile(virNetworkObjPtr network, +networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef, dnsmasqContext *dctx, bool force) { @@ -375,8 +374,8 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, if (! force && virFileExists(dctx->hostsfile->path)) return 0; - for (i = 0 ; i < network->def->nhosts ; i++) { - virNetworkDHCPHostDefPtr host = &(network->def->hosts[i]); + for (i = 0; i < ipdef->nhosts; i++) { + virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]); if ((host->mac) && VIR_SOCKET_HAS_ADDR(&host->ip)) dnsmasqAddDhcpHost(dctx, host->mac, &host->ip, host->name); } @@ -390,13 +389,14 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, static int networkBuildDnsmasqArgv(virNetworkObjPtr network, + virNetworkIpDefPtr ipdef, const char *pidfile, virCommandPtr cmd) { int r, ret = -1; int nbleases = 0; char *bridgeaddr; - if (!(bridgeaddr = virSocketFormatAddr(&network->def->ipAddress))) + if (!(bridgeaddr = virSocketFormatAddr(&ipdef->address))) goto cleanup; /* * NB, be careful about syntax for dnsmasq options in long format. @@ -438,18 +438,18 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * clearly not practical * * virCommandAddArg(cmd, "--interface"); - * virCommandAddArg(cmd, network->def->bridge); + * virCommandAddArg(cmd, ipdef->bridge); */ virCommandAddArgList(cmd, "--listen-address", bridgeaddr, "--except-interface", "lo", NULL); - for (r = 0 ; r < network->def->nranges ; r++) { - char *saddr = virSocketFormatAddr(&network->def->ranges[r].start); + for (r = 0 ; r < ipdef->nranges ; r++) { + char *saddr = virSocketFormatAddr(&ipdef->ranges[r].start); if (!saddr) goto cleanup; - char *eaddr = virSocketFormatAddr(&network->def->ranges[r].end); + char *eaddr = virSocketFormatAddr(&ipdef->ranges[r].end); if (!eaddr) { VIR_FREE(saddr); goto cleanup; @@ -458,8 +458,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr); VIR_FREE(saddr); VIR_FREE(eaddr); - nbleases += virSocketGetRange(&network->def->ranges[r].start, - &network->def->ranges[r].end); + nbleases += virSocketGetRange(&ipdef->ranges[r].start, + &ipdef->ranges[r].end); } /* @@ -467,19 +467,19 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * we have to add a special --dhcp-range option to enable the service in * dnsmasq. */ - if (!network->def->nranges && network->def->nhosts) { + if (!ipdef->nranges && ipdef->nhosts) { virCommandAddArg(cmd, "--dhcp-range"); virCommandAddArgFormat(cmd, "%s,static", bridgeaddr); } - if (network->def->nranges > 0) { + if (ipdef->nranges > 0) { virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases); } - if (network->def->nranges || network->def->nhosts) + if (ipdef->nranges || ipdef->nhosts) virCommandAddArg(cmd, "--dhcp-no-override"); - if (network->def->nhosts > 0) { + if (ipdef->nhosts > 0) { dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); if (dctx == NULL) { @@ -487,31 +487,30 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, goto cleanup; } - if (networkSaveDnsmasqHostsfile(network, dctx, false) < 0) { + if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) < 0) { virCommandAddArgPair(cmd, "--dhcp-hostsfile", dctx->hostsfile->path); } dnsmasqContextFree(dctx); } - if (network->def->tftproot) { + if (ipdef->tftproot) { virCommandAddArgList(cmd, "--enable-tftp", - "--tftp-root", network->def->tftproot, + "--tftp-root", ipdef->tftproot, NULL); } - if (network->def->bootfile) { - + if (ipdef->bootfile) { virCommandAddArg(cmd, "--dhcp-boot"); - if (VIR_SOCKET_HAS_ADDR(&network->def->bootserver)) { - char *bootserver = virSocketFormatAddr(&network->def->bootserver); + if (VIR_SOCKET_HAS_ADDR(&ipdef->bootserver)) { + char *bootserver = virSocketFormatAddr(&ipdef->bootserver); if (!bootserver) goto cleanup; virCommandAddArgFormat(cmd, "%s%s%s", - network->def->bootfile, ",,", bootserver); + ipdef->bootfile, ",,", bootserver); VIR_FREE(bootserver); } else { - virCommandAddArg(cmd, network->def->bootfile); + virCommandAddArg(cmd, ipdef->bootfile); } } @@ -521,9 +520,9 @@ cleanup: return ret; } - static int -dhcpStartDhcpDaemon(virNetworkObjPtr network) +dhcpStartDhcpDaemon(virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) { virCommandPtr cmd = NULL; char *pidfile = NULL; @@ -531,7 +530,7 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) network->dnsmasqPid = -1; - if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { + if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot start dhcp daemon without IPv4 address for server")); goto cleanup; @@ -556,7 +555,7 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) } cmd = virCommandNew(DNSMASQ); - if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) { + if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd) < 0) { VIR_FREE(pidfile); goto cleanup; } @@ -585,8 +584,10 @@ cleanup: static int networkAddMasqueradingIptablesRules(struct network_driver *driver, - virNetworkObjPtr network) { - int prefix = virNetworkDefPrefix(network->def); + virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) +{ + int prefix = virNetworkIpDefPrefix(ipdef); if (prefix < 0) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -597,7 +598,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* allow forwarding packets from the bridge interface */ if (iptablesAddForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev) < 0) { @@ -609,7 +610,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* allow forwarding packets to the bridge interface if they are part of an existing connection */ if (iptablesAddForwardAllowRelatedIn(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev) < 0) { @@ -644,7 +645,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* First the generic masquerade rule for other protocols */ if (iptablesAddForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, NULL) < 0) { @@ -656,7 +657,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* UDP with a source port restriction */ if (iptablesAddForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, "udp") < 0) { @@ -668,7 +669,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* TCP with a source port restriction */ if (iptablesAddForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, "tcp") < 0) { @@ -682,25 +683,25 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, masqerr5: iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); masqerr2: iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); @@ -710,8 +711,9 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, static int networkAddRoutingIptablesRules(struct network_driver *driver, - virNetworkObjPtr network) { - int prefix = virNetworkDefPrefix(network->def); + virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) { + int prefix = virNetworkIpDefPrefix(ipdef); if (prefix < 0) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -722,7 +724,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, /* allow routing packets from the bridge interface */ if (iptablesAddForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev) < 0) { @@ -734,7 +736,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, /* allow routing packets to the bridge interface */ if (iptablesAddForwardAllowIn(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev) < 0) { @@ -749,7 +751,7 @@ networkAddRoutingIptablesRules(struct network_driver *driver, routeerr2: iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); @@ -759,7 +761,8 @@ networkAddRoutingIptablesRules(struct network_driver *driver, static int networkAddIptablesRules(struct network_driver *driver, - virNetworkObjPtr network) { + virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) { /* allow DHCP requests through to dnsmasq */ if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0) { @@ -792,7 +795,7 @@ networkAddIptablesRules(struct network_driver *driver, } /* allow TFTP requests through to dnsmasq */ - if (network->def->tftproot && + if (ipdef && ipdef->tftproot && iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow TFTP requests from '%s'"), @@ -825,29 +828,30 @@ networkAddIptablesRules(struct network_driver *driver, goto err7; } - - /* If masquerading is enabled, set up the rules*/ - if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT && - networkAddMasqueradingIptablesRules(driver, network) < 0) - goto err8; - /* else if routing is enabled, set up the rules*/ - else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE && - networkAddRoutingIptablesRules(driver, network) < 0) - goto err8; - - /* If we are doing local DHCP service on this network, attempt to - * add a rule that will fixup the checksum of DHCP response - * packets back to the guests (but report failure without - * aborting, since not all iptables implementations support it). - */ - - if ((VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) || - network->def->nranges) && - (iptablesAddOutputFixUdpChecksum(driver->iptables, - network->def->bridge, 68) < 0)) { - VIR_WARN("Could not add rule to fixup DHCP response checksums " - "on network '%s'.", network->def->name); - VIR_WARN0("May need to update iptables package & kernel to support CHECKSUM rule."); + if (ipdef) { + /* If masquerading is enabled, set up the rules*/ + if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT && + networkAddMasqueradingIptablesRules(driver, network, ipdef) < 0) + goto err8; + /* else if routing is enabled, set up the rules*/ + else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE && + networkAddRoutingIptablesRules(driver, network, ipdef) < 0) + goto err8; + + /* If we are doing local DHCP service on this network, attempt to + * add a rule that will fixup the checksum of DHCP response + * packets back to the guests (but report failure without + * aborting, since not all iptables implementations support it). + */ + + if (ipdef && (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET) || + ipdef->nranges) && + (iptablesAddOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68) < 0)) { + VIR_WARN("Could not add rule to fixup DHCP response checksums " + "on network '%s'.", network->def->name); + VIR_WARN0("May need to update iptables package & kernel to support CHECKSUM rule."); + } } return 0; @@ -862,7 +866,7 @@ networkAddIptablesRules(struct network_driver *driver, iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); err5: - if (network->def->tftproot) { + if (ipdef && ipdef->tftproot) { iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69); } err4tftp: @@ -879,14 +883,16 @@ networkAddIptablesRules(struct network_driver *driver, static void networkRemoveIptablesRules(struct network_driver *driver, - virNetworkObjPtr network) { - if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) || - network->def->nranges) { + virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) { + + if (ipdef && (VIR_SOCKET_HAS_ADDR(&ipdef->address) || + ipdef->nranges)) { iptablesRemoveOutputFixUdpChecksum(driver->iptables, network->def->bridge, 68); } - if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { - int prefix = virNetworkDefPrefix(network->def); + if (ipdef && network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { + int prefix = virNetworkIpDefPrefix(ipdef); if (prefix < 0) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -897,34 +903,35 @@ networkRemoveIptablesRules(struct network_driver *driver, if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->forwardDev, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); - } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) + } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { iptablesRemoveForwardAllowIn(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); + } iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->ipAddress, + &ipdef->address, prefix, network->def->bridge, network->def->forwardDev); @@ -933,7 +940,7 @@ error: iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge); iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); - if (network->def->tftproot) + if (ipdef && ipdef->tftproot) iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69); iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); @@ -952,10 +959,18 @@ networkReloadIptablesRules(struct network_driver *driver) virNetworkObjLock(driver->networks.objs[i]); if (virNetworkObjIsActive(driver->networks.objs[i])) { - networkRemoveIptablesRules(driver, driver->networks.objs[i]); - if (networkAddIptablesRules(driver, driver->networks.objs[i]) < 0) { - /* failed to add but already logged */ - } + virNetworkIpDefPtr ipv4def; + + /* Find the one allowed IPv4 ip address in the definition */ + /* Even if none is found, we still call the functions below */ + ipv4def = virNetworkDefGetIpByIndex(driver->networks.objs[i]->def, + AF_INET, 0); + networkRemoveIptablesRules(driver, driver->networks.objs[i], + ipv4def); + if (networkAddIptablesRules(driver, driver->networks.objs[i], + ipv4def) < 0) { + /* failed to add but already logged */ + } } virNetworkObjUnlock(driver->networks.objs[i]); @@ -1029,7 +1044,8 @@ cleanup: * other scenarios where we can ruin host network connectivity. * XXX: Using a proper library is preferred over parsing /proc */ -static int networkCheckRouteCollision(virNetworkObjPtr network) +static int networkCheckRouteCollision(virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) { int ret = -1, len; unsigned int net_dest; @@ -1037,18 +1053,18 @@ static int networkCheckRouteCollision(virNetworkObjPtr network) char *cur, *buf = NULL; enum {MAX_ROUTE_SIZE = 1024*64}; - if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { + if (!VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) { /* Only support collision check for IPv4 */ return 0; } - if (virNetworkDefNetmask(network->def, &netmask) < 0) { + if (virNetworkIpDefNetmask(ipdef, &netmask) < 0) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get netmask of '%s'"), network->def->bridge); } - net_dest = (network->def->ipAddress.data.inet4.sin_addr.s_addr & + net_dest = (ipdef->address.data.inet4.sin_addr.s_addr & netmask.data.inet4.sin_addr.s_addr); /* Read whole routing table into memory */ @@ -1122,6 +1138,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver, { int err; virErrorPtr save_err; + virNetworkIpDefPtr ipv4def; if (virNetworkObjIsActive(network)) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -1129,8 +1146,11 @@ static int networkStartNetworkDaemon(struct network_driver *driver, return -1; } + /* find the one allowed IPv4 ip address in the definition */ + ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0); + /* Check to see if network collides with an existing route */ - if (networkCheckRouteCollision(network) < 0) + if (ipv4def && networkCheckRouteCollision(network, ipv4def) < 0) return -1; if ((err = brAddBridge(driver->brctl, network->def->bridge))) { @@ -1159,8 +1179,8 @@ static int networkStartNetworkDaemon(struct network_driver *driver, goto err_delbr; } - if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) { - int prefix = virNetworkDefPrefix(network->def); + if (ipv4def) { + int prefix = virNetworkIpDefPrefix(ipv4def); if (prefix < 0) { networkReportError(VIR_ERR_INTERNAL_ERROR, @@ -1170,7 +1190,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver, } if ((err = brAddInetAddress(driver->brctl, network->def->bridge, - &network->def->ipAddress, prefix))) { + &ipv4def->address, prefix))) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("cannot set IP address on bridge '%s'"), network->def->bridge); @@ -1185,7 +1205,7 @@ static int networkStartNetworkDaemon(struct network_driver *driver, goto err_delbr; } - if (networkAddIptablesRules(driver, network) < 0) + if (ipv4def && networkAddIptablesRules(driver, network, ipv4def) < 0) goto err_delbr1; if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && @@ -1195,12 +1215,13 @@ static int networkStartNetworkDaemon(struct network_driver *driver, goto err_delbr2; } - if ((VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) || - network->def->nranges) && - dhcpStartDhcpDaemon(network) < 0) + /* + * Start the dhcp daemon for the 1st (and only supported) ipv4 + * address. + */ + if (ipv4def && dhcpStartDhcpDaemon(network, ipv4def) < 0) goto err_delbr2; - /* Persist the live configuration now we have bridge info */ if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) { goto err_kill; @@ -1218,7 +1239,8 @@ static int networkStartNetworkDaemon(struct network_driver *driver, err_delbr2: save_err = virSaveLastError(); - networkRemoveIptablesRules(driver, network); + if (ipv4def) + networkRemoveIptablesRules(driver, network, ipv4def); if (save_err) { virSetError(save_err); virFreeError(save_err); @@ -1247,6 +1269,7 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver, { int err; char *stateFile; + virNetworkIpDefPtr ipv4def; VIR_INFO(_("Shutting down network '%s'"), network->def->name); @@ -1263,7 +1286,10 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver, if (network->dnsmasqPid > 0) kill(network->dnsmasqPid, SIGTERM); - networkRemoveIptablesRules(driver, network); + /* find the one allowed IPv4 ip address in the definition */ + ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0); + if (ipv4def) + networkRemoveIptablesRules(driver, network, ipv4def); char ebuf[1024]; if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { @@ -1527,6 +1553,7 @@ cleanup: static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { struct network_driver *driver = conn->networkPrivateData; + virNetworkIpDefPtr ipv4def; virNetworkDefPtr def; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; @@ -1557,12 +1584,15 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { goto cleanup; } - if (network->def->nhosts > 0) { + /* we only support dhcp on one IPv4 address per defined network */ + ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0); + + if (ipv4def && ipv4def->nhosts > 0) { dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); if (dctx == NULL) goto cleanup; - networkSaveDnsmasqHostsfile(network, dctx, true); + networkSaveDnsmasqHostsfile(ipv4def, dctx, true); dnsmasqContextFree(dctx); } @@ -1578,7 +1608,8 @@ cleanup: static int networkUndefine(virNetworkPtr net) { struct network_driver *driver = net->conn->networkPrivateData; - virNetworkObjPtr network = NULL; + virNetworkObjPtr network; + virNetworkIpDefPtr ipv4def; int ret = -1; networkDriverLock(driver); @@ -1601,7 +1632,10 @@ static int networkUndefine(virNetworkPtr net) { network) < 0) goto cleanup; - if (network->def->nhosts > 0) { + /* find the one allowed IPv4 ip address in the definition */ + ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, 0); + + if (ipv4def && ipv4def->nhosts > 0) { dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); if (dctx == NULL) goto cleanup; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 728c501..03defb5 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -7038,9 +7038,23 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char * IHostNetworkInterface *networkInterface = NULL; virNetworkDefPtr def = virNetworkDefParseString(xml); + virNetworkIpDefPtr ipdef; + virSocketAddr netmask; if ( (!def) - || (def->forwardType != VIR_NETWORK_FORWARD_NONE)) + || (def->forwardType != VIR_NETWORK_FORWARD_NONE) + || (def->nips == 0 || !def->ips)) + goto cleanup; + + /* Look for the first IPv4 IP address definition and use that. + * If there weren't any IPv4 addresses, ignore the network (since it's + * required below to have an IPv4 address) + */ + ipdef = virNetworkDefGetIpByIndex(def, AF_INET, 0); + if (!ipdef) + goto cleanup; + + if (virNetworkIpDefNetmask(ipdef, &netmask) < 0) goto cleanup; /* the current limitation of hostonly network is that you can't @@ -7096,9 +7110,9 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char * /* Currently support only one dhcp server per network * with contigious address space from start to end */ - if ((def->nranges >= 1) && - VIR_SOCKET_HAS_ADDR(&def->ranges[0].start) && - VIR_SOCKET_HAS_ADDR(&def->ranges[0].end)) { + if ((ipdef->nranges >= 1) && + VIR_SOCKET_HAS_ADDR(&ipdef->ranges[0].start) && + VIR_SOCKET_HAS_ADDR(&ipdef->ranges[0].end)) { IDHCPServer *dhcpServer = NULL; data->vboxObj->vtbl->FindDHCPServerByNetworkName(data->vboxObj, @@ -7118,10 +7132,10 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char * PRUnichar *toIPAddressUtf16 = NULL; PRUnichar *trunkTypeUtf16 = NULL; - ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &def->ipAddress); - networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &def->netmask); - fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &def->ranges[0].start); - toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &def->ranges[0].end); + ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->address); + networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask); + fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->ranges[0].start); + toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->ranges[0].end); if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL || fromIPAddressUtf16 == NULL || toIPAddressUtf16 == NULL) { @@ -7158,13 +7172,13 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char * } } - if ((def->nhosts >= 1) && - VIR_SOCKET_HAS_ADDR(&def->hosts[0].ip)) { + if ((ipdef->nhosts >= 1) && + VIR_SOCKET_HAS_ADDR(&ipdef->hosts[0].ip)) { PRUnichar *ipAddressUtf16 = NULL; PRUnichar *networkMaskUtf16 = NULL; - ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &def->hosts[0].ip); - networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &def->netmask); + ipAddressUtf16 = vboxSocketFormatAddrUtf16(data, &ipdef->hosts[0].ip); + networkMaskUtf16 = vboxSocketFormatAddrUtf16(data, &netmask); if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL) { VBOX_UTF16_FREE(ipAddressUtf16); @@ -7385,12 +7399,19 @@ static int vboxNetworkDestroy(virNetworkPtr network) { static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSED) { VBOX_OBJECT_HOST_CHECK(network->conn, char *, NULL); virNetworkDefPtr def = NULL; + virNetworkIpDefPtr ipdef = NULL; char *networkNameUtf8 = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); goto cleanup; } + if (VIR_ALLOC(ipdef) < 0) { + virReportOOMError(); + goto cleanup; + } + def->ips = ipdef; + def->nips = 1; if (virAsprintf(&networkNameUtf8, "HostInterfaceNetworking-%s", network->name) < 0) { virReportOOMError(); @@ -7427,8 +7448,8 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE networkNameUtf16, &dhcpServer); if (dhcpServer) { - def->nranges = 1; - if (VIR_ALLOC_N(def->ranges, def->nranges) >=0 ) { + ipdef->nranges = 1; + if (VIR_ALLOC_N(ipdef->ranges, ipdef->nranges) >=0 ) { PRUnichar *ipAddressUtf16 = NULL; PRUnichar *networkMaskUtf16 = NULL; PRUnichar *fromIPAddressUtf16 = NULL; @@ -7443,13 +7464,13 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE * with contigious address space from start to end */ if (vboxSocketParseAddrUtf16(data, ipAddressUtf16, - &def->ipAddress) < 0 || + &ipdef->address) < 0 || vboxSocketParseAddrUtf16(data, networkMaskUtf16, - &def->netmask) < 0 || + &ipdef->netmask) < 0 || vboxSocketParseAddrUtf16(data, fromIPAddressUtf16, - &def->ranges[0].start) < 0 || + &ipdef->ranges[0].start) < 0 || vboxSocketParseAddrUtf16(data, toIPAddressUtf16, - &def->ranges[0].end) < 0) { + &ipdef->ranges[0].end) < 0) { errorOccurred = true; } @@ -7462,16 +7483,16 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE goto cleanup; } } else { - def->nranges = 0; + ipdef->nranges = 0; virReportOOMError(); } - def->nhosts = 1; - if (VIR_ALLOC_N(def->hosts, def->nhosts) >=0 ) { - def->hosts[0].name = strdup(network->name); - if (def->hosts[0].name == NULL) { - VIR_FREE(def->hosts); - def->nhosts = 0; + ipdef->nhosts = 1; + if (VIR_ALLOC_N(ipdef->hosts, ipdef->nhosts) >=0 ) { + ipdef->hosts[0].name = strdup(network->name); + if (ipdef->hosts[0].name == NULL) { + VIR_FREE(ipdef->hosts); + ipdef->nhosts = 0; virReportOOMError(); } else { PRUnichar *macAddressUtf16 = NULL; @@ -7481,10 +7502,10 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE networkInterface->vtbl->GetHardwareAddress(networkInterface, &macAddressUtf16); networkInterface->vtbl->GetIPAddress(networkInterface, &ipAddressUtf16); - VBOX_UTF16_TO_UTF8(macAddressUtf16, &def->hosts[0].mac); + VBOX_UTF16_TO_UTF8(macAddressUtf16, &ipdef->hosts[0].mac); if (vboxSocketParseAddrUtf16(data, ipAddressUtf16, - &def->hosts[0].ip) < 0) { + &ipdef->hosts[0].ip) < 0) { errorOccurred = true; } @@ -7496,7 +7517,7 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE } } } else { - def->nhosts = 0; + ipdef->nhosts = 0; } VBOX_RELEASE(dhcpServer); @@ -7509,9 +7530,9 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE networkInterface->vtbl->GetIPAddress(networkInterface, &ipAddressUtf16); if (vboxSocketParseAddrUtf16(data, networkMaskUtf16, - &def->netmask) < 0 || + &ipdef->netmask) < 0 || vboxSocketParseAddrUtf16(data, ipAddressUtf16, - &def->ipAddress) < 0) { + &ipdef->address) < 0) { errorOccurred = true; } diff --git a/tests/networkxml2xmlin/nat-network.xml b/tests/networkxml2xmlin/nat-network.xml index 93ab186..23f7fcb 100644 --- a/tests/networkxml2xmlin/nat-network.xml +++ b/tests/networkxml2xmlin/nat-network.xml @@ -10,4 +10,12 @@ <host mac="00:16:3e:3e:a9:1a" name="b.example.com" ip="192.168.122.11" /> </dhcp> </ip> + <ip family="ipv4" address="192.168.123.1" netmask="255.255.255.0"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fe01::1" prefix="64"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fd01::1" prefix="64"> + </ip> + <ip family="ipv4" address="10.24.10.1"> + </ip> </network> diff --git a/tests/networkxml2xmlout/nat-network.xml b/tests/networkxml2xmlout/nat-network.xml index 036d4fb..eb71d9e 100644 --- a/tests/networkxml2xmlout/nat-network.xml +++ b/tests/networkxml2xmlout/nat-network.xml @@ -10,4 +10,12 @@ <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11' /> </dhcp> </ip> + <ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'> + </ip> + <ip family='ipv4' address='10.24.10.1'> + </ip> </network> -- 1.7.3.4

All of the iptables functions eventually call down to a single bottom-level function, and fortunately, ip6tables syntax (for all the args that we use) is identical to iptables format (except the addresses), so all we need to do is: 1) Get an address family down to the lowest level function in each case, either implied through an address, or explicitly when no address is in the parameter list, and 2) At the lowest level, just decide whether to call "iptables" or "ip6tables" based on the family. The location of the ip6tables binary is determined at build time by autoconf. If a particular target system happens to not have ip6tables installed, any attempts to run it will generate an error, but that won't happen unless someone tries to define an IPv6 address for a network. This is identical behavior to IPv4 addresses and iptables. --- configure.ac | 3 ++ src/network/bridge_driver.c | 54 ++++++++++++++++++------------- src/util/iptables.c | 75 +++++++++++++++++++++++++++++++----------- src/util/iptables.h | 10 ++++++ 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/configure.ac b/configure.ac index 357d59b..279ccd2 100644 --- a/configure.ac +++ b/configure.ac @@ -312,6 +312,9 @@ AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary]) AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary]) +AC_PATH_PROG([IP6TABLES_PATH], [ip6tables], /sbin/ip6tables, [/usr/sbin:$PATH]) +AC_DEFINE_UNQUOTED([IP6TABLES_PATH], "$IP6TABLES_PATH", [path to ip6tables binary]) + AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables binary]) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 606dde3..e405429 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -843,14 +843,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver, /* allow DHCP requests through to dnsmasq */ - if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 67) < 0) { + if (iptablesAddTcpInput(driver->iptables, AF_INET, + network->def->bridge, 67) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DHCP requests from '%s'"), network->def->bridge); goto err1; } - if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 67) < 0) { + if (iptablesAddUdpInput(driver->iptables, AF_INET, + network->def->bridge, 67) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DHCP requests from '%s'"), network->def->bridge); @@ -872,14 +874,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver, } /* allow DNS requests through to dnsmasq */ - if (iptablesAddTcpInput(driver->iptables, network->def->bridge, 53) < 0) { + if (iptablesAddTcpInput(driver->iptables, AF_INET, + network->def->bridge, 53) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DNS requests from '%s'"), network->def->bridge); goto err3; } - if (iptablesAddUdpInput(driver->iptables, network->def->bridge, 53) < 0) { + if (iptablesAddUdpInput(driver->iptables, AF_INET, + network->def->bridge, 53) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DNS requests from '%s'"), network->def->bridge); @@ -888,7 +892,8 @@ networkAddGeneralIptablesRules(struct network_driver *driver, /* allow TFTP requests through to dnsmasq if necessary */ if (ipv4def && ipv4def->tftproot && - iptablesAddUdpInput(driver->iptables, network->def->bridge, 69) < 0) { + iptablesAddUdpInput(driver->iptables, AF_INET, + network->def->bridge, 69) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow TFTP requests from '%s'"), network->def->bridge); @@ -897,14 +902,16 @@ networkAddGeneralIptablesRules(struct network_driver *driver, /* Catch all rules to block forwarding to/from bridges */ - if (iptablesAddForwardRejectOut(driver->iptables, network->def->bridge) < 0) { + if (iptablesAddForwardRejectOut(driver->iptables, AF_INET, + network->def->bridge) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to block outbound traffic from '%s'"), network->def->bridge); goto err6; } - if (iptablesAddForwardRejectIn(driver->iptables, network->def->bridge) < 0) { + if (iptablesAddForwardRejectIn(driver->iptables, AF_INET, + network->def->bridge) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to block inbound traffic to '%s'"), network->def->bridge); @@ -912,7 +919,8 @@ networkAddGeneralIptablesRules(struct network_driver *driver, } /* Allow traffic between guests on the same bridge */ - if (iptablesAddForwardAllowCross(driver->iptables, network->def->bridge) < 0) { + if (iptablesAddForwardAllowCross(driver->iptables, AF_INET, + network->def->bridge) < 0) { networkReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow cross bridge traffic on '%s'"), network->def->bridge); @@ -923,21 +931,21 @@ networkAddGeneralIptablesRules(struct network_driver *driver, /* unwind in reverse order from the point of failure */ err8: - iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); + iptablesRemoveForwardRejectIn(driver->iptables, AF_INET, network->def->bridge); err7: - iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); + iptablesRemoveForwardRejectOut(driver->iptables, AF_INET, network->def->bridge); err6: if (ipv4def && ipv4def->tftproot) { - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 69); } err5: - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 53); err4: - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); + iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge, 53); err3: - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 67); err2: - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); + iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge, 67); err1: return -1; } @@ -956,20 +964,20 @@ networkRemoveGeneralIptablesRules(struct network_driver *driver, break; } - iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge); - iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); - iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); + iptablesRemoveForwardAllowCross(driver->iptables, AF_INET, network->def->bridge); + iptablesRemoveForwardRejectIn(driver->iptables, AF_INET, network->def->bridge); + iptablesRemoveForwardRejectOut(driver->iptables, AF_INET, network->def->bridge); if (ipv4def && ipv4def->tftproot) { - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 69); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 69); } - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 53); + iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge, 53); if (ipv4def && (ipv4def->nranges || ipv4def->nhosts)) { iptablesRemoveOutputFixUdpChecksum(driver->iptables, network->def->bridge, 68); } - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); + iptablesRemoveUdpInput(driver->iptables, AF_INET, network->def->bridge, 67); + iptablesRemoveTcpInput(driver->iptables, AF_INET, network->def->bridge, 67); } static int diff --git a/src/util/iptables.c b/src/util/iptables.c index 89e05b7..c3e18cb 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -99,14 +99,17 @@ iptRulesNew(const char *table, } static int ATTRIBUTE_SENTINEL -iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) +iptablesAddRemoveRule(iptRules *rules, int family, int action, + const char *arg, ...) { va_list args; int ret; virCommandPtr cmd; const char *s; - cmd = virCommandNew(IPTABLES_PATH); + cmd = virCommandNew((family == AF_INET6) + ? IP6TABLES_PATH : IPTABLES_PATH); + virCommandAddArgList(cmd, "--table", rules->table, action == ADD ? "--insert" : "--delete", rules->chain, arg, NULL); @@ -177,6 +180,7 @@ iptablesContextFree(iptablesContext *ctx) static int iptablesInput(iptablesContext *ctx, + int family, const char *iface, int port, int action, @@ -188,6 +192,7 @@ iptablesInput(iptablesContext *ctx, portstr[sizeof(portstr) - 1] = '\0'; return iptablesAddRemoveRule(ctx->input_filter, + family, action, "--in-interface", iface, "--protocol", tcp ? "tcp" : "udp", @@ -210,10 +215,11 @@ iptablesInput(iptablesContext *ctx, int iptablesAddTcpInput(iptablesContext *ctx, + int family, const char *iface, int port) { - return iptablesInput(ctx, iface, port, ADD, 1); + return iptablesInput(ctx, family, iface, port, ADD, 1); } /** @@ -229,10 +235,11 @@ iptablesAddTcpInput(iptablesContext *ctx, */ int iptablesRemoveTcpInput(iptablesContext *ctx, + int family, const char *iface, int port) { - return iptablesInput(ctx, iface, port, REMOVE, 1); + return iptablesInput(ctx, family, iface, port, REMOVE, 1); } /** @@ -249,10 +256,11 @@ iptablesRemoveTcpInput(iptablesContext *ctx, int iptablesAddUdpInput(iptablesContext *ctx, + int family, const char *iface, int port) { - return iptablesInput(ctx, iface, port, ADD, 0); + return iptablesInput(ctx, family, iface, port, ADD, 0); } /** @@ -268,10 +276,11 @@ iptablesAddUdpInput(iptablesContext *ctx, */ int iptablesRemoveUdpInput(iptablesContext *ctx, + int family, const char *iface, int port) { - return iptablesInput(ctx, iface, port, REMOVE, 0); + return iptablesInput(ctx, family, iface, port, REMOVE, 0); } @@ -282,9 +291,10 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, char *netstr; char *ret; - if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET)) { + if (!(VIR_SOCKET_IS_FAMILY(netaddr, AF_INET) || + VIR_SOCKET_IS_FAMILY(netaddr, AF_INET6))) { iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only IPv4 addresses can be used with iptables")); + _("Only IPv4 or IPv6 addresses can be used with iptables")); return NULL; } @@ -327,6 +337,7 @@ iptablesForwardAllowOut(iptablesContext *ctx, if (physdev && physdev[0]) { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "--in-interface", iface, @@ -335,6 +346,7 @@ iptablesForwardAllowOut(iptablesContext *ctx, NULL); } else { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "--in-interface", iface, @@ -411,6 +423,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx, if (physdev && physdev[0]) { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--destination", networkstr, "--in-interface", physdev, @@ -421,6 +434,7 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx, NULL); } else { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--destination", networkstr, "--out-interface", iface, @@ -497,6 +511,7 @@ iptablesForwardAllowIn(iptablesContext *ctx, if (physdev && physdev[0]) { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--destination", networkstr, "--in-interface", physdev, @@ -505,6 +520,7 @@ iptablesForwardAllowIn(iptablesContext *ctx, NULL); } else { ret = iptablesAddRemoveRule(ctx->forward_filter, + VIR_SOCKET_FAMILY(netaddr), action, "--destination", networkstr, "--out-interface", iface, @@ -567,10 +583,12 @@ iptablesRemoveForwardAllowIn(iptablesContext *ctx, */ static int iptablesForwardAllowCross(iptablesContext *ctx, + int family, const char *iface, int action) { return iptablesAddRemoveRule(ctx->forward_filter, + family, action, "--in-interface", iface, "--out-interface", iface, @@ -591,8 +609,10 @@ iptablesForwardAllowCross(iptablesContext *ctx, */ int iptablesAddForwardAllowCross(iptablesContext *ctx, - const char *iface) { - return iptablesForwardAllowCross(ctx, iface, ADD); + int family, + const char *iface) +{ + return iptablesForwardAllowCross(ctx, family, iface, ADD); } /** @@ -608,8 +628,10 @@ iptablesAddForwardAllowCross(iptablesContext *ctx, */ int iptablesRemoveForwardAllowCross(iptablesContext *ctx, - const char *iface) { - return iptablesForwardAllowCross(ctx, iface, REMOVE); + int family, + const char *iface) +{ + return iptablesForwardAllowCross(ctx, family, iface, REMOVE); } @@ -618,14 +640,16 @@ iptablesRemoveForwardAllowCross(iptablesContext *ctx, */ static int iptablesForwardRejectOut(iptablesContext *ctx, + int family, const char *iface, int action) { return iptablesAddRemoveRule(ctx->forward_filter, - action, - "--in-interface", iface, - "--jump", "REJECT", - NULL); + family, + action, + "--in-interface", iface, + "--jump", "REJECT", + NULL); } /** @@ -640,9 +664,10 @@ iptablesForwardRejectOut(iptablesContext *ctx, */ int iptablesAddForwardRejectOut(iptablesContext *ctx, + int family, const char *iface) { - return iptablesForwardRejectOut(ctx, iface, ADD); + return iptablesForwardRejectOut(ctx, family, iface, ADD); } /** @@ -657,9 +682,10 @@ iptablesAddForwardRejectOut(iptablesContext *ctx, */ int iptablesRemoveForwardRejectOut(iptablesContext *ctx, + int family, const char *iface) { - return iptablesForwardRejectOut(ctx, iface, REMOVE); + return iptablesForwardRejectOut(ctx, family, iface, REMOVE); } @@ -670,10 +696,12 @@ iptablesRemoveForwardRejectOut(iptablesContext *ctx, */ static int iptablesForwardRejectIn(iptablesContext *ctx, + int family, const char *iface, int action) { return iptablesAddRemoveRule(ctx->forward_filter, + family, action, "--out-interface", iface, "--jump", "REJECT", @@ -692,9 +720,10 @@ iptablesForwardRejectIn(iptablesContext *ctx, */ int iptablesAddForwardRejectIn(iptablesContext *ctx, + int family, const char *iface) { - return iptablesForwardRejectIn(ctx, iface, ADD); + return iptablesForwardRejectIn(ctx, family, iface, ADD); } /** @@ -709,9 +738,10 @@ iptablesAddForwardRejectIn(iptablesContext *ctx, */ int iptablesRemoveForwardRejectIn(iptablesContext *ctx, + int family, const char *iface) { - return iptablesForwardRejectIn(ctx, iface, REMOVE); + return iptablesForwardRejectIn(ctx, family, iface, REMOVE); } @@ -735,6 +765,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (protocol && protocol[0]) { if (physdev && physdev[0]) { ret = iptablesAddRemoveRule(ctx->nat_postrouting, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "-p", protocol, @@ -745,6 +776,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, NULL); } else { ret = iptablesAddRemoveRule(ctx->nat_postrouting, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "-p", protocol, @@ -756,6 +788,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, } else { if (physdev && physdev[0]) { ret = iptablesAddRemoveRule(ctx->nat_postrouting, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "!", "--destination", networkstr, @@ -764,6 +797,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, NULL); } else { ret = iptablesAddRemoveRule(ctx->nat_postrouting, + VIR_SOCKET_FAMILY(netaddr), action, "--source", networkstr, "!", "--destination", networkstr, @@ -834,6 +868,7 @@ iptablesOutputFixUdpChecksum(iptablesContext *ctx, portstr[sizeof(portstr) - 1] = '\0'; return iptablesAddRemoveRule(ctx->mangle_postrouting, + AF_INET, action, "--out-interface", iface, "--protocol", "udp", diff --git a/src/util/iptables.h b/src/util/iptables.h index 6f59b92..8088c8e 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -30,16 +30,20 @@ iptablesContext *iptablesContextNew (void); void iptablesContextFree (iptablesContext *ctx); int iptablesAddTcpInput (iptablesContext *ctx, + int family, const char *iface, int port); int iptablesRemoveTcpInput (iptablesContext *ctx, + int family, const char *iface, int port); int iptablesAddUdpInput (iptablesContext *ctx, + int family, const char *iface, int port); int iptablesRemoveUdpInput (iptablesContext *ctx, + int family, const char *iface, int port); @@ -77,18 +81,24 @@ int iptablesRemoveForwardAllowIn (iptablesContext *ctx, const char *physdev); int iptablesAddForwardAllowCross (iptablesContext *ctx, + int family, const char *iface); int iptablesRemoveForwardAllowCross (iptablesContext *ctx, + int family, const char *iface); int iptablesAddForwardRejectOut (iptablesContext *ctx, + int family, const char *iface); int iptablesRemoveForwardRejectOut (iptablesContext *ctx, + int family, const char *iface); int iptablesAddForwardRejectIn (iptablesContext *ctx, + int family, const char *iface); int iptablesRemoveForwardRejectIn (iptablesContext *ctx, + int family, const char *iface); int iptablesAddForwardMasquerade (iptablesContext *ctx, -- 1.7.3.4

Running an instance of the router advertisement daemon (radvd) allows guests using the virtual network to automatically acquire and IPv6 address and default route. Note that acquiring an address only works for networks with a prefix length of exactly 64 - radvd is still run in other circumstances, and still advertises routes, but autoconf will not work because it requires exactly 64 bits of address info from the network prefix. This patch should not be committed as-is, because there is a race condition with the pidfile that I still need to remedy. Unlike dnsmasq, radvd does not guarantee that its pidfile has been written before its original process exits; instead it uses daemon(3) to fork and exit the parent process in a single step, then writes the pidfile later. The result is that libvirtd tries to read the pidfile before it's been created, and ends up not having the proper pid to use when it later tries to kill radvd. There are two possible solutions for this: 1) Don't attempt to immediately read the pidfile and store the pid in memory. Instead, just read the pidfile later when we want to kill radvd. (This could still lead to a race if networkStart and networkDestroy were called in tight sequence by some application) 2) Don't allow radvd to daemonize itself. Instead, run it with "-d 1" (setting a debuglevel > 0 implies that it should not daemonize), and use virCommand's own pidfile/daemonize functions. (The problem here is that there's no way to tell radvd to not write a pidfile itself, so we would end up with 2 pidfiles for each instance. This isn't a functional problem, just cosmetic). Another unresolved(?) problem is that the location of the radvd binary is found by autoconf during the build, so if it's not present on the build machine. Should this be handled by specfiles on specific distros adding a BuildRequires for the radvd package, or can/should more be done in the libvirt tree? (Current behavior is identical to dnsmasq, so I guess it's acceptable, as long as all the downstream builders are made aware of the need for radvd for proper IPv6 support). --- configure.ac | 4 + src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 232 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 216 insertions(+), 21 deletions(-) diff --git a/configure.ac b/configure.ac index 279ccd2..231d0c7 100644 --- a/configure.ac +++ b/configure.ac @@ -135,6 +135,8 @@ dnl detect them, in which case we'll search for the program dnl along the $PATH at runtime and fail if it's not there. AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([RADVD], [radvd], [radvd], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([BRCTL], [brctl], [brctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([UDEVADM], [udevadm], [], @@ -146,6 +148,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [], AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"], [Location or name of the dnsmasq program]) +AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], + [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"], [Location or name of the brctl program (see bridge-utils)]) if test -n "$UDEVADM"; then diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 47f0408..ca5784e 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -107,6 +107,7 @@ struct _virNetworkObj { virMutex lock; pid_t dnsmasqPid; + pid_t radvdPid; unsigned int active : 1; unsigned int autostart : 1; unsigned int persistent : 1; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 665a13f..37f0eed 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -65,6 +65,7 @@ #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network" #define DNSMASQ_STATE_DIR LOCALSTATEDIR "/lib/libvirt/dnsmasq" +#define RADVD_STATE_DIR LOCALSTATEDIR "/lib/libvirt/radvd" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -107,6 +108,25 @@ static void networkReloadIptablesRules(struct network_driver *driver); static struct network_driver *driverState = NULL; +static char * +networkRadvdPidfileBasename(const char *netname) +{ + /* this is simple but we want to be sure it's consistently done */ + char *pidfilebase; + + virAsprintf(&pidfilebase, "%s-radvd", netname); + return pidfilebase; +} + +static char * +networkRadvdConfigFileName(const char *netname) +{ + char *configfile; + + virAsprintf(&configfile, "%s/%s-radvd.conf", + RADVD_STATE_DIR, netname); + return configfile; +} static void networkFindActiveConfigs(struct network_driver *driver) { @@ -144,26 +164,43 @@ networkFindActiveConfigs(struct network_driver *driver) { brHasBridge(driver->brctl, obj->def->bridge) == 0) { obj->active = 1; - /* Finally try and read dnsmasq pid if any */ - if (obj->def->ips && (obj->def->nips > 0) && - virFileReadPid(NETWORK_PID_DIR, obj->def->name, - &obj->dnsmasqPid) == 0) { - - /* Check its still alive */ - if (kill(obj->dnsmasqPid, 0) != 0) - obj->dnsmasqPid = -1; - -#ifdef __linux__ - char *pidpath; + /* Try and read dnsmasq/radvd pids if any */ + if (obj->def->ips && (obj->def->nips > 0)) { + char *pidpath, *radvdpidbase; + + if (virFileReadPid(NETWORK_PID_DIR, obj->def->name, + &obj->dnsmasqPid) == 0) { + /* Check that it's still alive */ + if (kill(obj->dnsmasqPid, 0) != 0) + obj->dnsmasqPid = -1; + if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0) + obj->dnsmasqPid = -1; + VIR_FREE(pidpath); + } - if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) { + if (!(radvdpidbase = networkRadvdPidfileBasename(obj->def->name))) { virReportOOMError(); goto cleanup; } - if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0) - obj->dnsmasqPid = -1; - VIR_FREE(pidpath); -#endif + if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase, + &obj->radvdPid) == 0) { + /* Check that it's still alive */ + if (kill(obj->radvdPid, 0) != 0) + obj->radvdPid = -1; + if (virAsprintf(&pidpath, "/proc/%d/exe", obj->radvdPid) < 0) { + virReportOOMError(); + VIR_FREE(radvdpidbase); + goto cleanup; + } + if (virFileLinkPointsTo(pidpath, RADVD) == 0) + obj->radvdPid = -1; + VIR_FREE(pidpath); + } + VIR_FREE(radvdpidbase); } } @@ -588,6 +625,123 @@ cleanup: } static int +networkStartRadvd(virNetworkObjPtr network) +{ + char *pidfile = NULL; + char *radvdpidbase = NULL; + virBuffer configbuf = VIR_BUFFER_INITIALIZER;; + char *configstr = NULL; + char *configfile = NULL; + virCommandPtr cmd = NULL; + int ret = -1, err, ii; + virNetworkIpDefPtr ipdef; + + network->radvdPid = -1; + + if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { + virReportSystemError(err, + _("cannot create directory %s"), + NETWORK_PID_DIR); + goto cleanup; + } + if ((err = virFileMakePath(RADVD_STATE_DIR)) != 0) { + virReportSystemError(err, + _("cannot create directory %s"), + RADVD_STATE_DIR); + goto cleanup; + } + + /* construct pidfile name */ + if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) { + virReportOOMError(); + goto cleanup; + } + if (!(pidfile = virFilePid(NETWORK_PID_DIR, radvdpidbase))) { + virReportOOMError(); + goto cleanup; + } + + /* create radvd config file appropriate for this network */ + virBufferVSprintf(&configbuf, "interface %s\n" + "{\n" + " AdvSendAdvert on;\n" + " AdvManagedFlag off;\n" + " AdvOtherConfigFlag off;\n" + "\n", + network->def->bridge); + for (ii = 0; + (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii)); + ii++) { + int prefix; + char *netaddr; + + prefix = virNetworkIpDefPrefix(ipdef); + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("bridge '%s' has an invalid prefix"), + network->def->bridge); + goto cleanup; + } + if (!(netaddr = virSocketFormatAddr(&ipdef->address))) + goto cleanup; + virBufferVSprintf(&configbuf, + " prefix %s/%d\n" + " {\n" + " AdvOnLink on;\n" + " AdvAutonomous on;\n" + " AdvRouterAddr off;\n" + " };\n", + netaddr, prefix); + VIR_FREE(netaddr); + } + + virBufferAddLit(&configbuf, "};\n"); + + if (virBufferError(&configbuf)) { + virReportOOMError(); + goto cleanup; + } + if (!(configstr = virBufferContentAndReset(&configbuf))) { + virReportOOMError(); + goto cleanup; + } + + /* construct the filename */ + if (!(configfile = networkRadvdConfigFileName(network->def->name))) { + virReportOOMError(); + goto cleanup; + } + /* write the file */ + if (virFileWriteStr(configfile, configstr, 0600) < 0) { + virReportSystemError(errno, + _("couldn't write radvd config file '%s'"), + configfile); + goto cleanup; + } + + cmd = virCommandNew(RADVD); + virCommandAddArgList(cmd, "--config", configfile, + "--pidfile", pidfile, + NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase, + &network->radvdPid) < 0) + goto cleanup; + + ret = 0; +cleanup: + virCommandFree(cmd); + VIR_FREE(configfile); + VIR_FREE(configstr); + virBufferFreeAndReset(&configbuf); + VIR_FREE(radvdpidbase); + VIR_FREE(pidfile); + return ret; +} + +static int networkAddMasqueradingIptablesRules(struct network_driver *driver, virNetworkObjPtr network, virNetworkIpDefPtr ipdef) @@ -1154,9 +1308,14 @@ networkReloadIptablesRules(struct network_driver *driver) /* Enable IP Forwarding. Return 0 for success, -1 for failure. */ static int -networkEnableIpForwarding(void) +networkEnableIpForwarding(int enableIPv4, int enableIPv6) { - return virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0); + int ret = 0; + if (enableIPv4) + ret = virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0); + if (enableIPv6 && ret == 0) + ret = virFileWriteStr("/proc/sys/net/ipv6/conf/all/forwarding", "1\n", 0); + return ret; } #define SYSCTL_PATH "/proc/sys" @@ -1431,7 +1590,7 @@ networkStartNetworkDaemon(struct network_driver *driver, /* If forwardType != NONE, turn on global IP forwarding */ if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && - networkEnableIpForwarding() < 0) { + networkEnableIpForwarding(v4present, v6present) < 0) { virReportSystemError(errno, "%s", _("failed to enable IP forwarding")); goto err3; @@ -1442,15 +1601,28 @@ networkStartNetworkDaemon(struct network_driver *driver, if (v4present && networkStartDhcpDaemon(network) < 0) goto err3; + /* start radvd if there are any ipv6 addresses */ + if (v6present && networkStartRadvd(network) < 0) + goto err4; + /* Persist the live configuration now we have bridge info */ if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) { - goto err4; + goto err5; } network->active = 1; return 0; + err5: + if (!save_err) + save_err = virSaveLastError(); + + if (network->radvdPid > 0) { + kill(network->radvdPid, SIGTERM); + network->radvdPid = -1; + } + err4: if (!save_err) save_err = virSaveLastError(); @@ -1509,6 +1681,9 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver, unlink(stateFile); VIR_FREE(stateFile); + if (network->radvdPid > 0) + kill(network->radvdPid, SIGTERM); + if (network->dnsmasqPid > 0) kill(network->dnsmasqPid, SIGTERM); @@ -1529,8 +1704,13 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver, if (network->dnsmasqPid > 0 && (kill(network->dnsmasqPid, 0) == 0)) kill(network->dnsmasqPid, SIGKILL); - network->dnsmasqPid = -1; + + if (network->radvdPid > 0 && + (kill(network->radvdPid, 0) == 0)) + kill(network->radvdPid, SIGKILL); + network->radvdPid = -1; + network->active = 0; if (network->newDef) { @@ -1891,6 +2071,16 @@ static int networkUndefine(virNetworkPtr net) { dnsmasqContextFree(dctx); } + if (v6present) { + char *configfile = networkRadvdConfigFileName(network->def->name); + + if (!configfile) { + virReportOOMError(); + goto cleanup; + } + unlink(configfile); + } + virNetworkRemoveInactive(&driver->networks, network); network = NULL; -- 1.7.3.4