Convert the nwfilter ebiptablesTearOldRules method to use the
virFirewall object APIs instead of creating shell scripts
using virBuffer APIs. This provides a performance improvement
through allowing direct use of firewalld dbus APIs and will
facilitate automated testing.
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 228 ++++++++++++++++--------------
tests/nwfilterebiptablestest.c | 74 ++++++++++
2 files changed, 192 insertions(+), 110 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 8a53ceb..da4d096 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -685,16 +685,6 @@ _iptablesRemoveRootChainFW(virFirewallPtr fw,
static void
-iptablesRemoveRootChain(virBufferPtr buf,
- char prefix,
- bool incoming,
- const char *ifname)
-{
- _iptablesRemoveRootChain(buf, prefix, incoming, ifname, false);
-}
-
-
-static void
iptablesRemoveRootChainFW(virFirewallPtr fw,
virFirewallLayer layer,
char prefix,
@@ -727,16 +717,6 @@ iptablesRemoveTmpRootChains(virBufferPtr buf,
static void
-iptablesRemoveRootChains(virBufferPtr buf,
- const char *ifname)
-{
- iptablesRemoveRootChain(buf, 'F', false, ifname);
- iptablesRemoveRootChain(buf, 'F', true, ifname);
- iptablesRemoveRootChain(buf, 'H', true, ifname);
-}
-
-
-static void
iptablesRemoveRootChainsFW(virFirewallPtr fw,
virFirewallLayer layer,
const char *ifname)
@@ -911,16 +891,6 @@ _iptablesUnlinkRootChainFW(virFirewallPtr fw,
static void
-iptablesUnlinkRootChain(virBufferPtr buf,
- const char *basechain,
- char prefix,
- bool incoming, const char *ifname)
-{
- _iptablesUnlinkRootChain(buf,
- basechain, prefix, incoming, ifname, false);
-}
-
-static void
iptablesUnlinkRootChainFW(virFirewallPtr fw,
virFirewallLayer layer,
const char *basechain,
@@ -944,16 +914,6 @@ iptablesUnlinkTmpRootChain(virBufferPtr buf,
static void
-iptablesUnlinkRootChains(virBufferPtr buf,
- const char *ifname)
-{
- iptablesUnlinkRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname);
- iptablesUnlinkRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname);
- iptablesUnlinkRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname);
-}
-
-
-static void
iptablesUnlinkRootChainsFW(virFirewallPtr fw,
virFirewallLayer layer,
const char *ifname)
@@ -975,10 +935,11 @@ iptablesUnlinkTmpRootChains(virBufferPtr buf,
static void
-iptablesRenameTmpRootChain(virBufferPtr buf,
- char prefix,
- bool incoming,
- const char *ifname)
+iptablesRenameTmpRootChainFW(virFirewallPtr fw,
+ virFirewallLayer layer,
+ char prefix,
+ bool incoming,
+ const char *ifname)
{
char tmpchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
char tmpChainPrefix[2] = {
@@ -995,20 +956,19 @@ iptablesRenameTmpRootChain(virBufferPtr buf,
PRINT_IPT_ROOT_CHAIN(tmpchain, tmpChainPrefix, ifname);
PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname);
- virBufferAsprintf(buf,
- "$IPT -E %s %s" CMD_SEPARATOR,
- tmpchain,
- chain);
+ virFirewallAddRule(fw, layer,
+ "-E", tmpchain, chain, NULL);
}
static void
-iptablesRenameTmpRootChains(virBufferPtr buf,
- const char *ifname)
+iptablesRenameTmpRootChainsFW(virFirewallPtr fw,
+ virFirewallLayer layer,
+ const char *ifname)
{
- iptablesRenameTmpRootChain(buf, 'F', false, ifname);
- iptablesRenameTmpRootChain(buf, 'F', true, ifname);
- iptablesRenameTmpRootChain(buf, 'H', true, ifname);
+ iptablesRenameTmpRootChainFW(fw, layer, 'F', false, ifname);
+ iptablesRenameTmpRootChainFW(fw, layer, 'F', true, ifname);
+ iptablesRenameTmpRootChainFW(fw, layer, 'H', true, ifname);
}
@@ -3270,6 +3230,30 @@ ebtablesRenameTmpSubChain(virBufferPtr buf,
}
static void
+ebtablesRenameTmpSubChainFW(virFirewallPtr fw,
+ int incoming,
+ const char *ifname,
+ const char *protocol)
+{
+ char tmpchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
+ char tmpChainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
+ : CHAINPREFIX_HOST_OUT_TEMP;
+ char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN
+ : CHAINPREFIX_HOST_OUT;
+
+ if (protocol) {
+ PRINT_CHAIN(tmpchain, tmpChainPrefix, ifname, protocol);
+ PRINT_CHAIN(chain, chainPrefix, ifname, protocol);
+ } else {
+ PRINT_ROOT_CHAIN(tmpchain, tmpChainPrefix, ifname);
+ PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
+ }
+
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-E", tmpchain, chain, NULL);
+}
+
+static void
ebtablesRenameTmpRootChain(virBufferPtr buf,
bool incoming,
const char *ifname)
@@ -3278,38 +3262,77 @@ ebtablesRenameTmpRootChain(virBufferPtr buf,
}
static void
-ebtablesRenameTmpSubAndRootChains(virBufferPtr buf,
- const char *ifname)
+ebtablesRenameTmpRootChainFW(virFirewallPtr fw,
+ bool incoming,
+ const char *ifname)
+{
+ ebtablesRenameTmpSubChainFW(fw, incoming, ifname, NULL);
+}
+
+
+static int
+ebtablesRenameTmpSubAndRootChainsQuery(virFirewallPtr fw,
+ const char *const *lines,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ size_t i;
+ char newchain[MAX_CHAINNAME_LENGTH];
+
+ for (i = 0; lines[i] != NULL; i++) {
+ VIR_DEBUG("Considering '%s'", lines[i]);
+ char *tmp = strstr(lines[i], "-j ");
+ if (!tmp)
+ continue;
+ tmp = tmp + 3;
+ if (tmp[0] != CHAINPREFIX_HOST_IN_TEMP &&
+ tmp[0] != CHAINPREFIX_HOST_OUT_TEMP)
+ continue;
+ if (tmp[1] != '-')
+ continue;
+
+ ignore_value(virStrcpyStatic(newchain, tmp));
+ if (newchain[0] == CHAINPREFIX_HOST_IN_TEMP)
+ newchain[0] = CHAINPREFIX_HOST_IN;
+ else
+ newchain[0] = CHAINPREFIX_HOST_OUT;
+ VIR_DEBUG("Renaming chain '%s' to '%s'", tmp, newchain);
+ virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ false, ebtablesRenameTmpSubAndRootChainsQuery,
+ NULL,
+ "-t", "nat", "-L", tmp, NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-F", newchain, NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-X", newchain, NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-E", tmp, newchain, NULL);
+ }
+
+ return 0;
+}
+
+
+static void
+ebtablesRenameTmpSubAndRootChainsFW(virFirewallPtr fw,
+ const char *ifname)
{
char rootchain[MAX_CHAINNAME_LENGTH];
size_t i;
char chains[3] = {
CHAINPREFIX_HOST_IN_TEMP,
CHAINPREFIX_HOST_OUT_TEMP,
- 0};
-
- NWFILTER_SET_EBTABLES_SHELLVAR(buf);
-
- virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
- chains);
- virBufferAsprintf(buf, NWFILTER_FUNC_RENAME_CHAINS,
- CHAINPREFIX_HOST_IN_TEMP,
- CHAINPREFIX_HOST_IN,
- CHAINPREFIX_HOST_OUT_TEMP,
- CHAINPREFIX_HOST_OUT);
-
- virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS);
- virBufferAddLit(buf, "chains=\"$(collect_chains");
+ 0
+ };
for (i = 0; chains[i] != 0; i++) {
PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
- virBufferAsprintf(buf, " %s", rootchain);
+ virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ false, ebtablesRenameTmpSubAndRootChainsQuery,
+ NULL,
+ "-t", "nat", "-L", rootchain, NULL);
}
- virBufferAddLit(buf, ")\"\n");
-
- virBufferAddLit(buf, "rename_chains $chains\n");
- ebtablesRenameTmpRootChain(buf, true, ifname);
- ebtablesRenameTmpRootChain(buf, false, ifname);
+ ebtablesRenameTmpRootChainFW(fw, true, ifname);
+ ebtablesRenameTmpRootChainFW(fw, false, ifname);
}
static void
@@ -4248,46 +4271,31 @@ ebiptablesTearNewRules(const char *ifname)
static int
ebiptablesTearOldRules(const char *ifname)
{
- virBuffer buf = VIR_BUFFER_INITIALIZER;
-
- /* switch to new iptables user defined chains */
- if (iptables_cmd_path) {
- NWFILTER_SET_IPTABLES_SHELLVAR(&buf);
-
- iptablesUnlinkRootChains(&buf, ifname);
- iptablesRemoveRootChains(&buf, ifname);
-
- iptablesRenameTmpRootChains(&buf, ifname);
- ebiptablesExecCLI(&buf, true, NULL);
- }
-
- if (ip6tables_cmd_path) {
- NWFILTER_SET_IP6TABLES_SHELLVAR(&buf);
-
- iptablesUnlinkRootChains(&buf, ifname);
- iptablesRemoveRootChains(&buf, ifname);
-
- iptablesRenameTmpRootChains(&buf, ifname);
- ebiptablesExecCLI(&buf, true, NULL);
- }
-
- if (ebtables_cmd_path) {
- NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
-
- ebtablesUnlinkRootChain(&buf, true, ifname);
- ebtablesUnlinkRootChain(&buf, false, ifname);
+ virFirewallPtr fw = virFirewallNew();
+ int ret = -1;
- ebtablesRemoveSubChains(&buf, ifname);
+ virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
- ebtablesRemoveRootChain(&buf, true, ifname);
- ebtablesRemoveRootChain(&buf, false, ifname);
+ iptablesUnlinkRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
+ iptablesRemoveRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
+ iptablesRenameTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
- ebtablesRenameTmpSubAndRootChains(&buf, ifname);
+ iptablesUnlinkRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
+ iptablesRemoveRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
+ iptablesRenameTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
- ebiptablesExecCLI(&buf, true, NULL);
- }
+ ebtablesUnlinkRootChainFW(fw, true, ifname);
+ ebtablesUnlinkRootChainFW(fw, false, ifname);
+ ebtablesRemoveSubChainsFW(fw, ifname);
+ ebtablesRemoveRootChainFW(fw, true, ifname);
+ ebtablesRemoveRootChainFW(fw, false, ifname);
+ ebtablesRenameTmpSubAndRootChainsFW(fw, ifname);
- return 0;
+ virMutexLock(&execCLIMutex);
+ ret = virFirewallApply(fw);
+ virMutexUnlock(&execCLIMutex);
+ virFirewallFree(fw);
+ return ret;
}
diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index c05682b..85077f1 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -95,6 +95,75 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque ATTRIBUTE_UNUSED)
static int
+testNWFilterEBIPTablesTearOldRules(const void *opaque ATTRIBUTE_UNUSED)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *expected =
+ "/usr/sbin/iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+ "/usr/sbin/iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+ "/usr/sbin/iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+ "/usr/sbin/iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+ "/usr/sbin/iptables -F FO-vnet0\n"
+ "/usr/sbin/iptables -X FO-vnet0\n"
+ "/usr/sbin/iptables -F FI-vnet0\n"
+ "/usr/sbin/iptables -X FI-vnet0\n"
+ "/usr/sbin/iptables -F HI-vnet0\n"
+ "/usr/sbin/iptables -X HI-vnet0\n"
+ "/usr/sbin/iptables -E FP-vnet0 FO-vnet0\n"
+ "/usr/sbin/iptables -E FJ-vnet0 FI-vnet0\n"
+ "/usr/sbin/iptables -E HJ-vnet0 HI-vnet0\n"
+ "/usr/sbin/ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+ "/usr/sbin/ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+ "/usr/sbin/ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+ "/usr/sbin/ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+ "/usr/sbin/ip6tables -F FO-vnet0\n"
+ "/usr/sbin/ip6tables -X FO-vnet0\n"
+ "/usr/sbin/ip6tables -F FI-vnet0\n"
+ "/usr/sbin/ip6tables -X FI-vnet0\n"
+ "/usr/sbin/ip6tables -F HI-vnet0\n"
+ "/usr/sbin/ip6tables -X HI-vnet0\n"
+ "/usr/sbin/ip6tables -E FP-vnet0 FO-vnet0\n"
+ "/usr/sbin/ip6tables -E FJ-vnet0 FI-vnet0\n"
+ "/usr/sbin/ip6tables -E HJ-vnet0 HI-vnet0\n"
+ "/usr/sbin/ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0\n"
+ "/usr/sbin/ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0\n"
+ "/usr/sbin/ebtables -t nat -L libvirt-I-vnet0\n"
+ "/usr/sbin/ebtables -t nat -L libvirt-O-vnet0\n"
+ "/usr/sbin/ebtables -t nat -F libvirt-I-vnet0\n"
+ "/usr/sbin/ebtables -t nat -X libvirt-I-vnet0\n"
+ "/usr/sbin/ebtables -t nat -F libvirt-O-vnet0\n"
+ "/usr/sbin/ebtables -t nat -X libvirt-O-vnet0\n"
+ "/usr/sbin/ebtables -t nat -L libvirt-J-vnet0\n"
+ "/usr/sbin/ebtables -t nat -L libvirt-P-vnet0\n"
+ "/usr/sbin/ebtables -t nat -E libvirt-J-vnet0 libvirt-I-vnet0\n"
+ "/usr/sbin/ebtables -t nat -E libvirt-P-vnet0 libvirt-O-vnet0\n";
+ const char *actual = NULL;
+ int ret = -1;
+
+ virCommandSetDryRun(&buf, NULL, NULL);
+
+ if (ebiptables_driver.tearOldRules("vnet0") < 0)
+ goto cleanup;
+
+ if (virBufferError(&buf))
+ goto cleanup;
+
+ actual = virBufferCurrentContent(&buf);
+
+ if (STRNEQ_NULLABLE(actual, expected)) {
+ virtTestDifference(stderr, actual, expected);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ virCommandSetDryRun(NULL, NULL, NULL);
+ virBufferFreeAndReset(&buf);
+ return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -109,6 +178,11 @@ mymain(void)
NULL) < 0)
ret = -1;
+ if (virtTestRun("ebiptablesTearOldRules",
+ testNWFilterEBIPTablesTearOldRules,
+ NULL) < 0)
+ ret = -1;
+
cleanup:
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.9.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list