[libvirt] [PATCH 17/26] Convert nwfilter ebiptablesTearOldRules to virFirewall

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(a)redhat.com&gt; --- 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