ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

acl: fix stats-segment counters validation on acl update

Browse Source 
The stats-segment validation/clear logic for acl counters was wrong,
fix it. Also add the code to the unittests to cover that case,
add a vat command to enable/disable counters, clean up
the unnecessary endian conversion and remove the stray clib_warning()

Change-Id: I421297a92e4aeb885c468c72a97cec25981df615
Type: fix
Ticket: VPP-1744
Fixes: f995c7122ba0d024b17bc3232e8edd18d5e25088
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
This commit is contained in:
Andrew Yourtchenko
2019-08-15 12:26:17 +00:00
parent 2f51729bb3
commit 4c945dacb9
3 changed files with 82 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/plugins/acl/acl.c
View File

@ -391,14 +391,19 @@ validate_and_reset_acl_counters (acl_main_t * am, u32 acl_index)
/* filled in once only */
am->combined_acl_counters[i].stat_segment_name = (void *)
format (0, "/acl/%d/matches%c", i, 0);
clib_warning ("add stats segment: %s",
am->combined_acl_counters[i].stat_segment_name);
i32 rule_count = vec_len (am->acls[acl_index].rules);
i32 rule_count = vec_len (am->acls[i].rules);
/* Validate one extra so we always have at least one counter for an ACL */
vlib_validate_combined_counter (&am->combined_acl_counters[i],
rule_count);
vlib_zero_combined_counter (&am->combined_acl_counters[i], rule_count);
vlib_clear_combined_counters (&am->combined_acl_counters[i]);
}
/* (re)validate for the actual ACL that is getting added/updated */
i32 rule_count = vec_len (am->acls[acl_index].rules);
/* Validate one extra so we always have at least one counter for an ACL */
vlib_validate_combined_counter (&am->combined_acl_counters[acl_index],
rule_count);
vlib_clear_combined_counters (&am->combined_acl_counters[acl_index]);
acl_plugin_counter_unlock (am);
}
@ -1945,7 +1950,7 @@ static void
vl_api_acl_stats_intf_counters_enable_reply_t *rmp;
int rv;
rv = acl_stats_intf_counters_enable_disable (am, ntohl (mp->enable));
rv = acl_stats_intf_counters_enable_disable (am, mp->enable);
REPLY_MACRO (VL_API_ACL_DEL_REPLY);
}

39
src/plugins/acl/acl_test.c
View File

