acl-plugin: rework the optimization 7383, fortify acl-plugin memory behavior (VPP-910)

The further prolonged testing from testbed that reported VPP-910
has uncovered a couple of deeper issues with optimization from
7384, and the usage of subscripts rather than vec_elt_at_index()
allowed to hide a couple of further errors in the code.
Also, the current acl-plugin behavior of using the global
heap for its dynamic data is problematic - it makes
the troubleshooting much harder by potentially spreading
the problem around.

Based on this experience, this commits makes a few changes to fix
the issues seen, also improving the serviceability of the acl-plugin
code for the future:

- Use separate mheaps for any ACL-related control plane
operations and separate for the hash lookup datastructures,
to compartmentalize any memory-related issues for the ACL plugin.

- Ensure vec_elt_at_index() usage throughout the hash_lookup.c file.

- Use vectors rather than raw memory for storing the "ordinary" ACL rules.

- Rework the optimization from 7384 to use a separate tail pointer
rather than overloading the "prev" field.

- Make get_session_ptr() more conservative and adjust is_valid_session_ptr
accordingly

Change-Id: Ifda85193f361de5ed3782a4acd39622bd33c5830
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit bd9c5ffe39e9ce61db95d74d150e07d738f24da1)

1 files changed, 151 insertions, 102 deletions

diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c
index 027dabd0e6a..a337eb84586 100644
--- a/src/plugins/acl/hash_lookup.c
+++ b/src/plugins/acl/hash_lookup.c
@@ -33,6 +33,16 @@
 #include "hash_lookup.h"
 #include "hash_lookup_private.h"
 
+
+static inline applied_hash_ace_entry_t **get_applied_hash_aces(acl_main_t *am, int is_input, u32 sw_if_index)
+{
+  applied_hash_ace_entry_t **applied_hash_aces = is_input ? vec_elt_at_index(am->input_hash_entry_vec_by_sw_if_index, sw_if_index)
+                                                          : vec_elt_at_index(am->output_hash_entry_vec_by_sw_if_index, sw_if_index);
+  return applied_hash_aces;
+}
+
+
+
 /*
  * This returns true if there is indeed a match on the portranges.
  * With all these levels of indirections, this is not going to be very fast,
@@ -41,11 +51,10 @@
 static int
 match_portranges(acl_main_t *am, fa_5tuple_t *match, u32 index)
 {
-  applied_hash_ace_entry_t **applied_hash_aces = match->pkt.is_input ? &am->input_hash_entry_vec_by_sw_if_index[match->pkt.sw_if_index] :
-                                                    &am->output_hash_entry_vec_by_sw_if_index[match->pkt.sw_if_index];
-  applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[index]);
 
-  // hash_acl_info_t *ha = &am->hash_acl_infos[pae->acl_index];
+  applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, match->pkt.is_input, match->pkt.sw_if_index);
+  applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), index);
+
   acl_rule_t *r = &(am->acls[pae->acl_index].rules[pae->ace_index]);
   DBG("PORTMATCH: %d <= %d <= %d && %d <= %d <= %d ?",
 		r->src_port_or_type_first, match->l4.port[0], r->src_port_or_type_last,
@@ -70,8 +79,7 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match)
 
   u32 sw_if_index = match->pkt.sw_if_index;
   u8 is_input = match->pkt.is_input;
-  applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] :
-                                                    &am->output_hash_entry_vec_by_sw_if_index[sw_if_index];
+  applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index);
   applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index :
                                                     &am->output_applied_hash_acl_info_by_sw_if_index;
 
@@ -79,11 +87,11 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match)
 	       pmatch[0], pmatch[1], pmatch[2], pmatch[3], pmatch[4], pmatch[5]);
 
   for(mask_type_index=0; mask_type_index < pool_len(am->ace_mask_type_pool); mask_type_index++) {
-    if (!clib_bitmap_get((*applied_hash_acls)[sw_if_index].mask_type_index_bitmap, mask_type_index)) {
+    if (!clib_bitmap_get(vec_elt_at_index((*applied_hash_acls), sw_if_index)->mask_type_index_bitmap, mask_type_index)) {
       /* This bit is not set. Avoid trying to match */
       continue;
     }
