From a5e614f76fda9efb7bd1577ec18f9c0407d95d03 Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Thu, 27 Jul 2017 15:39:50 +0200 Subject: [PATCH] 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 (cherry picked from commit bd9c5ffe39e9ce61db95d74d150e07d738f24da1) --- src/plugins/acl/acl.c | 156 ++++++++++++++++------ src/plugins/acl/acl.h | 6 + src/plugins/acl/fa_node.c | 16 ++- src/plugins/acl/hash_lookup.c | 253 +++++++++++++++++++++--------------- src/plugins/acl/hash_lookup_types.h | 8 +- 5 files changed, 289 insertions(+), 150 deletions(-) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index 6cd2d0c605f..bc38265a6c5 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -83,6 +83,18 @@ VLIB_PLUGIN_REGISTER () = { }; /* *INDENT-ON* */ + +static void * +acl_set_heap(acl_main_t *am) +{ + if (0 == am->acl_mheap) { + am->acl_mheap = mheap_alloc (0 /* use VM */ , 2 << 29); + } + void *oldheap = clib_mem_set_heap(am->acl_mheap); + return oldheap; +} + + static void vl_api_acl_plugin_get_version_t_handler (vl_api_acl_plugin_get_version_t * mp) { @@ -130,7 +142,7 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], acl_main_t *am = &acl_main; acl_list_t *a; acl_rule_t *r; - acl_rule_t *acl_new_rules; + acl_rule_t *acl_new_rules = 0; int i; if (*acl_list_index != ~0) @@ -139,22 +151,23 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], if (pool_is_free_index (am->acls, *acl_list_index)) { /* tried to replace a non-existent ACL, no point doing anything */ + clib_warning("acl-plugin-error: Trying to replace nonexistent ACL %d (tag %s)", *acl_list_index, tag); return -1; } } + if (0 == count) { + clib_warning("acl-plugin-warning: supplied no rules for ACL %d (tag %s)", *acl_list_index, tag); + } + + void *oldheap = acl_set_heap(am); /* Create and populate the rules */ - acl_new_rules = clib_mem_alloc_aligned (sizeof (acl_rule_t) * count, - CLIB_CACHE_LINE_BYTES); - if (!acl_new_rules) - { - /* Could not allocate rules. New or existing ACL - bail out regardless */ - return -1; - } + if (count > 0) + vec_validate(acl_new_rules, count-1); for (i = 0; i < count; i++) { - r = &acl_new_rules[i]; + r = vec_elt_at_index(acl_new_rules, i); memset(r, 0, sizeof(*r)); r->is_permit = rules[i].is_permit; r->is_ipv6 = rules[i].is_ipv6; @@ -192,13 +205,14 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], a = am->acls + *acl_list_index; hash_acl_delete(am, *acl_list_index); /* Get rid of the old rules */ - clib_mem_free (a->rules); + if (a->rules) + vec_free (a->rules); } a->rules = acl_new_rules; a->count = count; memcpy (a->tag, tag, sizeof (a->tag)); hash_acl_add(am, *acl_list_index); - + clib_mem_set_heap (oldheap); return 0; } @@ -226,6 +240,7 @@ acl_del_list (u32 acl_list_index) } } + void *oldheap = acl_set_heap(am); /* delete any references to the ACL */ for (i = 0; i < vec_len (am->output_acl_vec_by_sw_if_index); i++) { @@ -263,10 +278,10 @@ acl_del_list (u32 acl_list_index) /* now we can delete the ACL itself */ a = &am->acls[acl_list_index]; if (a->rules) - { - clib_mem_free (a->rules); - } + vec_free (a->rules); + pool_put (am->acls, a); + clib_mem_set_heap (oldheap); return 0; } @@ -359,13 +374,15 @@ acl_classify_add_del_table_tiny (vnet_classify_main_t * cm, u8 * mask, if (0 == match) match = 1; - - return vnet_classify_add_del_table (cm, skip_mask_ptr, nbuckets, + void *oldheap = clib_mem_set_heap (cm->vlib_main->heap_base); + int ret = vnet_classify_add_del_table (cm, skip_mask_ptr, nbuckets, memory_size, skip, match, next_table_index, miss_next_index, table_index, current_data_flag, current_data_offset, is_add, 1 /* delete_chain */); + clib_mem_set_heap (oldheap); + return ret; } static int @@ -385,12 +402,15 @@ acl_classify_add_del_table_small (vnet_classify_main_t * cm, u8 * mask, if (0 == match) match = 1; - return vnet_classify_add_del_table (cm, skip_mask_ptr, nbuckets, + void *oldheap = clib_mem_set_heap (cm->vlib_main->heap_base); + int ret = vnet_classify_add_del_table (cm, skip_mask_ptr, nbuckets, memory_size, skip, match, next_table_index, miss_next_index, table_index, current_data_flag, current_data_offset, is_add, 1 /* delete_chain */); + return ret; + clib_mem_set_heap (oldheap); } @@ -400,12 +420,15 @@ acl_unhook_l2_input_classify (acl_main_t * am, u32 sw_if_index) vnet_classify_main_t *cm = &vnet_classify_main; u32 ip4_table_index = ~0; u32 ip6_table_index = ~0; + void *oldheap = acl_set_heap(am); vec_validate_init_empty (am->acl_ip4_input_classify_table_by_sw_if_index, sw_if_index, ~0); vec_validate_init_empty (am->acl_ip6_input_classify_table_by_sw_if_index, sw_if_index, ~0); + /* switch to global heap while calling vnet_* functions */ + clib_mem_set_heap (cm->vlib_main->heap_base); vnet_l2_input_classify_enable_disable (sw_if_index, 0); if (am->acl_ip4_input_classify_table_by_sw_if_index[sw_if_index] != ~0) @@ -428,7 +451,7 @@ acl_unhook_l2_input_classify (acl_main_t * am, u32 sw_if_index) am->l2_input_classify_next_acl_ip6, &ip6_table_index, 0); } - + clib_mem_set_heap (oldheap); return 0; } @@ -438,12 +461,16 @@ acl_unhook_l2_output_classify (acl_main_t * am, u32 sw_if_index) vnet_classify_main_t *cm = &vnet_classify_main; u32 ip4_table_index = ~0; u32 ip6_table_index = ~0; + void *oldheap = acl_set_heap(am); vec_validate_init_empty (am->acl_ip4_output_classify_table_by_sw_if_index, sw_if_index, ~0); vec_validate_init_empty (am->acl_ip6_output_classify_table_by_sw_if_index, sw_if_index, ~0); + /* switch to global heap while calling vnet_* functions */ + clib_mem_set_heap (cm->vlib_main->heap_base); + vnet_l2_output_classify_enable_disable (sw_if_index, 0); if (am->acl_ip4_output_classify_table_by_sw_if_index[sw_if_index] != ~0) @@ -466,7 +493,7 @@ acl_unhook_l2_output_classify (acl_main_t * am, u32 sw_if_index) am->l2_output_classify_next_acl_ip6, &ip6_table_index, 0); } - + clib_mem_set_heap (oldheap); return 0; } @@ -478,6 +505,8 @@ acl_hook_l2_input_classify (acl_main_t * am, u32 sw_if_index) u32 ip6_table_index = ~0; int rv; + void *prevheap = clib_mem_set_heap (cm->vlib_main->heap_base); + /* in case there were previous tables attached */ acl_unhook_l2_input_classify (am, sw_if_index); rv = @@ -486,7 +515,7 @@ acl_hook_l2_input_classify (acl_main_t * am, u32 sw_if_index) am->l2_input_classify_next_acl_ip4, &ip4_table_index, 1); if (rv) - return rv; + goto done; rv = acl_classify_add_del_table_tiny (cm, ip6_5tuple_mask, sizeof (ip6_5tuple_mask) - 1, ~0, @@ -498,7 +527,7 @@ acl_hook_l2_input_classify (acl_main_t * am, u32 sw_if_index) sizeof (ip4_5tuple_mask) - 1, ~0, am->l2_input_classify_next_acl_ip4, &ip4_table_index, 0); - return rv; + goto done; } rv = vnet_l2_input_classify_set_tables (sw_if_index, ip4_table_index, @@ -516,7 +545,7 @@ acl_hook_l2_input_classify (acl_main_t * am, u32 sw_if_index) sizeof (ip4_5tuple_mask) - 1, ~0, am->l2_input_classify_next_acl_ip4, &ip4_table_index, 0); - return rv; + goto done; } am->acl_ip4_input_classify_table_by_sw_if_index[sw_if_index] = @@ -525,6 +554,8 @@ acl_hook_l2_input_classify (acl_main_t * am, u32 sw_if_index) ip6_table_index; vnet_l2_input_classify_enable_disable (sw_if_index, 1); +done: + clib_mem_set_heap (prevheap); return rv; } @@ -536,6 +567,8 @@ acl_hook_l2_output_classify (acl_main_t * am, u32 sw_if_index) u32 ip6_table_index = ~0; int rv; + void *prevheap = clib_mem_set_heap (cm->vlib_main->heap_base); + /* in case there were previous tables attached */ acl_unhook_l2_output_classify (am, sw_if_index); rv = @@ -544,7 +577,7 @@ acl_hook_l2_output_classify (acl_main_t * am, u32 sw_if_index) am->l2_output_classify_next_acl_ip4, &ip4_table_index, 1); if (rv) - return rv; + goto done; rv = acl_classify_add_del_table_tiny (cm, ip6_5tuple_mask, sizeof (ip6_5tuple_mask) - 1, ~0, @@ -556,7 +589,7 @@ acl_hook_l2_output_classify (acl_main_t * am, u32 sw_if_index) sizeof (ip4_5tuple_mask) - 1, ~0, am->l2_output_classify_next_acl_ip4, &ip4_table_index, 0); - return rv; + goto done; } rv = vnet_l2_output_classify_set_tables (sw_if_index, ip4_table_index, @@ -574,7 +607,7 @@ acl_hook_l2_output_classify (acl_main_t * am, u32 sw_if_index) sizeof (ip4_5tuple_mask) - 1, ~0, am->l2_output_classify_next_acl_ip4, &ip4_table_index, 0); - return rv; + goto done; } am->acl_ip4_output_classify_table_by_sw_if_index[sw_if_index] = @@ -583,6 +616,8 @@ acl_hook_l2_output_classify (acl_main_t * am, u32 sw_if_index) ip6_table_index; vnet_l2_output_classify_enable_disable (sw_if_index, 1); +done: + clib_mem_set_heap (prevheap); return rv; } @@ -653,6 +688,7 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) /* ACL is not defined. Can not apply */ return -1; } + void *oldheap = acl_set_heap(am); if (is_input) { @@ -663,6 +699,7 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) clib_warning("ACL %d is already applied inbound on sw_if_index %d (index %d)", acl_list_index, sw_if_index, index); /* the entry is already there */ + clib_mem_set_heap (oldheap); return -1; } /* if there was no ACL applied before, enable the ACL processing */ @@ -684,6 +721,7 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) clib_warning("ACL %d is already applied outbound on sw_if_index %d (index %d)", acl_list_index, sw_if_index, index); /* the entry is already there */ + clib_mem_set_heap (oldheap); return -1; } /* if there was no ACL applied before, enable the ACL processing */ @@ -696,6 +734,7 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) vec_add (am->output_sw_if_index_vec_by_acl[acl_list_index], &sw_if_index, 1); } + clib_mem_set_heap (oldheap); return 0; } @@ -706,6 +745,7 @@ acl_interface_del_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) acl_main_t *am = &acl_main; int i; int rv = -1; + void *oldheap = acl_set_heap(am); if (is_input) { vec_validate (am->input_acl_vec_by_sw_if_index, sw_if_index); @@ -764,6 +804,7 @@ acl_interface_del_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) acl_interface_out_enable_disable (am, sw_if_index, 0); } } + clib_mem_set_heap (oldheap); return rv; } @@ -772,6 +813,7 @@ acl_interface_reset_inout_acls (u32 sw_if_index, u8 is_input) { acl_main_t *am = &acl_main; int i; + void *oldheap = acl_set_heap(am); if (is_input) { vec_validate (am->input_acl_vec_by_sw_if_index, sw_if_index); @@ -812,6 +854,7 @@ acl_interface_reset_inout_acls (u32 sw_if_index, u8 is_input) vec_reset_length (am->output_acl_vec_by_sw_if_index[sw_if_index]); } + clib_mem_set_heap (oldheap); } static int @@ -820,6 +863,7 @@ acl_interface_add_del_inout_acl (u32 sw_if_index, u8 is_add, u8 is_input, { int rv = -1; acl_main_t *am = &acl_main; + void *oldheap = acl_set_heap(am); if (is_add) { rv = @@ -835,6 +879,7 @@ acl_interface_add_del_inout_acl (u32 sw_if_index, u8 is_add, u8 is_input, rv = acl_interface_del_inout_acl (sw_if_index, is_input, acl_list_index); } + clib_mem_set_heap (oldheap); return rv; } @@ -1015,6 +1060,7 @@ macip_create_classify_tables (acl_main_t * am, u32 macip_acl_index) macip_find_match_type (mvec, a->rules[i].src_mac_mask, a->rules[i].src_prefixlen, a->rules[i].is_ipv6); + ASSERT(match_type_index != ~0); /* add session to table mvec[match_type_index].table_index; */ vnet_classify_add_del_session (cm, mvec[match_type_index].table_index, mask, a->rules[i].is_permit ? ~0 : 0, i, @@ -1066,17 +1112,15 @@ macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], acl_main_t *am = &acl_main; macip_acl_list_t *a; macip_acl_rule_t *r; - macip_acl_rule_t *acl_new_rules; + macip_acl_rule_t *acl_new_rules = 0; int i; - + if (0 == count) { + clib_warning("acl-plugin-warning: Trying to create empty MACIP ACL (tag %s)", tag); + } + void *oldheap = acl_set_heap(am); /* Create and populate the rules */ - acl_new_rules = clib_mem_alloc_aligned (sizeof (macip_acl_rule_t) * count, - CLIB_CACHE_LINE_BYTES); - if (!acl_new_rules) - { - /* Could not allocate rules. New or existing ACL - bail out regardless */ - return -1; - } + if (count > 0) + vec_validate(acl_new_rules, count-1); for (i = 0; i < count; i++) { @@ -1104,7 +1148,7 @@ macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], /* Create and populate the classifer tables */ macip_create_classify_tables (am, *acl_list_index); - + clib_mem_set_heap (oldheap); return 0; } @@ -1117,7 +1161,9 @@ macip_acl_interface_del_acl (acl_main_t * am, u32 sw_if_index) int rv; u32 macip_acl_index; macip_acl_list_t *a; + void *oldheap = acl_set_heap(am); vec_validate_init_empty (am->macip_acl_by_sw_if_index, sw_if_index, ~0); + clib_mem_set_heap (oldheap); macip_acl_index = am->macip_acl_by_sw_if_index[sw_if_index]; /* No point in deleting MACIP ACL which is not applied */ if (~0 == macip_acl_index) @@ -1144,12 +1190,15 @@ macip_acl_interface_add_acl (acl_main_t * am, u32 sw_if_index, { return -1; } + void *oldheap = acl_set_heap(am); a = &am->macip_acls[macip_acl_index]; vec_validate_init_empty (am->macip_acl_by_sw_if_index, sw_if_index, ~0); /* If there already a MACIP ACL applied, unapply it */ if (~0 != am->macip_acl_by_sw_if_index[sw_if_index]) macip_acl_interface_del_acl(am, sw_if_index); am->macip_acl_by_sw_if_index[sw_if_index] = macip_acl_index; + clib_mem_set_heap (oldheap); + /* Apply the classifier tables for L2 ACLs */ rv = vnet_set_input_acl_intfc (am->vlib_main, sw_if_index, a->ip4_table_index, @@ -1161,6 +1210,7 @@ static int macip_acl_del_list (u32 acl_list_index) { acl_main_t *am = &acl_main; + void *oldheap = acl_set_heap(am); macip_acl_list_t *a; int i; if (pool_is_free_index (am->macip_acls, acl_list_index)) @@ -1184,9 +1234,10 @@ macip_acl_del_list (u32 acl_list_index) a = &am->macip_acls[acl_list_index]; if (a->rules) { - clib_mem_free (a->rules); + vec_free (a->rules); } pool_put (am->macip_acls, a); + clib_mem_set_heap (oldheap); return 0; } @@ -1196,6 +1247,7 @@ macip_acl_interface_add_del_acl (u32 sw_if_index, u8 is_add, u32 acl_list_index) { acl_main_t *am = &acl_main; + void *oldheap = acl_set_heap(am); int rv = -1; if (is_add) { @@ -1205,6 +1257,7 @@ macip_acl_interface_add_del_acl (u32 sw_if_index, u8 is_add, { rv = macip_acl_interface_del_acl (am, sw_if_index); } + clib_mem_set_heap (oldheap); return rv; } @@ -1363,6 +1416,7 @@ send_acl_details (acl_main_t * am, unix_shared_memory_queue_t * q, vl_api_acl_rule_t *rules; int i; int msg_size = sizeof (*mp) + sizeof (mp->r[0]) * acl->count; + void *oldheap = acl_set_heap(am); mp = vl_msg_api_alloc (msg_size); memset (mp, 0, msg_size); @@ -1381,6 +1435,7 @@ send_acl_details (acl_main_t * am, unix_shared_memory_queue_t * q, } clib_warning("Sending acl details for ACL index %d", ntohl(mp->acl_index)); + clib_mem_set_heap (oldheap); vl_msg_api_send_shmem (q, (u8 *) & mp); } @@ -1439,6 +1494,7 @@ send_acl_interface_list_details (acl_main_t * am, int n_output; int count; int i = 0; + void *oldheap = acl_set_heap(am); vec_validate (am->input_acl_vec_by_sw_if_index, sw_if_index); vec_validate (am->output_acl_vec_by_sw_if_index, sw_if_index); @@ -1469,7 +1525,7 @@ send_acl_interface_list_details (acl_main_t * am, mp->acls[n_input + i] = htonl (am->output_acl_vec_by_sw_if_index[sw_if_index][i]); } - + clib_mem_set_heap (oldheap); vl_msg_api_send_shmem (q, (u8 *) & mp); } @@ -1784,6 +1840,7 @@ static int acl_set_skip_ipv6_eh(u32 eh, u32 value) { acl_main_t *am = &acl_main; + if ((eh < 256) && (value < 2)) { am->fa_ipv6_known_eh_bitmap = clib_bitmap_set(am->fa_ipv6_known_eh_bitmap, eh, value); @@ -2104,6 +2161,21 @@ acl_show_aclplugin_fn (vlib_main_t * vm, vlib_cli_output(vm, "\n%s\n", out0); vec_free(out0); } + else if (unformat (input, "memory")) + { + vlib_cli_output (vm, "ACL plugin main heap statistics:\n"); + if (am->acl_mheap) { + vlib_cli_output (vm, " %U\n", format_mheap, am->acl_mheap, 1); + } else { + vlib_cli_output (vm, " Not initialized\n"); + } + vlib_cli_output (vm, "ACL hash lookup support heap statistics:\n"); + if (am->hash_lookup_mheap) { + vlib_cli_output (vm, " %U\n", format_mheap, am->hash_lookup_mheap, 1); + } else { + vlib_cli_output (vm, " Not initialized\n"); + } + } else if (unformat (input, "tables")) { ace_mask_type_entry_t *mte; @@ -2198,9 +2270,9 @@ acl_show_aclplugin_fn (vlib_main_t * vm, out0 = format(out0, " input lookup applied entries:\n"); for(j=0; jinput_hash_entry_vec_by_sw_if_index[swi]); j++) { applied_hash_ace_entry_t *pae = &am->input_hash_entry_vec_by_sw_if_index[swi][j]; - out0 = format(out0, " %4d: acl %d rule %d action %d bitmask-ready rule %d next %d prev %d\n", + out0 = format(out0, " %4d: acl %d rule %d action %d bitmask-ready rule %d next %d prev %d tail %d\n", j, pae->acl_index, pae->ace_index, pae->action, pae->hash_ace_info_index, - pae->next_applied_entry_index, pae->prev_applied_entry_index); + pae->next_applied_entry_index, pae->prev_applied_entry_index, pae->tail_applied_entry_index); } } @@ -2212,9 +2284,9 @@ acl_show_aclplugin_fn (vlib_main_t * vm, out0 = format(out0, " output lookup applied entries:\n"); for(j=0; joutput_hash_entry_vec_by_sw_if_index[swi]); j++) { applied_hash_ace_entry_t *pae = &am->output_hash_entry_vec_by_sw_if_index[swi][j]; - out0 = format(out0, " %4d: acl %d rule %d action %d bitmask-ready rule %d next %d prev %d\n", + out0 = format(out0, " %4d: acl %d rule %d action %d bitmask-ready rule %d next %d prev %d tail %d\n", j, pae->acl_index, pae->ace_index, pae->action, pae->hash_ace_info_index, - pae->next_applied_entry_index, pae->prev_applied_entry_index); + pae->next_applied_entry_index, pae->prev_applied_entry_index, pae->tail_applied_entry_index); } } @@ -2288,6 +2360,7 @@ acl_init (vlib_main_t * vm) vec_free (name); acl_setup_fa_nodes(); + am->session_timeout_sec[ACL_TIMEOUT_TCP_TRANSIENT] = TCP_SESSION_TRANSIENT_TIMEOUT_SEC; am->session_timeout_sec[ACL_TIMEOUT_TCP_IDLE] = TCP_SESSION_IDLE_TIMEOUT_SEC; am->session_timeout_sec[ACL_TIMEOUT_UDP_IDLE] = UDP_SESSION_IDLE_TIMEOUT_SEC; @@ -2329,7 +2402,6 @@ acl_init (vlib_main_t * vm) am->l4_match_nonfirst_fragment = 1; /* use the new fancy hash-based matching */ - // NOT IMMEDIATELY am->use_hash_acl_matching = 1; return error; diff --git a/src/plugins/acl/acl.h b/src/plugins/acl/acl.h index b84ed7a42dc..14c6129c07f 100644 --- a/src/plugins/acl/acl.h +++ b/src/plugins/acl/acl.h @@ -122,12 +122,18 @@ typedef struct } ace_mask_type_entry_t; typedef struct { + /* mheap to hold all the ACL module related allocations, other than hash */ + void *acl_mheap; + /* API message ID base */ u16 msg_id_base; acl_list_t *acls; /* Pool of ACLs */ hash_acl_info_t *hash_acl_infos; /* corresponding hash matching housekeeping info */ clib_bihash_48_8_t acl_lookup_hash; /* ACL lookup hash table. */ + + /* mheap to hold all the miscellaneous allocations related to hash-based lookups */ + void *hash_lookup_mheap; int acl_lookup_hash_initialized; applied_hash_ace_entry_t **input_hash_entry_vec_by_sw_if_index; applied_hash_ace_entry_t **output_hash_entry_vec_by_sw_if_index; 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); } } diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c index 027dabd0e6a..a337eb84586 100644 --- a/src/plugins/acl/hash_lookup.c +++ b/src/plugins/acl/hash_lookup.c @@ -33,6 +33,16 @@ #include "hash_lookup.h" #include "hash_lookup_private.h" + +static inline applied_hash_ace_entry_t **get_applied_hash_aces(acl_main_t *am, int is_input, u32 sw_if_index) +{ + applied_hash_ace_entry_t **applied_hash_aces = is_input ? vec_elt_at_index(am->input_hash_entry_vec_by_sw_if_index, sw_if_index) + : vec_elt_at_index(am->output_hash_entry_vec_by_sw_if_index, sw_if_index); + return applied_hash_aces; +} + + + /* * This returns true if there is indeed a match on the portranges. * With all these levels of indirections, this is not going to be very fast, @@ -41,11 +51,10 @@ static int match_portranges(acl_main_t *am, fa_5tuple_t *match, u32 index) { - applied_hash_ace_entry_t **applied_hash_aces = match->pkt.is_input ? &am->input_hash_entry_vec_by_sw_if_index[match->pkt.sw_if_index] : - &am->output_hash_entry_vec_by_sw_if_index[match->pkt.sw_if_index]; - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[index]); - // hash_acl_info_t *ha = &am->hash_acl_infos[pae->acl_index]; + applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, match->pkt.is_input, match->pkt.sw_if_index); + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), index); + acl_rule_t *r = &(am->acls[pae->acl_index].rules[pae->ace_index]); DBG("PORTMATCH: %d <= %d <= %d && %d <= %d <= %d ?", r->src_port_or_type_first, match->l4.port[0], r->src_port_or_type_last, @@ -70,8 +79,7 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match) u32 sw_if_index = match->pkt.sw_if_index; u8 is_input = match->pkt.is_input; - applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] : - &am->output_hash_entry_vec_by_sw_if_index[sw_if_index]; + applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index); applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index : &am->output_applied_hash_acl_info_by_sw_if_index; @@ -79,11 +87,11 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match) pmatch[0], pmatch[1], pmatch[2], pmatch[3], pmatch[4], pmatch[5]); for(mask_type_index=0; mask_type_index < pool_len(am->ace_mask_type_pool); mask_type_index++) { - if (!clib_bitmap_get((*applied_hash_acls)[sw_if_index].mask_type_index_bitmap, mask_type_index)) { + if (!clib_bitmap_get(vec_elt_at_index((*applied_hash_acls), sw_if_index)->mask_type_index_bitmap, mask_type_index)) { /* This bit is not set. Avoid trying to match */ continue; } - ace_mask_type_entry_t *mte = &am->ace_mask_type_pool[mask_type_index]; + ace_mask_type_entry_t *mte = vec_elt_at_index(am->ace_mask_type_pool, mask_type_index); pmatch = (u64 *)match; pmask = (u64 *)&mte->mask; pkey = (u64 *)kv.key; @@ -120,7 +128,7 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match) u32 curr_index = result_val->applied_entry_index; while ((curr_index != ~0) && !match_portranges(am, match, curr_index)) { /* while no match and there are more entries, walk... */ - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[curr_index]); + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces),curr_index); DBG("entry %d did not portmatch, advancing to %d", curr_index, pae->next_applied_entry_index); curr_index = pae->next_applied_entry_index; } @@ -153,9 +161,9 @@ multi_acl_match_get_applied_ace_index(acl_main_t *am, fa_5tuple_t *match) static void hashtable_add_del(acl_main_t *am, clib_bihash_kv_48_8_t *kv, int is_add) { - DBG("HASH ADD/DEL: %016llx %016llx %016llx %016llx %016llx %016llx add %d", + DBG("HASH ADD/DEL: %016llx %016llx %016llx %016llx %016llx %016llx %016llx add %d", kv->key[0], kv->key[1], kv->key[2], - kv->key[3], kv->key[4], kv->key[5], is_add); + kv->key[3], kv->key[4], kv->key[5], kv->value, is_add); BV (clib_bihash_add_del) (&am->acl_lookup_hash, kv, is_add); } @@ -167,17 +175,17 @@ fill_applied_hash_ace_kv(acl_main_t *am, { fa_5tuple_t *kv_key = (fa_5tuple_t *)kv->key; hash_acl_lookup_value_t *kv_val = (hash_acl_lookup_value_t *)&kv->value; - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]); - hash_acl_info_t *ha = &am->hash_acl_infos[pae->acl_index]; + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index); + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, pae->acl_index); - memcpy(kv_key, &ha->rules[pae->hash_ace_info_index].match, sizeof(*kv_key)); + memcpy(kv_key, &(vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->match), sizeof(*kv_key)); /* initialize the sw_if_index and direction */ kv_key->pkt.sw_if_index = sw_if_index; kv_key->pkt.is_input = is_input; kv_val->as_u64 = 0; kv_val->applied_entry_index = new_index; - kv_val->need_portrange_check = ha->rules[pae->hash_ace_info_index].src_portrange_not_powerof2 || - ha->rules[pae->hash_ace_info_index].dst_portrange_not_powerof2; + kv_val->need_portrange_check = vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->src_portrange_not_powerof2 || + vec_elt_at_index(ha->rules, pae->hash_ace_info_index)->dst_portrange_not_powerof2; /* by default assume all values are shadowed -> check all mask types */ kv_val->shadowed = 1; } @@ -203,7 +211,9 @@ activate_applied_ace_hash_entry(acl_main_t *am, u32 new_index) { clib_bihash_kv_48_8_t kv; - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]); + ASSERT(new_index != ~0); + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index); + DBG("activate_applied_ace_hash_entry sw_if_index %d is_input %d new_index %d", sw_if_index, is_input, new_index); fill_applied_hash_ace_kv(am, applied_hash_aces, sw_if_index, is_input, new_index, &kv); @@ -214,28 +224,28 @@ activate_applied_ace_hash_entry(acl_main_t *am, clib_bihash_kv_48_8_t result; hash_acl_lookup_value_t *result_val = (hash_acl_lookup_value_t *)&result.value; int res = BV (clib_bihash_search) (&am->acl_lookup_hash, &kv, &result); + ASSERT(new_index != ~0); + ASSERT(new_index < vec_len((*applied_hash_aces))); if (res == 0) { /* There already exists an entry or more. Append at the end. */ u32 first_index = result_val->applied_entry_index; - DBG("A key already exists, with applied entry index: %d", existing_index); ASSERT(first_index != ~0); - applied_hash_ace_entry_t *first_pae = &((*applied_hash_aces)[first_index]); - /* move to "prev" by one, this should land us in the end of the list */ - u32 last_index = first_pae->prev_applied_entry_index; + DBG("A key already exists, with applied entry index: %d", first_index); + applied_hash_ace_entry_t *first_pae = vec_elt_at_index((*applied_hash_aces), first_index); + u32 last_index = first_pae->tail_applied_entry_index; ASSERT(last_index != ~0); - applied_hash_ace_entry_t *last_pae = &((*applied_hash_aces)[last_index]); - ASSERT(last_pae->next_applied_entry_index == ~0); + applied_hash_ace_entry_t *last_pae = vec_elt_at_index((*applied_hash_aces), last_index); + DBG("...advance to chained entry index: %d", last_index); /* link ourseves in */ last_pae->next_applied_entry_index = new_index; pae->prev_applied_entry_index = last_index; - /* make a new reference from the very first element to new tail */ - first_pae->prev_applied_entry_index = new_index; + /* adjust the pointer to the new tail */ + first_pae->tail_applied_entry_index = new_index; } else { /* It's the very first entry */ hashtable_add_del(am, &kv, 1); - pae->is_first_entry = 1; - /* we are the tail */ - pae->prev_applied_entry_index = new_index; + ASSERT(new_index != ~0); + pae->tail_applied_entry_index = new_index; } } @@ -254,31 +264,44 @@ applied_hash_entries_analyze(acl_main_t *am, applied_hash_ace_entry_t **applied_ */ } +static void * +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); + } + void *oldheap = clib_mem_set_heap(am->hash_lookup_mheap); + return oldheap; +} + void hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) { int i; DBG("HASH ACL apply: sw_if_index %d is_input %d acl %d", sw_if_index, is_input, acl_index); - u32 *acl_vec = is_input ? am->input_acl_vec_by_sw_if_index[sw_if_index] : - am->output_acl_vec_by_sw_if_index[sw_if_index]; + if (!am->acl_lookup_hash_initialized) { + BV (clib_bihash_init) (&am->acl_lookup_hash, "ACL plugin rule lookup bihash", + 65536, 2 << 25); + am->acl_lookup_hash_initialized = 1; + } + + u32 *acl_vec = is_input ? *vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index) + : *vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index); + + void *oldheap = hash_acl_set_heap(am); if (is_input) { vec_validate(am->input_hash_entry_vec_by_sw_if_index, sw_if_index); } else { vec_validate(am->output_hash_entry_vec_by_sw_if_index, sw_if_index); } vec_validate(am->hash_acl_infos, acl_index); - applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] : - &am->output_hash_entry_vec_by_sw_if_index[sw_if_index]; + applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index); + u32 order_index = vec_search(acl_vec, acl_index); - hash_acl_info_t *ha = &am->hash_acl_infos[acl_index]; + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index); ASSERT(order_index != ~0); - if (!am->acl_lookup_hash_initialized) { - BV (clib_bihash_init) (&am->acl_lookup_hash, "ACL plugin rule lookup bihash", - 65536, 2 << 25); - am->acl_lookup_hash_initialized = 1; - } int base_offset = vec_len(*applied_hash_aces); /* Update the bitmap of the mask types with which the lookup @@ -286,7 +309,7 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index : &am->output_applied_hash_acl_info_by_sw_if_index; vec_validate((*applied_hash_acls), sw_if_index); - applied_hash_acl_info_t *pal = &(*applied_hash_acls)[sw_if_index]; + applied_hash_acl_info_t *pal = vec_elt_at_index((*applied_hash_acls), sw_if_index); pal->mask_type_index_bitmap = clib_bitmap_or(pal->mask_type_index_bitmap, ha->mask_type_index_bitmap); /* @@ -305,8 +328,7 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) /* add the rules from the ACL to the hash table for lookup and append to the vector*/ for(i=0; i < vec_len(ha->rules); i++) { u32 new_index = base_offset + i; - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[new_index]); - pae->is_first_entry = 0; + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), new_index); pae->acl_index = acl_index; pae->ace_index = ha->rules[i].ace_index; pae->action = ha->rules[i].action; @@ -314,9 +336,29 @@ hash_acl_apply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) /* we might link it in later */ pae->next_applied_entry_index = ~0; pae->prev_applied_entry_index = ~0; + pae->tail_applied_entry_index = ~0; activate_applied_ace_hash_entry(am, sw_if_index, is_input, applied_hash_aces, new_index); } applied_hash_entries_analyze(am, applied_hash_aces); + clib_mem_set_heap (oldheap); +} + +static u32 +find_head_applied_ace_index(applied_hash_ace_entry_t **applied_hash_aces, u32 curr_index) +{ + /* + * find back the first entry. Inefficient so might need to be a bit cleverer + * if this proves to be a problem.. + */ + u32 an_index = curr_index; + ASSERT(an_index != ~0); + applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), an_index); + while(head_pae->prev_applied_entry_index != ~0) { + an_index = head_pae->prev_applied_entry_index; + ASSERT(an_index != ~0); + head_pae = vec_elt_at_index((*applied_hash_aces), an_index); + } + return an_index; } static void @@ -325,43 +367,43 @@ move_applied_ace_hash_entry(acl_main_t *am, applied_hash_ace_entry_t **applied_hash_aces, u32 old_index, u32 new_index) { + ASSERT(old_index != ~0); + ASSERT(new_index != ~0); /* move the entry */ - (*applied_hash_aces)[new_index] = (*applied_hash_aces)[old_index]; + *vec_elt_at_index((*applied_hash_aces), new_index) = *vec_elt_at_index((*applied_hash_aces), old_index); /* update the linkage and hash table if necessary */ - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[old_index]); + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), old_index); - if (!pae->is_first_entry) { - applied_hash_ace_entry_t *prev_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]); + if (pae->prev_applied_entry_index != ~0) { + applied_hash_ace_entry_t *prev_pae = vec_elt_at_index((*applied_hash_aces), pae->prev_applied_entry_index); ASSERT(prev_pae->next_applied_entry_index == old_index); prev_pae->next_applied_entry_index = new_index; } else { /* first entry - so the hash points to it, update */ add_del_hashtable_entry(am, sw_if_index, is_input, applied_hash_aces, new_index, 1); + ASSERT(pae->tail_applied_entry_index != ~0); } if (pae->next_applied_entry_index != ~0) { - applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]); + applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index); ASSERT(next_pae->prev_applied_entry_index == old_index); next_pae->prev_applied_entry_index = new_index; } else { /* * Moving the very last entry, so we need to update the tail pointer in the first one. - * find back the first entry. Inefficient so might need to be a bit cleverer - * if this proves to be a problem.. */ - u32 an_index = pae->prev_applied_entry_index; - applied_hash_ace_entry_t *head_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]); - while(!head_pae->is_first_entry) { - an_index = head_pae->prev_applied_entry_index; - head_pae = &((*applied_hash_aces)[an_index]); - } - ASSERT(head_pae->prev_applied_entry_index == old_index); - head_pae->prev_applied_entry_index = new_index; + u32 head_index = find_head_applied_ace_index(applied_hash_aces, old_index); + ASSERT(head_index != ~0); + applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), head_index); + + ASSERT(head_pae->tail_applied_entry_index == old_index); + head_pae->tail_applied_entry_index = new_index; } /* invalidate the old entry */ pae->prev_applied_entry_index = ~0; pae->next_applied_entry_index = ~0; + pae->tail_applied_entry_index = ~0; } static void @@ -370,39 +412,37 @@ deactivate_applied_ace_hash_entry(acl_main_t *am, applied_hash_ace_entry_t **applied_hash_aces, u32 old_index) { - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[old_index]); - - if (pae->next_applied_entry_index != ~0) { - applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]); - ASSERT(next_pae->prev_applied_entry_index == old_index); - next_pae->prev_applied_entry_index = pae->prev_applied_entry_index; - } + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), old_index); + DBG("UNAPPLY DEACTIVATE: sw_if_index %d is_input %d, applied index %d", sw_if_index, is_input, old_index); - if (!pae->is_first_entry) { - applied_hash_ace_entry_t *prev_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]); + if (pae->prev_applied_entry_index != ~0) { + DBG("UNAPPLY = index %d has prev_applied_entry_index %d", old_index, pae->prev_applied_entry_index); + applied_hash_ace_entry_t *prev_pae = vec_elt_at_index((*applied_hash_aces), pae->prev_applied_entry_index); ASSERT(prev_pae->next_applied_entry_index == old_index); prev_pae->next_applied_entry_index = pae->next_applied_entry_index; if (pae->next_applied_entry_index == ~0) { /* it was a last entry we removed, update the pointer on the first one */ - u32 an_index = pae->prev_applied_entry_index; - applied_hash_ace_entry_t *head_pae = &((*applied_hash_aces)[pae->prev_applied_entry_index]); - while(!head_pae->is_first_entry) { - an_index = head_pae->prev_applied_entry_index; - head_pae = &((*applied_hash_aces)[an_index]); - } - ASSERT(head_pae->prev_applied_entry_index == old_index); - head_pae->prev_applied_entry_index = pae->prev_applied_entry_index; + u32 head_index = find_head_applied_ace_index(applied_hash_aces, old_index); + DBG("UNAPPLY = index %d head index to update %d", old_index, head_index); + ASSERT(head_index != ~0); + applied_hash_ace_entry_t *head_pae = vec_elt_at_index((*applied_hash_aces), head_index); + + ASSERT(head_pae->tail_applied_entry_index == old_index); + head_pae->tail_applied_entry_index = pae->prev_applied_entry_index; + } else { + applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index); + next_pae->prev_applied_entry_index = pae->prev_applied_entry_index; } } else { /* It was the first entry. We need either to reset the hash entry or delete it */ - pae->is_first_entry = 0; if (pae->next_applied_entry_index != ~0) { - /* a custom case of relinking for the first node, with the tail not forward linked to it */ - applied_hash_ace_entry_t *next_pae = &((*applied_hash_aces)[pae->next_applied_entry_index]); - /* this is the tail the new head should be aware of */ - next_pae->prev_applied_entry_index = pae->prev_applied_entry_index; - next_pae->is_first_entry = 1; - + /* the next element becomes the new first one, so needs the tail pointer to be set */ + applied_hash_ace_entry_t *next_pae = vec_elt_at_index((*applied_hash_aces), pae->next_applied_entry_index); + ASSERT(pae->tail_applied_entry_index != ~0); + next_pae->tail_applied_entry_index = pae->tail_applied_entry_index; + DBG("Resetting the hash table entry from %d to %d, setting tail index to %d", old_index, pae->next_applied_entry_index, pae->tail_applied_entry_index); + /* unlink from the next element */ + next_pae->prev_applied_entry_index = ~0; add_del_hashtable_entry(am, sw_if_index, is_input, applied_hash_aces, pae->next_applied_entry_index, 1); } else { @@ -411,6 +451,10 @@ deactivate_applied_ace_hash_entry(acl_main_t *am, applied_hash_aces, old_index, 0); } } + /* invalidate the old entry */ + pae->prev_applied_entry_index = ~0; + pae->next_applied_entry_index = ~0; + pae->tail_applied_entry_index = ~0; } @@ -419,14 +463,14 @@ hash_acl_build_applied_lookup_bitmap(acl_main_t *am, u32 sw_if_index, u8 is_inpu { int i; uword *new_lookup_bitmap = 0; - u32 **applied_acls = is_input ? &am->input_acl_vec_by_sw_if_index[sw_if_index] : - &am->output_acl_vec_by_sw_if_index[sw_if_index]; - applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index : - &am->output_applied_hash_acl_info_by_sw_if_index; - applied_hash_acl_info_t *pal = &(*applied_hash_acls)[sw_if_index]; + u32 **applied_acls = is_input ? vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index) + : vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index); + applied_hash_acl_info_t **applied_hash_acls = is_input ? &am->input_applied_hash_acl_info_by_sw_if_index + : &am->output_applied_hash_acl_info_by_sw_if_index; + applied_hash_acl_info_t *pal = vec_elt_at_index((*applied_hash_acls), sw_if_index); for(i=0; i < vec_len(*applied_acls); i++) { - u32 a_acl_index = (*applied_acls)[i]; - hash_acl_info_t *ha = &am->hash_acl_infos[a_acl_index]; + u32 a_acl_index = *vec_elt_at_index((*applied_acls), i); + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, a_acl_index); new_lookup_bitmap = clib_bitmap_or(new_lookup_bitmap, ha->mask_type_index_bitmap); } @@ -441,12 +485,12 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) int i; DBG("HASH ACL unapply: sw_if_index %d is_input %d acl %d", sw_if_index, is_input, acl_index); - hash_acl_info_t *ha = &am->hash_acl_infos[acl_index]; - applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] : - &am->output_hash_entry_vec_by_sw_if_index[sw_if_index]; + + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index); + applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index); for(i=0; i < vec_len((*applied_hash_aces)); i++) { - if ((*applied_hash_aces)[i].acl_index == acl_index) { + if (vec_elt_at_index(*applied_hash_aces,i)->acl_index == acl_index) { DBG("Found applied ACL#%d at applied index %d", acl_index, i); break; } @@ -456,13 +500,14 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) /* we went all the way without finding any entries. Probably a list was empty. */ return; } + + void *oldheap = hash_acl_set_heap(am); int base_offset = i; int tail_offset = base_offset + vec_len(ha->rules); int tail_len = vec_len((*applied_hash_aces)) - tail_offset; DBG("base_offset: %d, tail_offset: %d, tail_len: %d", base_offset, tail_offset, tail_len); for(i=0; i < vec_len(ha->rules); i ++) { - DBG("UNAPPLY DEACTIVATE: sw_if_index %d is_input %d, applied index %d", sw_if_index, is_input, base_offset + i); deactivate_applied_ace_hash_entry(am, sw_if_index, is_input, applied_hash_aces, base_offset + i); } @@ -479,6 +524,7 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) /* After deletion we might not need some of the mask-types anymore... */ hash_acl_build_applied_lookup_bitmap(am, sw_if_index, is_input); + clib_mem_set_heap (oldheap); } /* @@ -493,8 +539,8 @@ hash_acl_unapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) void hash_acl_reapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) { - u32 **applied_acls = is_input ? &am->input_acl_vec_by_sw_if_index[sw_if_index] : - &am->output_acl_vec_by_sw_if_index[sw_if_index]; + u32 **applied_acls = is_input ? vec_elt_at_index(am->input_acl_vec_by_sw_if_index, sw_if_index) + : vec_elt_at_index(am->output_acl_vec_by_sw_if_index, sw_if_index); int i; int start_index = vec_search((*applied_acls), acl_index); /* @@ -505,10 +551,10 @@ hash_acl_reapply(acl_main_t *am, u32 sw_if_index, u8 is_input, int acl_index) /* unapply all the ACLs till the current one */ for(i = vec_len(*applied_acls) - 1; i >= start_index; i--) { - hash_acl_unapply(am, sw_if_index, is_input, (*applied_acls)[i]); + hash_acl_unapply(am, sw_if_index, is_input, *vec_elt_at_index(*applied_acls, i)); } for(i = start_index; i < vec_len(*applied_acls); i++) { - hash_acl_apply(am, sw_if_index, is_input, (*applied_acls)[i]); + hash_acl_apply(am, sw_if_index, is_input, *vec_elt_at_index(*applied_acls, i)); } } @@ -649,7 +695,7 @@ assign_mask_type_index(acl_main_t *am, fa_5tuple_t *mask) static void release_mask_type_index(acl_main_t *am, u32 mask_type_index) { - ace_mask_type_entry_t *mte = &am->ace_mask_type_pool[mask_type_index]; + ace_mask_type_entry_t *mte = pool_elt_at_index(am->ace_mask_type_pool, mask_type_index); mte->refcount--; if (mte->refcount == 0) { /* we are not using this entry anymore */ @@ -659,11 +705,12 @@ release_mask_type_index(acl_main_t *am, u32 mask_type_index) void hash_acl_add(acl_main_t *am, int acl_index) { + void *oldheap = hash_acl_set_heap(am); DBG("HASH ACL add : %d", acl_index); int i; acl_list_t *a = &am->acls[acl_index]; vec_validate(am->hash_acl_infos, acl_index); - hash_acl_info_t *ha = &am->hash_acl_infos[acl_index]; + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index); memset(ha, 0, sizeof(*ha)); /* walk the newly added ACL entries and ensure that for each of them there @@ -710,10 +757,12 @@ void hash_acl_add(acl_main_t *am, int acl_index) hash_acl_reapply(am, *sw_if_index, 0, acl_index); } } + clib_mem_set_heap (oldheap); } void hash_acl_delete(acl_main_t *am, int acl_index) { + void *oldheap = hash_acl_set_heap(am); DBG("HASH ACL delete : %d", acl_index); /* * If the ACL is applied somewhere, remove the references of it (call hash_acl_unapply) @@ -739,12 +788,13 @@ void hash_acl_delete(acl_main_t *am, int acl_index) /* walk the mask types for the ACL about-to-be-deleted, and decrease * the reference count, possibly freeing up some of them */ int i; - hash_acl_info_t *ha = &am->hash_acl_infos[acl_index]; + hash_acl_info_t *ha = vec_elt_at_index(am->hash_acl_infos, acl_index); for(i=0; i < vec_len(ha->rules); i++) { release_mask_type_index(am, ha->rules[i].mask_type_index); } clib_bitmap_free(ha->mask_type_index_bitmap); vec_free(ha->rules); + clib_mem_set_heap (oldheap); } u8 @@ -753,11 +803,10 @@ hash_multi_acl_match_5tuple (u32 sw_if_index, fa_5tuple_t * pkt_5tuple, int is_l u32 * rule_match_p, u32 * trace_bitmap) { acl_main_t *am = &acl_main; - applied_hash_ace_entry_t **applied_hash_aces = is_input ? &am->input_hash_entry_vec_by_sw_if_index[sw_if_index] : - &am->output_hash_entry_vec_by_sw_if_index[sw_if_index]; + applied_hash_ace_entry_t **applied_hash_aces = get_applied_hash_aces(am, is_input, sw_if_index); u32 match_index = multi_acl_match_get_applied_ace_index(am, pkt_5tuple); if (match_index < vec_len((*applied_hash_aces))) { - applied_hash_ace_entry_t *pae = &((*applied_hash_aces)[match_index]); + applied_hash_ace_entry_t *pae = vec_elt_at_index((*applied_hash_aces), match_index); *acl_match_p = pae->acl_index; *rule_match_p = pae->ace_index; return pae->action; diff --git a/src/plugins/acl/hash_lookup_types.h b/src/plugins/acl/hash_lookup_types.h index 1848c5ffc6a..fbc9777b83d 100644 --- a/src/plugins/acl/hash_lookup_types.h +++ b/src/plugins/acl/hash_lookup_types.h @@ -53,14 +53,14 @@ typedef struct { */ u32 next_applied_entry_index; /* - * previous entry in the ring list of the chained ones. + * previous entry in the list of the chained ones, + * if ~0 then this is entry in the hash. */ u32 prev_applied_entry_index; /* - * 1 if it is the very first entry in the list, - * referenced from the hash. + * chain tail, if this is the first entry */ - u8 is_first_entry; + u32 tail_applied_entry_index; /* * Action of this applied ACE */ -- 2.16.6