nat: bihash: fix buckets calc and remove mem param

Calculate bihash buckets as n_elts / 2.5 rounded to closest pow2
per Damjan's recommendation. Remove memory configuration parameters
because bihash init ignores them anyway as it resides in main heap now.

Type: improvement
Change-Id: I189f463f3c4640106cce4f12d3c5a62969276a82
Signed-off-by: Klement Sekera <ksekera@cisco.com>

5 files changed, 33 insertions, 92 deletions

diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
index cb0e346c8fc..b60014d085e 100644
--- a/src/plugins/nat/nat.c
+++ b/src/plugins/nat/nat.c
@@ -200,8 +200,6 @@ snat_get_worker_in2out_cb (ip4_header_t * ip0, u32 rx_fib_index0,
 
 static u32 nat_calc_bihash_buckets (u32 n_elts);
 
-static u32 nat_calc_bihash_memory (u32 n_buckets, uword kv_size);
-
 u8 *format_static_mapping_kvp (u8 * s, va_list * args);
 
 u8 *format_ed_session_kvp (u8 * s, va_list * args);
@@ -2886,13 +2884,6 @@ nat44_plugin_enable (nat44_config_t c)
   sm->max_users_per_thread = c.users;
   sm->user_buckets = nat_calc_bihash_buckets (c.users);
 
-  if (!c.user_memory)
-    {
-      c.user_memory =
-	nat_calc_bihash_memory (c.users, sizeof (clib_bihash_8_8_t));
-    }
-  sm->user_memory_size = c.user_memory;
-
   if (!c.sessions)
     {
       // default value based on legacy setting of load factor 10 * default
@@ -2902,13 +2893,6 @@ nat44_plugin_enable (nat44_config_t c)
   sm->max_translations_per_thread = c.sessions;
   sm->translation_buckets = nat_calc_bihash_buckets (c.sessions);
 
-  if (!c.session_memory)
-    {
-      c.session_memory =
-	nat_calc_bihash_memory
-	(sm->translation_buckets, sizeof (clib_bihash_16_8_t));
-    }
-  sm->translation_memory_size = c.session_memory;
   vec_add1 (sm->max_translations_per_fib, sm->max_translations_per_thread);
   sm->max_translations_per_user
     = c.user_sessions ? c.user_sessions : sm->max_translations_per_thread;
@@ -2934,8 +2918,7 @@ nat44_plugin_enable (nat44_config_t c)
       sm->icmp_match_in2out_cb = icmp_match_in2out_ed;
 
       clib_bihash_init_16_8 (&sm->out2in_ed, "out2in-ed",
-			     sm->translation_buckets,
-			     sm->translation_memory_size);
+			     sm->translation_buckets, 0);
       clib_bihash_set_kvp_format_fn_16_8 (&sm->out2in_ed,
 					  format_ed_session_kvp);
 
@@ -4283,13 +4266,21 @@ nat_ha_sref_ed_cb (ip4_address_t * out_addr, u16 out_port,
 static u32
 nat_calc_bihash_buckets (u32 n_elts)
 {
-  return 1 << (max_log2 (n_elts >> 1) + 1);
-}
-
-static u32
-nat_calc_bihash_memory (u32 n_buckets, uword kv_size)
-{
-  return n_buckets * (8 + kv_size * 4);
+  n_elts = n_elts / 2.5;
+  u64 lower_pow2 = 1;
+  while (lower_pow2 * 2 < n_elts)
+    {
+      lower_pow2 = 2 * lower_pow2;
+    }
+  u64 upper_pow2 = 2 * lower_pow2;
+  if ((upper_pow2 - n_elts) < (n_elts - lower_pow2))
+    {
+      if (upper_pow2 <= UINT32_MAX)
+	{
+	  return upper_pow2;
+	}
+    }
+  return lower_pow2;
 }
 
 u32
@@ -4337,13 +4328,6 @@ nat44_update_session_limit (u32 session_limit, u32 vrf_id)
   sm->translation_buckets =
     nat_calc_bihash_buckets (sm->max_translations_per_thread);
 
-  if (!sm->translation_memory_size_set)
-    {
-      sm->translation_memory_size =
-	nat_calc_bihash_memory (sm->translation_buckets,
-				sizeof (clib_bihash_16_8_t));
-    }
-
   nat44_sessions_clear ();
   return 0;
 }
@@ -4381,29 +4365,25 @@ nat44_db_init (snat_main_per_thread_data_t * tsm)
   if (sm->endpoint_dependent)
     {
       clib_bihash_init_16_8 (&tsm->in2out_ed, "in2out-ed",
-			     sm->translation_buckets,
-			     sm->translation_memory_size);
+			     sm->translation_buckets, 0);
       clib_bihash_set_kvp_format_fn_16_8 (&tsm->in2out_ed,
 					  format_ed_session_kvp);
 
     }
   else
     {
-      clib_bihash_init_8_8 (&tsm->in2out, "in2out",
-			    sm->translation_buckets,
-			    sm->translation_memory_size);
+      clib_bihash_init_8_8 (&tsm->in2out, "in2out", sm->translation_buckets,
+			    0);
       clib_bihash_set_kvp_format_fn_8_8 (&tsm->in2out, format_session_kvp);
-      clib_bihash_init_8_8 (&tsm->out2in, "out2in",
-			    sm->translation_buckets,
-			    sm->translation_memory_size);
+      clib_bihash_init_8_8 (&tsm->out2in, "out2in", sm->translation_buckets,
+			    0);
       clib_bihash_set_kvp_format_fn_8_8 (&tsm->out2in, format_session_kvp);
     }
 
   // TODO: ED nat is not using these
   // before removal large refactor required
   pool_alloc (tsm->list_pool, sm->max_translations_per_thread);
