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@redhat.com>

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 1f4d39e0d7b6bf7f0ef539b17218cda7ab69395e..2b794a1d73626af2fdd0120edd8aebe5cb281c89 100644 (file)
--- 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;
-
-    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 1b3b523be3f243ae0982d38a3dd6008cc8443829..d3b05bfa30fa09c0749fa4d3c68df6620ea1b868 100644 (file)
--- 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;
 }