acl-plugin: optimize hash memory usage + fix the startup config parsing for memory...

In a couple of places vec_add1()-style was repeatedly called in a loop for
smallish vectors where the number of additions was known in advance.
With a test with large number of ACEs these numbers contribute to heap
fragmentation noticeably.

Minimize the number of allocations by preallocating the known size and
then resetting the length accordingly, and then calling vec_add1()

Also unify the parsing of the memory-related startup config parameters.

Change-Id: If8fba344eb1dee8f865ffe7b396ca3b6bd9dc1d0
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>

diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c
index 93d7305..d4cbeb2 100644 (file)
--- a/src/plugins/acl/acl.c
+++ b/src/plugins/acl/acl.c
@@ -3470,12 +3470,12 @@ acl_plugin_config (vlib_main_t * vm, unformat_input_t * input)
 {
   acl_main_t *am = &acl_main;
   u32 conn_table_hash_buckets;
-  u32 conn_table_hash_memory_size;
+  uword conn_table_hash_memory_size;
   u32 conn_table_max_entries;
   uword main_heap_size;
   uword hash_heap_size;
   u32 hash_lookup_hash_buckets;
-  u32 hash_lookup_hash_memory;
+  uword hash_lookup_hash_memory;
   u32 reclassify_sessions;
   u32 use_tuple_merge;
   u32 tuple_merge_split_threshold;
@@ -3485,8 +3485,10 @@ acl_plugin_config (vlib_main_t * vm, unformat_input_t * input)
       if (unformat
          (input, "connection hash buckets %d", &conn_table_hash_buckets))
        am->fa_conn_table_hash_num_buckets = conn_table_hash_buckets;
-      else if (unformat (input, "connection hash memory %d",
-                        &conn_table_hash_memory_size))
+      else
+       if (unformat
+           (input, "connection hash memory %U", unformat_memory_size,
+            &conn_table_hash_memory_size))
        am->fa_conn_table_hash_memory_size = conn_table_hash_memory_size;
       else if (unformat (input, "connection count max %d",
                         &conn_table_max_entries))
@@ -3504,8 +3506,10 @@ acl_plugin_config (vlib_main_t * vm, unformat_input_t * input)
       else if (unformat (input, "hash lookup hash buckets %d",
                         &hash_lookup_hash_buckets))
        am->hash_lookup_hash_buckets = hash_lookup_hash_buckets;
-      else if (unformat (input, "hash lookup hash memory %d",
-                        &hash_lookup_hash_memory))
+      else
+       if (unformat
+           (input, "hash lookup hash memory %U", unformat_memory_size,
+            &hash_lookup_hash_memory))
        am->hash_lookup_hash_memory = hash_lookup_hash_memory;
       else if (unformat (input, "use tuple merge %d", &use_tuple_merge))
        am->use_tuple_merge = use_tuple_merge;

diff --git a/src/plugins/acl/acl.h b/src/plugins/acl/acl.h
index 1d1ee44..ef2f25a 100644 (file)
--- a/src/plugins/acl/acl.h
+++ b/src/plugins/acl/acl.h
@@ -142,7 +142,7 @@ typedef struct {
   hash_acl_info_t *hash_acl_infos; /* corresponding hash matching housekeeping info */
   clib_bihash_48_8_t acl_lookup_hash; /* ACL lookup hash table. */
   u32 hash_lookup_hash_buckets;
-  u32 hash_lookup_hash_memory;
+  uword hash_lookup_hash_memory;
 
   /* mheap to hold all the miscellaneous allocations related to hash-based lookups */
   void *hash_lookup_mheap;

diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c
index aeec004..0568a67 100644 (file)
--- a/src/plugins/acl/hash_lookup.c
+++ b/src/plugins/acl/hash_lookup.c
@@ -603,6 +603,17 @@ hash_acl_set_heap(acl_main_t *am)
     am->hash_lookup_mheap = mheap_alloc_with_lock (0 /* use VM */ , 
                                                    am->hash_lookup_mheap_size,
                                                    1 /* locked */);
+#if USE_DLMALLOC != 0
+    /*
+     * DLMALLOC is being "helpful" in that it ignores the heap size parameter
+     * by default and tries to allocate the larger amount of memory.
+     *
+     * Pin the heap so this does not happen and if we run out of memory
+     * in this heap, we will bail out with "out of memory", rather than
+     * an obscure error sometime later.
+     */
+    mspace_disable_expand(am->hash_lookup_mheap);
+#endif
     if (0 == am->hash_lookup_mheap) {
         clib_error("ACL plugin failed to allocate lookup heap of %U bytes", 
                    format_memory_size, am->hash_lookup_mheap_size);
@@ -736,6 +747,12 @@ hash_acl_apply(acl_main_t *am, u32 lc_index, int acl_index, u32 acl_position)
 
 
   vec_validate(am->hash_applied_mask_info_vec_by_lc_index, lc_index);
+
+  /* since we know (in case of no split) how much we expand, preallocate that space */
+  int old_vec_len = vec_len(*applied_hash_aces);
+  vec_validate((*applied_hash_aces), old_vec_len + vec_len(ha->rules) - 1);
+  _vec_len((*applied_hash_aces)) = old_vec_len;
+
   /* 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++) {
     /*
@@ -1171,6 +1188,11 @@ void hash_acl_add(acl_main_t *am, int acl_index)
 
   /* walk the newly added ACL entries and ensure that for each of them there
      is a mask type, increment a reference count for that mask type */
+
+  /* avoid small requests by preallocating the entire vector before running the additions */
+  vec_validate(ha->rules, a->count-1);
+  vec_reset_length(ha->rules);
+
   for(i=0; i < a->count; i++) {
     hash_ace_info_t ace_info;
     fa_5tuple_t mask;