From 5f8ebfa10a27d06062983d3f3bce2b86c6929600 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Thu, 11 Nov 2021 03:06:28 +0200 Subject: [PATCH] [AclOrch] move ACL counters to flex counter infrastructure (#1943) * [FlexCounterManager] Add ACL_COUNTER FC type What I did Added ACL Flex Counter group. Re-implemented ACL counters implemention to work with Flex Counter Infrastructure. Why I did it To improve orchagent performance when large amount of ACL rules are configured. How I verified it Together with depends PRs. Run ACL/Everflow test suite. Signed-off-by: Stepan Blyschak --- orchagent/aclorch.cpp | 286 ++++++++++-------- orchagent/aclorch.h | 83 +++-- .../flex_counter/flex_counter_manager.cpp | 1 + orchagent/flex_counter/flex_counter_manager.h | 1 + orchagent/flexcounterorch.cpp | 2 + tests/dvslib/dvs_acl.py | 13 + tests/test_flex_counters.py | 28 +- 7 files changed, 242 insertions(+), 172 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 4f42ca415d7f..d9dc26e99e94 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -15,15 +15,12 @@ using namespace std; using namespace swss; -mutex AclOrch::m_countersMutex; map AclRange::m_ranges; -condition_variable AclOrch::m_sleepGuard; -bool AclOrch::m_bCollectCounters = true; sai_uint32_t AclRule::m_minPriority = 0; sai_uint32_t AclRule::m_maxPriority = 0; -swss::DBConnector AclOrch::m_db("COUNTERS_DB", 0); -swss::Table AclOrch::m_countersTable(&m_db, "COUNTERS"); +swss::DBConnector AclOrch::m_countersDb("COUNTERS_DB", 0); +swss::Table AclOrch::m_countersTable(&m_countersDb, "COUNTERS"); extern sai_acl_api_t* sai_acl_api; extern sai_port_api_t* sai_port_api; @@ -35,6 +32,10 @@ extern CrmOrch *gCrmOrch; #define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID #define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID +#define COUNTERS_ACL_COUNTER_RULE_MAP "ACL_COUNTER_RULE_MAP" +#define ACL_COUNTER_DEFAULT_POLLING_INTERVAL_MS 10000 // ms +#define ACL_COUNTER_DEFAULT_ENABLED_STATE false + const int TCP_PROTOCOL_NUM = 6; // TCP protocol number acl_rule_attr_lookup_t aclMatchLookup = @@ -157,6 +158,12 @@ static acl_ip_type_lookup_t aclIpTypeLookup = { IP_TYPE_ARP_REPLY, SAI_ACL_IP_TYPE_ARP_REPLY } }; +static map aclCounterLookup = +{ + {SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT, SAI_ACL_COUNTER_ATTR_BYTES}, + {SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT, SAI_ACL_COUNTER_ATTR_PACKETS}, +}; + AclRule::AclRule(AclOrch *pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter) : m_pAclOrch(pAclOrch), m_id(rule), @@ -470,6 +477,22 @@ bool AclRule::processIpType(string type, sai_uint32_t &ip_type) } bool AclRule::create() +{ + if (m_createCounter && !createCounter()) + { + return false; + } + + if (!createRule()) + { + removeCounter(); + return false; + } + + return true; +} + +bool AclRule::createRule() { SWSS_LOG_ENTER(); @@ -481,11 +504,6 @@ bool AclRule::create() sai_attribute_t attr; sai_status_t status; - if (m_createCounter && !createCounter()) - { - return false; - } - // store table oid this rule belongs to attr.id = SAI_ACL_ENTRY_ATTR_TABLE_ID; attr.value.oid = table_oid; @@ -610,14 +628,19 @@ bool AclRule::isActionSupported(sai_acl_entry_attr_t action) const return m_pAclOrch->isAclActionSupported(pTable->stage, action_type); } -bool AclRule::remove() +bool AclRule::removeRule() { SWSS_LOG_ENTER(); - sai_status_t res; - if (sai_acl_api->remove_acl_entry(m_ruleOid) != SAI_STATUS_SUCCESS) + if (m_ruleOid == SAI_NULL_OBJECT_ID) + { + return true; + } + + auto status = sai_acl_api->remove_acl_entry(m_ruleOid); + if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to delete ACL rule"); + SWSS_LOG_ERROR("Failed to delete ACL rule, status %s", sai_serialize_status(status).c_str()); return false; } @@ -627,7 +650,19 @@ bool AclRule::remove() decreaseNextHopRefCount(); - res = removeRanges(); + return true; +} + +bool AclRule::remove() +{ + SWSS_LOG_ENTER(); + + if (!removeRule()) + { + return false; + } + + auto res = removeRanges(); res &= removeCounter(); return res; @@ -650,28 +685,6 @@ void AclRule::updateInPorts() } } -AclRuleCounters AclRule::getCounters() -{ - SWSS_LOG_ENTER(); - - if (m_counterOid == SAI_NULL_OBJECT_ID) - { - return AclRuleCounters(); - } - - sai_attribute_t counter_attr[2]; - counter_attr[0].id = SAI_ACL_COUNTER_ATTR_PACKETS; - counter_attr[1].id = SAI_ACL_COUNTER_ATTR_BYTES; - - if (sai_acl_api->get_acl_counter_attribute(m_counterOid, 2, counter_attr) != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to get counters for %s rule", m_id.c_str()); - return AclRuleCounters(); - } - - return AclRuleCounters(counter_attr[0].value.u64, counter_attr[1].value.u64); -} - shared_ptr AclRule::makeShared(acl_table_type_t type, AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data) { string action; @@ -851,13 +864,12 @@ bool AclRule::createCounter() attr.value.oid = m_tableOid; counter_attrs.push_back(attr); - attr.id = SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT; - attr.value.booldata = true; - counter_attrs.push_back(attr); - - attr.id = SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT; - attr.value.booldata = true; - counter_attrs.push_back(attr); + for (const auto& counterAttrPair: aclCounterLookup) + { + tie(attr.id, std::ignore) = counterAttrPair; + attr.value.booldata = true; + counter_attrs.push_back(attr); + } if (sai_acl_api->create_acl_counter(&m_counterOid, gSwitchId, (uint32_t)counter_attrs.size(), counter_attrs.data()) != SAI_STATUS_SUCCESS) { @@ -903,9 +915,6 @@ bool AclRule::removeCounter() gCrmOrch->decCrmAclTableUsedCounter(CrmResourceType::CRM_ACL_COUNTER, m_tableOid); - SWSS_LOG_INFO("Removing record about the counter %" PRIx64 " from the DB", m_counterOid); - AclOrch::getCountersTable().del(getTableId() + ":" + getId()); - m_counterOid = SAI_NULL_OBJECT_ID; SWSS_LOG_INFO("Removed counter for the rule %s in table %s", m_id.c_str(), m_tableId.c_str()); @@ -1286,7 +1295,19 @@ bool AclRuleMirror::validate() return true; } -bool AclRuleMirror::create() +bool AclRuleMirror::createRule() +{ + SWSS_LOG_ENTER(); + + return activate(); +} + +bool AclRuleMirror::removeRule() +{ + return deactivate(); +} + +bool AclRuleMirror::activate() { SWSS_LOG_ENTER(); @@ -1321,7 +1342,7 @@ bool AclRuleMirror::create() it.second.aclaction.parameter.objlist.count = 1; } - if (!AclRule::create()) + if (!AclRule::createRule()) { return false; } @@ -1334,23 +1355,26 @@ bool AclRuleMirror::create() m_state = true; return true; + } -bool AclRuleMirror::remove() +bool AclRuleMirror::deactivate() { + SWSS_LOG_ENTER(); + if (!m_state) { return true; } - if (!AclRule::remove()) + if (!AclRule::removeRule()) { return false; } if (!m_pMirrorOrch->decreaseRefCount(m_sessionName)) { - throw runtime_error("Failed to decrease mirror session reference count"); + SWSS_LOG_THROW("Failed to decrease mirror session reference count for session %s", m_sessionName.c_str()); } m_state = false; @@ -1375,15 +1399,12 @@ void AclRuleMirror::update(SubjectType type, void *cntx) if (update->active) { SWSS_LOG_INFO("Activating mirroring ACL %s for session %s", m_id.c_str(), m_sessionName.c_str()); - create(); + activate(); } else { - // Store counters before deactivating ACL rule - counters += AclRule::getCounters(); - SWSS_LOG_INFO("Deactivating mirroring ACL %s for session %s", m_id.c_str(), m_sessionName.c_str()); - remove(); + deactivate(); } } @@ -2014,18 +2035,6 @@ bool AclTable::clear() return true; } -AclRuleCounters AclRuleMirror::getCounters() -{ - AclRuleCounters cnt(counters); - - if (m_state) - { - cnt += AclRule::getCounters(); - } - - return cnt; -} - AclRuleDTelFlowWatchListEntry::AclRuleDTelFlowWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table, acl_table_type_t type) : AclRule(aclOrch, rule, table, type), m_pDTelOrch(dtel) @@ -2133,7 +2142,19 @@ bool AclRuleDTelFlowWatchListEntry::validate() return true; } -bool AclRuleDTelFlowWatchListEntry::create() +bool AclRuleDTelFlowWatchListEntry::createRule() +{ + SWSS_LOG_ENTER(); + + return activate(); +} + +bool AclRuleDTelFlowWatchListEntry::removeRule() +{ + return deactivate(); +} + +bool AclRuleDTelFlowWatchListEntry::activate() { SWSS_LOG_ENTER(); @@ -2147,16 +2168,13 @@ bool AclRuleDTelFlowWatchListEntry::create() return true; } - if (!AclRule::create()) - { - return false; - } - - return true; + return AclRule::createRule(); } -bool AclRuleDTelFlowWatchListEntry::remove() +bool AclRuleDTelFlowWatchListEntry::deactivate() { + SWSS_LOG_ENTER(); + if (!m_pDTelOrch) { return false; @@ -2167,7 +2185,7 @@ bool AclRuleDTelFlowWatchListEntry::remove() return true; } - if (!AclRule::remove()) + if (!AclRule::removeRule()) { return false; } @@ -2231,12 +2249,12 @@ void AclRuleDTelFlowWatchListEntry::update(SubjectType type, void *cntx) INT_session_valid = true; - create(); + activate(); } else { SWSS_LOG_INFO("Deactivating INT watchlist %s for session %s", m_id.c_str(), m_intSessionId.c_str()); - remove(); + deactivate(); INT_session_valid = false; } } @@ -2523,14 +2541,6 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr // Attach observers m_mirrorOrch->attach(this); gPortsOrch->attach(this); - - // Should be initialized last to guaranty that object is - // initialized before thread start. - auto interv = timespec { .tv_sec = COUNTERS_READ_INTERVAL, .tv_nsec = 0 }; - auto timer = new SelectableTimer(interv); - auto executor = new ExecutableTimer(timer, this, "ACL_POLL_TIMER"); - Orch::addExecutor(executor); - timer->start(); } void AclOrch::queryAclActionCapability() @@ -2758,7 +2768,13 @@ AclOrch::AclOrch(vector& connectors, SwitchOrch *switchOrch, m_mirrorOrch(mirrorOrch), m_neighOrch(neighOrch), m_routeOrch(routeOrch), - m_dTelOrch(dtelOrch) + m_dTelOrch(dtelOrch), + m_flex_counter_manager( + ACL_COUNTER_FLEX_COUNTER_GROUP, + StatsMode::READ, + ACL_COUNTER_DEFAULT_POLLING_INTERVAL_MS, + ACL_COUNTER_DEFAULT_ENABLED_STATE + ) { SWSS_LOG_ENTER(); @@ -2780,9 +2796,6 @@ AclOrch::~AclOrch() m_dTelOrch->detach(this); } - m_bCollectCounters = false; - m_sleepGuard.notify_all(); - deleteDTelWatchListTables(); } @@ -2797,8 +2810,6 @@ void AclOrch::update(SubjectType type, void *cntx) return; } - unique_lock lock(m_countersMutex); - // ACL table deals with port change // ACL rule deals with mirror session change and int session change for (auto& table : m_AclTables) @@ -2830,12 +2841,10 @@ void AclOrch::doTask(Consumer &consumer) if (table_name == CFG_ACL_TABLE_TABLE_NAME || table_name == APP_ACL_TABLE_TABLE_NAME) { - unique_lock lock(m_countersMutex); doAclTableTask(consumer); } else if (table_name == CFG_ACL_RULE_TABLE_NAME || table_name == APP_ACL_RULE_TABLE_NAME) { - unique_lock lock(m_countersMutex); doAclRuleTask(consumer); } else @@ -3164,7 +3173,17 @@ bool AclOrch::addAclRule(shared_ptr newRule, string table_id) return false; } - return m_AclTables[table_oid].add(newRule); + if (!m_AclTables[table_oid].add(newRule)) + { + return false; + } + + if (newRule->hasCounter()) + { + registerFlexCounter(*newRule); + } + + return true; } bool AclOrch::removeAclRule(string table_id, string rule_id) @@ -3176,6 +3195,17 @@ bool AclOrch::removeAclRule(string table_id, string rule_id) return true; } + auto rule = getAclRule(table_id, rule_id); + if (!rule) + { + return false; + } + + if (rule->hasCounter()) + { + deregisterFlexCounter(*rule); + } + return m_AclTables[table_oid].remove(rule_id); } @@ -3307,9 +3337,11 @@ bool AclOrch::updateAclRule(string table_id, string rule_id, bool enableCounter) return false; } + registerFlexCounter(*rule); return true; } + deregisterFlexCounter(*rule); if (!rule->disableCounter()) { SWSS_LOG_ERROR( @@ -3823,30 +3855,6 @@ sai_status_t AclOrch::deleteUnbindAclTable(sai_object_id_t table_oid) return sai_acl_api->remove_acl_table(table_oid); } -void AclOrch::doTask(SelectableTimer &timer) -{ - SWSS_LOG_ENTER(); - - for (auto& table_it : m_AclTables) - { - vector values; - - for (auto rule_it : table_it.second.rules) - { - AclRuleCounters cnt = rule_it.second->getCounters(); - - swss::FieldValueTuple fvtp("Packets", to_string(cnt.packets)); - values.push_back(fvtp); - swss::FieldValueTuple fvtb("Bytes", to_string(cnt.bytes)); - values.push_back(fvtb); - - AclOrch::getCountersTable().set(rule_it.second->getTableId() + ":" - + rule_it.second->getId(), values, ""); - } - values.clear(); - } -} - sai_status_t AclOrch::bindAclTable(AclTable &aclTable, bool bind) { SWSS_LOG_ENTER(); @@ -4082,6 +4090,42 @@ sai_status_t AclOrch::deleteDTelWatchListTables() return SAI_STATUS_SUCCESS; } +void AclOrch::registerFlexCounter(const AclRule& rule) +{ + SWSS_LOG_ENTER(); + + auto ruleIdentifier = generateAclRuleIdentifierInCountersDb(rule); + auto counterOidStr = sai_serialize_object_id(rule.getCounterOid()); + + unordered_set serializedCounterStatAttrs; + for (const auto& counterAttrPair: aclCounterLookup) + { + sai_acl_counter_attr_t id {}; + tie(std::ignore, id) = counterAttrPair; + auto meta = sai_metadata_get_attr_metadata(SAI_OBJECT_TYPE_ACL_COUNTER, id); + if (!meta) + { + SWSS_LOG_THROW("SAI Bug: Failed to get metadata of attribute %d for SAI_OBJECT_TYPE_ACL_COUNTER", id); + } + serializedCounterStatAttrs.insert(sai_serialize_attr_id(*meta)); + } + + m_flex_counter_manager.setCounterIdList(rule.getCounterOid(), CounterType::ACL_COUNTER, serializedCounterStatAttrs); + m_countersDb.hset(COUNTERS_ACL_COUNTER_RULE_MAP, ruleIdentifier, counterOidStr); +} + +void AclOrch::deregisterFlexCounter(const AclRule& rule) +{ + auto ruleIdentifier = generateAclRuleIdentifierInCountersDb(rule); + m_countersDb.hdel(COUNTERS_ACL_COUNTER_RULE_MAP, rule.getId()); + m_flex_counter_manager.clearCounterIdList(rule.getCounterOid()); +} + +string AclOrch::generateAclRuleIdentifierInCountersDb(const AclRule& rule) const +{ + return rule.getTableId() + m_countersTable.getTableNameSeparator() + rule.getId(); +} + bool AclOrch::getAclBindPortId(Port &port, sai_object_id_t &port_id) { SWSS_LOG_ENTER(); diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index f2a5e0826b87..a34d06846e1d 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -15,14 +15,10 @@ #include "mirrororch.h" #include "dtelorch.h" #include "observer.h" +#include "flex_counter_manager.h" #include "acltable.h" -// ACL counters update interval in the DB -// Value is in seconds. Should not be less than 5 seconds -// (in worst case update of 1265 counters takes almost 5 sec) -#define COUNTERS_READ_INTERVAL 10 - #define RULE_PRIORITY "PRIORITY" #define MATCH_IN_PORTS "IN_PORTS" #define MATCH_OUT_PORTS "OUT_PORTS" @@ -94,6 +90,8 @@ #define RULE_OPER_ADD 0 #define RULE_OPER_DELETE 1 +#define ACL_COUNTER_FLEX_COUNTER_GROUP "ACL_STAT_COUNTER" + typedef map acl_rule_attr_lookup_t; typedef map acl_ip_type_lookup_t; typedef map acl_dtel_flow_op_type_lookup_t; @@ -126,31 +124,6 @@ class AclRange static map m_ranges; }; -struct AclRuleCounters -{ - uint64_t packets; - uint64_t bytes; - - AclRuleCounters(uint64_t p = 0, uint64_t b = 0) : - packets(p), - bytes(b) - { - } - - AclRuleCounters(const AclRuleCounters& rhs) : - packets(rhs.packets), - bytes(rhs.bytes) - { - } - - AclRuleCounters& operator +=(const AclRuleCounters& rhs) - { - packets += rhs.packets; - bytes += rhs.bytes; - return *this; - } -}; - class AclRule { public: @@ -173,24 +146,33 @@ class AclRule virtual bool enableCounter(); virtual bool disableCounter(); - virtual AclRuleCounters getCounters(); - string getId() + sai_object_id_t getOid() const + { + return m_ruleOid; + } + + string getId() const { return m_id; } - string getTableId() + string getTableId() const { return m_tableId; } - sai_object_id_t getCounterOid() + sai_object_id_t getCounterOid() const { return m_counterOid; } - vector getInPorts() + bool hasCounter() const + { + return getCounterOid() != SAI_NULL_OBJECT_ID; + } + + vector getInPorts() { return m_inPorts; } @@ -200,8 +182,10 @@ class AclRule protected: virtual bool createCounter(); + virtual bool createRule(); virtual bool removeCounter(); virtual bool removeRanges(); + virtual bool removeRule(); void decreaseNextHopRefCount(); @@ -270,15 +254,16 @@ class AclRuleMirror: public AclRule bool validateAddAction(string attr_name, string attr_value); bool validateAddMatch(string attr_name, string attr_value); bool validate(); - bool create(); - bool remove(); + bool createRule(); + bool removeRule(); void update(SubjectType, void *); - AclRuleCounters getCounters(); + + bool activate(); + bool deactivate(); protected: bool m_state {false}; string m_sessionName; - AclRuleCounters counters; MirrorOrch *m_pMirrorOrch {nullptr}; }; @@ -288,10 +273,13 @@ class AclRuleDTelFlowWatchListEntry: public AclRule AclRuleDTelFlowWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table, acl_table_type_t type); bool validateAddAction(string attr_name, string attr_value); bool validate(); - bool create(); - bool remove(); + bool createRule(); + bool removeRule(); void update(SubjectType, void *); + bool activate(); + bool deactivate(); + protected: DTelOrch *m_pDTelOrch; string m_intSessionId; @@ -428,7 +416,7 @@ class AclOrch : public Orch, public Observer map m_mirrorTableCapabilities; static sai_acl_action_type_t getAclActionFromAclEntry(sai_acl_entry_attr_t attr); - + // Get the OID for the ACL bind point for a given port static bool getAclBindPortId(Port& port, sai_object_id_t& port_id); @@ -443,7 +431,6 @@ class AclOrch : public Orch, public Observer void doTask(Consumer &consumer); void doAclTableTask(Consumer &consumer); void doAclRuleTask(Consumer &consumer); - void doTask(SelectableTimer &timer); void init(vector& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch); void queryMirrorTableCapability(); @@ -476,14 +463,15 @@ class AclOrch : public Orch, public Observer sai_status_t createDTelWatchListTables(); sai_status_t deleteDTelWatchListTables(); + void registerFlexCounter(const AclRule& rule); + void deregisterFlexCounter(const AclRule& rule); + string generateAclRuleIdentifierInCountersDb(const AclRule& rule) const; + map m_AclTables; // TODO: Move all ACL tables into one map: name -> instance map m_ctrlAclTables; - static mutex m_countersMutex; - static condition_variable m_sleepGuard; - static bool m_bCollectCounters; - static DBConnector m_db; + static DBConnector m_countersDb; static Table m_countersTable; map m_mirrorTableId; @@ -491,6 +479,7 @@ class AclOrch : public Orch, public Observer acl_capabilities_t m_aclCapabilities; acl_action_enum_values_capabilities_t m_aclEnumActionCapabilities; + FlexCounterManager m_flex_counter_manager; }; #endif /* SWSS_ACLORCH_H */ diff --git a/orchagent/flex_counter/flex_counter_manager.cpp b/orchagent/flex_counter/flex_counter_manager.cpp index c924b269e85c..461506ba55e0 100644 --- a/orchagent/flex_counter/flex_counter_manager.cpp +++ b/orchagent/flex_counter/flex_counter_manager.cpp @@ -39,6 +39,7 @@ const unordered_map FlexCounterManager::counter_id_field_lo { CounterType::PORT, PORT_COUNTER_ID_LIST }, { CounterType::QUEUE, QUEUE_COUNTER_ID_LIST }, { CounterType::MACSEC_SA_ATTR, MACSEC_SA_ATTR_ID_LIST }, + { CounterType::ACL_COUNTER, ACL_COUNTER_ATTR_ID_LIST }, { CounterType::TUNNEL, TUNNEL_COUNTER_ID_LIST }, }; diff --git a/orchagent/flex_counter/flex_counter_manager.h b/orchagent/flex_counter/flex_counter_manager.h index 2d531d508442..b66a982ab8dd 100644 --- a/orchagent/flex_counter/flex_counter_manager.h +++ b/orchagent/flex_counter/flex_counter_manager.h @@ -26,6 +26,7 @@ enum class CounterType PORT_DEBUG, SWITCH_DEBUG, MACSEC_SA_ATTR, + ACL_COUNTER, TUNNEL, }; diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index df22301bdb4d..7ccc52e06ca3 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -25,6 +25,7 @@ extern Directory gDirectory; #define QUEUE_KEY "QUEUE" #define PG_WATERMARK_KEY "PG_WATERMARK" #define RIF_KEY "RIF" +#define ACL_KEY "ACL" #define TUNNEL_KEY "TUNNEL" unordered_map flexCounterGroupMap = @@ -41,6 +42,7 @@ unordered_map flexCounterGroupMap = {"RIF", RIF_STAT_COUNTER_FLEX_COUNTER_GROUP}, {"RIF_RATES", RIF_RATE_COUNTER_FLEX_COUNTER_GROUP}, {"DEBUG_COUNTER", DEBUG_COUNTER_FLEX_COUNTER_GROUP}, + {"ACL", ACL_COUNTER_FLEX_COUNTER_GROUP}, {"TUNNEL", TUNNEL_STAT_COUNTER_FLEX_COUNTER_GROUP}, }; diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index 6e351139044f..dbf9791b5389 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -427,6 +427,7 @@ def verify_acl_rule( fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) self._check_acl_entry_base(fvs, sai_qualifiers, action, priority) self._check_acl_entry_packet_action(fvs, action) + self._check_acl_entry_counters_map(acl_rule_id) def verify_redirect_acl_rule( self, @@ -451,6 +452,7 @@ def verify_redirect_acl_rule( fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) self._check_acl_entry_base(fvs, sai_qualifiers, "REDIRECT", priority) self._check_acl_entry_redirect_action(fvs, expected_destination) + self._check_acl_entry_counters_map(acl_rule_id) def verify_nat_acl_rule( self, @@ -471,6 +473,7 @@ def verify_nat_acl_rule( fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) self._check_acl_entry_base(fvs, sai_qualifiers, "DO_NOT_NAT", priority) + self._check_acl_entry_counters_map(acl_rule_id) def verify_mirror_acl_rule( self, @@ -496,6 +499,7 @@ def verify_mirror_acl_rule( fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) self._check_acl_entry_base(fvs, sai_qualifiers, "MIRROR", priority) self._check_acl_entry_mirror_action(fvs, session_oid, stage) + self._check_acl_entry_counters_map(acl_rule_id) def verify_acl_rule_set( self, @@ -628,3 +632,12 @@ def _check_acl_entry_redirect_action(self, entry: Dict[str, str], expected_desti def _check_acl_entry_mirror_action(self, entry: Dict[str, str], session_oid: str, stage: str) -> None: assert stage in self.ADB_MIRROR_ACTION_LOOKUP assert entry.get(self.ADB_MIRROR_ACTION_LOOKUP[stage]) == session_oid + + def _check_acl_entry_counters_map(self, acl_entry_oid: str): + entry = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_entry_oid) + counter_oid = entry.get("SAI_ACL_ENTRY_ATTR_ACTION_COUNTER") + if counter_oid is None: + 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 diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index 6fe3e67500ee..60a3a129d176 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -8,6 +8,7 @@ BUFFER_POOL_WATERMARK_KEY = "BUFFER_POOL_WATERMARK" PORT_BUFFER_DROP_KEY = "PORT_BUFFER_DROP" PG_WATERMARK_KEY = "PG_WATERMARK" +ACL_KEY = "ACL" TUNNEL_KEY = "TUNNEL" # Counter stats on FlexCountersDB @@ -17,6 +18,7 @@ BUFFER_POOL_WATERMARK_STAT = "BUFFER_POOL_WATERMARK_STAT_COUNTER" PORT_BUFFER_DROP_STAT = "PORT_BUFFER_DROP_STAT" PG_WATERMARK_STAT = "PG_WATERMARK_STAT_COUNTER" +ACL_STAT = "ACL_STAT_COUNTER" TUNNEL_STAT = "TUNNEL_STAT_COUNTER" # Counter maps on CountersDB @@ -26,6 +28,7 @@ BUFFER_POOL_WATERMARK_MAP = "COUNTERS_BUFFER_POOL_NAME_MAP" PORT_BUFFER_DROP_MAP = "COUNTERS_PORT_NAME_MAP" PG_WATERMARK_MAP = "COUNTERS_PG_NAME_MAP" +ACL_MAP = "ACL_COUNTER_RULE_MAP" TUNNEL_MAP = "COUNTERS_TUNNEL_NAME_MAP" TUNNEL_TYPE_MAP = "COUNTERS_TUNNEL_TYPE_MAP" @@ -38,6 +41,7 @@ "buffer_pool_watermark_counter":[BUFFER_POOL_WATERMARK_KEY, BUFFER_POOL_WATERMARK_STAT, BUFFER_POOL_WATERMARK_MAP], "port_buffer_drop_counter":[PORT_BUFFER_DROP_KEY, PORT_BUFFER_DROP_STAT, PORT_BUFFER_DROP_MAP], "pg_watermark_counter":[PG_WATERMARK_KEY, PG_WATERMARK_STAT, PG_WATERMARK_MAP], + "acl_counter":[ACL_KEY, ACL_STAT, ACL_MAP], "vxlan_tunnel_counter":[TUNNEL_KEY, TUNNEL_STAT, TUNNEL_MAP]} @@ -124,6 +128,21 @@ def test_flex_counters(self, dvs, counter_type): if counter_type == "rif_counter": self.config_db.db_connection.hset('INTERFACE|Ethernet0', "NULL", "NULL") self.config_db.db_connection.hset('INTERFACE|Ethernet0|192.168.0.1/24', "NULL", "NULL") + elif counter_type == "acl_counter": + self.config_db.create_entry('ACL_TABLE', 'DATAACL', + { + 'STAGE': 'INGRESS', + 'PORTS': 'Ethernet0', + 'TYPE': 'L3' + } + ) + self.config_db.create_entry('ACL_RULE', 'DATAACL|RULE0', + { + 'ETHER_TYPE': '2048', + 'PACKET_ACTION': 'FORWARD', + 'PRIORITY': '9999' + } + ) if counter_type == "vxlan_tunnel_counter": self.config_db.db_connection.hset("VLAN|Vlan10", "vlanid", "10") @@ -136,11 +155,12 @@ def test_flex_counters(self, dvs, counter_type): if counter_type == "port_counter": self.verify_only_phy_ports_created() - - if counter_type == "rif_counter": + elif counter_type == "rif_counter": self.config_db.db_connection.hdel('INTERFACE|Ethernet0|192.168.0.1/24', "NULL") - - if counter_type == "vxlan_tunnel_counter": + elif counter_type == "acl_counter": + self.config_db.delete_entry('ACL_RULE', 'DATAACL|RULE0') + self.config_db.delete_entry('ACL_TABLE', 'DATAACL') + elif counter_type == "vxlan_tunnel_counter": self.verify_tunnel_type_vxlan(counter_map, TUNNEL_TYPE_MAP) self.config_db.delete_entry("VLAN","Vlan10") self.config_db.delete_entry("VLAN_TUNNEL","vtep1")