-  clib_bihash_init_8_8 (&tsm->user_hash, "users", sm->user_buckets,
-			sm->user_memory_size);
+  clib_bihash_init_8_8 (&tsm->user_hash, "users", sm->user_buckets, 0);
   clib_bihash_set_kvp_format_fn_8_8 (&tsm->user_hash, format_user_kvp);
 }
 
@@ -4442,10 +4422,9 @@ nat44_sessions_clear ()
     {
       clib_bihash_free_16_8 (&sm->out2in_ed);
       clib_bihash_init_16_8 (&sm->out2in_ed, "out2in-ed",
-			     clib_max (1, sm->num_workers) *
-			     sm->translation_buckets,
-			     clib_max (1, sm->num_workers) *
-			     sm->translation_memory_size);
+			     clib_max (1,
+				       sm->num_workers) *
+			     sm->translation_buckets, 0);
       clib_bihash_set_kvp_format_fn_16_8 (&sm->out2in_ed,
 					  format_ed_session_kvp);
     }
diff --git a/src/plugins/nat/nat.h b/src/plugins/nat/nat.h
index 2d16507df7a..76d819b6ee3 100644
--- a/src/plugins/nat/nat.h
+++ b/src/plugins/nat/nat.h
@@ -76,11 +76,9 @@ typedef struct
 
   /* maximum number of users */
   u32 users;
-  u32 user_memory;
 
   /* maximum number of sessions */
   u32 sessions;
-  u32 session_memory;
 
   /* maximum number of ssessions per user */
   u32 user_sessions;
@@ -631,12 +629,10 @@ typedef struct snat_main_s
   u8 translation_memory_size_set;
 
   u32 translation_buckets;
-  uword translation_memory_size;
   u32 max_translations_per_thread;
   u32 *max_translations_per_fib;
   u32 max_users_per_thread;
   u32 user_buckets;
-  uword user_memory_size;
   u32 max_translations_per_user;
 
   u32 outside_vrf_id;
