[aclshow] enhance ACL counters to work with FC infrastructure (sonic-…

…net#1858)

#### What I did

Made a change for aclshow and counterpoll that adds support for ACL flex counters.

DEPENDS ON: sonic-net/sonic-swss-common#533 sonic-net/sonic-sairedis#953 sonic-net/sonic-swss#1943
HLD: sonic-net/SONiC#857

#### How I did it

Modified aclshow and counterpoll and UT.

#### How to verify it

Together with depends PRs. Run ACL/Everflow test suite.

counterpoll/main.py

@@ -6,6 +6,7 @@
 BUFFER_POOL_WATERMARK = "BUFFER_POOL_WATERMARK"
 PORT_BUFFER_DROP = "PORT_BUFFER_DROP"
 PG_DROP = "PG_DROP"
+ACL = "ACL"
 DISABLE = "disable"
 ENABLE = "enable"
 DEFLT_60_SEC= "default (60000)"
@@ -241,6 +242,45 @@ def disable():
     configdb.mod_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK", fc_info)
     configdb.mod_entry("FLEX_COUNTER_TABLE", BUFFER_POOL_WATERMARK, fc_info)
 
+# ACL counter commands
+@cli.group()
+@click.pass_context
+def acl(ctx):
+    """  ACL counter commands """
+    ctx.obj = ConfigDBConnector()
+    ctx.obj.connect()
+
+@acl.command()
+@click.argument('poll_interval', type=click.IntRange(1000, 30000))
+@click.pass_context
+def interval(ctx, poll_interval):
+    """
+    Set ACL counters query interval
+    interval is between 1s and 30s.
+    """
+
+    fc_group_cfg = {}
+    fc_group_cfg['POLL_INTERVAL'] = poll_interval
+    ctx.obj.mod_entry("FLEX_COUNTER_TABLE", ACL, fc_group_cfg)
+
+@acl.command()
+@click.pass_context
+def enable(ctx):
+    """ Enable ACL counter query """
+
+    fc_group_cfg = {}
+    fc_group_cfg['FLEX_COUNTER_STATUS'] = ENABLE
+    ctx.obj.mod_entry("FLEX_COUNTER_TABLE", ACL, fc_group_cfg)
+
+@acl.command()
+@click.pass_context
+def disable(ctx):
+    """ Disable ACL counter query """
+
+    fc_group_cfg = {}
+    fc_group_cfg['FLEX_COUNTER_STATUS'] = DISABLE
+    ctx.obj.mod_entry("FLEX_COUNTER_TABLE", ACL, fc_group_cfg)
+
 # Tunnel counter commands
 @cli.group()
 def tunnel():
@@ -287,8 +327,9 @@ def show():
     pg_wm_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'PG_WATERMARK')
     pg_drop_info = configdb.get_entry('FLEX_COUNTER_TABLE', PG_DROP)
     buffer_pool_wm_info = configdb.get_entry('FLEX_COUNTER_TABLE', BUFFER_POOL_WATERMARK)
+    acl_info = configdb.get_entry('FLEX_COUNTER_TABLE', ACL)
     tunnel_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'TUNNEL')
-    
+
     header = ("Type", "Interval (in ms)", "Status")
     data = []
     if queue_info:
@@ -307,6 +348,8 @@ def show():
         data.append(['PG_DROP_STAT', pg_drop_info.get("POLL_INTERVAL", DEFLT_10_SEC), pg_drop_info.get("FLEX_COUNTER_STATUS", DISABLE)])
     if buffer_pool_wm_info:
         data.append(["BUFFER_POOL_WATERMARK_STAT", buffer_pool_wm_info.get("POLL_INTERVAL", DEFLT_10_SEC), buffer_pool_wm_info.get("FLEX_COUNTER_STATUS", DISABLE)])
+    if acl_info:
+        data.append([ACL, pg_drop_info.get("POLL_INTERVAL", DEFLT_10_SEC), acl_info.get("FLEX_COUNTER_STATUS", DISABLE)])
     if tunnel_info:
         data.append(["TUNNEL_STAT", rif_info.get("POLL_INTERVAL", DEFLT_10_SEC), rif_info.get("FLEX_COUNTER_STATUS", DISABLE)])
 

scripts/aclshow

