[libvirt] [PATCH V1 0/6] Make part of inner workings of nwfilters more flexible

The following series of patches re-does some of the inner workings of nwfilters with the goal to enable users to write filters that have other than the system-known chains supported right now ('root','arp','rarp','ipv4' and 'ipv6'). Ideally users should be able to provide a chain name in the chains XML attribute and either be able to jump to it as an 'action' or have the chain created automatically as it is the case right now for those chains enumerated before. I am introducing internal priorities for the chains mentioned above so that their creation can be made more flexible -- currently their creation and the order in which they are accessed is hardcoded. This largely does away with the hardcoded stuff. Further, filters will be automatically accessed from the (ebtables) 'root' chain using the prefix of the name of the chain. As an example, this filter will be accessed from the root chain for 'arp' packets since its name 'arpmac' has the prefix 'arp'. <filter name='allow-arpmac' chain='arpmac'> <uuid>94abeecc-c956-0ac8-1f49-a06ee8995688</uuid> <rule action='accept' direction='out' priority='100'> <arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC' arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/> </rule> <rule action='accept' direction='inout' priority='500'/> </filter> In the future the chains may have priorities supported in the XML in order to control the order in which they are accessed. <filter name='allow-arpmac' chain='arpmac' prirotiy='650'> [...] The series does not enable users yet to provide names of chains but instead pushes the problem of their later enablement into the XML parser and prepares the rest of the code to handle it (as far as this can be seen). I did the testing with the test cases in libvirt-tck and did not see regressions. Regards, Stefan

Add a function to the virHashTable for getting an array of the hash table's keys and have the keys optionally sorted. --- src/libvirt_private.syms | 3 + src/util/hash.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/hash.h | 14 ++++++ 3 files changed, 115 insertions(+) Index: libvirt-acl/src/util/hash.c =================================================================== --- libvirt-acl.orig/src/util/hash.c +++ libvirt-acl/src/util/hash.c @@ -618,3 +618,101 @@ void *virHashSearch(virHashTablePtr tabl return NULL; } + + +struct getKeysIter +{ + virHashTablePtr table; + void **array; + virHashKeyValuePair *sortArray; + unsigned arrayIdx; + bool error; +}; + +static void virHashGetKeysIterator(void *payload, + const void *name, void *data) +{ + struct getKeysIter *iter = data; + void *key; + + if (iter->error) + return; + + key = iter->table->keyCopy(name); + + if (key == NULL) { + virReportOOMError(); + iter->error = true; + return; + } + + if (iter->sortArray) { + iter->sortArray[iter->arrayIdx].key = key; + iter->sortArray[iter->arrayIdx].value = payload; + } else { + iter->array[iter->arrayIdx] = iter->table->keyCopy(name); + } + iter->arrayIdx++; +} + +void virHashFreeKeys(virHashTablePtr table, void **keys) +{ + unsigned i = 0; + + if (keys == NULL) + return; + + while (keys[i]) + table->keyFree(keys[i++]); + + VIR_FREE(keys); +} + +typedef int (*qsort_comp)(const void *, const void *); + +void **virHashGetKeys(virHashTablePtr table, + virHashKeyComparator compar) +{ + int i, numElems = virHashSize(table); + struct getKeysIter iter = { + .table = table, + .arrayIdx = 0, + .error = false, + }; + + if (numElems < 0) { + return NULL; + } + + if (VIR_ALLOC_N(iter.array, numElems + 1)) { + virReportOOMError(); + return NULL; + } + + /* null-terminate array */ + iter.array[numElems] = NULL; + + if (compar && + VIR_ALLOC_N(iter.sortArray, numElems)) { + VIR_FREE(iter.array); + virReportOOMError(); + return NULL; + } + + virHashForEach(table, virHashGetKeysIterator, &iter); + if (iter.error) { + VIR_FREE(iter.sortArray); + virHashFreeKeys(table, iter.array); + return NULL; + } + + if (compar) { + qsort(&iter.sortArray[0], numElems, sizeof(iter.sortArray[0]), + (qsort_comp)compar); + for (i = 0; i < numElems; i++) + iter.array[i] = iter.sortArray[i].key; + VIR_FREE(iter.sortArray); + } + + return iter.array; +} Index: libvirt-acl/src/util/hash.h =================================================================== --- libvirt-acl.orig/src/util/hash.h +++ libvirt-acl/src/util/hash.h @@ -130,6 +130,20 @@ void *virHashLookup(virHashTablePtr tabl */ void *virHashSteal(virHashTablePtr table, const void *name); +/* + * Get the set of keys and have them optionally sorted. + */ +typedef struct _virHashKeyValuePair virHashKeyValuePair; +typedef virHashKeyValuePair *virHashKeyValuePairPtr; +struct _virHashKeyValuePair { + void *key; + const void *value; +}; +typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr , + const virHashKeyValuePairPtr ); +void **virHashGetKeys(virHashTablePtr table, virHashKeyComparator compar); +void virHashFreeKeys(virHashTablePtr table, void **); + /* * Iterators Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -552,6 +552,8 @@ virHashAddEntry; virHashCreate; virHashForEach; virHashFree; +virHashFreeKeys; +virHashGetKeys; virHashLookup; virHashRemoveEntry; virHashRemoveSet; @@ -559,6 +561,7 @@ virHashSearch; virHashSize; virHashSteal; virHashTableSize; +virHashUpdateEntry; # hooks.h

For better handling of the sorting of chains introduce an internally used priority. Use a lookup table to store the priorities. For now their actual values do not matter just that the values cause the chains to be properly sorted through changes in the following patches. --- src/conf/nwfilter_conf.c | 19 +++++++++++++++++++ src/conf/nwfilter_conf.h | 3 +++ src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++++ src/nwfilter/nwfilter_ebiptables_driver.h | 1 + 4 files changed, 27 insertions(+) Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -355,6 +355,7 @@ enum virNWFilterEbtablesTableType { }; +# define MIN_RULE_PRIORITY 0 # define MAX_RULE_PRIORITY 1000 enum virNWFilterRuleFlags { @@ -434,6 +435,7 @@ enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_LAST, }; +typedef int32_t virNWFilterChainPriority; typedef struct _virNWFilterDef virNWFilterDef; typedef virNWFilterDef *virNWFilterDefPtr; @@ -443,6 +445,7 @@ struct _virNWFilterDef { unsigned char uuid[VIR_UUID_BUFLEN]; int chainsuffix; /*enum virNWFilterChainSuffixType */ + virNWFilterChainPriority chainPriority; int nentries; virNWFilterEntryPtr *filterEntries; Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -120,6 +120,20 @@ struct int_map { #define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL } #define INTMAP_ENTRY_LAST { .val = NULL } +#define NWFILTER_ROOT_FILTER_PRI 0 +#define NWFILTER_IPV4_FILTER_PRI 200 +#define NWFILTER_IPV6_FILTER_PRI 400 +#define NWFILTER_ARP_FILTER_PRI 600 +#define NWFILTER_RARP_FILTER_PRI 800 + +static const struct int_map chain_priorities[] = { + INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, "root"), + INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, "ipv4"), + INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, "ipv6"), + INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , "arp" ), + INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, "rarp"), + INTMAP_ENTRY_LAST, +}; /* * only one filter update allowed @@ -2026,6 +2040,11 @@ virNWFilterDefParseXML(xmlXPathContextPt _("unknown chain suffix '%s'"), chain); goto cleanup; } + /* assign an implicit priority -- support XML attribute later */ + if (intMapGetByString(chain_priorities, chain, 0, + &ret->chainPriority) == false) { + ret->chainPriority = (MAX_RULE_PRIORITY + MIN_RULE_PRIORITY) / 2; + } } uuid = virXPathString("string(./uuid)", ctxt); Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -36,6 +36,7 @@ typedef ebiptablesRuleInst *ebiptablesRu struct _ebiptablesRuleInst { char *commandTemplate; enum virNWFilterChainSuffixType neededProtocolChain; + virNWFilterChainPriority chainPriority; char chainprefix; /* I for incoming, O for outgoing */ unsigned int priority; enum RuleType ruleType; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -327,6 +327,7 @@ static int ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, char *commandTemplate, enum virNWFilterChainSuffixType neededChain, + virNWFilterChainPriority chainPriority, char chainprefix, unsigned int priority, enum RuleType ruleType) @@ -340,6 +341,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns inst->commandTemplate = commandTemplate; inst->neededProtocolChain = neededChain; + inst->chainPriority = chainPriority; inst->chainprefix = chainprefix; inst->priority = priority; inst->ruleType = ruleType; @@ -1588,6 +1590,7 @@ _iptablesCreateRuleInstance(int directio return ebiptablesAddRuleInst(res, virBufferContentAndReset(final), nwfilter->chainsuffix, + nwfilter->chainPriority, '\0', rule->priority, (isIPv6) ? RT_IP6TABLES : RT_IPTABLES); @@ -2337,6 +2340,7 @@ ebtablesCreateRuleInstance(char chainPre return ebiptablesAddRuleInst(res, virBufferContentAndReset(&buf), nwfilter->chainsuffix, + nwfilter->chainPriority, chainPrefix, rule->priority, RT_EBTABLES);

Use the previously introduced chain priorities to sort the chains for access from the 'root' table and have them created in the proper order. This gets rid of a lot of code that was previously creating the chains in a more hardcoded way. To detmerine what protocol a filter is used for evaluation do prefix- matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol, 'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter for the 'arp' protocol. --- src/nwfilter/nwfilter_ebiptables_driver.c | 130 ++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 32 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2774,6 +2774,7 @@ ebtablesCreateTmpSubChain(virBufferPtr b int incoming, const char *ifname, enum l3_proto_idx protoidx, + const char *filtername, int stopOnError) { char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; @@ -2781,7 +2782,8 @@ ebtablesCreateTmpSubChain(virBufferPtr b : CHAINPREFIX_HOST_OUT_TEMP; PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); - PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val); + PRINT_CHAIN(chain, chainPrefix, ifname, + (filtername) ? filtername : l3_protocols[protoidx].val); virBufferAsprintf(buf, CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR @@ -3288,6 +3290,13 @@ ebiptablesRuleOrderSort(const void *a, c return ((*insta)->priority - (*instb)->priority); } +static int +ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a, + const virHashKeyValuePairPtr b) +{ + return *(virNWFilterChainPriority *)a->value - + *(virNWFilterChainPriority *)b->value; +} static void iptablesCheckBridgeNFCallEnabled(bool isIPv6) @@ -3327,6 +3336,56 @@ iptablesCheckBridgeNFCallEnabled(bool is } } +/* + * Given a filtername determine the protocol it is used for evaluating + * We do prefix-matching to determine the protocol. + */ +static enum l3_proto_idx +ebtablesGetProtoIdxByFiltername(const char *filtername) +{ + enum l3_proto_idx idx; + + for (idx = 0; idx < L3_PROTO_LAST_IDX; idx++) { + if (STRPREFIX(filtername, l3_protocols[idx].val)) { + return idx; + } + } + + return -1; +} + +static int +ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, + const char *ifname, + virHashTablePtr chains, int direction) +{ + int rc = 0, i; + + if (virHashSize(chains) != 0) { + char **filter_names; + + ebtablesCreateTmpRootChain(buf, direction, ifname, 1); + filter_names = (char **)virHashGetKeys(chains, + ebiptablesFilterOrderSort); + if (filter_names == NULL) { + virReportOOMError(); + rc = 1; + goto err_exit; + } + for (i = 0; filter_names[i]; i++) { + enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername( + filter_names[i]); + if ((int)idx < 0) + continue; + ebtablesCreateTmpSubChain(buf, direction, ifname, idx, + filter_names[i], 1); + } + virHashFreeKeys(chains, (void **)filter_names); + } + + err_exit: + return rc; +} static int ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -3337,24 +3396,43 @@ ebiptablesApplyNewRules(virConnectPtr co int i; int cli_status; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; - int chains_in = 0, chains_out = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; + virHashTablePtr chains_in_set = virHashCreate(10, NULL), + chains_out_set = virHashCreate(10, NULL); bool haveIptables = false; bool haveIp6tables = false; + if (!chains_in_set || !chains_out_set) { + virReportOOMError(); + goto exit_free_sets; + } + if (nruleInstances > 1 && inst) qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort); + /* scan the rules to see which chains need to be created */ for (i = 0; i < nruleInstances; i++) { sa_assert (inst); if (inst[i]->ruleType == RT_EBTABLES) { - if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) - chains_in |= (1 << inst[i]->neededProtocolChain); - else - chains_out |= (1 << inst[i]->neededProtocolChain); + const char *name = virNWFilterChainSuffixTypeToString( + inst[i]->neededProtocolChain); + if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) { + if (virHashUpdateEntry(chains_in_set, name, + &inst[i]->chainPriority)) { + virReportOOMError(); + goto exit_free_sets; + } + } else { + if (virHashUpdateEntry(chains_out_set, name, + &inst[i]->chainPriority)) { + virReportOOMError(); + goto exit_free_sets; + } + } } } + /* cleanup whatever may exist */ if (ebtables_cmd_path) { ebtablesUnlinkTmpRootChain(&buf, 1, ifname); ebtablesUnlinkTmpRootChain(&buf, 0, ifname); @@ -3364,30 +3442,11 @@ ebiptablesApplyNewRules(virConnectPtr co ebiptablesExecCLI(&buf, &cli_status); } - if (chains_in != 0) - ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); - if (chains_out != 0) - ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); - - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV4_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV4_IDX, 1); - - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV6_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV6_IDX, 1); - - /* keep arp,rarp as last */ - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARP_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARP_IDX, 1); - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_RARP_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_RARP_IDX, 1); + /* create needed chains */ + if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1) || + ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0)) { + goto tear_down_tmpebchains; + } if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_tmpebchains; @@ -3477,14 +3536,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCheckBridgeNFCallEnabled(true); } - if (chains_in != 0) + if (virHashSize(chains_in_set) != 0) ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); - if (chains_out != 0) + if (virHashSize(chains_out_set) != 0) ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_ebsubchains_and_unlink; + virHashFree(chains_in_set); + virHashFree(chains_out_set); + return 0; tear_down_ebsubchains_and_unlink: @@ -3519,6 +3581,10 @@ tear_down_tmpebchains: "interface %s."), ifname); +exit_free_sets: + virHashFree(chains_in_set); + virHashFree(chains_out_set); + return 1; }

