summaryrefslogtreecommitdiffstats
path: root/src/plugins/acl/fa_node.c
diff options
context:
space:
mode:
authorAndrew Yourtchenko <ayourtch@gmail.com>2017-07-27 15:39:50 +0200
committerAndrew Yourtchenko <ayourtch@gmail.com>2017-08-08 09:43:53 +0000
commita5e614f76fda9efb7bd1577ec18f9c0407d95d03 (patch)
tree555bdde2277dc8693ef152caf11c9229330b86bd /src/plugins/acl/fa_node.c
parent7048ff1e3af7a3979a9134cbbb8157df8e1a8a53 (diff)
acl-plugin: rework the optimization 7383, fortify acl-plugin memory behavior (VPP-910)
The further prolonged testing from testbed that reported VPP-910 has uncovered a couple of deeper issues with optimization from 7384, and the usage of subscripts rather than vec_elt_at_index() allowed to hide a couple of further errors in the code. Also, the current acl-plugin behavior of using the global heap for its dynamic data is problematic - it makes the troubleshooting much harder by potentially spreading the problem around. Based on this experience, this commits makes a few changes to fix the issues seen, also improving the serviceability of the acl-plugin code for the future: - Use separate mheaps for any ACL-related control plane operations and separate for the hash lookup datastructures, to compartmentalize any memory-related issues for the ACL plugin. - Ensure vec_elt_at_index() usage throughout the hash_lookup.c file. - Use vectors rather than raw memory for storing the "ordinary" ACL rules. - Rework the optimization from 7384 to use a separate tail pointer rather than overloading the "prev" field. - Make get_session_ptr() more conservative and adjust is_valid_session_ptr accordingly Change-Id: Ifda85193f361de5ed3782a4acd39622bd33c5830 Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com> (cherry picked from commit bd9c5ffe39e9ce61db95d74d150e07d738f24da1)
Diffstat (limited to 'src/plugins/acl/fa_node.c')
-rw-r--r--src/plugins/acl/fa_node.c16
1 files changed, 14 insertions, 2 deletions
diff --git a/src/plugins/acl/fa_node.c b/src/plugins/acl/fa_node.c
index 3181a22c81a..74079a2d4c3 100644
--- a/src/plugins/acl/fa_node.c
+++ b/src/plugins/acl/fa_node.c
@@ -621,14 +621,14 @@ acl_fa_verify_init_sessions (acl_main_t * am)
static inline fa_session_t *get_session_ptr(acl_main_t *am, u16 thread_index, u32 session_index)
{
acl_fa_per_worker_data_t *pw = &am->per_worker_data[thread_index];
- fa_session_t *sess = pw->fa_sessions_pool + session_index;
+ fa_session_t *sess = pool_is_free_index (pw->fa_sessions_pool, session_index) ? 0 : pool_elt_at_index(pw->fa_sessions_pool, session_index);
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));
+ return ((sess != 0) && ((sess - pw->fa_sessions_pool) < pool_len(pw->fa_sessions_pool)));
}
static void
@@ -731,6 +731,7 @@ acl_fa_track_session (acl_main_t * am, int is_input, u32 sw_if_index, u64 now,
static void
acl_fa_delete_session (acl_main_t * am, u32 sw_if_index, fa_full_session_id_t sess_id)
{
+ void *oldheap = clib_mem_set_heap(am->acl_mheap);
fa_session_t *sess = get_session_ptr(am, sess_id.thread_index, sess_id.session_index);
ASSERT(sess->thread_index == os_get_thread_index ());
BV (clib_bihash_add_del) (&am->fa_sessions_hash,
@@ -740,6 +741,7 @@ acl_fa_delete_session (acl_main_t * am, u32 sw_if_index, fa_full_session_id_t se
/* Deleting from timer structures not needed,
as the caller must have dealt with the timers. */
vec_validate (pw->fa_session_dels_by_sw_if_index, sw_if_index);
+ clib_mem_set_heap (oldheap);
pw->fa_session_dels_by_sw_if_index[sw_if_index]++;
clib_smp_atomic_add(&am->fa_session_total_dels, 1);
}
@@ -877,6 +879,7 @@ acl_fa_add_session (acl_main_t * am, int is_input, u32 sw_if_index, u64 now,
clib_bihash_kv_40_8_t kv;
fa_full_session_id_t f_sess_id;
uword thread_index = os_get_thread_index();
+ void *oldheap = clib_mem_set_heap(am->acl_mheap);
acl_fa_per_worker_data_t *pw = &am->per_worker_data[thread_index];
f_sess_id.thread_index = thread_index;
@@ -909,6 +912,7 @@ acl_fa_add_session (acl_main_t * am, int is_input, u32 sw_if_index, u64 now,
acl_fa_conn_list_add_session(am, f_sess_id, now);
vec_validate (pw->fa_session_adds_by_sw_if_index, sw_if_index);
+ clib_mem_set_heap (oldheap);
pw->fa_session_adds_by_sw_if_index[sw_if_index]++;
clib_smp_atomic_add(&am->fa_session_total_adds, 1);
}
@@ -1616,8 +1620,10 @@ acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable)
if (enable_disable) {
acl_fa_verify_init_sessions(am);
am->fa_total_enabled_count++;
+ void *oldheap = clib_mem_set_heap (am->vlib_main->heap_base);
vlib_process_signal_event (am->vlib_main, am->fa_cleaner_node_index,
ACL_FA_CLEANER_RESCHEDULE, 0);
+ clib_mem_set_heap (oldheap);
} else {
am->fa_total_enabled_count--;
}
@@ -1625,10 +1631,12 @@ acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable)
if (is_input)
{
ASSERT(clib_bitmap_get(am->fa_in_acl_on_sw_if_index, sw_if_index) != enable_disable);
+ void *oldheap = clib_mem_set_heap (am->vlib_main->heap_base);
vnet_feature_enable_disable ("ip4-unicast", "acl-plugin-in-ip4-fa",
sw_if_index, enable_disable, 0, 0);
vnet_feature_enable_disable ("ip6-unicast", "acl-plugin-in-ip6-fa",
sw_if_index, enable_disable, 0, 0);
+ clib_mem_set_heap (oldheap);
am->fa_in_acl_on_sw_if_index =
clib_bitmap_set (am->fa_in_acl_on_sw_if_index, sw_if_index,
enable_disable);
@@ -1636,10 +1644,12 @@ acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable)
else
{
ASSERT(clib_bitmap_get(am->fa_out_acl_on_sw_if_index, sw_if_index) != enable_disable);
+ void *oldheap = clib_mem_set_heap (am->vlib_main->heap_base);
vnet_feature_enable_disable ("ip4-output", "acl-plugin-out-ip4-fa",
sw_if_index, enable_disable, 0, 0);
vnet_feature_enable_disable ("ip6-output", "acl-plugin-out-ip6-fa",
sw_if_index, enable_disable, 0, 0);
+ clib_mem_set_heap (oldheap);
am->fa_out_acl_on_sw_if_index =
clib_bitmap_set (am->fa_out_acl_on_sw_if_index, sw_if_index,
enable_disable);
@@ -1650,9 +1660,11 @@ acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable)
#ifdef FA_NODE_VERBOSE_DEBUG
clib_warning("ENABLE-DISABLE: clean the connections on interface %d", sw_if_index);
#endif
+ void *oldheap = clib_mem_set_heap (am->vlib_main->heap_base);
vlib_process_signal_event (am->vlib_main, am->fa_cleaner_node_index,
ACL_FA_CLEANER_DELETE_BY_SW_IF_INDEX,
sw_if_index);
+ clib_mem_set_heap (oldheap);
}
}