From e6423bef32ca2ffcfcd7a092eb4673badd53ea4c Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Tue, 8 Aug 2017 20:10:12 +0200 Subject: acl-plugin: avoid crash in multithreaded setup adding/deleting ACLs with traffic (VPP-910/VPP-929) The commit fixing the VPP-910 and separating the memory operations into separate heaps has missed setting the MHEAP_FLAG_THREAD_SAFE, which quite obviously caused the issues in the multithread setup. Fix that. Also, add the debug CLIs "set acl-plugin heap {main|hash} {validate|trace} {1|0}" to toggle the memory instrumentation, in case we ever need it in the future. Change-Id: I8bd4f7978613f5ea75a030cfb90674dac34ae7bf Signed-off-by: Andrew Yourtchenko --- src/plugins/acl/acl.c | 50 +++++++++++++++++++++++++++++++++++++++++++ src/plugins/acl/hash_lookup.c | 29 +++++++++++++++++++++++++ src/plugins/acl/hash_lookup.h | 4 ++++ 3 files changed, 83 insertions(+) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index 6e8613b8571..c02ba42aa96 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -89,11 +89,39 @@ acl_set_heap(acl_main_t *am) { if (0 == am->acl_mheap) { am->acl_mheap = mheap_alloc (0 /* use VM */ , 2 << 29); + mheap_t *h = mheap_header (am->acl_mheap); + h->flags |= MHEAP_FLAG_THREAD_SAFE; } void *oldheap = clib_mem_set_heap(am->acl_mheap); return oldheap; } +void +acl_plugin_acl_set_validate_heap(acl_main_t *am, int on) +{ + clib_mem_set_heap(acl_set_heap(am)); + mheap_t *h = mheap_header (am->acl_mheap); + if (on) { + h->flags |= MHEAP_FLAG_VALIDATE; + h->flags &= ~MHEAP_FLAG_SMALL_OBJECT_CACHE; + mheap_validate(h); + } else { + h->flags &= ~MHEAP_FLAG_VALIDATE; + h->flags |= MHEAP_FLAG_SMALL_OBJECT_CACHE; + } +} + +void +acl_plugin_acl_set_trace_heap(acl_main_t *am, int on) +{ + clib_mem_set_heap(acl_set_heap(am)); + mheap_t *h = mheap_header (am->acl_mheap); + if (on) { + h->flags |= MHEAP_FLAG_TRACE; + } else { + h->flags &= ~MHEAP_FLAG_TRACE; + } +} static void vl_api_acl_plugin_get_version_t_handler (vl_api_acl_plugin_get_version_t * mp) @@ -1868,6 +1896,8 @@ acl_sw_interface_add_del (vnet_main_t * vnm, u32 sw_if_index, u32 is_add) VNET_SW_INTERFACE_ADD_DEL_FUNCTION (acl_sw_interface_add_del); + + static clib_error_t * acl_set_aclplugin_fn (vlib_main_t * vm, unformat_input_t * input, @@ -1896,6 +1926,26 @@ acl_set_aclplugin_fn (vlib_main_t * vm, am->l4_match_nonfirst_fragment = (val != 0); goto done; } + if (unformat (input, "heap")) + { + if (unformat(input, "main")) + { + if (unformat(input, "validate %u", &val)) + acl_plugin_acl_set_validate_heap(am, val); + else if (unformat(input, "trace %u", &val)) + acl_plugin_acl_set_trace_heap(am, val); + goto done; + } + else if (unformat(input, "hash")) + { + if (unformat(input, "validate %u", &val)) + acl_plugin_hash_acl_set_validate_heap(am, val); + else if (unformat(input, "trace %u", &val)) + acl_plugin_hash_acl_set_trace_heap(am, val); + goto done; + } + goto done; + } if (unformat (input, "session")) { if (unformat (input, "table")) { /* The commands here are for tuning/testing. No user-serviceable parts inside */ diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c index a337eb84586..ae522d921cc 100644 --- a/src/plugins/acl/hash_lookup.c +++ b/src/plugins/acl/hash_lookup.c @@ -269,11 +269,40 @@ hash_acl_set_heap(acl_main_t *am) { if (0 == am->hash_lookup_mheap) { am->hash_lookup_mheap = mheap_alloc (0 /* use VM */ , 2 << 25); + mheap_t *h = mheap_header (am->hash_lookup_mheap); + h->flags |= MHEAP_FLAG_THREAD_SAFE; } void *oldheap = clib_mem_set_heap(am->hash_lookup_mheap); return oldheap; } +void +acl_plugin_hash_acl_set_validate_heap(acl_main_t *am, int on) +{ + clib_mem_set_heap(hash_acl_set_heap(am)); + mheap_t *h = mheap_header (am->hash_lookup_mheap); + if (on) { + h->flags |= MHEAP_FLAG_VALIDATE; + h->flags &= ~MHEAP_FLAG_SMALL_OBJECT_CACHE; + mheap_validate(h); + } else { + h->flags &= ~MHEAP_FLAG_VALIDATE; + h->flags |= MHEAP_FLAG_SMALL_OBJECT_CACHE; + } +} + +void +acl_plugin_hash_acl_set_trace_heap(acl_main_t *am, int on) +{ + clib_mem_set_heap(hash_acl_set_heap(am)); + mheap_t *h = mheap_header (am->hash_lookup_mheap); + if (on) { + h->flags |= MHEAP_FLAG_TRACE; + } else { + h->flags &= ~MHEAP_FLAG_TRACE; + } +} + void hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) { diff --git a/src/plugins/acl/hash_lookup.h b/src/plugins/acl/hash_lookup.h index c5913624f81..2d7058e80ee 100644 --- a/src/plugins/acl/hash_lookup.h +++ b/src/plugins/acl/hash_lookup.h @@ -57,4 +57,8 @@ hash_multi_acl_match_5tuple (u32 sw_if_index, fa_5tuple_t * pkt_5tuple, int is_l */ void show_hash_acl_hash(vlib_main_t * vm, acl_main_t *am, u32 verbose); +/* Debug functions to turn validate/trace on and off */ +void acl_plugin_hash_acl_set_validate_heap(acl_main_t *am, int on); +void acl_plugin_hash_acl_set_trace_heap(acl_main_t *am, int on); + #endif -- cgit 1.2.3-korg