On 02/11/2013 09:54 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@xxxxxxxxxxxxxxx>
> ---
> 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 | 56 +++++++++++++---
> src/util/viriptables.h | 4 ++
> 6 files changed, 235 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..61d086a 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 *addr_start = NULL;
> + char *addr_end = NULL;
Although you'll find both underscore_separated variable names and
studlyCapped variable names in libvirt, there seems to be more of a
preference for the latter. I'm changing all addr_start to addrStart, and
all addr_end to addrEnd.
> + 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) {
> + addr_start = virXMLPropString(*natAddrNodes, "start");
> + if (addr_start == NULL) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("missing 'start' attribute in <address> element in <nat> in "
> + "<forward> in network %s"), networkName);
> + goto cleanup;
> + }
> + addr_end = virXMLPropString(*natAddrNodes, "end");
> + if (addr_end == NULL) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("missing 'end' attribute in <address> element in <nat> in "
> + "<forward> in network %s"), networkName);
> + goto cleanup;
> + }
> + }
> +
> + if (addr_start && virSocketAddrParse(&def->addr_start, addr_start, AF_INET) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Bad ipv4 start address '%s' in <nat> in <forward> in "
> + "network '%s'"), addr_start, networkName);
> + goto cleanup;
> + }
> +
> + if (addr_end && virSocketAddrParse(&def->addr_end, addr_end, AF_INET) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Bad ipv4 end address '%s' in <nat> in <forward> in "
> + "network '%s'"), addr_end, networkName);
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + VIR_FREE(addr_start);
> + VIR_FREE(addr_end);
You forgot to 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;
I think the official rules for libvirt may only say that braces are
necessary if the body of a compound statement is > 1 line, but I prefer
to also add braces when the conditional is > 1 line.
> + }
> +
> 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)
This should be named consistent with the parse function -
virNetworkForwardNatDefFormat().
> +{
> + char *addr_start = NULL;
> + char *addr_end = NULL;
> + int ret = -1;
> +
> + if (VIR_SOCKET_ADDR_VALID(&fwd->addr_start)) {
> + addr_start = virSocketAddrFormat(&fwd->addr_start);
> + if (!addr_start)
> + goto cleanup;
> + }
> +
> + if (VIR_SOCKET_ADDR_VALID(&fwd->addr_end)) {
> + addr_end = virSocketAddrFormat(&fwd->addr_end);
> + if (!addr_end)
> + goto cleanup;
> + }
> +
> + if (!addr_end && !addr_start)
> + return 0;
> +
> + virBufferAddLit(buf, "<nat>\n");
> + virBufferAdjustIndent(buf, 2);
> +
> + virBufferAsprintf(buf, "<address start='%s'", addr_start);
> + if (addr_end)
> + virBufferAsprintf(buf, " end='%s'", addr_end);
> + virBufferAsprintf(buf, "/>\n");
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAsprintf(buf, "</nat>\n");
> + ret = 0;
> +
> +cleanup:
> + VIR_FREE(addr_start);
> + VIR_FREE(addr_end);
> + 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.addr_start)
> + || VIR_SOCKET_ADDR_VALID(&def->forward.addr_end));
Yes, nice refactor to eliminate the duplicated conditional below.
> + 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..1a598e3 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 addr_start, addr_end;
> };
>
> typedef struct _virPortGroupDef virPortGroupDef;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index c834f83..6d74c1f 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.addr_start,
> + &network->def->forward.addr_end,
> NULL) < 0) {
> virReportError(VIR_ERR_SYSTEM_ERROR,
> forwardIf ?
> @@ -1601,6 +1603,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> "udp") < 0) {
> virReportError(VIR_ERR_SYSTEM_ERROR,
> forwardIf ?
> @@ -1615,6 +1619,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> "tcp") < 0) {
> virReportError(VIR_ERR_SYSTEM_ERROR,
> forwardIf ?
> @@ -1631,12 +1637,16 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> "udp");
> masqerr4:
> iptablesRemoveForwardMasquerade(driver->iptables,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> NULL);
> masqerr3:
> iptablesRemoveForwardAllowRelatedIn(driver->iptables,
> @@ -1667,16 +1677,22 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> "tcp");
> iptablesRemoveForwardMasquerade(driver->iptables,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> "udp");
> iptablesRemoveForwardMasquerade(driver->iptables,
> &ipdef->address,
> prefix,
> forwardIf,
> + &network->def->forward.addr_start,
> + &network->def->forward.addr_end,
> NULL);
>
> iptablesRemoveForwardAllowRelatedIn(driver->iptables,
> diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> index 41fe780..3f0dcf0 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -805,11 +805,15 @@ iptablesForwardMasquerade(iptablesContext *ctx,
> virSocketAddr *netaddr,
> unsigned int prefix,
> const char *physdev,
> + virSocketAddr *addr_start,
> + virSocketAddr *addr_end,
> const char *protocol,
> int action)
> {
> - int ret;
> - char *networkstr;
> + int ret = -1;
> + char *networkstr = NULL;
> + char *addr_start_str = NULL;
> + char *addr_end_str = NULL;
> virCommandPtr cmd = NULL;
>
> if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
> @@ -820,8 +824,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(addr_start, AF_INET)) {
> + addr_start_str = virSocketAddrFormat(addr_start);
> + if (!addr_start_str)
> + goto cleanup;
> + if (VIR_SOCKET_ADDR_IS_FAMILY(addr_end, AF_INET)) {
> + addr_end_str = virSocketAddrFormat(addr_end);
> + if (!addr_end_str)
> + goto cleanup;
The indentation went wrong here.
> + }
> }
>
> cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action);
> @@ -835,12 +849,32 @@ 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 (addr_start_str && addr_start_str[0]) {
> + char tmpstr[sizeof("123.123.123.123-123.123.123.123:65535-65535")];
Rather than doing this and calling snprintf, we prefer to call
virAsprintf(), which allocates the memory as required and doesn't
consume space on the stack (of course then you have to free the memory
afterwards). I changed tmpstr into addrRangeStr, declared at the
toplevel of the function, changed the snprintf's to virAsprintfs() (with
proper checking for error return), and freed addrRangeStr during the
function's cleanup.
> + const char *portstr = "";
> +
> + memset(tmpstr, 0, sizeof(tmpstr));
> + if (protocol && protocol[0])
> + portstr = ":1024-65535";
> + if (addr_end_str && addr_end_str[0]) {
> + snprintf(tmpstr, sizeof(tmpstr), "%s-%s%s",
> + addr_start_str, addr_end_str, portstr);
> + } else {
> + snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, portstr);
> + }
>
> - if (protocol && protocol[0])
> - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL);
> + 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);
You didn't free addr_start_str or addr_end_str.
> return ret;
> }
> @@ -863,9 +897,11 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
> virSocketAddr *netaddr,
> unsigned int prefix,
> const char *physdev,
> + virSocketAddr *addr_start,
> + virSocketAddr *addr_end,
> const char *protocol)
> {
> - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD);
> + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, ADD);
> }
>
> /**
> @@ -886,9 +922,11 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
> virSocketAddr *netaddr,
> unsigned int prefix,
> const char *physdev,
> + virSocketAddr *addr_start,
> + virSocketAddr *addr_end,
> const char *protocol)
> {
> - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE);
> + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, REMOVE);
> }
>
>
> diff --git a/src/util/viriptables.h b/src/util/viriptables.h
> index d7fa731..4241380 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 *addr_start,
> + virSocketAddr *addr_end,
> const char *protocol);
> int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
> virSocketAddr *netaddr,
> unsigned int prefix,
> const char *physdev,
> + virSocketAddr *addr_start,
> + virSocketAddr *addr_end,
> const char *protocol);
> int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
> const char *iface,
If I'd seen the need to change snprintf() to virAsprintf() when I
started, I might have just marked up this diff and asked for a V3, but
by the time I got there, I'd already done the other trivial changes I
listed above, so I made the virAsprintf() change as well.
Can someone check the interdiff I'm attaching to this message. If that
gets an ACK, then
I'll ACK the combination of that with the original (because the
interdiff addresses everything I mention in my review) and push.
>From 753cdc954aa34127feaf2cda894744fbcc0cb77e Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@xxxxxxxxx>
Date: Tue, 12 Feb 2013 16:54:41 -0500
Subject: [PATCH] changes from review of nat address range patch
---
src/conf/network_conf.c | 68 +++++++++++++++++++++++----------------------
src/conf/network_conf.h | 2 +-
src/network/bridge_driver.c | 32 ++++++++++-----------
src/util/viriptables.c | 61 +++++++++++++++++++++++-----------------
src/util/viriptables.h | 8 +++---
5 files changed, 91 insertions(+), 80 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 61d086a..d7b31cf 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1333,8 +1333,8 @@ virNetworkForwardNatDefParseXML(const char *networkName,
int ret = -1;
xmlNodePtr *natAddrNodes = NULL;
int nNatAddrs;
- char *addr_start = NULL;
- char *addr_end = NULL;
+ char *addrStart = NULL;
+ char *addrEnd = NULL;
xmlNodePtr save = ctxt->node;
ctxt->node = node;
@@ -1359,15 +1359,15 @@ virNetworkForwardNatDefParseXML(const char *networkName,
"<forward> in network %s"), networkName);
goto cleanup;
} else if (nNatAddrs == 1) {
- addr_start = virXMLPropString(*natAddrNodes, "start");
- if (addr_start == NULL) {
+ 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;
}
- addr_end = virXMLPropString(*natAddrNodes, "end");
- if (addr_end == NULL) {
+ 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);
@@ -1375,25 +1375,27 @@ virNetworkForwardNatDefParseXML(const char *networkName,
}
}
- if (addr_start && virSocketAddrParse(&def->addr_start, addr_start, AF_INET) < 0) {
+ 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'"), addr_start, networkName);
+ "network '%s'"), addrStart, networkName);
goto cleanup;
}
- if (addr_end && virSocketAddrParse(&def->addr_end, addr_end, AF_INET) < 0) {
+ 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'"), addr_end, networkName);
+ "network '%s'"), addrEnd, networkName);
goto cleanup;
}
ret = 0;
cleanup:
- VIR_FREE(addr_start);
- VIR_FREE(addr_end);
+ VIR_FREE(addrStart);
+ VIR_FREE(addrEnd);
+ VIR_FREE(natAddrNodes);
ctxt->node = save;
return ret;
}
@@ -1472,9 +1474,9 @@ virNetworkForwardDefParseXML(const char *networkName,
goto cleanup;
} else if (nForwardNats == 1) {
if (virNetworkForwardNatDefParseXML(networkName,
- *forwardNatNodes,
- ctxt, def) < 0)
+ *forwardNatNodes, ctxt, def) < 0) {
goto cleanup;
+ }
}
if (((nForwardIfs > 0) + (nForwardAddrs > 0) + (nForwardPfs > 0)) > 1) {
@@ -2173,34 +2175,34 @@ virPortGroupDefFormat(virBufferPtr buf,
}
static int
-virNatDefFormat(virBufferPtr buf,
- const virNetworkForwardDefPtr fwd)
+virNetworkForwardNatDefFormat(virBufferPtr buf,
+ const virNetworkForwardDefPtr fwd)
{
- char *addr_start = NULL;
- char *addr_end = NULL;
+ char *addrStart = NULL;
+ char *addrEnd = NULL;
int ret = -1;
- if (VIR_SOCKET_ADDR_VALID(&fwd->addr_start)) {
- addr_start = virSocketAddrFormat(&fwd->addr_start);
- if (!addr_start)
+ if (VIR_SOCKET_ADDR_VALID(&fwd->addrStart)) {
+ addrStart = virSocketAddrFormat(&fwd->addrStart);
+ if (!addrStart)
goto cleanup;
}
- if (VIR_SOCKET_ADDR_VALID(&fwd->addr_end)) {
- addr_end = virSocketAddrFormat(&fwd->addr_end);
- if (!addr_end)
+ if (VIR_SOCKET_ADDR_VALID(&fwd->addrEnd)) {
+ addrEnd = virSocketAddrFormat(&fwd->addrEnd);
+ if (!addrEnd)
goto cleanup;
}
- if (!addr_end && !addr_start)
+ if (!addrEnd && !addrStart)
return 0;
virBufferAddLit(buf, "<nat>\n");
virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf, "<address start='%s'", addr_start);
- if (addr_end)
- virBufferAsprintf(buf, " end='%s'", addr_end);
+ virBufferAsprintf(buf, "<address start='%s'", addrStart);
+ if (addrEnd)
+ virBufferAsprintf(buf, " end='%s'", addrEnd);
virBufferAsprintf(buf, "/>\n");
virBufferAdjustIndent(buf, -2);
@@ -2208,8 +2210,8 @@ virNatDefFormat(virBufferPtr buf,
ret = 0;
cleanup:
- VIR_FREE(addr_start);
- VIR_FREE(addr_end);
+ VIR_FREE(addrStart);
+ VIR_FREE(addrEnd);
return ret;
}
@@ -2258,13 +2260,13 @@ virNetworkDefFormatInternal(virBufferPtr buf,
virBufferAddLit(buf, " managed='no'");
}
shortforward = !(def->forward.nifs || def->forward.npfs
- || VIR_SOCKET_ADDR_VALID(&def->forward.addr_start)
- || VIR_SOCKET_ADDR_VALID(&def->forward.addr_end));
+ || 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)
+ if (virNetworkForwardNatDefFormat(buf, &def->forward) < 0)
goto error;
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 1a598e3..11d6c9c 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -176,7 +176,7 @@ struct _virNetworkForwardDef {
virNetworkForwardIfDefPtr ifs;
/* adresses for SNAT */
- virSocketAddr addr_start, addr_end;
+ virSocketAddr addrStart, addrEnd;
};
typedef struct _virPortGroupDef virPortGroupDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6d74c1f..9f502a5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1587,8 +1587,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
NULL) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
@@ -1603,8 +1603,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
"udp") < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
@@ -1619,8 +1619,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
"tcp") < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
@@ -1637,16 +1637,16 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
"udp");
masqerr4:
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
NULL);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
@@ -1677,22 +1677,22 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
"tcp");
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &network->def->forward.addrStart,
+ &network->def->forward.addrEnd,
"udp");
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- &network->def->forward.addr_start,
- &network->def->forward.addr_end,
+ &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 3f0dcf0..f8a09bf 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -805,15 +805,16 @@ iptablesForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- virSocketAddr *addr_start,
- virSocketAddr *addr_end,
+ virSocketAddr *addrStart,
+ virSocketAddr *addrEnd,
const char *protocol,
int action)
{
int ret = -1;
char *networkstr = NULL;
- char *addr_start_str = NULL;
- char *addr_end_str = NULL;
+ char *addrStartStr = NULL;
+ char *addrEndStr = NULL;
+ char *addrRangeStr = NULL;
virCommandPtr cmd = NULL;
if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
@@ -827,13 +828,13 @@ iptablesForwardMasquerade(iptablesContext *ctx,
goto cleanup;
}
- if (VIR_SOCKET_ADDR_IS_FAMILY(addr_start, AF_INET)) {
- addr_start_str = virSocketAddrFormat(addr_start);
- if (!addr_start_str)
+ if (VIR_SOCKET_ADDR_IS_FAMILY(addrStart, AF_INET)) {
+ addrStartStr = virSocketAddrFormat(addrStart);
+ if (!addrStartStr)
goto cleanup;
- if (VIR_SOCKET_ADDR_IS_FAMILY(addr_end, AF_INET)) {
- addr_end_str = virSocketAddrFormat(addr_end);
- if (!addr_end_str)
+ if (VIR_SOCKET_ADDR_IS_FAMILY(addrEnd, AF_INET)) {
+ addrEndStr = virSocketAddrFormat(addrEnd);
+ if (!addrEndStr)
goto cleanup;
}
}
@@ -849,23 +850,26 @@ iptablesForwardMasquerade(iptablesContext *ctx,
if (physdev && physdev[0])
virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
- /* Use --jump SNAT if public addr is specified */
- if (addr_start_str && addr_start_str[0]) {
- char tmpstr[sizeof("123.123.123.123-123.123.123.123:65535-65535")];
+ /* Use --jump SNAT if public addr is spec1ified */
+ if (addrStartStr && addrStartStr[0]) {
const char *portstr = "";
- memset(tmpstr, 0, sizeof(tmpstr));
if (protocol && protocol[0])
portstr = ":1024-65535";
- if (addr_end_str && addr_end_str[0]) {
- snprintf(tmpstr, sizeof(tmpstr), "%s-%s%s",
- addr_start_str, addr_end_str, portstr);
+ if (addrEndStr && addrEndStr[0]) {
+ if (virAsprintf(&addrRangeStr, "%s-%s%s",
+ addrStartStr, addrEndStr, portstr) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
} else {
- snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, portstr);
+ if (virAsprintf(&addrRangeStr, "%s%s", addrStartStr, portstr) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
}
-
virCommandAddArgList(cmd, "--jump", "SNAT",
- "--to-source", tmpstr, NULL);
+ "--to-source", addrRangeStr, NULL);
} else {
virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL);
@@ -876,6 +880,9 @@ iptablesForwardMasquerade(iptablesContext *ctx,
ret = iptablesCommandRunAndFree(cmd);
cleanup:
VIR_FREE(networkstr);
+ VIR_FREE(addrStartStr);
+ VIR_FREE(addrEndStr);
+ VIR_FREE(addrRangeStr);
return ret;
}
@@ -897,11 +904,12 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- virSocketAddr *addr_start,
- virSocketAddr *addr_end,
+ virSocketAddr *addrStart,
+ virSocketAddr *addrEnd,
const char *protocol)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, ADD);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev,
+ addrStart, addrEnd, protocol, ADD);
}
/**
@@ -922,11 +930,12 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- virSocketAddr *addr_start,
- virSocketAddr *addr_end,
+ virSocketAddr *addrStart,
+ virSocketAddr *addrEnd,
const char *protocol)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, 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 4241380..05362da 100644
--- a/src/util/viriptables.h
+++ b/src/util/viriptables.h
@@ -107,15 +107,15 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- virSocketAddr *addr_start,
- virSocketAddr *addr_end,
+ virSocketAddr *addrStart,
+ virSocketAddr *addrEnd,
const char *protocol);
int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- virSocketAddr *addr_start,
- virSocketAddr *addr_end,
+ virSocketAddr *addrStart,
+ virSocketAddr *addrEnd,
const char *protocol);
int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
const char *iface,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list