NAT44: delete user with zero sessions (VPP-1282)

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

6 files changed, 64 insertions, 20 deletions

diff --git a/src/plugins/nat/in2out.c b/src/plugins/nat/in2out.c
index be1ddb3db7e..ac0490f36e9 100755
--- a/src/plugins/nat/in2out.c
+++ b/src/plugins/nat/in2out.c
@@ -347,14 +347,6 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
 
   key1.protocol = key0->protocol;
 
-  u = nat_user_get_or_create (sm, &ip0->src_address, rx_fib_index0,
-                              thread_index);
-  if (!u)
-    {
-      nat_log_warn ("create NAT user failed");
-      return SNAT_IN2OUT_NEXT_DROP;
-    }
-
   /* First try to match static mapping by local address and port */
   if (snat_static_mapping_match (sm, *key0, &key1, 0, 0, 0, 0))
     {
@@ -372,9 +364,18 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
   else
     is_sm = 1;
 
+  u = nat_user_get_or_create (sm, &ip0->src_address, rx_fib_index0,
+                              thread_index);
+  if (!u)
+    {
+      nat_log_warn ("create NAT user failed");
+      return SNAT_IN2OUT_NEXT_DROP;
+    }
+
   s = nat_session_alloc_or_recycle (sm, u, thread_index);
   if (!s)
     {
+      nat44_delete_user_with_no_session (sm, u, thread_index);
       nat_log_warn ("create NAT session failed");
       return SNAT_IN2OUT_NEXT_DROP;
     }
@@ -2442,6 +2443,7 @@ slow_path_ed (snat_main_t *sm,
   s = nat_session_alloc_or_recycle (sm, u, thread_index);
   if (!s)
     {
+      nat44_delete_user_with_no_session (sm, u, thread_index);
       nat_log_warn ("create NAT session failed");
       return SNAT_IN2OUT_NEXT_DROP;
     }
@@ -2924,6 +2926,7 @@ create_ses:
       s = nat_session_alloc_or_recycle (sm, u, thread_index);
       if (!s)
         {
+          nat44_delete_user_with_no_session (sm, u, thread_index);
           nat_log_warn ("create NAT session failed");
           return 0;
         }
diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
index cdf05fd351a..79d11136240 100755
--- a/src/plugins/nat/nat.c
+++ b/src/plugins/nat/nat.c
@@ -328,6 +328,8 @@ nat_user_get_or_create (snat_main_t *sm, ip4_address_t *addr, u32 fib_index,
       /* add user */
       if (clib_bihash_add_del_8_8 (&tsm->user_hash, &kv, 1))
         nat_log_warn ("user_hash keay add failed");
+
+      clib_warning("%U %d", format_ip4_address, addr, fib_index);
     }
   else
     {
@@ -1086,7 +1088,7 @@ int snat_add_static_mapping(ip4_address_t l_addr, ip4_address_t e_addr,
                       nat_free_session_data (sm, s, tsm - sm->per_thread_data);
                       nat44_delete_session (sm, s, tsm - sm->per_thread_data);
 
-                      if (!addr_only)
+                      if (!addr_only && !sm->endpoint_dependent)
                         break;
                     }
                 }
@@ -1192,14 +1194,9 @@ int snat_add_static_mapping(ip4_address_t l_addr, ip4_address_t e_addr,
                       nat_free_session_data (sm, s, tsm - sm->per_thread_data);
                       nat44_delete_session (sm, s, tsm - sm->per_thread_data);
 
-                      if (!addr_only)
+                      if (!addr_only && !sm->endpoint_dependent)
                         break;
                     }
-                  if (addr_only && (u->nstaticsessions == 0) && (u->nsessions == 0))
-                    {
-                      pool_put (tsm->users, u);
-                      clib_bihash_add_del_8_8 (&tsm->user_hash, &kv, 0);
-                    }
                 }
             }
         }
diff --git a/src/plugins/nat/nat_api.c b/src/plugins/nat/nat_api.c
index 5887efe5fea..d467861d94c 100644
--- a/src/plugins/nat/nat_api.c
+++ b/src/plugins/nat/nat_api.c
@@ -1195,8 +1195,12 @@ vl_api_nat44_user_dump_t_handler (vl_api_nat44_user_dump_t * mp)
 
   /* *INDENT-OFF* */
   vec_foreach (tsm, sm->per_thread_data)
-    vec_foreach (u, tsm->users)
-      send_nat44_user_details (u, reg, mp->context);
+    {
+      pool_foreach (u, tsm->users,
+      ({
+        send_nat44_user_details (u, reg, mp->context);
+      }));
+    }
   /* *INDENT-ON* */
 }
 
diff --git a/src/plugins/nat/nat_inlines.h b/src/plugins/nat/nat_inlines.h
index 3724986b9b8..a069d66bad9 100644
--- a/src/plugins/nat/nat_inlines.h
+++ b/src/plugins/nat/nat_inlines.h
@@ -141,6 +141,26 @@ user_session_increment (snat_main_t * sm, snat_user_t * u, u8 is_static)
 }
 
 always_inline void
