[libvirt] [PATCH 21/26] Convert nwfilter ebtablesApplyDHCPOnlyRules to virFirewall

Daniel P. Berrange berrange at redhat.com
Tue Apr 8 15:38:13 UTC 2014


Convert the nwfilter ebtablesApplyDHCPOnlyRules 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 at redhat.com>
---
 src/nwfilter/nwfilter_ebiptables_driver.c | 120 ++++++++++++------------------
 tests/nwfilterebiptablestest.c            |  92 +++++++++++++++++++++++
 2 files changed, 140 insertions(+), 72 deletions(-)

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index d9c6822..62866ac 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3529,127 +3529,103 @@ ebtablesApplyDHCPOnlyRules(const char *ifname,
                            virNWFilterVarValuePtr dhcpsrvrs,
                            bool leaveTemporary)
 {
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
     char chain_in [MAX_CHAINNAME_LENGTH],
          chain_out[MAX_CHAINNAME_LENGTH];
     char macaddr_str[VIR_MAC_STRING_BUFLEN];
     unsigned int idx = 0;
     unsigned int num_dhcpsrvrs;
-
-    if (!ebtables_cmd_path) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("cannot create rules since ebtables tool is "
-                         "missing."));
-        return -1;
-    }
+    virFirewallPtr fw = virFirewallNew();
 
     virMacAddrFormat(macaddr, macaddr_str);
 
-    ebiptablesAllTeardown(ifname);
+    if (ebiptablesAllTeardown(ifname) < 0)
+        goto error;
 
-    NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
+    virFirewallStartTransaction(fw, 0);
 
-    ebtablesCreateTmpRootChain(&buf, true, ifname);
-    ebtablesCreateTmpRootChain(&buf, false, ifname);
+    ebtablesCreateTmpRootChainFW(fw, true, ifname);
+    ebtablesCreateTmpRootChainFW(fw, false, ifname);
 
     PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname);
     PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname);
 
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s"
-                              " -s %s"
-                              " -p ipv4 --ip-protocol udp"
-                              " --ip-sport 68 --ip-dport 67"
-                              " -j ACCEPT") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain_in,
-                      macaddr_str,
-                      CMD_STOPONERR(true));
-
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain_in,
+                       "-s", macaddr_str,
+                       "-p", "ipv4", "--ip-protocol", "udp",
+                       "--ip-sport", "68", "--ip-dport", "67",
+                       "-j", "ACCEPT", NULL);
 
-                      chain_in,
-                      CMD_STOPONERR(true));
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain_in,
+                       "-j", "DROP", NULL);
 
     num_dhcpsrvrs = (dhcpsrvrs != NULL)
                     ? virNWFilterVarValueGetCardinality(dhcpsrvrs)
                     : 0;
 
     while (true) {
-        char *srcIPParam = NULL;
+        const char *dhcpserver = NULL;
         int ctr;
 
-        if (idx < num_dhcpsrvrs) {
-            const char *dhcpserver;
-
+        if (idx < num_dhcpsrvrs)
             dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx);
 
-            if (virAsprintf(&srcIPParam, "--ip-src %s", dhcpserver) < 0)
-                goto tear_down_tmpebchains;
-        }
-
         /*
          * create two rules allowing response to MAC address of VM
          * or to broadcast MAC address
          */
         for (ctr = 0; ctr < 2; ctr++) {
-            virBufferAsprintf(&buf,
-                              CMD_DEF("$EBT -t nat -A %s"
-                                      " -d %s"
-                                      " -p ipv4 --ip-protocol udp"
-                                      " %s"
-                                      " --ip-sport 67 --ip-dport 68"
-                                      " -j ACCEPT") CMD_SEPARATOR
-                              CMD_EXEC
-                              "%s",
-
-                              chain_out,
-                              (ctr == 0) ? macaddr_str : "ff:ff:ff:ff:ff:ff",
-                              srcIPParam != NULL ? srcIPParam : "",
-                              CMD_STOPONERR(true));
+            if (dhcpserver)
+                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                                   "-t", "nat", "-A", chain_out,
+                                   "-d", (ctr == 0) ? macaddr_str : "ff:ff:ff:ff:ff:ff",
+                                   "-p", "ipv4", "--ip-protocol", "udp",
+                                   "--ip-src", dhcpserver,
+                                   "--ip-sport", "67", "--ip-dport", "68",
+                                   "-j", "ACCEPT", NULL);
+            else
+                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                                   "-t", "nat", "-A", chain_out,
+                                   "-d", (ctr == 0) ? macaddr_str : "ff:ff:ff:ff:ff:ff",
+                                   "-p", "ipv4", "--ip-protocol", "udp",
+                                   "--ip-sport", "67", "--ip-dport", "68",
+                                   "-j", "ACCEPT", NULL);
         }
 
-        VIR_FREE(srcIPParam);
-
         idx++;
 
         if (idx >= num_dhcpsrvrs)
             break;
     }
 
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain_out,
-                      CMD_STOPONERR(true));
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain_out,
+                       "-j", "DROP", NULL);
 
