session: application namespace may reference a deleted vrf table

lock the vrf table when adding an application namespace and
unlock the vrf table when deleting an application namespace.

Free the session table when no more application namespace
uses it anymore to avoid memory leaks.

Type: fix

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

7 files changed, 148 insertions, 5 deletions

diff --git a/src/vnet/session/application_namespace.c b/src/vnet/session/application_namespace.c
index f547dcfc031..2520188d88c 100644
--- a/src/vnet/session/application_namespace.c
+++ b/src/vnet/session/application_namespace.c
@@ -22,6 +22,12 @@
 #include <vppinfra/format_table.h>
 #include <vlib/unix/unix.h>
 
+/*
+ * fib source when locking the fib table
+ */
+static fib_source_t app_namespace_fib_src = FIB_SOURCE_INVALID;
+static u32 *fib_index_to_lock_count[FIB_PROTOCOL_IP6 + 1];
+
 /**
  * Hash table of application namespaces by app ns ids
  */
@@ -81,6 +87,50 @@ app_namespace_alloc (const u8 *ns_id)
   return app_ns;
 }
 
+static void
+app_namespace_fib_table_lock (u32 fib_index, u32 protocol)
+{
+  fib_table_lock (fib_index, protocol, app_namespace_fib_src);
+  vec_validate (fib_index_to_lock_count[protocol], fib_index);
+  fib_index_to_lock_count[protocol][fib_index]++;
+  ASSERT (fib_index_to_lock_count[protocol][fib_index] > 0);
+}
+
+static void
+app_namespace_fib_table_unlock (u32 fib_index, u32 protocol)
+{
+  fib_table_unlock (fib_index, protocol, app_namespace_fib_src);
+  ASSERT (fib_index_to_lock_count[protocol][fib_index] > 0);
+  fib_index_to_lock_count[protocol][fib_index]--;
+}
+
+static void
+app_namespace_del_global_table (app_namespace_t *app_ns)
+{
+  session_table_t *st;
+  u32 table_index;
+
+  app_namespace_fib_table_unlock (app_ns->ip4_fib_index, FIB_PROTOCOL_IP4);
+  if (fib_index_to_lock_count[FIB_PROTOCOL_IP4][app_ns->ip4_fib_index] == 0)
+    {
+      table_index = session_lookup_get_index_for_fib (FIB_PROTOCOL_IP4,
+						      app_ns->ip4_fib_index);
+      st = session_table_get (table_index);
+      if (st)
+	session_table_free (st, FIB_PROTOCOL_IP4);
+    }
+
+  app_namespace_fib_table_unlock (app_ns->ip6_fib_index, FIB_PROTOCOL_IP6);
+  if (fib_index_to_lock_count[FIB_PROTOCOL_IP6][app_ns->ip6_fib_index] == 0)
+    {
+      table_index = session_lookup_get_index_for_fib (FIB_PROTOCOL_IP6,
+						      app_ns->ip6_fib_index);
+      st = session_table_get (table_index);
+      if (st)
+	session_table_free (st, FIB_PROTOCOL_IP6);
+    }
+}
+
 session_error_t
 vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
 {
@@ -136,12 +186,14 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
 
       app_ns->ns_secret = a->secret;
       app_ns->sw_if_index = a->sw_if_index;
-      app_ns->ip4_fib_index =
-	fib_table_find (FIB_PROTOCOL_IP4, a->ip4_fib_id);
-      app_ns->ip6_fib_index =
-	fib_table_find (FIB_PROTOCOL_IP6, a->ip6_fib_id);
-      session_lookup_set_tables_appns (app_ns);
 
+      app_ns->ip4_fib_index = fib_table_find (FIB_PROTOCOL_IP4, a->ip4_fib_id);
+      app_namespace_fib_table_lock (app_ns->ip4_fib_index, FIB_PROTOCOL_IP4);
+
+      app_ns->ip6_fib_index = fib_table_find (FIB_PROTOCOL_IP6, a->ip6_fib_id);
+      app_namespace_fib_table_lock (app_ns->ip6_fib_index, FIB_PROTOCOL_IP6);
+
+      session_lookup_set_tables_appns (app_ns);
     }
   else
     {
@@ -164,6 +216,8 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
       if (app_ns->sock_name)
 	vec_free (app_ns->sock_name);
 
+      app_namespace_del_global_table (app_ns);
+
       app_namespace_free (app_ns);
     }
 
@@ -236,6 +290,15 @@ app_namespaces_init (void)
 {
   u8 *ns_id = format (0, "default");
 
+  /* We are not contributing any route to the fib. But we allocate a fib source
+   * so that when we lock the fib table, we can view that we have a lock on the
+   * particular fib table in case we wonder why the fib table is not free after
+   * "ip table del"
+   */
+  if (app_namespace_fib_src == FIB_SOURCE_INVALID)
+    app_namespace_fib_src = fib_source_allocate (
+      "application namespace", FIB_SOURCE_PRIORITY_LOW, FIB_SOURCE_BH_SIMPLE);
+
   if (!app_namespace_lookup_table)
     app_namespace_lookup_table =
       hash_create_vec (0, sizeof (u8), sizeof (uword));
diff --git a/test/asf/test_quic.py b/test/asf/test_quic.py
index e453bd5b3e5..c4fa6912114 100644
--- a/test/asf/test_quic.py
+++ b/test/asf/test_quic.py
@@ -117,6 +117,18 @@ class QUICTestCase(VppAsfTestCase):
         self.logger.debug(self.vapi.cli("show ip fib"))
 
     def tearDown(self):
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0,
+            namespace_id=self.server_appns,
+            secret=self.server_appns_secret,
+            sw_if_index=self.loop0.sw_if_index,
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0,
+            namespace_id=self.client_appns,
+            secret=self.client_appns_secret,
+            sw_if_index=self.loop1.sw_if_index,
+        )
         # Delete inter-table routes
         self.ip_t01.remove_vpp_config()
         self.ip_t10.remove_vpp_config()
