2:11 p.m.
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. The latter is now added in this patch series
as well.
I am first 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. All assigned priorities have negative values.
Later on the priorities for the chains are made accessible via an XML
attribute.
Further, filters will be automatically accessed from the (ebtables)
interface 'root' chain using the prefix of the name of the chain. As an
example, the following filter will be accessed from the root chain for 'arp'
packets since its name 'arp-xyz' has the prefix 'arp'.
<filter name='test-arp-xyz' chain='arp-xyz'
priority='-650'>
<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 another step the priorities of rules is extended to also allow negativ
values. This then allows the creation of rules and chains in the interface
'root' chain to be mixed so that the following layout becomes possible:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
In the above list of rules the '-p ARP -j ACCEPT' can now be found in
between the 'jumps' to protocol-specific chains, which allows for more
efficient rule evaluation.
I did testing with the test cases in libvirt-tck as well as those in
the tests/ directory and did not see any regressions.
v5:
- addressed Eric Blake's comments
v4:
- assign priority to chain according to name prefix
- change default (internal) priorities of chains (by +200)
- fix memory leak
Regards,
Stefan
2:11 p.m.
New subject: [libvirt] [PATCH V5 01/10] Add function to get hash tables key/value pairs
Add a function to the virHashTable for getting an array of the hash table's
key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- follwed Eric Blake's suggestions:
- added better description to new function
- return array of key-value pairs rather than only keys
---
src/libvirt_private.syms | 2 ++
src/util/hash.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
src/util/hash.h | 21 +++++++++++++++++++++
3 files changed, 68 insertions(+)
Index: libvirt-acl/src/util/hash.c
===================================================================
--- libvirt-acl.orig/src/util/hash.c
+++ libvirt-acl/src/util/hash.c
@@ -618,3 +618,48 @@ void *virHashSearch(virHashTablePtr tabl
return NULL;
}
+
+struct getKeysIter
+{
+ virHashKeyValuePair *sortArray;
+ unsigned arrayIdx;
+};
+
+static void virHashGetKeysIterator(void *payload,
+ const void *key, void *data)
+{
+ struct getKeysIter *iter = data;
+
+ iter->sortArray[iter->arrayIdx].key = key;
+ iter->sortArray[iter->arrayIdx].value = payload;
+
+ iter->arrayIdx++;
+}
+
+typedef int (*qsort_comp)(const void *, const void *);
+
+virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
+ virHashKeyComparator compar)
+{
+ int numElems = virHashSize(table);
+ struct getKeysIter iter = {
+ .arrayIdx = 0,
+ .sortArray = NULL,
+ };
+
+ if (numElems < 0)
+ return NULL;
+
+ if (VIR_ALLOC_N(iter.sortArray, numElems + 1)) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ virHashForEach(table, virHashGetKeysIterator, &iter);
+
+ if (compar)
+ qsort(&iter.sortArray[0], numElems, sizeof(iter.sortArray[0]),
+ (qsort_comp)compar);
+
+ return iter.sortArray;
+}
Index: libvirt-acl/src/util/hash.h
===================================================================
--- libvirt-acl.orig/src/util/hash.h
+++ libvirt-acl/src/util/hash.h
@@ -130,6 +130,27 @@ void *virHashLookup(virHashTablePtr tabl
*/
void *virHashSteal(virHashTablePtr table, const void *name);
+/*
+ * Get the hash table's key/value pairs and have them optionally sorted.
+ * The returned array contains virHashSize() elements. Aditionally,
+ * an empty element has been added to the end of the array whose key == NULL
+ * also indicates the end of the array.
+ * The key/value pairs are only valid as long as the underlying hash
+ * table is not modified, i.e., keys removed or the hash table deleted.
+ * The caller must only free the returned array using VIR_FREE().
+ * The caller must make copies of all returned keys and values if they are
+ * to be used somewhere else.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+ const void *key;
+ const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+ const virHashKeyValuePairPtr );
+virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
+ virHashKeyComparator compar);
/*
* Iterators
Index: libvirt-acl/src/libvirt_private.syms
===================================================================
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -559,6 +559,7 @@ virHashAddEntry;
virHashCreate;
virHashForEach;
virHashFree;
+virHashGetItems;
virHashLookup;
virHashRemoveEntry;
virHashRemoveSet;
@@ -566,6 +567,7 @@ virHashSearch;
virHashSize;
virHashSteal;
virHashTableSize;
+virHashUpdateEntry;
# hooks.h
3:27 p.m.
New subject: [libvirt] [PATCH V5 01/10] Add function to get hash tables key/value pairs
On 11/17/2011 01:11 PM, Stefan Berger wrote:
Add a function to the virHashTable for getting an array of the hash
table's
key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- follwed Eric Blake's suggestions:
- added better description to new function
- return array of key-value pairs rather than only keys
Yep, looks like it will be more reusable now. And the more I think
about it, the more I've convinced myself this is a useful interface
(allows you to sort the hashtable in multiple ways, based on what
comparator you pass in; using a data structure to store sorted elements
with hashtable lookup speeds still only gives you a single sort order).
---
src/libvirt_private.syms | 2 ++
src/util/hash.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
src/util/hash.h | 21 +++++++++++++++++++++
I'd still feel a bit better if we had coverage for the new function in
tests/hashtest.c, but I'll let that slide to another patch; this patch
already blocks enough other useful stuff that it will get some good
field testing from the tck runs you do, while we work on enhancing
hashtest.c.
}
+
+struct getKeysIter
+{
+ virHashKeyValuePair *sortArray;
+ unsigned arrayIdx;
+};
Lots simpler, huh? :)
+/*
+ * Get the hash table's key/value pairs and have them optionally sorted.
+ * The returned array contains virHashSize() elements. Aditionally,
s/Aditionally/Additionally/
+ * an empty element has been added to the end of the array whose key
== NULL
+ * also indicates the end of the array.
We don't need both "Additionally" and "also" in the same sentence.
How
about:
s/also indicates/to indicate/
+ * The key/value pairs are only valid as long as the underlying
hash
+ * table is not modified, i.e., keys removed or the hash table deleted.
s/keys removed or the hash table deleted/no keys are removed or
inserted, and the hash table is not deleted/
+ * The caller must only free the returned array using VIR_FREE().
+ * The caller must make copies of all returned keys and values if they are
+ * to be used somewhere else.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+ const void *key;
+ const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+ const virHashKeyValuePairPtr );
The spacing looks awkward on these two lines;
s/ \([,)]\)/\1/
ACK. No need to see a v6 on this one, as the fixes are trivial.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:07 p.m.
New subject: [libvirt] [PATCH V5 01.5/10] tests: test recent hash addition
Excercise the new hash API, to ensure we avoid regressions.
* tests/hashtest.c (testHashGetItems): New test.
---
I'd still feel a bit better if we had coverage for the new
function in
tests/hashtest.c, but I'll let that slide to another patch;
On second thought, writing a test now wasn't too hard.
tests/hashtest.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/tests/hashtest.c b/tests/hashtest.c
index f02b3a9..898a95d 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -9,6 +9,7 @@
#include "hash.h"
#include "hashdata.h"
#include "testutils.h"
+#include "memory.h"
#define testError(...) \
@@ -491,6 +492,90 @@ cleanup:
static int
+testHashGetItemsCompKey(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ return strcmp (a->key, b->key);
+}
+
+static int
+testHashGetItemsCompValue(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ return strcmp (a->value, b->value);
+}
+
+static int
+testHashGetItems(const void *data ATTRIBUTE_UNUSED)
+{
+ virHashTablePtr hash;
+ virHashKeyValuePairPtr array = NULL;
+ int ret = -1;
+ char keya[] = "a";
+ char keyb[] = "b";
+ char keyc[] = "c";
+ char value1[] = "1";
+ char value2[] = "2";
+ char value3[] = "3";
+
+ if (!(hash = virHashCreate(0, NULL)) ||
+ virHashAddEntry(hash, keya, value3) < 0 ||
+ virHashAddEntry(hash, keyc, value1) < 0 ||
+ virHashAddEntry(hash, keyb, value2) < 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to create hash");
+ }
+ goto cleanup;
+ }
+
+ if (!(array = virHashGetItems(hash, NULL)) ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with NULL sort");
+ }
+ goto cleanup;
+ }
+ VIR_FREE(array);
+
+ if (!(array = virHashGetItems(hash, testHashGetItemsCompKey)) ||
+ STRNEQ(array[0].key, "a") ||
+ STRNEQ(array[0].value, "3") ||
+ STRNEQ(array[1].key, "b") ||
+ STRNEQ(array[1].value, "2") ||
+ STRNEQ(array[2].key, "c") ||
+ STRNEQ(array[2].value, "1") ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with key sort");
+ }
+ goto cleanup;
+ }
+ VIR_FREE(array);
+
+ if (!(array = virHashGetItems(hash, testHashGetItemsCompValue)) ||
+ STRNEQ(array[0].key, "c") ||
+ STRNEQ(array[0].value, "1") ||
+ STRNEQ(array[1].key, "b") ||
+ STRNEQ(array[1].value, "2") ||
+ STRNEQ(array[2].key, "a") ||
+ STRNEQ(array[2].value, "3") ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with value sort");
+ }
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(array);
+ virHashFree(hash);
+ return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -526,6 +611,7 @@ mymain(void)
DO_TEST("Forbidden ops in ForEach", ForEach);
DO_TEST("RemoveSet", RemoveSet);
DO_TEST("Search", Search);
+ DO_TEST("GetItems", GetItems);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.7.7.1
9:17 a.m.
New subject: [libvirt] [PATCH V5 01.5/10] tests: test recent hash addition
On 11/17/2011 05:07 PM, Eric Blake wrote:
Excercise the new hash API, to ensure we avoid regressions.
* tests/hashtest.c (testHashGetItems): New test.
---
> I'd still feel a bit better if we had coverage for the new function in
> tests/hashtest.c, but I'll let that slide to another patch;
On second thought, writing a test now wasn't too hard.
tests/hashtest.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/tests/hashtest.c b/tests/hashtest.c
index f02b3a9..898a95d 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -9,6 +9,7 @@
#include "hash.h"
#include "hashdata.h"
#include "testutils.h"
+#include "memory.h"
#define testError(...) \
@@ -491,6 +492,90 @@ cleanup:
static int
+testHashGetItemsCompKey(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ return strcmp (a->key, b->key);
+}
+
+static int
+testHashGetItemsCompValue(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ return strcmp (a->value, b->value);
+}
+
+static int
+testHashGetItems(const void *data ATTRIBUTE_UNUSED)
+{
+ virHashTablePtr hash;
+ virHashKeyValuePairPtr array = NULL;
+ int ret = -1;
+ char keya[] = "a";
+ char keyb[] = "b";
+ char keyc[] = "c";
+ char value1[] = "1";
+ char value2[] = "2";
+ char value3[] = "3";
+
+ if (!(hash = virHashCreate(0, NULL)) ||
+ virHashAddEntry(hash, keya, value3)< 0 ||
+ virHashAddEntry(hash, keyc, value1)< 0 ||
+ virHashAddEntry(hash, keyb, value2)< 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to create hash");
+ }
+ goto cleanup;
+ }
+
+ if (!(array = virHashGetItems(hash, NULL)) ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with NULL sort");
+ }
+ goto cleanup;
+ }
+ VIR_FREE(array);
+
+ if (!(array = virHashGetItems(hash, testHashGetItemsCompKey)) ||
+ STRNEQ(array[0].key, "a") ||
+ STRNEQ(array[0].value, "3") ||
+ STRNEQ(array[1].key, "b") ||
+ STRNEQ(array[1].value, "2") ||
+ STRNEQ(array[2].key, "c") ||
+ STRNEQ(array[2].value, "1") ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with key sort");
+ }
+ goto cleanup;
+ }
+ VIR_FREE(array);
+
+ if (!(array = virHashGetItems(hash, testHashGetItemsCompValue)) ||
+ STRNEQ(array[0].key, "c") ||
+ STRNEQ(array[0].value, "1") ||
+ STRNEQ(array[1].key, "b") ||
+ STRNEQ(array[1].value, "2") ||
+ STRNEQ(array[2].key, "a") ||
+ STRNEQ(array[2].value, "3") ||
+ array[3].key || array[3].value) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to get items with value sort");
+ }
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(array);
+ virHashFree(hash);
+ return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -526,6 +611,7 @@ mymain(void)
DO_TEST("Forbidden ops in ForEach", ForEach);
DO_TEST("RemoveSet", RemoveSet);
DO_TEST("Search", Search);
+ DO_TEST("GetItems", GetItems);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
ACK
11:42 a.m.
New subject: [libvirt] [PATCH V5 01.5/10] tests: test recent hash addition
On 11/18/2011 08:17 AM, Stefan Berger wrote:
On 11/17/2011 05:07 PM, Eric Blake wrote:
> Excercise the new hash API, to ensure we avoid regressions.
>
> * tests/hashtest.c (testHashGetItems): New test.
> ---
ACK
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 02/10] Introduce an internal priority for chains
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. However, the values are
chosen as negative so that once they are sorted along with filtering rules
(whose priority may only be positive for now) they will always be instantiated
before them (lower values cause instantiation before higher values). This
is done to maintain backwards compatibility.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v3:
- increased filter priorities to have more room before them
---
src/conf/nwfilter_conf.c | 14 ++++++++++++++
src/conf/nwfilter_conf.h | 12 ++++++++++++
src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++++
src/nwfilter/nwfilter_ebiptables_driver.h | 1 +
4 files changed, 31 insertions(+)
Index: libvirt-acl/src/conf/nwfilter_conf.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -357,8 +357,18 @@ enum virNWFilterEbtablesTableType {
};
+# define MIN_RULE_PRIORITY 0
# define MAX_RULE_PRIORITY 1000
+# define NWFILTER_MIN_FILTER_PRIORITY -1000
+# define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY
+
+# define NWFILTER_ROOT_FILTER_PRI 0
+# define NWFILTER_IPV4_FILTER_PRI -700
+# define NWFILTER_IPV6_FILTER_PRI -600
+# define NWFILTER_ARP_FILTER_PRI -500
+# define NWFILTER_RARP_FILTER_PRI -400
+
enum virNWFilterRuleFlags {
RULE_FLAG_NO_STATEMATCH = (1 << 0),
RULE_FLAG_STATE_NEW = (1 << 1),
@@ -436,6 +446,7 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
+typedef int32_t virNWFilterChainPriority;
typedef struct _virNWFilterDef virNWFilterDef;
typedef virNWFilterDef *virNWFilterDefPtr;
@@ -445,6 +456,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
@@ -124,6 +124,14 @@ struct int_map {
#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
#define INTMAP_ENTRY_LAST { .val = NULL }
+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
@@ -2028,6 +2036,12 @@ 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 = (NWFILTER_MAX_FILTER_PRIORITY +
+ NWFILTER_MIN_FILTER_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
@@ -328,6 +328,7 @@ static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
enum virNWFilterChainSuffixType neededChain,
+ virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
enum RuleType ruleType)
@@ -341,6 +342,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
inst->commandTemplate = commandTemplate;
inst->neededProtocolChain = neededChain;
+ inst->chainPriority = chainPriority;
inst->chainprefix = chainprefix;
inst->priority = priority;
inst->ruleType = ruleType;
@@ -1589,6 +1591,7 @@ _iptablesCreateRuleInstance(int directio
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(final),
nwfilter->chainsuffix,
+ nwfilter->chainPriority,
'\0',
rule->priority,
(isIPv6) ? RT_IP6TABLES : RT_IPTABLES);
@@ -2338,6 +2341,7 @@ ebtablesCreateRuleInstance(char chainPre
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(&buf),
nwfilter->chainsuffix,
+ nwfilter->chainPriority,
chainPrefix,
rule->priority,
RT_EBTABLES);
4:25 p.m.
New subject: [libvirt] [PATCH V5 02/10] Introduce an internal priority for chains
On 11/17/2011 01:11 PM, Stefan Berger wrote:
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. However, the values are
chosen as negative so that once they are sorted along with filtering rules
(whose priority may only be positive for now) they will always be instantiated
before them (lower values cause instantiation before higher values). This
is done to maintain backwards compatibility.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v3:
- increased filter priorities to have more room before them
---
src/conf/nwfilter_conf.c | 14 ++++++++++++++
src/conf/nwfilter_conf.h | 12 ++++++++++++
src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++++
src/nwfilter/nwfilter_ebiptables_driver.h | 1 +
4 files changed, 31 insertions(+)
ACK.
@@ -2028,6 +2036,12 @@ 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) {
I tend not to compare to true or false, but it's not wrong so you don't
have to change it. My argument is that comparison to bool is redundant,
since the other side of the comparison is already in bool context; I
would have used:
if (!intMapGetByString(chain_priorities, chain, 0,
&ret->chainPriority)) {
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 03/10] Make filter creation in root table more flexible
Use the previously introduced chain priorities to sort the chains for access
from an interface's '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 determine 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 following the prefix 'arp' in its name.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- followed Eric Blake's suggestions:
- return -1 as error code
- add comments as appropriate
- other style fixes
---
src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++++++++++++++++++++++--------
1 file changed, 102 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
@@ -140,6 +140,11 @@ enum l3_proto_idx {
#define USHORTMAP_ENTRY_IDX(IDX, ATT, VAL) [IDX] = { .attr = ATT, .val = VAL }
+/* A lookup table for translating ethernet protocol IDs to human readable
+ * strings. None of the human readable strings must be found as a prefix
+ * in another entry here (example 'ab' would be found in 'abc') to allow
+ * for prefix matching.
+ */
static const struct ushort_map l3_protocols[] = {
USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"),
USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"),
@@ -2662,6 +2667,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];
@@ -2669,7 +2675,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
@@ -3176,6 +3183,14 @@ ebiptablesRuleOrderSort(const void *a, c
return ((*insta)->priority - (*instb)->priority);
}
+static int
+ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ /* elements' values has been limited to range [-1000, 1000] */
+ return *(virNWFilterChainPriority *)a->value -
+ *(virNWFilterChainPriority *)b->value;
+}
static void
iptablesCheckBridgeNFCallEnabled(bool isIPv6)
@@ -3215,6 +3230,54 @@ 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;
+ virHashKeyValuePairPtr filter_names;
+
+ if (ebtablesCreateTmpRootChain(buf, direction, ifname, 1) < 0)
+ return -1;
+
+ filter_names = virHashGetItems(chains,
+ ebiptablesFilterOrderSort);
+ if (filter_names == NULL)
+ return -1;
+
+ for (i = 0; filter_names[i].key; i++) {
+ enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(
+ filter_names[i].key);
+ if ((int)idx < 0)
+ continue;
+ rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
+ filter_names[i].key, 1);
+ if (rc < 0)
+ break;
+ }
+
+ VIR_FREE(filter_names);
+ return rc;
+}
static int
ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3225,24 +3288,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);
+ virHashTablePtr 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);
@@ -3252,30 +3334,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;
@@ -3365,14 +3428,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:
@@ -3407,6 +3473,10 @@ tear_down_tmpebchains:
"interface %s."),
ifname);
+exit_free_sets:
+ virHashFree(chains_in_set);
+ virHashFree(chains_out_set);
+
return 1;
}
4:28 p.m.
New subject: [libvirt] [PATCH V5 03/10] Make filter creation in root table more flexible
On 11/17/2011 01:11 PM, Stefan Berger wrote:
Use the previously introduced chain priorities to sort the chains for
access
from an interface's '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 determine 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 following the prefix 'arp' in its name.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- followed Eric Blake's suggestions:
- return -1 as error code
- add comments as appropriate
- other style fixes
---
src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++++++++++++++++++++++--------
1 file changed, 102 insertions(+), 32 deletions(-)
Looks like you covered my v4 findings.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 04/10] Use scripting for cleaning and renaming of chains
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
chains that are accessed from this chain and its sub-chains using 'jumps'.
The 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.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- followng Eric Blake's suggestion on a major overhaul of the embedded
scripts
v2:
- setting IFS to intended value; works with bash and dash (phew!)
---
src/nwfilter/nwfilter_ebiptables_driver.c | 217 +++++++++++++++++-------------
1 file changed, 129 insertions(+), 88 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
@@ -92,6 +92,59 @@ static char *gawk_cmd_path;
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix)
+/* The collect_chains() script recursively determines all names
+ * of ebtables (nat) chains that are 'children' of a given 'root' chain.
+ * The typical out of an ebtables call is as follows:
+ *
+ * #> ebtables -t nat -L libvirt-I-tck-test205002
+ * Bridge table: nat
+ *
+ * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT
+ * -p IPv4 -j I-tck-test205002-ipv4
+ * -p ARP -j I-tck-test205002-arp
+ * -p 0x8035 -j I-tck-test205002-rarp
+ * -p 0x835 -j ACCEPT
+ * -j DROP
+ */
+static const char ebtables_script_func_collect_chains[] =
+ "collect_chains()\n"
+ "{\n"
+ " for tmp2 in $*; do\n"
+ " for tmp in $(%s -t %s -L $tmp2 | \\\n"
+ " sed -n \"/Bridge chain/,\\$ s/.*-j
\\\\([%s]-.*\\\\)/\\\\1/p\");\n"
+ " do\n"
+ " echo $tmp\n"
+ " collect_chains $tmp\n"
+ " done\n"
+ " done\n"
+ "}\n";
+
+static const char ebiptables_script_func_rm_chains[] =
+ "rm_chains()\n"
+ "{\n"
+ " for tmp in $*; do %s -t %s -F $tmp; done\n"
+ " for tmp in $*; do %s -t %s -X $tmp; done\n"
+ "}\n";
+
+static const char ebiptables_script_func_rename_chains[] =
+ "rename_chains()\n"
+ "{\n"
+ " for tmp in $*; do\n"
+ " case $tmp in\n"
+ " %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n"
+ " %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n"
+ " esac\n"
+ " done\n"
+ "}\n";
+
+static const char ebiptables_script_set_ifs[] =
+ "tmp='\n'\n"
+ "IFS=' ''\t'$tmp\n";
+
+#define NWFILTER_FUNC_COLLECT_CHAINS ebtables_script_func_collect_chains
+#define NWFILTER_FUNC_RM_CHAINS ebiptables_script_func_rm_chains
+#define NWFILTER_FUNC_RENAME_CHAINS ebiptables_script_func_rename_chains
+#define NWFILTER_FUNC_SET_IFS ebiptables_script_set_ifs
#define VIRT_IN_CHAIN "libvirt-in"
#define VIRT_OUT_CHAIN "libvirt-out"
@@ -2698,95 +2751,66 @@ ebtablesCreateTmpSubChain(virBufferPtr b
return 0;
}
-
static int
-_ebtablesRemoveSubChain(virBufferPtr buf,
- int incoming,
- const char *ifname,
- enum l3_proto_idx protoidx,
- int isTempChain)
-{
- 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;
- }
-
- 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,
+_ebtablesRemoveSubChains(virBufferPtr buf,
+ const char *ifname,
+ const char *chains)
+{
+ char rootchain[MAX_CHAINNAME_LENGTH];
+ unsigned i;
+
+ virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
+ virBufferAsprintf(buf, NWFILTER_FUNC_RM_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,
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS);
+ virBufferAddLit(buf, "a=\"$(collect_chains ");
+ for (i = 0; chains[i] != 0; i++) {
+ PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+ virBufferAsprintf(buf, "%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,
@@ -2813,31 +2837,51 @@ 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);
+}
+
+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, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
+ virBufferAsprintf(buf, NWFILTER_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);
- 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);
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS);
+ virBufferAddLit(buf, "a=\"$(collect_chains ");
+ for (i = 0; chains[i] != 0; i++) {
+ PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+ virBufferAsprintf(buf, "%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,
@@ -3546,9 +3590,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);
}
@@ -3633,12 +3675,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;
4:49 p.m.
New subject: [libvirt] [PATCH V5 04/10] Use scripting for cleaning and renaming of chains
On 11/17/2011 01:11 PM, Stefan Berger wrote:
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
chains that are accessed from this chain and its sub-chains using 'jumps'.
The 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.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
v5:
- followng Eric Blake's suggestion on a major overhaul of the embedded
scripts
Thanks for bearing with me, and the results look a lot nicer! I guess
it pays to have one of the autoconf maintainers as a reviewer, since
that means you have feedback from someone who knows shell inside and out ;)
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -92,6 +92,59 @@ static char *gawk_cmd_path;
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix)
+/* The collect_chains() script recursively determines all names
+ * of ebtables (nat) chains that are 'children' of a given 'root'
chain.
+ * The typical out of an ebtables call is as follows:
s/out/output/
+ *
+ * #> ebtables -t nat -L libvirt-I-tck-test205002
+ * Bridge table: nat
+ *
+ * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT
+ * -p IPv4 -j I-tck-test205002-ipv4
+ * -p ARP -j I-tck-test205002-arp
+ * -p 0x8035 -j I-tck-test205002-rarp
+ * -p 0x835 -j ACCEPT
+ * -j DROP
+ */
The comments go a long way to help. Thanks.
+static const char ebtables_script_func_collect_chains[] =
+ "collect_chains()\n"
Food for thought, but not strictly necessary, and certainly not worth
delaying this patch any further:
One of the interesting things learned in autoconf is that you want two
levels of comments: those that explain how the generator works (and the
generated file does not have them), and those that explain how the
generated code works (those are useful both in the generator and for
anyone reading the generated output). Right now, you are only using
level one comments (nwfilter_ebiptables_driver.c has comments about what
it is generating, but the generated script has no comments at all);
depending on how complex the generated script is getting (and it _is_
getting more complex as we add more stuff), it may be worth starting to
add level two comments so that someone reading the generated script can
understand it without referring back to the C code. For example, this
would mean doing:
+static const char ebtables_script_func_collect_chains[] =
+ "# collect_chains CHAIN...\n"
+ "# for each CHAIN, recursively output the list of sub-chains\n"
+ "# that it references through jump rules\n"
+ "collect_chains()\n"
+
+static const char ebiptables_script_set_ifs[] =
+ "tmp='\n'\n"
+ "IFS=' ''\t'$tmp\n";
Much nicer ;)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 05/10] Use the actual names of chains in data structure
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').
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
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
@@ -309,6 +309,7 @@ virNWFilterDefFree(virNWFilterDefPtr def
virNWFilterEntryFree(def->filterEntries[i]);
VIR_FREE(def->filterEntries);
+ VIR_FREE(def->chainsuffix);
VIR_FREE(def);
}
@@ -2027,21 +2028,28 @@ 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 = (NWFILTER_MAX_FILTER_PRIORITY +
NWFILTER_MIN_FILTER_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);
@@ -2843,7 +2851,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
@@ -455,7 +455,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
@@ -385,7 +385,7 @@ ebiptablesRuleInstFree(ebiptablesRuleIns
static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
- enum virNWFilterChainSuffixType neededChain,
+ const char *neededChain,
virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
@@ -1961,11 +1961,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) {
@@ -2532,7 +2534,7 @@ ebiptablesDisplayRuleInstance(virConnect
ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst;
VIR_INFO("Command Template: '%s', Needed protocol: '%s'",
inst->commandTemplate,
- virNWFilterChainSuffixTypeToString(inst->neededProtocolChain));
+ inst->neededProtocolChain);
return 0;
}
@@ -3350,8 +3352,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;
4:59 p.m.
New subject: [libvirt] [PATCH V5 05/10] Use the actual names of chains in data structure
On 11/17/2011 01:11 PM, Stefan Berger wrote:
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').
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
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(-)
ACK; fairly mechanical.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 06/10] Extend the filter XML to support priorities of chains
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:
<filter name='allow-arpxyz' chain='arp-xyz' priority='200'>
[...]
</filter>
The permitted values for priorities are [-1000, 1000].
By setting the pririty of a chain the order in which it is accessed
from the interface root chain can be influenced.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 7 ++++++-
src/conf/nwfilter_conf.c | 42 +++++++++++++++++++++++++++++++++++++-----
2 files changed, 43 insertions(+), 6 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2014,7 +2014,9 @@ virNWFilterDefParseXML(xmlXPathContextPt
xmlNodePtr curr = ctxt->node;
char *uuid = NULL;
char *chain = NULL;
+ char *chain_pri_s = NULL;
virNWFilterEntryPtr entry;
+ int chain_priority;
if (VIR_ALLOC(ret) < 0) {
virReportOOMError();
@@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
+ chain_pri_s = virXPathString("string(./@priority)", ctxt);
+ if (chain_pri_s) {
+ if (sscanf(chain_pri_s, "%d", &chain_priority) != 1) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Could not parse chain priority
'%s'"),
+ chain_pri_s);
+ goto cleanup;
+ }
+ if (chain_priority < NWFILTER_MIN_FILTER_PRIORITY ||
+ chain_priority > NWFILTER_MAX_FILTER_PRIORITY) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Priority '%d' is outside valid "
+ "range of [%d,%d]"),
+ chain_priority,
+ NWFILTER_MIN_FILTER_PRIORITY,
+ NWFILTER_MAX_FILTER_PRIORITY);
+ goto cleanup;
+ }
+ }
+
chain = virXPathString("string(./@chain)", ctxt);
if (chain) {
if (virNWFilterChainSuffixTypeFromString(chain) < 0) {
@@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
ret->chainsuffix = chain;
- /* assign an implicit priority -- support XML attribute later */
- if (intMapGetByString(chain_priorities, chain, 0,
- &ret->chainPriority) == false) {
- ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
- NWFILTER_MIN_FILTER_PRIORITY) / 2;
+
+ if (chain_pri_s) {
+ ret->chainPriority = chain_priority;
+ } else {
+ /* assign an implicit priority -- support XML attribute later */
+ if (intMapGetByString(chain_priorities, chain, 0,
+ &ret->chainPriority) == false) {
+ ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
+ NWFILTER_MIN_FILTER_PRIORITY) / 2;
+ }
}
chain = NULL;
} else {
@@ -2097,6 +2124,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
}
VIR_FREE(chain);
+ VIR_FREE(chain_pri_s);
return ret;
@@ -2104,6 +2132,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
virNWFilterDefFree(ret);
VIR_FREE(chain);
VIR_FREE(uuid);
+ VIR_FREE(chain_pri_s);
return NULL;
}
@@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
virBufferAsprintf(&buf, "<filter name='%s'
chain='%s'",
def->name,
def->chainsuffix);
+ if (def->chainPriority != 0)
+ virBufferAsprintf(&buf, " priority='%d'",
+ def->chainPriority);
virBufferAddLit(&buf, ">\n");
virUUIDFormat(def->uuid, uuid);
Index: libvirt-acl/docs/schemas/nwfilter.rng
===================================================================
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -293,6 +293,11 @@
</choice>
</attribute>
</optional>
+ <optional>
+ <attribute name="priority">
+ <ref name='priority-type'/>
+ </attribute>
+ </optional>
</define>
<define name="filterref-node-attributes">
@@ -881,7 +886,7 @@
<define name='priority-type'>
<data type="int">
- <param name="minInclusive">0</param>
+ <param name="minInclusive">-1000</param>
<param name="maxInclusive">1000</param>
</data>
</define>
5:15 p.m.
New subject: [libvirt] [PATCH V5 06/10] Extend the filter XML to support priorities of chains
On 11/17/2011 01:11 PM, Stefan Berger wrote:
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:
<filter name='allow-arpxyz' chain='arp-xyz'
priority='200'>
[...]
</filter>
The permitted values for priorities are [-1000, 1000].
By setting the pririty of a chain the order in which it is accessed
from the interface root chain can be influenced.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 7 ++++++-
Missing documentation in docs/formatnwfilter.html.in. I'll live up to
my hard-line reputation on this one, and request a v6 with documentation
(for example, it's worth documenting whether priority 100 is accessed
before or after priority 200).
But as to the code...
@@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
+ chain_pri_s = virXPathString("string(./@priority)", ctxt);
+ if (chain_pri_s) {
+ if (sscanf(chain_pri_s, "%d", &chain_priority) != 1) {
Let's use virStrToLong_i() instead of sscanf(); much nicer on the error
handling aspect.
@@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
ret->chainsuffix = chain;
- /* assign an implicit priority -- support XML attribute later */
- if (intMapGetByString(chain_priorities, chain, 0,
- &ret->chainPriority) == false) {
- ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
- NWFILTER_MIN_FILTER_PRIORITY) / 2;
+
+ if (chain_pri_s) {
+ ret->chainPriority = chain_priority;
+ } else {
+ /* assign an implicit priority -- support XML attribute later */
Is this comment still accurate, now that you have an XML attribute?
@@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
virBufferAsprintf(&buf, "<filter name='%s'
chain='%s'",
def->name,
def->chainsuffix);
+ if (def->chainPriority != 0)
+ virBufferAsprintf(&buf, " priority='%d'",
+ def->chainPriority);
That means an explicit pririoty='0' by the user is eaten and does not
appear on the output. But that's not too bad, and as long as we
document that priority is 0 unless explicitly specified, we're covered
(hence my plea for documentation...)
Everything else looks okay.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6 a.m.
New subject: [libvirt] [PATCH V5 06/10] Extend the filter XML to support priorities of chains
On 11/17/2011 06:15 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
> This patch extends the filter XML to support priorities of chains
> in the XML. An example would be:
>
> <filter name='allow-arpxyz' chain='arp-xyz'
priority='200'>
> [...]
> </filter>
>
> The permitted values for priorities are [-1000, 1000].
> By setting the pririty of a chain the order in which it is accessed
> from the interface root chain can be influenced.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 7 ++++++-
Missing documentation in docs/formatnwfilter.html.in. I'll live up to
my hard-line reputation on this one, and request a v6 with documentation
(for example, it's worth documenting whether priority 100 is accessed
before or after priority 200).
I have such a patch much further down the queue. I'll pull the relevant
parts into v6.
But as to the code...
> @@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
> goto cleanup;
> }
>
> + chain_pri_s = virXPathString("string(./@priority)", ctxt);
> + if (chain_pri_s) {
> + if (sscanf(chain_pri_s, "%d",&chain_priority) != 1) {
Let's use virStrToLong_i() instead of sscanf(); much nicer on the error
handling aspect.
Done.
> @@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
> goto cleanup;
> }
> ret->chainsuffix = chain;
> - /* assign an implicit priority -- support XML attribute later */
> - if (intMapGetByString(chain_priorities, chain, 0,
> -&ret->chainPriority) == false) {
> - ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
> - NWFILTER_MIN_FILTER_PRIORITY) / 2;
> +
> + if (chain_pri_s) {
> + ret->chainPriority = chain_priority;
> + } else {
> + /* assign an implicit priority -- support XML attribute later */
Is this comment still accurate, now that you have an XML attribute?
Fixed.
> @@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
> virBufferAsprintf(&buf, "<filter name='%s'
chain='%s'",
> def->name,
> def->chainsuffix);
> + if (def->chainPriority != 0)
> + virBufferAsprintf(&buf, " priority='%d'",
> + def->chainPriority);
That means an explicit pririoty='0' by the user is eaten and does not
appear on the output. But that's not too bad, and as long as we
document that priority is 0 unless explicitly specified, we're covered
(hence my plea for documentation...)
Everything else looks okay.
2:11 p.m.
New subject: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
This patch enables chains that have a known prefix in their name.
Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All
prefixes
are also protocols that can be evaluated on the ebtables level.
Following the prefix they will be automatically connected to an interface's
'root' chain and jumped into following the protocol they evalute, i.e.,
a table 'arp-xyz' will be accessed from the root table using
ebtables -t nat -A <iface root table> -p arp -j I-<ifname>-arp-xyz
thus generating a 'root' chain like this one here:
Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
-p IPv4 -j O-vnet0-ipv4
-p ARP -j O-vnet0-arp
-p 0x8035 -j O-vnet0-rarp
-p ARP -j O-vnet0-arp-xyz
-j DROP
where the chain 'arp-xyz' is accessed for filtering of ARP packets.
v3:
- assign a priority to filters that have an allowed prefix, e.g., assign
the arp chain priority to a filter arp-xyz unless user provided a
priority in the XML
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 16 ++++++--
src/conf/nwfilter_conf.c | 87 ++++++++++++++++++++++++++++++++++++++++++----
src/conf/nwfilter_conf.h | 3 +
3 files changed, 96 insertions(+), 10 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2007,6 +2007,80 @@ err_exit:
goto cleanup;
}
+static bool
+virNWFilterIsValidChainName(const char *chainname)
+{
+ return chainname[strspn(chainname, VALID_CHAINNAME)] == 0;
+}
+
+/*
+ * Test whether the name of the chain is supported.
+ * It current has to have a prefix of either one of the strings found in
+ * virNWFilterChainSuffixTypeToString().
+ */
+static const char *
+virNWFilterIsAllowedChain(const char *chainname)
+{
+ enum virNWFilterChainSuffixType i;
+ const char *name, *msg;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ bool printed = false;
+
+ if (!virNWFilterIsValidChainName(chainname)) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Chain name contains illegal characters"));
+ return NULL;
+ }
+
+ if (strlen(chainname) > MAX_CHAIN_SUFFIX_SIZE) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Name of chain is longer than %u
characters"),
+ MAX_CHAIN_SUFFIX_SIZE);
+ return NULL;
+ }
+
+ for (i = 0; i < VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+ name = virNWFilterChainSuffixTypeToString(i);
+ if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT) {
+ /* allow 'root' as a complete name but not as a prefix */
+ if (STREQ(chainname, name))
+ return name;
+ if (STRPREFIX(chainname, name))
+ return NULL;
+ }
+ if (STRPREFIX(chainname, name))
+ return name;
+ }
+
+ virBufferAsprintf(&buf,
+ _("Illegal chain name '%s'. Please use a chain name
"
+ "called '%s' or either one of the following prefixes:
"),
+ virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT),
+ chainname);
+ for (i = 0; i < VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+ if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT)
+ continue;
+ if (printed)
+ virBufferAddLit(&buf, ", ");
+ virBufferAdd(&buf, virNWFilterChainSuffixTypeToString(i), -1);
+ printed = true;
+ }
+
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ virBufferFreeAndReset(&buf);
+ goto err_exit;
+ }
+
+ msg = virBufferContentAndReset(&buf);
+
+ virNWFilterReportError(VIR_ERR_INVALID_ARG, _("%s"), msg);
+ VIR_FREE(msg);
+
+err_exit:
+ return NULL;
+}
static virNWFilterDefPtr
virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
@@ -2017,6 +2091,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
char *chain_pri_s = NULL;
virNWFilterEntryPtr entry;
int chain_priority;
+ const char *name_prefix;
if (VIR_ALLOC(ret) < 0) {
virReportOOMError();
@@ -2052,19 +2127,19 @@ virNWFilterDefParseXML(xmlXPathContextPt
chain = virXPathString("string(./@chain)", ctxt);
if (chain) {
- if (virNWFilterChainSuffixTypeFromString(chain) < 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown chain suffix '%s'"),
chain);
+ name_prefix = virNWFilterIsAllowedChain(chain);
+ if (name_prefix == NULL)
goto cleanup;
- }
ret->chainsuffix = chain;
if (chain_pri_s) {
ret->chainPriority = chain_priority;
} else {
- /* assign an implicit priority -- support XML attribute later */
- if (intMapGetByString(chain_priorities, chain, 0,
+ /* assign an implicit priority */
+ if (!name_prefix ||
+ intMapGetByString(chain_priorities, name_prefix, 0,
&ret->chainPriority) == false) {
+ /* assign default chain priority */
ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
NWFILTER_MIN_FILTER_PRIORITY) / 2;
}
Index: libvirt-acl/src/conf/nwfilter_conf.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -446,6 +446,9 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
+# define VALID_CHAINNAME \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:-"
+
typedef int32_t virNWFilterChainPriority;
typedef struct _virNWFilterDef virNWFilterDef;
Index: libvirt-acl/docs/schemas/nwfilter.rng
===================================================================
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -286,10 +286,18 @@
<attribute name="chain">
<choice>
<value>root</value>
- <value>arp</value>
- <value>rarp</value>
- <value>ipv4</value>
- <value>ipv6</value>
+ <data type="string">
+ <param name="pattern">arp[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
+ <param name="pattern">rarp[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
+ <param name="pattern">ipv4[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
+ <param name="pattern">ipv6[a-zA-Z0-9_\.:-]*</param>
+ </data>
</choice>
</attribute>
</optional>
5:35 p.m.
New subject: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 01:11 PM, Stefan Berger wrote:
This patch enables chains that have a known prefix in their name.
Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All
prefixes
are also protocols that can be evaluated on the ebtables level.
Following the prefix they will be automatically connected to an interface's
'root' chain and jumped into following the protocol they evalute, i.e.,
s/evalute/evaluate/
a table 'arp-xyz' will be accessed from the root table using
ebtables -t nat -A <iface root table> -p arp -j I-<ifname>-arp-xyz
thus generating a 'root' chain like this one here:
Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
-p IPv4 -j O-vnet0-ipv4
-p ARP -j O-vnet0-arp
-p 0x8035 -j O-vnet0-rarp
-p ARP -j O-vnet0-arp-xyz
-j DROP
where the chain 'arp-xyz' is accessed for filtering of ARP packets.
v3:
- assign a priority to filters that have an allowed prefix, e.g., assign
the arp chain priority to a filter arp-xyz unless user provided a
priority in the XML
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 16 ++++++--
src/conf/nwfilter_conf.c | 87 ++++++++++++++++++++++++++++++++++++++++++----
src/conf/nwfilter_conf.h | 3 +
3 files changed, 96 insertions(+), 10 deletions(-)
Another patch without docs. (It's okay if the docs comes as a separate
patch, and if a single doc patch covers the XML additions in both 6 and
7; my main concern is only that the docs get patched by the time the
series is completed, and I didn't see it when glancing ahead to 10/10).
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2007,6 +2007,80 @@ err_exit:
goto cleanup;
}
+static bool
+virNWFilterIsValidChainName(const char *chainname)
+{
+ return chainname[strspn(chainname, VALID_CHAINNAME)] == 0;
+}
Thanks; this validation is essential since your shell scripting was
pretty loose on quoting (that is, if a user could name a chain with an
embedded space, or worse a semicolon, your script would probably blow up).
+
+ if (!virNWFilterIsValidChainName(chainname)) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Chain name contains illegal characters"));
+ return NULL;
+ }
+
+ if (strlen(chainname) > MAX_CHAIN_SUFFIX_SIZE) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Name of chain is longer than %u
characters"),
+ MAX_CHAIN_SUFFIX_SIZE);
+ return NULL;
+ }
I think it would be worth inlining this second check (the strlen) into
the first one. That is, it would make more sense if
virNWFilterIsValidChainName checks both for valid characters and for
valid length; and all other callers only have to make one call for
validation purposes instead of two.
+ virBufferAsprintf(&buf,
+ _("Illegal chain name '%s'. Please use a chain name
"
+ "called '%s' or either one of the following prefixes:
"),
Illegal implies breaking a law; I prefer the term "Invalid".
s/either one of/any of/
In English, "either" is only appropriate for a choice among 2 items, but
you are listing more than 2.
+ virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT),
+ chainname);
+ for (i = 0; i < VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+ if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT)
+ continue;
+ if (printed)
+ virBufferAddLit(&buf, ", ");
+ virBufferAdd(&buf, virNWFilterChainSuffixTypeToString(i), -1);
+ printed = true;
+ }
[Hmm, wondering if this would be any simpler if we added the
virBufferTruncate that was mentioned as a possibility in another thread;
then you wouldn't need a 'printed' flag in your loop. But that's a
potential cleanup for another day.]
+
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ virBufferFreeAndReset(&buf);
+ goto err_exit;
+ }
+
+ msg = virBufferContentAndReset(&buf);
+
+ virNWFilterReportError(VIR_ERR_INVALID_ARG, _("%s"), msg);
Don't mark "%s" for translation. 'make syntax-check' should be
just
fine with unadorned virNWFilterReportError(VIR_ERR_INVALID_ARG, "%s", msg).
if (chain_pri_s) {
ret->chainPriority = chain_priority;
} else {
- /* assign an implicit priority -- support XML attribute later */
- if (intMapGetByString(chain_priorities, chain, 0,
+ /* assign an implicit priority */
Ah, the comment line I mentioned in patch 6 got changed here anyways.
Oh well, sorry if I'm causing you needless churn in conflict resolution
when you rebase.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6 a.m.
New subject: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 06:35 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
> This patch enables chains that have a known prefix in their name.
> Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'.
All prefixes
> are also protocols that can be evaluated on the ebtables level.
>
> Following the prefix they will be automatically connected to an interface's
> 'root' chain and jumped into following the protocol they evalute, i.e.,
s/evalute/evaluate/
> a table 'arp-xyz' will be accessed from the root table using
>
> ebtables -t nat -A<iface root table> -p arp -j I-<ifname>-arp-xyz
>
> thus generating a 'root' chain like this one here:
>
> Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
> -p IPv4 -j O-vnet0-ipv4
> -p ARP -j O-vnet0-arp
> -p 0x8035 -j O-vnet0-rarp
> -p ARP -j O-vnet0-arp-xyz
> -j DROP
>
> where the chain 'arp-xyz' is accessed for filtering of ARP packets.
>
> v3:
> - assign a priority to filters that have an allowed prefix, e.g., assign
> the arp chain priority to a filter arp-xyz unless user provided a
> priority in the XML
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 16 ++++++--
> src/conf/nwfilter_conf.c | 87 ++++++++++++++++++++++++++++++++++++++++++----
> src/conf/nwfilter_conf.h | 3 +
> 3 files changed, 96 insertions(+), 10 deletions(-)
Another patch without docs. (It's okay if the docs comes as a separate
patch, and if a single doc patch covers the XML additions in both 6 and
7; my main concern is only that the docs get patched by the time the
series is completed, and I didn't see it when glancing ahead to 10/10).
Will cover this part also.
> Index: libvirt-acl/src/conf/nwfilter_conf.c
> ===================================================================
> --- libvirt-acl.orig/src/conf/nwfilter_conf.c
> +++ libvirt-acl/src/conf/nwfilter_conf.c
> @@ -2007,6 +2007,80 @@ err_exit:
> goto cleanup;
> }
>
> +static bool
> +virNWFilterIsValidChainName(const char *chainname)
> +{
> + return chainname[strspn(chainname, VALID_CHAINNAME)] == 0;
> +}
Thanks; this validation is essential since your shell scripting was
pretty loose on quoting (that is, if a user could name a chain with an
embedded space, or worse a semicolon, your script would probably blow up).
> +
> + if (!virNWFilterIsValidChainName(chainname)) {
> + virNWFilterReportError(VIR_ERR_INVALID_ARG,
> + _("Chain name contains illegal
characters"));
> + return NULL;
> + }
> +
> + if (strlen(chainname)> MAX_CHAIN_SUFFIX_SIZE) {
> + virNWFilterReportError(VIR_ERR_INVALID_ARG,
> + _("Name of chain is longer than %u
characters"),
> + MAX_CHAIN_SUFFIX_SIZE);
> + return NULL;
> + }
I think it would be worth inlining this second check (the strlen) into
the first one. That is, it would make more sense if
virNWFilterIsValidChainName checks both for valid characters and for
valid length; and all other callers only have to make one call for
validation purposes instead of two.
Done.
> + virBufferAsprintf(&buf,
> + _("Illegal chain name '%s'. Please use a chain
name "
> + "called '%s' or either one of the following
prefixes: "),
Illegal implies breaking a law; I prefer the term "Invalid".
s/either one of/any of/
In English, "either" is only appropriate for a choice among 2 items, but
you are listing more than 2.
> + virNWFilterChainSuffixTypeToString(
> + VIR_NWFILTER_CHAINSUFFIX_ROOT),
> + chainname);
> + for (i = 0; i< VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
> + if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT)
> + continue;
> + if (printed)
> + virBufferAddLit(&buf, ", ");
> + virBufferAdd(&buf, virNWFilterChainSuffixTypeToString(i), -1);
> + printed = true;
> + }
[Hmm, wondering if this would be any simpler if we added the
virBufferTruncate that was mentioned as a possibility in another thread;
then you wouldn't need a 'printed' flag in your loop. But that's a
potential cleanup for another day.]
> +
> + if (virBufferError(&buf)) {
> + virReportOOMError();
> + virBufferFreeAndReset(&buf);
> + goto err_exit;
> + }
> +
> + msg = virBufferContentAndReset(&buf);
> +
> + virNWFilterReportError(VIR_ERR_INVALID_ARG, _("%s"), msg);
Don't mark "%s" for translation. 'make syntax-check' should be
just
fine with unadorned virNWFilterReportError(VIR_ERR_INVALID_ARG, "%s", msg).
Fixed.
> if (chain_pri_s) {
> ret->chainPriority = chain_priority;
> } else {
> - /* assign an implicit priority -- support XML attribute later */
> - if (intMapGetByString(chain_priorities, chain, 0,
> + /* assign an implicit priority */
Ah, the comment line I mentioned in patch 6 got changed here anyways.
Oh well, sorry if I'm causing you needless churn in conflict resolution
when you rebase.
Fixed it now.
5:38 p.m.
New subject: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 01:11 PM, Stefan Berger wrote:
+
+ if (strlen(chainname) > MAX_CHAIN_SUFFIX_SIZE) {
+ virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _("Name of chain is longer than %u
characters"),
+ MAX_CHAIN_SUFFIX_SIZE);
+ return NULL;
+ }
Oh, one other thought...
+ <data type="string">
+ <param name="pattern">arp[a-zA-Z0-9_\.:-]*</param>
+ </data>
Since it's easy enough to do, we should probably enforce the strlen rule
during XML validation, by writing this with {0,9} instead of * (since
MAX_CHAIN_SUFFIX_SIZE of 12 minus strlen("arp") is 9), and so forth for
the other patterns.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6 a.m.
New subject: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 06:38 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
> +
> + if (strlen(chainname)> MAX_CHAIN_SUFFIX_SIZE) {
> + virNWFilterReportError(VIR_ERR_INVALID_ARG,
> + _("Name of chain is longer than %u
characters"),
> + MAX_CHAIN_SUFFIX_SIZE);
> + return NULL;
> + }
Oh, one other thought...
> +<data type="string">
> +<param name="pattern">arp[a-zA-Z0-9_\.:-]*</param>
> +</data>
Since it's easy enough to do, we should probably enforce the strlen rule
during XML validation, by writing this with {0,9} instead of * (since
MAX_CHAIN_SUFFIX_SIZE of 12 minus strlen("arp") is 9), and so forth for
the other patterns.
Fixed everywhere, also in subsequent patch series adding vlan and stp...
2:11 p.m.
New subject: [libvirt] [PATCH V5 08/10] Extend rule priorities into negative numbers
So far rules' priorities have only been valid in the range [0,1000].
Now I am extending their priority into the range [-1000, 1000] for subsequently
being able to sort rules and the access of (jumps into) chains following
priorities.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/nwfilter_conf.c | 7 ++++---
src/conf/nwfilter_conf.h | 6 ++++--
src/nwfilter/nwfilter_ebiptables_driver.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.h | 2 +-
4 files changed, 10 insertions(+), 7 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -1897,7 +1897,7 @@ virNWFilterRuleParse(xmlNodePtr node)
char *statematch;
int found;
int found_i = 0;
- unsigned int priority;
+ int priority;
xmlNodePtr cur;
virNWFilterRuleDefPtr ret;
@@ -1943,8 +1943,9 @@ virNWFilterRuleParse(xmlNodePtr node)
ret->priority = MAX_RULE_PRIORITY / 2;
if (prio) {
- if (virStrToLong_ui(prio, NULL, 10, &priority) >= 0) {
- if (priority <= MAX_RULE_PRIORITY)
+ if (virStrToLong_i(prio, NULL, 10, &priority) >= 0) {
+ if (priority <= MAX_RULE_PRIORITY &&
+ priority >= MIN_RULE_PRIORITY)
ret->priority = priority;
}
}
Index: libvirt-acl/src/conf/nwfilter_conf.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -357,7 +357,7 @@ enum virNWFilterEbtablesTableType {
};
-# define MIN_RULE_PRIORITY 0
+# define MIN_RULE_PRIORITY -1000
# define MAX_RULE_PRIORITY 1000
# define NWFILTER_MIN_FILTER_PRIORITY -1000
@@ -389,10 +389,12 @@ enum virNWFilterRuleFlags {
void virNWFilterPrintStateMatchFlags(virBufferPtr buf, const char *prefix,
int32_t flags, bool disp_none);
+typedef int32_t virNWFilterRulePriority;
+
typedef struct _virNWFilterRuleDef virNWFilterRuleDef;
typedef virNWFilterRuleDef *virNWFilterRuleDefPtr;
struct _virNWFilterRuleDef {
- unsigned int priority;
+ virNWFilterRulePriority priority;
enum virNWFilterRuleFlags flags;
int action; /*enum virNWFilterRuleActionType*/
int tt; /*enum virNWFilterRuleDirectionType*/
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
@@ -38,7 +38,7 @@ struct _ebiptablesRuleInst {
const char *neededProtocolChain;
virNWFilterChainPriority chainPriority;
char chainprefix; /* I for incoming, O for outgoing */
- unsigned int priority;
+ virNWFilterRulePriority 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
@@ -388,7 +388,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
const char *neededChain,
virNWFilterChainPriority chainPriority,
char chainprefix,
- unsigned int priority,
+ virNWFilterRulePriority priority,
enum RuleType ruleType)
{
ebiptablesRuleInstPtr inst;
5:43 p.m.
New subject: [libvirt] [PATCH V5 08/10] Extend rule priorities into negative numbers
On 11/17/2011 01:11 PM, Stefan Berger wrote:
So far rules' priorities have only been valid in the range
[0,1000].
Now I am extending their priority into the range [-1000, 1000] for subsequently
being able to sort rules and the access of (jumps into) chains following
priorities.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/nwfilter_conf.c | 7 ++++---
src/conf/nwfilter_conf.h | 6 ++++--
src/nwfilter/nwfilter_ebiptables_driver.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.h | 2 +-
4 files changed, 10 insertions(+), 7 deletions(-)
.rng change was made previously.
ACK to this one.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:11 p.m.
New subject: [libvirt] [PATCH V5 09/10] Interleave jumping into chains with filtering rules in root table
The previous patch extends the priority of filtering rules into negative
numbers. We now use this possibility to interleave the jumping into
chains with filtering rules to for example create the 'root' table of
an interface with the following sequence of rules:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
The '-p ARP -j ACCEPT' rule now appears between the jumps.
Since the 'arp' chain has been assigned priority -700 and the 'rarp'
chain -600, the above ordering can now be achieved with the following
rule:
<rule action='accept' direction='out' priority='-650'>
<mac protocolid='arp'/>
</rule>
This patch now sorts the commands generating the above shown jumps into
chains and interleaves their execution with those for generating rules.
v3:
- fix memory leak
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 115 ++++++++++++++++++++++++++----
1 file changed, 103 insertions(+), 12 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
@@ -2718,13 +2718,17 @@ ebtablesUnlinkTmpRootChain(virBufferPtr
static int
-ebtablesCreateTmpSubChain(virBufferPtr buf,
+ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst,
+ int *nRuleInstances,
int incoming,
const char *ifname,
enum l3_proto_idx protoidx,
const char *filtername,
- int stopOnError)
+ int stopOnError,
+ virNWFilterChainPriority priority)
{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ ebiptablesRuleInstPtr tmp = *inst;
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
: CHAINPREFIX_HOST_OUT_TEMP;
@@ -2733,15 +2737,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
- virBufferAsprintf(buf,
+ virBufferAsprintf(&buf,
+ CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR
+ CMD_EXEC
+ CMD_DEF("%s -t %s -X %s") CMD_SEPARATOR
+ CMD_EXEC
CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
CMD_EXEC
"%s"
- CMD_DEF("%s -t %s -A %s -p 0x%x -j %s") CMD_SEPARATOR
+ CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
+ CMD_SEPARATOR
CMD_EXEC
"%s",
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
CMD_STOPONERR(stopOnError),
@@ -2750,6 +2761,24 @@ ebtablesCreateTmpSubChain(virBufferPtr b
CMD_STOPONERR(stopOnError));
+ if (virBufferError(&buf) ||
+ VIR_REALLOC_N(tmp, (*nRuleInstances)+1) < 0) {
+ virReportOOMError();
+ virBufferFreeAndReset(&buf);
+ return -1;
+ }
+
+ *inst = tmp;
+
+ memset(&tmp[*nRuleInstances], 0, sizeof(tmp[0]));
+ tmp[*nRuleInstances].priority = priority;
+ tmp[*nRuleInstances].commandTemplate =
+ virBufferContentAndReset(&buf);
+ tmp[*nRuleInstances].neededProtocolChain =
+ virNWFilterChainSuffixTypeToString(VIR_NWFILTER_CHAINSUFFIX_ROOT);
+
+ (*nRuleInstances)++;
+
return 0;
}
@@ -3224,9 +3253,34 @@ static int ebtablesCleanAll(const char *
static int
ebiptablesRuleOrderSort(const void *a, const void *b)
{
+ const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a;
+ const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b;
+ const char *root = virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT);
+ bool root_a = STREQ(insta->neededProtocolChain, root);
+ bool root_b = STREQ(instb->neededProtocolChain, root);
+
+ /* ensure root chain commands appear before all others since
+ we will need them to create the child chains */
+ if (root_a) {
+ if (root_b) {
+ goto normal;
+ }
+ return -1; /* a before b */
+ }
+ if (root_b) {
+ return 1; /* b before a */
+ }
+normal:
+ return (insta->priority - instb->priority);
+}
+
+static int
+ebiptablesRuleOrderSortPtr(const void *a, const void *b)
+{
const ebiptablesRuleInstPtr *insta = a;
const ebiptablesRuleInstPtr *instb = b;
- return ((*insta)->priority - (*instb)->priority);
+ return ebiptablesRuleOrderSort(*insta, *instb);
}
static int
@@ -3297,10 +3351,13 @@ ebtablesGetProtoIdxByFiltername(const ch
static int
ebtablesCreateTmpRootAndSubChains(virBufferPtr buf,
const char *ifname,
- virHashTablePtr chains, int direction)
+ virHashTablePtr chains, int direction,
+ ebiptablesRuleInstPtr *inst,
+ int *nRuleInstances)
{
int rc = 0, i;
virHashKeyValuePairPtr filter_names;
+ virNWFilterChainPriority *priority;
if (ebtablesCreateTmpRootChain(buf, direction, ifname, 1) < 0)
return -1;
@@ -3315,8 +3372,11 @@ ebtablesCreateTmpRootAndSubChains(virBuf
filter_names[i].key);
if ((int)idx < 0)
continue;
- rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
- filter_names[i].key, 1);
+ priority = virHashLookup(chains, filter_names[i].key);
+ rc = ebtablesCreateTmpSubChain(inst, nRuleInstances,
+ direction, ifname, idx,
+ filter_names[i].key, 1,
+ *priority);
if (rc < 0)
break;
}
@@ -3331,7 +3391,7 @@ ebiptablesApplyNewRules(virConnectPtr co
int nruleInstances,
void **_inst)
{
- int i;
+ int i, j;
int cli_status;
ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst;
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -3339,6 +3399,8 @@ ebiptablesApplyNewRules(virConnectPtr co
virHashTablePtr chains_out_set = virHashCreate(10, NULL);
bool haveIptables = false;
bool haveIp6tables = false;
+ ebiptablesRuleInstPtr ebtChains = NULL;
+ int nEbtChains = 0;
if (!chains_in_set || !chains_out_set) {
virReportOOMError();
@@ -3346,7 +3408,8 @@ ebiptablesApplyNewRules(virConnectPtr co
}
if (nruleInstances > 1 && inst)
- qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort);
+ qsort(inst, nruleInstances, sizeof(inst[0]),
+ ebiptablesRuleOrderSortPtr);
/* scan the rules to see which chains need to be created */
for (i = 0; i < nruleInstances; i++) {
@@ -3380,18 +3443,33 @@ ebiptablesApplyNewRules(virConnectPtr co
}
/* create needed chains */
- if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1) ||
- ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0)) {
+ if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1,
+ &ebtChains, &nEbtChains) ||
+ ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0,
+ &ebtChains, &nEbtChains)) {
goto tear_down_tmpebchains;
}
+ if (nEbtChains > 0)
+ qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]),
+ ebiptablesRuleOrderSort);
+
if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
goto tear_down_tmpebchains;
+ /* process ebtables commands; interleave commands from filters with
+ commands for creating and connecting ebtables chains */
+ j = 0;
for (i = 0; i < nruleInstances; i++) {
sa_assert (inst);
switch (inst[i]->ruleType) {
case RT_EBTABLES:
+ while (j < nEbtChains &&
+ ebtChains[j].priority <= inst[i]->priority) {
+ ebiptablesInstCommand(&buf,
+ ebtChains[j++].commandTemplate,
+ 'A', -1, 1);
+ }
ebiptablesInstCommand(&buf,
inst[i]->commandTemplate,
'A', -1, 1);
@@ -3405,6 +3483,11 @@ ebiptablesApplyNewRules(virConnectPtr co
}
}
+ while (j < nEbtChains)
+ ebiptablesInstCommand(&buf,
+ ebtChains[j++].commandTemplate,
+ 'A', -1, 1);
+
if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
goto tear_down_tmpebchains;
@@ -3484,6 +3567,10 @@ ebiptablesApplyNewRules(virConnectPtr co
virHashFree(chains_in_set);
virHashFree(chains_out_set);
+ for (i = 0; i < nEbtChains; i++)
+ VIR_FREE(ebtChains[i].commandTemplate);
+ VIR_FREE(ebtChains);
+
return 0;
tear_down_ebsubchains_and_unlink:
@@ -3522,6 +3609,10 @@ exit_free_sets:
virHashFree(chains_in_set);
virHashFree(chains_out_set);
+ for (i = 0; i < nEbtChains; i++)
+ VIR_FREE(ebtChains[i].commandTemplate);
+ VIR_FREE(ebtChains);
+
return 1;
}
6:15 p.m.
New subject: [libvirt] [PATCH V5 09/10] Interleave jumping into chains with filtering rules in root table
On 11/17/2011 01:11 PM, Stefan Berger wrote:
The previous patch extends the priority of filtering rules into
negative
numbers. We now use this possibility to interleave the jumping into
chains with filtering rules to for example create the 'root' table of
an interface with the following sequence of rules:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
The '-p ARP -j ACCEPT' rule now appears between the jumps.
Since the 'arp' chain has been assigned priority -700 and the 'rarp'
chain -600, the above ordering can now be achieved with the following
rule:
<rule action='accept' direction='out' priority='-650'>
<mac protocolid='arp'/>
</rule>
This patch now sorts the commands generating the above shown jumps into
chains and interleaves their execution with those for generating rules.
Overall, it looks like it does what you say. But it may be worth a v6.
@@ -2733,15 +2737,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
- virBufferAsprintf(buf,
+ virBufferAsprintf(&buf,
+ CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR
+ CMD_EXEC
+ CMD_DEF("%s -t %s -X %s") CMD_SEPARATOR
+ CMD_EXEC
Looks like my comments on v4 4/10 would apply here as well - a patch
11/10 that refactored things to use a shell variable initialized once up
front, instead of passing repetitive command names through %s all over
the place, might make this generator easier to follow. But not a
problem for the context of this patch. This hunk adds 6 printf args,
CMD_DEF("%s -t %s -N %s")
CMD_SEPARATOR
CMD_EXEC
"%s"
- CMD_DEF("%s -t %s -A %s -p 0x%x -j %s") CMD_SEPARATOR
+ CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
+ CMD_SEPARATOR
and this hunk has no net effect, but generates a string which will be
handed as the format string to yet another printf? Wow, that's a bit
hard to follow...
CMD_EXEC
"%s",
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
CMD_STOPONERR(stopOnError),
@@ -2750,6 +2761,24 @@ ebtablesCreateTmpSubChain(virBufferPtr b
CMD_STOPONERR(stopOnError));
+ if (virBufferError(&buf) ||
+ VIR_REALLOC_N(tmp, (*nRuleInstances)+1) < 0) {
+ virReportOOMError();
+ virBufferFreeAndReset(&buf);
+ return -1;
+ }
+
+ *inst = tmp;
+
+ memset(&tmp[*nRuleInstances], 0, sizeof(tmp[0]));
Using VIR_EXPAND_N instead of VIR_REALLOC_N would take care of this
memset for you.
+ tmp[*nRuleInstances].priority = priority;
+ tmp[*nRuleInstances].commandTemplate =
+ virBufferContentAndReset(&buf);
...If I followed things correctly, commandTemplate now has exactly two
print arguments, %c and %s. But looking further, it looks like you
already have other commandTemplate uses just like this.
ebiptablesRuleOrderSort(const void *a, const void *b)
{
+ const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a;
+ const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b;
+ const char *root = virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT);
+ bool root_a = STREQ(insta->neededProtocolChain, root);
+ bool root_b = STREQ(instb->neededProtocolChain, root);
+
+ /* ensure root chain commands appear before all others since
+ we will need them to create the child chains */
+ if (root_a) {
+ if (root_b) {
+ goto normal;
+ }
+ return -1; /* a before b */
+ }
+ if (root_b) {
+ return 1; /* b before a */
+ }
+normal:
+ return (insta->priority - instb->priority);
Refer to my review earlier in the series about adding a comment how
priority is in a bounded range, so the subtraction is safe.
@@ -3315,8 +3372,11 @@ ebtablesCreateTmpRootAndSubChains(virBuf
filter_names[i].key);
if ((int)idx < 0)
continue;
- rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
- filter_names[i].key, 1);
+ priority = virHashLookup(chains, filter_names[i].key);
Why do a virHashLookup, when you already have filter_names[i].value?
(See, I knew there was a reason to return key/value pairs).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6 a.m.
New subject: [libvirt] [PATCH V5 09/10] Interleave jumping into chains with filtering rules in root table
On 11/17/2011 07:15 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
> The previous patch extends the priority of filtering rules into negative
> numbers. We now use this possibility to interleave the jumping into
> chains with filtering rules to for example create the 'root' table of
> an interface with the following sequence of rules:
>
> Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
> -p IPv4 -j I-vnet0-ipv4
> -p ARP -j I-vnet0-arp
> -p ARP -j ACCEPT
> -p 0x8035 -j I-vnet0-rarp
> -p 0x835 -j ACCEPT
> -j DROP
>
> The '-p ARP -j ACCEPT' rule now appears between the jumps.
> Since the 'arp' chain has been assigned priority -700 and the 'rarp'
> chain -600, the above ordering can now be achieved with the following
> rule:
>
> <rule action='accept' direction='out'
priority='-650'>
> <mac protocolid='arp'/>
> </rule>
>
> This patch now sorts the commands generating the above shown jumps into
> chains and interleaves their execution with those for generating rules.
>
Overall, it looks like it does what you say. But it may be worth a v6.
> @@ -2733,15 +2737,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b
> PRINT_CHAIN(chain, chainPrefix, ifname,
> (filtername) ? filtername : l3_protocols[protoidx].val);
>
> - virBufferAsprintf(buf,
> + virBufferAsprintf(&buf,
> + CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR
> + CMD_EXEC
> + CMD_DEF("%s -t %s -X %s") CMD_SEPARATOR
> + CMD_EXEC
Looks like my comments on v4 4/10 would apply here as well - a patch
11/10 that refactored things to use a shell variable initialized once up
front, instead of passing repetitive command names through %s all over
the place, might make this generator easier to follow. But not a
problem for the context of this patch. This hunk adds 6 printf args,
I'll do it, but can we defer this patch for later so it doesn't cause
too much churn on all other pending patches (series)?
> CMD_DEF("%s -t %s -N %s")
CMD_SEPARATOR
> CMD_EXEC
> "%s"
> - CMD_DEF("%s -t %s -A %s -p 0x%x -j %s")
CMD_SEPARATOR
> + CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
> + CMD_SEPARATOR
and this hunk has no net effect, but generates a string which will be
handed as the format string to yet another printf? Wow, that's a bit
hard to follow...
> CMD_EXEC
> "%s",
>
> ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
> + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
> + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
>
> CMD_STOPONERR(stopOnError),
>
> @@ -2750,6 +2761,24 @@ ebtablesCreateTmpSubChain(virBufferPtr b
>
> CMD_STOPONERR(stopOnError));
>
> + if (virBufferError(&buf) ||
> + VIR_REALLOC_N(tmp, (*nRuleInstances)+1)< 0) {
> + virReportOOMError();
> + virBufferFreeAndReset(&buf);
> + return -1;
> + }
> +
> + *inst = tmp;
> +
> + memset(&tmp[*nRuleInstances], 0, sizeof(tmp[0]));
Using VIR_EXPAND_N instead of VIR_REALLOC_N would take care of this
memset for you.
With the side effect that I need an additional variable
count = *nRuleInstances;
Converted..
> + tmp[*nRuleInstances].priority = priority;
> + tmp[*nRuleInstances].commandTemplate =
> + virBufferContentAndReset(&buf);
...If I followed things correctly, commandTemplate now has exactly two
print arguments, %c and %s. But looking further, it looks like you
already have other commandTemplate uses just like this.
Yes, there are others. I had to convert it to be able to treat the
'jumping into subchains' equivalent to 'normal filtering rules'.
> ebiptablesRuleOrderSort(const void *a, const void *b)
> {
> + const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a;
> + const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b;
> + const char *root = virNWFilterChainSuffixTypeToString(
> + VIR_NWFILTER_CHAINSUFFIX_ROOT);
> + bool root_a = STREQ(insta->neededProtocolChain, root);
> + bool root_b = STREQ(instb->neededProtocolChain, root);
> +
> + /* ensure root chain commands appear before all others since
> + we will need them to create the child chains */
> + if (root_a) {
> + if (root_b) {
> + goto normal;
> + }
> + return -1; /* a before b */
> + }
> + if (root_b) {
> + return 1; /* b before a */
> + }
> +normal:
> + return (insta->priority - instb->priority);
Refer to my review earlier in the series about adding a comment how
priority is in a bounded range, so the subtraction is safe.
Done.
> @@ -3315,8 +3372,11 @@ ebtablesCreateTmpRootAndSubChains(virBuf
> filter_names[i].key);
> if ((int)idx< 0)
> continue;
> - rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
> - filter_names[i].key, 1);
> + priority = virHashLookup(chains, filter_names[i].key);
Why do a virHashLookup, when you already have filter_names[i].value?
(See, I knew there was a reason to return key/value pairs).
I guess I didn't pay enough attention when converting the code. Fixed
this instance.
2:11 p.m.
New subject: [libvirt] [PATCH V5 10/10] Add test cases
Add test case for the chain names with known prefixes and the chain
priority.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 +++++++++++++++++++++++++
tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++++++++++++++
tests/nwfilterxml2xmltest.c | 2 +
3 files changed, 60 insertions(+)
Index: libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml
@@ -0,0 +1,37 @@
+<filter name='testcase' chain='arp-testme'
priority='-650'>
+ <uuid>e5700920-a333-4c05-8016-b669e46b7599</uuid>
+ <rule action='accept' direction='out'>
+ <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ protocolid='arp'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ hwtype='12'
+ protocoltype='34'
+ opcode='Request'
+ arpsrcmacaddr='1:2:3:4:5:6'
+ arpdstmacaddr='a:b:c:d:e:f'/>
+ </rule>
+
+ <rule action='accept' direction='out'>
+ <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ opcode='1' hwtype='255' protocoltype='255'/>
+ </rule>
+
+ <rule action='accept' direction='out'>
+ <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ opcode='11' hwtype='256' protocoltype='256'/>
+ </rule>
+
+ <rule action='accept' direction='out'>
+ <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ opcode='65535' hwtype='65535' protocoltype='65535'
/>
+ </rule>
+
+ <rule action='accept' direction='out'>
+ <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ opcode='65536' hwtype='65536' protocoltype='65536'
/>
+ </rule>
+
+ <rule action='accept' direction='in'>
+ <arp gratuitous='true'/>
+ </rule>
+</filter>
Index: libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml
@@ -0,0 +1,21 @@
+<filter name='testcase' chain='arp-testme'
priority='-650'>
+ <uuid>e5700920-a333-4c05-8016-b669e46b7599</uuid>
+ <rule action='accept' direction='out' priority='500'>
+ <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'
dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
hwtype='12' protocoltype='34' opcode='Request'
arpsrcmacaddr='01:02:03:04:05:06' arpdstmacaddr='0a:0b:0c:0d:0e:0f'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'
hwtype='255' protocoltype='255' opcode='Request'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'
hwtype='256' protocoltype='256' opcode='11'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'
hwtype='65535' protocoltype='65535' opcode='65535'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <arp srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff'/>
+ </rule>
+ <rule action='accept' direction='in' priority='500'>
+ <arp gratuitous='true'/>
+ </rule>
+</filter>
Index: libvirt-acl/tests/nwfilterxml2xmltest.c
===================================================================
--- libvirt-acl.orig/tests/nwfilterxml2xmltest.c
+++ libvirt-acl/tests/nwfilterxml2xmltest.c
@@ -148,6 +148,8 @@ mymain(void)
DO_TEST("example-1", false);
DO_TEST("example-2", false);
+ DO_TEST("chain_prefixtest1", true); /* derived from arp-test */
+
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
6:17 p.m.
New subject: [libvirt] [PATCH V5 10/10] Add test cases
On 11/17/2011 01:11 PM, Stefan Berger wrote:
Add test case for the chain names with known prefixes and the chain
priority.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 +++++++++++++++++++++++++
tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++++++++++++++
tests/nwfilterxml2xmltest.c | 2 +
3 files changed, 60 insertions(+)
Yay - more tests is always a good thing.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:01 a.m.
New subject: [libvirt] [PATCH V5 10/10] Add test cases
On 11/17/2011 07:17 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
> Add test case for the chain names with known prefixes and the chain
> priority.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 +++++++++++++++++++++++++
> tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++++++++++++++
> tests/nwfilterxml2xmltest.c | 2 +
> 3 files changed, 60 insertions(+)
Yay - more tests is always a good thing.
ACK.
I will add a part 11 with documentation of priorities of the chains. The
'shell variable patch' would then come later, but I will post it.
Thanks again for the review. It really does improve things -- especially
the shell code :-)
Stefan
4753
days inactive
4754
days old
29 comments
2 participants
participants (2)
-
Eric Blake -
Stefan Berger