acl-plugin: use 16_8 bihash for IPv4 sessions and 40_8 bihash for IPv6 sessions

Add a new kv_16_8 field into 5tuple union, rename
the existing kv into kv_40_8 for clarity, and
add the compile-time alignment constraints.

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

7 files changed, 165 insertions, 84 deletions

diff --git a/src/plugins/acl/acl.h b/src/plugins/acl/acl.h
index 0c0a6db5af7..9d333da4059 100644
--- a/src/plugins/acl/acl.h
+++ b/src/plugins/acl/acl.h
@@ -26,6 +26,7 @@
 #include <vppinfra/elog.h>
 #include <vppinfra/bihash_48_8.h>
 #include <vppinfra/bihash_40_8.h>
+#include <vppinfra/bihash_16_8.h>
 
 #include "fa_node.h"
 #include "hash_lookup_types.h"
@@ -246,7 +247,8 @@ typedef struct {
   uword *fa_out_acl_on_sw_if_index;
   /* bihash holding all of the sessions */
   int fa_sessions_hash_is_initialized;
-  clib_bihash_40_8_t fa_sessions_hash;
+  clib_bihash_40_8_t fa_ip6_sessions_hash;
+  clib_bihash_16_8_t fa_ip4_sessions_hash;
   /* The process node which orcherstrates the cleanup */
   u32 fa_cleaner_node_index;
   /* FA session timeouts, in seconds */
diff --git a/src/plugins/acl/dataplane_node.c b/src/plugins/acl/dataplane_node.c
index dead2ec131e..a6c7b981ad5 100644
--- a/src/plugins/acl/dataplane_node.c
+++ b/src/plugins/acl/dataplane_node.c
@@ -82,7 +82,6 @@ acl_fa_node_fn (vlib_main_t * vm,
   u32 trace_bitmap = 0;
   acl_main_t *am = &acl_main;
   fa_5tuple_t fa_5tuple;
-  clib_bihash_kv_40_8_t value_sess;
   vlib_node_runtime_t *error_node;
   u64 now = clib_cpu_time_now ();
   uword thread_index = os_get_thread_index ();
@@ -153,14 +152,16 @@ acl_fa_node_fn (vlib_main_t * vm,
 
       if (acl_fa_ifc_has_sessions (am, sw_if_index0))
 	{
-	  if (acl_fa_find_session (am, sw_if_index0, &fa_5tuple, &value_sess)
-	      && (value_sess.value != ~0ULL))
+	  u64 value_sess = ~0ULL;
+	  if (acl_fa_find_session
+	      (am, is_ip6, sw_if_index0, &fa_5tuple, &value_sess)
+	      && (value_sess != ~0ULL))
 	    {
 	      trace_bitmap |= 0x80000000;
 	      error0 = ACL_FA_ERROR_ACL_EXIST_SESSION;
 	      fa_full_session_id_t f_sess_id;
 
-	      f_sess_id.as_u64 = value_sess.value;
+	      f_sess_id.as_u64 = value_sess;
 	      ASSERT (f_sess_id.thread_index < vec_len (vlib_mains));
 
 	      fa_session_t *sess =
@@ -292,12 +293,12 @@ acl_fa_node_fn (vlib_main_t * vm,
 	  t->next_index = next0;
 	  t->match_acl_in_index = match_acl_in_index;
 	  t->match_rule_index = match_rule_index;
-	  t->packet_info[0] = fa_5tuple.kv.key[0];
-	  t->packet_info[1] = fa_5tuple.kv.key[1];
-	  t->packet_info[2] = fa_5tuple.kv.key[2];
-	  t->packet_info[3] = fa_5tuple.kv.key[3];
-	  t->packet_info[4] = fa_5tuple.kv.key[4];
-	  t->packet_info[5] = fa_5tuple.kv.value;
+	  t->packet_info[0] = fa_5tuple.kv_40_8.key[0];
+	  t->packet_info[1] = fa_5tuple.kv_40_8.key[1];
+	  t->packet_info[2] = fa_5tuple.kv_40_8.key[2];
+	  t->packet_info[3] = fa_5tuple.kv_40_8.key[3];
+	  t->packet_info[4] = fa_5tuple.kv_40_8.key[4];
+	  t->packet_info[5] = fa_5tuple.kv_40_8.value;
 	  t->action = action;
 	  t->trace_bitmap = trace_bitmap;
 	}
diff --git a/src/plugins/acl/fa_node.h b/src/plugins/acl/fa_node.h
index ba080446613..102922c2807 100644
--- a/src/plugins/acl/fa_node.h
+++ b/src/plugins/acl/fa_node.h
@@ -2,6 +2,7 @@
 #define _FA_NODE_H_
 
 #include <stddef.h>
+#include <vppinfra/bihash_16_8.h>
 #include <vppinfra/bihash_40_8.h>
 
 // #define FA_NODE_VERBOSE_DEBUG 3
@@ -66,10 +67,14 @@ typedef union {
       ip6_address_t ip6_addr[2];
     };
     fa_session_l4_key_t l4;
-    /* This field should align with u64 value in bihash_40_8 keyvalue struct */
+    /* This field should align with u64 value in bihash_40_8 and bihash_16_8 keyvalue struct */
     fa_packet_info_t pkt;
   };
-  clib_bihash_kv_40_8_t kv;
+  clib_bihash_kv_40_8_t kv_40_8;
+  struct {
+    u64 padding_for_kv_16_8[3];
+    clib_bihash_kv_16_8_t kv_16_8;
+  };
 } fa_5tuple_t;
 
 typedef struct {
@@ -121,9 +126,13 @@ typedef struct {
 
 #define CT_ASSERT_EQUAL(name, x,y) typedef int assert_ ## name ## _compile_time_assertion_failed[((x) == (y))-1]
 CT_ASSERT_EQUAL(fa_l3_key_size_is_40, offsetof(fa_5tuple_t, pkt), offsetof(clib_bihash_kv_40_8_t, value));
+CT_ASSERT_EQUAL(fa_ip6_kv_val_at_pkt, offsetof(fa_5tuple_t, pkt), offsetof(fa_5tuple_t, kv_40_8.value));
+CT_ASSERT_EQUAL(fa_ip4_kv_val_at_pkt, offsetof(fa_5tuple_t, pkt), offsetof(fa_5tuple_t, kv_16_8.value));
 CT_ASSERT_EQUAL(fa_l4_key_t_is_8, sizeof(fa_session_l4_key_t), sizeof(u64));
 CT_ASSERT_EQUAL(fa_packet_info_t_is_8, sizeof(fa_packet_info_t), sizeof(u64));
 CT_ASSERT_EQUAL(fa_l3_kv_size_is_48, sizeof(fa_5tuple_t), sizeof(clib_bihash_kv_40_8_t));
+CT_ASSERT_EQUAL(fa_ip4_starts_at_kv16_key, offsetof(fa_5tuple_t, ip4_addr), offsetof(fa_5tuple_t, kv_16_8));
+CT_ASSERT_EQUAL(fa_ip4_and_ip6_kv_value_match, offsetof(fa_5tuple_t, kv_16_8.value), offsetof(fa_5tuple_t, kv_40_8.value));
 
 /* Let's try to fit within two cachelines */
 CT_ASSERT_EQUAL(fa_session_t_size_is_128, sizeof(fa_session_t), 128);
diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c
index 4bcd9050c73..7e76794969d 100644
--- a/src/plugins/acl/hash_lookup.c
+++ b/src/plugins/acl/hash_lookup.c
@@ -804,8 +804,8 @@ acl_plugin_show_tables_mask_type (void)
     ({
       vlib_cli_output(vm, "     %3d: %016llx %016llx %016llx %016llx %016llx %016llx  refcount %d",
 		    mte - am->ace_mask_type_pool,
-		    mte->mask.kv.key[0], mte->mask.kv.key[1], mte->mask.kv.key[2],
-		    mte->mask.kv.key[3], mte->mask.kv.key[4], mte->mask.kv.value, mte->refcount);
+		    mte->mask.kv_40_8.key[0], mte->mask.kv_40_8.key[1], mte->mask.kv_40_8.key[2],
+		    mte->mask.kv_40_8.key[3], mte->mask.kv_40_8.key[4], mte->mask.kv_40_8.value, mte->refcount);
     }));
     /* *INDENT-ON* */
 }
diff --git a/src/plugins/acl/public_inlines.h b/src/plugins/acl/public_inlines.h
index 6b464b36b7a..6d8f0ddb125 100644
--- a/src/plugins/acl/public_inlines.h
+++ b/src/plugins/acl/public_inlines.h
@@ -208,8 +208,8 @@ acl_fill_5tuple (acl_main_t * am, vlib_buffer_t * b0, int is_ip6,
 
   /* key[0..3] contains src/dst address and is cleared/set below */
   /* Remainder of the key and per-packet non-key data */
-  p5tuple_pkt->kv.key[4] = 0;
-  p5tuple_pkt->kv.value = 0;
+  p5tuple_pkt->kv_40_8.key[4] = 0;
+  p5tuple_pkt->kv_40_8.value = 0;
   p5tuple_pkt->pkt.is_ip6 = is_ip6;
 
   if (is_ip6)
diff --git a/src/plugins/acl/sess_mgmt_node.c b/src/plugins/acl/sess_mgmt_node.c
index 6c01643aca2..bfc357f667f 100644
--- a/src/plugins/acl/sess_mgmt_node.c
+++ b/src/plugins/acl/sess_mgmt_node.c
@@ -49,30 +49,43 @@ fa_session_get_shortest_timeout (acl_main_t * am)
 }
 
 static u8 *
-format_session_bihash_5tuple (u8 * s, va_list * args)
+format_ip6_session_bihash_kv (u8 * s, va_list * args)
 {
-  fa_5tuple_t *p5t = va_arg (*args, fa_5tuple_t *);
-  fa_full_session_id_t *sess = (void *) &p5t->pkt;
-  if (is_ip6_5tuple (p5t))
-    return (format (s, "l3 %U -> %U"
-		    " l4 lsb_of_sw_if_index %d proto %d l4_is_input %d l4_slow_path %d l4_reserved0 %d port %d -> %d | sess id %d thread id %d epoch %04x",
-		    format_ip6_address, &p5t->ip6_addr[0],
-		    format_ip6_address, &p5t->ip6_addr[1],
-		    p5t->l4.lsb_of_sw_if_index,
-		    p5t->l4.proto, p5t->l4.is_input, p5t->l4.is_slowpath,
-		    p5t->l4.reserved0, p5t->l4.port[0], p5t->l4.port[1],
-		    sess->session_index, sess->thread_index,
-		    sess->intf_policy_epoch));
-  else
-    return (format (s, "l3 %U -> %U"
-		    " l4 lsb_of_sw_if_index %d proto %d l4_is_input %d l4_slow_path %d l4_reserved0 %d port %d -> %d | sess id %d thread id %d epoch %04x",
-		    format_ip4_address, &p5t->ip4_addr[0],
-		    format_ip4_address, &p5t->ip4_addr[1],
-		    p5t->l4.lsb_of_sw_if_index,
-		    p5t->l4.proto, p5t->l4.is_input, p5t->l4.is_slowpath,
-		    p5t->l4.reserved0, p5t->l4.port[0], p5t->l4.port[1],
-		    sess->session_index, sess->thread_index,
-		    sess->intf_policy_epoch));
+  clib_bihash_kv_40_8_t *kv_40_8 = va_arg (*args, clib_bihash_kv_40_8_t *);
+  fa_5tuple_t a5t;
+
+  a5t.kv_40_8 = *kv_40_8;
+  fa_full_session_id_t *sess = (fa_full_session_id_t *) & a5t.pkt;
+
+  return (format (s, "l3 %U -> %U"
+		  " l4 lsb_of_sw_if_index %d proto %d l4_is_input %d l4_slow_path %d l4_reserved0 %d port %d -> %d | sess id %d thread id %d epoch %04x",
+		  format_ip6_address, &a5t.ip6_addr[0],
+		  format_ip6_address, &a5t.ip6_addr[1],
+		  a5t.l4.lsb_of_sw_if_index,
+		  a5t.l4.proto, a5t.l4.is_input, a5t.l4.is_slowpath,
+		  a5t.l4.reserved0, a5t.l4.port[0], a5t.l4.port[1],
+		  sess->session_index, sess->thread_index,
+		  sess->intf_policy_epoch));
+}
+
+static u8 *
+format_ip4_session_bihash_kv (u8 * s, va_list * args)
+{
+  clib_bihash_kv_16_8_t *kv_16_8 = va_arg (*args, clib_bihash_kv_16_8_t *);
+  fa_5tuple_t a5t;
+
+  a5t.kv_16_8 = *kv_16_8;
+  fa_full_session_id_t *sess = (fa_full_session_id_t *) & a5t.pkt;
+
+  return (format (s, "l3 %U -> %U"
+		  " l4 lsb_of_sw_if_index %d proto %d l4_is_input %d l4_slow_path %d l4_reserved0 %d port %d -> %d | sess id %d thread id %d epoch %04x",
+		  format_ip4_address, &a5t.ip4_addr[0],
+		  format_ip4_address, &a5t.ip4_addr[1],
+		  a5t.l4.lsb_of_sw_if_index,
+		  a5t.l4.proto, a5t.l4.is_input, a5t.l4.is_slowpath,
+		  a5t.l4.reserved0, a5t.l4.port[0], a5t.l4.port[1],
+		  sess->session_index, sess->thread_index,
+		  sess->intf_policy_epoch));
 }
 
 
@@ -97,12 +110,20 @@ acl_fa_verify_init_sessions (acl_main_t * am)
 	}
 
       /* ... and the interface session hash table */
-      clib_bihash_init_40_8 (&am->fa_sessions_hash,
-			     "ACL plugin FA session bihash",
+      clib_bihash_init_40_8 (&am->fa_ip6_sessions_hash,
+			     "ACL plugin FA IPv6 session bihash",
 			     am->fa_conn_table_hash_num_buckets,
 			     am->fa_conn_table_hash_memory_size);
-      clib_bihash_set_kvp_format_fn_40_8 (&am->fa_sessions_hash,
-					  format_session_bihash_5tuple);
+      clib_bihash_set_kvp_format_fn_40_8 (&am->fa_ip6_sessions_hash,
+					  format_ip6_session_bihash_kv);
+
+      clib_bihash_init_16_8 (&am->fa_ip4_sessions_hash,
+			     "ACL plugin FA IPv4 session bihash",
+			     am->fa_conn_table_hash_num_buckets,
+			     am->fa_conn_table_hash_memory_size);
+      clib_bihash_set_kvp_format_fn_16_8 (&am->fa_ip4_sessions_hash,
+					  format_ip4_session_bihash_kv);
+
       am->fa_sessions_hash_is_initialized = 1;
     }
 }
