[libvirt] [PATCH v2 0/4] net: support set source address(es) and ports for NAT

This is a rework of the previous sent 'set public ip for nat' patches [1]. Changes v2: - Use separate attributes for addresses and ports as suggested by Laine. - support set port range without setting public ip [1] http://www.redhat.com/archives/libvir-list/2012-December/msg00140.html Natanael Copa (4): util: refactor iptables command construction into multiple steps net: support set public ip for forward mode nat net: support a public address range for forward mode nat net: add support for specifying port range for forward mode nat docs/formatnetwork.html.in | 37 ++++++++ src/conf/network_conf.c | 206 ++++++++++++++++++++++++++++++++++++++++++-- src/conf/network_conf.h | 4 + src/network/bridge_driver.c | 32 +++++++ src/util/viriptables.c | 195 +++++++++++++++++++++++++---------------- src/util/viriptables.h | 8 ++ 6 files changed, 402 insertions(+), 80 deletions(-) -- 1.8.1.2

Instead of creating an iptables command in one shot, do it in steps so we can add conditional options like physdev and protocol. This removes code duplication while keeping existing behaviour. Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- This patch is unmodified since last time i sent it [1]. [1] https://www.redhat.com/archives/libvir-list/2012-November/msg00986.html src/util/viriptables.c | 130 ++++++++++++++++++++++--------------------------- 1 file changed, 58 insertions(+), 72 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 2c4290a..b03b0e9 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -129,15 +129,10 @@ iptRulesNew(const char *table, return NULL; } -static int ATTRIBUTE_SENTINEL -iptablesAddRemoveRule(iptRules *rules, int family, int action, - const char *arg, ...) +static virCommandPtr +iptablesCommandNew(iptRules *rules, int family, int action) { - va_list args; - int ret; virCommandPtr cmd = NULL; - const char *s; - #if HAVE_FIREWALLD virIpTablesInitialize(); if (firewall_cmd_path) { @@ -154,16 +149,36 @@ iptablesAddRemoveRule(iptRules *rules, int family, int action, virCommandAddArgList(cmd, "--table", rules->table, action == ADD ? "--insert" : "--delete", - rules->chain, arg, NULL); + rules->chain, NULL); + return cmd; +} + +static int +iptablesCommandRunAndFree(virCommandPtr cmd) +{ + int ret; + ret = virCommandRun(cmd, NULL); + virCommandFree(cmd); + return ret; +} + +static int ATTRIBUTE_SENTINEL +iptablesAddRemoveRule(iptRules *rules, int family, int action, + const char *arg, ...) +{ + va_list args; + virCommandPtr cmd = NULL; + const char *s; + + cmd = iptablesCommandNew(rules, family, action); + virCommandAddArg(cmd, arg); va_start(args, arg); while ((s = va_arg(args, const char *))) virCommandAddArg(cmd, s); va_end(args); - ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + return iptablesCommandRunAndFree(cmd); } /** @@ -372,28 +387,24 @@ iptablesForwardAllowOut(iptablesContext *ctx, { int ret; char *networkstr; + virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; - if (physdev && physdev[0]) { - ret = iptablesAddRemoveRule(ctx->forward_filter, - VIR_SOCKET_ADDR_FAMILY(netaddr), - action, - "--source", networkstr, - "--in-interface", iface, - "--out-interface", physdev, - "--jump", "ACCEPT", - NULL); - } else { - ret = iptablesAddRemoveRule(ctx->forward_filter, - VIR_SOCKET_ADDR_FAMILY(netaddr), - action, - "--source", networkstr, - "--in-interface", iface, - "--jump", "ACCEPT", - NULL); - } + cmd = iptablesCommandNew(ctx->forward_filter, + VIR_SOCKET_ADDR_FAMILY(netaddr), + action); + virCommandAddArgList(cmd, + "--source", networkstr, + "--in-interface", iface, NULL); + + if (physdev && physdev[0]) + virCommandAddArgList(cmd, "--out-interface", physdev, NULL); + + virCommandAddArgList(cmd, "--jump", "ACCEPT", NULL); + + ret = iptablesCommandRunAndFree(cmd); VIR_FREE(networkstr); return ret; } @@ -799,6 +810,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, { int ret; char *networkstr; + virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; @@ -812,49 +824,23 @@ iptablesForwardMasquerade(iptablesContext *ctx, return -1; } - if (protocol && protocol[0]) { - if (physdev && physdev[0]) { - ret = iptablesAddRemoveRule(ctx->nat_postrouting, - AF_INET, - action, - "--source", networkstr, - "-p", protocol, - "!", "--destination", networkstr, - "--out-interface", physdev, - "--jump", "MASQUERADE", - "--to-ports", "1024-65535", - NULL); - } else { - ret = iptablesAddRemoveRule(ctx->nat_postrouting, - AF_INET, - action, - "--source", networkstr, - "-p", protocol, - "!", "--destination", networkstr, - "--jump", "MASQUERADE", - "--to-ports", "1024-65535", - NULL); - } - } else { - if (physdev && physdev[0]) { - ret = iptablesAddRemoveRule(ctx->nat_postrouting, - AF_INET, - action, - "--source", networkstr, - "!", "--destination", networkstr, - "--out-interface", physdev, - "--jump", "MASQUERADE", - NULL); - } else { - ret = iptablesAddRemoveRule(ctx->nat_postrouting, - AF_INET, - action, - "--source", networkstr, - "!", "--destination", networkstr, - "--jump", "MASQUERADE", - NULL); - } - } + cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action); + virCommandAddArgList(cmd, "--source", networkstr, NULL); + + if (protocol && protocol[0]) + virCommandAddArgList(cmd, "-p", protocol, NULL); + + virCommandAddArgList(cmd, "!", "--destination", networkstr, NULL); + + if (physdev && physdev[0]) + virCommandAddArgList(cmd, "--out-interface", physdev, NULL); + + virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); + + if (protocol && protocol[0]) + virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + + ret = iptablesCommandRunAndFree(cmd); VIR_FREE(networkstr); return ret; } -- 1.8.1.2

