Re: [libvirt] [PATCH 23/26] Convert nwfilter ebiptablesApplyNewRules to virFirewall

Convert the nwfilter ebtablesApplyNewRules 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;

@@ -679,8 +540,12 @@ _iptablesRemoveRootChainFW(virFirewallPtr fw, PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname); - virFirewallAddRule(fw, layer, "-F", chain, NULL); - virFirewallAddRule(fw, layer, "-X", chain, NULL); + virFirewallAddRuleFull(fw, layer, + true, NULL, NULL, + "-F", chain, NULL); + virFirewallAddRuleFull(fw, layer, + true, NULL, NULL, + "-X", chain, NULL); }

@@ -1088,138 +866,116 @@ iptablesHandleIpHdr(virBufferPtr buf, dstrange = "--src-range"; } - if (HAS_ENTRY_ITEM(&ipHdr->dataIPSet) && - HAS_ENTRY_ITEM(&ipHdr->dataIPSetFlags)) { - - if (printDataType(vars, - str, sizeof(str), - &ipHdr->dataIPSet) < 0) - goto err_exit; - - virBufferAsprintf(afterStateMatch, - " -m set --match-set \"%s\" ", - str); - - if (printDataTypeDirection(vars, - str, sizeof(str), - &ipHdr->dataIPSetFlags, directionIn) < 0) - goto err_exit; - - virBufferAdd(afterStateMatch, str, -1); - } -

} if (HAS_ENTRY_ITEM(&ipHdr->dataComment)) { - printCommentVar(prefix, ipHdr->dataComment.u.string); - /* keep comments behind everything else -- they are packet eval. no-ops */ - virBufferAddLit(afterStateMatch, - " -m comment --comment \"$" COMMENT_VARNAME "\""); + virFirewallRuleAddArgList(fw, fwrule, + "-m", "comment", + "--comment", ipHdr->dataComment.u.string, + NULL); }

@@ -4038,188 +3457,173 @@ ebiptablesApplyNewRules(const char *ifname, } } - /* process ebtables commands; interleave commands from filters with - commands for creating and connecting ebtables chains */ - j = 0; for (i = 0; i < nrules; i++) { if (virNWFilterRuleIsProtocolEthernet(rules[i]->def)) { - while (j < nEbtChains && - ebtChains[j].priority <= rules[i]->priority) { - ebiptablesInstCommand(&buf, - ebtChains[j++].commandTemplate); - } - ebtablesRuleInstCommand(&buf, - ifname, - rules[i]); + haveEbtables = true; } else { if (virNWFilterRuleIsProtocolIPv4(rules[i]->def)) haveIptables = true; - else if (virNWFilterRuleIsProtocolIPv4(rules[i]->def)) + else if (virNWFilterRuleIsProtocolIPv6(rules[i]->def)) haveIp6tables = true;

} } + /* process ebtables commands; interleave commands from filters with + commands for creating and connecting ebtables chains */ + if (haveEbtables) { - while (j < nEbtChains) - ebiptablesInstCommand(&buf, - ebtChains[j++].commandTemplate); + /* scan the rules to see which chains need to be created */ + for (i = 0; i < nrules; i++) { + if (virNWFilterRuleIsProtocolEthernet(rules[i]->def)) { + const char *name = rules[i]->chainSuffix; + if (rules[i]->def->tt == VIR_NWFILTER_RULE_DIRECTION_OUT || + rules[i]->def->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT) { + if (virHashUpdateEntry(chains_in_set, name, + &rules[i]->chainPriority) < 0) + goto cleanup; + } + if (rules[i]->def->tt == VIR_NWFILTER_RULE_DIRECTION_IN || + rules[i]->def->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT) { + if (virHashUpdateEntry(chains_out_set, name, + &rules[i]->chainPriority) < 0) + goto cleanup; + } + } + } - if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) - goto tear_down_tmpebchains; + /* create needed chains */ + if (virHashSize(chains_in_set) > 0) { + ebtablesCreateTmpRootChainFW(fw, true, ifname); + if (ebtablesGetSubChainInsts(chains_in_set, + true, + &subchains, + &nsubchains) < 0) + goto cleanup; + } + if (virHashSize(chains_out_set) > 0) { + ebtablesCreateTmpRootChainFW(fw, false, ifname); + if (ebtablesGetSubChainInsts(chains_out_set, + false, + &subchains, + &nsubchains) < 0) + goto cleanup; + } + + if (nsubchains > 0) + qsort(subchains, nsubchains, sizeof(subchains[0]), + ebtablesSubChainInstSort); + + for (i = 0, j = 0; i < nrules; i++) { + if (virNWFilterRuleIsProtocolEthernet(rules[i]->def)) { + while (j < nsubchains && + subchains[j]->priority <= rules[i]->priority) { + ebtablesCreateTmpSubChainFW(fw, + subchains[j]->incoming, + ifname, + subchains[j]->protoidx, + subchains[j]->filtername); + j++; + } + if (ebtablesRuleInstCommand(fw, + ifname, + rules[i]) < 0) + goto cleanup; + } + } + while (j < nsubchains) { + ebtablesCreateTmpSubChainFW(fw, + subchains[j]->incoming, + ifname, + subchains[j]->protoidx, + subchains[j]->filtername); + j++; + } + } if (haveIptables) {

- NWFILTER_SET_IPTABLES_SHELLVAR(&buf); - - iptablesUnlinkTmpRootChains(&buf, ifname); - iptablesRemoveTmpRootChains(&buf, ifname); - - iptablesCreateBaseChains(&buf); - - if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) - goto tear_down_tmpebchains; - - NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + iptablesUnlinkTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname); + iptablesRemoveTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname); - iptablesCreateTmpRootChains(&buf, ifname); + iptablesCreateBaseChainsFW(fw, VIR_FIREWALL_LAYER_IPV4); + iptablesCreateTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname); - if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) - goto tear_down_tmpiptchains; - - NWFILTER_SET_IPTABLES_SHELLVAR(&buf); - - iptablesLinkTmpRootChains(&buf, ifname); - iptablesSetupVirtInPost(&buf, ifname); - if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) - goto tear_down_tmpiptchains; - - NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + iptablesLinkTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname); + iptablesSetupVirtInPostFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname); for (i = 0; i < nrules; i++) { - if (virNWFilterRuleIsProtocolIPv4(rules[i]->def)) - iptablesRuleInstCommand(&buf, - ifname, - rules[i]); + if (virNWFilterRuleIsProtocolIPv4(rules[i]->def)) { + if (iptablesRuleInstCommand(fw, + ifname, + rules[i]) < 0) + goto cleanup; + } } - if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) - goto tear_down_tmpiptchains; - iptablesCheckBridgeNFCallEnabled(false); } if (haveIp6tables) {