12:22 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.
Regards,
Stefan
12:22 p.m.
New subject: [libvirt] [PATCH V2 01/10] Extend virHashTable with function to get hash tables keys
Add a function to the virHashTable for getting an array of the hash table's
keys and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/libvirt_private.syms | 3 +
src/util/hash.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/hash.h | 14 ++++++
3 files changed, 115 insertions(+)
Index: libvirt-acl/src/util/hash.c
===================================================================
--- libvirt-acl.orig/src/util/hash.c
+++ libvirt-acl/src/util/hash.c
@@ -618,3 +618,101 @@ void *virHashSearch(virHashTablePtr tabl
return NULL;
}
+
+
+struct getKeysIter
+{
+ virHashTablePtr table;
+ void **array;
+ virHashKeyValuePair *sortArray;
+ unsigned arrayIdx;
+ bool error;
+};
+
+static void virHashGetKeysIterator(void *payload,
+ const void *name, void *data)
+{
+ struct getKeysIter *iter = data;
+ void *key;
+
+ if (iter->error)
+ return;
+
+ key = iter->table->keyCopy(name);
+
+ if (key == NULL) {
+ virReportOOMError();
+ iter->error = true;
+ return;
+ }
+
+ if (iter->sortArray) {
+ iter->sortArray[iter->arrayIdx].key = key;
+ iter->sortArray[iter->arrayIdx].value = payload;
+ } else {
+ iter->array[iter->arrayIdx] = iter->table->keyCopy(name);
+ }
+ iter->arrayIdx++;
+}
+
+void virHashFreeKeys(virHashTablePtr table, void **keys)
+{
+ unsigned i = 0;
+
+ if (keys == NULL)
+ return;
+
+ while (keys[i])
+ table->keyFree(keys[i++]);
+
+ VIR_FREE(keys);
+}
+
+typedef int (*qsort_comp)(const void *, const void *);
+
+void **virHashGetKeys(virHashTablePtr table,
+ virHashKeyComparator compar)
+{
+ int i, numElems = virHashSize(table);
+ struct getKeysIter iter = {
+ .table = table,
+ .arrayIdx = 0,
+ .error = false,
+ };
+
+ if (numElems < 0) {
+ return NULL;
+ }
+
+ if (VIR_ALLOC_N(iter.array, numElems + 1)) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ /* null-terminate array */
+ iter.array[numElems] = NULL;
+
+ if (compar &&
+ VIR_ALLOC_N(iter.sortArray, numElems)) {
+ VIR_FREE(iter.array);
+ virReportOOMError();
+ return NULL;
+ }
+
+ virHashForEach(table, virHashGetKeysIterator, &iter);
+ if (iter.error) {
+ VIR_FREE(iter.sortArray);
+ virHashFreeKeys(table, iter.array);
+ return NULL;
+ }
+
+ if (compar) {
+ qsort(&iter.sortArray[0], numElems, sizeof(iter.sortArray[0]),
+ (qsort_comp)compar);
+ for (i = 0; i < numElems; i++)
+ iter.array[i] = iter.sortArray[i].key;
+ VIR_FREE(iter.sortArray);
+ }
+
+ return iter.array;
+}
Index: libvirt-acl/src/util/hash.h
===================================================================
--- libvirt-acl.orig/src/util/hash.h
+++ libvirt-acl/src/util/hash.h
@@ -130,6 +130,20 @@ void *virHashLookup(virHashTablePtr tabl
*/
void *virHashSteal(virHashTablePtr table, const void *name);
+/*
+ * Get the set of keys and have them optionally sorted.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+ void *key;
+ const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+ const virHashKeyValuePairPtr );
+void **virHashGetKeys(virHashTablePtr table, virHashKeyComparator compar);
+void virHashFreeKeys(virHashTablePtr table, void **);
+
/*
* Iterators
Index: libvirt-acl/src/libvirt_private.syms
===================================================================
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -552,6 +552,8 @@ virHashAddEntry;
virHashCreate;
virHashForEach;
virHashFree;
+virHashFreeKeys;
+virHashGetKeys;
virHashLookup;
virHashRemoveEntry;
virHashRemoveSet;
@@ -559,6 +561,7 @@ virHashSearch;
virHashSize;
virHashSteal;
virHashTableSize;
+virHashUpdateEntry;
# hooks.h
12:22 p.m.
New subject: [libvirt] [PATCH V2 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>
---
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
@@ -355,8 +355,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 -900
+# define NWFILTER_IPV6_FILTER_PRI -800
+# define NWFILTER_ARP_FILTER_PRI -700
+# define NWFILTER_RARP_FILTER_PRI -600
+
enum virNWFilterRuleFlags {
RULE_FLAG_NO_STATEMATCH = (1 << 0),
RULE_FLAG_STATE_NEW = (1 << 1),
@@ -434,6 +444,7 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
+typedef int32_t virNWFilterChainPriority;
typedef struct _virNWFilterDef virNWFilterDef;
typedef virNWFilterDef *virNWFilterDefPtr;
@@ -443,6 +454,7 @@ struct _virNWFilterDef {
unsigned char uuid[VIR_UUID_BUFLEN];
int chainsuffix; /*enum virNWFilterChainSuffixType */
+ virNWFilterChainPriority chainPriority;
int nentries;
virNWFilterEntryPtr *filterEntries;
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -120,6 +120,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
@@ -2026,6 +2034,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
@@ -327,6 +327,7 @@ static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
enum virNWFilterChainSuffixType neededChain,
+ virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
enum RuleType ruleType)
@@ -340,6 +341,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
inst->commandTemplate = commandTemplate;
inst->neededProtocolChain = neededChain;
+ inst->chainPriority = chainPriority;
inst->chainprefix = chainprefix;
inst->priority = priority;
inst->ruleType = ruleType;
@@ -1588,6 +1590,7 @@ _iptablesCreateRuleInstance(int directio
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(final),
nwfilter->chainsuffix,
+ nwfilter->chainPriority,
'\0',
rule->priority,
(isIPv6) ? RT_IP6TABLES : RT_IPTABLES);
@@ -2337,6 +2340,7 @@ ebtablesCreateRuleInstance(char chainPre
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(&buf),
nwfilter->chainsuffix,
+ nwfilter->chainPriority,
chainPrefix,
rule->priority,
RT_EBTABLES);
12:22 p.m.
New subject: [libvirt] [PATCH V2 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>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 130 ++++++++++++++++++++++--------
1 file changed, 98 insertions(+), 32 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2774,6 +2774,7 @@ ebtablesCreateTmpSubChain(virBufferPtr b
int incoming,
const char *ifname,
enum l3_proto_idx protoidx,
+ const char *filtername,
int stopOnError)
{
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
@@ -2781,7 +2782,8 @@ ebtablesCreateTmpSubChain(virBufferPtr b
: CHAINPREFIX_HOST_OUT_TEMP;
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
- PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
+ PRINT_CHAIN(chain, chainPrefix, ifname,
+ (filtername) ? filtername : l3_protocols[protoidx].val);
virBufferAsprintf(buf,
CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
@@ -3288,6 +3290,13 @@ ebiptablesRuleOrderSort(const void *a, c
return ((*insta)->priority - (*instb)->priority);
}
+static int
+ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+ return *(virNWFilterChainPriority *)a->value -
+ *(virNWFilterChainPriority *)b->value;
+}
static void
iptablesCheckBridgeNFCallEnabled(bool isIPv6)
@@ -3327,6 +3336,56 @@ iptablesCheckBridgeNFCallEnabled(bool is
}
}
+/*
+ * Given a filtername determine the protocol it is used for evaluating
+ * We do prefix-matching to determine the protocol.
+ */
+static enum l3_proto_idx
+ebtablesGetProtoIdxByFiltername(const char *filtername)
+{
+ enum l3_proto_idx idx;
+
+ for (idx = 0; idx < L3_PROTO_LAST_IDX; idx++) {
+ if (STRPREFIX(filtername, l3_protocols[idx].val)) {
+ return idx;
+ }
+ }
+
+ return -1;
+}
+
+static int
+ebtablesCreateTmpRootAndSubChains(virBufferPtr buf,
+ const char *ifname,
+ virHashTablePtr chains, int direction)
+{
+ int rc = 0, i;
+
+ if (virHashSize(chains) != 0) {
+ char **filter_names;
+
+ ebtablesCreateTmpRootChain(buf, direction, ifname, 1);
+ filter_names = (char **)virHashGetKeys(chains,
+ ebiptablesFilterOrderSort);
+ if (filter_names == NULL) {
+ virReportOOMError();
+ rc = 1;
+ goto err_exit;
+ }
+ for (i = 0; filter_names[i]; i++) {
+ enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(
+ filter_names[i]);
+ if ((int)idx < 0)
+ continue;
+ ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
+ filter_names[i], 1);
+ }
+ virHashFreeKeys(chains, (void **)filter_names);
+ }
+
+ err_exit:
+ return rc;
+}
static int
ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3337,24 +3396,43 @@ ebiptablesApplyNewRules(virConnectPtr co
int i;
int cli_status;
ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst;
- int chains_in = 0, chains_out = 0;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virHashTablePtr chains_in_set = virHashCreate(10, NULL),
+ chains_out_set = virHashCreate(10, NULL);
bool haveIptables = false;
bool haveIp6tables = false;
+ if (!chains_in_set || !chains_out_set) {
+ virReportOOMError();
+ goto exit_free_sets;
+ }
+
if (nruleInstances > 1 && inst)
qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort);
+ /* scan the rules to see which chains need to be created */
for (i = 0; i < nruleInstances; i++) {
sa_assert (inst);
if (inst[i]->ruleType == RT_EBTABLES) {
- if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP)
- chains_in |= (1 << inst[i]->neededProtocolChain);
- else
- chains_out |= (1 << inst[i]->neededProtocolChain);
+ const char *name = virNWFilterChainSuffixTypeToString(
+ inst[i]->neededProtocolChain);
+ if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) {
+ if (virHashUpdateEntry(chains_in_set, name,
+ &inst[i]->chainPriority)) {
+ virReportOOMError();
+ goto exit_free_sets;
+ }
+ } else {
+ if (virHashUpdateEntry(chains_out_set, name,
+ &inst[i]->chainPriority)) {
+ virReportOOMError();
+ goto exit_free_sets;
+ }
+ }
}
}
+ /* cleanup whatever may exist */
if (ebtables_cmd_path) {
ebtablesUnlinkTmpRootChain(&buf, 1, ifname);
ebtablesUnlinkTmpRootChain(&buf, 0, ifname);
@@ -3364,30 +3442,11 @@ ebiptablesApplyNewRules(virConnectPtr co
ebiptablesExecCLI(&buf, &cli_status);
}
- if (chains_in != 0)
- ebtablesCreateTmpRootChain(&buf, 1, ifname, 1);
- if (chains_out != 0)
- ebtablesCreateTmpRootChain(&buf, 0, ifname, 1);
-
- if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4))
- ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV4_IDX, 1);
- if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4))
- ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV4_IDX, 1);
-
- if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6))
- ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV6_IDX, 1);
- if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6))
- ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV6_IDX, 1);
-
- /* keep arp,rarp as last */
- if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP))
- ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARP_IDX, 1);
- if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP))
- ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARP_IDX, 1);
- if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP))
- ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_RARP_IDX, 1);
- if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP))
- ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_RARP_IDX, 1);
+ /* create needed chains */
+ if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1) ||
+ ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0)) {
+ goto tear_down_tmpebchains;
+ }
if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
goto tear_down_tmpebchains;
@@ -3477,14 +3536,17 @@ ebiptablesApplyNewRules(virConnectPtr co
iptablesCheckBridgeNFCallEnabled(true);
}
- if (chains_in != 0)
+ if (virHashSize(chains_in_set) != 0)
ebtablesLinkTmpRootChain(&buf, 1, ifname, 1);
- if (chains_out != 0)
+ if (virHashSize(chains_out_set) != 0)
ebtablesLinkTmpRootChain(&buf, 0, ifname, 1);
if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
goto tear_down_ebsubchains_and_unlink;
+ virHashFree(chains_in_set);
+ virHashFree(chains_out_set);
+
return 0;
tear_down_ebsubchains_and_unlink:
@@ -3519,6 +3581,10 @@ tear_down_tmpebchains:
"interface %s."),
ifname);
+exit_free_sets:
+ virHashFree(chains_in_set);
+ virHashFree(chains_out_set);
+
return 1;
}
12:22 p.m.
New subject: [libvirt] [PATCH V2 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
chanins 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.
v2:
- setting IFS to intended value; works with bash and dash (phew!)
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 204 +++++++++++++++++-------------
1 file changed, 117 insertions(+), 87 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -91,6 +91,49 @@ static char *gawk_cmd_path;
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix)
+static const char ebtables_script_func_collect_chains[] =
+ "collect_chains()\n"
+ "{\n"
+ " local sc\n"
+ " sc=$(%s -t %s -L $1 | \\\n"
+ " sed -n \"/Bridge chain*/,$ s/.*\\-j
\\([%s]-.*\\)/\\1/p\")\n"
+ " for tmp in `echo \"$sc\"`; do\n"
+ " [ -n \"$tmp\" ] && sc=\"$sc $(collect_chains
$tmp)\"\n"
+ " done\n"
+ " echo \"$sc\"\n"
+ "}\n";
+
+static const char ebiptables_script_func_rm_chains[] =
+ "rm_chains()\n"
+ "{\n"
+ " for tmp in `echo \"$1\"`; do [ -n \"$tmp\" ] && %s
-t %s -F $tmp; done\n"
+ " for tmp in `echo \"$1\"`; do [ -n \"$tmp\" ] && %s
-t %s -X $tmp; done\n"
+ "}\n";
+
+static const char ebiptables_script_func_rename_chains[] =
+ "rename_chains()\n"
+ "{\n"
+ " for tmp in `echo \"$1\"`; do\n"
+ " if [ -n \"$tmp\" ]; then\n"
+ " tmp2=`expr substr \"$tmp\" 1 1`\n"
+ " if [ $tmp2 = \"%c\" ]; then\n"
+ " %s -t %s -E \"$tmp\" \"%c\"`expr substr
\"$tmp\" 2 33`\n"
+ " elif [ $tmp2 = \"%c\" ]; then\n"
+ " %s -t %s -E \"$tmp\" \"%c\"`expr substr
\"$tmp\" 2 33`\n"
+ " fi\n"
+ " fi\n"
+ " done\n"
+ "}\n";
+
+static const char ebiptables_script_set_ifs[] =
+ "IFS=$' \\t\\n'\n"
+ "[ `expr length \"$IFS\"` != 3 ] && "
+ "IFS=$(echo | %s '{ print \"\\t\\n \"}')\n";
+
+#define NWFILTER_FUNC_COLLECT_CHAINS ebtables_script_func_collect_chains
+#define NWFILTER_FUNC_DELETE_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"
@@ -2805,95 +2848,65 @@ ebtablesCreateTmpSubChain(virBufferPtr b
return 0;
}
-
-static int
-_ebtablesRemoveSubChain(virBufferPtr buf,
- int incoming,
- const char *ifname,
- enum l3_proto_idx protoidx,
- int isTempChain)
+static int _ebtablesRemoveSubChains(virBufferPtr buf,
+ const char *ifname,
+ const char *chains)
{
- char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
- char chainPrefix;
-
- if (isTempChain) {
- chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN_TEMP
- : CHAINPREFIX_HOST_OUT_TEMP;
- } else {
- chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN
- : CHAINPREFIX_HOST_OUT;
- }
+ char rootchain[MAX_CHAINNAME_LENGTH];
+ unsigned i;
- PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
- PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
-
- virBufferAsprintf(buf,
- "%s -t %s -D %s -p 0x%x -j %s" CMD_SEPARATOR
- "%s -t %s -F %s" CMD_SEPARATOR
- "%s -t %s -X %s" CMD_SEPARATOR,
+ virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
+ virBufferAsprintf(buf, NWFILTER_FUNC_DELETE_CHAINS,
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
- rootchain, l3_protocols[protoidx].attr, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE);
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS, gawk_cmd_path);
+ virBufferAddLit(buf, "a=\"");
+ for (i = 0; chains[i] != 0; i++) {
+ PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+ virBufferAsprintf(buf, "$(collect_chains %s) ", rootchain);
+ }
+ virBufferAddLit(buf, "\"\n");
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+ for (i = 0; chains[i] != 0; i++) {
+ PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+ virBufferAsprintf(buf,
+ "%s -t %s -F %s\n",
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
+ rootchain);
+ }
+ virBufferAddLit(buf, "rm_chains \"$a\"\n");
return 0;
}
-
-static int
-ebtablesRemoveSubChain(virBufferPtr buf,
- int incoming,
- const char *ifname,
- enum l3_proto_idx protoidx)
-{
- return _ebtablesRemoveSubChain(buf,
- incoming, ifname, protoidx, 0);
-}
-
-
static int
ebtablesRemoveSubChains(virBufferPtr buf,
const char *ifname)
{
- enum l3_proto_idx i;
-
- for (i = 0; i < L3_PROTO_LAST_IDX; i++) {
- ebtablesRemoveSubChain(buf, 1, ifname, i);
- ebtablesRemoveSubChain(buf, 0, ifname, i);
- }
+ char chains[3] = {
+ CHAINPREFIX_HOST_IN,
+ CHAINPREFIX_HOST_OUT,
+ 0
+ };
- return 0;
+ return _ebtablesRemoveSubChains(buf, ifname, chains);
}
-
-static int
-ebtablesRemoveTmpSubChain(virBufferPtr buf,
- int incoming,
- const char *ifname,
- enum l3_proto_idx protoidx)
-{
- return _ebtablesRemoveSubChain(buf,
- incoming, ifname, protoidx, 1);
-}
-
-
static int
ebtablesRemoveTmpSubChains(virBufferPtr buf,
const char *ifname)
{
- enum l3_proto_idx i;
-
- for (i = 0; i < L3_PROTO_LAST_IDX; i++) {
- ebtablesRemoveTmpSubChain(buf, 1, ifname, i);
- ebtablesRemoveTmpSubChain(buf, 0, ifname, i);
- }
+ char chains[3] = {
+ CHAINPREFIX_HOST_IN_TEMP,
+ CHAINPREFIX_HOST_OUT_TEMP,
+ 0
+ };
- return 0;
+ return _ebtablesRemoveSubChains(buf, ifname, chains);
}
-
static int
ebtablesRenameTmpSubChain(virBufferPtr buf,
int incoming,
@@ -2920,31 +2933,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);
+}
- for (i = 0; i < L3_PROTO_LAST_IDX; i++) {
- ebtablesRenameTmpSubChain (buf, 1, ifname, l3_protocols[i].val);
- ebtablesRenameTmpSubChain (buf, 0, ifname, l3_protocols[i].val);
+static int
+ebtablesRenameTmpSubAndRootChains(virBufferPtr buf,
+ const char *ifname)
+{
+ char rootchain[MAX_CHAINNAME_LENGTH];
+ unsigned i;
+ char chains[3] = {
+ CHAINPREFIX_HOST_IN_TEMP,
+ CHAINPREFIX_HOST_OUT_TEMP,
+ 0};
+
+ virBufferAsprintf(buf, 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);
+
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS, gawk_cmd_path);
+ virBufferAddLit(buf, "a=\"");
+ for (i = 0; chains[i] != 0; i++) {
+ PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+ virBufferAsprintf(buf, "$(collect_chains %s) ", rootchain);
}
+ virBufferAddLit(buf, "\"\n");
- return 0;
-}
+ virBufferAddLit(buf, "rename_chains \"$a\"\n");
+ ebtablesRenameTmpRootChain(buf, 1, ifname);
+ ebtablesRenameTmpRootChain(buf, 0, ifname);
-static int
-ebtablesRenameTmpRootChain(virBufferPtr buf,
- int incoming,
- const char *ifname)
-{
- return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL);
+ return 0;
}
-
static void
ebiptablesInstCommand(virBufferPtr buf,
const char *templ, char cmd, int pos,
@@ -3654,9 +3687,7 @@ ebiptablesTearOldRules(virConnectPtr con
ebtablesRemoveRootChain(&buf, 1, ifname);
ebtablesRemoveRootChain(&buf, 0, ifname);
- ebtablesRenameTmpSubChains(&buf, ifname);
- ebtablesRenameTmpRootChain(&buf, 1, ifname);
- ebtablesRenameTmpRootChain(&buf, 0, ifname);
+ ebtablesRenameTmpSubAndRootChains(&buf, ifname);
ebiptablesExecCLI(&buf, &cli_status);
}
@@ -3741,12 +3772,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;
12:22 p.m.
New subject: [libvirt] [PATCH V2 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
@@ -305,6 +305,7 @@ virNWFilterDefFree(virNWFilterDefPtr def
virNWFilterEntryFree(def->filterEntries[i]);
VIR_FREE(def->filterEntries);
+ VIR_FREE(def->chainsuffix);
VIR_FREE(def);
}
@@ -2025,21 +2026,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);
@@ -2905,7 +2913,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
@@ -453,7 +453,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
@@ -369,7 +369,7 @@ ebiptablesRuleInstFree(ebiptablesRuleIns
static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
- enum virNWFilterChainSuffixType neededChain,
+ const char *neededChain,
virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
@@ -1945,11 +1945,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) {
@@ -2516,7 +2518,7 @@ ebiptablesDisplayRuleInstance(virConnect
ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst;
VIR_INFO("Command Template: '%s', Needed protocol: '%s'",
inst->commandTemplate,
- virNWFilterChainSuffixTypeToString(inst->neededProtocolChain));
+ inst->neededProtocolChain);
return 0;
}
@@ -3447,8 +3449,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;
12:22 p.m.
New subject: [libvirt] [PATCH V2 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
@@ -2012,7 +2012,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();
@@ -2026,6 +2028,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_INTERNAL_ERROR,
+ _("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_INTERNAL_ERROR,
+ _("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) {
@@ -2034,11 +2056,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 {
@@ -2095,6 +2122,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
}
VIR_FREE(chain);
+ VIR_FREE(chain_pri_s);
return ret;
@@ -2102,6 +2130,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
virNWFilterDefFree(ret);
VIR_FREE(chain);
VIR_FREE(uuid);
+ VIR_FREE(chain_pri_s);
return NULL;
}
@@ -2914,6 +2943,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">
@@ -879,7 +884,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>
12:22 p.m.
New subject: [libvirt] [PATCH V2 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.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 16 ++++++---
src/conf/nwfilter_conf.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
src/conf/nwfilter_conf.h | 3 +
3 files changed, 90 insertions(+), 8 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2005,6 +2005,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 bool
+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_INTERNAL_ERROR,
+ _("Chain name contains illegal characters"));
+ return false;
+ }
+
+ if (strlen(chainname) > MAX_CHAIN_SUFFIX_SIZE) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("name of chain is longer than %u
characters"),
+ MAX_CHAIN_SUFFIX_SIZE);
+ return false;
+ }
+
+ 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 true;
+ if (STRPREFIX(chainname, name))
+ return false;
+ }
+ if (STRPREFIX(chainname, name))
+ return true;
+ }
+
+ 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_INTERNAL_ERROR, _("%s"), msg);
+ VIR_FREE(msg);
+
+err_exit:
+ return false;
+}
static virNWFilterDefPtr
virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
@@ -2050,11 +2124,8 @@ virNWFilterDefParseXML(xmlXPathContextPt
chain = virXPathString("string(./@chain)", ctxt);
if (chain) {
- if (virNWFilterChainSuffixTypeFromString(chain) < 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown chain suffix '%s'"),
chain);
+ if (virNWFilterIsAllowedChain(chain) != true)
goto cleanup;
- }
ret->chainsuffix = chain;
if (chain_pri_s) {
Index: libvirt-acl/src/conf/nwfilter_conf.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -444,6 +444,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>
12:22 p.m.
New subject: [libvirt] [PATCH V2 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
@@ -1895,7 +1895,7 @@ virNWFilterRuleParse(xmlNodePtr node)
char *statematch;
int found;
int found_i = 0;
- unsigned int priority;
+ int priority;
xmlNodePtr cur;
virNWFilterRuleDefPtr ret;
@@ -1941,8 +1941,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
@@ -355,7 +355,7 @@ enum virNWFilterEbtablesTableType {
};
-# define MIN_RULE_PRIORITY 0
+# define MIN_RULE_PRIORITY -1000
# define MAX_RULE_PRIORITY 1000
# define NWFILTER_MIN_FILTER_PRIORITY -1000
@@ -387,10 +387,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
@@ -372,7 +372,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
const char *neededChain,
virNWFilterChainPriority chainPriority,
char chainprefix,
- unsigned int priority,
+ virNWFilterRulePriority priority,
enum RuleType ruleType)
{
ebiptablesRuleInstPtr inst;
12:22 p.m.
New subject: [libvirt] [PATCH V2 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.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 114 ++++++++++++++++++++++++++----
1 file changed, 101 insertions(+), 13 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
@@ -2815,13 +2815,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;
@@ -2830,15 +2834,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),
@@ -2847,6 +2858,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;
}
@@ -3320,9 +3349,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
@@ -3392,9 +3446,12 @@ 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;
+ virNWFilterChainPriority *priority;
if (virHashSize(chains) != 0) {
char **filter_names;
@@ -3412,13 +3469,19 @@ ebtablesCreateTmpRootAndSubChains(virBuf
filter_names[i]);
if ((int)idx < 0)
continue;
- ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
- filter_names[i], 1);
+ priority = virHashLookup(chains, filter_names[i]);
+ if (ebtablesCreateTmpSubChain(inst, nRuleInstances,
+ direction, ifname, idx,
+ filter_names[i], 1,
+ *priority)) {
+ goto exit_free_keys;
+ }
}
+exit_free_keys:
virHashFreeKeys(chains, (void **)filter_names);
}
- err_exit:
+err_exit:
return rc;
}
@@ -3428,7 +3491,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;
@@ -3436,6 +3499,8 @@ ebiptablesApplyNewRules(virConnectPtr co
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();
@@ -3443,7 +3508,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++) {
@@ -3477,18 +3543,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);
@@ -3502,6 +3583,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;
@@ -3580,6 +3666,7 @@ ebiptablesApplyNewRules(virConnectPtr co
virHashFree(chains_in_set);
virHashFree(chains_out_set);
+ VIR_FREE(ebtChains);
return 0;
@@ -3618,6 +3705,7 @@ tear_down_tmpebchains:
exit_free_sets:
virHashFree(chains_in_set);
virHashFree(chains_out_set);
+ VIR_FREE(ebtChains);
return 1;
}
12:22 p.m.
New subject: [libvirt] [PATCH V2 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);
}
2:25 p.m.
On 10/19/2011 03:02 PM, David Stevens wrote:
Stefan,
Can't you achieve the same thing by reserving an early block
of priorities (and a late one, for system stuff that should be done late)?
The
problem is that at the moment rules (in the 'root' table) can have
priorities [0, 1000]. So nothing prevents one to write a rule with
priority 0. However, due to how nwfilters works right now the jumps into
the protocol-specific tables will always be created *before* those
rules. I am trying to address this now with assigning negative numbers
to the chains to achieve the same sorting and maintain backwards
compatibility.
If you use negative numbers, then you lose the
capability of
ever extending priorities to interpret the negative number as "from
the end"
as done in ebtables/iptables line numbers. I think that is more
useful, and
having to do that outside of priorities would mean extra parsing and
encoding
to get that effect.
There is no relation between priorities and the ordering
parameter to
the ebtables / iptables commands. The priorities were introduced so that
more complex filters can be built by composing them of individual
filters and yet have their filtering rules be created in the 'proper'
order that goes beyond of how they are reference through filter
references inside the filters and their appearance in the XML.
I don't see how this could be changed, but I'd be curious to see 'how'.
I also think that nwfilters ought to reflect the
underlying filter
mechanisms as much as possible. Really, I'd prefer they were simply
parameterized shell scripts of ebtables/iptables commands run at
significant
events (start-up, shutdown, migrate) instead of XML-encoded things. Then
the full feature sets of ebtables/iptables would be available "for
free", instead
of requiring libvirt patches to, e.g., add "return/continue" or
multiple chains.
If you want to design another filtering subsytem for libvirt,
please go
ahead. mwfilters currently works with XML and I don't see we can change
that so easily.
Barring that, at least I think what nwfilters provides
should be a close
map to ebtables/iptables capabilities. Mapping line numbers into a
wide range
It was not intended to be a 1:1 mapping but allow portability to other
system. Of course, the lack of similar functionality on other system may
be quite a bit of work to overcome first.
of priorities is straightforward, but if you use negative numbers in
an ordinary
sort, you can no longer use the sign as ebtables/iptables does. Because
you've limited the range, you could do something hacky with offsets
(anything
below "-1000" is "from the end" or some such), but that's arcane.
Right, 'arcane'...
Using priorities in multiple places is very like
programming in basic
and what both ebtables/iptables and nwfilters could use better I think
would
be the capability to label rules by name and reference the label to
identify the
rule location. Then you might, e.g., add a rule at "myrules + 5" and
don't care
what particular priority/line number "myrules" is.
You have patches for
that?
+-DLS
4:23 p.m.
On 10/19/2011 04:55 PM, David Stevens wrote:
-----Stefan Berger <stefanb(a)linux.vnet.ibm.com> wrote: -----
>
> The problem is that at the moment rules (in the 'root' table) can
> have priorities [0, 1000]. So nothing prevents one to write a
>rule
> with priority 0. However, due to how nwfilters works right now
>the
> jumps into the protocol-specific tables will always be created
> *before* those rules. I am trying to address this now with
>assigning
> negative numbers to the chains to achieve the same sorting and
> maintain backwards compatibility.
Priorities aren't directly line numbers, but I think extending
negative
numbers to be "from the end" as ebtables/iptables would be a useful
thing (and less arcane due to the precedent already there).
But since you are extending to allow more chains, couldn't you
in fact just expose the root chain to XML and create a default that
It's
already exposed.. you can add the following rule to the
'clean-traffic' filter
<rule action='drop' direction='inout' priority='999'>
<arp/>
</rule>
and it will appear in the interface root chain.
does the protocol switch that's generated in code now? Then, you
The protocol 'switch', assuming we both talk about 'the same',
becomes a
lot more flexible and dynamic with this patch series. It lets you insert
jumps to new chains (whose names have known protocol names as prefixes)
and interleave them with rules in the 'root' table.
could insert new chains before the standard ones, reorder, or do
anything you want -- with "RETURN" as the policy or action for acceptable
frames in the early chains so later chains are still run.
I do have the intention
to accept your 'RETURN' and 'CONTINUE' patch. So
that's not going to be the problem. I just have some concerns about how
the multiple IP address support is implemented. I don't think it works
with multiples of anything else than what is in the IP variable, i.e.,
comma-separated 'items' in other variables.
If you didn't want to expose libvirt-{I,O}-vnetXX chains,
couldn't you
at least accomplish the same thing with new chains in the code-generated
chains by adding a "pre-protocol" chain assuming "RETURN" is
available.
Then, a filter that adds to "pre-protocol" (or whatever you call it)
would always
happen before the standard chains. A similar chain hook could be done for
post-protocol chains, if those were converted to do "RETURN" instead of
ACCEPT/DROP too.
> Using priorities in multiple places is very
> like programming in basic
>
> and what both ebtables/iptables and nwfilters could use
>better
> I think would
>
> be the capability to label rules by name and reference d
> label to identify the
>
> rule location. Then you might, e.g., add a rule at "myrules
>+
> 5" and don't care
>
> what particular priority/line number "myrules" is.
>
>
>
> You have patches for that?
Actually, you do. If the labels are new chains then
the existing priority within that chain is essentially the same. But the
existing priority mechanism doesn't have the full flexibility available in
ebtables/iptables with "insert before", "append", "offset from
beginning",
"offset from end". Wouldn't you get that with the combination of
user-defined
chains, a user-modifiable root chain, and priorities interpretted like
this:
Priorities are used for sorting and not for absolute position of a rule
within a chain. You can compose filters of building blocks and leave
some building blocks out and you'll end up missing rules with certain
priorities. So the mapping to positions from priorities directly is not
possible in that way.
Stefan
0 - insert at beginning
N - insert at N
MAX - append at end (and this ought to be 2G-1, not 1000)
-N - insert at END-N.
?
+-DLS
2:42 p.m.
2011/10/19 David Stevens <dlstevens(a)us.ibm.com>:
I also think that nwfilters ought to reflect the
underlying
filter
mechanisms as much as possible. Really, I'd prefer they were simply
parameterized shell scripts of ebtables/iptables commands run at significant
events (start-up, shutdown, migrate) instead of XML-encoded things. Then
the full feature sets of ebtables/iptables would be available "for free",
instead
of requiring libvirt patches to, e.g., add "return/continue" or multiple
chains.
Well, you miss the point that nwfilters is meant as a general firewall
interface. ebtables/iptables just happens to be an implementation of
this interface. Using ebtables/iptables specific shell scripts would
replace the generic interface with something specific to
ebtables/iptables.
The general nwfilters interface allows to have an ebtables/iptables
based implementation for the Linux based hypervisors and an ipfw based
implementation for FreeBSD and other implementations that are specific
to VirtualBox, ESX, Hyper-V, PowerHypervisor etc. How well the
nwfilters interface maps to all those different firewalls is another
question, but with this general interface there is at least the
possibility to configure the different firewalls of different
hypervisors via libvirt.
--
Matthias Bolte
http://photron.blogspot.com
4:39 p.m.
2011/10/19 David Stevens <dlstevens(a)us.ibm.com>:
-----Matthias Bolte <matthias.bolte(a)googlemail.com> wrote: -----
>
>Well, you miss the point that nwfilters is meant as a general
>firewall
>interface. ebtables/iptables just happens to be an implementation of
>this interface. Using ebtables/iptables specific shell scripts would
>replace the generic interface with something specific to
>ebtables/iptables.
No, I just don't agree with it. I think an administrator on OS
"X"
is already familiar with the firewall capabilities on his/her OS and so
having
a new, less-capable abstraction instead of the firewall s/he already knows
is not a benefit. If these were instead hooks in libvirt that called sample
scripts
per-OS, administrators could easily do whatever they want to do when an
interface is brought up, brought down, or migrated. They could then also
make full use of their firewall capabilities and customize completely as
needed.
The goal of libvirt is to give you an interface that is not specific
to the underlying hypervisor, firewall, storage etc.
In case of your suggestion about firewall specific scripts that are
triggered by libvirt on specific events, this is already possible.
Libvirt provides you with such hooks: http://libvirt.org/hooks.html
--
Matthias Bolte
http://photron.blogspot.com
3:09 a.m.
Hi David,
<font face="Default Sans
Serif,Verdana,Arial,Helvetica,sans-serif"
size="2"><div><br></div><font
color="#990099">-----Matthias Bolte <a
class="moz-txt-link-rfc2396E"
href="mailto:matthias.bolte@googlemail.com"><matthias.bolte@googlemail.com></a>
wrote:
-----<br></font><br>><br>>Well, you miss the point that nwfilters is meant as a general<br>>firewall<br>>interface. ebtables/iptables just happens to be an implementation of<br>>this interface. Using ebtables/iptables specific shell scripts would<br>>replace the generic interface with something specific to<br>>ebtables/iptables.<br><br>
...
Could you please configure your mail client to stop sending this HTML garbage
or at least make it send plain text together with HTML?
Thanks,
Jirka
3:39 a.m.
On Wed, Oct 19, 2011 at 03:14:20PM -0600, David Stevens wrote:
-----Matthias Bolte [1]<matthias.bolte(a)googlemail.com>
wrote: -----
>
>Well, you miss the point that nwfilters is meant as a general
>firewall
>interface. ebtables/iptables just happens to be an implementation of
>this interface. Using ebtables/iptables specific shell scripts would
>replace the generic interface with something specific to
>ebtables/iptables.
No, I just don't agree with it. I think an administrator on OS
"X"
is already familiar with the firewall capabilities on his/her OS and so
having
a new, less-capable abstraction instead of the firewall s/he already knows
is not a benefit. If these were instead hooks in libvirt that called
sample scripts
per-OS, administrators could easily do whatever they want to do when an
interface is brought up, brought down, or migrated. They could then also
make full use of their firewall capabilities and customize completely as
needed.
Whether you agree with it or not is irrelevant for libvirt patch review
discussions. The abstraction into a implementation independant syntax &
API is the primary reason for libvirt's existance, and is not up for
debate.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4784
days inactive
4785
days old
19 comments
5 participants
participants (5)
-
Daniel P. Berrange -
David Stevens -
Jiri Denemark -
Matthias Bolte -
Stefan Berger