[libvirt] [PATCH 22/26] Convert nwfilter ebtablesApplyDropAllRules to virFirewall

Convert the nwfilter ebtablesApplyDropAllRules 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 | 93 ++++++++----------------------- tests/nwfilterebiptablestest.c | 75 +++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 69 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 62866ac..db6e324 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3284,31 +3284,6 @@ ebtablesRemoveTmpSubChainsFW(virFirewallPtr fw, } static void -ebtablesRenameTmpSubChain(virBufferPtr buf, - bool 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); - } - - virBufferAsprintf(buf, - "$EBT -t nat -E %s %s" CMD_SEPARATOR, - tmpchain, chain); -} - -static void ebtablesRenameTmpSubChainFW(virFirewallPtr fw, int incoming, const char *ifname, @@ -3333,14 +3308,6 @@ ebtablesRenameTmpSubChainFW(virFirewallPtr fw, } static void -ebtablesRenameTmpRootChain(virBufferPtr buf, - bool incoming, - const char *ifname) -{ - ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); -} - -static void ebtablesRenameTmpRootChainFW(virFirewallPtr fw, bool incoming, const char *ifname) @@ -3642,60 +3609,48 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, static int ebtablesApplyDropAllRules(const char *ifname) { - virBuffer buf = VIR_BUFFER_INITIALIZER; char chain_in [MAX_CHAINNAME_LENGTH], chain_out[MAX_CHAINNAME_LENGTH]; + virFirewallPtr fw = virFirewallNew(); - if (!ebtables_cmd_path) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create rules since ebtables tool is " - "missing.")); - return -1; - } - - ebiptablesAllTeardown(ifname); + if (ebiptablesAllTeardown(ifname) < 0) + goto error; - NWFILTER_SET_EBTABLES_SHELLVAR(&buf); + virFirewallStartTransaction(fw, 0); - ebtablesCreateTmpRootChain(&buf, true, ifname); - ebtablesCreateTmpRootChain(&buf, false, ifname); + ebtablesCreateTmpRootChainFW(fw, true, ifname); + ebtablesCreateTmpRootChainFW(fw, false, ifname); PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname); PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname); - virBufferAsprintf(&buf, - CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR - CMD_EXEC - "%s", - - chain_in, - CMD_STOPONERR(true)); - - virBufferAsprintf(&buf, - CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR - CMD_EXEC - "%s", + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET, + "-t", "nat", "-A", chain_in, + "-j", "DROP", NULL); - chain_out, - CMD_STOPONERR(true)); + virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET, + "-t", "nat", "-A", chain_out, + "-j", "DROP", NULL); - ebtablesLinkTmpRootChain(&buf, true, ifname); - ebtablesLinkTmpRootChain(&buf, false, ifname); - ebtablesRenameTmpRootChain(&buf, true, ifname); - ebtablesRenameTmpRootChain(&buf, false, ifname); + ebtablesLinkTmpRootChainFW(fw, true, ifname); + ebtablesLinkTmpRootChainFW(fw, false, ifname); + ebtablesRenameTmpRootChainFW(fw, true, ifname); + ebtablesRenameTmpRootChainFW(fw, false, ifname); - if (ebiptablesExecCLI(&buf, false, NULL) < 0) + virMutexLock(&execCLIMutex); + if (virFirewallApply(fw) < 0) { + virMutexUnlock(&execCLIMutex); goto tear_down_tmpebchains; + } + virMutexUnlock(&execCLIMutex); + virFirewallFree(fw); return 0; tear_down_tmpebchains: ebtablesCleanAll(ifname); - - virReportError(VIR_ERR_BUILD_FIREWALL, - "%s", - _("Some rules could not be created.")); - + error: + virFirewallFree(fw); return -1; } diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c index 3010668..f994331 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -426,6 +426,76 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque ATTRIBUTE_UNUSED) } + +static int +testNWFilterEBIPTablesApplyDropAllRules(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 -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\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/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 -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\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/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 -N libvirt-J-vnet0\n" + "/usr/sbin/ebtables -t nat -N libvirt-P-vnet0\n" + "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -j DROP\n" + "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -j DROP\n" + "/usr/sbin/ebtables -t nat -A PREROUTING -i vnet0 -j libvirt-J-vnet0\n" + "/usr/sbin/ebtables -t nat -A POSTROUTING -o vnet0 -j 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.applyDropAllRules("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) { @@ -466,6 +536,11 @@ mymain(void) NULL) < 0) ret = -1; + if (virtTestRun("ebiptablesApplyDropAllRules", + testNWFilterEBIPTablesApplyDropAllRules, + NULL) < 0) + ret = -1; + cleanup: return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.9.0