User session counters stay <= per-user limit

When a user session is allocated/reused, only increase
one of the session counters for that user if the counters
are below the per-user limit.

THis addresses a SEGV that arises after the following
sequence of events:

- an outside interface IP address is put in a pool
- a user exceeds the number of per-user translations by
an amount greater than the number of per-user translations
(nsessions + nstaticsessions > 100 + 100)
- the outside interface IP address is deleted and then added
again (observed when using DHCP client, likely happens if
address changed via CLI, API also)
- the user sends more packets that should be translated

When nsessions is > the per-user limit,
nat_session_alloc_or_recycle() reclaims the oldest existing
user session. When an outside address is deleted, the
corresponding user sessions are deleted. If the counters were
far above the per-user limit, the deletions wouldn't result
in the counters dropping back below the limit. So no session
could be reclaimed -> SEGV.

Change-Id: I940bafba0fd5385a563e2ce87534688eb9469f12
Signed-off-by: Matthew Smith <mgsmith@netgate.com>

3 files changed, 28 insertions, 22 deletions

diff --git a/src/plugins/nat/in2out.c b/src/plugins/nat/in2out.c
index b3b205acbda..3a74a559e1c 100755
--- a/src/plugins/nat/in2out.c
+++ b/src/plugins/nat/in2out.c
@@ -286,6 +286,7 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
   u32 outside_fib_index;
   uword * p;
   udp_header_t * udp0 = ip4_next_header (ip0);
+  u8 is_sm = 0;
 
   if (PREDICT_FALSE (maximum_sessions_exceeded(sm, thread_index)))
     {
@@ -311,13 +312,6 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
       return SNAT_IN2OUT_NEXT_DROP;
     }
 
-  s = nat_session_alloc_or_recycle (sm, u, thread_index);
-  if (!s)
-    {
-      clib_warning ("create NAT session 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))
     {
@@ -331,14 +325,20 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
           b0->error = node->errors[SNAT_IN2OUT_ERROR_OUT_OF_PORTS];
           return SNAT_IN2OUT_NEXT_DROP;
         }
-      u->nsessions++;
     }
   else
+    is_sm = 1;
+
+  s = nat_session_alloc_or_recycle (sm, u, thread_index);
+  if (!s)
     {
-      u->nstaticsessions++;
-      s->flags |= SNAT_SESSION_FLAG_STATIC_MAPPING;
+      clib_warning ("create NAT session failed");
+      return SNAT_IN2OUT_NEXT_DROP;
     }
 
+  if (is_sm)
+    s->flags |= SNAT_SESSION_FLAG_STATIC_MAPPING;
+  user_session_increment (sm, u, is_sm);
   s->outside_address_index = address_index;
   s->in2out = *key0;
   s->out2in = key1;
@@ -1198,14 +1198,8 @@ create_ses:
       s->in2out.fib_index = rx_fib_index;
       s->in2out.port = s->out2in.port = ip->protocol;
       if (is_sm)
-        {
-          u->nstaticsessions++;
-          s->flags |= SNAT_SESSION_FLAG_STATIC_MAPPING;
-        }
-      else
-        {
-          u->nsessions++;
-        }
+	s->flags |= SNAT_SESSION_FLAG_STATIC_MAPPING;
+      user_session_increment (sm, u, is_sm);
 
       /* Add to lookup tables */
       key.l_addr.as_u32 = old_addr;
@@ -1325,7 +1319,7 @@ snat_in2out_lb (snat_main_t *sm,
       s->in2out = l_key;
       s->out2in = e_key;
       s->out2in.protocol = l_key.protocol;
-      u->nstaticsessions++;
+      user_session_increment (sm, u, 1 /* static */);
 
       /* Add to lookup tables */
       s_kv.value = s - tsm->sessions;
diff --git a/src/plugins/nat/nat.h b/src/plugins/nat/nat.h
index 1e8e3ca0403..4f463caa1d5 100644
--- a/src/plugins/nat/nat.h
+++ b/src/plugins/nat/nat.h
@@ -641,4 +641,16 @@ nat_send_all_to_node(vlib_main_t *vm, u32 *bi_vector,
   }
 }
 
+always_inline void
+user_session_increment(snat_main_t *sm, snat_user_t *u, u8 is_static)
+{
+  if (u->nsessions + u->nstaticsessions < sm->max_translations_per_user)
+    {
+      if (is_static)
+	u->nstaticsessions++;
+      else
+	u->nsessions++;
+    }
+}
+
 #endif /* __included_snat_h__ */
diff --git a/src/plugins/nat/out2in.c b/src/plugins/nat/out2in.c
index 397344d769a..ac47a56eb68 100755
--- a/src/plugins/nat/out2in.c
+++ b/src/plugins/nat/out2in.c
@@ -191,7 +191,7 @@ create_session_for_static_mapping (snat_main_t *sm,
   s->flags |= SNAT_SESSION_FLAG_STATIC_MAPPING;
   s->ext_host_addr.as_u32 = ip0->src_address.as_u32;
   s->ext_host_port = udp0->src_port;
-  u->nstaticsessions++;
+  user_session_increment (sm, u, 1 /* static */);
   s->in2out = in2out;
   s->out2in = out2in;
   s->in2out.protocol = out2in.protocol;
@@ -769,7 +769,7 @@ snat_out2in_unknown_proto (snat_main_t *sm,
       s->in2out.addr.as_u32 = new_addr;
       s->in2out.fib_index = m->fib_index;
       s->in2out.port = s->out2in.port = ip->protocol;
-      u->nstaticsessions++;
+      user_session_increment (sm, u, 1 /* static */);
 
       /* Add to lookup tables */
       s_kv.value = s - tsm->sessions;
@@ -881,7 +881,7 @@ snat_out2in_lb (snat_main_t *sm,
       s->outside_address_index = ~0;
       s->out2in = e_key;
       s->in2out = l_key;
-      u->nstaticsessions++;
+      user_session_increment (sm, u, 1 /* static */);
 
       /* Add to lookup tables */
       s_kv.value = s - tsm->sessions;