@@ -29,13 +29,15 @@ from tabulate import tabulate
 ### if we could have a SAI command to clear counters will be better, so no need to maintain
 ### counters in temp loaction for clear conter action
 COUNTER_POSITION = '/tmp/.counters_acl.p'
+COUNTERS = "COUNTERS"
+ACL_COUNTER_RULE_MAP = "ACL_COUNTER_RULE_MAP"
 
 ### acl display header
 ACL_HEADER = ["RULE NAME", "TABLE NAME", "PRIO", "PACKETS COUNT", "BYTES COUNT"]
 
-# some constants for rule properties
-PACKETS_COUNTER = "packets counter"
-BYTES_COUNTER = "bytes counter"
+COUNTER_PACKETS_ATTR = "SAI_ACL_COUNTER_ATTR_PACKETS"
+COUNTER_BYTES_ATTR = "SAI_ACL_COUNTER_ATTR_BYTES"
+
 
 class AclStat(object):
     """
@@ -91,8 +93,13 @@ class AclStat(object):
         read redis database for acl counters
         """
 
-        def lowercase_keys(dictionary):
-            return dict((k.lower(), v) for k, v in dictionary.items()) if dictionary else None
+        def get_acl_rule_counter_map():
+            """
+            Return ACL_COUNTER_RULE_MAP
+            """
+            if self.db.exists(self.db.COUNTERS_DB, ACL_COUNTER_RULE_MAP):
+                return self.db.get_all(self.db.COUNTERS_DB, ACL_COUNTER_RULE_MAP)
+            return {}
 
         def fetch_acl_tables():
             """
@@ -124,8 +131,18 @@ class AclStat(object):
             """
             Get ACL counters from the DB
             """
+            counters_db_separator = self.db.get_db_separator(self.db.COUNTERS_DB)
+            rule_to_counter_map = get_acl_rule_counter_map()
             for table, rule in self.acl_rules:
-                cnt_props = lowercase_keys(self.db.get_all(self.db.COUNTERS_DB, "COUNTERS:%s:%s" % (table, rule)))
+                self.acl_counters[table, rule] = {}
+                rule_identifier = table + counters_db_separator + rule
+                if not rule_to_counter_map:
+                    continue
+                counter_oid = rule_to_counter_map.get(rule_identifier)
+                if not counter_oid:
+                    continue
+                counters_db_key = COUNTERS + counters_db_separator + counter_oid
+                cnt_props = self.db.get_all(self.db.COUNTERS_DB, counters_db_key)
                 self.acl_counters[table, rule] = cnt_props
 
             if verboseflag:
@@ -164,8 +181,8 @@ class AclStat(object):
         header = ACL_HEADER
         aclstat = []
         for rule_key in self.acl_rules:
-            if not display_all and (self.get_counter_value(rule_key, 'packets') == '0' or \
-                    self.get_counter_value(rule_key, 'packets') == 'N/A'):
+            if not display_all and (self.get_counter_value(rule_key, COUNTER_PACKETS_ATTR) == '0' or \
+                    self.get_counter_value(rule_key, COUNTER_PACKETS_ATTR) == 'N/A'):
                 continue
             rule = self.acl_rules[rule_key]
             rule_priority = -1
@@ -174,8 +191,8 @@ class AclStat(object):
                     rule_priority = val
             line = [rule_key[1], rule_key[0],
                     rule_priority,
-                    self.get_counter_value(rule_key, 'packets'),
-                    self.get_counter_value(rule_key, 'bytes')]
+                    self.get_counter_value(rule_key, COUNTER_PACKETS_ATTR),
+                    self.get_counter_value(rule_key, COUNTER_BYTES_ATTR)]
             aclstat.append(line)
 
         # sort the list with table name first and then descending priority

show/plugins/pbh.py

@@ -16,6 +16,12 @@
 
 PBH_COUNTERS_LOCATION = '/tmp/.pbh_counters.txt'
 
+COUNTER_PACKETS_ATTR = "SAI_ACL_COUNTER_ATTR_PACKETS"
+COUNTER_BYTES_ATTR = "SAI_ACL_COUNTER_ATTR_BYTES"
+
+COUNTERS = "COUNTERS"
+ACL_COUNTER_RULE_MAP = "ACL_COUNTER_RULE_MAP"
+
 pbh_hash_field_tbl_name = 'PBH_HASH_FIELD'
 pbh_hash_tbl_name = 'PBH_HASH'
 pbh_table_tbl_name = 'PBH_TABLE'
@@ -377,8 +383,8 @@ def PBH_STATISTICS(db):
             row = [
                 key[0],
                 key[1],
-                get_counter_value(pbh_counters, saved_pbh_counters, key, 'packets'),
-                get_counter_value(pbh_counters, saved_pbh_counters, key, 'bytes'),
+                get_counter_value(pbh_counters, saved_pbh_counters, key, COUNTER_PACKETS_ATTR),
+                get_counter_value(pbh_counters, saved_pbh_counters, key, COUNTER_BYTES_ATTR),
             ]
             body.append(row)
 
@@ -415,14 +421,30 @@ def read_saved_pbh_counters():
     return {}
 
 
+def read_acl_rule_counter_map(db_connector):
+    if db_connector.exists(db_connector.COUNTERS_DB, ACL_COUNTER_RULE_MAP):
+        return db_connector.get_all(db_connector.COUNTERS_DB, ACL_COUNTER_RULE_MAP)
+    return {}
+
+
 def read_pbh_counters(pbh_rules) -> dict:
     pbh_counters = {}
 
     db_connector = SonicV2Connector(use_unix_socket_path=False)
     db_connector.connect(db_connector.COUNTERS_DB)
+    counters_db_separator = db_connector.get_db_separator(db_connector.COUNTERS_DB)
+    rule_to_counter_map = read_acl_rule_counter_map(db_connector)
 
     for table, rule in natsort.natsorted(pbh_rules):
-        counter_props = lowercase_keys(db_connector.get_all(db_connector.COUNTERS_DB, "COUNTERS:%s:%s" % (table, rule)))
+        pbh_counters[table, rule] = {}
+        rule_identifier = table + counters_db_separator + rule
+        if not rule_to_counter_map:
+            continue
+        counter_oid = rule_to_counter_map.get(rule_identifier)
+        if not counter_oid:
+            continue
+        counters_db_key = COUNTERS + counters_db_separator + counter_oid
+        counter_props = db_connector.get_all(db_connector.COUNTERS_DB, counters_db_key)
         if counter_props:
             pbh_counters[table, rule] = counter_props
 
@@ -457,10 +479,6 @@ def inject_symmetric_field(obj_list):
         counter = 0
 
 
-def lowercase_keys(dictionary):
-    return dict((k.lower(), v) for k, v in dictionary.items()) if dictionary else None
-
-
 def register(cli):
     cli_node = PBH
     if cli_node.name in cli.commands:

tests/counterpoll_test.py

@@ -26,6 +26,7 @@
 QUEUE_WATERMARK_STAT               10000  enable
 PG_WATERMARK_STAT                  10000  enable
 PG_DROP_STAT                       10000  enable
+ACL                                10000  enable
 """
 
 class TestCounterpoll(object):
