[libvirt] [PATCH 20/26] Convert nwfilter ebtablesApplyBasicRules to virFirewall

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(a)redhat.com&gt; --- 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