[libvirt] [PATCH v5 0/3] net: support set source address(es) and ports for NAT
Fixes the commented nitpicks[1]. I also provide an optional 3rd patch, that uses structs for address and port ranges. There was a struct for DHCP address range that could be used after a rename. I put the structs in virsocketaddr.h so they are accessible from util/viriptables. I don't think it is worth its own headerfile and sockaddr was what I thought was most relevant. Skip the third patch if unsure. Please CC me as i don't subscribe to the list. Changes v5: - Remove unused "longdef" leftover from rebase - UseCamelCaseVariableNames - Use virAsprintf instead of snprintf and avoid strings on stack - Add an optional 3rd patch that uses structs for addr and port ranges. Natanael Copa (3): net: support set public ip range for forward mode nat net: add support for specifying port range for forward mode nat net: use structs for address and port ranges docs/formatnetwork.html.in | 33 +++++++ src/conf/network_conf.c | 206 +++++++++++++++++++++++++++++++++++++++++--- src/conf/network_conf.h | 13 ++- src/network/bridge_driver.c | 16 ++++ src/util/viriptables.c | 83 ++++++++++++++++-- src/util/viriptables.h | 4 + src/util/virsocketaddr.h | 14 +++ 7 files changed, 341 insertions(+), 28 deletions(-) [1] http://www.redhat.com/archives/libvir-list/2013-February/msg00817.html -- 1.8.1.3
Support setting which public ip to use for NAT via attribute address in subelement <nat> in <forward>: ... <forward mode='nat'> <address start='1.2.3.4' end='1.2.3.10'/> </forward> ... This will construct an iptables line using: '-j SNAT --to-source <start>-<end>' instead of: '-j MASQUERADE' Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 18 ++++++ src/conf/network_conf.c | 152 ++++++++++++++++++++++++++++++++++++++++++-- src/conf/network_conf.h | 3 + src/network/bridge_driver.c | 16 +++++ src/util/viriptables.c | 63 +++++++++++++++--- src/util/viriptables.h | 4 ++ 6 files changed, 242 insertions(+), 14 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7b42529..5fbd0a9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -136,6 +136,24 @@ network, and to/from the host to the guests, are unrestricted and not NATed.<span class="since">Since 0.4.2</span> + + <p><span class="since">Since 1.0.3</span> it is possible to + specify a public IPv4 address range to be used for the NAT by + using the <code><nat></code> and + <code><address></code> subelements. + <pre> +... + <forward mode='nat'> + <nat> + <address start='1.2.3.4' end='1.2.3.10'/> + </nat> + </forward> +... + </pre> + An singe IPv4 address can be set by setting + <code>start</code> and <code>end</code> attributes to + the same value. + </p> </dd> <dt><code>route</code></dt> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3604ff7..620110c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1325,6 +1325,80 @@ cleanup: } static int +virNetworkForwardNatDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkForwardDefPtr def) +{ + int ret = -1; + xmlNodePtr *natAddrNodes = NULL; + int nNatAddrs; + char *addrStart = NULL; + char *addrEnd = NULL; + xmlNodePtr save = ctxt->node; + + ctxt->node = node; + + if (def->type != VIR_NETWORK_FORWARD_NAT) { + virReportError(VIR_ERR_XML_ERROR, + _("The <nat> element can only be used when <forward> 'mode' is 'nat' in network %s"), + networkName); + goto cleanup; + } + + /* addresses for SNAT */ + nNatAddrs = virXPathNodeSet("./address", ctxt, &natAddrNodes); + if (nNatAddrs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <address> element found in <forward> of " + "network %s"), networkName); + goto cleanup; + } else if (nNatAddrs > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <address> element is allowed in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } else if (nNatAddrs == 1) { + addrStart = virXMLPropString(*natAddrNodes, "start"); + if (addrStart == NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'start' attribute in <address> element in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } + addrEnd = virXMLPropString(*natAddrNodes, "end"); + if (addrEnd == NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'end' attribute in <address> element in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } + } + + if (addrStart && virSocketAddrParse(&def->addrStart, addrStart, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 start address '%s' in <nat> in <forward> in " + "network '%s'"), addrStart, networkName); + goto cleanup; + } + + if (addrEnd && virSocketAddrParse(&def->addrEnd, addrEnd, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 end address '%s' in <nat> in <forward> in " + "network '%s'"), addrEnd, networkName); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(addrStart); + VIR_FREE(addrEnd); + ctxt->node = save; + return ret; +} + +static int virNetworkForwardDefParseXML(const char *networkName, xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -1334,7 +1408,8 @@ virNetworkForwardDefParseXML(const char *networkName, xmlNodePtr *forwardIfNodes = NULL; xmlNodePtr *forwardPfNodes = NULL; xmlNodePtr *forwardAddrNodes = NULL; - int nForwardIfs, nForwardAddrs, nForwardPfs; + xmlNodePtr *forwardNatNodes = NULL; + int nForwardIfs, nForwardAddrs, nForwardPfs, nForwardNats; char *forwardDev = NULL; char *forwardManaged = NULL; char *type = NULL; @@ -1384,6 +1459,24 @@ virNetworkForwardDefParseXML(const char *networkName, goto cleanup; } + nForwardNats = virXPathNodeSet("./nat", ctxt, &forwardNatNodes); + if (nForwardNats < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <nat> element found in <forward> of network %s"), + networkName); + goto cleanup; + } else if (nForwardNats > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <nat> element is allowed in <forward> of network %s"), + networkName); + goto cleanup; + } else if (nForwardNats == 1) { + if (virNetworkForwardNatDefParseXML(networkName, + *forwardNatNodes, + ctxt, def) < 0) + goto cleanup; + } + if (((nForwardIfs > 0) + (nForwardAddrs > 0) + (nForwardPfs > 0)) > 1) { virReportError(VIR_ERR_XML_ERROR, _("<address>, <interface>, and <pf> elements in <forward> " @@ -1525,6 +1618,7 @@ cleanup: VIR_FREE(forwardPfNodes); VIR_FREE(forwardIfNodes); VIR_FREE(forwardAddrNodes); + VIR_FREE(forwardNatNodes); ctxt->node = save; return ret; } @@ -2079,13 +2173,54 @@ virPortGroupDefFormat(virBufferPtr buf, } static int +virNatDefFormat(virBufferPtr buf, + const virNetworkForwardDefPtr fwd) +{ + char *addrStart = NULL; + char *addrEnd = NULL; + int ret = -1; + + if (VIR_SOCKET_ADDR_VALID(&fwd->addrStart)) { + addrStart = virSocketAddrFormat(&fwd->addrStart); + if (!addrStart) + goto cleanup; + } + + if (VIR_SOCKET_ADDR_VALID(&fwd->addrEnd)) { + addrEnd = virSocketAddrFormat(&fwd->addrEnd); + if (!addrEnd) + goto cleanup; + } + + if (!addrEnd && !addrStart) + return 0; + + virBufferAddLit(buf, "<nat>\n"); + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<address start='%s'", addrStart); + if (addrEnd) + virBufferAsprintf(buf, " end='%s'", addrEnd); + virBufferAsprintf(buf, "/>\n"); + + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</nat>\n"); + ret = 0; + +cleanup: + VIR_FREE(addrStart); + VIR_FREE(addrEnd); + return ret; +} + +static int virNetworkDefFormatInternal(virBufferPtr buf, const virNetworkDefPtr def, unsigned int flags) { unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; - int ii; + int ii, shortforward; virBufferAddLit(buf, "<network"); if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0)) { @@ -2122,10 +2257,17 @@ virNetworkDefFormatInternal(virBufferPtr buf, else virBufferAddLit(buf, " managed='no'"); } - virBufferAsprintf(buf, "%s>\n", - (def->forward.nifs || def->forward.npfs) ? "" : "/"); + shortforward = !(def->forward.nifs || def->forward.npfs + || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart) + || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd)); + virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2); + if (def->forward.type == VIR_NETWORK_FORWARD_NAT) { + if (virNatDefFormat(buf, &def->forward) < 0) + goto error; + } + /* For now, hard-coded to at most 1 forward.pfs */ if (def->forward.npfs) virBufferEscapeString(buf, "<pf dev='%s'/>\n", @@ -2155,7 +2297,7 @@ virNetworkDefFormatInternal(virBufferPtr buf, } } virBufferAdjustIndent(buf, -2); - if (def->forward.npfs || def->forward.nifs) + if (!shortforward) virBufferAddLit(buf, "</forward>\n"); } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4c634ed..11d6c9c 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -174,6 +174,9 @@ struct _virNetworkForwardDef { size_t nifs; virNetworkForwardIfDefPtr ifs; + + /* adresses for SNAT */ + virSocketAddr addrStart, addrEnd; }; typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c834f83..9f502a5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1587,6 +1587,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1601,6 +1603,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1615,6 +1619,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1631,12 +1637,16 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1667,16 +1677,22 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 41fe780..f44236a 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -805,11 +805,16 @@ iptablesForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol, int action) { - int ret; - char *networkstr; + int ret = -1; + char *networkstr = NULL; + char *addrStartStr = NULL; + char *addrEndStr = NULL; + char *tmpStr = NULL; virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -820,8 +825,18 @@ iptablesForwardMasquerade(iptablesContext *ctx, virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempted to NAT '%s'. NAT is only supported for IPv4."), networkstr); - VIR_FREE(networkstr); - return -1; + goto cleanup; + } + + if (VIR_SOCKET_ADDR_IS_FAMILY(addrStart, AF_INET)) { + addrStartStr = virSocketAddrFormat(addrStart); + if (!addrStartStr) + goto cleanup; + if (VIR_SOCKET_ADDR_IS_FAMILY(addrEnd, AF_INET)) { + addrEndStr = virSocketAddrFormat(addrEnd); + if (!addrEndStr) + goto cleanup; + } } cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action); @@ -835,13 +850,39 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL); - virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); + /* Use --jump SNAT if public addr is specified */ + if (addrStartStr && addrStartStr[0]) { + const char *portStr = ""; + int r = 0; - if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + if (protocol && protocol[0]) + portStr = ":1024-65535"; + + if (addrEndStr && addrEndStr[0]) { + r = virAsprintf(&tmpStr, "%s-%s%s", addrStartStr, addrEndStr, + portStr); + } else { + r = virAsprintf(&tmpStr, "%s%s", addrStartStr, portStr); + } + + if (r < 0) { + virReportOOMError(); + goto cleanup; + } + + virCommandAddArgList(cmd, "--jump", "SNAT", + "--to-source", tmpStr, NULL); + } else { + virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); + + if (protocol && protocol[0]) + virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + } ret = iptablesCommandRunAndFree(cmd); +cleanup: VIR_FREE(networkstr); + VIR_FREE(tmpStr); return ret; } @@ -863,9 +904,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, ADD); } /** @@ -886,9 +929,11 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, REMOVE); } diff --git a/src/util/viriptables.h b/src/util/viriptables.h index d7fa731..05362da 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -107,11 +107,15 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface, -- 1.8.1.3
On 02/19/2013 05:44 AM, Natanael Copa wrote:
Support setting which public ip to use for NAT via attribute address in subelement <nat> in <forward>:
... <forward mode='nat'> <address start='1.2.3.4' end='1.2.3.10'/> </forward> ...
This will construct an iptables line using:
'-j SNAT --to-source <start>-<end>'
instead of:
'-j MASQUERADE'
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 18 ++++++ src/conf/network_conf.c | 152 ++++++++++++++++++++++++++++++++++++++++++-- src/conf/network_conf.h | 3 + src/network/bridge_driver.c | 16 +++++ src/util/viriptables.c | 63 +++++++++++++++--- src/util/viriptables.h | 4 ++ 6 files changed, 242 insertions(+), 14 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7b42529..5fbd0a9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -136,6 +136,24 @@ network, and to/from the host to the guests, are unrestricted and not NATed.<span class="since">Since 0.4.2</span> + + <p><span class="since">Since 1.0.3</span> it is possible to + specify a public IPv4 address range to be used for the NAT by + using the <code><nat></code> and + <code><address></code> subelements. + <pre> +... + <forward mode='nat'> + <nat> + <address start='1.2.3.4' end='1.2.3.10'/> + </nat> + </forward> +... + </pre> + An singe IPv4 address can be set by setting + <code>start</code> and <code>end</code> attributes to + the same value. + </p> </dd>
<dt><code>route</code></dt> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3604ff7..620110c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1325,6 +1325,80 @@ cleanup: }
static int +virNetworkForwardNatDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkForwardDefPtr def) +{ + int ret = -1; + xmlNodePtr *natAddrNodes = NULL; + int nNatAddrs; + char *addrStart = NULL; + char *addrEnd = NULL; + xmlNodePtr save = ctxt->node; + + ctxt->node = node; + + if (def->type != VIR_NETWORK_FORWARD_NAT) { + virReportError(VIR_ERR_XML_ERROR, + _("The <nat> element can only be used when <forward> 'mode' is 'nat' in network %s"), + networkName); + goto cleanup; + } + + /* addresses for SNAT */ + nNatAddrs = virXPathNodeSet("./address", ctxt, &natAddrNodes); + if (nNatAddrs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <address> element found in <forward> of " + "network %s"), networkName); + goto cleanup; + } else if (nNatAddrs > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <address> element is allowed in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } else if (nNatAddrs == 1) { + addrStart = virXMLPropString(*natAddrNodes, "start"); + if (addrStart == NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'start' attribute in <address> element in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } + addrEnd = virXMLPropString(*natAddrNodes, "end"); + if (addrEnd == NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'end' attribute in <address> element in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } + } + + if (addrStart && virSocketAddrParse(&def->addrStart, addrStart, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 start address '%s' in <nat> in <forward> in " + "network '%s'"), addrStart, networkName); + goto cleanup; + } + + if (addrEnd && virSocketAddrParse(&def->addrEnd, addrEnd, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 end address '%s' in <nat> in <forward> in " + "network '%s'"), addrEnd, networkName); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(addrStart); + VIR_FREE(addrEnd);
You still forgot VIR_FREE(natAddrNodes).
+ ctxt->node = save; + return ret; +} + +static int virNetworkForwardDefParseXML(const char *networkName, xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -1334,7 +1408,8 @@ virNetworkForwardDefParseXML(const char *networkName, xmlNodePtr *forwardIfNodes = NULL; xmlNodePtr *forwardPfNodes = NULL; xmlNodePtr *forwardAddrNodes = NULL; - int nForwardIfs, nForwardAddrs, nForwardPfs; + xmlNodePtr *forwardNatNodes = NULL; + int nForwardIfs, nForwardAddrs, nForwardPfs, nForwardNats; char *forwardDev = NULL; char *forwardManaged = NULL; char *type = NULL; @@ -1384,6 +1459,24 @@ virNetworkForwardDefParseXML(const char *networkName, goto cleanup; }
+ nForwardNats = virXPathNodeSet("./nat", ctxt, &forwardNatNodes); + if (nForwardNats < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <nat> element found in <forward> of network %s"), + networkName); + goto cleanup; + } else if (nForwardNats > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <nat> element is allowed in <forward> of network %s"), + networkName); + goto cleanup; + } else if (nForwardNats == 1) { + if (virNetworkForwardNatDefParseXML(networkName, + *forwardNatNodes, + ctxt, def) < 0) + goto cleanup; + } + if (((nForwardIfs > 0) + (nForwardAddrs > 0) + (nForwardPfs > 0)) > 1) { virReportError(VIR_ERR_XML_ERROR, _("<address>, <interface>, and <pf> elements in <forward> " @@ -1525,6 +1618,7 @@ cleanup: VIR_FREE(forwardPfNodes); VIR_FREE(forwardIfNodes); VIR_FREE(forwardAddrNodes); + VIR_FREE(forwardNatNodes); ctxt->node = save; return ret; } @@ -2079,13 +2173,54 @@ virPortGroupDefFormat(virBufferPtr buf, }
static int +virNatDefFormat(virBufferPtr buf,
Another thing I pointed out in my previous review: this should be named consistent with the parse function - virNetworkForwardNatDefFormat() (I know virPortgroupDefFormat() is also named inconsistently (and that's probably why you named this one the way you did) - I'm going to fix that in a separate patch).
+ const virNetworkForwardDefPtr fwd) +{ + char *addrStart = NULL; + char *addrEnd = NULL; + int ret = -1; + + if (VIR_SOCKET_ADDR_VALID(&fwd->addrStart)) { + addrStart = virSocketAddrFormat(&fwd->addrStart); + if (!addrStart) + goto cleanup; + } + + if (VIR_SOCKET_ADDR_VALID(&fwd->addrEnd)) { + addrEnd = virSocketAddrFormat(&fwd->addrEnd); + if (!addrEnd) + goto cleanup; + } + + if (!addrEnd && !addrStart) + return 0; + + virBufferAddLit(buf, "<nat>\n"); + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<address start='%s'", addrStart); + if (addrEnd) + virBufferAsprintf(buf, " end='%s'", addrEnd); + virBufferAsprintf(buf, "/>\n");
Use virBufferAddLit() instead of virBufferAsprintf() when there are no args.
+ + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</nat>\n");
Again, use virBufferAsprintf().
+ ret = 0; + +cleanup: + VIR_FREE(addrStart); + VIR_FREE(addrEnd); + return ret; +} + +static int virNetworkDefFormatInternal(virBufferPtr buf, const virNetworkDefPtr def, unsigned int flags) { unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; - int ii; + int ii, shortforward;
virBufferAddLit(buf, "<network"); if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0)) { @@ -2122,10 +2257,17 @@ virNetworkDefFormatInternal(virBufferPtr buf, else virBufferAddLit(buf, " managed='no'"); } - virBufferAsprintf(buf, "%s>\n", - (def->forward.nifs || def->forward.npfs) ? "" : "/"); + shortforward = !(def->forward.nifs || def->forward.npfs + || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart) + || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd)); + virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2);
+ if (def->forward.type == VIR_NETWORK_FORWARD_NAT) { + if (virNatDefFormat(buf, &def->forward) < 0) + goto error; + } + /* For now, hard-coded to at most 1 forward.pfs */ if (def->forward.npfs) virBufferEscapeString(buf, "<pf dev='%s'/>\n", @@ -2155,7 +2297,7 @@ virNetworkDefFormatInternal(virBufferPtr buf, } } virBufferAdjustIndent(buf, -2); - if (def->forward.npfs || def->forward.nifs) + if (!shortforward) virBufferAddLit(buf, "</forward>\n"); }
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4c634ed..11d6c9c 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -174,6 +174,9 @@ struct _virNetworkForwardDef {
size_t nifs; virNetworkForwardIfDefPtr ifs; + + /* adresses for SNAT */ + virSocketAddr addrStart, addrEnd; };
typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c834f83..9f502a5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1587,6 +1587,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1601,6 +1603,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1615,6 +1619,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1631,12 +1637,16 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1667,16 +1677,22 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addrStart, + &network->def->forward.addrEnd, NULL);
iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 41fe780..f44236a 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -805,11 +805,16 @@ iptablesForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol, int action) { - int ret; - char *networkstr; + int ret = -1; + char *networkstr = NULL; + char *addrStartStr = NULL; + char *addrEndStr = NULL; + char *tmpStr = NULL;
I'm changing this to natRangeStr.
virCommandPtr cmd = NULL;
if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -820,8 +825,18 @@ iptablesForwardMasquerade(iptablesContext *ctx, virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempted to NAT '%s'. NAT is only supported for IPv4."), networkstr); - VIR_FREE(networkstr); - return -1; + goto cleanup; + } + + if (VIR_SOCKET_ADDR_IS_FAMILY(addrStart, AF_INET)) { + addrStartStr = virSocketAddrFormat(addrStart); + if (!addrStartStr) + goto cleanup; + if (VIR_SOCKET_ADDR_IS_FAMILY(addrEnd, AF_INET)) { + addrEndStr = virSocketAddrFormat(addrEnd); + if (!addrEndStr) + goto cleanup;
indentation is still messed up here.
+ } }
cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action); @@ -835,13 +850,39 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
- virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); + /* Use --jump SNAT if public addr is specified */ + if (addrStartStr && addrStartStr[0]) { + const char *portStr = ""; + int r = 0;
- if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + if (protocol && protocol[0]) + portStr = ":1024-65535"; + + if (addrEndStr && addrEndStr[0]) { + r = virAsprintf(&tmpStr, "%s-%s%s", addrStartStr, addrEndStr, + portStr); + } else { + r = virAsprintf(&tmpStr, "%s%s", addrStartStr, portStr); + } + + if (r < 0) { + virReportOOMError(); + goto cleanup; + } + + virCommandAddArgList(cmd, "--jump", "SNAT", + "--to-source", tmpStr, NULL); + } else { + virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); + + if (protocol && protocol[0]) + virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + }
ret = iptablesCommandRunAndFree(cmd); +cleanup: VIR_FREE(networkstr); + VIR_FREE(tmpStr);
You also need to free addrStartStr and addrEndStr (as mentioned in the review of V1). Additionally, I just noticed that in the case of an error that is caught after virCommandNew(), but prior to iptablesCommandRunAndFree(), we will be leaking one virCommand. So I'm eliminating the call to iptablesCommandRunAndFree() and just calling virCommandRun directly, with virCommandFree() added after the cleanup label.
return ret; }
@@ -863,9 +904,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, ADD); }
/** @@ -886,9 +929,11 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, REMOVE); }
diff --git a/src/util/viriptables.h b/src/util/viriptables.h index d7fa731..05362da 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -107,11 +107,15 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addrStart, + virSocketAddr *addrEnd, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface,
ACK with the nits I pointed out above. I made those changes and pushed the result. Thanks for the contribution!
Let users set the port range to be used for forward mode NAT: ... <forward mode='nat'> <nat> <port start='1024' end='65535'/> </nat> </forward> ... Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 21 ++++++++++++++--- src/conf/network_conf.c | 56 +++++++++++++++++++++++++++++++++++++++------ src/conf/network_conf.h | 3 ++- src/network/bridge_driver.c | 16 +++++++++++++ src/util/viriptables.c | 49 +++++++++++++++++++++++++++++++-------- src/util/viriptables.h | 4 ++++ 6 files changed, 128 insertions(+), 21 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 5fbd0a9..adb5bb9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -138,9 +138,11 @@ 0.4.2</span> <p><span class="since">Since 1.0.3</span> it is possible to - specify a public IPv4 address range to be used for the NAT by - using the <code><nat></code> and - <code><address></code> subelements. + specify a public IPv4 address and port range to be used for + the NAT by using the <code><nat></code> subelement. + The address range is set with the <code><address></code> + subelements and <code>start</code> and <code>stop</code> + attributes: <pre> ... <forward mode='nat'> @@ -154,6 +156,19 @@ <code>start</code> and <code>end</code> attributes to the same value. </p> + <p> + The port range to be used for the <code><nat></code> can + be set via the subelement <code><port></code>: + <pre> +... + <forward mode='nat'> + <nat> + <port start='500' end='1000'/> + </nat> + </forward> +... + </pre> + </p> </dd> <dt><code>route</code></dt> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 620110c..bfdbeef 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1332,7 +1332,8 @@ virNetworkForwardNatDefParseXML(const char *networkName, { int ret = -1; xmlNodePtr *natAddrNodes = NULL; - int nNatAddrs; + xmlNodePtr *natPortNodes = NULL; + int nNatAddrs, nNatPorts; char *addrStart = NULL; char *addrEnd = NULL; xmlNodePtr save = ctxt->node; @@ -1389,6 +1390,36 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; } + /* ports for SNAT and MASQUERADE */ + nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); + if (nNatPorts < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <port> element found in <forward> of " + "network %s"), networkName); + goto cleanup; + } else if (nNatPorts > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <port> element is allowed in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } else if (nNatPorts == 1) { + if (virXPathUInt("string(./port[1]/@start)", ctxt, &def->portStart) < 0 + || def->portStart > 65535) { + + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid 'start' attribute in <port> " + "in <nat> in <forward> in network %s"), + networkName); + goto cleanup; + } + if (virXPathUInt("string(./port[1]/@end)", ctxt, &def->portEnd) < 0 + || def->portEnd > 65535 || def->portEnd < def->portStart) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid 'end' attribute in <port> in " + "<nat> in <forward> in network %s"), networkName); + goto cleanup; + } + } ret = 0; cleanup: @@ -2192,16 +2223,25 @@ virNatDefFormat(virBufferPtr buf, goto cleanup; } - if (!addrEnd && !addrStart) + if (!addrEnd && !addrStart && !fwd->portStart && !fwd->portEnd) return 0; virBufferAddLit(buf, "<nat>\n"); virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<address start='%s'", addrStart); - if (addrEnd) - virBufferAsprintf(buf, " end='%s'", addrEnd); - virBufferAsprintf(buf, "/>\n"); + if (addrStart) { + virBufferAsprintf(buf, "<address start='%s'", addrStart); + if (addrEnd) + virBufferAsprintf(buf, " end='%s'", addrEnd); + virBufferAsprintf(buf, "/>\n"); + } + + if (fwd->portStart || fwd->portEnd) { + virBufferAsprintf(buf, "<port start='%d'", fwd->portStart); + if (fwd->portEnd) + virBufferAsprintf(buf, " end='%d'", fwd->portEnd); + virBufferAsprintf(buf, "/>\n"); + } virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, "</nat>\n"); @@ -2259,7 +2299,9 @@ virNetworkDefFormatInternal(virBufferPtr buf, } shortforward = !(def->forward.nifs || def->forward.npfs || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart) - || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd)); + || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd) + || def->forward.portStart + || def->forward.portEnd); virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 11d6c9c..515115b 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -175,8 +175,9 @@ struct _virNetworkForwardDef { size_t nifs; virNetworkForwardIfDefPtr ifs; - /* adresses for SNAT */ + /* ranges for NAT */ virSocketAddr addrStart, addrEnd; + unsigned int portStart, portEnd; }; typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9f502a5..cf47ec4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1589,6 +1589,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1605,6 +1607,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1621,6 +1625,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1639,6 +1645,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, @@ -1647,6 +1655,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1679,6 +1689,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1686,6 +1698,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1693,6 +1707,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index f44236a..f9f28b4 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -807,6 +807,8 @@ iptablesForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol, int action) { @@ -815,6 +817,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, char *addrStartStr = NULL; char *addrEndStr = NULL; char *tmpStr = NULL; + char *portRangeStr = NULL; virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -850,19 +853,34 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL); + if (protocol && protocol[0]) { + if (portStart == 0 && portEnd == 0) { + portStart = 1024; + portEnd = 65535; + } + + if (portStart < portEnd && portEnd < 65536) { + if (virAsprintf(&portRangeStr, ":%u-%u", portStart, portEnd) < 0) { + virReportOOMError(); + goto cleanup; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid port range '%u-%u'."), + portStart, portEnd); + } + } + /* Use --jump SNAT if public addr is specified */ if (addrStartStr && addrStartStr[0]) { - const char *portStr = ""; int r = 0; - if (protocol && protocol[0]) - portStr = ":1024-65535"; - if (addrEndStr && addrEndStr[0]) { r = virAsprintf(&tmpStr, "%s-%s%s", addrStartStr, addrEndStr, - portStr); + portRangeStr ? portRangeStr : ""); } else { - r = virAsprintf(&tmpStr, "%s%s", addrStartStr, portStr); + r = virAsprintf(&tmpStr, "%s%s", addrStartStr, + portRangeStr ? portRangeStr : ""); } if (r < 0) { @@ -875,14 +893,15 @@ iptablesForwardMasquerade(iptablesContext *ctx, } else { virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); - if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + if (portRangeStr && portRangeStr[0]) + virCommandAddArgList(cmd, "--to-ports", &portRangeStr[1], NULL); } ret = iptablesCommandRunAndFree(cmd); cleanup: VIR_FREE(networkstr); VIR_FREE(tmpStr); + VIR_FREE(portRangeStr); return ret; } @@ -906,9 +925,14 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addrStart, addrEnd, + portStart, portEnd, + protocol, ADD); } /** @@ -931,9 +955,14 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addrStart, addrEnd, + portStart, portEnd, + protocol, REMOVE); } diff --git a/src/util/viriptables.h b/src/util/viriptables.h index 05362da..ca6adcc 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -109,6 +109,8 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, @@ -116,6 +118,8 @@ int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface, -- 1.8.1.3
On 02/19/2013 05:44 AM, Natanael Copa wrote:
Let users set the port range to be used for forward mode NAT:
... <forward mode='nat'> <nat> <port start='1024' end='65535'/> </nat> </forward> ...
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 21 ++++++++++++++--- src/conf/network_conf.c | 56 +++++++++++++++++++++++++++++++++++++++------ src/conf/network_conf.h | 3 ++- src/network/bridge_driver.c | 16 +++++++++++++ src/util/viriptables.c | 49 +++++++++++++++++++++++++++++++-------- src/util/viriptables.h | 4 ++++ 6 files changed, 128 insertions(+), 21 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 5fbd0a9..adb5bb9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -138,9 +138,11 @@ 0.4.2</span>
<p><span class="since">Since 1.0.3</span> it is possible to - specify a public IPv4 address range to be used for the NAT by - using the <code><nat></code> and - <code><address></code> subelements. + specify a public IPv4 address and port range to be used for + the NAT by using the <code><nat></code> subelement. + The address range is set with the <code><address></code> + subelements and <code>start</code> and <code>stop</code> + attributes: <pre> ... <forward mode='nat'> @@ -154,6 +156,19 @@ <code>start</code> and <code>end</code> attributes to the same value. </p> + <p> + The port range to be used for the <code><nat></code> can + be set via the subelement <code><port></code>: + <pre> +... + <forward mode='nat'> + <nat> + <port start='500' end='1000'/> + </nat> + </forward> +... + </pre> + </p> </dd>
<dt><code>route</code></dt> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 620110c..bfdbeef 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1332,7 +1332,8 @@ virNetworkForwardNatDefParseXML(const char *networkName, { int ret = -1; xmlNodePtr *natAddrNodes = NULL; - int nNatAddrs; + xmlNodePtr *natPortNodes = NULL; + int nNatAddrs, nNatPorts; char *addrStart = NULL; char *addrEnd = NULL; xmlNodePtr save = ctxt->node; @@ -1389,6 +1390,36 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; }
+ /* ports for SNAT and MASQUERADE */ + nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); + if (nNatPorts < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <port> element found in <forward> of " + "network %s"), networkName); + goto cleanup; + } else if (nNatPorts > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <port> element is allowed in <nat> in " + "<forward> in network %s"), networkName); + goto cleanup; + } else if (nNatPorts == 1) { + if (virXPathUInt("string(./port[1]/@start)", ctxt, &def->portStart) < 0 + || def->portStart > 65535) { + + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid 'start' attribute in <port> " + "in <nat> in <forward> in network %s"), + networkName); + goto cleanup; + } + if (virXPathUInt("string(./port[1]/@end)", ctxt, &def->portEnd) < 0 + || def->portEnd > 65535 || def->portEnd < def->portStart) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid 'end' attribute in <port> in " + "<nat> in <forward> in network %s"), networkName); + goto cleanup; + } + } ret = 0;
cleanup: @@ -2192,16 +2223,25 @@ virNatDefFormat(virBufferPtr buf, goto cleanup; }
- if (!addrEnd && !addrStart) + if (!addrEnd && !addrStart && !fwd->portStart && !fwd->portEnd) return 0;
virBufferAddLit(buf, "<nat>\n"); virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf, "<address start='%s'", addrStart); - if (addrEnd) - virBufferAsprintf(buf, " end='%s'", addrEnd); - virBufferAsprintf(buf, "/>\n"); + if (addrStart) { + virBufferAsprintf(buf, "<address start='%s'", addrStart); + if (addrEnd) + virBufferAsprintf(buf, " end='%s'", addrEnd); + virBufferAsprintf(buf, "/>\n"); + } + + if (fwd->portStart || fwd->portEnd) { + virBufferAsprintf(buf, "<port start='%d'", fwd->portStart); + if (fwd->portEnd) + virBufferAsprintf(buf, " end='%d'", fwd->portEnd); + virBufferAsprintf(buf, "/>\n");
Again - virBufferAddLit()
+ }
virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, "</nat>\n"); @@ -2259,7 +2299,9 @@ virNetworkDefFormatInternal(virBufferPtr buf, } shortforward = !(def->forward.nifs || def->forward.npfs || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart) - || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd)); + || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd) + || def->forward.portStart + || def->forward.portEnd); virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 11d6c9c..515115b 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -175,8 +175,9 @@ struct _virNetworkForwardDef { size_t nifs; virNetworkForwardIfDefPtr ifs;
- /* adresses for SNAT */ + /* ranges for NAT */ virSocketAddr addrStart, addrEnd; + unsigned int portStart, portEnd; };
typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9f502a5..cf47ec4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1589,6 +1589,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1605,6 +1607,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1621,6 +1625,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1639,6 +1645,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, @@ -1647,6 +1655,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1679,6 +1689,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1686,6 +1698,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1693,6 +1707,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addrStart, &network->def->forward.addrEnd, + network->def->forward.portStart, + network->def->forward.portEnd, NULL);
iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index f44236a..f9f28b4 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -807,6 +807,8 @@ iptablesForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol, int action) { @@ -815,6 +817,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, char *addrStartStr = NULL; char *addrEndStr = NULL; char *tmpStr = NULL; + char *portRangeStr = NULL; virCommandPtr cmd = NULL;
if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -850,19 +853,34 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
+ if (protocol && protocol[0]) { + if (portStart == 0 && portEnd == 0) { + portStart = 1024; + portEnd = 65535; + } + + if (portStart < portEnd && portEnd < 65536) { + if (virAsprintf(&portRangeStr, ":%u-%u", portStart, portEnd) < 0) { + virReportOOMError(); + goto cleanup; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid port range '%u-%u'."), + portStart, portEnd); + } + } + /* Use --jump SNAT if public addr is specified */ if (addrStartStr && addrStartStr[0]) { - const char *portStr = ""; int r = 0;
- if (protocol && protocol[0]) - portStr = ":1024-65535"; - if (addrEndStr && addrEndStr[0]) { r = virAsprintf(&tmpStr, "%s-%s%s", addrStartStr, addrEndStr, - portStr); + portRangeStr ? portRangeStr : ""); } else { - r = virAsprintf(&tmpStr, "%s%s", addrStartStr, portStr); + r = virAsprintf(&tmpStr, "%s%s", addrStartStr, + portRangeStr ? portRangeStr : ""); }
if (r < 0) { @@ -875,14 +893,15 @@ iptablesForwardMasquerade(iptablesContext *ctx, } else { virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL);
- if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + if (portRangeStr && portRangeStr[0]) + virCommandAddArgList(cmd, "--to-ports", &portRangeStr[1], NULL); }
ret = iptablesCommandRunAndFree(cmd); cleanup: VIR_FREE(networkstr); VIR_FREE(tmpStr); + VIR_FREE(portRangeStr); return ret; }
@@ -906,9 +925,14 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addrStart, addrEnd, + portStart, portEnd, + protocol, ADD); }
/** @@ -931,9 +955,14 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addrStart, addrEnd, + portStart, portEnd, + protocol, REMOVE); }
diff --git a/src/util/viriptables.h b/src/util/viriptables.h index 05362da..ca6adcc 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -109,6 +109,8 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, @@ -116,6 +118,8 @@ int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addrStart, virSocketAddr *addrEnd, + unsigned int portStart, + unsigned int portEnd, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface,
ACK with the one small change I hadn't noticed before. I made that change and pushed.
We pass over the address/port start/end values many times so we put them in structs. Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- Skip this patch if there are doubts. src/conf/network_conf.c | 50 ++++++++++++++++++++++----------------------- src/conf/network_conf.h | 13 +++--------- src/network/bridge_driver.c | 48 +++++++++++++++---------------------------- src/util/viriptables.c | 47 +++++++++++++++++------------------------- src/util/viriptables.h | 12 ++++------- src/util/virsocketaddr.h | 14 +++++++++++++ 6 files changed, 81 insertions(+), 103 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index bfdbeef..d384e38 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -628,9 +628,9 @@ int virNetworkIpDefNetmask(const virNetworkIpDefPtr def, static int -virNetworkDHCPRangeDefParseXML(const char *networkName, +virSocketAddrRangeParseXML(const char *networkName, xmlNodePtr node, - virNetworkDHCPRangeDefPtr range) + virSocketAddrRangePtr range) { @@ -793,7 +793,7 @@ virNetworkDHCPDefParseXML(const char *networkName, virReportOOMError(); return -1; } - if (virNetworkDHCPRangeDefParseXML(networkName, cur, + if (virSocketAddrRangeParseXML(networkName, cur, &def->ranges[def->nranges]) < 0) { return -1; } @@ -1376,14 +1376,14 @@ virNetworkForwardNatDefParseXML(const char *networkName, } } - if (addrStart && virSocketAddrParse(&def->addrStart, addrStart, AF_INET) < 0) { + if (addrStart && virSocketAddrParse(&def->addr.start, addrStart, AF_INET) < 0) { virReportError(VIR_ERR_XML_ERROR, _("Bad ipv4 start address '%s' in <nat> in <forward> in " "network '%s'"), addrStart, networkName); goto cleanup; } - if (addrEnd && virSocketAddrParse(&def->addrEnd, addrEnd, AF_INET) < 0) { + if (addrEnd && virSocketAddrParse(&def->addr.end, addrEnd, AF_INET) < 0) { virReportError(VIR_ERR_XML_ERROR, _("Bad ipv4 end address '%s' in <nat> in <forward> in " "network '%s'"), addrEnd, networkName); @@ -1403,8 +1403,8 @@ virNetworkForwardNatDefParseXML(const char *networkName, "<forward> in network %s"), networkName); goto cleanup; } else if (nNatPorts == 1) { - if (virXPathUInt("string(./port[1]/@start)", ctxt, &def->portStart) < 0 - || def->portStart > 65535) { + if (virXPathUInt("string(./port[1]/@start)", ctxt, &def->port.start) < 0 + || def->port.start > 65535) { virReportError(VIR_ERR_XML_DETAIL, _("Missing or invalid 'start' attribute in <port> " @@ -1412,8 +1412,8 @@ virNetworkForwardNatDefParseXML(const char *networkName, networkName); goto cleanup; } - if (virXPathUInt("string(./port[1]/@end)", ctxt, &def->portEnd) < 0 - || def->portEnd > 65535 || def->portEnd < def->portStart) { + if (virXPathUInt("string(./port[1]/@end)", ctxt, &def->port.end) < 0 + || def->port.end > 65535 || def->port.end < def->port.start) { virReportError(VIR_ERR_XML_DETAIL, _("Missing or invalid 'end' attribute in <port> in " "<nat> in <forward> in network %s"), networkName); @@ -2211,19 +2211,19 @@ virNatDefFormat(virBufferPtr buf, char *addrEnd = NULL; int ret = -1; - if (VIR_SOCKET_ADDR_VALID(&fwd->addrStart)) { - addrStart = virSocketAddrFormat(&fwd->addrStart); + if (VIR_SOCKET_ADDR_VALID(&fwd->addr.start)) { + addrStart = virSocketAddrFormat(&fwd->addr.start); if (!addrStart) goto cleanup; } - if (VIR_SOCKET_ADDR_VALID(&fwd->addrEnd)) { - addrEnd = virSocketAddrFormat(&fwd->addrEnd); + if (VIR_SOCKET_ADDR_VALID(&fwd->addr.end)) { + addrEnd = virSocketAddrFormat(&fwd->addr.end); if (!addrEnd) goto cleanup; } - if (!addrEnd && !addrStart && !fwd->portStart && !fwd->portEnd) + if (!addrEnd && !addrStart && !fwd->port.start && !fwd->port.end) return 0; virBufferAddLit(buf, "<nat>\n"); @@ -2236,10 +2236,10 @@ virNatDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "/>\n"); } - if (fwd->portStart || fwd->portEnd) { - virBufferAsprintf(buf, "<port start='%d'", fwd->portStart); - if (fwd->portEnd) - virBufferAsprintf(buf, " end='%d'", fwd->portEnd); + if (fwd->port.start || fwd->port.end) { + virBufferAsprintf(buf, "<port start='%d'", fwd->port.start); + if (fwd->port.end) + virBufferAsprintf(buf, " end='%d'", fwd->port.end); virBufferAsprintf(buf, "/>\n"); } @@ -2298,10 +2298,10 @@ virNetworkDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, " managed='no'"); } shortforward = !(def->forward.nifs || def->forward.npfs - || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart) - || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd) - || def->forward.portStart - || def->forward.portEnd); + || VIR_SOCKET_ADDR_VALID(&def->forward.addr.start) + || VIR_SOCKET_ADDR_VALID(&def->forward.addr.end) + || def->forward.port.start + || def->forward.port.end); virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2); @@ -3015,7 +3015,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, { int ii, ret = -1; virNetworkIpDefPtr ipdef = virNetworkIpDefByIndex(def, parentIndex); - virNetworkDHCPRangeDef range; + virSocketAddrRange range; memset(&range, 0, sizeof(range)); @@ -3026,7 +3026,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, if (!ipdef) goto cleanup; - /* parse the xml into a virNetworkDHCPRangeDef */ + /* parse the xml into a virSocketAddrRange */ if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { virReportError(VIR_ERR_NO_SUPPORT, "%s", @@ -3035,7 +3035,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, goto cleanup; } - if (virNetworkDHCPRangeDefParseXML(def->name, ctxt->node, &range) < 0) + if (virSocketAddrRangeParseXML(def->name, ctxt->node, &range) < 0) goto cleanup; /* check if an entry with same name/address/ip already exists */ diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 515115b..d5de7d1 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -62,13 +62,6 @@ enum virNetworkForwardHostdevDeviceType { VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST, }; -typedef struct _virNetworkDHCPRangeDef virNetworkDHCPRangeDef; -typedef virNetworkDHCPRangeDef *virNetworkDHCPRangeDefPtr; -struct _virNetworkDHCPRangeDef { - virSocketAddr start; - virSocketAddr end; -}; - typedef struct _virNetworkDHCPHostDef virNetworkDHCPHostDef; typedef virNetworkDHCPHostDef *virNetworkDHCPHostDefPtr; struct _virNetworkDHCPHostDef { @@ -131,7 +124,7 @@ struct _virNetworkIpDef { virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */ size_t nranges; /* Zero or more dhcp ranges */ - virNetworkDHCPRangeDefPtr ranges; + virSocketAddrRangePtr ranges; size_t nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; @@ -176,8 +169,8 @@ struct _virNetworkForwardDef { virNetworkForwardIfDefPtr ifs; /* ranges for NAT */ - virSocketAddr addrStart, addrEnd; - unsigned int portStart, portEnd; + virSocketAddrRange addr; + virPortRange port; }; typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index cf47ec4..d7f6df9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1587,10 +1587,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1605,10 +1603,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1623,10 +1619,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1643,20 +1637,16 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1687,28 +1677,22 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, - &network->def->forward.addrStart, - &network->def->forward.addrEnd, - network->def->forward.portStart, - network->def->forward.portEnd, + &network->def->forward.addr, + &network->def->forward.port, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index f9f28b4..d142454 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -805,10 +805,8 @@ iptablesForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, - virSocketAddr *addrStart, - virSocketAddr *addrEnd, - unsigned int portStart, - unsigned int portEnd, + virSocketAddrRangePtr addr, + virPortRangePtr port, const char *protocol, int action) { @@ -831,12 +829,12 @@ iptablesForwardMasquerade(iptablesContext *ctx, goto cleanup; } - if (VIR_SOCKET_ADDR_IS_FAMILY(addrStart, AF_INET)) { - addrStartStr = virSocketAddrFormat(addrStart); + if (VIR_SOCKET_ADDR_IS_FAMILY(&addr->start, AF_INET)) { + addrStartStr = virSocketAddrFormat(&addr->start); if (!addrStartStr) goto cleanup; - if (VIR_SOCKET_ADDR_IS_FAMILY(addrEnd, AF_INET)) { - addrEndStr = virSocketAddrFormat(addrEnd); + if (VIR_SOCKET_ADDR_IS_FAMILY(&addr->end, AF_INET)) { + addrEndStr = virSocketAddrFormat(&addr->end); if (!addrEndStr) goto cleanup; } @@ -854,20 +852,21 @@ iptablesForwardMasquerade(iptablesContext *ctx, virCommandAddArgList(cmd, "--out-interface", physdev, NULL); if (protocol && protocol[0]) { - if (portStart == 0 && portEnd == 0) { - portStart = 1024; - portEnd = 65535; + if (port->start == 0 && port->end == 0) { + port->start = 1024; + port->end = 65535; } - if (portStart < portEnd && portEnd < 65536) { - if (virAsprintf(&portRangeStr, ":%u-%u", portStart, portEnd) < 0) { + if (port->start < port->end && port->end < 65536) { + if (virAsprintf(&portRangeStr, ":%u-%u", + port->start, port->end) < 0) { virReportOOMError(); goto cleanup; } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid port range '%u-%u'."), - portStart, portEnd); + port->start, port->end); } } @@ -923,15 +922,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, - virSocketAddr *addrStart, - virSocketAddr *addrEnd, - unsigned int portStart, - unsigned int portEnd, + virSocketAddrRangePtr addr, + virPortRangePtr port, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, - addrStart, addrEnd, - portStart, portEnd, + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr, port, protocol, ADD); } @@ -953,15 +948,11 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, - virSocketAddr *addrStart, - virSocketAddr *addrEnd, - unsigned int portStart, - unsigned int portEnd, + virSocketAddrRangePtr addr, + virPortRangePtr port, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, - addrStart, addrEnd, - portStart, portEnd, + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr, port, protocol, REMOVE); } diff --git a/src/util/viriptables.h b/src/util/viriptables.h index ca6adcc..b7ce59b 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -107,19 +107,15 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, - virSocketAddr *addrStart, - virSocketAddr *addrEnd, - unsigned int portStart, - unsigned int portEnd, + virSocketAddrRangePtr addr, + virPortRangePtr port, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, - virSocketAddr *addrStart, - virSocketAddr *addrEnd, - unsigned int portStart, - unsigned int portEnd, + virSocketAddrRangePtr addr, + virPortRangePtr port, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface, diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 66d4265..8993f7b 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -56,6 +56,20 @@ typedef struct { typedef virSocketAddr *virSocketAddrPtr; +typedef struct _virSocketAddrRange virSocketAddrRange; +typedef virSocketAddrRange *virSocketAddrRangePtr; +struct _virSocketAddrRange { + virSocketAddr start; + virSocketAddr end; +}; + +typedef struct _virPortRange virPortRange; +typedef virPortRange *virPortRangePtr; +struct _virPortRange { + unsigned int start; + unsigned int end; +}; + int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family); -- 1.8.1.3
On 02/19/2013 05:44 AM, Natanael Copa wrote:
We pass over the address/port start/end values many times so we put them in structs.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> ---
Skip this patch if there are doubts.
Like you, I'm unsure if virsocketaddr.h is the best place for virPortRange, but don't see any better place, and this reduces clutter and seems generally useful, so ACK. I'm pushing it along with the other two in the series. Thanks again for taking the time to implement this feature!
participants (2)
-
Laine Stump -
Natanael Copa