+nat44_delete_user_with_no_session (snat_main_t * sm, snat_user_t * u,
+				   u32 thread_index)
+{
+  clib_bihash_kv_8_8_t kv;
+  snat_user_key_t u_key;
+  snat_main_per_thread_data_t *tsm = vec_elt_at_index (sm->per_thread_data,
+						       thread_index);
+
+  if (u->nstaticsessions == 0 && u->nsessions == 0)
+    {
+      u_key.addr.as_u32 = u->addr.as_u32;
+      u_key.fib_index = u->fib_index;
+      kv.key = u_key.as_u64;
+      pool_put_index (tsm->list_pool, u->sessions_per_user_list_head_index);
+      pool_put (tsm->users, u);
+      clib_bihash_add_del_8_8 (&tsm->user_hash, &kv, 0);
+    }
+}
+
+always_inline void
 nat44_delete_session (snat_main_t * sm, snat_session_t * ses,
 		      u32 thread_index)
 {
@@ -151,6 +171,11 @@ nat44_delete_session (snat_main_t * sm, snat_session_t * ses,
   snat_user_t *u;
 
   nat_log_debug ("session deleted %U", format_snat_session, tsm, ses);
+
+  clib_dlist_remove (tsm->list_pool, ses->per_user_index);
+  pool_put_index (tsm->list_pool, ses->per_user_index);
+  pool_put (tsm->sessions, ses);
+
   u_key.addr = ses->in2out.addr;
   u_key.fib_index = ses->in2out.fib_index;
   kv.key = u_key.as_u64;
@@ -161,10 +186,9 @@ nat44_delete_session (snat_main_t * sm, snat_session_t * ses,
 	u->nstaticsessions--;
       else
 	u->nsessions--;
+
+      nat44_delete_user_with_no_session (sm, u, thread_index);
     }
-  clib_dlist_remove (tsm->list_pool, ses->per_user_index);
-  pool_put_index (tsm->list_pool, ses->per_user_index);
-  pool_put (tsm->sessions, ses);
 }
 
 /** \brief Set TCP session state.
diff --git a/src/plugins/nat/out2in.c b/src/plugins/nat/out2in.c
index 156b7283fef..774ae67d50a 100755
--- a/src/plugins/nat/out2in.c
+++ b/src/plugins/nat/out2in.c
@@ -188,6 +188,7 @@ create_session_for_static_mapping (snat_main_t *sm,
   s = nat_session_alloc_or_recycle (sm, u, thread_index);
   if (!s)
     {
+      nat44_delete_user_with_no_session (sm, u, thread_index);
       nat_log_warn ("create NAT session failed");
       return 0;
     }
@@ -1599,6 +1600,7 @@ create_session_for_static_mapping_ed (snat_main_t * sm,
   s = nat_session_alloc_or_recycle (sm, u, thread_index);
   if (!s)
     {
+      nat44_delete_user_with_no_session (sm, u, thread_index);
       nat_log_warn ("create NAT session failed");
       return 0;
     }
@@ -1780,6 +1782,7 @@ create_bypass_for_fwd(snat_main_t * sm, ip4_header_t * ip, u32 rx_fib_index,
       s = nat_session_alloc_or_recycle (sm, u, thread_index);
       if (!s)
         {
+          nat44_delete_user_with_no_session (sm, u, thread_index);
           nat_log_warn ("create NAT session failed");
           return;
         }
@@ -1980,6 +1983,7 @@ nat44_ed_out2in_unknown_proto (snat_main_t *sm,
       s = nat_session_alloc_or_recycle (sm, u, thread_index);
       if (!s)
         {
+          nat44_delete_user_with_no_session (sm, u, thread_index);
           nat_log_warn ("create NAT session failed");
           return 0;
         }
diff --git a/test/test_nat.py b/test/test_nat.py
index 4bae7015145..0d723b0a688 100644
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -136,6 +136,7 @@ class MethodHolder(VppTestCase):
 
         self.vapi.nat_set_reass()
         self.vapi.nat_set_reass(is_ip6=1)
+        self.verify_no_nat44_user()
 
     def nat44_add_static_mapping(self, local_ip, external_ip='0.0.0.0',
                                  local_port=0, external_port=0, vrf_id=0,
@@ -987,6 +988,11 @@ class MethodHolder(VppTestCase):
         # postNAPTDestinationTransportPort
         self.assertEqual(struct.pack("!H", dst_port), record[228])
 
+    def verify_no_nat44_user(self):
+        """ Verify that there is no NAT44 user """
+        users = self.vapi.nat44_user_dump()
+        self.assertEqual(len(users), 0)
+
 
 class TestNAT44(MethodHolder):
     """ NAT44 Test Cases """
@@ -2909,6 +2915,12 @@ class TestNAT44(MethodHolder):
         sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4n, 0)
         self.assertEqual(nsessions - len(sessions), 2)
 
+        self.vapi.nat44_del_session(sessions[0].inside_ip_address,
+                                    sessions[0].inside_port,
+                                    sessions[0].protocol)
+
+        self.verify_no_nat44_user()
+
     def test_set_get_reass(self):
         """ NAT44 set/get virtual fragmentation reassembly """
         reas_cfg1 = self.vapi.nat_get_reass()