Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Namespace]: Fix SAI_ID key used in cpfcIfTable and csqIfQosGroupStatsTable implementation #138

Merged
merged 11 commits into from
Aug 13, 2020
15 changes: 8 additions & 7 deletions src/sonic_ax_impl/mibs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,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 = {}
Expand All @@ -356,7 +356,8 @@ def init_sync_d_queue_tables(db_conn):
queue_stat_name = queue_table(sai_id)
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queue_stat_name [](start = 8, length = 15)

SAI Object ID is unique in this db_conn. So the original implementation of init_sync_d_queue_tables makes sense.

Let's block this PR and continue exploration the dbs dict strategy, as also discussed in another PR (#137) #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the data will eventually merged together , we have to make sure that the keys are unique in these dictionaries.
So in maps which use SAI ID as key, modified it to use a combination of if_idx and 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)]
Expand Down Expand Up @@ -577,7 +578,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
Expand Down
12 changes: 7 additions & 5 deletions src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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):
"""
Expand All @@ -45,9 +46,11 @@ 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, \
Expand Down Expand Up @@ -88,13 +91,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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is oid_sai_map is being used or this can be removed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed oid_sai_map as it is not required. The required information can be retrieved from oid_name_map.


# 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
Expand Down
26 changes: 20 additions & 6 deletions src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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):
"""
Expand All @@ -76,11 +78,17 @@ def reinit_data(self):
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):
Expand All @@ -90,9 +98,13 @@ 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:
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if queue_stat is not None [](start = 12, length = 25)

Otherwise, del the index from self.queue_stat_map #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if queue_stat is not None [](start = 12, length = 25)

Otherwise, del the index from self.queue_stat_map

Added as per comment.

self.queue_stat_map[queue_stat_name] = queue_stat
self.queue_stat_map[queue_stat_idx] = queue_stat

self.update_stats()

Expand All @@ -116,6 +128,7 @@ def update_stats(self):
# 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
Expand All @@ -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():
Expand Down
6 changes: 3 additions & 3 deletions tests/mock_tables/asic1/asic_db.json
Original file line number Diff line number Diff line change
@@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what trigger this changes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified namespace mock dbs to reflect the idea that SAI ID will be repeated across namespaces.

"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"
}
}
Loading