6:15 p.m.
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