@@ -849,8 +870,13 @@ show_fa_sessions_hash (vlib_main_t * vm, u32 verbose)
   acl_main_t *am = &acl_main;
   if (am->fa_sessions_hash_is_initialized)
     {
-      vlib_cli_output (vm, "\nSession lookup hash table:\n%U\n\n",
-		       format_bihash_40_8, &am->fa_sessions_hash, verbose);
+      vlib_cli_output (vm, "\nIPv6 Session lookup hash table:\n%U\n\n",
+		       format_bihash_40_8, &am->fa_ip6_sessions_hash,
+		       verbose);
+
+      vlib_cli_output (vm, "\nIPv4 Session lookup hash table:\n%U\n\n",
+		       format_bihash_16_8, &am->fa_ip4_sessions_hash,
+		       verbose);
     }
   else
     {
diff --git a/src/plugins/acl/session_inlines.h b/src/plugins/acl/session_inlines.h
index 01dface323e..4d5dfe7587a 100644
--- a/src/plugins/acl/session_inlines.h
+++ b/src/plugins/acl/session_inlines.h
@@ -365,28 +365,31 @@ reverse_l4_u64 (u64 l4, int is_ip6)
 }
 
 always_inline void
-reverse_session_add_del (acl_main_t * am, int is_ip6,
-			 clib_bihash_kv_40_8_t * pkv, int is_add)
+reverse_session_add_del_ip6 (acl_main_t * am,
+			     clib_bihash_kv_40_8_t * pkv, int is_add)
 {
   clib_bihash_kv_40_8_t kv2;
-  if (is_ip6)
-    {
-      kv2.key[0] = pkv->key[2];
-      kv2.key[1] = pkv->key[3];
-      kv2.key[2] = pkv->key[0];
-      kv2.key[3] = pkv->key[1];
-    }
-  else
-    {
-      kv2.key[0] = kv2.key[1] = kv2.key[2] = 0;
-      kv2.key[3] =
-	((pkv->key[3] & 0xffffffff) << 32) | ((pkv->key[3] >> 32) &
-					      0xffffffff);
-    }
+  kv2.key[0] = pkv->key[2];
+  kv2.key[1] = pkv->key[3];
+  kv2.key[2] = pkv->key[0];
+  kv2.key[3] = pkv->key[1];
+  /* the last u64 needs special treatment (ports, etc.) */
+  kv2.key[4] = reverse_l4_u64 (pkv->key[4], 1);
+  kv2.value = pkv->value;
+  clib_bihash_add_del_40_8 (&am->fa_ip6_sessions_hash, &kv2, is_add);
+}
+
+always_inline void
+reverse_session_add_del_ip4 (acl_main_t * am,
+			     clib_bihash_kv_16_8_t * pkv, int is_add)
+{
+  clib_bihash_kv_16_8_t kv2;
+  kv2.key[0] =
+    ((pkv->key[0] & 0xffffffff) << 32) | ((pkv->key[0] >> 32) & 0xffffffff);
   /* the last u64 needs special treatment (ports, etc.) */
-  kv2.key[4] = reverse_l4_u64 (pkv->key[4], is_ip6);
+  kv2.key[1] = reverse_l4_u64 (pkv->key[1], 0);
   kv2.value = pkv->value;
-  clib_bihash_add_del_40_8 (&am->fa_sessions_hash, &kv2, is_add);
+  clib_bihash_add_del_16_8 (&am->fa_ip4_sessions_hash, &kv2, is_add);
 }
 
 always_inline void
