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

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