session: session table holding free appns index

session table may be shared among multiple appns's.
  app ns add id blue secret 1 if tap0
  app ns add id red secret 1 if tap0

session table holds the last added app_ns's appns_index.
If the last app_ns is deleted, session table is not free
since there is still an appns which uses the same session
table. In that case, session table is holding the free
app_ns's appns_index and it can cause problem.

The fix is to modify appns_index in session table to hold
a vector of appns_index's instead of just the appns_index
that was last added. When the app ns is deleted, remove the
deleted appns_index from the session table's vector of
appns_index's.

Type: fix

Change-Id: Ied8bc97f185071dc89b9b56656e18efbd2995131
Signed-off-by: Steven Luong <sluong@cisco.com>

src/plugins/unittest/session_test.c

@@ -825,6 +825,8 @@ session_test_rule_table (vlib_main_t * vm, unformat_input_t * input)
   session_test_enable_rule_table_engine (vm);
 
   session_table_init (st, FIB_PROTOCOL_MAX);
+  vec_add1 (st->appns_index,
+	    app_namespace_index (app_namespace_get_default ()));
   session_rules_table_init (st, FIB_PROTOCOL_MAX);
 
   ip4_address_t lcl_ip = {
@@ -2238,6 +2240,8 @@ session_test_sdl (vlib_main_t *vm, unformat_input_t *input)
   session_test_enable_sdl_engine (vm);
 
   session_table_init (st, FIB_PROTOCOL_MAX);
+  vec_add1 (st->appns_index,
+	    app_namespace_index (app_namespace_get_default ()));
   session_rules_table_init (st, FIB_PROTOCOL_MAX);
 
   /* Add 1.2.0.0/16 */

src/vnet/session/application_namespace.c

@@ -127,7 +127,7 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
 	  st = session_table_alloc ();
 	  session_table_init (st, FIB_PROTOCOL_MAX);
 	  st->is_local = 1;
-	  st->appns_index = app_namespace_index (app_ns);
+	  vec_add1 (st->appns_index, app_namespace_index (app_ns));
 	  app_ns->local_table_index = session_table_index (st);
 	  if (a->sock_name)
 	    {
@@ -173,8 +173,10 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
       if (app_ns->sock_name)
 	vec_free (app_ns->sock_name);
 
-      session_lookup_table_cleanup (FIB_PROTOCOL_IP4, app_ns->ip4_fib_index);
-      session_lookup_table_cleanup (FIB_PROTOCOL_IP6, app_ns->ip6_fib_index);
+      session_lookup_table_cleanup (FIB_PROTOCOL_IP4, app_ns->ip4_fib_index,
+				    ns_index);
+      session_lookup_table_cleanup (FIB_PROTOCOL_IP6, app_ns->ip6_fib_index,
+				    ns_index);
 
       app_namespace_free (app_ns);
     }

src/vnet/session/session.api

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-option version = "4.0.2";
+option version = "4.0.3";
 
 import "vnet/interface_types.api";
 import "vnet/ip/ip_types.api";
@@ -411,6 +411,7 @@ autoreply define session_rule_add_del {
  */
 define session_rules_dump
 {
+  option deprecated;
   u32 client_index;
   u32 context;
 };
@@ -434,6 +435,7 @@ define session_rules_dump
   */
 define session_rules_details
 {
+  option deprecated;
   u32 context;
   vl_api_transport_proto_t transport_proto;
   vl_api_prefix_t lcl;
@@ -446,6 +448,49 @@ define session_rules_details
   string tag[64];
 };
 
+/** \brief Dump session rules
+    @param client_index - opaque cookie to identify the sender
+    @param context - sender context, to match reply w/ request
+ */
+define session_rules_v2_dump
+{
+  u32 client_index;
+  u32 context;
+};
+
+/** \brief Session rules details
+    @param context - sender context, to match reply w/ request
+    @param transport_proto - transport protocol
+    @param is_ip4 - flag to indicate if ip addresses are ip4 or 6
+    @param lcl_ip - local ip
+    @param lcl_plen - local prefix length
+    @param rmt_ip - remote ip
+    @param rmt_ple - remote prefix length
+    @param lcl_port - local port
+    @param rmt_port - remote port
+    @param action_index - the only action defined now is forward to
+                          application with index action_index
+    @param scope - enum that indicates scope of the rule: global or local.
+                   If 0, default is global, 1 is global 2 is local, 3 is both
+    @param tag - tag
+    @param count - count of the number of appns_index
+    @param appns_index - application namespaces where rule is to be applied to
+  */
+define session_rules_v2_details
+{
+  u32 context;
+  vl_api_transport_proto_t transport_proto;
+  vl_api_prefix_t lcl;
+  vl_api_prefix_t rmt;
+  u16 lcl_port;
+  u16 rmt_port;
+  u32 action_index;
+  vl_api_session_rule_scope_t scope;
+  string tag[64];
+  u32 count;
+  u32 appns_index[count];
+};
+
 autoreply define session_sdl_add_del {
   option deprecated;
   u32 client_index;
@@ -500,6 +545,7 @@ define session_sdl_details
  */
 define session_sdl_v2_dump
 {
+  option deprecated;
   u32 client_index;
   u32 context;
 };
@@ -514,6 +560,7 @@ define session_sdl_v2_dump
   */
 define session_sdl_v2_details
 {
+  option deprecated;
   u32 context;
   vl_api_prefix_t rmt;
   u32 action_index;
@@ -521,6 +568,35 @@ define session_sdl_v2_details
   string tag[64];
 };
 
+/** \brief Dump session sdl v3
+    @param client_index - opaque cookie to identify the sender
+    @param context - sender context, to match reply w/ request
+ */
+define session_sdl_v3_dump
+{
+  u32 client_index;
+  u32 context;
+};
+
+/** \brief Session sdl details v3
+    @param context - sender context, to match reply w/ request
+    @param rmt - remote prefix
+    @param action_index - the only action defined now is forward to
+                          application with index action_index
+    @param tag - tag
+    @param count - count of the number of appns_index
+    @param appns_index - application namespaces where rule is to be applied to
+  */
+define session_sdl_v3_details
+{
+  u32 context;
+  vl_api_prefix_t rmt;
+  u32 action_index;
+  string tag[64];
+  u32 count;
+  u32 appns_index[count];
+};
+
 /*
  * Local Variables:
  * eval: (c-set-style "gnu")

src/vnet/session/session_lookup.c

@@ -1455,7 +1455,7 @@ session_lookup_set_tables_appns (app_namespace_t * app_ns)
       st = session_table_get_or_alloc (fp, fib_index);
       if (st)
 	{
-	  st->appns_index = app_namespace_index (app_ns);
+	  vec_add1 (st->appns_index, app_namespace_index (app_ns));
 	  session_lookup_fib_table_lock (fib_index, fp);
 	}
     }
@@ -1945,23 +1945,30 @@ session_lookup_init (void)
 }
 
 void
-session_lookup_table_cleanup (u32 fib_proto, u32 fib_index)
+session_lookup_table_cleanup (u32 fib_proto, u32 fib_index, u32 ns_index)
 {
   session_table_t *st;
-  u32 table_index;
+  u32 table_index, appns_index;
+  int i;
 
   session_lookup_fib_table_unlock (fib_index, fib_proto);
+  table_index = session_lookup_get_index_for_fib (fib_proto, fib_index);
+  st = session_table_get (table_index);
+  if (st == 0)
+    return;
   if (fib_index_to_lock_count[fib_proto][fib_index] == 0)
     {
-      table_index = session_lookup_get_index_for_fib (fib_proto, fib_index);
-      st = session_table_get (table_index);
-      if (st)
-	{
-	  session_table_free (st, fib_proto);
-	  if (vec_len (fib_index_to_table_index[fib_proto]) > fib_index)
-	    fib_index_to_table_index[fib_proto][fib_index] = ~0;
-	}
+      session_table_free (st, fib_proto);
+      if (vec_len (fib_index_to_table_index[fib_proto]) > fib_index)
+	fib_index_to_table_index[fib_proto][fib_index] = ~0;
     }
+  else
+    vec_foreach_index (i, st->appns_index)
+      {
+	appns_index = *vec_elt_at_index (st->appns_index, i);
+	if (ns_index == appns_index)
+	  vec_del1 (st->appns_index, i);
+      }
 }
 
 /*

src/vnet/session/session_sdl.c

@@ -244,13 +244,17 @@ session_sdl_table_init (session_table_t *st, u8 fib_proto)
   session_sdl_block_t *sdlb;
   u8 all = fib_proto > FIB_PROTOCOL_IP6 ? 1 : 0;
   char name[80];
-  app_namespace_t *app_ns = app_namespace_get (st->appns_index);
+  u32 appns_index;
+  app_namespace_t *app_ns;
   session_rules_table_group_t *srtg;
 
   /* Don't support local table */
   if (st->is_local == 1)
     return;
 
+  appns_index =
+    *vec_elt_at_index (st->appns_index, vec_len (st->appns_index) - 1);
+  app_ns = app_namespace_get (appns_index);
   srtg = srtg_instance_alloc (st, 0);
   srt = srtg->session_rules;
   sdlb = &srt->sdl_block;

src/vnet/session/session_table.c

@@ -79,6 +79,7 @@ session_table_free (session_table_t *slt, u8 fib_proto)
       clib_bihash_free_48_8 (&slt->v6_half_open_hash);
     }
 
+  vec_free (slt->appns_index);
   pool_put (lookup_tables, slt);
 }
 
@@ -222,7 +223,17 @@ u8 *
 format_session_table (u8 *s, va_list *args)
 {
   session_table_t *st = va_arg (*args, session_table_t *);
+  u32 appns_index, i;
 
+  s = format (s, "appns index: ");
+  vec_foreach_index (i, st->appns_index)
+    {
+      appns_index = *vec_elt_at_index (st->appns_index, i);
+      if (i > 0)
+	s = format (s, ", ");
+      s = format (s, "%d", appns_index);
+    }
+  s = format (s, "\n");
   if (clib_bihash_is_initialised_16_8 (&st->v4_session_hash))
     {
       s = format (s, "%U", format_bihash_16_8, &st->v4_session_hash, 0);

src/vnet/session/session_table.h

@@ -42,7 +42,7 @@ typedef struct _session_lookup_table
   u8 is_local;
 
   /** Namespace this table belongs to */
-  u32 appns_index;
+  u32 *appns_index;
 
   /** For global tables only one fib proto is active. This is a
    * byproduct of fib table ids not necessarily being the same for
@@ -77,7 +77,7 @@ session_table_t *_get_session_tables ();
 #define session_table_foreach(VAR, BODY)		\
   pool_foreach (VAR, _get_session_tables ()) BODY
 
-void session_lookup_table_cleanup (u32 fib_proto, u32 fib_index);
+void session_lookup_table_cleanup (u32 fib_proto, u32 fib_index, u32 ns_index);
 
 #endif /* SRC_VNET_SESSION_SESSION_TABLE_H_ */
 /*

src/vnet/session/session_test.c

@@ -382,6 +382,11 @@ vl_api_session_sdl_v2_details_t_handler (vl_api_session_sdl_v2_details_t *mp)
 {
 }
 
+static void
+vl_api_session_sdl_v3_details_t_handler (vl_api_session_sdl_v3_details_t *mp)
+{
+}
+
 static int
 api_session_sdl_dump (vat_main_t *vam)
 {
@@ -394,6 +399,24 @@ api_session_sdl_v2_dump (vat_main_t *vam)
   return -1;
 }
 
+static int
+api_session_sdl_v3_dump (vat_main_t *vam)
+{
+  return -1;
+}
+
+static void
+vl_api_session_rules_v2_details_t_handler (
+  vl_api_session_rules_v2_details_t *mp)
+{
+}
+
+static int
+api_session_rules_v2_dump (vat_main_t *vam)
+{
+  return -1;
+}
+
 #include <vnet/session/session.api_test.c>
 
 /*

test/asf/test_session.py

@@ -71,7 +71,7 @@ class TestSession(VppAsfTestCase):
         )
 
         super(TestSession, self).tearDown()
-        self.vapi.session_enable_disable(is_enable=1)
+        self.vapi.session_enable_disable(is_enable=0)
 
     def test_segment_manager_alloc(self):
         """Session Segment Manager Multiple Segment Allocation"""
@@ -122,6 +122,89 @@ class TestSession(VppAsfTestCase):
         ip_t10.remove_vpp_config()
 
 
+@tag_fixme_vpp_workers
+class TestApplicationNamespace(VppAsfTestCase):
+    """Application Namespacee"""
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestApplicationNamespace, cls).setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        super(TestApplicationNamespace, cls).tearDownClass()
+
+    def setUp(self):
+        super(TestApplicationNamespace, self).setUp()
+        self.create_loopback_interfaces(1)
+
+    def tearDown(self):
+        super(TestApplicationNamespace, self).tearDown()
+        self.vapi.session_enable_disable_v2(
+            rt_engine_type=VppEnum.vl_api_rt_backend_engine_t.RT_BACKEND_ENGINE_API_DISABLE
+        )
+
+    def test_application_namespace(self):
+        """Application Namespace Create"""
+
+        self.vapi.session_enable_disable_v2(
+            rt_engine_type=VppEnum.vl_api_rt_backend_engine_t.RT_BACKEND_ENGINE_API_RULE_TABLE
+        )
+
+        # Configure 2 namespaces, sharing the same interface
+        app0 = self.vapi.app_namespace_add_del_v4(
+            namespace_id="0", sw_if_index=self.loop0.sw_if_index
+        )
+        app1 = self.vapi.app_namespace_add_del_v4(
+            namespace_id="1", sw_if_index=self.loop0.sw_if_index
+        )
+
+        self.vapi.session_rule_add_del(
+            transport_proto=VppEnum.vl_api_transport_proto_t.TRANSPORT_PROTO_API_TCP,
+            lcl="172.100.1.1/32",
+            rmt="172.100.1.2/32",
+            lcl_port=5000,
+            rmt_port=5000,
+            action_index=1,
+            appns_index=app0.appns_index,
+            scope=VppEnum.vl_api_session_rule_scope_t.SESSION_RULE_SCOPE_API_GLOBAL,
+            is_add=1,
+        )
+        dump = self.vapi.session_rules_v2_dump()
+        # session table should contain 3 appns's indices (default, app0, and app1)
+        self.assertEqual(len(dump[1].appns_index), 3)
+        self.assertEqual(dump[1].count, 3)
+        self.assertEqual(dump[1].appns_index[0], 0)
+        self.assertEqual(dump[1].appns_index[1], app0.appns_index)
+        self.assertEqual(dump[1].appns_index[2], app1.appns_index)
+
+        # remove the last namespace
+        self.vapi.app_namespace_add_del_v4(
+            namespace_id="1", sw_if_index=self.loop0.sw_if_index, is_add=0
+        )
+        dump = self.vapi.session_rules_v2_dump()
+        # session table should contain the remainging appns's index
+        self.assertEqual(len(dump[1].appns_index), 2)
+        self.assertEqual(dump[1].count, 2)
+        self.assertEqual(dump[1].appns_index[0], 0)
+        self.assertEqual(dump[1].appns_index[1], app0.appns_index)
+
+        self.vapi.app_namespace_add_del_v4(
+            namespace_id="0", sw_if_index=self.loop0.sw_if_index, is_add=0
+        )
+        self.vapi.session_rule_add_del(
+            transport_proto=VppEnum.vl_api_transport_proto_t.TRANSPORT_PROTO_API_TCP,
+            lcl="172.100.1.1/32",
+            rmt="172.100.1.2/32",
+            lcl_port=5000,
+            rmt_port=5000,
+            action_index=1,
+            appns_index=app0.appns_index,
+            scope=VppEnum.vl_api_session_rule_scope_t.SESSION_RULE_SCOPE_API_GLOBAL,
+            is_add=0,
+        )
+
+
 @tag_fixme_vpp_workers
 class TestSessionUnitTests(VppAsfTestCase):
     """Session Unit Tests Case"""

test/asf/test_session_sdl.py

@@ -78,10 +78,10 @@ class TestSessionSDL(VppTestCase):
         )
 
         # Configure namespaces
-        self.vapi.app_namespace_add_del_v4(
+        app0 = self.vapi.app_namespace_add_del_v4(
             namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
-        self.vapi.app_namespace_add_del_v4(
+        app1 = self.vapi.app_namespace_add_del_v4(
             namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )
 
@@ -120,8 +120,12 @@ class TestSessionSDL(VppTestCase):
         )
         self.apply_rules(rules, is_add=1, appns_index=0)
 
-        filter = self.vapi.session_sdl_v2_dump()
+        filter = self.vapi.session_sdl_v3_dump()
         self.assertEqual(filter[0].rmt, IPv4Network(self.loop1.local_ip4 + "/32"))
+        self.assertEqual(len(filter[0].appns_index), 2)
+        self.assertEqual(filter[0].count, 2)
+        self.assertEqual(filter[0].appns_index[0], 0)
+        self.assertEqual(filter[0].appns_index[1], app0.appns_index)
 
         # irrelevant rules - add 64k entries in one API call
         rules = []
@@ -158,6 +162,11 @@ class TestSessionSDL(VppTestCase):
         self.vapi.app_namespace_add_del_v4(
             is_add=0, namespace_id="0", sw_if_index=self.loop0.sw_if_index
         )
+        filter = self.vapi.session_sdl_v3_dump()
+        self.assertEqual(len(filter[0].appns_index), 1)
+        self.assertEqual(filter[0].count, 1)
+        self.assertEqual(filter[0].appns_index[0], 0)
+
         self.vapi.app_namespace_add_del_v4(
             is_add=0, namespace_id="1", sw_if_index=self.loop1.sw_if_index
         )