-    ace_mask_type_entry_t *mte = &am->ace_mask_type_pool[mask_type_index];
+    ace_mask_type_entry_t *mte = vec_elt_at_index(am->ace_mask_type_pool, mask_type_index);
     pmatch = (u64 *)match;
     pmask = (u64 *)&mte->mask;
     pkey = (u64 *)kv.key;
@@ -120,7 +128,7 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match)
           u32 curr_index = result_val->applied_entry_index;
           while ((curr_index != ~0) && !match_portranges(am, match, curr_index)) {
             /* while no match and there are more entries, walk... */
-            applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[curr_index]);
+            applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces),curr_index);
             DBG("entry %d did not portmatch, advancing to %d", curr_index, pae->next_applied_entry_index);
             curr_index = pae->next_applied_entry_index;
           }
@@ -153,9 +161,9 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match)
 static void
 hashtable_add_del(acl_main_t *am, clib_bihash_kv_48_8_t *kv, int is_add)
 {
-    DBG("HASH ADD/DEL: %016llx %016llx %016llx %016llx %016llx %016llx add %d",
+    DBG("HASH ADD/DEL: %016llx %016llx %016llx %016llx %016llx %016llx %016llx add %d",
                         kv->key[0], kv->key[1], kv->key[2],
-                        kv->key[3], kv->key[4], kv->key[5], is_add);
+                        kv->key[3], kv->key[4], kv->key[5], kv->value, is_add);
     BV (clib_bihash_add_del) (&am->acl_lookup_hash, kv, is_add);
 }
 
@@ -167,17 +175,17 @@ fill_applied_hash_ace_kv(acl_main_t *am,
 {
   fa_5tuple_t *kv_key = (fa_5tuple_t *)kv->key;
   hash_acl_lookup_value_t *kv_val = (hash_acl_lookup_value_t *)&kv->value;
-  applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]);
-  hash_acl_info_t *ha = &am->hash_acl_infos[pae->acl_index];
+  applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index);
+  hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, pae->acl_index);
 
-  memcpy(kv_key, &ha->rules[pae->hash_ace_info_index].match, sizeof(*kv_key));
+  memcpy(kv_key, &(vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->match), sizeof(*kv_key));
   /* initialize the sw_if_index and direction */
   kv_key->pkt.sw_if_index = sw_if_index;
   kv_key->pkt.is_input = is_input;
   kv_val->as_u64 = 0;
   kv_val->applied_entry_index = new_index;
-  kv_val->need_portrange_check = ha->rules[pae->hash_ace_info_index].src_portrange_not_powerof2 ||
-				   ha->rules[pae->hash_ace_info_index].dst_portrange_not_powerof2;
+  kv_val->need_portrange_check = vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->src_portrange_not_powerof2 ||
+				   vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->dst_portrange_not_powerof2;
   /* by default assume all values are shadowed -> check all mask types */
   kv_val->shadowed = 1;
 }
@@ -203,7 +211,9 @@ activate_applied_ace_hash_entry(acl_main_t *am,
                             u32 new_index)
 {
   clib_bihash_kv_48_8_t kv;
-  applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]);
+  ASSERT(new_index != ~0);
+  applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index);
+  DBG("activate_applied_ace_hash_entry sw_if_index %d is_input %d new_index %d", sw_if_index, is_input, new_index);
 
   fill_applied_hash_ace_kv(am, applied_hash_aces, sw_if_index, is_input, new_index, &kv);
 
