diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 6906744cc2..5ad908f082 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2586,6 +2586,12 @@ bool AclTable::add(shared_ptr newRule) if (ruleIter != rules.end()) { // If ACL rule already exists, delete it first + if (ruleIter->second->hasCounter()) + { + // Deregister the flex counter before deleting the rule + // A new flex counter will be created when the new rule is added + m_pAclOrch->deregisterFlexCounter(*(ruleIter->second)); + } if (ruleIter->second->remove()) { rules.erase(ruleIter); diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index 4315da3798..236ccaa0fc 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -685,6 +685,17 @@ def _match_acl_range(sai_acl_range): return True return _match_acl_range + + def get_acl_counter_oid(self, acl_rule_id=None) -> str: + if not acl_rule_id: + acl_rule_id = self._get_acl_rule_id() + + entry = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) + counter_oid = entry.get("SAI_ACL_ENTRY_ATTR_ACTION_COUNTER") + return counter_oid + + def get_acl_rule_id(self) -> str: + return self._get_acl_rule_id() def _get_acl_rule_id(self) -> str: num_keys = len(self.asic_db.default_acl_entries) + 1 @@ -742,7 +753,12 @@ def _check_acl_entry_counters_map(self, acl_entry_oid: str): return rule_to_counter_map = self.counters_db.get_entry("ACL_COUNTER_RULE_MAP", "") counter_to_rule_map = {v: k for k, v in rule_to_counter_map.items()} - assert counter_oid in counter_to_rule_map + assert counter_oid in counter_to_rule_map + + def check_acl_counter_not_in_counters_map(self, acl_counter_oid: str): + rule_to_counter_map = self.counters_db.get_entry("ACL_COUNTER_RULE_MAP", "") + counter_to_rule_map = {v: k for k, v in rule_to_counter_map.items()} + assert acl_counter_oid not in counter_to_rule_map def verify_acl_table_status( self, diff --git a/tests/test_acl.py b/tests/test_acl.py index cf68d1516e..ed5789b2b0 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -1,5 +1,6 @@ import pytest from requests import request +import time L3_TABLE_TYPE = "L3" L3_TABLE_NAME = "L3_TEST" @@ -131,6 +132,38 @@ def test_InvalidAclRuleCreation(self, dvs_acl, l3_acl_table): dvs_acl.verify_acl_rule_status(L3_TABLE_NAME, "INVALID_RULE", None) dvs_acl.verify_no_acl_rules() + def test_AclRuleUpdate(self, dvs_acl, l3_acl_table): + """The test is to verify there is no duplicated flex counter when updating an ACL rule + """ + config_qualifiers = {"SRC_IP": "10.10.10.10/32"} + expected_sai_qualifiers = { + "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP": dvs_acl.get_simple_qualifier_comparator("10.10.10.10&mask:255.255.255.255") + } + + dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers) + dvs_acl.verify_acl_rule(expected_sai_qualifiers) + + acl_rule_id = dvs_acl.get_acl_rule_id() + counter_id = dvs_acl.get_acl_counter_oid() + + new_config_qualifiers = {"SRC_IP": "10.10.10.11/32"} + new_expected_sai_qualifiers = { + "SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP": dvs_acl.get_simple_qualifier_comparator("10.10.10.11&mask:255.255.255.255") + } + dvs_acl.update_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, new_config_qualifiers) + # Verify the rule has been updated + retry = 5 + while dvs_acl.get_acl_rule_id() == acl_rule_id and retry >= 0: + retry -= 1 + time.sleep(1) + assert retry > 0 + dvs_acl.verify_acl_rule(new_expected_sai_qualifiers) + # Verify the previous counter is removed + if counter_id: + dvs_acl.check_acl_counter_not_in_counters_map(counter_id) + dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME) + dvs_acl.verify_no_acl_rules() + def test_AclRuleL4SrcPort(self, dvs_acl, l3_acl_table): config_qualifiers = {"L4_SRC_PORT": "65000"} expected_sai_qualifiers = {