8:02 a.m.
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(a)alpinelinux.org>
---
This started with me wanting to add support for setting the public ip source
address when network mode='nat' and there are multiple public ip addresses
on the external interface.
On IRC we talked about adding an option in the xml like this:
<network>
<forward mode='nat' publicaddr='n.n.n.n'/>
</network>
Which would make iptables use '-j SNAT --to-source n.n.n.n' instead of
'-j MASQUERADE'.
However with the current approach we would need to construct 8 different
iptables command combinations, hence the need for refactoring.
Thanks!
src/util/iptables.c | 130 +++++++++++++++++++++++-----------------------------
1 file changed, 58 insertions(+), 72 deletions(-)
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 00a1c29..407ca3a 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -127,15 +127,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) {
@@ -152,16 +147,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);
}
/**
@@ -370,28 +385,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;
}
@@ -797,6 +808,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
{
int ret;
char *networkstr;
+ virCommandPtr cmd = NULL;
if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
return -1;
@@ -810,49 +822,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.0
9:53 a.m.
On Thu, 22 Nov 2012 15:02:18 +0100
Natanael Copa <ncopa(a)alpinelinux.org> 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(a)alpinelinux.org>
---
This started with me wanting to add support for setting the public ip source
address when network mode='nat' and there are multiple public ip addresses
on the external interface.
On IRC we talked about adding an option in the xml like this:
<network>
<forward mode='nat' publicaddr='n.n.n.n'/>
</network>
Which would make iptables use '-j SNAT --to-source n.n.n.n' instead of
'-j MASQUERADE'.
However with the current approach we would need to construct 8 different
iptables command combinations, hence the need for refactoring.
I forgot to add that I havent' tested if the code runs.
-nc
9:28 a.m.
On Thu, 22 Nov 2012 15:02:18 +0100
Natanael Copa <ncopa(a)alpinelinux.org> 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(a)alpinelinux.org>
---
This started with me wanting to add support for setting the public ip source
address when network mode='nat' and there are multiple public ip addresses
on the external interface.
On IRC we talked about adding an option in the xml like this:
<network>
<forward mode='nat' publicaddr='n.n.n.n'/>
</network>
Which would make iptables use '-j SNAT --to-source n.n.n.n' instead of
'-j MASQUERADE'.
I have a working patch for the above and it appears to work. I wonder
if we want go for 'publicaddr' as the attribute name?
-nc
2:54 p.m.
New subject: [libvirt] [PATCH] net: support set public ip for forward mode nat
Support setting which public ip to use for NAT via attribute
publicaddr. This will construct an iptables line using '-j SNAT
--to-source <publicaddr>' instead of '-j MASQUERADE'.
Signed-off-by: Natanael Copa <ncopa(a)alpinelinux.org>
---
This depends on previous sent iptables refactor patch.
http://www.mail-archive.com/libvir-list@redhat.com/msg66788.html
The validation of the publicaddr is very basic, but at least it is a
start.
Please let me know if you have a better suggestion for the attribute
name 'publicaddr' and I'll resend the patch.
Thanks!
docs/formatnetwork.html.in | 4 +++-
src/conf/network_conf.c | 33 +++++++++++++++++++++++++++++++++
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 24 ++++++++++++++++--------
src/util/iptables.c | 31 ++++++++++++++++++++++++-------
src/util/iptables.h | 6 ++++--
6 files changed, 81 insertions(+), 18 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..07f9783 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -125,7 +125,9 @@
other network device whether ethernet, wireless, dialup,
or VPN. If the <code>dev</code> attribute is set, the
firewall rules will restrict forwarding to the named
- device only. Inbound connections from other networks are
+ device only. If the <code>publicaddr</code> attribute is set,
+ the given source address will be used with iptables' SNAT
+ target. Inbound connections from other networks are
all prohibited; all connections between guests on the same
network, and to/from the host to the guests, are
unrestricted and not NATed.<span class="since">Since
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6ce2e63..36128ac 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,6 +174,7 @@ void virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def->name);
VIR_FREE(def->bridge);
VIR_FREE(def->domain);
+ VIR_FREE(def->publicaddr);
for (ii = 0 ; ii < def->nForwardPfs && def->forwardPfs ; ii++) {
virNetworkForwardPfDefClear(&def->forwardPfs[ii]);
@@ -1211,6 +1212,22 @@ error:
return result;
}
+static int
+virValidPublicaddr(const char *publicaddr)
+{
+ /* only check for max len and valid chars for now */
+ const int maxlen =
sizeof("123.123.123.123-123.123.123.123:65535-65534")-1;
+ int len = strlen(publicaddr);
+
+ if (len > maxlen)
+ return 0;
+
+ if (strspn(publicaddr, "0123456789.-:") < len)
+ return 0;
+
+ return 1;
+}
+
static virNetworkDefPtr
virNetworkDefParseXML(xmlXPathContextPtr ctxt)
{
@@ -1387,6 +1404,21 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->managed = 1;
}
+ def->publicaddr = virXPathString("string(./@publicaddr)", ctxt);
+ if (def->publicaddr != NULL) {
+ char *errstr = NULL;
+ if (def->forwardType != VIR_NETWORK_FORWARD_NAT) {
+ errstr = "Attribute 'publicaddr' is only valid with
mode='nat'";
+ } else if (!virValidPublicaddr(def->publicaddr)) {
+ errstr = "Attribute 'publicaddr' must be in the format:
ipaddr[-ipaddr][:port[-port]]";
+ }
+
+ if (errstr != NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s", _(errstr));
+ goto error;
+ }
+ }
+
/* all of these modes can use a pool of physical interfaces */
nForwardIfs = virXPathNodeSet("./interface", ctxt,
&forwardIfNodes);
nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);
@@ -1861,6 +1893,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int
flags)
}
virBufferAddLit(&buf, "<forward");
virBufferEscapeString(&buf, " dev='%s'", dev);
+ virBufferEscapeString(&buf, " publicaddr='%s'",
def->publicaddr);
virBufferAsprintf(&buf, " mode='%s'", mode);
if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
if (def->managed == 1)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3e46304..76fb591 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -206,6 +206,7 @@ struct _virNetworkDef {
virPortGroupDefPtr portGroups;
virNetDevBandwidthPtr bandwidth;
virNetDevVlan vlan;
+ char *publicaddr;
};
typedef struct _virNetworkObj virNetworkObj;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 75f3c3a..04f178b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1438,7 +1438,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- NULL) < 0) {
+ NULL,
+ network->def->publicaddr) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable masquerading to
%s") :
@@ -1452,7 +1453,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- "udp") < 0) {
+ "udp",
+ network->def->publicaddr) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable UDP masquerading to
%s") :
@@ -1466,7 +1468,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- "tcp") < 0) {
+ "tcp",
+ network->def->publicaddr) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable TCP masquerading to
%s") :
@@ -1482,13 +1485,15 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
&ipdef->address,
prefix,
forwardIf,
- "udp");
+ "udp",
+ network->def->publicaddr);
masqerr4:
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- NULL);
+ NULL,
+ network->def->publicaddr);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&ipdef->address,
@@ -1518,17 +1523,20 @@ networkRemoveMasqueradingIptablesRules(struct network_driver
*driver,
&ipdef->address,
prefix,
forwardIf,
- "tcp");
+ "tcp",
+ network->def->publicaddr);
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- "udp");
+ "udp",
+ network->def->publicaddr);
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- NULL);
+ NULL,
+ network->def->publicaddr);
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&ipdef->address,
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 407ca3a..4a89673 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -804,6 +804,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
unsigned int prefix,
const char *physdev,
const char *protocol,
+ const char *publicaddr,
int action)
{
int ret;
@@ -833,10 +834,24 @@ 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 (publicaddr && publicaddr[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] && (strchr(publicaddr, ':')
== NULL))
+ portstr = ":1024-65535";
+ snprintf(tmpstr, sizeof(tmpstr), "%s%s", publicaddr, 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);
VIR_FREE(networkstr);
@@ -861,9 +876,10 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol)
+ const char *protocol,
+ const char *publicaddr)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, publicaddr,
ADD);
}
/**
@@ -884,9 +900,10 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol)
+ const char *protocol,
+ const char *publicaddr)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, publicaddr,
REMOVE);
}
diff --git a/src/util/iptables.h b/src/util/iptables.h
index e54f8b1..a9d2772 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -105,12 +105,14 @@ int iptablesAddForwardMasquerade (iptablesContext
*ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol);
+ const char *protocol,
+ const char *publicaddr);
int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol);
+ const char *protocol,
+ const char *publicaddr);
int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
const char *iface,
int port);
--
1.8.0.1
1:50 p.m.
New subject: [libvirt] [PATCH] net: support set public ip for forward mode nat
On 12/04/2012 03:54 PM, Natanael Copa wrote:
Support setting which public ip to use for NAT via attribute
publicaddr. This will construct an iptables line using '-j SNAT
--to-source <publicaddr>' instead of '-j MASQUERADE'.
Signed-off-by: Natanael Copa <ncopa(a)alpinelinux.org>
---
This depends on previous sent iptables refactor patch.
http://www.mail-archive.com/libvir-list@redhat.com/msg66788.html
The validation of the publicaddr is very basic, but at least it is a
start.
Please let me know if you have a better suggestion for the attribute
name 'publicaddr' and I'll resend the patch.
Thanks!
docs/formatnetwork.html.in | 4 +++-
src/conf/network_conf.c | 33 +++++++++++++++++++++++++++++++++
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 24 ++++++++++++++++--------
src/util/iptables.c | 31 ++++++++++++++++++++++++-------
src/util/iptables.h | 6 ++++--
6 files changed, 81 insertions(+), 18 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..07f9783 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -125,7 +125,9 @@
other network device whether ethernet, wireless, dialup,
or VPN. If the <code>dev</code> attribute is set, the
firewall rules will restrict forwarding to the named
- device only. Inbound connections from other networks are
+ device only. If the <code>publicaddr</code> attribute is set,
+ the given source address will be used with iptables' SNAT
+ target. Inbound connections from other networks are
all prohibited; all connections between guests on the same
network, and to/from the host to the guests, are
unrestricted and not NATed.<span class="since">Since
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6ce2e63..36128ac 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,6 +174,7 @@ void virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def->name);
VIR_FREE(def->bridge);
VIR_FREE(def->domain);
+ VIR_FREE(def->publicaddr);
for (ii = 0 ; ii < def->nForwardPfs && def->forwardPfs ; ii++) {
virNetworkForwardPfDefClear(&def->forwardPfs[ii]);
@@ -1211,6 +1212,22 @@ error:
return result;
}
+static int
+virValidPublicaddr(const char *publicaddr)
+{
+ /* only check for max len and valid chars for now */
+ const int maxlen =
sizeof("123.123.123.123-123.123.123.123:65535-65534")-1;
+ int len = strlen(publicaddr);
+
+ if (len > maxlen)
+ return 0;
+
+ if (strspn(publicaddr, "0123456789.-:") < len)
+ return 0;
+
+ return 1;
+}
+
static virNetworkDefPtr
virNetworkDefParseXML(xmlXPathContextPtr ctxt)
{
@@ -1387,6 +1404,21 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->managed = 1;
}
+ def->publicaddr = virXPathString("string(./@publicaddr)", ctxt);
+ if (def->publicaddr != NULL) {
+ char *errstr = NULL;
+ if (def->forwardType != VIR_NETWORK_FORWARD_NAT) {
+ errstr = "Attribute 'publicaddr' is only valid with
mode='nat'";
+ } else if (!virValidPublicaddr(def->publicaddr)) {
+ errstr = "Attribute 'publicaddr' must be in the format:
ipaddr[-ipaddr][:port[-port]]";
In libvirt we don't like having multiple attributes embedded in a single
XML attribute. If a range of addresses and ports is to be supported,
then there should be a separate attribute for the start and end of each
of these ranges (or perhaps a separate subelement of <forward>, just as
dhcp ranges are a subelement of <dhcp>).
Maybe you could put these in a <nat> subelement of <forward>? Something
like:
<forward mode='nat'>
<nat addrstart='1.2.3.4' addrend='1.2.3.10'
portstart='500'
portend='1000'/>
</forward>
(so the simplest case would just add "<nat
addrstart='1.2.3.4'/>")
or even:
<forward mode='nat'>
<nat>
<address start='1.2.3.4' end='1.2.3.10'/>
<port start='500' end='1000'/>
</nat>
</forward>
(the simplest case would add "<nat> <address start='1.2.3.4'/>
</nat>")
Anybody else have a suggestion or opinion?
+ }
+
+ if (errstr != NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s", _(errstr));
+ goto error;
A few times in the past I've seen (maybe even written?) this type of
"set a pointer to an error log string, then log it later" code, and I
recall that it was frowned upon. I'm not even sure that the right thing
will happen with translation if you do it this way (I think you would
have to put the _() around the string constants directly).
At any rate, it's just as simple to immediately call virReportError and
goto error when the error is encountered, rather than have this extra step.
+ }
+ }
+
/* all of these modes can use a pool of physical interfaces */
nForwardIfs = virXPathNodeSet("./interface", ctxt,
&forwardIfNodes);
nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);
@@ -1861,6 +1893,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int
flags)
}
virBufferAddLit(&buf, "<forward");
virBufferEscapeString(&buf, " dev='%s'", dev);
+ virBufferEscapeString(&buf, " publicaddr='%s'",
def->publicaddr);
virBufferAsprintf(&buf, " mode='%s'", mode);
if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
if (def->managed == 1)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3e46304..76fb591 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -206,6 +206,7 @@ struct _virNetworkDef {
virPortGroupDefPtr portGroups;
virNetDevBandwidthPtr bandwidth;
virNetDevVlan vlan;
+ char *publicaddr;
};
typedef struct _virNetworkObj virNetworkObj;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 75f3c3a..04f178b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1438,7 +1438,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- NULL) < 0) {
+ NULL,
+ network->def->publicaddr) < 0) {
Aside from the fact that this will now either be multiple args (or maybe
a struct that contains all 4), I think I prefer this to be earlier in
the arglist, maybe just after forwardIf.
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable masquerading to
%s") :
@@ -1452,7 +1453,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- "udp") < 0) {
+ "udp",
+ network->def->publicaddr) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable UDP masquerading to
%s") :
@@ -1466,7 +1468,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
&ipdef->address,
prefix,
forwardIf,
- "tcp") < 0) {
+ "tcp",
+ network->def->publicaddr) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
forwardIf ?
_("failed to add iptables rule to enable TCP masquerading to
%s") :
@@ -1482,13 +1485,15 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
&ipdef->address,
prefix,
forwardIf,
- "udp");
+ "udp",
+ network->def->publicaddr);
masqerr4:
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- NULL);
+ NULL,
+ network->def->publicaddr);
masqerr3:
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&ipdef->address,
@@ -1518,17 +1523,20 @@ networkRemoveMasqueradingIptablesRules(struct network_driver
*driver,
&ipdef->address,
prefix,
forwardIf,
- "tcp");
+ "tcp",
+ network->def->publicaddr);
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- "udp");
+ "udp",
+ network->def->publicaddr);
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
prefix,
forwardIf,
- NULL);
+ NULL,
+ network->def->publicaddr);
iptablesRemoveForwardAllowRelatedIn(driver->iptables,
&ipdef->address,
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 407ca3a..4a89673 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -804,6 +804,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
unsigned int prefix,
const char *physdev,
const char *protocol,
+ const char *publicaddr,
int action)
{
int ret;
@@ -833,10 +834,24 @@ 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 (publicaddr && publicaddr[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);
Hmm. Having the ports as separate attributes would allow specifying a
port range without an address range (which has been requested before)
+ memset(tmpstr, 0, sizeof(tmpstr));
+ if (protocol && protocol[0] && (strchr(publicaddr, ':')
== NULL))
+ portstr = ":1024-65535";
+ snprintf(tmpstr, sizeof(tmpstr), "%s%s", publicaddr, 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);
VIR_FREE(networkstr);
@@ -861,9 +876,10 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol)
+ const char *protocol,
+ const char *publicaddr)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, ADD);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol,
publicaddr, ADD);
}
/**
@@ -884,9 +900,10 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol)
+ const char *protocol,
+ const char *publicaddr)
{
- return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol, REMOVE);
+ return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, protocol,
publicaddr, REMOVE);
}
diff --git a/src/util/iptables.h b/src/util/iptables.h
index e54f8b1..a9d2772 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -105,12 +105,14 @@ int iptablesAddForwardMasquerade (iptablesContext
*ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol);
+ const char *protocol,
+ const char *publicaddr);
int iptablesRemoveForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
const char *physdev,
- const char *protocol);
+ const char *protocol,
+ const char *publicaddr);
int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
const char *iface,
int port);
4369
days inactive
4384
days old
4 comments
2 participants
participants (2)
-
Laine Stump -
Natanael Copa