Convert nwfilter ebiptablesTearNewRules to virFirewall

Convert the nwfilter ebiptablesTearNewRules 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 2b794a1d73626af2fdd0120edd8aebe5cb281c89..293e585c30b0b6f64c038d809b375c54044da88d 100644 (file)
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -722,6 +722,18 @@ iptablesRemoveTmpRootChain(virBufferPtr buf,
 }
 
 
+static void
+iptablesRemoveTmpRootChainFW(virFirewallPtr fw,
+                             virFirewallLayer layer,
+                             char prefix,
+                             bool incoming,
+                             const char *ifname)
+{
+    _iptablesRemoveRootChainFW(fw, layer, prefix,
+                               incoming, ifname, 1);
+}
+
+
 static void
 iptablesRemoveTmpRootChains(virBufferPtr buf,
                             const char *ifname)
@@ -732,6 +744,17 @@ iptablesRemoveTmpRootChains(virBufferPtr buf,
 }
 
 
+static void
+iptablesRemoveTmpRootChainsFW(virFirewallPtr fw,
+                              virFirewallLayer layer,
+                              const char *ifname)
+{
+    iptablesRemoveTmpRootChainFW(fw, layer, 'F', false, ifname);
+    iptablesRemoveTmpRootChainFW(fw, layer, 'F', true, ifname);
+    iptablesRemoveTmpRootChainFW(fw, layer, 'H', true, ifname);
+}
+
+
 static void
 iptablesRemoveRootChainsFW(virFirewallPtr fw,
                            virFirewallLayer layer,
@@ -933,6 +956,18 @@ iptablesUnlinkTmpRootChain(virBufferPtr buf,
 }
 
 
+static void
+iptablesUnlinkTmpRootChainFW(virFirewallPtr fw,
+                             virFirewallLayer layer,
+                             const char *basechain,
+                             char prefix,
+                             bool incoming, const char *ifname)
+{
+    _iptablesUnlinkRootChainFW(fw, layer,
+                               basechain, prefix, incoming, ifname, 1);
+}
+
+
 static void
 iptablesUnlinkRootChainsFW(virFirewallPtr fw,
                            virFirewallLayer layer,
@@ -954,6 +989,17 @@ iptablesUnlinkTmpRootChains(virBufferPtr buf,
 }
 
 
+static void
+iptablesUnlinkTmpRootChainsFW(virFirewallPtr fw,
+                              virFirewallLayer layer,
+                              const char *ifname)
+{
+    iptablesUnlinkTmpRootChainFW(fw, layer, VIRT_OUT_CHAIN, 'F', false, ifname);
+    iptablesUnlinkTmpRootChainFW(fw, layer, VIRT_IN_CHAIN,  'F', true, ifname);
+    iptablesUnlinkTmpRootChainFW(fw, layer, HOST_IN_CHAIN,  'H', true, ifname);
+}
+
+
 static void
 iptablesRenameTmpRootChainFW(virFirewallPtr fw,
                              virFirewallLayer layer,
@@ -4251,36 +4297,28 @@ ebiptablesApplyNewRules(const char *ifname,
 static int
 ebiptablesTearNewRules(const char *ifname)
 {
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
-
-    if (iptables_cmd_path) {
-        NWFILTER_SET_IPTABLES_SHELLVAR(&buf);
-
-        iptablesUnlinkTmpRootChains(&buf, ifname);
-        iptablesRemoveTmpRootChains(&buf, ifname);
-    }
-
-    if (ip6tables_cmd_path) {
-        NWFILTER_SET_IP6TABLES_SHELLVAR(&buf);
+    virFirewallPtr fw = virFirewallNew();
+    int ret = -1;
 
-        iptablesUnlinkTmpRootChains(&buf, ifname);
-        iptablesRemoveTmpRootChains(&buf, ifname);
-    }
+    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
 
-    if (ebtables_cmd_path) {
-        NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
+    iptablesUnlinkTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
+    iptablesRemoveTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
 
-        ebtablesUnlinkTmpRootChain(&buf, true, ifname);
-        ebtablesUnlinkTmpRootChain(&buf, false, ifname);
+    iptablesUnlinkTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
+    iptablesRemoveTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
 
-        ebtablesRemoveTmpSubChains(&buf, ifname);
-        ebtablesRemoveTmpRootChain(&buf, true, ifname);
-        ebtablesRemoveTmpRootChain(&buf, false, ifname);
-    }
-
-    ebiptablesExecCLI(&buf, true, NULL);
+    ebtablesUnlinkTmpRootChainFW(fw, true, ifname);
+    ebtablesUnlinkTmpRootChainFW(fw, false, ifname);
+    ebtablesRemoveTmpSubChainsFW(fw, ifname);
+    ebtablesRemoveTmpRootChainFW(fw, true, ifname);
+    ebtablesRemoveTmpRootChainFW(fw, false, 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 d3b05bfa30fa09c0749fa4d3c68df6620ea1b868..2cd93291cbf2997d63001cb47b7b1e1919cb3988 100644 (file)
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -216,6 +216,67 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque ATTRIBUTE_UNUSED)
 }
 
 
+static int
+testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    const char *expected =
+        "iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n"
+        "iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n"
+        "iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n"
+        "iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n"
+        "iptables -F FP-vnet0\n"
+        "iptables -X FP-vnet0\n"
+        "iptables -F FJ-vnet0\n"
+        "iptables -X FJ-vnet0\n"
+        "iptables -F HJ-vnet0\n"
+        "iptables -X HJ-vnet0\n"
+        "ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FP-vnet0\n"
+        "ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FP-vnet0\n"
+        "ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FJ-vnet0\n"
+        "ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HJ-vnet0\n"
+        "ip6tables -F FP-vnet0\n"
+        "ip6tables -X FP-vnet0\n"
+        "ip6tables -F FJ-vnet0\n"
+        "ip6tables -X FJ-vnet0\n"
+        "ip6tables -F HJ-vnet0\n"
+        "ip6tables -X HJ-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.tearNewRules("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)
 {
@@ -241,6 +302,11 @@ mymain(void)
                     NULL) < 0)
         ret = -1;
 
+    if (virtTestRun("ebiptablesTearNewRules",
+                    testNWFilterEBIPTablesTearNewRules,
+                    NULL) < 0)
+        ret = -1;
+
  cleanup:
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }