[libvirt] [PATCH 0/6] nwfilter: Enable access to variables via iterator or index
This patch enables access to variables in filters using indep. iterators ($TEST[$@2]) or via index ($TEST[1]). Three test cases are added that are also being used for libvirt-TCK to check that the instantiation of the filtering rules is correct. Regards, Stefan
In this patch we introduce testing whether the iterator points to a unique set of entries that have not been seen before at one of the previous iterations. The point is to eliminate duplicates and with that unnecessary filtering rules by preventing identical filtering rules from being instantiated. Example with two lists: list1 = [1,2,1] list2 = [1,3,1] The 1st iteration would take the 1st items of each list -> 1,1 The 2nd iteration would take the 2nd items of each list -> 2,3 The 3rd iteration would take the 3rd items of each list -> 1,1 but skip them since this same pair has already been encountered in the 1st iteration Implementation-wise this is solved by taking the n-th element of list1 and comparing it against elements 1..n-1. If no equivalent is found, then there is no possibility of this being a duplicate. In case an equivalent element is found at position i, then the n-th element in the 2nd list is compared against the i-th element in the 2nd list and if that is not the same, then this is a unique pair, otherwise it is not unique and we may need to do the same comparison on the 3rd list. --- src/conf/nwfilter_params.c | 55 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) Index: libvirt-iterator/src/conf/nwfilter_params.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.c +++ libvirt-iterator/src/conf/nwfilter_params.c @@ -348,6 +348,52 @@ virNWFilterVarCombIterAddVariable(virNWF } /* + * Test whether the iterator entry points to a distinguished set of entries + * that have not been seen before at one of the previous iterations. + * + * The point of this function is to eliminate duplicates. + * Example with two lists: + * + * list1 = [1,2,1] + * list2 = [1,3,1] + * + * The 1st iteration would take the 1st items of each list -> 1,1 + * The 2nd iteration would take the 2nd items of each list -> 2,3 + * The 3rd iteration would take the 3rd items of each list -> 1,1 but + * skip them since this pair has already been encountered in the 1st iteration + */ +static bool +virNWFilterVarCombIterEntryAreUniqueEntries(virNWFilterVarCombIterEntryPtr cie, + virNWFilterHashTablePtr hash) +{ + unsigned int i, j; + virNWFilterVarValuePtr varValue, tmp; + const char *value; + + varValue = virHashLookup(hash->hashTable, cie->varNames[0]); + + value = virNWFilterVarValueGetNthValue(varValue, cie->curValue); + + for (i = 0; i < cie->curValue; i++) { + if (STREQ(value, virNWFilterVarValueGetNthValue(varValue, i))) { + bool isSame = true; + for (j = 1; j < cie->nVarNames; j++) { + tmp = virHashLookup(hash->hashTable, cie->varNames[j]); + if (!STREQ(virNWFilterVarValueGetNthValue(tmp, cie->curValue), + virNWFilterVarValueGetNthValue(tmp, i))) { + isSame = false; + break; + } + } + if (isSame) + return false; + } + } + + return true; +} + +/* * Create an iterator over the contents of the given variables. All variables * must have entries in the hash table. * The iterator that is created processes all given variables in parallel, @@ -414,10 +460,15 @@ virNWFilterVarCombIterNext(virNWFilterVa unsigned int i; for (i = 0; i < ci->nIter; i++) { +next: ci->iter[i].curValue++; - if (ci->iter[i].curValue <= ci->iter[i].maxValue) + if (ci->iter[i].curValue <= ci->iter[i].maxValue) { + if (!virNWFilterVarCombIterEntryAreUniqueEntries( + &ci->iter[i], ci->hashTable)) { + goto next; + } break; - else + } else ci->iter[i].curValue = 0; }
On 12/09/2011 08:07 AM, Stefan Berger wrote:
In this patch we introduce testing whether the iterator points to a unique set of entries that have not been seen before at one of the previous iterations. The point is to eliminate duplicates and with that unnecessary filtering rules by preventing identical filtering rules from being instantiated. Example with two lists:
list1 = [1,2,1] list2 = [1,3,1]
Is this something that happens frequently in practice due to user input (that is, does the user really output list elements more than once), or something that happens more because of combinatorial combinations as you expand user inputs into something more precise? I guess I'd like to know why such repetitions are likely to occur, although I can agree with doing less firewall rules if we can determine that the rules are duplicates.
The 1st iteration would take the 1st items of each list -> 1,1 The 2nd iteration would take the 2nd items of each list -> 2,3 The 3rd iteration would take the 3rd items of each list -> 1,1 but skip them since this same pair has already been encountered in the 1st iteration
Implementation-wise this is solved by taking the n-th element of list1 and comparing it against elements 1..n-1. If no equivalent is found, then there is no possibility of this being a duplicate. In case an equivalent element is found at position i, then the n-th element in the 2nd list is compared against the i-th element in the 2nd list and if that is not the same, then this is a unique pair, otherwise it is not unique and we may need to do the same comparison on the 3rd list.
--- src/conf/nwfilter_params.c | 55 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-)
Index: libvirt-iterator/src/conf/nwfilter_params.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.c +++ libvirt-iterator/src/conf/nwfilter_params.c @@ -348,6 +348,52 @@ virNWFilterVarCombIterAddVariable(virNWF }
/* + * Test whether the iterator entry points to a distinguished set of entries + * that have not been seen before at one of the previous iterations. + * + * The point of this function is to eliminate duplicates. + * Example with two lists: + * + * list1 = [1,2,1] + * list2 = [1,3,1] + * + * The 1st iteration would take the 1st items of each list -> 1,1 + * The 2nd iteration would take the 2nd items of each list -> 2,3 + * The 3rd iteration would take the 3rd items of each list -> 1,1 but + * skip them since this pair has already been encountered in the 1st iteration + */ +static bool +virNWFilterVarCombIterEntryAreUniqueEntries(virNWFilterVarCombIterEntryPtr cie, + virNWFilterHashTablePtr hash) +{ + unsigned int i, j; + virNWFilterVarValuePtr varValue, tmp; + const char *value; + + varValue = virHashLookup(hash->hashTable, cie->varNames[0]); + + value = virNWFilterVarValueGetNthValue(varValue, cie->curValue); + + for (i = 0; i < cie->curValue; i++) { + if (STREQ(value, virNWFilterVarValueGetNthValue(varValue, i))) { + bool isSame = true;
This function is called in a loop over the list, and itself does a loop over the list, making it a quadratic check for duplicates. Am I correct that lists can provided by the user with arbitrary lengths?
+ for (j = 1; j < cie->nVarNames; j++) {
Furthermore, we add another layer of looping (this time over the number of parallel lists being combined), for an overall cubic algorithm. Is the number of parallel lists that can be provided by the user arbitrary? That is, I'm wondering if we need to start worrying about reducing complexity, and coming up with an algorithm that isn't quite so brute force so as not to take quite so long on arbitrary-length inputs. On the other hand, until someone actually demonstrates slow input, I'm okay with making assumptions that most users won't be merging large amounts of long parallel lists, and that the overhead of reducing algorithmic complexity may cost more overhead than all expected use cases just being brute-forced. So I'm okay checking this in as-is, until (and unless) we have performance numbers proving that we need to be smarter.
@@ -414,10 +460,15 @@ virNWFilterVarCombIterNext(virNWFilterVa unsigned int i;
for (i = 0; i < ci->nIter; i++) { +next: ci->iter[i].curValue++; - if (ci->iter[i].curValue <= ci->iter[i].maxValue) + if (ci->iter[i].curValue <= ci->iter[i].maxValue) { + if (!virNWFilterVarCombIterEntryAreUniqueEntries( + &ci->iter[i], ci->hashTable)) { + goto next;
Why do we need a backwards goto, when a 'continue' would get to the same location?
+ } break; - else + } else ci->iter[i].curValue = 0; }
Coding style - the else needs {} since you added {} for the if. I think this is okay, but wait until I review the rest of the series to see how it was used in context. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/09/2012 07:05 PM, Eric Blake wrote:
On 12/09/2011 08:07 AM, Stefan Berger wrote:
In this patch we introduce testing whether the iterator points to a unique set of entries that have not been seen before at one of the previous iterations. The point is to eliminate duplicates and with that unnecessary filtering rules by preventing identical filtering rules from being instantiated. Example with two lists:
list1 = [1,2,1] list2 = [1,3,1] Is this something that happens frequently in practice due to user input (that is, does the user really output list elements more than once), or something that happens more because of combinatorial combinations as you expand user inputs into something more precise? I guess I'd like to know why such repetitions are likely to occur, although I can agree with doing less firewall rules if we can determine that the rules are duplicates.
Fair question. It's an optimization that I would admit is not absolutely necessary, but reduces the number of rules that are being generated in case duplicates would occur. Doing this calculation upfront rather than having duplicate rules evaluated on possibly thousands of packets seems worth the effort. The algorithm is rather short but maybe not as efficient as it should be.
/* + * Test whether the iterator entry points to a distinguished set of entries + * that have not been seen before at one of the previous iterations. + * + * The point of this function is to eliminate duplicates. + * Example with two lists: + * + * list1 = [1,2,1] + * list2 = [1,3,1] + * + * The 1st iteration would take the 1st items of each list -> 1,1 + * The 2nd iteration would take the 2nd items of each list -> 2,3 + * The 3rd iteration would take the 3rd items of each list -> 1,1 but + * skip them since this pair has already been encountered in the 1st iteration + */ +static bool +virNWFilterVarCombIterEntryAreUniqueEntries(virNWFilterVarCombIterEntryPtr cie, + virNWFilterHashTablePtr hash) +{ + unsigned int i, j; + virNWFilterVarValuePtr varValue, tmp; + const char *value; + + varValue = virHashLookup(hash->hashTable, cie->varNames[0]); + + value = virNWFilterVarValueGetNthValue(varValue, cie->curValue); + + for (i = 0; i< cie->curValue; i++) { + if (STREQ(value, virNWFilterVarValueGetNthValue(varValue, i))) { + bool isSame = true; This function is called in a loop over the list, and itself does a loop over the list, making it a quadratic check for duplicates. Am I correct that lists can provided by the user with arbitrary lengths?
Each variable can have an number of elements >= 1, for example A=[10,20,30,20].
+ for (j = 1; j< cie->nVarNames; j++) { Furthermore, we add another layer of looping (this time over the number of parallel lists being combined), for an overall cubic algorithm. Is the number of parallel lists that can be provided by the user arbitrary?
The number of lists depends on the number of variables that are provided in one rule. So, $A, $B, and $C in one rule in different fields like this one below has 3 different lists (where the iterators ('@<n>') only change the behaviour for how the instantiated rules are generated): <rule action='accept' direction='out'> <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]' dstportstart='$C[@3]' dscp='4'/> </rule>
That is, I'm wondering if we need to start worrying about reducing complexity, and coming up with an algorithm that isn't quite so brute force so as not to take quite so long on arbitrary-length inputs.
In case of no duplicates in $A, we have a complexity of O(n) [optimal case] for checking of a single rule (only 'vertical' checking, no 'horizontal' checking) In case $A has duplicates the complexity then becomes O(n^2) in the worst case since we do horizonal checking accross $B and $C as well. I think the elements in $A for example would have to be sorted for faster detection of duplicates. A = [10,20,30,20] -> A = [10,20,20,30]. Now if I step on A[2], I would see that A[2-1] = A[2] and wouldn't have to look at A[0] at all. So that would be O[1]. But sorting the elements may alter their meaning in terms of the sequential processing of the rules. Got to think about this...
On the other hand, until someone actually demonstrates slow input, I'm okay with making assumptions that most users won't be merging large amounts of long parallel lists, and that the overhead of reducing algorithmic complexity may cost more overhead than all expected use cases just being brute-forced. So I'm okay checking this in as-is, until (and unless) we have performance numbers proving that we need to be smarter.
OK.
@@ -414,10 +460,15 @@ virNWFilterVarCombIterNext(virNWFilterVa unsigned int i;
for (i = 0; i< ci->nIter; i++) { +next: ci->iter[i].curValue++; - if (ci->iter[i].curValue<= ci->iter[i].maxValue) + if (ci->iter[i].curValue<= ci->iter[i].maxValue) { + if (!virNWFilterVarCombIterEntryAreUniqueEntries( +&ci->iter[i], ci->hashTable)) { + goto next; Why do we need a backwards goto, when a 'continue' would get to the same location?
Using the goto we don't increase 'i' but only do ci->iter[i].curValue++, which is what is needed.
+ } break; - else + } else ci->iter[i].curValue = 0; } Coding style - the else needs {} since you added {} for the if. Fixed.
I think this is okay, but wait until I review the rest of the series to see how it was used in context.
I'll wait. Thanks. Stefan
This patch introduces the capability to use a different iterator per variable. The currently supported notation of variables in a filtering rule like <rule action='accept' direction='out'> <tcp srcipaddr='$A' srcportstart='$B'/> </rule> processes the two lists 'A' and 'B' in parallel. This means that A and B must have the same number of 'N' elements and that 'N' rules will be instantiated (assuming all tuples from A and B are unique). In this patch we now introduce the assignment of variables to different iterators. Therefore a rule like <rule action='accept' direction='out'> <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]'/> </rule> will now create every combination of elements in A with elements in B since A has been assigned to an iterator with Id '1' and B has been assigned to an iterator with Id '2', thus processing their value independently. The first rule has an equivalent notation of <rule action='accept' direction='out'> <tcp srcipaddr='$A[@0]' srcportstart='$B[@0]'/> </rule> --- docs/schemas/nwfilter.rng | 71 ++------ src/conf/nwfilter_conf.c | 35 ++-- src/conf/nwfilter_conf.h | 6 src/conf/nwfilter_params.c | 239 +++++++++++++++++++++++++++--- src/conf/nwfilter_params.h | 38 ++++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 13 + src/nwfilter/nwfilter_gentech_driver.c | 8 - 8 files changed, 307 insertions(+), 104 deletions(-) Index: libvirt-iterator/src/conf/nwfilter_conf.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_conf.h +++ libvirt-iterator/src/conf/nwfilter_conf.h @@ -121,7 +121,7 @@ typedef struct _nwItemDesc nwItemDesc; typedef nwItemDesc *nwItemDescPtr; struct _nwItemDesc { enum virNWFilterEntryItemFlags flags; - char *var; + virNWFilterVarAccessPtr varAccess; enum attrDatatype datatype; union { nwMACAddress macaddr; @@ -470,8 +470,8 @@ struct _virNWFilterRuleDef { sctpHdrFilterDef sctpHdrFilter; } p; - int nvars; - char **vars; + size_t nVarAccess; + virNWFilterVarAccessPtr *varAccess; int nstrings; char **strings; Index: libvirt-iterator/src/conf/nwfilter_conf.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_conf.c +++ libvirt-iterator/src/conf/nwfilter_conf.c @@ -272,13 +272,13 @@ virNWFilterRuleDefFree(virNWFilterRuleDe if (!def) return; - for (i = 0; i < def->nvars; i++) - VIR_FREE(def->vars[i]); + for (i = 0; i < def->nVarAccess; i++) + virNWFilterVarAccessFree(def->varAccess[i]); for (i = 0; i < def->nstrings; i++) VIR_FREE(def->strings[i]); - VIR_FREE(def->vars); + VIR_FREE(def->varAccess); VIR_FREE(def->strings); VIR_FREE(def); @@ -358,28 +358,28 @@ virNWFilterRuleDefAddVar(virNWFilterRule const char *var) { int i = 0; + virNWFilterVarAccessPtr varAccess; - if (nwf->vars) { - for (i = 0; i < nwf->nvars; i++) - if (STREQ(nwf->vars[i], var)) { - item->var = nwf->vars[i]; + varAccess = virNWFilterVarAccessParse(var); + if (varAccess == NULL) + return -1; + + if (nwf->varAccess) { + for (i = 0; i < nwf->nVarAccess; i++) + if (virNWFilterVarAccessEqual(nwf->varAccess[i], varAccess)) { + virNWFilterVarAccessFree(varAccess); + item->varAccess = nwf->varAccess[i]; return 0; } } - if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) { - virReportOOMError(); - return -1; - } - - nwf->vars[nwf->nvars] = strdup(var); - - if (!nwf->vars[nwf->nvars]) { + if (VIR_REALLOC_N(nwf->varAccess, nwf->nVarAccess + 1) < 0) { virReportOOMError(); return -1; } - item->var = nwf->vars[nwf->nvars++]; + nwf->varAccess[nwf->nVarAccess++] = varAccess; + item->varAccess = varAccess; return 0; } @@ -3069,7 +3069,8 @@ virNWFilterRuleDefDetailsFormat(virBuffe goto err_exit; } } else if ((flags & NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) { - virBufferAsprintf(buf, "$%s", item->var); + virBufferAddLit(buf, "$"); + virNWFilterVarAccessPrint(item->varAccess, buf); } else { asHex = false; Index: libvirt-iterator/src/conf/nwfilter_params.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.c +++ libvirt-iterator/src/conf/nwfilter_params.c @@ -310,10 +310,11 @@ virNWFilterVarCombIterEntryInit(virNWFil static int virNWFilterVarCombIterAddVariable(virNWFilterVarCombIterEntryPtr cie, virNWFilterHashTablePtr hash, - const char *varName) + const virNWFilterVarAccessPtr varAccess) { virNWFilterVarValuePtr varValue; unsigned int cardinality; + const char *varName = virNWFilterVarAccessGetVarName(varAccess); varValue = virHashLookup(hash->hashTable, varName); if (varValue == NULL) { @@ -409,13 +410,14 @@ virNWFilterVarCombIterEntryAreUniqueEntr */ virNWFilterVarCombIterPtr virNWFilterVarCombIterCreate(virNWFilterHashTablePtr hash, - char * const *vars, unsigned int nVars) + const virNWFilterVarAccessPtr *varAccess, + size_t nVarAccess) { virNWFilterVarCombIterPtr res; unsigned int i, iterId; - int iterIndex; + int iterIndex = -1; - if (VIR_ALLOC_VAR(res, virNWFilterVarCombIterEntry, 1) < 0) { + if (VIR_ALLOC_VAR(res, virNWFilterVarCombIterEntry, 1 + nVarAccess) < 0) { virReportOOMError(); return NULL; } @@ -428,22 +430,24 @@ virNWFilterVarCombIterCreate(virNWFilter res->nIter = 1; virNWFilterVarCombIterEntryInit(&res->iter[0], iterId); - for (i = 0; i < nVars; i++) { - - /* currently always access @0 */ - iterId = 0; - - iterIndex = virNWFilterVarCombIterGetIndexByIterId(res, iterId); - if (iterIndex < 0) { - /* future: create new iterator. for now it's a bug */ - virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not find iterator with id %u"), - iterId); - goto err_exit; + for (i = 0; i < nVarAccess; i++) { + switch (virNWFilterVarAccessGetType(varAccess[i])) { + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + iterId = virNWFilterVarAccessGetIterId(varAccess[i]); + iterIndex = virNWFilterVarCombIterGetIndexByIterId(res, iterId); + if (iterIndex < 0) { + iterIndex = res->nIter; + virNWFilterVarCombIterEntryInit(&res->iter[iterIndex], iterId); + res->nIter++; + } + break; + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + case VIR_NWFILTER_VAR_ACCESS_LAST: + break; } if (virNWFilterVarCombIterAddVariable(&res->iter[iterIndex], - hash, vars[i]) < 0) + hash, varAccess[i]) < 0) goto err_exit; } @@ -482,16 +486,33 @@ next: const char * virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr ci, - const char *varName) + const virNWFilterVarAccessPtr vap) { - unsigned int i; + unsigned int i, iterId; bool found = false; const char *res = NULL; virNWFilterVarValuePtr value; - unsigned int iterIndex; + int iterIndex = -1; + const char *varName = virNWFilterVarAccessGetVarName(vap); - /* currently always accessing iter @0 */ - iterIndex = 0; + switch (virNWFilterVarAccessGetType(vap)) { + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + iterId = virNWFilterVarAccessGetIterId(vap); + iterIndex = virNWFilterVarCombIterGetIndexByIterId(ci, iterId); + if (iterIndex < 0) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get iterator index for " + "iterator ID %u"), iterId); + return NULL; + } + break; + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("Element access via index is not possible")); + return NULL; + case VIR_NWFILTER_VAR_ACCESS_LAST: + return NULL; + } for (i = 0; i < ci->iter[iterIndex].nVarNames; i++) { if (STREQ(ci->iter[iterIndex].varNames[i], varName)) { @@ -830,3 +851,177 @@ virNWFilterFormatParamAttributes(virBuff return 0; } + +void +virNWFilterVarAccessFree(virNWFilterVarAccessPtr varAccess) +{ + if (!varAccess) + return; + + VIR_FREE(varAccess->varName); + VIR_FREE(varAccess); +} + +bool +virNWFilterVarAccessEqual(const virNWFilterVarAccessPtr a, + const virNWFilterVarAccessPtr b) +{ + if (a->accessType != b->accessType) + return false; + + if (STRNEQ(a->varName, b->varName)) + return false; + + switch (a->accessType) { + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + return (a->u.index == b->u.index); + break; + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + return (a->u.iterId == b->u.iterId); + break; + case VIR_NWFILTER_VAR_ACCESS_LAST: + break; + } + return false; +} + +/* + * Parse a variable access like + * IP, IP[@2], IP[3] + */ +virNWFilterVarAccessPtr +virNWFilterVarAccessParse(const char *varAccess) +{ + size_t idx, varNameLen; + virNWFilterVarAccessPtr dest; + const char *input = varAccess; + + if (VIR_ALLOC(dest) < 0) { + virReportOOMError(); + return NULL; + } + + idx = strspn(input, VALID_VARNAME); + + if (input[idx] == '\0') { + /* in the form 'IP', which is equivalent to IP[@0] */ + dest->varName = strndup(input, idx); + if (!dest->varName) { + virReportOOMError(); + goto err_exit; + } + dest->accessType = VIR_NWFILTER_VAR_ACCESS_ITERATOR; + dest->u.iterId = 0; + return dest; + } + + if (input[idx] == '[') { + char *end_ptr; + unsigned int result; + bool parseError = false; + + varNameLen = idx; + + dest->varName = strndup(input, varNameLen); + if (!dest->varName) { + virReportOOMError(); + goto err_exit; + } + + input += idx + 1; + virSkipSpaces(&input); + + if (*input == '@') { + /* in the form 'IP[@<number>] -> iterator */ + dest->accessType = VIR_NWFILTER_VAR_ACCESS_ITERATOR; + input++; + } else { + /* in the form 'IP[<number>] -> element */ + dest->accessType = VIR_NWFILTER_VAR_ACCESS_ELEMENT; + /* not supported (yet) */ + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Variable access in the form " + "var[<index>] is not supported")); + goto err_exit; + } + + if (virStrToLong_ui(input, &end_ptr, 10, &result) < 0) + parseError = true; + if (!parseError) { + input = end_ptr; + virSkipSpaces(&input); + if (*input != ']') + parseError = true; + } + if (parseError) { + const char *what = "iterator id"; + if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT) + what = "array index"; + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Malformatted %s"), what); + goto err_exit; + } + + switch (dest->accessType) { + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + dest->u.index = result; + break; + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + if (result > VIR_NWFILTER_MAX_ITERID) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Iterator ID exceeds maximum ID " + "of %u"), VIR_NWFILTER_MAX_ITERID); + goto err_exit; + } + dest->u.iterId = result; + break; + case VIR_NWFILTER_VAR_ACCESS_LAST: + goto err_exit; + } + + return dest; + } else { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Malformatted variable")); + } + +err_exit: + virNWFilterVarAccessFree(dest); + + return NULL; +} + +void +virNWFilterVarAccessPrint(virNWFilterVarAccessPtr vap, virBufferPtr buf) +{ + virBufferAdd(buf, vap->varName, -1); + switch (vap->accessType) { + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + virBufferAsprintf(buf, "[%u]", vap->u.index); + break; + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + if (vap->u.iterId != 0) + virBufferAsprintf(buf, "[@%u]", vap->u.iterId); + break; + case VIR_NWFILTER_VAR_ACCESS_LAST: + break; + } +} + +const char * +virNWFilterVarAccessGetVarName(const virNWFilterVarAccessPtr vap) +{ + return vap->varName; +} + +enum virNWFilterVarAccessType +virNWFilterVarAccessGetType(const virNWFilterVarAccessPtr vap) +{ + return vap->accessType; +} + +unsigned int +virNWFilterVarAccessGetIterId(const virNWFilterVarAccessPtr vap) +{ + return vap->u.iterId; +} Index: libvirt-iterator/src/conf/nwfilter_params.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.h +++ libvirt-iterator/src/conf/nwfilter_params.h @@ -91,6 +91,38 @@ int virNWFilterHashTablePutAll(virNWFilt # define VALID_VARVALUE \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:" +enum virNWFilterVarAccessType { + VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0, + VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1, + + VIR_NWFILTER_VAR_ACCESS_LAST, +}; + +typedef struct _virNWFilterVarAccess virNWFilterVarAccess; +typedef virNWFilterVarAccess *virNWFilterVarAccessPtr; +struct _virNWFilterVarAccess { + enum virNWFilterVarAccessType accessType; + union { + unsigned int index; + unsigned int iterId; + } u; + char *varName; +}; + +# define VIR_NWFILTER_MAX_ITERID 1000 + +void virNWFilterVarAccessFree(virNWFilterVarAccessPtr varAccess); +bool virNWFilterVarAccessEqual(const virNWFilterVarAccessPtr a, + const virNWFilterVarAccessPtr b); +virNWFilterVarAccessPtr virNWFilterVarAccessParse(const char *varAccess); +void virNWFilterVarAccessPrint(virNWFilterVarAccessPtr vap, + virBufferPtr buf); +const char *virNWFilterVarAccessGetVarName(const virNWFilterVarAccessPtr vap); +enum virNWFilterVarAccessType virNWFilterVarAccessGetType( + const virNWFilterVarAccessPtr vap); +unsigned int virNWFilterVarAccessGetIterId(const virNWFilterVarAccessPtr vap); + + typedef struct _virNWFilterVarCombIterEntry virNWFilterVarCombIterEntry; typedef virNWFilterVarCombIterEntry *virNWFilterVarCombIterEntryPtr; struct _virNWFilterVarCombIterEntry { @@ -110,12 +142,14 @@ struct _virNWFilterVarCombIter { }; virNWFilterVarCombIterPtr virNWFilterVarCombIterCreate( virNWFilterHashTablePtr hash, - char * const *vars, unsigned int nVars); + const virNWFilterVarAccessPtr *vars, + size_t nVars); void virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci); virNWFilterVarCombIterPtr virNWFilterVarCombIterNext( virNWFilterVarCombIterPtr ci); const char *virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr ci, - const char *varname); + const virNWFilterVarAccessPtr); + #endif /* NWFILTER_PARAMS_H */ Index: libvirt-iterator/src/libvirt_private.syms =================================================================== --- libvirt-iterator.orig/src/libvirt_private.syms +++ libvirt-iterator/src/libvirt_private.syms @@ -844,6 +844,7 @@ virNWFilterHashTableFree; virNWFilterHashTablePut; virNWFilterHashTablePutAll; virNWFilterHashTableRemoveEntry; +virNWFilterVarAccessGetVarName; virNWFilterVarCombIterCreate; virNWFilterVarCombIterFree; virNWFilterVarCombIterGetVarValue; Index: libvirt-iterator/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-iterator.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-iterator/src/nwfilter/nwfilter_ebiptables_driver.c @@ -230,17 +230,19 @@ printVar(virNWFilterVarCombIterPtr vars, if ((item->flags & NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) { const char *val; - val = virNWFilterVarCombIterGetVarValue(vars, item->var); + val = virNWFilterVarCombIterGetVarValue(vars, item->varAccess); if (!val) { /* error has been reported */ return -1; } if (!virStrcpy(buf, val, bufsize)) { + const char *varName; + + varName = virNWFilterVarAccessGetVarName(item->varAccess); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _("Buffer too small to print MAC address " - "'%s' into"), - item->var); + _("Buffer too small to print variable " + "'%s' into"), varName); return -1; } @@ -2631,7 +2633,8 @@ ebiptablesCreateRuleInstanceIterate( * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ - vciter = virNWFilterVarCombIterCreate(vars, rule->vars, rule->nvars); + vciter = virNWFilterVarCombIterCreate(vars, + rule->varAccess, rule->nVarAccess); if (!vciter) return -1; Index: libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-iterator.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c @@ -500,14 +500,16 @@ virNWFilterDetermineMissingVarsRec(virNW virNWFilterIncludeDefPtr inc = filter->filterEntries[i]->include; if (rule) { /* check all variables of this rule */ - for (j = 0; j < rule->nvars; j++) { - if (!virHashLookup(vars->hashTable, rule->vars[j])) { + for (j = 0; j < rule->nVarAccess; j++) { + const char *varName; + varName = virNWFilterVarAccessGetVarName(rule->varAccess[j]); + if (!virHashLookup(vars->hashTable, varName)) { val = virNWFilterVarValueCreateSimpleCopyValue("1"); if (!val) { rc = -1; break; } - virNWFilterHashTablePut(missing_vars, rule->vars[j], + virNWFilterHashTablePut(missing_vars, varName, val, 1); } } Index: libvirt-iterator/docs/schemas/nwfilter.rng =================================================================== --- libvirt-iterator.orig/docs/schemas/nwfilter.rng +++ libvirt-iterator/docs/schemas/nwfilter.rng @@ -811,12 +811,15 @@ </choice> </define> + <define name="variable-name-type"> + <data type="string"> + <param name="pattern">$[a-zA-Z0-9_]+(\[[ ]*[@]?[0-9]+[ ]*\])?</param> + </data> + </define> + <define name="addrMAC"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">([a-fA-F0-9]{1,2}:){5}[a-fA-F0-9]{1,2}</param> @@ -826,10 +829,7 @@ <define name="addrIP"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">([0-2]?[0-9]?[0-9]\.){3}[0-2]?[0-9]?[0-9]</param> @@ -839,10 +839,7 @@ <define name="addrIPv6"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">([a-fA-F0-9]{0,4}:){2,7}([a-fA-F0-9]*)(([0-2]?[0-9]?[0-9]\.){3}[0-2]?[0-9]?[0-9])?</param> @@ -852,10 +849,7 @@ <define name="addrMask"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="int"> <param name="minInclusive">0</param> @@ -870,10 +864,7 @@ <define name="addrMaskv6"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="int"> <param name="minInclusive">0</param> @@ -892,10 +883,7 @@ <param name="pattern">0x([0-3][0-9a-fA-F]|[0-9a-fA-F])</param> </data> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="int"> <param name="minInclusive">0</param> @@ -906,10 +894,7 @@ <define name="mac-protocolid"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x([6-9a-fA-F][0-9a-fA-F]{2}|[0-9a-fA-F]{4})</param> @@ -932,10 +917,7 @@ <define name="vlan-vlanid"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x([0-9a-fA-F]{1,3})</param> @@ -950,10 +932,7 @@ <define name="uint8range"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x[0-9a-fA-F]{1,2}</param> @@ -968,10 +947,7 @@ <define name="uint16range"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x[0-9a-fA-F]{1,4}</param> @@ -986,10 +962,7 @@ <define name="uint32range"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x[0-9a-fA-F]{1,8}</param> @@ -1015,10 +988,7 @@ <define name="arpOpcodeType"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="int"> <param name="minInclusive">0</param> @@ -1034,10 +1004,7 @@ <define name="ipProtocolType"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param> - </data> + <ref name="variable-name-type"/> <data type="string"> <param name="pattern">0x[0-9a-fA-F]{1,2}</param>
s/possiblity/possibility/ in the subject On 12/09/2011 08:07 AM, Stefan Berger wrote:
This patch introduces the capability to use a different iterator per variable.
The currently supported notation of variables in a filtering rule like
<rule action='accept' direction='out'> <tcp srcipaddr='$A' srcportstart='$B'/> </rule>
processes the two lists 'A' and 'B' in parallel. This means that A and B must have the same number of 'N' elements and that 'N' rules will be instantiated (assuming all tuples from A and B are unique).
In this patch we now introduce the assignment of variables to different iterators. Therefore a rule like
<rule action='accept' direction='out'> <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]'/> </rule>
will now create every combination of elements in A with elements in B since A has been assigned to an iterator with Id '1' and B has been assigned to an iterator with Id '2', thus processing their value independently.
The first rule has an equivalent notation of
<rule action='accept' direction='out'> <tcp srcipaddr='$A[@0]' srcportstart='$B[@0]'/> </rule>
--- docs/schemas/nwfilter.rng | 71 ++------
This needs something in docs/formatnwfilter.html.in as well.
src/conf/nwfilter_conf.c | 35 ++-- src/conf/nwfilter_conf.h | 6 src/conf/nwfilter_params.c | 239 +++++++++++++++++++++++++++--- src/conf/nwfilter_params.h | 38 ++++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 13 + src/nwfilter/nwfilter_gentech_driver.c | 8 - 8 files changed, 307 insertions(+), 104 deletions(-)
- if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) { - virReportOOMError(); - return -1; - } - - nwf->vars[nwf->nvars] = strdup(var); - - if (!nwf->vars[nwf->nvars]) { + if (VIR_REALLOC_N(nwf->varAccess, nwf->nVarAccess + 1) < 0) {
I'm okay with this as-is, but you might want to switch to VIR_EXPAND_N for slightly easier usage.
@@ -3069,7 +3069,8 @@ virNWFilterRuleDefDetailsFormat(virBuffe goto err_exit; } } else if ((flags & NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) { - virBufferAsprintf(buf, "$%s", item->var); + virBufferAddLit(buf, "$");
I prefer virBufferAddChar(buf, '$'); but under the hood, it's practically the same amount of work.
+ if (parseError) { + const char *what = "iterator id"; + if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT) + what = "array index";
"iterator id" and "array index" should probably be marked for translation, but see my next comment...
+ virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Malformatted %s"), what);
That doesn't make life easy for translators (in languages where nouns can be masculine or feminine, the adjective translation for "malformatted" may have a different spelling for the two possilibities for what). Rather, I'd write: if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT) virNWFilterReportError(VIR_ERR_INVALID_ARG, _("Malformatted array index")); else virNWFilterReportError(VIR_ERR_INVALID_ARG, _("Malformatted iterator id"));
Index: libvirt-iterator/src/conf/nwfilter_params.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.h +++ libvirt-iterator/src/conf/nwfilter_params.h @@ -91,6 +91,38 @@ int virNWFilterHashTablePutAll(virNWFilt # define VALID_VARVALUE \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
+enum virNWFilterVarAccessType { + VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0, + VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1, + + VIR_NWFILTER_VAR_ACCESS_LAST, +}; + +typedef struct _virNWFilterVarAccess virNWFilterVarAccess;
Is the double-space intentional?
Index: libvirt-iterator/docs/schemas/nwfilter.rng =================================================================== --- libvirt-iterator.orig/docs/schemas/nwfilter.rng +++ libvirt-iterator/docs/schemas/nwfilter.rng @@ -811,12 +811,15 @@ </choice> </define>
+ <define name="variable-name-type"> + <data type="string"> + <param name="pattern">$[a-zA-Z0-9_]+(\[[ ]*[@]?[0-9]+[ ]*\])?</param>
Does this need to be \$ instead of $, to avoid the meta-meaning of $ as EOL?
+ </data> + </define> + <define name="addrMAC"> <choice> - <!-- variable --> - <data type="string"> - <param name="pattern">$[a-zA-Z0-9_]+</param>
Then again, you were previously using it unquoted, so maybe this is a latent bug. My guess is that we need to add some sample files somewhere under tests/*data/*.xml, and ensure that they get schema-tested as part of 'make check', to prove that our schema makes sense. A quick git grep 'srcmacaddr=.\$' tests didn't turn up any existing xml files that would exercise this part of the rng file. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/10/2012 10:40 AM, Eric Blake wrote:
will now create every combination of elements in A with elements in B since A has been assigned to an iterator with Id '1' and B has been assigned to an iterator with Id '2', thus processing their value independently.
The first rule has an equivalent notation of
<rule action='accept' direction='out'> <tcp srcipaddr='$A[@0]' srcportstart='$B[@0]'/> </rule>
--- docs/schemas/nwfilter.rng | 71 ++------
This needs something in docs/formatnwfilter.html.in as well.
Serves me right for not glancing through the whole series first; I see you did this in 6/6. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/10/2012 12:40 PM, Eric Blake wrote:
s/possiblity/possibility/ in the subject
Fixed.
On 12/09/2011 08:07 AM, Stefan Berger wrote:
This patch introduces the capability to use a different iterator per variable.
The currently supported notation of variables in a filtering rule like
<rule action='accept' direction='out'> <tcp srcipaddr='$A' srcportstart='$B'/> </rule>
processes the two lists 'A' and 'B' in parallel. This means that A and B must have the same number of 'N' elements and that 'N' rules will be instantiated (assuming all tuples from A and B are unique).
In this patch we now introduce the assignment of variables to different iterators. Therefore a rule like
<rule action='accept' direction='out'> <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]'/> </rule>
will now create every combination of elements in A with elements in B since A has been assigned to an iterator with Id '1' and B has been assigned to an iterator with Id '2', thus processing their value independently.
The first rule has an equivalent notation of
<rule action='accept' direction='out'> <tcp srcipaddr='$A[@0]' srcportstart='$B[@0]'/> </rule>
--- docs/schemas/nwfilter.rng | 71 ++------ This needs something in docs/formatnwfilter.html.in as well.
In 6/6.
src/conf/nwfilter_conf.c | 35 ++-- src/conf/nwfilter_conf.h | 6 src/conf/nwfilter_params.c | 239 +++++++++++++++++++++++++++--- src/conf/nwfilter_params.h | 38 ++++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 13 + src/nwfilter/nwfilter_gentech_driver.c | 8 - 8 files changed, 307 insertions(+), 104 deletions(-)
- if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1)< 0) { - virReportOOMError(); - return -1; - } - - nwf->vars[nwf->nvars] = strdup(var); - - if (!nwf->vars[nwf->nvars]) { + if (VIR_REALLOC_N(nwf->varAccess, nwf->nVarAccess + 1)< 0) { I'm okay with this as-is, but you might want to switch to VIR_EXPAND_N for slightly easier usage.
Fixed.
@@ -3069,7 +3069,8 @@ virNWFilterRuleDefDetailsFormat(virBuffe goto err_exit; } } else if ((flags& NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) { - virBufferAsprintf(buf, "$%s", item->var); + virBufferAddLit(buf, "$"); I prefer virBufferAddChar(buf, '$'); but under the hood, it's practically the same amount of work.
Changed.
+ if (parseError) { + const char *what = "iterator id"; + if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT) + what = "array index"; "iterator id" and "array index" should probably be marked for translation, but see my next comment...
+ virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Malformatted %s"), what); That doesn't make life easy for translators (in languages where nouns can be masculine or feminine, the adjective translation for "malformatted" may have a different spelling for the two possilibities for what). Rather, I'd write:
if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT) virNWFilterReportError(VIR_ERR_INVALID_ARG, _("Malformatted array index")); else virNWFilterReportError(VIR_ERR_INVALID_ARG, _("Malformatted iterator id"));
Fixed. Thanks.
Index: libvirt-iterator/src/conf/nwfilter_params.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.h +++ libvirt-iterator/src/conf/nwfilter_params.h @@ -91,6 +91,38 @@ int virNWFilterHashTablePutAll(virNWFilt # define VALID_VARVALUE \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
+enum virNWFilterVarAccessType { + VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0, + VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1, + + VIR_NWFILTER_VAR_ACCESS_LAST, +}; + +typedef struct _virNWFilterVarAccess virNWFilterVarAccess; Is the double-space intentional?
No, fixed.
Index: libvirt-iterator/docs/schemas/nwfilter.rng =================================================================== --- libvirt-iterator.orig/docs/schemas/nwfilter.rng +++ libvirt-iterator/docs/schemas/nwfilter.rng @@ -811,12 +811,15 @@ </choice> </define>
+<define name="variable-name-type"> +<data type="string"> +<param name="pattern">$[a-zA-Z0-9_]+(\[[ ]*[@]?[0-9]+[ ]*\])?</param> Does this need to be \$ instead of $, to avoid the meta-meaning of $ as EOL?
It has to be just $. With \$ the test cases don't pass anymore.
+</data> +</define> + <define name="addrMAC"> <choice> -<!-- variable --> -<data type="string"> -<param name="pattern">$[a-zA-Z0-9_]+</param> Then again, you were previously using it unquoted, so maybe this is a latent bug. My guess is that we need to add some sample files somewhere under tests/*data/*.xml, and ensure that they get schema-tested as part of 'make check', to prove that our schema makes sense. A quick git grep 'srcmacaddr=.\$' tests didn't turn up any existing xml files that would exercise this part of the rng file.
Stefan
This patch adds access to single elements of variables via index. Example: <rule action='accept' direction='in' priority='500'> <tcp srcipaddr='$ADDR[1]' srcportstart='$B[2]'/> </rule> --- src/conf/nwfilter_params.c | 82 +++++++++++++++++++++++++++++++++++---------- src/conf/nwfilter_params.h | 9 +++- 2 files changed, 71 insertions(+), 20 deletions(-) Index: libvirt-iterator/src/conf/nwfilter_params.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.c +++ libvirt-iterator/src/conf/nwfilter_params.c @@ -35,7 +35,10 @@ #define VIR_FROM_THIS VIR_FROM_NWFILTER static bool isValidVarValue(const char *value); - +static void virNWFilterVarAccessSetIntIterId(virNWFilterVarAccessPtr, + unsigned int); +static unsigned int virNWFilterVarAccessGetIntIterId( + const virNWFilterVarAccessPtr); void virNWFilterVarValueFree(virNWFilterVarValuePtr val) @@ -313,7 +316,7 @@ virNWFilterVarCombIterAddVariable(virNWF const virNWFilterVarAccessPtr varAccess) { virNWFilterVarValuePtr varValue; - unsigned int cardinality; + unsigned int maxValue, minValue; const char *varName = virNWFilterVarAccessGetVarName(varAccess); varValue = virHashLookup(hash->hashTable, varName); @@ -324,12 +327,25 @@ virNWFilterVarCombIterAddVariable(virNWF return -1; } - cardinality = virNWFilterVarValueGetCardinality(varValue); + switch (virNWFilterVarAccessGetType(varAccess)) { + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + maxValue = virNWFilterVarAccessGetIndex(varAccess); + minValue = maxValue; + break; + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + maxValue = virNWFilterVarValueGetCardinality(varValue) - 1; + minValue = 0; + break; + case VIR_NWFILTER_VAR_ACCESS_LAST: + return -1; + } if (cie->nVarNames == 0) { - cie->maxValue = cardinality - 1; + cie->maxValue = maxValue; + cie->minValue = minValue; + cie->curValue = minValue; } else { - if (cie->maxValue != cardinality - 1) { + if (cie->maxValue != maxValue) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Cardinality of list items must be " "the same for processing them in " @@ -410,12 +426,13 @@ virNWFilterVarCombIterEntryAreUniqueEntr */ virNWFilterVarCombIterPtr virNWFilterVarCombIterCreate(virNWFilterHashTablePtr hash, - const virNWFilterVarAccessPtr *varAccess, + virNWFilterVarAccessPtr *varAccess, size_t nVarAccess) { virNWFilterVarCombIterPtr res; unsigned int i, iterId; int iterIndex = -1; + unsigned int nextIntIterId = VIR_NWFILTER_MAX_ITERID + 1; if (VIR_ALLOC_VAR(res, virNWFilterVarCombIterEntry, 1 + nVarAccess) < 0) { virReportOOMError(); @@ -442,6 +459,13 @@ virNWFilterVarCombIterCreate(virNWFilter } break; case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + iterIndex = res->nIter; + virNWFilterVarAccessSetIntIterId(varAccess[i], nextIntIterId); + virNWFilterVarCombIterEntryInit(&res->iter[iterIndex], + nextIntIterId); + nextIntIterId++; + res->nIter++; + break; case VIR_NWFILTER_VAR_ACCESS_LAST: break; } @@ -473,7 +497,7 @@ next: } break; } else - ci->iter[i].curValue = 0; + ci->iter[i].curValue = ci->iter[i].minValue; } if (ci->nIter == i) { @@ -507,9 +531,15 @@ virNWFilterVarCombIterGetVarValue(virNWF } break; case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _("Element access via index is not possible")); - return NULL; + iterId = virNWFilterVarAccessGetIntIterId(vap); + iterIndex = virNWFilterVarCombIterGetIndexByIterId(ci, iterId); + if (iterIndex < 0) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get iterator index for " + "(internal) iterator ID %u"), iterId); + return NULL; + } + break; case VIR_NWFILTER_VAR_ACCESS_LAST: return NULL; } @@ -874,7 +904,8 @@ virNWFilterVarAccessEqual(const virNWFil switch (a->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - return (a->u.index == b->u.index); + return (a->u.index.index == b->u.index.index && + a->u.index.intIterId == b->u.index.intIterId); break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: return (a->u.iterId == b->u.iterId); @@ -938,11 +969,6 @@ virNWFilterVarAccessParse(const char *va } else { /* in the form 'IP[<number>] -> element */ dest->accessType = VIR_NWFILTER_VAR_ACCESS_ELEMENT; - /* not supported (yet) */ - virNWFilterReportError(VIR_ERR_INVALID_ARG, - _("Variable access in the form " - "var[<index>] is not supported")); - goto err_exit; } if (virStrToLong_ui(input, &end_ptr, 10, &result) < 0) @@ -964,7 +990,8 @@ virNWFilterVarAccessParse(const char *va switch (dest->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - dest->u.index = result; + dest->u.index.index = result; + dest->u.index.intIterId = ~0; break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: if (result > VIR_NWFILTER_MAX_ITERID) { @@ -997,7 +1024,7 @@ virNWFilterVarAccessPrint(virNWFilterVar virBufferAdd(buf, vap->varName, -1); switch (vap->accessType) { case VIR_NWFILTER_VAR_ACCESS_ELEMENT: - virBufferAsprintf(buf, "[%u]", vap->u.index); + virBufferAsprintf(buf, "[%u]", vap->u.index.index); break; case VIR_NWFILTER_VAR_ACCESS_ITERATOR: if (vap->u.iterId != 0) @@ -1025,3 +1052,22 @@ virNWFilterVarAccessGetIterId(const virN { return vap->u.iterId; } + +unsigned int +virNWFilterVarAccessGetIndex(const virNWFilterVarAccessPtr vap) +{ + return vap->u.index.index; +} + +static void +virNWFilterVarAccessSetIntIterId(virNWFilterVarAccessPtr vap, + unsigned int intIterId) +{ + vap->u.index.intIterId = intIterId; +} + +static unsigned int +virNWFilterVarAccessGetIntIterId(const virNWFilterVarAccessPtr vap) +{ + return vap->u.index.intIterId; +} Index: libvirt-iterator/src/conf/nwfilter_params.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.h +++ libvirt-iterator/src/conf/nwfilter_params.h @@ -103,7 +103,10 @@ typedef virNWFilterVarAccess *virNWFilte struct _virNWFilterVarAccess { enum virNWFilterVarAccessType accessType; union { - unsigned int index; + struct { + unsigned int index; + unsigned int intIterId; + } index; unsigned int iterId; } u; char *varName; @@ -121,6 +124,7 @@ const char *virNWFilterVarAccessGetVarNa enum virNWFilterVarAccessType virNWFilterVarAccessGetType( const virNWFilterVarAccessPtr vap); unsigned int virNWFilterVarAccessGetIterId(const virNWFilterVarAccessPtr vap); +unsigned int virNWFilterVarAccessGetIndex(const virNWFilterVarAccessPtr vap); typedef struct _virNWFilterVarCombIterEntry virNWFilterVarCombIterEntry; @@ -131,6 +135,7 @@ struct _virNWFilterVarCombIterEntry { size_t nVarNames; unsigned int maxValue; unsigned int curValue; + unsigned int minValue; }; typedef struct _virNWFilterVarCombIter virNWFilterVarCombIter; @@ -142,7 +147,7 @@ struct _virNWFilterVarCombIter { }; virNWFilterVarCombIterPtr virNWFilterVarCombIterCreate( virNWFilterHashTablePtr hash, - const virNWFilterVarAccessPtr *vars, + virNWFilterVarAccessPtr *vars, size_t nVars); void virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci);
On 12/09/2011 08:07 AM, Stefan Berger wrote:
This patch adds access to single elements of variables via index. Example:
<rule action='accept' direction='in' priority='500'> <tcp srcipaddr='$ADDR[1]' srcportstart='$B[2]'/> </rule>
I suppose you can mix the two: pair _just_ the element $ADDR[1] with all possibilities of the list $B[@1]. Looks reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Address side effect of accessing a variable via an index: Filters accessing a variable where an element is accessed that is beyond the size of the list (for example $TEST[10] and only 2 elements are available) cannot instantiate that filter. Test for this and report proper error to user. --- src/conf/nwfilter_params.c | 29 +++++++++++++++++++++++++++++ src/conf/nwfilter_params.h | 3 ++- src/libvirt_private.syms | 2 ++ src/nwfilter/nwfilter_gentech_driver.c | 23 ++++++++++++++++++----- 4 files changed, 51 insertions(+), 6 deletions(-) Index: libvirt-iterator/src/conf/nwfilter_params.h =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.h +++ libvirt-iterator/src/conf/nwfilter_params.h @@ -125,7 +125,8 @@ enum virNWFilterVarAccessType virNWFilte const virNWFilterVarAccessPtr vap); unsigned int virNWFilterVarAccessGetIterId(const virNWFilterVarAccessPtr vap); unsigned int virNWFilterVarAccessGetIndex(const virNWFilterVarAccessPtr vap); - +bool virNWFilterVarAccessIsAvailable(const virNWFilterVarAccessPtr vap, + const virNWFilterHashTablePtr hash); typedef struct _virNWFilterVarCombIterEntry virNWFilterVarCombIterEntry; typedef virNWFilterVarCombIterEntry *virNWFilterVarCombIterEntryPtr; Index: libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-iterator.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c @@ -501,16 +501,29 @@ virNWFilterDetermineMissingVarsRec(virNW if (rule) { /* check all variables of this rule */ for (j = 0; j < rule->nVarAccess; j++) { - const char *varName; - varName = virNWFilterVarAccessGetVarName(rule->varAccess[j]); - if (!virHashLookup(vars->hashTable, varName)) { + if (!virNWFilterVarAccessIsAvailable(rule->varAccess[j], + vars)) { + const char *varAccess; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virNWFilterVarAccessPrint(rule->varAccess[j], &buf); + if (virBufferError(&buf)) { + virReportOOMError(); + rc = -1; + break; + } + val = virNWFilterVarValueCreateSimpleCopyValue("1"); if (!val) { + virBufferFreeAndReset(&buf); rc = -1; break; } - virNWFilterHashTablePut(missing_vars, varName, + + varAccess = virBufferContentAndReset(&buf); + virNWFilterHashTablePut(missing_vars, varAccess, val, 1); + VIR_FREE(varAccess); } } if (rc) @@ -752,7 +765,7 @@ err_unresolvable_vars: if (buf) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot instantiate filter due to unresolvable " - "variables: %s"), buf); + "variables or unavailable list elements: %s"), buf); VIR_FREE(buf); } Index: libvirt-iterator/src/conf/nwfilter_params.c =================================================================== --- libvirt-iterator.orig/src/conf/nwfilter_params.c +++ libvirt-iterator/src/conf/nwfilter_params.c @@ -1071,3 +1071,32 @@ virNWFilterVarAccessGetIntIterId(const v { return vap->u.index.intIterId; } + +bool +virNWFilterVarAccessIsAvailable(const virNWFilterVarAccessPtr varAccess, + const virNWFilterHashTablePtr hash) +{ + const char *varName = virNWFilterVarAccessGetVarName(varAccess); + const char *res; + unsigned int idx; + virNWFilterVarValuePtr varValue; + + varValue = virHashLookup(hash->hashTable, varName); + if (!varValue) + return false; + + switch (virNWFilterVarAccessGetType(varAccess)) { + case VIR_NWFILTER_VAR_ACCESS_ELEMENT: + idx = virNWFilterVarAccessGetIndex(varAccess); + res = virNWFilterVarValueGetNthValue(varValue, idx); + if (res == NULL) + return false; + break; + case VIR_NWFILTER_VAR_ACCESS_ITERATOR: + break; + case VIR_NWFILTER_VAR_ACCESS_LAST: + return false; + } + + return true; +} Index: libvirt-iterator/src/libvirt_private.syms =================================================================== --- libvirt-iterator.orig/src/libvirt_private.syms +++ libvirt-iterator/src/libvirt_private.syms @@ -845,6 +845,8 @@ virNWFilterHashTablePut; virNWFilterHashTablePutAll; virNWFilterHashTableRemoveEntry; virNWFilterVarAccessGetVarName; +virNWFilterVarAccessIsAvailable; +virNWFilterVarAccessPrint; virNWFilterVarCombIterCreate; virNWFilterVarCombIterFree; virNWFilterVarCombIterGetVarValue;
On 12/09/2011 08:07 AM, Stefan Berger wrote:
Address side effect of accessing a variable via an index: Filters accessing a variable where an element is accessed that is beyond the size of the list (for example $TEST[10] and only 2 elements are available) cannot instantiate that filter. Test for this and report proper error to user.
--- src/conf/nwfilter_params.c | 29 +++++++++++++++++++++++++++++ src/conf/nwfilter_params.h | 3 ++- src/libvirt_private.syms | 2 ++ src/nwfilter/nwfilter_gentech_driver.c | 23 ++++++++++++++++++----- 4 files changed, 51 insertions(+), 6 deletions(-)
Makes sense. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This patch adds a couple of XML parser / schema validator test cases for the new 'ways' to access variables via index or iterator. --- tests/nwfilterxml2xmlin/iter-test1.xml | 6 ++++++ tests/nwfilterxml2xmlin/iter-test2.xml | 23 +++++++++++++++++++++++ tests/nwfilterxml2xmlin/iter-test3.xml | 13 +++++++++++++ tests/nwfilterxml2xmlout/iter-test1.xml | 6 ++++++ tests/nwfilterxml2xmlout/iter-test2.xml | 21 +++++++++++++++++++++ tests/nwfilterxml2xmlout/iter-test3.xml | 12 ++++++++++++ tests/nwfilterxml2xmltest.c | 3 +++ 7 files changed, 84 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmlin/iter-test1.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/iter-test1.xml @@ -0,0 +1,6 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out'> + <tcp srcipaddr='$A' srcportstart='$B' dscp='2'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlin/iter-test2.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/iter-test2.xml @@ -0,0 +1,23 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out'> + <tcp srcipaddr='$A' srcportstart='$B[@0]' dscp='1'/> + </rule> + <rule action='accept' direction='out'> + <udp srcipaddr='$A[@1]' srcportstart='$B[@2]' dscp='2'/> + </rule> + <rule action='accept' direction='out'> + <sctp srcipaddr='$A[@1]' srcportstart='$B[@2]' dstportstart='$C[@2]' + dscp='3'/> + </rule> + <rule action='accept' direction='out'> + <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]' dstportstart='$C[@3]' + dscp='4'/> + </rule> + <rule action='accept' direction='out'> + <udp srcipaddr='$A[@1]' dstipaddr='$A[@2]' dscp='5'/> + </rule> + <rule action='accept' direction='out'> + <sctp srcipaddr='$A' dstipaddr='$A' dscp='6'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlin/iter-test3.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/iter-test3.xml @@ -0,0 +1,13 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out'> + <tcp srcipaddr='$A[ 0]' srcportstart='$B[ @0 ] ' dscp='1'/> + </rule> + <rule action='accept' direction='out'> + <udp srcipaddr='$A[1 ]' srcportstart='$B[ @2 ]' dscp='2'/> + </rule> + <rule action='accept' direction='out'> + <sctp srcipaddr='$A[ 1 ] ' srcportstart='$B[2 ] ' dstportstart='$C[ 2 ]' + dscp='3'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/iter-test1.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/iter-test1.xml @@ -0,0 +1,6 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out' priority='500'> + <tcp srcipaddr='$A' dscp='2' srcportstart='$B'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/iter-test2.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/iter-test2.xml @@ -0,0 +1,21 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out' priority='500'> + <tcp srcipaddr='$A' dscp='1' srcportstart='$B'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp srcipaddr='$A[@1]' dscp='2' srcportstart='$B[@2]'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <sctp srcipaddr='$A[@1]' dscp='3' srcportstart='$B[@2]' dstportstart='$C[@2]'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <tcp srcipaddr='$A[@1]' dscp='4' srcportstart='$B[@2]' dstportstart='$C[@3]'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp srcipaddr='$A[@1]' dstipaddr='$A[@2]' dscp='5'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <sctp srcipaddr='$A' dstipaddr='$A' dscp='6'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/iter-test3.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/iter-test3.xml @@ -0,0 +1,12 @@ +<filter name='testcase' chain='root'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='accept' direction='out' priority='500'> + <tcp srcipaddr='$A[0]' dscp='1' srcportstart='$B'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp srcipaddr='$A[1]' dscp='2' srcportstart='$B[@2]'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <sctp srcipaddr='$A[1]' dscp='3' srcportstart='$B[2]' dstportstart='$C[2]'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmltest.c =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -153,6 +153,9 @@ mymain(void) DO_TEST("chain_prefixtest1", true); /* derived from arp-test */ DO_TEST("attr-value-test", false); + DO_TEST("iter-test1", false); + DO_TEST("iter-test2", false); + DO_TEST("iter-test3", false); return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); }
On 12/09/2011 08:07 AM, Stefan Berger wrote:
This patch adds a couple of XML parser / schema validator test cases for the new 'ways' to access variables via index or iterator.
--- tests/nwfilterxml2xmlin/iter-test1.xml | 6 ++++++ tests/nwfilterxml2xmlin/iter-test2.xml | 23 +++++++++++++++++++++++ tests/nwfilterxml2xmlin/iter-test3.xml | 13 +++++++++++++ tests/nwfilterxml2xmlout/iter-test1.xml | 6 ++++++ tests/nwfilterxml2xmlout/iter-test2.xml | 21 +++++++++++++++++++++ tests/nwfilterxml2xmlout/iter-test3.xml | 12 ++++++++++++ tests/nwfilterxml2xmltest.c | 3 +++ 7 files changed, 84 insertions(+)
Ah, there's the test cases I was looking for. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This patch adds documentation about the new 'ways' that users can access the contents of variables in filters: - access via index: $TEST[2] - access via iterators $TEST[@1] --- docs/formatnwfilter.html.in | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) Index: libvirt-iterator/docs/formatnwfilter.html.in =================================================================== --- libvirt-iterator.orig/docs/formatnwfilter.html.in +++ libvirt-iterator/docs/formatnwfilter.html.in @@ -308,7 +308,37 @@ </rule> ... </pre> - + <p> + <span class="since">Since 0.9.9</span> it is possible to access + individual elements of a variable holding a list of elements. + A filtering rule like the following accesses the 2nd element + of the variable DSTPORTS. + </p> +<pre> + ... + <rule action='accept' direction='in' priority='500'> + <udp dstportstart='$DSTPORTS[1]'/> + </rule> + ... +</pre> + <p> + <span class="since">Since 0.9.9</span> it is possible to create + filtering rules that instantiate all combinations of rules from + different lists using the notation of + <code>$VARIABLE[@<iterator ID>]</code>. + The following rule allows a virtual machine to + receive traffic on a set of ports, which are specified in DSTPORTS, + from the set of source IP address specified in SRCIPADDRESSES. + The rule generates all combinations of elements of the variable + DSTPORT with those of SRCIPADDRESSES. + </p> +<pre> + ... + <rule action='accept' direction='in' priority='500'> + <ip srcipaddr='$SRCIPADDRESSES[@1]' dstportstart='$DSTPORTS[@2]'/> + </rule> + ... +</pre> <h2><a name="nwfelems">Element and attribute overview</a></h2> <p>
On 12/09/2011 08:07 AM, Stefan Berger wrote:
This patch adds documentation about the new 'ways' that users can access the contents of variables in filters:
- access via index: $TEST[2] - access via iterators $TEST[@1]
--- docs/formatnwfilter.html.in | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)
Index: libvirt-iterator/docs/formatnwfilter.html.in =================================================================== --- libvirt-iterator.orig/docs/formatnwfilter.html.in +++ libvirt-iterator/docs/formatnwfilter.html.in @@ -308,7 +308,37 @@ </rule> ... </pre> - + <p> + <span class="since">Since 0.9.9</span> it is possible to access
We missed the release; s/0.9.9/0.9.10/
+ individual elements of a variable holding a list of elements. + A filtering rule like the following accesses the 2nd element + of the variable DSTPORTS. + </p> +<pre> + ... + <rule action='accept' direction='in' priority='500'> + <udp dstportstart='$DSTPORTS[1]'/> + </rule> + ... +</pre> + <p> + <span class="since">Since 0.9.9</span> it is possible to create
Likewise.
+ filtering rules that instantiate all combinations of rules from + different lists using the notation of + <code>$VARIABLE[@<iterator ID>]</code>. + The following rule allows a virtual machine to + receive traffic on a set of ports, which are specified in DSTPORTS, + from the set of source IP address specified in SRCIPADDRESSES. + The rule generates all combinations of elements of the variable + DSTPORT with those of SRCIPADDRESSES.
I would also mention: $VARIABLE is short-hand for $VARIABLE[@0]. When combining variables, each variable that uses the same iterator ID will be directly combined; it is the use of different iterator IDs that causes all combinations to be generated. And maybe even mention an example: If variable A is the list [1, 2], and variable B is the list [3, 4], then combining $A[@0] and $B[@0] produces the list [1/3, 2/4], while combining $A[@0] and $B[@1] produces the list [1/3, 1/4, 2/3, 2/4]. Overall, I like what is in this series, but I think I had enough minor findings that it would be worth posting a v2 before pushing anything. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/10/2012 01:04 PM, Eric Blake wrote:
On 12/09/2011 08:07 AM, Stefan Berger wrote:
- +<p> +<span class="since">Since 0.9.9</span> it is possible to access We missed the release; s/0.9.9/0.9.10/
Fixed.
+ filtering rules that instantiate all combinations of rules from + different lists using the notation of +<code>$VARIABLE[@<iterator ID>]</code>. + The following rule allows a virtual machine to + receive traffic on a set of ports, which are specified in DSTPORTS, + from the set of source IP address specified in SRCIPADDRESSES. + The rule generates all combinations of elements of the variable + DSTPORT with those of SRCIPADDRESSES. I would also mention:
$VARIABLE is short-hand for $VARIABLE[@0]. When combining variables, each variable that uses the same iterator ID will be directly combined; it is the use of different iterator IDs that causes all combinations to be generated.
And maybe even mention an example:
If variable A is the list [1, 2], and variable B is the list [3, 4], then combining $A[@0] and $B[@0] produces the list [1/3, 2/4], while combining $A[@0] and $B[@1] produces the list [1/3, 1/4, 2/3, 2/4].
Overall, I like what is in this series, but I think I had enough minor findings that it would be worth posting a v2 before pushing anything.
I added a concrete example now and mention the $VARIABLE being equivalent to $VARIABLE[@0]. Thanks. Stefan
On 12/09/2011 10:07 AM, Stefan Berger wrote:
This patch enables access to variables in filters using indep. iterators ($TEST[$@2]) or via index ($TEST[1]).
Three test cases are added that are also being used for libvirt-TCK to check that the instantiation of the filtering rules is correct.
Does someone have time to review this set? Stefan
participants (2)
-
Eric Blake -
Stefan Berger