Convert the nwfilter ebtablesApplyBasicRules 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>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 113 +++++++++++++++++-------------
tests/nwfilterebiptablestest.c | 74 +++++++++++++++++++
2 files changed, 137 insertions(+), 50 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 4093bd9..d9c6822 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2877,6 +2877,21 @@ ebtablesCreateTmpRootChain(virBufferPtr buf,
static void
+ebtablesCreateTmpRootChainFW(virFirewallPtr fw,
+ int incoming, const char *ifname)
+{
+ char chain[MAX_CHAINNAME_LENGTH];
+ char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
+ : CHAINPREFIX_HOST_OUT_TEMP;
+
+ PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
+
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-N", chain, NULL);
+}
+
+
+static void
ebtablesLinkTmpRootChain(virBufferPtr buf,
bool incoming, const char *ifname)
{
@@ -2900,6 +2915,24 @@ ebtablesLinkTmpRootChain(virBufferPtr buf,
static void
+ebtablesLinkTmpRootChainFW(virFirewallPtr fw,
+ int incoming, const char *ifname)
+{
+ char chain[MAX_CHAINNAME_LENGTH];
+ char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP
+ : CHAINPREFIX_HOST_OUT_TEMP;
+
+ PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
+
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-A",
+ incoming ? EBTABLES_CHAIN_INCOMING : EBTABLES_CHAIN_OUTGOING,
+ incoming ? "-i" : "-o",
+ ifname, "-j", chain, NULL);
+}
+
+
+static void
_ebtablesRemoveRootChain(virBufferPtr buf,
bool incoming, const char *ifname,
bool isTempChain)
@@ -3421,74 +3454,54 @@ static int
ebtablesApplyBasicRules(const char *ifname,
const virMacAddr *macaddr)
{
- virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virFirewallPtr fw = virFirewallNew();
char chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = CHAINPREFIX_HOST_IN_TEMP;
char macaddr_str[VIR_MAC_STRING_BUFLEN];
- if (!ebtables_cmd_path) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("cannot create rules since ebtables tool is "
- "missing."));
- return -1;
- }
-
virMacAddrFormat(macaddr, macaddr_str);
- ebiptablesAllTeardown(ifname);
+ if (ebiptablesAllTeardown(ifname) < 0)
+ goto error;
- NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
+ virFirewallStartTransaction(fw, 0);
- ebtablesCreateTmpRootChain(&buf, true, ifname);
+ ebtablesCreateTmpRootChainFW(fw, true, ifname);
PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
- virBufferAsprintf(&buf,
- CMD_DEF("$EBT -t nat -A %s -s ! %s -j DROP") CMD_SEPARATOR
- CMD_EXEC
- "%s",
-
- chain, macaddr_str,
- CMD_STOPONERR(true));
-
- virBufferAsprintf(&buf,
- CMD_DEF("$EBT -t nat -A %s -p IPv4 -j ACCEPT") CMD_SEPARATOR
- CMD_EXEC
- "%s",
-
- chain,
- CMD_STOPONERR(true));
-
- virBufferAsprintf(&buf,
- CMD_DEF("$EBT -t nat -A %s -p ARP -j ACCEPT") CMD_SEPARATOR
- CMD_EXEC
- "%s",
-
- chain,
- CMD_STOPONERR(true));
-
- virBufferAsprintf(&buf,
- CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR
- CMD_EXEC
- "%s",
-
- chain,
- CMD_STOPONERR(true));
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-A", chain,
+ "-s", "!", macaddr_str,
+ "-j", "DROP", NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-A", chain,
+ "-p", "IPv4",
+ "-j", "ACCEPT", NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-A", chain,
+ "-p", "ARP",
+ "-j", "ACCEPT", NULL);
+ virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+ "-t", "nat", "-A", chain,
+ "-j", "DROP", NULL);
- ebtablesLinkTmpRootChain(&buf, true, ifname);
- ebtablesRenameTmpRootChain(&buf, true, ifname);
+ ebtablesLinkTmpRootChainFW(fw, true, ifname);
+ ebtablesRenameTmpRootChainFW(fw, true, 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 d728e4c..22460fe 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -271,6 +271,75 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED)
static int
+testNWFilterEBIPTablesApplyBasicRules(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 -A libvirt-J-vnet0 -s '!' 10:20:30:40:50:60 -j DROP\n"
+ "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -p IPv4 -j ACCEPT\n"
+ "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -p ARP -j ACCEPT\n"
+ "/usr/sbin/ebtables -t nat -A libvirt-J-vnet0 -j DROP\n"
+ "/usr/sbin/ebtables -t nat -A PREROUTING -i vnet0 -j libvirt-J-vnet0\n"
+ "/usr/sbin/ebtables -t nat -E libvirt-J-vnet0 libvirt-I-vnet0\n";
+ const char *actual = NULL;
+ int ret = -1;
+ virMacAddr mac = { .addr = { 0x10, 0x20, 0x30, 0x40, 0x50, 0x60 } };
+
+ virCommandSetDryRun(&buf, NULL, NULL);
+
+ if (ebiptables_driver.applyBasicRules("vnet0", &mac) < 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;
@@ -300,6 +369,11 @@ mymain(void)
NULL) < 0)
ret = -1;
+ if (virtTestRun("ebiptablesApplyBasicRules",
+ testNWFilterEBIPTablesApplyBasicRules,
+ NULL) < 0)
+ ret = -1;
+
cleanup:
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.9.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list