@@ -214,28 +224,28 @@ activate_applied_ace_hash_entry(acl_main_t *am,
   clib_bihash_kv_48_8_t result;
   hash_acl_lookup_value_t *result_val = (hash_acl_lookup_value_t *)&result.value;
   int res = BV (clib_bihash_search) (&am->acl_lookup_hash, &kv, &result);
+  ASSERT(new_index != ~0);
+  ASSERT(new_index < vec_len((*applied_hash_aces)));
   if (res == 0) {
     /* There already exists an entry or more. Append at the end. */
     u32 first_index = result_val->applied_entry_index;
-    DBG("A key already exists, with applied entry index: %d", existing_index);
     ASSERT(first_index != ~0);
-    applied_hash_ace_entry_t *first_pae = &((*applied_hash_aces)[first_index]);
-    /* move to "prev" by one, this should land us in the end of the list */
-    u32 last_index = first_pae->prev_applied_entry_index;
+    DBG("A key already exists, with applied entry index: %d", first_index);
+    applied_hash_ace_entry_t *first_pae = vec_elt_at_index((*applied_hash_aces), first_index);
+    u32 last_index = first_pae->tail_applied_entry_index;
     ASSERT(last_index != ~0);
-    applied_hash_ace_entry_t *last_pae = &((*applied_hash_aces)[last_index]);
-    ASSERT(last_pae->next_applied_entry_index == ~0);
+    applied_hash_ace_entry_t *last_pae = vec_elt_at_index((*applied_hash_aces), last_index);
+    DBG("...advance to chained entry index: %d", last_index);
     /* link ourseves in */
     last_pae->next_applied_entry_index = new_index;
     pae->prev_applied_entry_index = last_index;
-    /* make a new reference from the very first element to new tail */
-    first_pae->prev_applied_entry_index = new_index;
+    /* adjust the pointer to the new tail */
+    first_pae->tail_applied_entry_index = new_index;
   } else {
     /* It's the very first entry */
     hashtable_add_del(am, &kv, 1);
-    pae->is_first_entry = 1;
-    /* we are the tail */
-    pae->prev_applied_entry_index = new_index;
+    ASSERT(new_index != ~0);
+    pae->tail_applied_entry_index = new_index;
   }
 }
 
@@ -254,31 +264,44 @@ applied_hash_entries_analyze(acl_main_t *am, applied_hash_ace_entry_t **applied_
    */
 }
 
+static void *
+hash_acl_set_heap(acl_main_t *am)
+{
+  if (0 == am->hash_lookup_mheap) {
+    am->hash_lookup_mheap = mheap_alloc (0 /* use VM */ , 2 << 25);
+  }
+  void *oldheap = clib_mem_set_heap(am->hash_lookup_mheap);
+  return oldheap;
+}
+
 void
 hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
 {
   int i;
 
   DBG("HASH ACL apply: sw_if_index %d is_input %d acl %d", sw_if_index, is_input, acl_index);
-  u32 *acl_vec = is_input ? am->input_acl_vec_by_sw_if_index[sw_if_index] :
-			    am->output_acl_vec_by_sw_if_index[sw_if_index];
+  if (!am->acl_lookup_hash_initialized) {
+    BV (clib_bihash_init) (&am->acl_lookup_hash, "ACL plugin rule lookup bihash",
+                           65536, 2 << 25);
+    am->acl_lookup_hash_initialized = 1;
+  }
+
+  u32 *acl_vec = is_input ? *vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index)
+                          : *vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index);
+
+  void *oldheap = hash_acl_set_heap(am);
   if (is_input) {
     vec_validate(am->input_hash_entry_vec_by_sw_if_index, sw_if_index);
   } else {
     vec_validate(am->output_hash_entry_vec_by_sw_if_index, sw_if_index);
   }
   vec_validate(am->hash_acl_infos, acl_index);
-  applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] :
-                                                    &am->output_hash_entry_vec_by_sw_if_index[sw_if_index];
+  applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index);
+
   u32 order_index = vec_search(acl_vec, acl_index);
-  hash_acl_info_t *ha = &am->hash_acl_infos[acl_index];
+  hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index);
   ASSERT(order_index != ~0);
 