@ -68,7 +68,8 @@ _(acl_interface_add_del_reply) \
_(macip_acl_interface_add_del_reply) \
_(acl_interface_set_acl_list_reply) \
_(acl_interface_set_etype_whitelist_reply) \
_(macip_acl_del_reply)
_(macip_acl_del_reply) \
_(acl_stats_intf_counters_enable_reply)
#define foreach_reply_retval_aclindex_handler \
_(acl_add_replace_reply) \
@ -310,7 +311,8 @@ _(MACIP_ACL_INTERFACE_ADD_DEL_REPLY, macip_acl_interface_add_del_reply) \
_(MACIP_ACL_INTERFACE_GET_REPLY, macip_acl_interface_get_reply) \
_(ACL_PLUGIN_CONTROL_PING_REPLY, acl_plugin_control_ping_reply) \
_(ACL_PLUGIN_GET_VERSION_REPLY, acl_plugin_get_version_reply) \
_(ACL_PLUGIN_GET_CONN_TABLE_MAX_ENTRIES_REPLY,acl_plugin_get_conn_table_max_entries_reply)
_(ACL_PLUGIN_GET_CONN_TABLE_MAX_ENTRIES_REPLY,acl_plugin_get_conn_table_max_entries_reply) \
_(ACL_STATS_INTF_COUNTERS_ENABLE_REPLY, acl_stats_intf_counters_enable_reply)
static int api_acl_plugin_get_version (vat_main_t * vam)
{
@ -574,6 +576,36 @@ static int api_acl_plugin_get_conn_table_max_entries (vat_main_t * vam)
return ret;
}
static int api_acl_stats_intf_counters_enable (vat_main_t * vam)
{
acl_test_main_t * sm = &acl_test_main;
unformat_input_t * i = vam->input;
vl_api_acl_stats_intf_counters_enable_t * mp;
u32 msg_size = sizeof(*mp);
int ret;
vam->result_ready = 0;
mp = vl_msg_api_alloc_as_if_client(msg_size);
memset (mp, 0, msg_size);
mp->_vl_msg_id = ntohs (VL_API_ACL_STATS_INTF_COUNTERS_ENABLE + sm->msg_id_base);
mp->client_index = vam->my_client_index;
mp->enable = 1;
while (unformat_check_input (i) != UNFORMAT_END_OF_INPUT) {
if (unformat (i, "disable"))
mp->enable = 0;
else
break;
}
/* send it... */
S(mp);
/* Wait for a reply... */
W (ret);
return ret;
}
/*
* Read the series of ACL entries from file in the following format:
@ -1485,7 +1517,8 @@ _(macip_acl_del, "<acl-idx>")\
_(macip_acl_dump, "[<acl-idx>]") \
_(macip_acl_interface_add_del, "<intfc> | sw_if_index <if-idx> [add|del] acl <acl-idx>") \
_(macip_acl_interface_get, "") \
_(acl_plugin_get_conn_table_max_entries, "")
_(acl_plugin_get_conn_table_max_entries, "") \
_(acl_stats_intf_counters_enable, "[disable]")
static

42
test/test_acl_plugin_l2l3.py
View File

@ -103,9 +103,11 @@ class TestACLpluginL2L3(VppTestCase):
half = cls.remote_hosts_count // 2
cls.pg0.remote_hosts = cls.loop0.remote_hosts[:half]
cls.pg1.remote_hosts = cls.loop0.remote_hosts[half:]
reply = cls.vapi.papi.acl_stats_intf_counters_enable(enable=1)
@classmethod
def tearDownClass(cls):
reply = cls.vapi.papi.acl_stats_intf_counters_enable(enable=0)
super(TestACLpluginL2L3, cls).tearDownClass()
def tearDown(self):
@ -471,6 +473,7 @@ class TestACLpluginL2L3(VppTestCase):
acls=[acl_idx['L2']])
self.applied_acl_shuffle(self.pg0.sw_if_index)
self.applied_acl_shuffle(self.pg2.sw_if_index)
return {'L2': acl_idx['L2'], 'L3': acl_idx['L3']}
def apply_acl_ip46_both_directions_reflect(self,
primary_is_bridged_to_routed,
@ -531,14 +534,22 @@ class TestACLpluginL2L3(VppTestCase):
def apply_acl_ip46_routed_to_bridged(self, test_l2_deny, is_ip6,
is_reflect, add_eh):
self.apply_acl_ip46_x_to_y(False, test_l2_deny, is_ip6,
return self.apply_acl_ip46_x_to_y(False, test_l2_deny, is_ip6,
is_reflect, add_eh)
def apply_acl_ip46_bridged_to_routed(self, test_l2_deny, is_ip6,
is_reflect, add_eh):
self.apply_acl_ip46_x_to_y(True, test_l2_deny, is_ip6,
return self.apply_acl_ip46_x_to_y(True, test_l2_deny, is_ip6,
is_reflect, add_eh)
def verify_acl_packet_count(self, acl_idx, packet_count):
matches = self.statistics.get_counter('/acl/%d/matches' % acl_idx)
self.logger.info("stat seg for ACL %d: %s" % (acl_idx, repr(matches)))
total_count = 0
for p in matches[0]:
total_count = total_count + p['packets']
self.assertEqual(total_count, packet_count)
def run_traffic_ip46_x_to_y(self, bridged_to_routed,
test_l2_deny, is_ip6,
is_reflect, is_established, add_eh,
@ -560,34 +571,41 @@ class TestACLpluginL2L3(VppTestCase):
packet_count = self.get_packet_count_for_if_idx(self.loop0.sw_if_index)
rcvd1 = rx_if.get_capture(packet_count)
self.verify_capture(self.loop0, self.pg2, rcvd1, bridged_to_routed)
return len(stream)
def run_traffic_ip46_routed_to_bridged(self, test_l2_deny, is_ip6,
is_reflect, is_established, add_eh,
stateful_icmp=False):
self.run_traffic_ip46_x_to_y(False, test_l2_deny, is_ip6,
return self.run_traffic_ip46_x_to_y(False, test_l2_deny, is_ip6,
is_reflect, is_established, add_eh,
stateful_icmp)
def run_traffic_ip46_bridged_to_routed(self, test_l2_deny, is_ip6,
is_reflect, is_established, add_eh,
stateful_icmp=False):
self.run_traffic_ip46_x_to_y(True, test_l2_deny, is_ip6,
return self.run_traffic_ip46_x_to_y(True, test_l2_deny, is_ip6,
is_reflect, is_established, add_eh,
stateful_icmp)
def run_test_ip46_routed_to_bridged(self, test_l2_deny,
is_ip6, is_reflect, add_eh):
self.apply_acl_ip46_routed_to_bridged(test_l2_deny,
is_ip6, is_reflect, add_eh)
self.run_traffic_ip46_routed_to_bridged(test_l2_deny, is_ip6,
is_reflect, False, add_eh)
acls = self.apply_acl_ip46_routed_to_bridged(test_l2_deny,
is_ip6, is_reflect,
add_eh)
pkts = self.run_traffic_ip46_routed_to_bridged(test_l2_deny, is_ip6,
is_reflect, False,
add_eh)
self.verify_acl_packet_count(acls['L3'], pkts)
def run_test_ip46_bridged_to_routed(self, test_l2_deny,
is_ip6, is_reflect, add_eh):
self.apply_acl_ip46_bridged_to_routed(test_l2_deny,
is_ip6, is_reflect, add_eh)
self.run_traffic_ip46_bridged_to_routed(test_l2_deny, is_ip6,
is_reflect, False, add_eh)
acls = self.apply_acl_ip46_bridged_to_routed(test_l2_deny,
is_ip6, is_reflect,
add_eh)
pkts = self.run_traffic_ip46_bridged_to_routed(test_l2_deny, is_ip6,
is_reflect, False,
add_eh)
self.verify_acl_packet_count(acls['L2'], pkts)
def run_test_ip46_routed_to_bridged_and_back(self, test_l2_action,
is_ip6, add_eh,