diff --git a/test/asf/test_session.py b/test/asf/test_session.py
index 741773d6989..184ec4fba54 100644
--- a/test/asf/test_session.py
+++ b/test/asf/test_session.py
@@ -60,6 +60,14 @@ class TestSession(VppAsfTestCase):
             i.set_table_ip4(0)
             i.admin_down()
 
+        # Unconfigure namespaces - remove our locks to the vrf tables
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="0", sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", sw_if_index=self.loop1.sw_if_index
+        )
+
         super(TestSession, self).tearDown()
         self.vapi.session_enable_disable(is_enable=1)
 
diff --git a/test/asf/test_tcp.py b/test/asf/test_tcp.py
index 3edcd718b00..23772d34c76 100644
--- a/test/asf/test_tcp.py
+++ b/test/asf/test_tcp.py
@@ -48,6 +48,12 @@ class TestTCP(VppAsfTestCase):
         )
 
     def tearDown(self):
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="0", sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", sw_if_index=self.loop1.sw_if_index
+        )
         for i in self.lo_interfaces:
             i.unconfig_ip4()
             i.set_table_ip4(0)
diff --git a/test/asf/test_tls.py b/test/asf/test_tls.py
index d2d1d9a4747..2ce87143339 100644
--- a/test/asf/test_tls.py
+++ b/test/asf/test_tls.py
@@ -91,6 +91,12 @@ class TestTLS(VppAsfTestCase):
         )
 
     def tearDown(self):
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="0", sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", sw_if_index=self.loop1.sw_if_index
+        )
         for i in self.lo_interfaces:
             i.unconfig_ip4()
             i.set_table_ip4(0)
diff --git a/test/asf/test_vcl.py b/test/asf/test_vcl.py
index a0141be80b2..a186c6fe8c5 100644
--- a/test/asf/test_vcl.py
+++ b/test/asf/test_vcl.py
@@ -189,6 +189,12 @@ class VCLTestCase(VppAsfTestCase):
         self.logger.debug(self.vapi.cli("show ip fib"))
 
     def thru_host_stack_tear_down(self):
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", secret=1234, sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="2", secret=5678, sw_if_index=self.loop1.sw_if_index
+        )
         for i in self.lo_interfaces:
             i.unconfig_ip4()
             i.set_table_ip4(0)
@@ -240,6 +246,12 @@ class VCLTestCase(VppAsfTestCase):
         self.logger.debug(self.vapi.cli("show ip6 fib"))
 
     def thru_host_stack_ipv6_tear_down(self):
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", secret=1234, sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="2", secret=5678, sw_if_index=self.loop1.sw_if_index
+        )
         for i in self.lo_interfaces:
             i.unconfig_ip6()
             i.set_table_ip6(0)
@@ -994,6 +1006,34 @@ class LDPThruHostStackIperf(VCLTestCase):
             iperf3, self.server_iperf3_args, iperf3, self.client_iperf3_args
         )
 
+
+class LDPThruHostStackIperfMss(VCLTestCase):
+    """LDP Thru Host Stack Iperf with MSS"""
+
+    @classmethod
+    def setUpClass(cls):
+        super(LDPThruHostStackIperfMss, cls).setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        super(LDPThruHostStackIperfMss, cls).tearDownClass()
+
+    def setUp(self):
+        super(LDPThruHostStackIperfMss, self).setUp()
+
+        self.thru_host_stack_setup()
+        self.client_iperf3_timeout = 20
+        self.client_iperf3_args = ["-4", "-t 2", "-c", self.loop0.local_ip4]
+        self.server_iperf3_args = ["-4", "-s"]
+
+    def tearDown(self):
+        self.thru_host_stack_tear_down()
+        super(LDPThruHostStackIperfMss, self).tearDown()
+
+    def show_commands_at_teardown(self):
+        self.logger.debug(self.vapi.cli("show session verbose 2"))
+        self.logger.debug(self.vapi.cli("show app mq"))
+
     @unittest.skipUnless(_have_iperf3, "'%s' not found, Skipping.")
     def test_ldp_thru_host_stack_iperf3_mss(self):
         """run LDP thru host stack iperf3 test with mss option"""
diff --git a/test/test_udp.py b/test/test_udp.py
index edcd293197f..2c0710ba2e2 100644
--- a/test/test_udp.py
+++ b/test/test_udp.py
@@ -725,6 +725,14 @@ class TestUDP(VppTestCase):
             i.unconfig_ip4()
             i.set_table_ip4(0)
             i.admin_down()
+        # Unconfigure namespaces - remove our locks to the vrf tables
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="0", sw_if_index=self.loop0.sw_if_index
+        )
+        self.vapi.app_namespace_add_del_v4(
+            is_add=0, namespace_id="1", sw_if_index=self.loop1.sw_if_index
+        )
+
         self.vapi.session_enable_disable(is_enable=0)
         super(TestUDP, self).tearDown()