-  if (!am->acl_lookup_hash_initialized) {
-    BV (clib_bihash_init) (&am->acl_lookup_hash, "ACL plugin rule lookup bihash",
-                           65536, 2 << 25);
-    am->acl_lookup_hash_initialized = 1;
-  }
   int base_offset = vec_len(*applied_hash_aces);
 
   /* Update the bitmap of the mask types with which the lookup
@@ -286,7 +309,7 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
   applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index :
                                                     &am->output_applied_hash_acl_info_by_sw_if_index;
   vec_validate((*applied_hash_acls), sw_if_index);
-  applied_hash_acl_info_t *pal = &(*applied_hash_acls)[sw_if_index];
+  applied_hash_acl_info_t *pal = vec_elt_at_index((*applied_hash_acls), sw_if_index);
   pal->mask_type_index_bitmap = clib_bitmap_or(pal->mask_type_index_bitmap,
                                      ha->mask_type_index_bitmap);
   /*
@@ -305,8 +328,7 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
   /* add the rules from the ACL to the hash table for lookup and append to the vector*/
   for(i=0; i < vec_len(ha->rules); i++) {
     u32 new_index = base_offset + i;
-    applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]);
-    pae->is_first_entry = 0;
+    applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index);
     pae->acl_index = acl_index;
     pae->ace_index = ha->rules[i].ace_index;
     pae->action = ha->rules[i].action;
@@ -314,9 +336,29 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
     /* we might link it in later */
     pae->next_applied_entry_index = ~0;
     pae->prev_applied_entry_index = ~0;
+    pae->tail_applied_entry_index = ~0;
     activate_applied_ace_hash_entry(am, sw_if_index, is_input, applied_hash_aces, new_index);
   }
   applied_hash_entries_analyze(am, applied_hash_aces);
+  clib_mem_set_heap (oldheap);
+}
+
+static u32
+find_head_applied_ace_index(applied_hash_ace_entry_t **applied_hash_aces, u32 curr_index)
+{
+  /*
+   * find back the first entry. Inefficient so might need to be a bit cleverer
+   * if this proves to be a problem..
+   */
+  u32 an_index = curr_index;
+  ASSERT(an_index != ~0);
+  applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), an_index);
+  while(head_pae->prev_applied_entry_index != ~0) {
+    an_index = head_pae->prev_applied_entry_index;
+    ASSERT(an_index != ~0);
+    head_pae = vec_elt_at_index((*applied_hash_aces), an_index);
+  }
+  return an_index;
 }
 
 static void
@@ -325,43 +367,43 @@ move_applied_ace_hash_entry(acl_main_t *am,
                             applied_hash_ace_entry_t **applied_hash_aces,
                             u32 old_index, u32 new_index)
 {
+  ASSERT(old_index != ~0);
+  ASSERT(new_index != ~0);
   /* move the entry */
-  (*applied_hash_aces)[new_index] = (*applied_hash_aces)[old_index];
+  *vec_elt_at_index((*applied_hash_aces), new_index) = *vec_elt_at_index((*applied_hash_aces), old_index);
 
   /* update the linkage and hash table if necessary */
-  applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[old_index]);
+  applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), old_index);
 
-  if (!pae->is_first_entry) {
-    applied_hash_ace_entry_t *prev_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]);
+  if (pae->prev_applied_entry_index != ~0) {
+    applied_hash_ace_entry_t *prev_pae = vec_elt_at_index((*applied_hash_aces), pae->prev_applied_entry_index);
     ASSERT(prev_pae->next_applied_entry_index == old_index);
     prev_pae->next_applied_entry_index = new_index;
   } else {
     /* first entry - so the hash points to it, update */
     add_del_hashtable_entry(am, sw_if_index, is_input,
                             applied_hash_aces, new_index, 1);
+    ASSERT(pae->tail_applied_entry_index != ~0);
   }
   if (pae->next_applied_entry_index != ~0) {
-    applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]);
+    applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index);
     ASSERT(next_pae->prev_applied_entry_index == old_index);
     next_pae->prev_applied_entry_index = new_index;
   } else {
     /*
      * Moving the very last entry, so we need to update the tail pointer in the first one.
-     * find back the first entry. Inefficient so might need to be a bit cleverer
-     * if this proves to be a problem..
      */
-    u32 an_index = pae->prev_applied_entry_index;
-    applied_hash_ace_entry_t *head_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]);
-    while(!head_pae->is_first_entry) {
-      an_index = head_pae->prev_applied_entry_index;
-      head_pae = &((*applied_hash_aces)[an_index]);
-    }
-    ASSERT(head_pae->prev_applied_entry_index == old_index);
-    head_pae->prev_applied_entry_index = new_index;
+    u32 head_index = find_head_applied_ace_index(applied_hash_aces, old_index);
+    ASSERT(head_index != ~0);
+    applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), head_index);
+
+    ASSERT(head_pae->tail_applied_entry_index == old_index);
+    head_pae->tail_applied_entry_index = new_index;
   }
   /* invalidate the old entry */
   pae->prev_applied_entry_index = ~0;
   pae->next_applied_entry_index = ~0;