On Mon, Feb 04, 2013 at 10:45:23AM +0100, Natanael Copa wrote:
Instead of creating an iptables command in one shot, do it in steps so we can add conditional options like physdev and protocol.
This removes code duplication while keeping existing behaviour.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- This patch is unmodified since last time i sent it [1].
[1] https://www.redhat.com/archives/libvir-list/2012-November/msg00986.html
src/util/viriptables.c | 130 ++++++++++++++++++++++--------------------------- 1 file changed, 58 insertions(+), 72 deletions(-)
ACK, looks fine. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/08/2013 10:25 AM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 10:45:23AM +0100, Natanael Copa wrote:
Instead of creating an iptables command in one shot, do it in steps so we can add conditional options like physdev and protocol.
This removes code duplication while keeping existing behaviour.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- This patch is unmodified since last time i sent it [1].
[1] https://www.redhat.com/archives/libvir-list/2012-November/msg00986.html
src/util/viriptables.c | 130 ++++++++++++++++++++++--------------------------- 1 file changed, 58 insertions(+), 72 deletions(-)
ACK, looks fine.
Well, almost fine - it had an embedded TAB and failed 'make syntax-check'. But I fixed that with the following squash-in, and pushed: diff --git i/src/util/viriptables.c w/src/util/viriptables.c index b03b0e9..41fe780 100644 --- i/src/util/viriptables.c +++ w/src/util/viriptables.c @@ -1,7 +1,7 @@ /* * viriptables.c: helper APIs for managing iptables * - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -397,7 +397,7 @@ iptablesForwardAllowOut(iptablesContext *ctx, action); virCommandAddArgList(cmd, "--source", networkstr, - "--in-interface", iface, NULL); + "--in-interface", iface, NULL); if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL); -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Support setting which public ip to use for NAT via attribute address in subelement <nat> in <forward>: ... <forward mode='nat'> <nat address='1.2.3.4'/> </forward> ... This will construct an iptables line using: '-j SNAT --to-source <address>' instead of: '-j MASQUERADE' Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 13 ++++++ src/conf/network_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++--- src/conf/network_conf.h | 3 ++ src/network/bridge_driver.c | 8 ++++ src/util/viriptables.c | 45 +++++++++++++++----- src/util/viriptables.h | 2 + 6 files changed, 156 insertions(+), 15 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7b42529..4ab1a75 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -136,6 +136,19 @@ 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 the public IPv4 address to be used for the NAT by using + the <code><nat></code> subelement and attribute + <code>address</code>: + <pre> +... + <forward mode='nat'> + <nat address='1.2.3.4'/> + </forward> +... + </pre> + </p> </dd> <dt><code>route</code></dt> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index c93916d..a9aa139 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1325,6 +1325,43 @@ cleanup: } static int +virNetworkForwardNatDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkForwardDefPtr def) +{ + int ret = -1; + char *addr_start = 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 */ + addr_start = virXPathString("string(./@address)", ctxt); + + if (addr_start && virSocketAddrParse(&def->addr_start, addr_start, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 address '%s' in <nat> in <forward> in " + "network '%s'"), addr_start, networkName); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(addr_start); + ctxt->node = save; + return ret; +} + +static int virNetworkForwardDefParseXML(const char *networkName, xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -1334,7 +1371,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 +1422,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 +1581,7 @@ cleanup: VIR_FREE(forwardPfNodes); VIR_FREE(forwardIfNodes); VIR_FREE(forwardAddrNodes); + VIR_FREE(forwardNatNodes); ctxt->node = save; return ret; } @@ -2078,13 +2135,40 @@ virPortGroupDefFormat(virBufferPtr buf, } static int +virNatDefFormat(virBufferPtr buf, + const virNetworkForwardDefPtr fwd) +{ + char *addr_start = NULL; + int ret = -1; + + if (VIR_SOCKET_ADDR_VALID(&fwd->addr_start)) { + addr_start = virSocketAddrFormat(&fwd->addr_start); + if (!addr_start) + goto cleanup; + } + + if (!addr_start) + return 0; + + virBufferAddLit(buf, "<nat"); + virBufferAsprintf(buf, " address='%s'", addr_start); + virBufferAsprintf(buf, "/>\n"); + + ret = 0; + +cleanup: + VIR_FREE(addr_start); + 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)) { @@ -2121,10 +2205,16 @@ 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)); + 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", @@ -2154,7 +2244,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..4a5ce92 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -174,6 +174,9 @@ struct _virNetworkForwardDef { size_t nifs; virNetworkForwardIfDefPtr ifs; + + /* adress for SNAT */ + virSocketAddr addr_start; }; typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 21255f0..05ef19c 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1590,6 +1590,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1604,6 +1605,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1618,6 +1620,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1634,12 +1637,14 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1670,16 +1675,19 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, prefix, forwardIf, + &network->def->forward.addr_start, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index b03b0e9..12d1b2e 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -397,7 +397,7 @@ iptablesForwardAllowOut(iptablesContext *ctx, action); virCommandAddArgList(cmd, "--source", networkstr, - "--in-interface", iface, NULL); + "--in-interface", iface, NULL); if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL); @@ -805,11 +805,13 @@ iptablesForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addr_start, const char *protocol, int action) { - int ret; - char *networkstr; + int ret = -1; + char *networkstr = NULL; + char *addr_start_str = NULL; virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -820,8 +822,13 @@ 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; } cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action); @@ -835,12 +842,28 @@ 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")]; + const char *portstr = ""; - if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + memset(tmpstr, 0, sizeof(tmpstr)); + if (protocol && protocol[0]) + portstr = ":1024-65535"; + + snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, portstr); + + 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); return ret; } @@ -863,9 +886,10 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addr_start, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, protocol, ADD); } /** @@ -886,9 +910,10 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addr_start, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, protocol, REMOVE); } diff --git a/src/util/viriptables.h b/src/util/viriptables.h index d7fa731..b75a5ea 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -107,11 +107,13 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addr_start, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, unsigned int prefix, const char *physdev, + virSocketAddr *addr_start, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface, -- 1.8.1.2

