[libvirt] [PATCH 18/26] Convert nwfilter ebtablesRemoveBasicRules to virFirewall

Convert the nwfilter ebtablesRemoveBasicRules 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 | 95 ++++++++++++++++--------------- tests/nwfilterebiptablestest.c | 52 +++++++++++++++++ 2 files changed, 100 insertions(+), 47 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index da4d096..7740960 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -206,7 +206,7 @@ static const char *m_physdev_out_old_str = "-m physdev --physdev-out"; static int ebtablesRemoveBasicRules(const char *ifname); static int ebiptablesDriverInit(bool privileged); static void ebiptablesDriverShutdown(void); -static void ebtablesCleanAll(const char *ifname); +static int ebtablesCleanAll(const char *ifname); static int ebiptablesAllTeardown(const char *ifname); static virMutex execCLIMutex = VIR_MUTEX_INITIALIZER; @@ -2901,14 +2901,6 @@ _ebtablesRemoveRootChainFW(virFirewallPtr fw, static void -ebtablesRemoveRootChain(virBufferPtr buf, - bool incoming, const char *ifname) -{ - _ebtablesRemoveRootChain(buf, incoming, ifname, false); -} - - -static void ebtablesRemoveRootChainFW(virFirewallPtr fw, bool incoming, const char *ifname) { @@ -2925,6 +2917,14 @@ ebtablesRemoveTmpRootChain(virBufferPtr buf, static void +ebtablesRemoveTmpRootChainFW(virFirewallPtr fw, + bool incoming, const char *ifname) +{ + _ebtablesRemoveRootChainFW(fw, incoming, ifname, 1); +} + + +static void _ebtablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname, bool isTempChain) @@ -2978,14 +2978,6 @@ _ebtablesUnlinkRootChainFW(virFirewallPtr fw, static void -ebtablesUnlinkRootChain(virBufferPtr buf, - bool incoming, const char *ifname) -{ - _ebtablesUnlinkRootChain(buf, incoming, ifname, false); -} - - -static void ebtablesUnlinkRootChainFW(virFirewallPtr fw, bool incoming, const char *ifname) { @@ -3001,6 +2993,14 @@ ebtablesUnlinkTmpRootChain(virBufferPtr buf, } +static void +ebtablesUnlinkTmpRootChainFW(virFirewallPtr fw, + int incoming, const char *ifname) +{ + _ebtablesUnlinkRootChainFW(fw, incoming, ifname, 1); +} + + static int ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, int *nRuleInstances, @@ -3166,8 +3166,8 @@ _ebtablesRemoveSubChainsFW(virFirewallPtr fw, } static void -ebtablesRemoveSubChains(virBufferPtr buf, - const char *ifname) +ebtablesRemoveSubChainsFW(virFirewallPtr fw, + const char *ifname) { char chains[3] = { CHAINPREFIX_HOST_IN, @@ -3175,25 +3175,25 @@ ebtablesRemoveSubChains(virBufferPtr buf, 0 }; - _ebtablesRemoveSubChains(buf, ifname, chains); + _ebtablesRemoveSubChainsFW(fw, ifname, chains); } static void -ebtablesRemoveSubChainsFW(virFirewallPtr fw, - const char *ifname) +ebtablesRemoveTmpSubChains(virBufferPtr buf, + const char *ifname) { char chains[3] = { - CHAINPREFIX_HOST_IN, - CHAINPREFIX_HOST_OUT, + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, 0 }; - _ebtablesRemoveSubChainsFW(fw, ifname, chains); + _ebtablesRemoveSubChains(buf, ifname, chains); } static void -ebtablesRemoveTmpSubChains(virBufferPtr buf, - const char *ifname) +ebtablesRemoveTmpSubChainsFW(virFirewallPtr fw, + const char *ifname) { char chains[3] = { CHAINPREFIX_HOST_IN_TEMP, @@ -3201,7 +3201,7 @@ ebtablesRemoveTmpSubChains(virBufferPtr buf, 0 }; - _ebtablesRemoveSubChains(buf, ifname, chains); + _ebtablesRemoveSubChainsFW(fw, ifname, chains); } static void @@ -3668,34 +3668,35 @@ ebtablesApplyDropAllRules(const char *ifname) static int ebtablesRemoveBasicRules(const char *ifname) { - ebtablesCleanAll(ifname); - return 0; + return ebtablesCleanAll(ifname); } -static void +static int ebtablesCleanAll(const char *ifname) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - - if (!ebtables_cmd_path) - return; + virFirewallPtr fw = virFirewallNew(); + int ret = -1; - NWFILTER_SET_EBTABLES_SHELLVAR(&buf); + virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); - ebtablesUnlinkRootChain(&buf, true, ifname); - ebtablesUnlinkRootChain(&buf, false, ifname); - ebtablesRemoveSubChains(&buf, ifname); - ebtablesRemoveRootChain(&buf, true, ifname); - ebtablesRemoveRootChain(&buf, false, ifname); + ebtablesUnlinkRootChainFW(fw, true, ifname); + ebtablesUnlinkRootChainFW(fw, false, ifname); + ebtablesRemoveSubChainsFW(fw, ifname); + ebtablesRemoveRootChainFW(fw, true, ifname); + ebtablesRemoveRootChainFW(fw, false, ifname); - ebtablesUnlinkTmpRootChain(&buf, true, ifname); - ebtablesUnlinkTmpRootChain(&buf, false, ifname); - ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, true, ifname); - ebtablesRemoveTmpRootChain(&buf, false, ifname); + ebtablesUnlinkTmpRootChainFW(fw, true, ifname); + ebtablesUnlinkTmpRootChainFW(fw, false, ifname); + ebtablesRemoveTmpSubChainsFW(fw, ifname); + ebtablesRemoveTmpRootChainFW(fw, true, ifname); + ebtablesRemoveTmpRootChainFW(fw, false, ifname); - ebiptablesExecCLI(&buf, true, NULL); + virMutexLock(&execCLIMutex); + ret = virFirewallApply(fw); + virMutexUnlock(&execCLIMutex); + virFirewallFree(fw); + return ret; } diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c index 85077f1..11b7fb5 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -164,6 +164,53 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque ATTRIBUTE_UNUSED) static int +testNWFilterEBIPTablesRemoveBasicRules(const void *opaque ATTRIBUTE_UNUSED) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *expected = + "/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 -D PREROUTING -i vnet0 -j libvirt-J-vnet0\n" + "/usr/sbin/ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-P-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 -F libvirt-J-vnet0\n" + "/usr/sbin/ebtables -t nat -X libvirt-J-vnet0\n" + "/usr/sbin/ebtables -t nat -F libvirt-P-vnet0\n" + "/usr/sbin/ebtables -t nat -X libvirt-P-vnet0\n"; + const char *actual = NULL; + int ret = -1; + + virCommandSetDryRun(&buf, NULL, NULL); + + if (ebiptables_driver.removeBasicRules("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; @@ -183,6 +230,11 @@ mymain(void) NULL) < 0) ret = -1; + if (virtTestRun("ebiptablesRemoveBasicRules", + testNWFilterEBIPTablesRemoveBasicRules, + NULL) < 0) + ret = -1; + cleanup: return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.9.0