diff --git a/src/plugins/nat/nat44_cli.c b/src/plugins/nat/nat44_cli.c
index 7c616cda801..9db7db6ef22 100644
--- a/src/plugins/nat/nat44_cli.c
+++ b/src/plugins/nat/nat44_cli.c
@@ -79,9 +79,7 @@ nat44_enable_command_fn (vlib_main_t * vm,
       else if (unformat (line_input, "inside-vrf %u", &c.inside_vrf));
       else if (unformat (line_input, "outside-vrf %u", &c.outside_vrf));
       else if (unformat (line_input, "users %u", &c.users));
-      else if (unformat (line_input, "user-memory %u", &c.user_memory));
       else if (unformat (line_input, "sessions %u", &c.sessions));
-      else if (unformat (line_input, "session-memory %u", &c.session_memory));
       else if (unformat (line_input, "user-sessions %u", &c.user_sessions));
       else
 	{
@@ -91,11 +89,9 @@ nat44_enable_command_fn (vlib_main_t * vm,
 	}
     }
 
-  if (c.sessions && c.session_memory)
+  if (!c.sessions)
     {
-      error =
-	clib_error_return (0,
-			   "either number of sessions or size of the memory is required");
+      error = clib_error_return (0, "number of sessions is required");
       goto done;
     }
 
@@ -331,13 +327,9 @@ nat44_show_hash_command_fn (vlib_main_t * vm, unformat_input_t * input,
 
   vlib_cli_output (vm, "-------- hash table parameters --------\n");
   vlib_cli_output (vm, "translation buckets: %u", sm->translation_buckets);
-  vlib_cli_output (vm, "translation memory size: %U",
-		   format_memory_size, sm->translation_memory_size);
   if (!sm->endpoint_dependent)
     {
       vlib_cli_output (vm, "user buckets: %u", sm->user_buckets);
-      vlib_cli_output (vm, "user memory size: %U",
-		       format_memory_size, sm->user_memory_size);
     }
   return 0;
 }
@@ -1994,17 +1986,13 @@ VLIB_CLI_COMMAND (nat44_debug_fib_registration_command, static) = {
  *  vpp# nat44 enable sessions <n> out2in-dpo
  * To enable nat44 endpoint-dependent, use:
  *  vpp# nat44 enable sessions <n> endpoint-dependent
- * To overwrite user hash configuration, use:
- *  vpp# nat44 enable sessions <n> user-memory <n>
- * To overwrite session hash configuration, use:
- *  vpp# nat44 enable session-memory <n>
  * To set inside-vrf outside-vrf, use:
  *  vpp# nat44 enable sessions <n> inside-vrf <id> outside-vrf <id>
  * @cliexend
 ?*/
 VLIB_CLI_COMMAND (nat44_enable_command, static) = {
   .path = "nat44 enable",
-  .short_help = "nat44 enable sessions <max-number> [users <max-number>] [static-mappig-only [connection-tracking]|out2in-dpo|endpoint-dependent] [inside-vrf <vrf-id>] [outside-vrf <vrf-id>] [user-memory <number>] [session-memory <number>] [user-sessions <max-number>]",
+  .short_help = "nat44 enable sessions <max-number> [users <max-number>] [static-mappig-only [connection-tracking]|out2in-dpo|endpoint-dependent] [inside-vrf <vrf-id>] [outside-vrf <vrf-id>] [user-sessions <max-number>]",
   .function = nat44_enable_command_fn,
 };
 
diff --git a/src/plugins/nat/nat_api.c b/src/plugins/nat/nat_api.c
index 39f1af70f9d..933d3a2c2e5 100644
--- a/src/plugins/nat/nat_api.c
+++ b/src/plugins/nat/nat_api.c
@@ -102,12 +102,9 @@ vl_api_nat_show_config_t_handler (vl_api_nat_show_config_t * mp)
   REPLY_MACRO2 (VL_API_NAT_SHOW_CONFIG_REPLY,
   ({
     rmp->translation_buckets = htonl (sm->translation_buckets);
-    rmp->translation_memory_size = clib_host_to_net_u32 (
-		    sm->translation_memory_size > 0xffffffffULL
-		    ? 0xffffffffUL
-		    : (u32)sm->translation_memory_size);
+    rmp->translation_memory_size = 0;
     rmp->user_buckets = htonl (sm->user_buckets);
-    rmp->user_memory_size = clib_host_to_net_u64 (sm->user_memory_size);
+    rmp->user_memory_size = 0;
     rmp->max_translations_per_user = htonl (sm->max_translations_per_user);
     rmp->outside_vrf_id = htonl (sm->outside_vrf_id);
     rmp->inside_vrf_id = htonl (sm->inside_vrf_id);
@@ -148,9 +145,9 @@ vl_api_nat_show_config_2_t_handler (vl_api_nat_show_config_2_t * mp)
   REPLY_MACRO2 (VL_API_NAT_SHOW_CONFIG_2_REPLY,
   ({
     rmp->translation_buckets = htonl (sm->translation_buckets);
-    rmp->translation_memory_size = clib_host_to_net_u64 (sm->translation_memory_size);
+    rmp->translation_memory_size = 0;
     rmp->user_buckets = htonl (sm->user_buckets);
-    rmp->user_memory_size = clib_host_to_net_u64 (sm->user_memory_size);
+    rmp->user_memory_size = 0;
     rmp->max_translations_per_user = htonl (sm->max_translations_per_user);
     rmp->outside_vrf_id = htonl (sm->outside_vrf_id);
     rmp->inside_vrf_id = htonl (sm->inside_vrf_id);
@@ -353,10 +350,8 @@ static void
       c.outside_vrf = ntohl (mp->outside_vrf);
 
       c.users = ntohl (mp->users);
-      c.user_memory = ntohl (mp->user_memory);
 
       c.sessions = ntohl (mp->sessions);
-      c.session_memory = ntohl (mp->session_memory);
 
       c.user_sessions = ntohl (mp->user_sessions);
 
@@ -1768,8 +1763,7 @@ nat_ed_users_destroy (snat_main_per_thread_data_t * tsm)
   pool_flush (u, tsm->users, { });
   /* *INDENT-ON* */
   clib_bihash_free_8_8 (&tsm->user_hash);
-  clib_bihash_init_8_8 (&tsm->user_hash, "users", snat_main.user_buckets,
-			snat_main.user_memory_size);
+  clib_bihash_init_8_8 (&tsm->user_hash, "users", snat_main.user_buckets, 0);
   clib_bihash_set_kvp_format_fn_8_8 (&tsm->user_hash, format_user_kvp);
 }
 
diff --git a/src/plugins/nat/test/test_nat.py b/src/plugins/nat/test/test_nat.py
index 3b19b71f199..d635abf9825 100644
--- a/src/plugins/nat/test/test_nat.py
+++ b/src/plugins/nat/test/test_nat.py
@@ -1178,22 +1178,6 @@ class TestNATMisc(MethodHolder):
             self.vapi.nat44_plugin_enable_disable(enable=0)
             self.vapi.cli("clear logging")
 
-    def test_show_config(self):
-        """ NAT config translation memory """
-
-        nat_config = self.vapi.nat_show_config()
-        mem = nat_config.translation_memory_size
-        self.assertTrue(mem > 0)
-        self.logger.info("max translation memory: %d" % mem)
-
-    def test_show_config_2(self):
-        """ NAT config2 translation memory """
-
-        nat_config = self.vapi.nat_show_config_2()
-        mem = nat_config.translation_memory_size
-        self.assertTrue(mem > 0)
-        self.logger.info("max translation memory: %d" % mem)
-
     def test_show_max_translations(self):
         """ API test - max translations per thread """
         nat_config = self.vapi.nat_show_config_2()