On Mon, Feb 04, 2013 at 10:45:24AM +0100, Natanael Copa wrote:
Support setting which public ip to use for NAT via attribute address in subelement <nat> in <forward>:
... <forward mode='nat'> <nat address='1.2.3.4'/> </forward>
Unless I'm mis-understanding, this is just identical to using a range, with the start + end addresses equal eg <forward mode='nat'> <nat> <address start='1.2.3.4' end='1.2.3.4'/> </nat> </forward> if so, then this is redundant - we should just use the <address start='1.2.3.4' end='1.2.3.4'/> syntax for everything, and not special case the scenario where start+end are equal. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Fri, 8 Feb 2013 17:25:11 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Mon, Feb 04, 2013 at 10:45:24AM +0100, Natanael Copa wrote:
Support setting which public ip to use for NAT via attribute address in subelement <nat> in <forward>:
... <forward mode='nat'> <nat address='1.2.3.4'/> </forward>
Unless I'm mis-understanding, this is just identical to using a range, with the start + end addresses equal eg
<forward mode='nat'> <nat> <address start='1.2.3.4' end='1.2.3.4'/> </nat> </forward>
Almost, its redundant with no 'end' attribute. <forward mode='nat'> <nat> <address start='1.2.3.4'/> </nat> </forward>
if so, then this is redundant - we should just use the <address start='1.2.3.4' end='1.2.3.4'/> syntax for everything, and not special case the scenario where start+end are equal.
I sent a rebase without 2/4 which is still somewhat redundant. If you specify 'start', but not 'end', it will generate different iptables lines: iptables ... --to-source 1.2.3.4 vs iptables ... --to-source 1.2.3.4-1.2.3.4 It appears that iptables currently accepts both forms and generate the same thing. I cannot guarantee that iptables maintainers will remove duplicate ways of specifying same rules in future or treat them different, so it might be an idea to be able to specify both variants. -nc

