From d7b83ab7c3e8268f91ea45562e975d417806f0a5 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Fri, 14 Mar 2014 12:14:13 +0000 Subject: [PATCH] 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 --- src/nwfilter/nwfilter_ebiptables_driver.c | 99 ++++++++++++----------- tests/nwfilterebiptablestest.c | 54 +++++++++++++ 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 1f4d39e0d7..2b794a1d73 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; @@ -253,6 +253,12 @@ static char chainprefixes_host[3] = { 0 }; +static char chainprefixes_host_temp[3] = { + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, + 0 +}; + static int printVar(virNWFilterVarCombIterPtr vars, char *buf, int bufsize, @@ -2916,14 +2922,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) @@ -2940,6 +2938,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, @@ -2994,14 +3000,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) @@ -3018,6 +3016,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, @@ -3184,19 +3190,6 @@ _ebtablesRemoveSubChainsFW(virFirewallPtr fw, } } -static void -ebtablesRemoveSubChains(virBufferPtr buf, - const char *ifname) -{ - char chains[3] = { - CHAINPREFIX_HOST_IN, - CHAINPREFIX_HOST_OUT, - 0 - }; - - _ebtablesRemoveSubChains(buf, ifname, chains); -} - static void ebtablesRemoveSubChainsFW(virFirewallPtr fw, const char *ifname) @@ -3217,6 +3210,13 @@ ebtablesRemoveTmpSubChains(virBufferPtr buf, _ebtablesRemoveSubChains(buf, ifname, chains); } +static void +ebtablesRemoveTmpSubChainsFW(virFirewallPtr fw, + const char *ifname) +{ + _ebtablesRemoveSubChainsFW(fw, ifname, chainprefixes_host_temp); +} + static void ebtablesRenameTmpSubChain(virBufferPtr buf, bool incoming, @@ -3683,34 +3683,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; + virFirewallPtr fw = virFirewallNew(); + int ret = -1; - if (!ebtables_cmd_path) - return; + virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); - NWFILTER_SET_EBTABLES_SHELLVAR(&buf); + ebtablesUnlinkRootChainFW(fw, true, ifname); + ebtablesUnlinkRootChainFW(fw, false, ifname); + ebtablesRemoveSubChainsFW(fw, ifname); + ebtablesRemoveRootChainFW(fw, true, ifname); + ebtablesRemoveRootChainFW(fw, false, ifname); - ebtablesUnlinkRootChain(&buf, true, ifname); - ebtablesUnlinkRootChain(&buf, false, ifname); - ebtablesRemoveSubChains(&buf, ifname); - ebtablesRemoveRootChain(&buf, true, ifname); - ebtablesRemoveRootChain(&buf, false, ifname); + ebtablesUnlinkTmpRootChainFW(fw, true, ifname); + ebtablesUnlinkTmpRootChainFW(fw, false, ifname); + ebtablesRemoveTmpSubChainsFW(fw, ifname); + ebtablesRemoveTmpRootChainFW(fw, true, ifname); + ebtablesRemoveTmpRootChainFW(fw, false, ifname); - ebtablesUnlinkTmpRootChain(&buf, true, ifname); - ebtablesUnlinkTmpRootChain(&buf, false, ifname); - ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, true, ifname); - ebtablesRemoveTmpRootChain(&buf, 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 1b3b523be3..d3b05bfa30 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -167,6 +167,55 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque ATTRIBUTE_UNUSED) } +static int +testNWFilterEBIPTablesRemoveBasicRules(const void *opaque ATTRIBUTE_UNUSED) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *expected = + "ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0\n" + "ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0\n" + "ebtables -t nat -L libvirt-I-vnet0\n" + "ebtables -t nat -L libvirt-O-vnet0\n" + "ebtables -t nat -F libvirt-I-vnet0\n" + "ebtables -t nat -X libvirt-I-vnet0\n" + "ebtables -t nat -F libvirt-O-vnet0\n" + "ebtables -t nat -X libvirt-O-vnet0\n" + "ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-J-vnet0\n" + "ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-P-vnet0\n" + "ebtables -t nat -L libvirt-J-vnet0\n" + "ebtables -t nat -L libvirt-P-vnet0\n" + "ebtables -t nat -F libvirt-J-vnet0\n" + "ebtables -t nat -X libvirt-J-vnet0\n" + "ebtables -t nat -F libvirt-P-vnet0\n" + "ebtables -t nat -X libvirt-P-vnet0\n"; + char *actual = NULL; + int ret = -1; + + virCommandSetDryRun(&buf, NULL, NULL); + + if (ebiptables_driver.removeBasicRules("vnet0") < 0) + goto cleanup; + + if (virBufferError(&buf)) + goto cleanup; + + actual = virBufferContentAndReset(&buf); + virtTestClearCommandPath(actual); + + if (STRNEQ_NULLABLE(actual, expected)) { + virtTestDifference(stderr, actual, expected); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandSetDryRun(NULL, NULL, NULL); + virBufferFreeAndReset(&buf); + VIR_FREE(actual); + return ret; +} + + static int mymain(void) { @@ -187,6 +236,11 @@ mymain(void) NULL) < 0) ret = -1; + if (virtTestRun("ebiptablesRemoveBasicRules", + testNWFilterEBIPTablesRemoveBasicRules, + NULL) < 0) + ret = -1; + cleanup: return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }