Re: [PATCH 16/26] Convert nwfilter ebiptablesAllTeardown to virFirewall

[Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>]

On 04/08/2014 11:38 AM, Daniel P. Berrange wrote:

Convert the nwfilter ebiptablesAllTeardown 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@xxxxxxxxxx>

@@ -2972,6 +3153,58 @@ _ebtablesRemoveSubChains(virBufferPtr buf,
     virBufferAddLit(buf, "rm_chains $chains\n");
 }

+
+static int
+ebtablesRemoveSubChainsQuery(virFirewallPtr fw,
+                             const char *const *lines,
+                             void *opaque)
+{
+    size_t i, j;
+    const char *chains = opaque;
+

The problem here is 'opaque' may have gone out of scope ... [continue below]

+    for (i = 0; lines[i] != NULL; i++) {
+        VIR_DEBUG("Considering '%s'", lines[i]);
+        char *tmp = strstr(lines[i], "-j ");
+        if (!tmp)
+            continue;
+        tmp = tmp + 3;
+        for (j = 0; chains[j]; j++) {
+            if (tmp[0] == chains[j] &&
+                tmp[1] == '-') {
+                VIR_DEBUG("Processing chain '%s'", tmp);
+                virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                                       false, ebtablesRemoveSubChainsQuery,
+                                       (void *)chains,
+                                        "-t", "nat", "-L", tmp, NULL);
+                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                                   "-t", "nat", "-F", tmp, NULL);
+                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                                   "-t", "nat", "-X", tmp, NULL);
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+static void
+_ebtablesRemoveSubChainsFW(virFirewallPtr fw,
+                           const char *ifname,
+                           const char *chains)
+{
+    char rootchain[MAX_CHAINNAME_LENGTH];
+    size_t i;
+

[...]

@@ -2986,6 +3219,19 @@ ebtablesRemoveSubChains(virBufferPtr buf,
 }

 static void
+ebtablesRemoveSubChainsFW(virFirewallPtr fw,
+                          const char *ifname)
+{
+    char chains[3] = {
+        CHAINPREFIX_HOST_IN,
+        CHAINPREFIX_HOST_OUT,
+        0
+    };
+

... due to these arrays residing on the stack. So I prepended 'static' to these arrays and the ebtables rules cleanup started working.

My suggested modification to this patch would be this here:

Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -139,6 +139,19 @@ static const struct ushort_map l3_protoc
 };
 
 
+static char chainprefixes_host[3] = {
+    CHAINPREFIX_HOST_IN,
+    CHAINPREFIX_HOST_OUT,
+    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,
@@ -2621,7 +2634,7 @@ ebtablesRemoveSubChainsQuery(virFirewall
                              void *opaque)
 {
     size_t i, j;
-    const char *chains = opaque;
+    const char *chainprefixes = opaque;
 
     for (i = 0; lines[i] != NULL; i++) {
         VIR_DEBUG("Considering '%s'", lines[i]);
@@ -2629,13 +2642,13 @@ ebtablesRemoveSubChainsQuery(virFirewall
         if (!tmp)
             continue;
         tmp = tmp + 3;
-        for (j = 0; chains[j]; j++) {
-            if (tmp[0] == chains[j] &&
+        for (j = 0; chainprefixes[j]; j++) {
+            if (tmp[0] == chainprefixes[j] &&
                 tmp[1] == '-') {
                 VIR_DEBUG("Processing chain '%s'", tmp);
                 virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                        false, ebtablesRemoveSubChainsQuery,
-                                       (void *)chains,
+                                       (void *)chainprefixes,
                                         "-t", "nat", "-L", tmp, NULL);
                 virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                    "-t", "nat", "-F", tmp, NULL);
@@ -2652,16 +2665,16 @@ ebtablesRemoveSubChainsQuery(virFirewall
 static void
 _ebtablesRemoveSubChainsFW(virFirewallPtr fw,
                            const char *ifname,
-                           const char *chains)
+                           const char *chainprefixes)
 {
     char rootchain[MAX_CHAINNAME_LENGTH];
     size_t i;
 
-    for (i = 0; chains[i] != 0; i++) {
-        PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+    for (i = 0; chainprefixes[i] != 0; i++) {
+        PRINT_ROOT_CHAIN(rootchain, chainprefixes[i], ifname);
         virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                false, ebtablesRemoveSubChainsQuery,
-                               (void *)chains,
+                               (void *)chainprefixes,
                                "-t", "nat", "-L", rootchain, NULL);
     }
 }
@@ -2670,13 +2683,7 @@ static void
 ebtablesRemoveSubChainsFW(virFirewallPtr fw,
                           const char *ifname)
 {
-    char chains[3] = {
-        CHAINPREFIX_HOST_IN,
-        CHAINPREFIX_HOST_OUT,
-        0
-    };
-
-    _ebtablesRemoveSubChainsFW(fw, ifname, chains);
+    _ebtablesRemoveSubChainsFW(fw, ifname, chainprefixes_host);
 }
 
 
@@ -2684,13 +2691,7 @@ static void
 ebtablesRemoveTmpSubChainsFW(virFirewallPtr fw,
                              const char *ifname)
 {
-    char chains[3] = {
-        CHAINPREFIX_HOST_IN_TEMP,
-        CHAINPREFIX_HOST_OUT_TEMP,
-        0
-    };
-
-    _ebtablesRemoveSubChainsFW(fw, ifname, chains);
+    _ebtablesRemoveSubChainsFW(fw, ifname, chainprefixes_host_temp);
 }
 
 static void

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list