NAT44: improve expired sessions reuse (VPP-1503)

Change-Id: Iab506f127136c94a641df31ded108016de26260b
Signed-off-by: Matus Fabian <matfabia@cisco.com>

3 files changed, 67 insertions, 96 deletions

diff --git a/src/plugins/nat/in2out_ed.c b/src/plugins/nat/in2out_ed.c
index f3999f00129..c42d4e79f9e 100644
--- a/src/plugins/nat/in2out_ed.c
+++ b/src/plugins/nat/in2out_ed.c
@@ -255,7 +255,8 @@ slow_path_ed (snat_main_t * sm,
 	      u32 rx_fib_index,
 	      clib_bihash_kv_16_8_t * kv,
 	      snat_session_t ** sessionp,
-	      vlib_node_runtime_t * node, u32 next, u32 thread_index, f64 now)
+	      vlib_node_runtime_t * node, u32 next, u32 thread_index, f64 now,
+	      tcp_header_t * tcp)
 {
   snat_session_t *s = 0;
   snat_user_t *u;
@@ -315,6 +316,15 @@ slow_path_ed (snat_main_t * sm,
       is_sm = 1;
     }
 
+  if (proto == SNAT_PROTOCOL_TCP)
+    {
+      if (!tcp_is_init (tcp))
+	{
+	  b->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
+	  return NAT_IN2OUT_ED_NEXT_DROP;
+	}
+    }
+
   u = nat_user_get_or_create (sm, &key->l_addr, rx_fib_index, thread_index);
   if (!u)
     {
@@ -626,7 +636,7 @@ icmp_match_in2out_ed (snat_main_t * sm, vlib_node_runtime_t * node,
 	}
 
       next = slow_path_ed (sm, b, rx_fib_index, &kv, &s, node, next,
-			   thread_index, vlib_time_now (sm->vlib_main));
+			   thread_index, vlib_time_now (sm->vlib_main), 0);
 
       if (PREDICT_FALSE (next == NAT_IN2OUT_ED_NEXT_DROP))
 	goto out;
@@ -1034,16 +1044,9 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace00;
 		    }
 
-		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
-		    {
-		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
-		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
-		      goto trace00;
-		    }
-
 		  next0 =
 		    slow_path_ed (sm, b0, rx_fib_index0, &kv0, &s0, node,
-				  next0, thread_index, now);
+				  next0, thread_index, now, tcp0);
 
 		  if (PREDICT_FALSE (next0 == NAT_IN2OUT_ED_NEXT_DROP))
 		    goto trace00;
@@ -1245,16 +1248,9 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace01;
 		    }
 
-		  if ((proto1 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp1))
-		    {
-		      b1->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
-		      next1 = NAT_IN2OUT_ED_NEXT_DROP;
-		      goto trace01;
-		    }
-
 		  next1 =
 		    slow_path_ed (sm, b1, rx_fib_index1, &kv1, &s1, node,
-				  next1, thread_index, now);
+				  next1, thread_index, now, tcp1);
 
 		  if (PREDICT_FALSE (next1 == NAT_IN2OUT_ED_NEXT_DROP))
 		    goto trace01;
@@ -1485,16 +1481,9 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace0;
 		    }
 
-		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
-		    {
-		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
-		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
-		      goto trace0;
-		    }
-
 		  next0 =
 		    slow_path_ed (sm, b0, rx_fib_index0, &kv0, &s0, node,
-				  next0, thread_index, now);
+				  next0, thread_index, now, tcp0);
 
 		  if (PREDICT_FALSE (next0 == NAT_IN2OUT_ED_NEXT_DROP))
 		    goto trace0;
@@ -1892,15 +1881,9 @@ nat44_ed_in2out_reass_node_fn_inline (vlib_main_t * vm,
 			}
 		    }
 
-		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
-		    {
-		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
-		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
-		      goto trace0;
-		    }
-
 		  next0 = slow_path_ed (sm, b0, rx_fib_index0, &kv0,
-					&s0, node, next0, thread_index, now);
+					&s0, node, next0, thread_index, now,
+					tcp0);
 
 		  if (PREDICT_FALSE (next0 == NAT_IN2OUT_ED_NEXT_DROP))
 		    goto trace0;
diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
index e7d59f78d08..b7ade881c3f 100755
--- a/src/plugins/nat/nat.c
+++ b/src/plugins/nat/nat.c
@@ -397,64 +397,67 @@ nat_ed_session_alloc (snat_main_t * sm, snat_user_t * u, u32 thread_index,
   u32 oldest_index;
   u64 sess_timeout_time;
 
-  if ((u->nsessions + u->nstaticsessions) >= sm->max_translations_per_user)
+  if (PREDICT_FALSE (!(u->nsessions) && !(u->nstaticsessions)))
+    goto alloc_new;
+
+  oldest_index =
+    clib_dlist_remove_head (tsm->list_pool,
+			    u->sessions_per_user_list_head_index);
+  oldest_elt = pool_elt_at_index (tsm->list_pool, oldest_index);
+  s = pool_elt_at_index (tsm->sessions, oldest_elt->value);
+  sess_timeout_time = s->last_heard + (f64) nat44_session_get_timeout (sm, s);
+  if (now >= sess_timeout_time)
     {
-      oldest_index =
-	clib_dlist_remove_head (tsm->list_pool,
-				u->sessions_per_user_list_head_index);
-      oldest_elt = pool_elt_at_index (tsm->list_pool, oldest_index);
-      s = pool_elt_at_index (tsm->sessions, oldest_elt->value);
-      sess_timeout_time =
-	s->last_heard + (f64) nat44_session_get_timeout (sm, s);
-      if (now >= sess_timeout_time)
-	{
-	  clib_dlist_addtail (tsm->list_pool,
-			      u->sessions_per_user_list_head_index,
-			      oldest_index);
-	  nat_free_session_data (sm, s, thread_index);
-	  if (snat_is_session_static (s))
-	    u->nstaticsessions--;
-	  else
-	    u->nsessions--;
-	  s->flags = 0;
-	  s->total_bytes = 0;
-	  s->total_pkts = 0;
-	  s->state = 0;
-	  s->ext_host_addr.as_u32 = 0;
-	  s->ext_host_port = 0;
-	  s->ext_host_nat_addr.as_u32 = 0;
-	  s->ext_host_nat_port = 0;
-	}
+      clib_dlist_addtail (tsm->list_pool,
+			  u->sessions_per_user_list_head_index, oldest_index);
+      nat_free_session_data (sm, s, thread_index);
+      if (snat_is_session_static (s))
+	u->nstaticsessions--;
       else
+	u->nsessions--;
+      s->flags = 0;
+      s->total_bytes = 0;
+      s->total_pkts = 0;
+      s->state = 0;
+      s->ext_host_addr.as_u32 = 0;
+      s->ext_host_port = 0;
+      s->ext_host_nat_addr.as_u32 = 0;
+      s->ext_host_nat_port = 0;
+    }
+  else
+    {
+      clib_dlist_addhead (tsm->list_pool,
+			  u->sessions_per_user_list_head_index, oldest_index);
+      if ((u->nsessions + u->nstaticsessions) >=
+	  sm->max_translations_per_user)
 	{
-	  clib_dlist_addhead (tsm->list_pool,
-			      u->sessions_per_user_list_head_index,
-			      oldest_index);
 	  nat_log_warn ("max translations per user %U", format_ip4_address,
 			&u->addr);
 	  snat_ipfix_logging_max_entries_per_user
 	    (sm->max_translations_per_user, u->addr.as_u32);
 	  return 0;
 	}
-    }
-  else
-    {
-      pool_get (tsm->sessions, s);
-      clib_memset (s, 0, sizeof (*s));
+      else
+	{
+	alloc_new:
+	  pool_get (tsm->sessions, s);
+	  clib_memset (s, 0, sizeof (*s));
 
-      /* Create list elts */
-      pool_get (tsm->list_pool, per_user_translation_list_elt);
-      clib_dlist_init (tsm->list_pool,
-		       per_user_translation_list_elt - tsm->list_pool);
+	  /* Create list elts */
+	  pool_get (tsm->list_pool, per_user_translation_list_elt);
+	  clib_dlist_init (tsm->list_pool,
+			   per_user_translation_list_elt - tsm->list_pool);
 
-      per_user_translation_list_elt->value = s - tsm->sessions;
-      s->per_user_index = per_user_translation_list_elt - tsm->list_pool;
-      s->per_user_list_head_index = u->sessions_per_user_list_head_index;
+	  per_user_translation_list_elt->value = s - tsm->sessions;
+	  s->per_user_index = per_user_translation_list_elt - tsm->list_pool;
+	  s->per_user_list_head_index = u->sessions_per_user_list_head_index;
 
-      clib_dlist_addtail (tsm->list_pool,
-			  s->per_user_list_head_index,
-			  per_user_translation_list_elt - tsm->list_pool);
+	  clib_dlist_addtail (tsm->list_pool,
+			      s->per_user_list_head_index,
+			      per_user_translation_list_elt - tsm->list_pool);
+	}
     }
+
   return s;
 }
 
diff --git a/test/test_nat.py b/test/test_nat.py
index 524301f302d..cf5cc63cc50 100644
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -5658,7 +5658,7 @@ class TestNAT44EndpointDependent(MethodHolder):
 
         pkts = []
         for i in range(0, max_sessions):
-            src = "10.10.%u.%u" % ((i & 0xFF00) >> 8, i & 0xFF)
+            src = "10.11.%u.%u" % ((i & 0xFF00) >> 8, i & 0xFF)
             p = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) /
                  IP(src=src, dst=self.pg1.remote_ip4) /
                  ICMP(id=1026, type='echo-request'))
@@ -5683,8 +5683,6 @@ class TestNAT44EndpointDependent(MethodHolder):
                                                   is_inside=0)
         self.vapi.nat_set_timeouts(tcp_transitory=5)
 
-        nat44_config = self.vapi.nat_show_config()
-
         self.initiate_tcp_session(self.pg0, self.pg1)
         p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) /
              IP(src=self.pg0.remote_ip4, dst=self.pg1.remote_ip4) /
@@ -5695,18 +5693,6 @@ class TestNAT44EndpointDependent(MethodHolder):
         self.pg_start()
         self.pg1.get_capture(1)
 
-        pkts_num = nat44_config.max_translations_per_user - 1
-        pkts = []
-        for i in range(0, pkts_num):
-            p = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) /
-                 IP(src=self.pg0.remote_ip4, dst=self.pg1.remote_ip4) /
-                 UDP(sport=1025 + i, dport=53))
-            pkts.append(p)
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        self.pg1.get_capture(pkts_num)
-
         sleep(6)
 
         p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) /
@@ -5722,8 +5708,7 @@ class TestNAT44EndpointDependent(MethodHolder):
         users = self.vapi.nat44_user_dump()
         self.assertEqual(len(users), 1)
         self.assertEqual(users[0].ip_address, self.pg0.remote_ip4n)
-        self.assertEqual(users[0].nsessions,
-                         nat44_config.max_translations_per_user)
+        self.assertEqual(users[0].nsessions, 1)
 
     @unittest.skipUnless(running_extended_tests(), "part of extended tests")
     def test_session_limit_per_user(self):