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@linux.vnet.ibm.com>

---
 src/nwfilter/nwfilter_ebiptables_driver.c |  120 ++++++++++++++++++++++++++----
 1 file changed, 107 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;
 
@@ -3581,6 +3667,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:
@@ -3619,6 +3709,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;
 }