[Namespace]: Fix SAI_ID key used in cpfcIfTable and csqIfQosGroupStatsTable implementation (sonic-net#138)

In multi-asic platform, SAI OID is not unique for the whole device. It is unique for an asic and within a single namespace.
This PR is to make sure that data structures that use SAI Object ID as a key to retrieve data from COUNTERS_DB updated so that a combination of SAI Object ID and port index is used as a key.
Update data structure to use combination SAI Object ID and port index as a key to retrieve data from COUNTERS_DB.
Update Unit-test mock DB in namespaces, to reflect the scenario where SAI Object is same across different namespaces.
Updates done to MIB implementation for the below MIB tables to get data from the right database instances:
cpfcIfTable
csqIfQosGroupStatsTable
* Remove usage of oid_sai_map data structure as it is not being used.
The required data can be retrieved from oid_name_map data structure.

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Author: SuvarnaMeenakshi

src/sonic_ax_impl/mibs/__init__.py

@@ -245,12 +245,6 @@ def init_sync_d_interface_tables(db_conn):
     logger.debug("Port name map:\n" + pprint.pformat(if_name_map, indent=2))
     logger.debug("Interface name map:\n" + pprint.pformat(if_id_map, indent=2))
 
-    # { OID -> sai_id }
-    oid_sai_map = {get_index(if_name): sai_id for if_name, sai_id in if_name_map.items()
-                   # only map the interface if it's a style understood to be a SONiC interface.
-                   if get_index(if_name) is not None}
-    logger.debug("OID sai map:\n" + pprint.pformat(oid_sai_map, indent=2))
-
     # { OID -> if_name (SONiC) }
     oid_name_map = {get_index(if_name): if_name for if_name in if_name_map
                     # only map the interface if it's a style understood to be a SONiC interface.
@@ -259,14 +253,14 @@ def init_sync_d_interface_tables(db_conn):
     logger.debug("OID name map:\n" + pprint.pformat(oid_name_map, indent=2))
 
     # SyncD consistency checks.
-    if not oid_sai_map:
+    if not oid_name_map:
         # In the event no interface exists that follows the SONiC pattern, no OIDs are able to be registered.
         # A RuntimeError here will prevent the 'main' module from loading. (This is desirable.)
         message = "No interfaces found matching pattern '{}'. SyncD database is incoherent." \
             .format(port_util.SONIC_ETHERNET_RE_PATTERN)
         logger.error(message)
         raise RuntimeError(message)
-    elif len(if_id_map) < len(if_name_map) or len(oid_sai_map) < len(if_name_map):
+    elif len(if_id_map) < len(if_name_map) or len(oid_name_map) < len(if_name_map):
         # a length mismatch indicates a bad interface name
         logger.warning("SyncD database contains incoherent interface names. Interfaces must match pattern '{}'"
                        .format(port_util.SONIC_ETHERNET_RE_PATTERN))
@@ -281,7 +275,7 @@ def init_sync_d_interface_tables(db_conn):
 
     logger.debug("Chassis name map:\n" + pprint.pformat(if_alias_map, indent=2))
 
-    return if_name_map, if_alias_map, if_id_map, oid_sai_map, oid_name_map
+    return if_name_map, if_alias_map, if_id_map, oid_name_map
 
 def init_sync_d_lag_tables(db_conn):
     """
@@ -333,15 +327,15 @@ def init_sync_d_queue_tables(db_conn):
     :return: tuple(port_queues_map, queue_stat_map)
     """
 
-    # { Port index : Queue index (SONiC) -> sai_id }
-    # ex: { "1:2" : "1000000000023" }
+    # { Port name : Queue index (SONiC) -> sai_id }
+    # ex: { "Ethernet0:2" : "1000000000023" }
     queue_name_map = db_conn.get_all(COUNTERS_DB, COUNTERS_QUEUE_NAME_MAP, blocking=True)
     logger.debug("Queue name map:\n" + pprint.pformat(queue_name_map, indent=2))
 
     # Parse the queue_name_map and create the following maps:
-    # port_queues_map -> {"if_index : queue_index" : sai_oid}
-    # queue_stat_map -> {queue stat table name : {counter name : value}}
-    # port_queue_list_map -> {if_index: [sorted queue list]}
+    # port_queues_map -> {"port_index : queue_index" : sai_oid}
+    # queue_stat_map -> {"port_index : queue stat table name" : {counter name : value}} 
+    # port_queue_list_map -> {port_index: [sorted queue list]}
     port_queues_map = {}
     queue_stat_map = {}
     port_queue_list_map = {}
@@ -356,7 +350,8 @@ def init_sync_d_queue_tables(db_conn):
         queue_stat_name = queue_table(sai_id)
         queue_stat = db_conn.get_all(COUNTERS_DB, queue_stat_name, blocking=False)
         if queue_stat is not None:
-            queue_stat_map[queue_stat_name] = queue_stat
+            queue_stat_key = queue_key(port_index, queue_stat_name)
+            queue_stat_map[queue_stat_key] = queue_stat
 
         if not port_queue_list_map.get(int(port_index)):
             port_queue_list_map[int(port_index)] = [int(queue_index)]
@@ -577,7 +572,7 @@ def get_non_host_dbs(dbs):
             return dbs
         else:
             return dbs[1:]
-        
+
     @staticmethod
     def get_sync_d_from_all_namespace(per_namespace_func, dbs):
         # return merged tuple of dictionaries retrieved from per

src/sonic_ax_impl/mibs/ieee802_1ab.py

@@ -145,7 +145,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
 
         self.mgmt_oid_name_map = {}
@@ -165,7 +164,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.mgmt_oid_name_map, \
@@ -382,7 +380,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
 
         self.mgmt_oid_name_map = {}
@@ -400,7 +397,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.mgmt_oid_name_map, _ = mibs.init_mgmt_interface_tables(self.db_conn[0])
@@ -566,7 +562,7 @@ def reinit_data(self):
         """
         Subclass reinit data routine.
         """
-        _, _, _, _, self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
+        _, _, _, self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.mgmt_oid_name_map, _ = mibs.init_mgmt_interface_tables(self.db_conn[0])
 

src/sonic_ax_impl/mibs/ietf/rfc1213.py

@@ -200,7 +200,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
         self.namespace_db_map = Namespace.get_namespace_db_map(self.db_conn)
 
@@ -211,7 +210,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
         """
         db_conn - will have db_conn to all namespace DBs and
@@ -236,7 +234,7 @@ def update_data(self):
         self.if_name_lag_name_map, \
         self.oid_lag_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_lag_tables, self.db_conn)
 
-        self.if_range = sorted(list(self.oid_sai_map.keys()) +
+        self.if_range = sorted(list(self.oid_name_map.keys()) +
                                list(self.oid_lag_name_map.keys()) +
                                list(self.mgmt_oid_name_map.keys()))
         self.if_range = [(i,) for i in self.if_range]

src/sonic_ax_impl/mibs/ietf/rfc2737.py

@@ -152,7 +152,7 @@ def reinit_data(self):
         self.physical_model_name_map = {}
 
         # update interface maps
-        _, self.if_alias_map, _, _, _ = \
+        _, self.if_alias_map, _, _ = \
             Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, Namespace.init_namespace_dbs())
 
         device_metadata = mibs.get_device_metadata(self.statedb[0])

src/sonic_ax_impl/mibs/ietf/rfc2863.py

@@ -61,7 +61,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
         self.lag_name_if_name_map = {}
         self.if_name_lag_name_map = {}
@@ -76,7 +75,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.lag_name_if_name_map, \
@@ -90,7 +88,7 @@ def reinit_data(self):
         self.mgmt_oid_name_map, \
         self.mgmt_alias_map = mibs.init_mgmt_interface_tables(self.db_conn[0])
 
-        self.if_range = sorted(list(self.oid_sai_map.keys()) +
+        self.if_range = sorted(list(self.oid_name_map.keys()) +
                                list(self.oid_lag_name_map.keys()) +
                                list(self.mgmt_oid_name_map.keys()))
         self.if_range = [(i,) for i in self.if_range]

src/sonic_ax_impl/mibs/ietf/rfc4363.py

@@ -15,7 +15,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
         self.vlanmac_ifindex_map = {}
         self.vlanmac_ifindex_list = []
@@ -40,7 +39,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.if_bpid_map = Namespace.dbs_get_bridge_port_map(self.db_conn, mibs.ASIC_DB)

src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py

@@ -17,7 +17,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
 
         self.lag_name_if_name_map = {}
@@ -27,6 +26,7 @@ def __init__(self):
         # cache of interface counters
         self.if_counters = {}
         self.if_range = []
+        self.namespace_db_map = Namespace.get_namespace_db_map(self.db_conn)
 
     def reinit_data(self):
         """
@@ -35,7 +35,6 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
         self.update_data()
@@ -45,15 +44,17 @@ def update_data(self):
         Update redis (caches config)
         Pulls the table references for each interface.
         """
-        self.if_counters = \
-            {sai_id: Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB, mibs.counter_table(sai_id), blocking=True)
-             for sai_id in self.if_id_map}
+        for sai_id_key in self.if_id_map:
+            namespace, sai_id = mibs.split_sai_id_key(sai_id_key)
+            if_idx = mibs.get_index_from_str(self.if_id_map[sai_id_key])
+            self.if_counters[if_idx] = self.namespace_db_map[namespace].get_all(mibs.COUNTERS_DB, \
+                    mibs.counter_table(sai_id), blocking=True)
 
         self.lag_name_if_name_map, \
         self.if_name_lag_name_map, \
         self.oid_lag_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_lag_tables, self.db_conn)
 
-        self.if_range = sorted(list(self.oid_sai_map.keys()) + list(self.oid_lag_name_map.keys()))
+        self.if_range = sorted(list(self.oid_name_map.keys()) + list(self.oid_lag_name_map.keys()))
         self.if_range = [(i,) for i in self.if_range]
 
     def get_next(self, sub_id):
@@ -88,13 +89,12 @@ def _get_counter(self, oid, counter_name):
         :param counter_name: the redis table (either IntEnum or string literal) to query.
         :return: the counter for the respective sub_id/table.
         """
-        sai_id = self.oid_sai_map[oid]
 
         # Enum.name or counter_name = 'name_of_the_table'
         _counter_name = bytes(getattr(counter_name, 'name', counter_name), 'utf-8')
 
         try:
-            counter_value = self.if_counters[sai_id][_counter_name]
+            counter_value = self.if_counters[oid][_counter_name] 
             counter_value = int(counter_value) & 0xffffffffffffffff
             # done!
             return counter_value

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py

@@ -54,7 +54,6 @@ def __init__(self):
         self.if_name_map = {}
         self.if_alias_map = {}
         self.if_id_map = {}
-        self.oid_sai_map = {}
         self.oid_name_map = {}
 
         self.port_queues_map = {}
@@ -65,6 +64,8 @@ def __init__(self):
         self.mib_oid_list = []
 
         self.queue_type_map = {}
+        self.port_index_namespace = {}
+        self.namespace_db_map = Namespace.get_namespace_db_map(self.db_conn)
 
     def reinit_data(self):
         """
@@ -73,14 +74,19 @@ def reinit_data(self):
         self.if_name_map, \
         self.if_alias_map, \
         self.if_id_map, \
-        self.oid_sai_map, \
         self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)
 
+        for sai_id_key in self.if_id_map:
+            namespace, sai_id = mibs.split_sai_id_key(sai_id_key)
+            if_idx = mibs.get_index_from_str(self.if_id_map[sai_id_key])
+            self.port_index_namespace[if_idx] = namespace
+
         self.port_queues_map, self.queue_stat_map, self.port_queue_list_map = \
             Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_queue_tables, self.db_conn)
 
-        self.queue_type_map = Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB, "COUNTERS_QUEUE_TYPE_MAP", blocking=False)
-
+        for db_conn in Namespace.get_non_host_dbs(self.db_conn):
+            self.queue_type_map[db_conn.namespace] = db_conn.get_all(mibs.COUNTERS_DB, "COUNTERS_QUEUE_TYPE_MAP", blocking=False)
+ 
         self.update_data()
 
     def update_data(self):
@@ -90,9 +96,15 @@ def update_data(self):
         """
         for queue_key, sai_id in self.port_queues_map.items():
             queue_stat_name = mibs.queue_table(sai_id)
-            queue_stat = Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB, queue_stat_name, blocking=False)
+            port_index, _ = queue_key.split(':')
+            queue_stat_idx = mibs.queue_key(port_index, queue_stat_name)
+            namespace = self.port_index_namespace[int(port_index)]
+            queue_stat = self.namespace_db_map[namespace].get_all( \
+                    mibs.COUNTERS_DB, queue_stat_name, blocking=False)
             if queue_stat is not None:
-                self.queue_stat_map[queue_stat_name] = queue_stat
+                self.queue_stat_map[queue_stat_idx] = queue_stat
+            else:
+                del self.queue_stat_map[queue_stat_idx]
 
         self.update_stats()
 
@@ -109,13 +121,14 @@ def update_stats(self):
         self.mib_oid_list = []
 
         # Sort the ports to keep the OID order in the MIB
-        if_range = list(self.oid_sai_map.keys())
+        if_range = list(self.oid_name_map.keys())
         # Update queue counters for port
         for if_index in if_range:
             if if_index not in self.port_queue_list_map:
                 # Port does not has a queues, continue..
                 continue
             if_queues = self.port_queue_list_map[if_index]
+            namespace = self.port_index_namespace[if_index]
 
             # The first half of queue id is for ucast, and second half is for mcast
             # To simulate vendor OID, we wrap queues by half distance
@@ -125,8 +138,9 @@ def update_stats(self):
                 # Get queue type and statistics
                 queue_sai_oid = self.port_queues_map[mibs.queue_key(if_index, queue)]
                 queue_stat_table_name = mibs.queue_table(queue_sai_oid)
-                queue_type = self.queue_type_map.get(queue_sai_oid)
-                queue_stat = self.queue_stat_map.get(queue_stat_table_name, {})
+                queue_stat_key = mibs.queue_key(if_index, queue_stat_table_name)
+                queue_type = self.queue_type_map[namespace].get(queue_sai_oid)
+                queue_stat = self.queue_stat_map.get(queue_stat_key, {})
 
                 # Add supported counters to MIBs list and store counters values
                 for (counter, counter_type), counter_mib_id in CounterMap.items():

tests/mock_tables/asic1/asic_db.json

@@ -1,11 +1,11 @@
 {
   "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bridge_id\":\"oid:0x0\",\"bridge_type\":\"SAI_FDB_ENTRY_BRIDGE_TYPE_1Q\",\"mac\":\"7C:FE:90:80:9F:10\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
-    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a0000000006208",
+    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000616",
     "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
   },
-  "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000620": {
+  "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000616": {
     "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
-    "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000010",
+    "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000005",
     "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
   }
 }