From 332ad03b8e4ce2866866b41388cb8abecf8aa453 Mon Sep 17 00:00:00 2001 From: Vadym Hlushko Date: Tue, 29 Mar 2022 12:41:49 +0000 Subject: [PATCH 1/5] [pbh] [aclorch] Addded the register and deregister functions for flex counter before updating counter for ACL rule Signed-off-by: Vadym Hlushko --- orchagent/aclorch.cpp | 4 ++++ orchagent/aclorch.h | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 9e873a47db..1f8b3d2436 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -984,9 +984,13 @@ bool AclRule::updateCounter(const AclRule& updatedRule) { return false; } + + m_pAclOrch->registerFlexCounter(*this); } else { + m_pAclOrch->deregisterFlexCounter(*this); + if (!disableCounter()) { return false; diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 9e6db3919c..710720a5c1 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -489,6 +489,9 @@ class AclOrch : public Orch, public Observer bool m_isCombinedMirrorV6Table = true; map m_mirrorTableCapabilities; + void registerFlexCounter(const AclRule& rule); + void deregisterFlexCounter(const AclRule& rule); + // Get the OID for the ACL bind point for a given port static bool getAclBindPortId(Port& port, sai_object_id_t& port_id); @@ -537,8 +540,6 @@ class AclOrch : public Orch, public Observer void createDTelWatchListTables(); void deleteDTelWatchListTables(); - void registerFlexCounter(const AclRule& rule); - void deregisterFlexCounter(const AclRule& rule); string generateAclRuleIdentifierInCountersDb(const AclRule& rule) const; map m_AclTables; From ee106ba61399a450770f3907f3532497e29af858 Mon Sep 17 00:00:00 2001 From: Vadym Hlushko Date: Fri, 1 Apr 2022 15:57:50 +0000 Subject: [PATCH 2/5] [pbh] [aclorch] Added UT Signed-off-by: Vadym Hlushko --- tests/test_pbh.py | 113 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/test_pbh.py b/tests/test_pbh.py index 65155ba2e9..90780e4c16 100644 --- a/tests/test_pbh.py +++ b/tests/test_pbh.py @@ -1,6 +1,8 @@ import pytest import logging +import test_flex_counters as flex_counter_module + PBH_HASH_FIELD_NAME = "inner_ip_proto" PBH_HASH_FIELD_HASH_FIELD = "INNER_IP_PROTOCOL" @@ -358,6 +360,117 @@ def test_PbhRuleUpdate(self, testlog): self.dvs_pbh.verify_pbh_hash_field_count(0) + def test_PbhRuleUpdateFlowCounter(self, dvs, testlog): + try: + # PBH hash field + pbhlogger.info("Create PBH hash field: {}".format(PBH_HASH_FIELD_NAME)) + self.dvs_pbh.create_pbh_hash_field( + hash_field_name=PBH_HASH_FIELD_NAME, + hash_field=PBH_HASH_FIELD_HASH_FIELD, + sequence_id=PBH_HASH_FIELD_SEQUENCE_ID + ) + self.dvs_pbh.verify_pbh_hash_field_count(1) + + # PBH hash + pbhlogger.info("Create PBH hash: {}".format(PBH_HASH_NAME)) + self.dvs_pbh.create_pbh_hash( + hash_name=PBH_HASH_NAME, + hash_field_list=PBH_HASH_HASH_FIELD_LIST + ) + self.dvs_pbh.verify_pbh_hash_count(1) + + # PBH table + pbhlogger.info("Create PBH table: {}".format(PBH_TABLE_NAME)) + self.dvs_pbh.create_pbh_table( + table_name=PBH_TABLE_NAME, + interface_list=PBH_TABLE_INTERFACE_LIST, + description=PBH_TABLE_DESCRIPTION + ) + self.dvs_acl.verify_acl_table_count(1) + + # Prepare ACL FLEX Counter environment + meta_data = flex_counter_module.counter_group_meta['acl_counter'] + counter_key = meta_data['key'] + counter_stat = meta_data['group_name'] + counter_map = meta_data['name_map'] + + test_flex_counters = flex_counter_module.TestFlexCounters() + test_flex_counters.setup_dbs(dvs) + test_flex_counters.verify_no_flex_counters_tables(counter_stat) + + # PBH rule + pbhlogger.info("Create PBH rule: {}".format(PBH_RULE_NAME)) + + attr_dict = { + "ether_type": PBH_RULE_ETHER_TYPE, + "ip_protocol": PBH_RULE_IP_PROTOCOL, + "gre_key": PBH_RULE_GRE_KEY, + "inner_ether_type": PBH_RULE_INNER_ETHER_TYPE + } + + self.dvs_pbh.create_pbh_rule( + table_name=PBH_TABLE_NAME, + rule_name=PBH_RULE_NAME, + priority=PBH_RULE_PRIORITY, + qualifiers=attr_dict, + hash_name=PBH_RULE_HASH, + packet_action="SET_ECMP_HASH", + flow_counter="ENABLED" + ) + self.dvs_acl.verify_acl_rule_count(1) + + pbhlogger.info("Enable a ACL FLEX counter") + test_flex_counters.set_flex_counter_group_status(counter_key, counter_map) + test_flex_counters.verify_flex_counters_populated(counter_map, counter_stat) + test_flex_counters.set_flex_counter_group_interval(counter_key, counter_stat, '2500') + self.dvs_acl.get_acl_counter_ids(1) + + pbhlogger.info("Disable a flow counter for PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.update_pbh_rule( + table_name=PBH_TABLE_NAME, + rule_name=PBH_RULE_NAME, + priority=PBH_RULE_PRIORITY, + qualifiers=attr_dict, + hash_name=PBH_RULE_HASH, + packet_action="SET_ECMP_HASH", + flow_counter="DISABLED" + ) + self.dvs_acl.get_acl_counter_ids(0) + + pbhlogger.info("Enable a flow counter for PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.update_pbh_rule( + table_name=PBH_TABLE_NAME, + rule_name=PBH_RULE_NAME, + priority=PBH_RULE_PRIORITY, + qualifiers=attr_dict, + hash_name=PBH_RULE_HASH, + packet_action="SET_ECMP_HASH", + flow_counter="ENABLED" + ) + self.dvs_acl.get_acl_counter_ids(1) + + finally: + # PBH rule + pbhlogger.info("Remove PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.remove_pbh_rule(PBH_TABLE_NAME, PBH_RULE_NAME) + self.dvs_acl.verify_acl_rule_count(0) + + # PBH table + pbhlogger.info("Remove PBH table: {}".format(PBH_TABLE_NAME)) + self.dvs_pbh.remove_pbh_table(PBH_TABLE_NAME) + self.dvs_acl.verify_acl_table_count(0) + + # PBH hash + pbhlogger.info("Remove PBH hash: {}".format(PBH_HASH_NAME)) + self.dvs_pbh.remove_pbh_hash(PBH_HASH_NAME) + self.dvs_pbh.verify_pbh_hash_count(0) + + # PBH hash field + pbhlogger.info("Remove PBH hash field: {}".format(PBH_HASH_FIELD_NAME)) + self.dvs_pbh.remove_pbh_hash_field(PBH_HASH_FIELD_NAME) + self.dvs_pbh.verify_pbh_hash_field_count(0) + + @pytest.mark.usefixtures("dvs_lag_manager") class TestPbhExtendedFlows: class PbhRefCountHelper(object): From f0e50405de1fc49844c6b5f971d8dcfb48d601db Mon Sep 17 00:00:00 2001 From: Vadym Hlushko Date: Thu, 7 Apr 2022 18:15:07 +0000 Subject: [PATCH 3/5] [pbh] [aclorch] Fixed deletion flow for ACL_COUNTER_RULE_MAP, fixed review comments Signed-off-by: Vadym Hlushko --- orchagent/aclorch.cpp | 2 +- tests/test_pbh.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 1f8b3d2436..0196204715 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -4471,7 +4471,7 @@ void AclOrch::registerFlexCounter(const AclRule& rule) void AclOrch::deregisterFlexCounter(const AclRule& rule) { auto ruleIdentifier = generateAclRuleIdentifierInCountersDb(rule); - m_countersDb.hdel(COUNTERS_ACL_COUNTER_RULE_MAP, rule.getId()); + m_countersDb.hdel(COUNTERS_ACL_COUNTER_RULE_MAP, ruleIdentifier); m_flex_counter_manager.clearCounterIdList(rule.getCounterOid()); } diff --git a/tests/test_pbh.py b/tests/test_pbh.py index 90780e4c16..01e6b67a7a 100644 --- a/tests/test_pbh.py +++ b/tests/test_pbh.py @@ -1,5 +1,6 @@ import pytest import logging +import time import test_flex_counters as flex_counter_module @@ -418,12 +419,13 @@ def test_PbhRuleUpdateFlowCounter(self, dvs, testlog): flow_counter="ENABLED" ) self.dvs_acl.verify_acl_rule_count(1) + self.dvs_acl.get_acl_counter_ids(1) pbhlogger.info("Enable a ACL FLEX counter") test_flex_counters.set_flex_counter_group_status(counter_key, counter_map) - test_flex_counters.verify_flex_counters_populated(counter_map, counter_stat) + time.sleep(1) test_flex_counters.set_flex_counter_group_interval(counter_key, counter_stat, '2500') - self.dvs_acl.get_acl_counter_ids(1) + test_flex_counters.verify_flex_counters_populated(counter_map, counter_stat) pbhlogger.info("Disable a flow counter for PBH rule: {}".format(PBH_RULE_NAME)) self.dvs_pbh.update_pbh_rule( @@ -470,6 +472,10 @@ def test_PbhRuleUpdateFlowCounter(self, dvs, testlog): self.dvs_pbh.remove_pbh_hash_field(PBH_HASH_FIELD_NAME) self.dvs_pbh.verify_pbh_hash_field_count(0) + # ACL FLEX counter + pbhlogger.info("Disable ACL FLEX counter") + test_flex_counters.post_trap_flow_counter_test(meta_data) + @pytest.mark.usefixtures("dvs_lag_manager") class TestPbhExtendedFlows: From af190f2aed30158c9e92b7cbef1f592dafb49ed5 Mon Sep 17 00:00:00 2001 From: Vadym Hlushko Date: Fri, 8 Apr 2022 08:40:36 +0000 Subject: [PATCH 4/5] Fixed review comments Signed-off-by: Vadym Hlushko --- tests/test_pbh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pbh.py b/tests/test_pbh.py index 01e6b67a7a..9dbe623dae 100644 --- a/tests/test_pbh.py +++ b/tests/test_pbh.py @@ -423,8 +423,8 @@ def test_PbhRuleUpdateFlowCounter(self, dvs, testlog): pbhlogger.info("Enable a ACL FLEX counter") test_flex_counters.set_flex_counter_group_status(counter_key, counter_map) + test_flex_counters.set_flex_counter_group_interval(counter_key, counter_stat, '1000') time.sleep(1) - test_flex_counters.set_flex_counter_group_interval(counter_key, counter_stat, '2500') test_flex_counters.verify_flex_counters_populated(counter_map, counter_stat) pbhlogger.info("Disable a flow counter for PBH rule: {}".format(PBH_RULE_NAME)) From 468cfb6d38ef52ecdd66006bd9dd9f29ede82a16 Mon Sep 17 00:00:00 2001 From: Vadym Hlushko Date: Fri, 8 Apr 2022 13:31:02 +0000 Subject: [PATCH 5/5] Removed time Signed-off-by: Vadym Hlushko --- tests/test_pbh.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_pbh.py b/tests/test_pbh.py index 9dbe623dae..270e59d429 100644 --- a/tests/test_pbh.py +++ b/tests/test_pbh.py @@ -1,6 +1,5 @@ import pytest import logging -import time import test_flex_counters as flex_counter_module @@ -424,7 +423,6 @@ def test_PbhRuleUpdateFlowCounter(self, dvs, testlog): pbhlogger.info("Enable a ACL FLEX counter") test_flex_counters.set_flex_counter_group_status(counter_key, counter_map) test_flex_counters.set_flex_counter_group_interval(counter_key, counter_stat, '1000') - time.sleep(1) test_flex_counters.verify_flex_counters_populated(counter_map, counter_stat) pbhlogger.info("Disable a flow counter for PBH rule: {}".format(PBH_RULE_NAME))