@@ -396,9 +399,19 @@ acl_fa_deactivate_session (acl_main_t * am, u32 sw_if_index,
   fa_session_t *sess =
     get_session_ptr (am, sess_id.thread_index, sess_id.session_index);
   ASSERT (sess->thread_index == os_get_thread_index ());
-  clib_bihash_add_del_40_8 (&am->fa_sessions_hash, &sess->info.kv, 0);
+  if (sess->is_ip6)
+    {
+      clib_bihash_add_del_40_8 (&am->fa_ip6_sessions_hash,
+				&sess->info.kv_40_8, 0);
+      reverse_session_add_del_ip6 (am, &sess->info.kv_40_8, 0);
+    }
+  else
+    {
+      clib_bihash_add_del_16_8 (&am->fa_ip4_sessions_hash,
+				&sess->info.kv_16_8, 0);
+      reverse_session_add_del_ip4 (am, &sess->info.kv_16_8, 0);
+    }
 
-  reverse_session_add_del (am, sess->is_ip6, &sess->info.kv, 0);
   sess->deleted = 1;
   clib_smp_atomic_add (&am->fa_session_total_deactivations, 1);
 }
@@ -498,8 +511,6 @@ acl_fa_add_session (acl_main_t * am, int is_input, int is_ip6,
 		    u32 sw_if_index, u64 now, fa_5tuple_t * p5tuple,
 		    u16 current_policy_epoch)
 {
-  clib_bihash_kv_40_8_t *pkv = &p5tuple->kv;
-  clib_bihash_kv_40_8_t kv;
   fa_full_session_id_t f_sess_id;
   uword thread_index = os_get_thread_index ();
   void *oldheap = clib_mem_set_heap (am->acl_mheap);
@@ -508,22 +519,31 @@ acl_fa_add_session (acl_main_t * am, int is_input, int is_ip6,
   f_sess_id.thread_index = thread_index;
   fa_session_t *sess;
 
+  if (f_sess_id.as_u64 == ~0)
+    {
+      clib_error ("Adding session with invalid value");
+    }
+
   pool_get_aligned (pw->fa_sessions_pool, sess, CLIB_CACHE_LINE_BYTES);
   f_sess_id.session_index = sess - pw->fa_sessions_pool;
   f_sess_id.intf_policy_epoch = current_policy_epoch;
 
-  kv.key[0] = pkv->key[0];
-  kv.key[1] = pkv->key[1];
-  kv.key[2] = pkv->key[2];
-  kv.key[3] = pkv->key[3];
-  kv.key[4] = pkv->key[4];
-  kv.value = f_sess_id.as_u64;
-  if (kv.value == ~0)
+  if (is_ip6)
     {
-      clib_error ("Adding session with invalid value");
+      sess->info.kv_40_8.key[0] = p5tuple->kv_40_8.key[0];
+      sess->info.kv_40_8.key[1] = p5tuple->kv_40_8.key[1];
+      sess->info.kv_40_8.key[2] = p5tuple->kv_40_8.key[2];
+      sess->info.kv_40_8.key[3] = p5tuple->kv_40_8.key[3];
+      sess->info.kv_40_8.key[4] = p5tuple->kv_40_8.key[4];
+      sess->info.kv_40_8.value = f_sess_id.as_u64;
+    }
+  else
+    {
+      sess->info.kv_16_8.key[0] = p5tuple->kv_16_8.key[0];
+      sess->info.kv_16_8.key[1] = p5tuple->kv_16_8.key[1];
+      sess->info.kv_16_8.value = f_sess_id.as_u64;
     }
 
-  memcpy (sess, pkv, sizeof (pkv->key));
   sess->last_active_time = now;
   sess->sw_if_index = sw_if_index;
   sess->tcp_flags_seen.as_u16 = 0;
@@ -537,9 +557,18 @@ acl_fa_add_session (acl_main_t * am, int is_input, int is_ip6,
   acl_fa_conn_list_add_session (am, f_sess_id, now);
 
   ASSERT (am->fa_sessions_hash_is_initialized == 1);
-
-  reverse_session_add_del (am, is_ip6, &kv, 1);
-  clib_bihash_add_del_40_8 (&am->fa_sessions_hash, &kv, 1);
+  if (is_ip6)
+    {
+      reverse_session_add_del_ip6 (am, &sess->info.kv_40_8, 1);
+      clib_bihash_add_del_40_8 (&am->fa_ip6_sessions_hash,
+				&sess->info.kv_40_8, 1);
+    }
+  else
+    {
+      reverse_session_add_del_ip4 (am, &sess->info.kv_16_8, 1);
+      clib_bihash_add_del_16_8 (&am->fa_ip4_sessions_hash,
+				&sess->info.kv_16_8, 1);
+    }
 
   vec_validate (pw->fa_session_adds_by_sw_if_index, sw_if_index);
   clib_mem_set_heap (oldheap);
@@ -549,11 +578,25 @@ acl_fa_add_session (acl_main_t * am, int is_input, int is_ip6,
 }
 
 always_inline int
-acl_fa_find_session (acl_main_t * am, u32 sw_if_index0, fa_5tuple_t * p5tuple,
-		     clib_bihash_kv_40_8_t * pvalue_sess)
+acl_fa_find_session (acl_main_t * am, int is_ip6, u32 sw_if_index0,
+		     fa_5tuple_t * p5tuple, u64 * pvalue_sess)
 {
-  return (clib_bihash_search_inline_2_40_8
-	  (&am->fa_sessions_hash, &p5tuple->kv, pvalue_sess) == 0);
+  int res = 0;
+  if (is_ip6)
+    {
+      clib_bihash_kv_40_8_t kv_result;
+      res = (clib_bihash_search_inline_2_40_8
+	     (&am->fa_ip6_sessions_hash, &p5tuple->kv_40_8, &kv_result) == 0);
+      *pvalue_sess = kv_result.value;
+    }
+  else
+    {
+      clib_bihash_kv_16_8_t kv_result;
+      res = (clib_bihash_search_inline_2_16_8
+	     (&am->fa_ip4_sessions_hash, &p5tuple->kv_16_8, &kv_result) == 0);
+      *pvalue_sess = kv_result.value;
+    }
+  return res;
 }
 
 /*