Use scripts for the renaming and cleaning up of chains. This allows us to get rid of some of the code that is only capable of renaming and removing chains whose names are hardcoded. A shell function 'collect_chains' is introduced that is given the name of an ebtables chain and then recursively determines the names of all chanins that are accessed from this chain and its sub-chains using 'jumps'. This resulting list of chain names is then used to delete all the found chains by first flushing and then deleting them. The same function is also used for renaming temporary filters to their final names. I tested this with the bash and dash as script interpreters. --- src/nwfilter/nwfilter_ebiptables_driver.c | 189 ++++++++++++++++-------------- 1 file changed, 102 insertions(+), 87 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -91,6 +91,37 @@ static char *gawk_cmd_path; #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix) +#define FUNC_COLLECT_CHAINS \ + "collect_chains()\n" \ + "{\n" \ + " local sc\n" \ + " sc=$(%s -t %s -L $1 | \\\n" \ + " sed -n \"/Bridge chain*/,$ s/.*\\-j \\([%s]-.*\\)/\\1/p\")\n" \ + " for tmp in `echo \"$sc\"`; do\n" \ + " sc=\"$sc $(collect_chains $tmp)\"\n" \ + " done\n" \ + " echo \"$sc\"\n" \ + "}\n" + +#define FUNC_DELETE_CHAINS \ + "rm_chains()\n" \ + "{\n" \ + " for tmp in `echo \"$1\"`; do %s -t %s -F $tmp; done\n" \ + " for tmp in `echo \"$1\"`; do %s -t %s -X $tmp; done\n" \ + "}\n" + +#define FUNC_RENAME_CHAINS \ + "rename_chains()\n" \ + "{\n" \ + " for tmp in `echo \"$1\"`; do\n" \ + " tmp2=`expr substr \"$tmp\" 1 1`\n" \ + " if [ $tmp2 = \"%c\" ]; then\n" \ + " %s -t %s -E \"$tmp\" \"%c\"`expr substr \"$tmp\" 2 33`\n" \ + " elif [ $tmp2 = \"%c\" ]; then\n" \ + " %s -t %s -E \"$tmp\" \"%c\"`expr substr \"$tmp\" 2 33`\n" \ + " fi\n" \ + " done\n" \ + "}\n" #define VIRT_IN_CHAIN "libvirt-in" #define VIRT_OUT_CHAIN "libvirt-out" @@ -2805,95 +2836,64 @@ ebtablesCreateTmpSubChain(virBufferPtr b return 0; } - -static int -_ebtablesRemoveSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx, - int isTempChain) +static int _ebtablesRemoveSubChains(virBufferPtr buf, + const char *ifname, + const char *chains) { - char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; - char chainPrefix; - - if (isTempChain) { - chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; - } else { - chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; - } + char rootchain[MAX_CHAINNAME_LENGTH]; + unsigned i; - PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); - PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val); - - virBufferAsprintf(buf, - "%s -t %s -D %s -p 0x%x -j %s" CMD_SEPARATOR - "%s -t %s -F %s" CMD_SEPARATOR - "%s -t %s -X %s" CMD_SEPARATOR, + virBufferAsprintf(buf, FUNC_COLLECT_CHAINS, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains); + virBufferAsprintf(buf, FUNC_DELETE_CHAINS, ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, - rootchain, l3_protocols[protoidx].attr, chain, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE); - ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain, + virBufferAddLit(buf, "a=\""); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "$(collect_chains %s) ", rootchain); + } + virBufferAddLit(buf, "\"\n"); - ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, + "%s -t %s -F %s\n", + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + rootchain); + } + virBufferAddLit(buf, "rm_chains \"$a\"\n"); return 0; } - -static int -ebtablesRemoveSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx) -{ - return _ebtablesRemoveSubChain(buf, - incoming, ifname, protoidx, 0); -} - - static int ebtablesRemoveSubChains(virBufferPtr buf, const char *ifname) { - enum l3_proto_idx i; - - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRemoveSubChain(buf, 1, ifname, i); - ebtablesRemoveSubChain(buf, 0, ifname, i); - } - - return 0; -} - + char chains[3] = { + CHAINPREFIX_HOST_IN, + CHAINPREFIX_HOST_OUT, + 0 + }; -static int -ebtablesRemoveTmpSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx) -{ - return _ebtablesRemoveSubChain(buf, - incoming, ifname, protoidx, 1); + return _ebtablesRemoveSubChains(buf, ifname, chains); } - static int ebtablesRemoveTmpSubChains(virBufferPtr buf, const char *ifname) { - enum l3_proto_idx i; - - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRemoveTmpSubChain(buf, 1, ifname, i); - ebtablesRemoveTmpSubChain(buf, 0, ifname, i); - } + char chains[3] = { + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, + 0 + }; - return 0; + return _ebtablesRemoveSubChains(buf, ifname, chains); } - static int ebtablesRenameTmpSubChain(virBufferPtr buf, int incoming, @@ -2920,31 +2920,49 @@ ebtablesRenameTmpSubChain(virBufferPtr b return 0; } - static int -ebtablesRenameTmpSubChains(virBufferPtr buf, +ebtablesRenameTmpRootChain(virBufferPtr buf, + int incoming, const char *ifname) { - enum l3_proto_idx i; + return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); +} - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRenameTmpSubChain (buf, 1, ifname, l3_protocols[i].val); - ebtablesRenameTmpSubChain (buf, 0, ifname, l3_protocols[i].val); +static int +ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, + const char *ifname) +{ + char rootchain[MAX_CHAINNAME_LENGTH]; + unsigned i; + char chains[3] = { + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, + 0}; + + virBufferAsprintf(buf, FUNC_COLLECT_CHAINS, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains); + virBufferAsprintf(buf, FUNC_RENAME_CHAINS, + CHAINPREFIX_HOST_IN_TEMP, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + CHAINPREFIX_HOST_IN, + CHAINPREFIX_HOST_OUT_TEMP, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + CHAINPREFIX_HOST_OUT); + virBufferAddLit(buf, "a=\""); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "$(collect_chains %s) ", rootchain); } + virBufferAddLit(buf, "\"\n"); - return 0; -} + virBufferAddLit(buf, "rename_chains \"$a\"\n"); + ebtablesRenameTmpRootChain(buf, 1, ifname); + ebtablesRenameTmpRootChain(buf, 0, ifname); -static int -ebtablesRenameTmpRootChain(virBufferPtr buf, - int incoming, - const char *ifname) -{ - return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); + return 0; } - static void ebiptablesInstCommand(virBufferPtr buf, const char *templ, char cmd, int pos, @@ -3654,9 +3672,7 @@ ebiptablesTearOldRules(virConnectPtr con ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); - ebtablesRenameTmpSubChains(&buf, ifname); - ebtablesRenameTmpRootChain(&buf, 1, ifname); - ebtablesRenameTmpRootChain(&buf, 0, ifname); + ebtablesRenameTmpSubAndRootChains(&buf, ifname); ebiptablesExecCLI(&buf, &cli_status); } @@ -3741,12 +3757,11 @@ ebiptablesAllTeardown(const char *ifname ebtablesUnlinkRootChain(&buf, 1, ifname); ebtablesUnlinkRootChain(&buf, 0, ifname); + ebtablesRemoveSubChains(&buf, ifname); + ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); - - ebtablesRemoveSubChains(&buf, ifname); } - ebiptablesExecCLI(&buf, &cli_status); return 0;

Use the name of the chain rather than its type index (enum). This pushes the later enablement of chains with user-given names into the XML parser. For now we still only allow those names that are well known ('root', 'arp', 'rarp', 'ipv4' and 'ipv6'). --- src/conf/nwfilter_conf.c | 16 ++++++++++++---- src/conf/nwfilter_conf.h | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 13 +++++++------ src/nwfilter/nwfilter_ebiptables_driver.h | 2 +- 4 files changed, 21 insertions(+), 12 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -311,6 +311,7 @@ virNWFilterDefFree(virNWFilterDefPtr def virNWFilterEntryFree(def->filterEntries[i]); VIR_FREE(def->filterEntries); + VIR_FREE(def->chainsuffix); VIR_FREE(def); } @@ -2031,20 +2032,27 @@ virNWFilterDefParseXML(xmlXPathContextPt goto cleanup; } - ret->chainsuffix = VIR_NWFILTER_CHAINSUFFIX_ROOT; chain = virXPathString("string(./@chain)", ctxt); if (chain) { - if ((ret->chainsuffix = - virNWFilterChainSuffixTypeFromString(chain)) < 0) { + if (virNWFilterChainSuffixTypeFromString(chain) < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("unknown chain suffix '%s'"), chain); goto cleanup; } + ret->chainsuffix = chain; /* assign an implicit priority -- support XML attribute later */ if (intMapGetByString(chain_priorities, chain, 0, &ret->chainPriority) == false) { ret->chainPriority = (MAX_RULE_PRIORITY + MIN_RULE_PRIORITY) / 2; } + chain = NULL; + } else { + ret->chainsuffix = strdup(virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT)); + if (ret->chainsuffix == NULL) { + virReportOOMError(); + goto cleanup; + } } uuid = virXPathString("string(./uuid)", ctxt); @@ -2910,7 +2918,7 @@ virNWFilterDefFormat(virNWFilterDefPtr d virBufferAsprintf(&buf, "<filter name='%s' chain='%s'", def->name, - virNWFilterChainSuffixTypeToString(def->chainsuffix)); + def->chainsuffix); virBufferAddLit(&buf, ">\n"); virUUIDFormat(def->uuid, uuid); Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -444,7 +444,7 @@ struct _virNWFilterDef { char *name; unsigned char uuid[VIR_UUID_BUFLEN]; - int chainsuffix; /*enum virNWFilterChainSuffixType */ + char *chainsuffix; virNWFilterChainPriority chainPriority; int nentries; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -357,7 +357,7 @@ ebiptablesRuleInstFree(ebiptablesRuleIns static int ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, char *commandTemplate, - enum virNWFilterChainSuffixType neededChain, + const char *neededChain, virNWFilterChainPriority chainPriority, char chainprefix, unsigned int priority, @@ -1933,11 +1933,13 @@ ebtablesCreateRuleInstance(char chainPre goto err_exit; } - if (nwfilter->chainsuffix == VIR_NWFILTER_CHAINSUFFIX_ROOT) + if (STREQ(nwfilter->chainsuffix, + virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT))) PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); else PRINT_CHAIN(chain, chainPrefix, ifname, - virNWFilterChainSuffixTypeToString(nwfilter->chainsuffix)); + nwfilter->chainsuffix); switch (rule->prtclType) { @@ -2504,7 +2506,7 @@ ebiptablesDisplayRuleInstance(virConnect ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst; VIR_INFO("Command Template: '%s', Needed protocol: '%s'", inst->commandTemplate, - virNWFilterChainSuffixTypeToString(inst->neededProtocolChain)); + inst->neededProtocolChain); return 0; } @@ -3432,8 +3434,7 @@ ebiptablesApplyNewRules(virConnectPtr co for (i = 0; i < nruleInstances; i++) { sa_assert (inst); if (inst[i]->ruleType == RT_EBTABLES) { - const char *name = virNWFilterChainSuffixTypeToString( - inst[i]->neededProtocolChain); + const char *name = inst[i]->neededProtocolChain; if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) { if (virHashUpdateEntry(chains_in_set, name, &inst[i]->chainPriority)) { Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -35,7 +35,7 @@ typedef struct _ebiptablesRuleInst ebipt typedef ebiptablesRuleInst *ebiptablesRuleInstPtr; struct _ebiptablesRuleInst { char *commandTemplate; - enum virNWFilterChainSuffixType neededProtocolChain; + const char *neededProtocolChain; virNWFilterChainPriority chainPriority; char chainprefix; /* I for incoming, O for outgoing */ unsigned int priority;

Only as an example show how to add a new 'system filter' called 'arpmac'. Using 'prefix matching' introduced in previous patches, it generates a table 'arpmac' that will be jumped into using '-p arp'. The below patch adds arpmac with a priority of 650, which helps sorting its entry in the 'root' table. Since previous code still doesn't allow arbitrary names we still need to add its name to the virNWFilerChainSuffixType and the list of strings. This patch would enable the following filter using the 'arpmac' chain. <filter name='allow-arpmac' chain='arpmac'> <uuid>94abeecc-c956-0ac8-1f49-a06ee8995688</uuid> <rule action='accept' direction='out' priority='100'> <arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC' arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/> </rule> <rule action='accept' direction='inout' priority='500'/> </filter> --- src/conf/nwfilter_conf.c | 5 ++++- src/conf/nwfilter_conf.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -81,7 +81,8 @@ VIR_ENUM_IMPL(virNWFilterChainSuffix, VI "arp", "rarp", "ipv4", - "ipv6"); + "ipv6", + "arpmac"); VIR_ENUM_IMPL(virNWFilterRuleProtocol, VIR_NWFILTER_RULE_PROTOCOL_LAST, "none", @@ -124,6 +125,7 @@ struct int_map { #define NWFILTER_IPV4_FILTER_PRI 200 #define NWFILTER_IPV6_FILTER_PRI 400 #define NWFILTER_ARP_FILTER_PRI 600 +#define NWFILTER_ARPMAC_FILTER_PRI 650 #define NWFILTER_RARP_FILTER_PRI 800 static const struct int_map chain_priorities[] = { @@ -132,6 +134,7 @@ static const struct int_map chain_priori INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, "ipv6"), INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , "arp" ), INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, "rarp"), + INTMAP_ENTRY(NWFILTER_ARPMAC_FILTER_PRI, "arpmac"), INTMAP_ENTRY_LAST, }; Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -431,6 +431,7 @@ enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_RARP, VIR_NWFILTER_CHAINSUFFIX_IPv4, VIR_NWFILTER_CHAINSUFFIX_IPv6, + VIR_NWFILTER_CHAINSUFFIX_ARPMAC, VIR_NWFILTER_CHAINSUFFIX_LAST, };
participants (1)
-
Stefan Berger