Allow setting a range of public ip addresses to be used as source address for forward mode nat: ... <forward mode='nat'> <nat> <address start='1.2.3.4' end='1.2.3.10'/> </nat> </forward> ... Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- docs/formatnetwork.html.in | 13 ++++++++ src/conf/network_conf.c | 72 ++++++++++++++++++++++++++++++++++++++++++--- src/conf/network_conf.h | 4 +-- src/network/bridge_driver.c | 8 +++++ src/util/viriptables.c | 21 ++++++++++--- src/util/viriptables.h | 2 ++ 6 files changed, 110 insertions(+), 10 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 4ab1a75..608fce1 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -148,6 +148,19 @@ </forward> ... </pre> + An address range can be set with the subelement + <code><address></code>. The range which will be + used in a round-robin: + <pre> +... + <forward mode='nat'> + <nat> + <address start='1.2.3.4' end='1.2.3.10'/> + </nat> + </forward> +... + </pre> + </p> </dd> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index a9aa139..fb57b70 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1331,7 +1331,10 @@ virNetworkForwardNatDefParseXML(const char *networkName, virNetworkForwardDefPtr def) { int ret = -1; + xmlNodePtr *natAddrNodes = NULL; + int nNatAddrs; char *addr_start = NULL; + char *addr_end = NULL; xmlNodePtr save = ctxt->node; ctxt->node = node; @@ -1346,6 +1349,35 @@ virNetworkForwardNatDefParseXML(const char *networkName, /* addresses for SNAT */ addr_start = virXPathString("string(./@address)", ctxt); + 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) { + if (addr_start != NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("the <nat> 'address' attribute cannot be used " + "when <address> sub-elements are present in " + "<forward> in network %s"), networkName); + goto cleanup; + } + 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_start && virSocketAddrParse(&def->addr_start, addr_start, AF_INET) < 0) { virReportError(VIR_ERR_XML_ERROR, _("Bad ipv4 address '%s' in <nat> in <forward> in " @@ -1353,10 +1385,18 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; } + if (addr_end && virSocketAddrParse(&def->addr_end, addr_end, AF_INET) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad ipv4 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); ctxt->node = save; return ret; } @@ -2139,6 +2179,7 @@ virNatDefFormat(virBufferPtr buf, const virNetworkForwardDefPtr fwd) { char *addr_start = NULL; + char *addr_end = NULL; int ret = -1; if (VIR_SOCKET_ADDR_VALID(&fwd->addr_start)) { @@ -2147,17 +2188,39 @@ virNatDefFormat(virBufferPtr buf, goto cleanup; } - if (!addr_start) + 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"); - virBufferAsprintf(buf, " address='%s'", addr_start); - virBufferAsprintf(buf, "/>\n"); + if (addr_start && !addr_end) + virBufferAsprintf(buf, " address='%s'", addr_start); + if (!addr_end) { + virBufferAsprintf(buf, "/>\n"); + ret = 0; + goto cleanup; + } + + virBufferAsprintf(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + + if (addr_end) + virBufferAsprintf(buf, "<address start='%s' end='%s'/>\n", + addr_start, addr_end); + + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</nat>\n"); ret = 0; cleanup: VIR_FREE(addr_start); + VIR_FREE(addr_end); return ret; } @@ -2206,7 +2269,8 @@ 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_start) + || VIR_SOCKET_ADDR_VALID(&def->forward.addr_end)); virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4a5ce92..1a598e3 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -175,8 +175,8 @@ struct _virNetworkForwardDef { size_t nifs; virNetworkForwardIfDefPtr ifs; - /* adress for SNAT */ - virSocketAddr addr_start; + /* 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 05ef19c..d444ddb 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1591,6 +1591,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, prefix, forwardIf, &network->def->forward.addr_start, + &network->def->forward.addr_end, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1606,6 +1607,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, prefix, forwardIf, &network->def->forward.addr_start, + &network->def->forward.addr_end, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1621,6 +1623,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, prefix, forwardIf, &network->def->forward.addr_start, + &network->def->forward.addr_end, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1638,6 +1641,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, prefix, forwardIf, &network->def->forward.addr_start, + &network->def->forward.addr_end, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, @@ -1645,6 +1649,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, prefix, forwardIf, &network->def->forward.addr_start, + &network->def->forward.addr_end, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1676,18 +1681,21 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, 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 12d1b2e..f2d15bf 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -806,12 +806,14 @@ iptablesForwardMasquerade(iptablesContext *ctx, unsigned int prefix, const char *physdev, virSocketAddr *addr_start, + virSocketAddr *addr_end, const char *protocol, int action) { int ret = -1; char *networkstr = NULL; char *addr_start_str = NULL; + char *addr_end_str = NULL; virCommandPtr cmd = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -829,6 +831,11 @@ iptablesForwardMasquerade(iptablesContext *ctx, 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; + } } cmd = iptablesCommandNew(ctx->nat_postrouting, AF_INET, action); @@ -850,8 +857,12 @@ iptablesForwardMasquerade(iptablesContext *ctx, memset(tmpstr, 0, sizeof(tmpstr)); if (protocol && protocol[0]) portstr = ":1024-65535"; - - snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, portstr); + 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); + } virCommandAddArgList(cmd, "--jump", "SNAT", "--to-source", tmpstr, NULL); @@ -887,9 +898,10 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, unsigned int prefix, const char *physdev, virSocketAddr *addr_start, + virSocketAddr *addr_end, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, ADD); } /** @@ -911,9 +923,10 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, unsigned int prefix, const char *physdev, virSocketAddr *addr_start, + virSocketAddr *addr_end, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, 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 b75a5ea..4241380 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -108,12 +108,14 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, 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, -- 1.8.1.2

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 | 15 ++++++++++++-- src/conf/network_conf.c | 50 +++++++++++++++++++++++++++++++++++++++++---- src/conf/network_conf.h | 3 ++- src/network/bridge_driver.c | 16 +++++++++++++++ src/util/viriptables.c | 39 +++++++++++++++++++++++++++-------- src/util/viriptables.h | 4 ++++ 6 files changed, 111 insertions(+), 16 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 608fce1..381de41 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -138,7 +138,8 @@ 0.4.2</span> <p><span class="since">Since 1.0.3</span> it is possible to - specify the public IPv4 address to be used for the NAT by using + specify the public IPv4 address(es) and ports to be used for + the NAT. A single public address can be specified with the the <code><nat></code> subelement and attribute <code>address</code>: <pre> @@ -160,7 +161,17 @@ </forward> ... </pre> - + 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> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index fb57b70..1d1dfd3 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 *addr_start = NULL; char *addr_end = NULL; xmlNodePtr save = ctxt->node; @@ -1392,6 +1393,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->port_start) < 0 + || def->port_start > 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->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); + goto cleanup; + } + } ret = 0; cleanup: @@ -2181,6 +2212,7 @@ virNatDefFormat(virBufferPtr buf, char *addr_start = NULL; char *addr_end = NULL; int ret = -1; + int longdef; if (VIR_SOCKET_ADDR_VALID(&fwd->addr_start)) { addr_start = virSocketAddrFormat(&fwd->addr_start); @@ -2194,14 +2226,15 @@ virNatDefFormat(virBufferPtr buf, goto cleanup; } - if (!addr_end && !addr_start) + longdef = addr_end || fwd->port_start || fwd->port_end; + if (!longdef && !addr_start) return 0; virBufferAddLit(buf, "<nat"); if (addr_start && !addr_end) virBufferAsprintf(buf, " address='%s'", addr_start); - if (!addr_end) { + if (!longdef) { virBufferAsprintf(buf, "/>\n"); ret = 0; goto cleanup; @@ -2214,6 +2247,13 @@ virNatDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<address start='%s' end='%s'/>\n", addr_start, addr_end); + 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"); + } + virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, "</nat>\n"); ret = 0; @@ -2270,7 +2310,9 @@ virNetworkDefFormatInternal(virBufferPtr buf, } 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.addr_end) + || def->forward.port_start + || def->forward.port_end); virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : ""); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 1a598e3..7df2426 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 addr_start, addr_end; + unsigned int port_start, port_end; }; typedef struct _virPortGroupDef virPortGroupDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d444ddb..0ea49e8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1592,6 +1592,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, NULL) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1608,6 +1610,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, "udp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1624,6 +1628,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, "tcp") < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, forwardIf ? @@ -1642,6 +1648,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, @@ -1650,6 +1658,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, @@ -1682,6 +1692,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1689,6 +1701,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, &ipdef->address, @@ -1696,6 +1710,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver, forwardIf, &network->def->forward.addr_start, &network->def->forward.addr_end, + network->def->forward.port_start, + network->def->forward.port_end, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index f2d15bf..3578ab8 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -807,6 +807,8 @@ iptablesForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addr_start, virSocketAddr *addr_end, + unsigned int port_start, + unsigned int port_end, const char *protocol, int action) { @@ -815,6 +817,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, char *addr_start_str = NULL; char *addr_end_str = NULL; virCommandPtr cmd = NULL; + char port_str[sizeof(":65535-65535")] = ""; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; @@ -849,19 +852,27 @@ iptablesForwardMasquerade(iptablesContext *ctx, if (physdev && physdev[0]) virCommandAddArgList(cmd, "--out-interface", physdev, NULL); + if (protocol && protocol[0]) { + if (port_start == 0 && port_end == 0) { + port_start = 1024; + port_end = 65535; + } + + if (port_start < port_end && port_end < 65536) + snprintf(port_str, sizeof(port_str), ":%d-%d", + port_start, port_end); + } + /* 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")]; - 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); + addr_start_str, addr_end_str, port_str); } else { - snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, portstr); + snprintf(tmpstr, sizeof(tmpstr), "%s%s", addr_start_str, port_str); } virCommandAddArgList(cmd, "--jump", "SNAT", @@ -869,8 +880,8 @@ iptablesForwardMasquerade(iptablesContext *ctx, } else { virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL); - if (protocol && protocol[0]) - virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL); + if (port_str[0]) + virCommandAddArgList(cmd, "--to-ports", &port_str[1], NULL); } ret = iptablesCommandRunAndFree(cmd); @@ -899,9 +910,14 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addr_start, virSocketAddr *addr_end, + unsigned int port_start, + unsigned int port_end, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addr_start, addr_end, + port_start, port_end, + protocol, ADD); } /** @@ -924,9 +940,14 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, const char *physdev, virSocketAddr *addr_start, virSocketAddr *addr_end, + unsigned int port_start, + unsigned int port_end, const char *protocol) { - return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addr_start, addr_end, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, + addr_start, addr_end, + port_start, port_end, + protocol, REMOVE); } diff --git a/src/util/viriptables.h b/src/util/viriptables.h index 4241380..f2db368 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -109,6 +109,8 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addr_start, virSocketAddr *addr_end, + unsigned int port_start, + unsigned int port_end, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, virSocketAddr *netaddr, @@ -116,6 +118,8 @@ int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *physdev, virSocketAddr *addr_start, virSocketAddr *addr_end, + unsigned int port_start, + unsigned int port_end, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, const char *iface, -- 1.8.1.2

On Mon, Feb 04, 2013 at 10:45:22AM +0100, Natanael Copa wrote:
This is a rework of the previous sent 'set public ip for nat' patches [1].
Changes v2: - Use separate attributes for addresses and ports as suggested by Laine. - support set port range without setting public ip
Broadly looks good to me. If you get rid of patch 2 which is just a special case of patch 3, then I'd ack 3 + 4. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/08/2013 10:27 AM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 10:45:22AM +0100, Natanael Copa wrote:
This is a rework of the previous sent 'set public ip for nat' patches [1].
Changes v2: - Use separate attributes for addresses and ports as suggested by Laine. - support set port range without setting public ip
Broadly looks good to me. If you get rid of patch 2 which is just a special case of patch 3, then I'd ack 3 + 4.
I didn't want to spend time doing the rebase work myself, so I'll wait for a v2 for patches 3 and 4. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Natanael Copa