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>
This commit is contained in:
Steven Luong
2024-07-17 16:16:05 -07:00
committed by Florin Coras
parent 5c4c1b63b9
commit 5682ca8ef6
5 changed files with 56 additions and 93 deletions

View File

@ -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 @@ 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)
{
@ -188,11 +138,7 @@ vnet_app_namespace_add_del (vnet_app_namespace_add_del_args_t *a)
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 @@ 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);
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 @@ 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));

View File

@ -37,6 +37,8 @@ static session_lookup_main_t sl_main;
*/
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 @@ vnet_session_rule_add_del (session_rule_add_del_args_t *args)
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 @@ session_lookup_set_tables_appns (app_namespace_t * app_ns)
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 @@ session_lookup_init (void)
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_lookup_init (void)
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
*

View File

@ -32,6 +32,7 @@ typedef enum session_lookup_result_
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,

View File

@ -78,6 +78,8 @@ 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);
#endif /* SRC_VNET_SESSION_SESSION_TABLE_H_ */
/*
* fd.io coding-style-patch-verification: ON

View File

@ -1006,34 +1006,6 @@ 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"""