summaryrefslogtreecommitdiffstats
path: root/src/plugins/acl/fa_node.c
diff options
context:
space:
mode:
authorAndrew Yourtchenko <ayourtch@gmail.com>2017-08-02 06:36:07 -0400
committerNeale Ranns <nranns@cisco.com>2017-08-03 17:00:35 +0000
commit22d32d916f4f3806501cf39b324be19e06b89c12 (patch)
tree95b6d1878d67b66f39f4fe4ab96266ac7760f3d0 /src/plugins/acl/fa_node.c
parent808c5b21c2759564689933d004223052b7895a42 (diff)
acl-plugin: multicore: CSIT c100k 2-core stateful ACL test does not pass (VPP-912)
Fix several threading-related issues uncovered by the CSIT scale/performance test: - make the per-interface add/del counters per-thread - preallocate the per-worker session pools rather than attempting to resize them within the datapath - move the bihash initialization to the moment of ACL being applied rather than later during the connection creation - adjust the connection cleaning logic to not require the signaling from workers to main thread - make the connection lists check in the main thread robust against workers updating the list heads at the same time - add more information to "show acl-plugin sessions" to aid in debugging Change-Id: If82ef715e4993614df11db5e9afa7fa6b522d9bc Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com> (cherry picked from commit 8e4222fc7e23a478b021930ade3cb7d20938e398)
Diffstat (limited to 'src/plugins/acl/fa_node.c')
-rw-r--r--src/plugins/acl/fa_node.c87
1 files changed, 59 insertions, 28 deletions
diff --git a/src/plugins/acl/fa_node.c b/src/plugins/acl/fa_node.c
index c483044dfd8..3181a22c81a 100644
--- a/src/plugins/acl/fa_node.c
+++ b/src/plugins/acl/fa_node.c
@@ -599,20 +599,23 @@ fa_session_get_timeout (acl_main_t * am, fa_session_t * sess)
}
static void
-acl_fa_ifc_init_sessions (acl_main_t * am, int sw_if_index0)
+acl_fa_verify_init_sessions (acl_main_t * am)
{
- /// FIXME-MULTICORE: lock around this function
-#ifdef FA_NODE_VERBOSE_DEBUG
- clib_warning
- ("Initializing bihash for sw_if_index %d num buckets %lu memory size %llu",
- sw_if_index0, am->fa_conn_table_hash_num_buckets,
- am->fa_conn_table_hash_memory_size);
-#endif
- BV (clib_bihash_init) (&am->fa_sessions_hash,
+ if (!am->fa_sessions_hash_is_initialized) {
+ u16 wk;
+ /* Allocate the per-worker sessions pools */
+ for (wk = 0; wk < vec_len (am->per_worker_data); wk++) {
+ acl_fa_per_worker_data_t *pw = &am->per_worker_data[wk];
+ pool_alloc_aligned(pw->fa_sessions_pool, am->fa_conn_table_max_entries, CLIB_CACHE_LINE_BYTES);
+ }
+
+ /* ... and the interface session hash table */
+ BV (clib_bihash_init) (&am->fa_sessions_hash,
"ACL plugin FA session bihash",
am->fa_conn_table_hash_num_buckets,
am->fa_conn_table_hash_memory_size);
- am->fa_sessions_hash_is_initialized = 1;
+ am->fa_sessions_hash_is_initialized = 1;
+ }
}
static inline fa_session_t *get_session_ptr(acl_main_t *am, u16 thread_index, u32 session_index)
@@ -622,6 +625,12 @@ static inline fa_session_t *get_session_ptr(acl_main_t *am, u16 thread_index, u3
return sess;
}
+static inline int is_valid_session_ptr(acl_main_t *am, u16 thread_index, fa_session_t *sess)
+{
+ acl_fa_per_worker_data_t *pw = &am->per_worker_data[thread_index];
+ return ((sess - pw->fa_sessions_pool) < pool_len(pw->fa_sessions_pool));
+}
+
static void
acl_fa_conn_list_add_session (acl_main_t * am, fa_full_session_id_t sess_id, u64 now)
{
@@ -648,9 +657,6 @@ acl_fa_conn_list_add_session (acl_main_t * am, fa_full_session_id_t sess_id, u64
if (~0 == pw->fa_conn_list_head[list_id]) {
pw->fa_conn_list_head[list_id] = sess_id.session_index;
- /* If it is a first conn in any list, kick the cleaner */
- vlib_process_signal_event (am->vlib_main, am->fa_cleaner_node_index,
- ACL_FA_CLEANER_RESCHEDULE, 0);
}
}
@@ -733,8 +739,8 @@ acl_fa_delete_session (acl_main_t * am, u32 sw_if_index, fa_full_session_id_t se
pool_put_index (pw->fa_sessions_pool, sess_id.session_index);
/* Deleting from timer structures not needed,
as the caller must have dealt with the timers. */
- vec_validate (am->fa_session_dels_by_sw_if_index, sw_if_index);
- am->fa_session_dels_by_sw_if_index[sw_if_index]++;
+ vec_validate (pw->fa_session_dels_by_sw_if_index, sw_if_index);
+ pw->fa_session_dels_by_sw_if_index[sw_if_index]++;
clib_smp_atomic_add(&am->fa_session_total_dels, 1);
}
@@ -749,10 +755,14 @@ acl_fa_can_add_session (acl_main_t * am, int is_input, u32 sw_if_index)
static u64
acl_fa_get_list_head_expiry_time(acl_main_t *am, acl_fa_per_worker_data_t *pw, u64 now, u16 thread_index, int timeout_type)
{
- if (~0 == pw->fa_conn_list_head[timeout_type]) {
+ fa_session_t *sess = get_session_ptr(am, thread_index, pw->fa_conn_list_head[timeout_type]);
+ /*
+ * We can not check just the index here because inbetween the worker thread might
+ * dequeue the connection from the head just as we are about to check it.
+ */
+ if (!is_valid_session_ptr(am, thread_index, sess)) {
return ~0LL; // infinity.
} else {
- fa_session_t *sess = get_session_ptr(am, thread_index, pw->fa_conn_list_head[timeout_type]);
u64 timeout_time =
sess->link_enqueue_time + fa_session_get_list_timeout (am, sess);
return timeout_time;
@@ -893,17 +903,13 @@ acl_fa_add_session (acl_main_t * am, int is_input, u32 sw_if_index, u64 now,
- if (!acl_fa_ifc_has_sessions (am, sw_if_index))
- {
- acl_fa_ifc_init_sessions (am, sw_if_index);
- }
-
+ ASSERT(am->fa_sessions_hash_is_initialized == 1);
BV (clib_bihash_add_del) (&am->fa_sessions_hash,
&kv, 1);
acl_fa_conn_list_add_session(am, f_sess_id, now);
- vec_validate (am->fa_session_adds_by_sw_if_index, sw_if_index);
- am->fa_session_adds_by_sw_if_index[sw_if_index]++;
+ vec_validate (pw->fa_session_adds_by_sw_if_index, sw_if_index);
+ pw->fa_session_adds_by_sw_if_index[sw_if_index]++;
clib_smp_atomic_add(&am->fa_session_total_adds, 1);
}
@@ -1337,8 +1343,10 @@ acl_fa_worker_conn_cleaner_process(vlib_main_t * vm,
if (num_expired >= am->fa_max_deleted_sessions_per_interval) {
/* there was too much work, we should get an interrupt ASAP */
pw->interrupt_is_needed = 1;
+ pw->interrupt_is_unwanted = 0;
} else if (num_expired <= am->fa_min_deleted_sessions_per_interval) {
/* signal that they should trigger us less */
+ pw->interrupt_is_needed = 0;
pw->interrupt_is_unwanted = 1;
} else {
/* the current rate of interrupts is ok */
@@ -1354,11 +1362,11 @@ send_one_worker_interrupt (vlib_main_t * vm, acl_main_t *am, int thread_index)
{
acl_fa_per_worker_data_t *pw = &am->per_worker_data[thread_index];
if (!pw->interrupt_is_pending) {
+ pw->interrupt_is_pending = 1;
vlib_node_set_interrupt_pending (vlib_mains[thread_index],
acl_fa_worker_session_cleaner_process_node.index);
- pw->interrupt_is_pending = 1;
/* if the interrupt was requested, mark that done. */
- pw->interrupt_is_needed = 0;
+ /* pw->interrupt_is_needed = 0; */
}
}
@@ -1425,8 +1433,8 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt,
}
}
- /* If no pending connections then no point in timing out */
- if (!has_pending_conns)
+ /* If no pending connections and no ACL applied then no point in timing out */
+ if (!has_pending_conns && (0 == am->fa_total_enabled_count))
{
am->fa_cleaner_cnt_wait_without_timeout++;
(void) vlib_process_wait_for_event (vm);
@@ -1488,6 +1496,10 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt,
clib_warning("ACL_FA_CLEANER_DELETE_BY_SW_IF_INDEX bitmap: %U", format_bitmap_hex, clear_sw_if_index_bitmap);
#endif
vec_foreach(pw0, am->per_worker_data) {
+ if ((pw0 == am->per_worker_data) && (vec_len(vlib_mains) > 1)) {
+ /* thread 0 in multithreaded scenario is not used */
+ continue;
+ }
CLIB_MEMORY_BARRIER ();
while (pw0->clear_in_process) {
CLIB_MEMORY_BARRIER ();
@@ -1522,6 +1534,10 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt,
clib_warning("CLEANER mains len: %d per-worker len: %d", vec_len(vlib_mains), vec_len(am->per_worker_data));
#endif
vec_foreach(pw0, am->per_worker_data) {
+ if ((pw0 == am->per_worker_data) && (vec_len(vlib_mains) > 1)) {
+ /* thread 0 in multithreaded scenario is not used */
+ continue;
+ }
CLIB_MEMORY_BARRIER ();
while (pw0->clear_in_process) {
CLIB_MEMORY_BARRIER ();
@@ -1563,6 +1579,10 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt,
int interrupts_unwanted = 0;
vec_foreach(pw0, am->per_worker_data) {
+ if ((pw0 == am->per_worker_data) && (vec_len(vlib_mains) > 1)) {
+ /* thread 0 in multithreaded scenario is not used */
+ continue;
+ }
if (pw0->interrupt_is_needed) {
interrupts_needed++;
/* the per-worker value is reset when sending the interrupt */
@@ -1575,6 +1595,8 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt,
if (interrupts_needed) {
/* they need more interrupts, do less waiting around next time */
am->fa_current_cleaner_timer_wait_interval /= 2;
+ /* never go into zero-wait either though - we need to give the space to others */
+ am->fa_current_cleaner_timer_wait_interval += 1;
} else if (interrupts_unwanted) {
/* slowly increase the amount of sleep up to a limit */
if (am->fa_current_cleaner_timer_wait_interval < max_timer_wait_interval)
@@ -1591,6 +1613,15 @@ void
acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable)
{
acl_main_t *am = &acl_main;
+ if (enable_disable) {
+ acl_fa_verify_init_sessions(am);
+ am->fa_total_enabled_count++;
+ vlib_process_signal_event (am->vlib_main, am->fa_cleaner_node_index,
+ ACL_FA_CLEANER_RESCHEDULE, 0);
+ } else {
+ am->fa_total_enabled_count--;
+ }
+
if (is_input)
{
ASSERT(clib_bitmap_get(am->fa_in_acl_on_sw_if_index, sw_if_index) != enable_disable);