User session counters stay <= per-user limit 42/11342/2
authorMatthew Smith <mgsmith@netgate.com>
Fri, 23 Mar 2018 13:30:16 +0000 (08:30 -0500)
committerFlorin Coras <florin.coras@gmail.com>
Sat, 24 Mar 2018 07:24:13 +0000 (07:24 +0000)
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>
src/plugins/nat/in2out.c
src/plugins/nat/nat.h
src/plugins/nat/out2in.c

index 8b565b1..c23d372 100755 (executable)
@@ -310,6 +310,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)))
     {
@@ -349,12 +350,9 @@ 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
-    {
-      u->nstaticsessions++;
-    }
+    is_sm = 1;
 
   s = nat_session_alloc_or_recycle (sm, u, thread_index);
   if (!s)
@@ -363,8 +361,9 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
       return SNAT_IN2OUT_NEXT_DROP;
     }
 
-  if (address_index == ~0)
+  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;
@@ -1266,14 +1265,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;
@@ -1396,7 +1389,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;
index 58bf8b3..15643aa 100644 (file)
@@ -652,4 +652,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__ */
index ebd0dc4..00f887d 100755 (executable)
@@ -192,7 +192,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;
@@ -833,7 +833,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;
@@ -945,7 +945,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;