From e48457a99d3e9294f2eced29d3840023cc4de4b9 Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Thu, 22 Jun 2017 14:51:06 +0200 Subject: [PATCH] acl-plugin: clean up the code enabling/disabling acl-plugin processing on interface Multiple subsequent calls to vnet_feature_enable_disable() to enable the feature cause the feature to be inserted into the processing graph multiple times in a row. This might be argued to be a bug in that function, but enabling already enabled feature is suboptimal anyway, so avoid that. The existing tests already catch this issue whenever the ASSERT() part of this patch was added. Change-Id: Ia2c06f7dc87bbe05795c2c7b7d19ea06270ce150 Signed-off-by: Andrew Yourtchenko --- src/plugins/acl/acl.c | 20 ++++++++++++++++---- src/plugins/acl/fa_node.c | 2 ++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index d9f22d826d4..704a839c363 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -665,12 +665,15 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) /* the entry is already there */ return -1; } + /* if there was no ACL applied before, enable the ACL processing */ + if (vec_len(am->input_acl_vec_by_sw_if_index[sw_if_index]) == 0) { + acl_interface_in_enable_disable (am, sw_if_index, 1); + } vec_add (am->input_acl_vec_by_sw_if_index[sw_if_index], &acl_list_index, 1); vec_validate (am->input_sw_if_index_vec_by_acl, acl_list_index); vec_add (am->input_sw_if_index_vec_by_acl[acl_list_index], &sw_if_index, 1); - acl_interface_in_enable_disable (am, sw_if_index, 1); } else { @@ -683,12 +686,15 @@ acl_interface_add_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) /* the entry is already there */ return -1; } + /* if there was no ACL applied before, enable the ACL processing */ + if (vec_len(am->output_acl_vec_by_sw_if_index[sw_if_index]) == 0) { + acl_interface_out_enable_disable (am, sw_if_index, 1); + } vec_add (am->output_acl_vec_by_sw_if_index[sw_if_index], &acl_list_index, 1); vec_validate (am->output_sw_if_index_vec_by_acl, acl_list_index); vec_add (am->output_sw_if_index_vec_by_acl[acl_list_index], &sw_if_index, 1); - acl_interface_out_enable_disable (am, sw_if_index, 1); } return 0; } @@ -723,6 +729,7 @@ acl_interface_del_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) } } + /* If there is no more ACLs applied on an interface, disable ACL processing */ if (0 == vec_len (am->input_acl_vec_by_sw_if_index[sw_if_index])) { acl_interface_in_enable_disable (am, sw_if_index, 0); @@ -751,6 +758,7 @@ acl_interface_del_inout_acl (u32 sw_if_index, u8 is_input, u32 acl_list_index) } } + /* If there is no more ACLs applied on an interface, disable ACL processing */ if (0 == vec_len (am->output_acl_vec_by_sw_if_index[sw_if_index])) { acl_interface_out_enable_disable (am, sw_if_index, 0); @@ -766,8 +774,10 @@ acl_interface_reset_inout_acls (u32 sw_if_index, u8 is_input) int i; if (is_input) { - acl_interface_in_enable_disable (am, sw_if_index, 0); vec_validate (am->input_acl_vec_by_sw_if_index, sw_if_index); + if (vec_len(am->input_acl_vec_by_sw_if_index[sw_if_index]) > 0) { + acl_interface_in_enable_disable (am, sw_if_index, 0); + } for(i = vec_len(am->input_acl_vec_by_sw_if_index[sw_if_index])-1; i>=0; i--) { u32 acl_list_index = am->input_acl_vec_by_sw_if_index[sw_if_index][i]; @@ -784,8 +794,10 @@ acl_interface_reset_inout_acls (u32 sw_if_index, u8 is_input) } else { - acl_interface_out_enable_disable (am, sw_if_index, 0); vec_validate (am->output_acl_vec_by_sw_if_index, sw_if_index); + if (vec_len(am->output_acl_vec_by_sw_if_index[sw_if_index]) > 0) { + acl_interface_out_enable_disable (am, sw_if_index, 0); + } for(i = vec_len(am->output_acl_vec_by_sw_if_index[sw_if_index])-1; i>=0; i--) { u32 acl_list_index = am->output_acl_vec_by_sw_if_index[sw_if_index][i]; diff --git a/src/plugins/acl/fa_node.c b/src/plugins/acl/fa_node.c index c0ff1a5e01a..0bbc7423a39 100644 --- a/src/plugins/acl/fa_node.c +++ b/src/plugins/acl/fa_node.c @@ -1598,6 +1598,7 @@ acl_fa_enable_disable (u32 sw_if_index, int is_input, int enable_disable) acl_main_t *am = &acl_main; if (is_input) { + ASSERT(clib_bitmap_get(am->fa_in_acl_on_sw_if_index, sw_if_index) != enable_disable); 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", @@ -1608,6 +1609,7 @@ 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); 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", -- 2.16.6