@@ -63,6 +64,15 @@ def test_pg_drop_interval_too_long(self):
         assert result.exit_code == 2
         assert expected in result.output
 
+    @pytest.mark.parametrize("interval", [100, 50000])
+    def test_acl_interval_range(self, interval):
+        runner = CliRunner()
+        result = runner.invoke(counterpoll.cli.commands["acl"].commands["interval"], [str(interval)])
+        print(result.output)
+        expected = "Invalid value for \"POLL_INTERVAL\": {} is not in the valid range of 1000 to 30000.".format(interval)
+        assert result.exit_code == 2
+        assert expected in result.output
+
     @pytest.fixture(scope='class')
     def _get_config_db_file(self):
         sample_config_db_file = os.path.join(test_path, "counterpoll_input", "config_db.json")
@@ -109,6 +119,30 @@ def test_update_pg_drop_interval(self):
         table = db.cfgdb.get_table('FLEX_COUNTER_TABLE')
         assert test_interval == table["PG_DROP"]["POLL_INTERVAL"]
 
+    @pytest.mark.parametrize("status", ["disable", "enable"])
+    def test_update_acl_status(self, status):
+        runner = CliRunner()
+        db = Db()
+
+        result = runner.invoke(counterpoll.cli.commands["acl"].commands[status], [], obj=db.cfgdb)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+
+        table = db.cfgdb.get_table("FLEX_COUNTER_TABLE")
+        assert status == table["ACL"]["FLEX_COUNTER_STATUS"]
+
+    def test_update_acl_interval(self):
+        runner = CliRunner()
+        db = Db()
+        test_interval = "20000"
+
+        result = runner.invoke(counterpoll.cli.commands["acl"].commands["interval"], [test_interval], obj=db.cfgdb)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+
+        table = db.cfgdb.get_table("FLEX_COUNTER_TABLE")
+        assert test_interval == table["ACL"]["POLL_INTERVAL"]
+
     @classmethod
     def teardown_class(cls):
         print("TEARDOWN")

