session: delete and add application namespace do not create the global session table
When an application namespace is added, we call session_table_is_alloced
to see if we need to allocate a new session table. That check returns true
even if we removed the session table.
The fix is when we delete an application's global session table,
we need to invalidate fib_index_to_table_index.
Fixed test_vcl test script to run two tests back to back.
The 1st test deletes the application namespace at the end.
The 2nd test adds the application namespace in the beginning.
Type: fix
Fixes: 67bae20b05cb46e5f6d19afeaf1f7a52a5309d59
Change-Id: I67f5cc1b726a07659597a9479df011717db08d0a
Signed-off-by: Steven Luong <sluong@cisco.com>
diff --git a/src/vnet/session/application_namespace.c b/src/vnet/session/application_namespace.c
index 2520188..dbc6a6b 100644
--- a/src/vnet/session/application_namespace.c
+++ b/src/vnet/session/application_namespace.c
@@ -22,12 +22,6 @@
#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
*/
@@ -87,50 +81,6 @@
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)
{
@@ -188,11 +138,7 @@
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_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
@@ -216,7 +162,8 @@
if (app_ns->sock_name)
vec_free (app_ns->sock_name);
- app_namespace_del_global_table (app_ns);
+ session_lookup_table_cleanup (FIB_PROTOCOL_IP4, app_ns->ip4_fib_index);
+ session_lookup_table_cleanup (FIB_PROTOCOL_IP6, app_ns->ip6_fib_index);
app_namespace_free (app_ns);
}
@@ -290,15 +237,6 @@
{
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/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c
index ff6e581..1e58e5e 100644
--- a/src/vnet/session/session_lookup.c
+++ b/src/vnet/session/session_lookup.c
@@ -37,6 +37,8 @@
*/
static u32 *fib_index_to_table_index[2];
+static u32 *fib_index_to_lock_count[2];
+
/* 16 octets */
typedef CLIB_PACKED (struct {
union
@@ -1392,6 +1394,23 @@
return rv;
}
+static void
+session_lookup_fib_table_lock (u32 fib_index, u32 protocol)
+{
+ fib_table_lock (fib_index, protocol, sl_main.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
+session_lookup_fib_table_unlock (u32 fib_index, u32 protocol)
+{
+ fib_table_unlock (fib_index, protocol, sl_main.fib_src);
+ ASSERT (fib_index_to_lock_count[protocol][fib_index] > 0);
+ fib_index_to_lock_count[protocol][fib_index]--;
+}
+
/**
* Mark (global) tables as pertaining to app ns
*/
@@ -1407,7 +1426,10 @@
fib_index = app_namespace_get_fib_index (app_ns, fp);
st = session_table_get_or_alloc (fp, fib_index);
if (st)
- st->appns_index = app_namespace_index (app_ns);
+ {
+ st->appns_index = app_namespace_index (app_ns);
+ session_lookup_fib_table_lock (fib_index, fp);
+ }
}
}
@@ -1859,6 +1881,14 @@
clib_spinlock_init (&slm->st_alloc_lock);
+ /* 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"
+ */
+ slm->fib_src = fib_source_allocate (
+ "session lookup", FIB_SOURCE_PRIORITY_LOW, FIB_SOURCE_BH_SIMPLE);
+
/*
* Allocate default table and map it to fib_index 0
*/
@@ -1874,6 +1904,26 @@
session_table_init (st, FIB_PROTOCOL_IP6);
}
+void
+session_lookup_table_cleanup (u32 fib_proto, u32 fib_index)
+{
+ session_table_t *st;
+ u32 table_index;
+
+ session_lookup_fib_table_unlock (fib_index, fib_proto);
+ 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;
+ }
+ }
+}
+
/*
* fd.io coding-style-patch-verification: ON
*
diff --git a/src/vnet/session/session_lookup.h b/src/vnet/session/session_lookup.h
index f9ffc15..95fcca6 100644
--- a/src/vnet/session/session_lookup.h
+++ b/src/vnet/session/session_lookup.h
@@ -32,6 +32,7 @@
typedef struct session_lookup_main_
{
clib_spinlock_t st_alloc_lock;
+ fib_source_t fib_src;
} session_lookup_main_t;
session_t *session_lookup_safe4 (u32 fib_index, ip4_address_t * lcl,
diff --git a/src/vnet/session/session_table.h b/src/vnet/session/session_table.h
index 636b8d7..bcbb6ec 100644
--- a/src/vnet/session/session_table.h
+++ b/src/vnet/session/session_table.h
@@ -78,6 +78,8 @@
#define session_table_foreach(VAR, BODY) \
pool_foreach (VAR, _get_session_tables ()) BODY
+void session_lookup_table_cleanup (u32 fib_proto, u32 fib_index);
+
#endif /* SRC_VNET_SESSION_SESSION_TABLE_H_ */
/*
* fd.io coding-style-patch-verification: ON