+  pae->tail_applied_entry_index = ~0;
 }
 
 static void
@@ -370,39 +412,37 @@ deactivate_applied_ace_hash_entry(acl_main_t *am,
                             applied_hash_ace_entry_t **applied_hash_aces,
                             u32 old_index)
 {
-  applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[old_index]);
-
-  if (pae->next_applied_entry_index != ~0) {
-    applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]);
-    ASSERT(next_pae->prev_applied_entry_index == old_index);
-    next_pae->prev_applied_entry_index = pae->prev_applied_entry_index;
-  }
+  applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), old_index);
+  DBG("UNAPPLY DEACTIVATE: sw_if_index %d is_input %d, applied index %d", sw_if_index, is_input, old_index);
 
-  if (!pae->is_first_entry) {
-    applied_hash_ace_entry_t *prev_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]);
+  if (pae->prev_applied_entry_index != ~0) {
+    DBG("UNAPPLY = index %d has prev_applied_entry_index %d", old_index, pae->prev_applied_entry_index);
+    applied_hash_ace_entry_t *prev_pae = vec_elt_at_index((*applied_hash_aces), pae->prev_applied_entry_index);
     ASSERT(prev_pae->next_applied_entry_index == old_index);
     prev_pae->next_applied_entry_index = pae->next_applied_entry_index;
     if (pae->next_applied_entry_index == ~0) {
       /* it was a last entry we removed, update the pointer on the first one */
-      u32 an_index = pae->prev_applied_entry_index;
-      applied_hash_ace_entry_t *head_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]);
-      while(!head_pae->is_first_entry) {
-	an_index = head_pae->prev_applied_entry_index;
-	head_pae = &((*applied_hash_aces)[an_index]);
-      }
-      ASSERT(head_pae->prev_applied_entry_index == old_index);
-      head_pae->prev_applied_entry_index = pae->prev_applied_entry_index;
+      u32 head_index = find_head_applied_ace_index(applied_hash_aces, old_index);
+      DBG("UNAPPLY = index %d head index to update %d", old_index, head_index);
+      ASSERT(head_index != ~0);
+      applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), head_index);
+
+      ASSERT(head_pae->tail_applied_entry_index == old_index);
+      head_pae->tail_applied_entry_index = pae->prev_applied_entry_index;
+    } else {
+      applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index);
+      next_pae->prev_applied_entry_index = pae->prev_applied_entry_index;
     }
   } else {
     /* It was the first entry. We need either to reset the hash entry or delete it */
-    pae->is_first_entry = 0;
     if (pae->next_applied_entry_index != ~0) {
-      /* a custom case of relinking for the first node, with the tail not forward linked to it */
-      applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]);
-      /* this is the tail the new head should be aware of */
-      next_pae->prev_applied_entry_index = pae->prev_applied_entry_index;
-      next_pae->is_first_entry = 1;
-
+      /* the next element becomes the new first one, so needs the tail pointer to be set */
+      applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index);
+      ASSERT(pae->tail_applied_entry_index != ~0);
+      next_pae->tail_applied_entry_index = pae->tail_applied_entry_index;
+      DBG("Resetting the hash table entry from %d to %d, setting tail index to %d", old_index, pae->next_applied_entry_index, pae->tail_applied_entry_index);
+      /* unlink from the next element */
+      next_pae->prev_applied_entry_index = ~0;
       add_del_hashtable_entry(am, sw_if_index, is_input,
                               applied_hash_aces, pae->next_applied_entry_index, 1);
     } else {
@@ -411,6 +451,10 @@ deactivate_applied_ace_hash_entry(acl_main_t *am,
                               applied_hash_aces, old_index, 0);
     }
   }