tests/mock_tables/asic0/counters_db.json

@@ -1419,49 +1419,62 @@
             "oid:0x600000000063d": "SAI_ROUTER_INTERFACE_TYPE_PORT",
             "oid:0x600000000065f": "SAI_ROUTER_INTERFACE_TYPE_PORT"
     },
-    "COUNTERS:DATAACL:DEFAULT_RULE": {
-            "Bytes": "1",
-            "Packets": "2"
-    },
-    "COUNTERS:DATAACL:RULE_1": {
-            "Bytes": "100",
-            "Packets": "101"
-    },
-    "COUNTERS:DATAACL:RULE_2": {
-            "Bytes": "200",
-            "Packets": "201"
-    },
-    "COUNTERS:DATAACL:RULE_3": {
-            "Bytes": "300",
-            "Packets": "301"
-    },
-    "COUNTERS:DATAACL:RULE_4": {
-            "Bytes": "400",
-            "Packets": "401"
-    },
-    "COUNTERS:DATAACL:RULE_05": {
-            "Bytes": "0",
-            "Packets": "0"
-    },
-    "COUNTERS:EVERFLOW:RULE_6": {
-            "Bytes": "600",
-            "Packets": "601"
-    },
-    "COUNTERS:DATAACL:RULE_7":{
-            "Bytes": "700",
-            "Packets": "701"
-    },
-    "COUNTERS:EVERFLOW:RULE_08": {
-            "Bytes": "0",
-            "Packets": "0"
-    },
-    "COUNTERS:DATAACL:RULE_9": {
-            "Bytes": "900",
-            "Packets": "901"
-    },
-    "COUNTERS:DATAACL:RULE_10": {
-            "Bytes": "1000",
-            "Packets": "1001"
+    "COUNTERS:oid:0x9000000000000": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "2",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "1"
+    },
+    "COUNTERS:oid:0x9000000000001": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "101",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "100"
+    },
+    "COUNTERS:oid:0x9000000000002": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "201",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "200"
+    },
+    "COUNTERS:oid:0x9000000000003": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "301",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "300"
+    },
+    "COUNTERS:oid:0x9000000000004": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "401",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "400"
+    },
+    "COUNTERS:oid:0x9000000000005": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "0",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "0"
+    },
+    "COUNTERS:oid:0x9000000000006": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "601",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "600"
+    },
+    "COUNTERS:oid:0x9000000000007":{
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "701",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "700"
+    },
+    "COUNTERS:oid:0x9000000000008": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "0",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "0"
+    },
+    "COUNTERS:oid:0x9000000000009": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "901",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "900"
+    },
+    "COUNTERS:oid:0x900000000000a": {
+        "SAI_ACL_COUNTER_ATTR_PACKETS": "1001",
+        "SAI_ACL_COUNTER_ATTR_BYTES": "1000"
+    },
+    "ACL_COUNTER_RULE_MAP": {
+        "DATAACL:DEFAULT_RULE": "oid:0x9000000000000",
+        "DATAACL:RULE_1": "oid:0x9000000000001",
+        "DATAACL:RULE_2": "oid:0x9000000000002",
+        "DATAACL:RULE_3": "oid:0x9000000000003",
+        "DATAACL:RULE_4": "oid:0x9000000000004",
+        "DATAACL:RULE_05": "oid:0x9000000000005",
+        "EVERFLOW:RULE_6": "oid:0x9000000000006",
+        "DATAACL:RULE_7": "oid:0x9000000000007",
+        "EVERFLOW:RULE_08": "oid:0x9000000000008",
+        "DATAACL:RULE_9": "oid:0x9000000000009",
+        "DATAACL:RULE_10": "oid:0x900000000000a"
     },
     "COUNTERS:oid:0x1000000000002": {
         "SAI_PORT_STAT_IF_IN_UCAST_PKTS": "8",