diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index b0dd7b78d1f1..57b6d14fe2b6 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1367,6 +1367,8 @@ bool AclTable::create() { gCrmOrch->incCrmAclUsedCounter( CrmResourceType::CRM_ACL_TABLE, (sai_acl_stage_t)attr.value.s32, SAI_ACL_BIND_POINT_TYPE_PORT); + gCrmOrch->incCrmAclUsedCounter( + CrmResourceType::CRM_ACL_TABLE, (sai_acl_stage_t)attr.value.s32, SAI_ACL_BIND_POINT_TYPE_LAG); } return status == SAI_STATUS_SUCCESS; @@ -1551,6 +1553,7 @@ bool AclTable::create() if (status == SAI_STATUS_SUCCESS) { gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, acl_stage, SAI_ACL_BIND_POINT_TYPE_PORT); + gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, acl_stage, SAI_ACL_BIND_POINT_TYPE_LAG); } return status == SAI_STATUS_SUCCESS; @@ -2837,9 +2840,16 @@ bool AclOrch::removeAclTable(string table_id) if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS) { auto stage = m_AclTables[table_oid].stage; + auto type = m_AclTables[table_oid].type; + sai_acl_stage_t sai_stage = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_PORT, table_oid); + if (type != ACL_TABLE_PFCWD) + { + gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_TABLE, sai_stage, SAI_ACL_BIND_POINT_TYPE_LAG, table_oid); + } + SWSS_LOG_NOTICE("Successfully deleted ACL table %s", table_id.c_str()); m_AclTables.erase(table_oid); diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index c166d1a8ce19..7b279a6571fc 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -593,67 +593,80 @@ namespace aclorch_test return true; } - // consistency validation with CRM + // Validate that ACL table resource count is consistent with CRM bool validateResourceCountWithCrm(const AclOrch *aclOrch, CrmOrch *crmOrch) { - // Verify ACL Tables auto const &resourceMap = Portal::CrmOrchInternal::getResourceMap(crmOrch); - uint32_t crm_acl_table_cnt = 0; - for (auto const &kv : resourceMap.at(CrmResourceType::CRM_ACL_TABLE).countersMap) + + // Verify the ACL tables + uint32_t crmAclTableBindingCount = 0; + for (auto const &kv: resourceMap.at(CrmResourceType::CRM_ACL_TABLE).countersMap) { - crm_acl_table_cnt += kv.second.usedCounter; + crmAclTableBindingCount += kv.second.usedCounter; + } + + uint32_t aclorchAclTableBindingCount = 0; + for (auto const &kv: Portal::AclOrchInternal::getAclTables(aclOrch)) + { + if (kv.second.type == ACL_TABLE_PFCWD) + { + aclorchAclTableBindingCount += 1; // port binding only + } + else + { + aclorchAclTableBindingCount += 2; // port + LAG binding + } } - if (crm_acl_table_cnt != Portal::AclOrchInternal::getAclTables(aclOrch).size()) + if (crmAclTableBindingCount != aclorchAclTableBindingCount) { - ADD_FAILURE() << "ACL table size is not consistent between CrmOrch (" << crm_acl_table_cnt - << ") and AclOrch " << Portal::AclOrchInternal::getAclTables(aclOrch).size(); + ADD_FAILURE() << "ACL table binding count is not consistent between CrmOrch (" + << crmAclTableBindingCount << ") and AclOrch (" + << aclorchAclTableBindingCount << ")"; return false; } + // Verify ACL rules and counters - // Verify ACL Rules - // - // for each CRM_ACL_ENTRY and CRM_ACL_COUNTER's entry => the ACL TABLE should exist - // - for (auto acl_entry_or_counter : { CrmResourceType::CRM_ACL_ENTRY, CrmResourceType::CRM_ACL_COUNTER }) + // For each CRM_ACL_ENTRY and CRM_ACL_COUNTER entry, there should be a corresponding ACL table + for (auto aclResourceType: {CrmResourceType::CRM_ACL_ENTRY, CrmResourceType::CRM_ACL_COUNTER}) { - auto const &resourceMap = Portal::CrmOrchInternal::getResourceMap(crmOrch); - for (auto const &kv : resourceMap.at(acl_entry_or_counter).countersMap) + for (auto const &kv: resourceMap.at(aclResourceType).countersMap) { - auto acl_oid = kv.second.id; + auto aclOid = kv.second.id; const auto &aclTables = Portal::AclOrchInternal::getAclTables(aclOrch); - if (aclTables.find(acl_oid) == aclTables.end()) + if (aclTables.find(aclOid) == aclTables.end()) { - ADD_FAILURE() << "Can't find ACL '" << sai_serialize_object_id(acl_oid) << "' in AclOrch"; + ADD_FAILURE() << "Can't find ACL '" << sai_serialize_object_id(aclOid) + << "' in AclOrch"; return false; } - if (kv.second.usedCounter != aclTables.at(acl_oid).rules.size()) + if (kv.second.usedCounter != aclTables.at(aclOid).rules.size()) { ADD_FAILURE() << "CRM usedCounter (" << kv.second.usedCounter - << ") is not equal rule in ACL (" << aclTables.at(acl_oid).rules.size() << ")"; + << ") is not equal rule in ACL (" + << aclTables.at(aclOid).rules.size() << ")"; return false; } } } - // - // for each ACL TABLE with rule count larger than one => it shoule exist a corresponding entry in CRM_ACL_ENTRY and CRM_ACL_COUNTER - // - for (const auto &kv : Portal::AclOrchInternal::getAclTables(aclOrch)) + // For each ACL table with at least one rule, there should be corresponding entries for CRM_ACL_ENTRY and CRM_ACL_COUNTER + for (const auto &kv: Portal::AclOrchInternal::getAclTables(aclOrch)) { - if (0 < kv.second.rules.size()) + if (kv.second.rules.size() > 0) { auto key = Portal::CrmOrchInternal::getCrmAclTableKey(crmOrch, kv.first); - for (auto acl_entry_or_counter : { CrmResourceType::CRM_ACL_ENTRY, CrmResourceType::CRM_ACL_COUNTER }) + for (auto aclResourceType: {CrmResourceType::CRM_ACL_ENTRY, CrmResourceType::CRM_ACL_COUNTER}) { - const auto &cntMap = Portal::CrmOrchInternal::getResourceMap(crmOrch).at(acl_entry_or_counter).countersMap; + const auto &cntMap = resourceMap.at(aclResourceType).countersMap; if (cntMap.find(key) == cntMap.end()) { ADD_FAILURE() << "Can't find ACL (" << sai_serialize_object_id(kv.first) - << ") in " << (acl_entry_or_counter == CrmResourceType::CRM_ACL_ENTRY ? "CrmResourceType::CRM_ACL_ENTRY" : "CrmResourceType::CRM_ACL_COUNTER"); + << ") in " + << (aclResourceType == CrmResourceType::CRM_ACL_ENTRY ? "CRM_ACL_ENTRY" : "CRM_ACL_COUNTER"); return false; } } diff --git a/tests/test_acl.py b/tests/test_acl.py index 86df9c518fbb..73106fa8921b 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -450,6 +450,57 @@ def test_AclRuleRedirect(self, dvs, dvs_acl, l3_acl_table, setup_teardown_neighb dvs_acl.verify_no_acl_rules() +class TestAclCrmUtilization: + @pytest.fixture(scope="class", autouse=True) + def configure_crm_polling_interval_for_test(self, dvs): + dvs.runcmd("crm config polling interval 1") + + yield + + dvs.runcmd("crm config polling interval 300") + + def test_ValidateAclTableBindingCrmUtilization(self, dvs, dvs_acl): + counter_db = dvs.get_counters_db() + + crm_port_stats = counter_db.get_entry("CRM", "ACL_STATS:INGRESS:PORT") + initial_acl_table_port_bindings_used = int(crm_port_stats.get("crm_stats_acl_table_used", 0)) + + crm_lag_stats = counter_db.get_entry("CRM", "ACL_STATS:INGRESS:LAG") + initial_acl_table_lag_bindings_used = int(crm_lag_stats.get("crm_stats_acl_table_used", 0)) + + dvs_acl.create_acl_table(L3_TABLE_NAME, L3_TABLE_TYPE, L3_BIND_PORTS) + dvs_acl.verify_acl_table_count(1) + + counter_db.wait_for_field_match( + "CRM", + "ACL_STATS:INGRESS:PORT", + {"crm_stats_acl_table_used": str(initial_acl_table_port_bindings_used + 1)} + ) + + counter_db.wait_for_field_match( + "CRM", + "ACL_STATS:INGRESS:LAG", + {"crm_stats_acl_table_used": str(initial_acl_table_lag_bindings_used + 1)} + ) + + dvs_acl.remove_acl_table(L3_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + + counter_db.wait_for_field_match( + "CRM", + "ACL_STATS:INGRESS:PORT", + {"crm_stats_acl_table_used": str(initial_acl_table_port_bindings_used)} + ) + + counter_db.wait_for_field_match( + "CRM", + "ACL_STATS:INGRESS:LAG", + {"crm_stats_acl_table_used": str(initial_acl_table_lag_bindings_used)} + ) + + +# TODO: Need to improve the clean-up/post-checks for these tests as currently we can't run anything +# afterwards. class TestAclRuleValidation: """Test class for cases that check if orchagent corectly validates ACL rules input."""