+  /* invalidate the old entry */
+  pae->prev_applied_entry_index = ~0;
+  pae->next_applied_entry_index = ~0;
+  pae->tail_applied_entry_index = ~0;
 }
 
 
@@ -419,14 +463,14 @@ hash_acl_build_applied_lookup_bitmap(acl_main_t *am, u32 sw_if_index, u8 is_inpu
 {
   int i;
   uword *new_lookup_bitmap = 0;
-  u32 **applied_acls = is_input ? &am->input_acl_vec_by_sw_if_index[sw_if_index] :
-                                  &am->output_acl_vec_by_sw_if_index[sw_if_index];
-  applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index :
-                                                    &am->output_applied_hash_acl_info_by_sw_if_index;
-  applied_hash_acl_info_t *pal = &(*applied_hash_acls)[sw_if_index];
+  u32 **applied_acls = is_input ? vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index)
+                                : vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index);
+  applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index
+                                                         : &am->output_applied_hash_acl_info_by_sw_if_index;
+  applied_hash_acl_info_t *pal = vec_elt_at_index((*applied_hash_acls), sw_if_index);
   for(i=0; i < vec_len(*applied_acls); i++) {
-    u32 a_acl_index = (*applied_acls)[i];
-    hash_acl_info_t *ha = &am->hash_acl_infos[a_acl_index];
+    u32 a_acl_index = *vec_elt_at_index((*applied_acls), i);
+    hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, a_acl_index);
     new_lookup_bitmap = clib_bitmap_or(new_lookup_bitmap,
                                        ha->mask_type_index_bitmap);
   }
@@ -441,12 +485,12 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
   int i;
 
   DBG("HASH ACL unapply: sw_if_index %d is_input %d acl %d", sw_if_index, is_input, acl_index);
-  hash_acl_info_t *ha = &am->hash_acl_infos[acl_index];
-  applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] :
-                                                    &am->output_hash_entry_vec_by_sw_if_index[sw_if_index];
+
+  hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index);
+  applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index);
 
   for(i=0; i < vec_len((*applied_hash_aces)); i++) {
-    if ((*applied_hash_aces)[i].acl_index == acl_index) {
+    if (vec_elt_at_index(*applied_hash_aces,i)->acl_index == acl_index) {
       DBG("Found applied ACL#%d at applied index %d", acl_index, i);
       break;
     }
@@ -456,13 +500,14 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
     /* we went all the way without finding any entries. Probably a list was empty. */
     return;
   }
+
+  void *oldheap = hash_acl_set_heap(am);
   int base_offset = i;
   int tail_offset = base_offset + vec_len(ha->rules);
   int tail_len = vec_len((*applied_hash_aces)) - tail_offset;
   DBG("base_offset: %d, tail_offset: %d, tail_len: %d", base_offset, tail_offset, tail_len);
 
   for(i=0; i < vec_len(ha->rules); i ++) {
-    DBG("UNAPPLY DEACTIVATE: sw_if_index %d is_input %d, applied index %d", sw_if_index, is_input, base_offset + i);
     deactivate_applied_ace_hash_entry(am, sw_if_index, is_input,
                                       applied_hash_aces, base_offset + i);
   }
@@ -479,6 +524,7 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
 
   /* After deletion we might not need some of the mask-types anymore... */
   hash_acl_build_applied_lookup_bitmap(am, sw_if_index, is_input);
+  clib_mem_set_heap (oldheap);
 }
 
 /*
@@ -493,8 +539,8 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
 void
 hash_acl_reapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
 {
-  u32 **applied_acls = is_input ? &am->input_acl_vec_by_sw_if_index[sw_if_index] :
-				  &am->output_acl_vec_by_sw_if_index[sw_if_index];
+  u32 **applied_acls = is_input ? vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index)
+                                : vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index);
   int i;
   int start_index = vec_search((*applied_acls), acl_index);
   /*
@@ -505,10 +551,10 @@ hash_acl_reapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index)
 
   /* unapply all the ACLs till the current one */
   for(i = vec_len(*applied_acls) - 1; i >= start_index; i--) {
-    hash_acl_unapply(am, sw_if_index, is_input, (*applied_acls)[i]);
+    hash_acl_unapply(am, sw_if_index, is_input, *vec_elt_at_index(*applied_acls, i));
   }
   for(i = start_index; i < vec_len(*applied_acls); i++) {
-    hash_acl_apply(am, sw_if_index, is_input, (*applied_acls)[i]);
+    hash_acl_apply(am, sw_if_index, is_input, *vec_elt_at_index(*applied_acls, i));
   }
 }
 