-    ebtablesLinkTmpRootChain(&buf, true, ifname);
-    ebtablesLinkTmpRootChain(&buf, false, ifname);
+    ebtablesLinkTmpRootChainFW(fw, true, ifname);
+    ebtablesLinkTmpRootChainFW(fw, false, ifname);
 
     if (!leaveTemporary) {
-        ebtablesRenameTmpRootChain(&buf, true, ifname);
-        ebtablesRenameTmpRootChain(&buf, false, ifname);
+        ebtablesRenameTmpRootChainFW(fw, true, ifname);
+        ebtablesRenameTmpRootChainFW(fw, false, ifname);
     }
 
-    if (ebiptablesExecCLI(&buf, false, NULL) < 0)
+    virMutexLock(&execCLIMutex);
+    if (virFirewallApply(fw) < 0) {
+        virMutexUnlock(&execCLIMutex);
         goto tear_down_tmpebchains;
+    }
+    virMutexUnlock(&execCLIMutex);
+
+    virFirewallFree(fw);
 
     return 0;
 
  tear_down_tmpebchains:
     ebtablesCleanAll(ifname);
-
-    virReportError(VIR_ERR_BUILD_FIREWALL,
-                   "%s",
-                   _("Some rules could not be created."));
-
+ error:
+    virFirewallFree(fw);
     return -1;
 }
 
diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index 22460fe..3010668 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -340,6 +340,93 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque ATTRIBUTE_UNUSED)
 
 
 static int
+testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    const char *expected =
+        "/usr/sbin/iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+        "/usr/sbin/iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+        "/usr/sbin/iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+        "/usr/sbin/iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+        "/usr/sbin/iptables -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\n"
+        "/usr/sbin/iptables -F FO-vnet0\n"
+        "/usr/sbin/iptables -X FO-vnet0\n"
+        "/usr/sbin/iptables -F FI-vnet0\n"
+        "/usr/sbin/iptables -X FI-vnet0\n"
+        "/usr/sbin/iptables -F HI-vnet0\n"
+        "/usr/sbin/iptables -X HI-vnet0\n"
+        "/usr/sbin/ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+        "/usr/sbin/ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+        "/usr/sbin/ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+        "/usr/sbin/ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+        "/usr/sbin/ip6tables -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\n"
+        "/usr/sbin/ip6tables -F FO-vnet0\n"
+        "/usr/sbin/ip6tables -X FO-vnet0\n"
+        "/usr/sbin/ip6tables -F FI-vnet0\n"
+        "/usr/sbin/ip6tables -X FI-vnet0\n"
+        "/usr/sbin/ip6tables -F HI-vnet0\n"
+        "/usr/sbin/ip6tables -X HI-vnet0\n"
+        "/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 -N libvirt-J-vnet0\n"
+        "/usr/sbin/ebtables -t nat -N libvirt-P-vnet0\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -s 10:20:30:40:50:60 -p ipv4 --ip-protocol udp --ip-sport 68 --ip-dport 67 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -j DROP\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d 10:20:30:40:50:60 -p ipv4 --ip-protocol udp --ip-src 192.168.122.1 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d ff:ff:ff:ff:ff:ff -p ipv4 --ip-protocol udp --ip-src 192.168.122.1 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d 10:20:30:40:50:60 -p ipv4 --ip-protocol udp --ip-src 10.0.0.1 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d ff:ff:ff:ff:ff:ff -p ipv4 --ip-protocol udp --ip-src 10.0.0.1 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d 10:20:30:40:50:60 -p ipv4 --ip-protocol udp --ip-src 10.0.0.2 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -d ff:ff:ff:ff:ff:ff -p ipv4 --ip-protocol udp --ip-src 10.0.0.2 --ip-sport 67 --ip-dport 68 -j ACCEPT\n"
+        "/usr/sbin/ebtables -t nat -A libvirt-P-vnet0 -j DROP\n"
+        "/usr/sbin/ebtables -t nat -A PREROUTING -i vnet0 -j libvirt-J-vnet0\n"
+        "/usr/sbin/ebtables -t nat -A POSTROUTING -o vnet0 -j libvirt-P-vnet0\n"
+        "/usr/sbin/ebtables -t nat -E libvirt-J-vnet0 libvirt-I-vnet0\n"
+        "/usr/sbin/ebtables -t nat -E libvirt-P-vnet0 libvirt-O-vnet0\n";
+    const char *actual = NULL;
+    int ret = -1;
+    virMacAddr mac = { .addr = { 0x10, 0x20, 0x30, 0x40, 0x50, 0x60 } };
+    const char *servers[] = { "192.168.122.1", "10.0.0.1", "10.0.0.2" };
+    virNWFilterVarValue val = {
+        .valType = NWFILTER_VALUE_TYPE_ARRAY,
+        .u = {
+            .array = {
+                .values = (char **)servers,
+                .nValues = 3,
+            }
+        }
+    };
+
+    virCommandSetDryRun(&buf, NULL, NULL);
+
+    if (ebiptables_driver.applyDHCPOnlyRules("vnet0", &mac, &val, false) < 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;
@@ -374,6 +461,11 @@ mymain(void)
                     NULL) < 0)
         ret = -1;
 
+    if (virtTestRun("ebiptablesApplyDHCPOnlyRules",
+                    testNWFilterEBIPTablesApplyDHCPOnlyRules,
+                    NULL) < 0)
+        ret = -1;
+
  cleanup:
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-- 
1.9.0




More information about the libvir-list mailing list