@@ -649,7 +695,7 @@ assign_mask_type_index(acl_main_t *am, fa_5tuple_t *mask)
 static void
 release_mask_type_index(acl_main_t *am, u32 mask_type_index)
 {
-  ace_mask_type_entry_t *mte = &am->ace_mask_type_pool[mask_type_index];
+  ace_mask_type_entry_t *mte = pool_elt_at_index(am->ace_mask_type_pool, mask_type_index);
   mte->refcount--;
   if (mte->refcount == 0) {
     /* we are not using this entry anymore */
@@ -659,11 +705,12 @@ release_mask_type_index(acl_main_t *am, u32 mask_type_index)
 
 void hash_acl_add(acl_main_t *am, int acl_index)
 {
+  void *oldheap = hash_acl_set_heap(am);
   DBG("HASH ACL add : %d", acl_index);
   int i;
   acl_list_t *a = &am->acls[acl_index];
   vec_validate(am->hash_acl_infos, acl_index);
-  hash_acl_info_t *ha = &am->hash_acl_infos[acl_index];
+  hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index);
   memset(ha, 0, sizeof(*ha));
 
   /* walk the newly added ACL entries and ensure that for each of them there
@@ -710,10 +757,12 @@ void hash_acl_add(acl_main_t *am, int acl_index)
       hash_acl_reapply(am, *sw_if_index, 0, acl_index);
     }
   }
+  clib_mem_set_heap (oldheap);
 }
 
 void hash_acl_delete(acl_main_t *am, int acl_index)
 {
+  void *oldheap = hash_acl_set_heap(am);
   DBG("HASH ACL delete : %d", acl_index);
   /*
    * If the ACL is applied somewhere, remove the references of it (call hash_acl_unapply)
@@ -739,12 +788,13 @@ void hash_acl_delete(acl_main_t *am, int acl_index)
   /* walk the mask types for the ACL about-to-be-deleted, and decrease
    * the reference count, possibly freeing up some of them */
   int i;
-  hash_acl_info_t *ha = &am->hash_acl_infos[acl_index];
+  hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index);
   for(i=0; i < vec_len(ha->rules); i++) {
     release_mask_type_index(am, ha->rules[i].mask_type_index);
   }
   clib_bitmap_free(ha->mask_type_index_bitmap);
   vec_free(ha->rules);
+  clib_mem_set_heap (oldheap);
 }
 
 u8
@@ -753,11 +803,10 @@ hash_multi_acl_match_5tuple (u32 sw_if_index, fa_5tuple_t * pkt_5tuple, int is_l
                        u32 * rule_match_p, u32 * trace_bitmap)
 {
   acl_main_t *am = &acl_main;
-  applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] :
-                                                    &am->output_hash_entry_vec_by_sw_if_index[sw_if_index];
+  applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index);
   u32 match_index = multi_acl_match_get_applied_ace_index(am, pkt_5tuple);
   if (match_index < vec_len((*applied_hash_aces))) {
-    applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[match_index]);
+    applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), match_index);
     *acl_match_p = pae->acl_index;
     *rule_match_p = pae->ace_index;
     return pae->action;