[Namespace]: Fix to reduce time and fix counters related MIB #139

SuvarnaMeenakshi · 2020-07-09T09:10:23Z

- What I did

Fix add to reduce time in Namespace get_all, which is taking more time due to additional check to see if the key exists in a database.
In multi-asic platform, sai id is unique only within namespace, so avoid using sai id as key.

- How I did it

Remove key exists check in dbs_get_all
Make changes to MIB implementation using SAI ID as key, to use interface index as key instead:
InterfacesMIB
InterfaceMIBObjects
cpfcIfTable
csqIfQosGroupStatsTable
Make changes to rif/vlan related counters to use interface index as key instead of SAI ID.

- How to verify it

- Description for the changelog

reduce the overall time taken. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

unique across the device. The SAI object ID is unique for asic or within a namespace. Modify the existing code to make sure that data structures using SAI OID, it is not used as a key directly. Modify implementation for MIB tables using COUNTER_DB and ASIC_DB so that SAI OID is used to retrieve data only from a single namespace. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

lgtm-com · 2020-07-09T09:17:25Z

This pull request introduces 3 alerts when merging 389d92a into 253f58e - view on LGTM.com

new alerts:

3 for Unused local variable

use sai oid as key. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

abdosi · 2020-07-09T19:22:09Z

src/sonic_ax_impl/mibs/ietf/rfc4363.py


-        self.if_bpid_map = Namespace.dbs_get_bridge_port_map(self.db_conn, mibs.ASIC_DB)
+        self.if_bpid_map = Namespace.dbs_get_bridge_port_map(self.db_conn)


why we don't need second Argument Here ?

dbs_get_bridge_port_map() does not use the db name, the function only retrieves data from ASIC_DB, so did not have to use an argument to specify the db name.

abdosi · 2020-07-09T19:24:10Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+                    if_idx = self.sai_oid_map[db_index][port_sai_id]
+                else:
+                    continue
+                rif_counters = self.db_conn[db_index].get_all(mibs.COUNTERS_DB, mibs.counter_table(rif_sai_id), blocking=True)


why blocking is True in this case ?

The previous code did a get_all with blocking=true, so kept the same. should not cause an issue because we get based on rif_sai_id from the specific db instance.

but it will add to time taken ? same as original problem

abdosi · 2020-07-09T19:25:49Z

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",


what trigger all these below changes ?

The changes in mock tables are done so that the SAI ID for port and other objects are same in different namespace. Earlier, in the mock tables, it was unique across the device. To reflect the actual scenario where the same SAI Id can appear in a different namespace, the changes are made in asic_db and counter_db.

abdosi · 2020-07-09T19:27:27Z

src/sonic_ax_impl/mibs/__init__.py

@@ -373,7 +376,7 @@ def init_sync_d_queue_tables(db_conn):

    # 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}}
+    # queue_stat_map -> {"if_index : queue stat table name" : {counter name : value}} 


Please add some comment as why these changes are needed (OID overlap) in Multi-Asic platforms

Updated as per comment.

abdosi · 2020-07-09T19:31:09Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+        for oid in self.vlan_oid_sai_map:
+            if oid in self.vlan_oid_namespace:
+                db_index = self.vlan_oid_namespace[oid]
+                self.rif_counters[oid] = self.db_conn[db_index].get_all(mibs.COUNTERS_DB, mibs.counter_table(self.vlan_oid_sai_map[oid]), blocking=True)


the if condition will it not add to time taken ?

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

qiluo-msft · 2020-07-09T21:46:48Z

Remove extra file foo #Closed

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi · 2020-07-20T07:59:09Z

Closing this PR as changes in this PR are not required.
Time increase and socket connection error is fixed using: #140
SAI ID fixes will be done in: #138

SuvarnaMeenakshi added 4 commits July 8, 2020 20:06

[Namespace]: Remove key exists check in dbs_get_all to

7480cf6

reduce the overall time taken. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Fix counters related MIB implementation.

cd71da6

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Make changes to use if_index as key instead of SAI OID.

389d92a

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi requested review from qiluo-msft and abdosi July 9, 2020 09:10

SuvarnaMeenakshi added 2 commits July 9, 2020 11:42

[Namespace]: Fix RIF counters implementation so that it does not

8473740

use sai oid as key. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Minor fix to remove debug code.

ce1fa08

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

abdosi reviewed Jul 9, 2020

View reviewed changes

Minor fix.

6c2d95d

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Remove bad file.

d6e8708

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi closed this Jul 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Namespace]: Fix to reduce time and fix counters related MIB #139

[Namespace]: Fix to reduce time and fix counters related MIB #139

SuvarnaMeenakshi commented Jul 9, 2020

lgtm-com bot commented Jul 9, 2020

abdosi Jul 9, 2020

SuvarnaMeenakshi Jul 9, 2020

abdosi Jul 9, 2020

SuvarnaMeenakshi Jul 9, 2020

abdosi Jul 9, 2020

abdosi Jul 9, 2020

SuvarnaMeenakshi Jul 9, 2020

abdosi Jul 9, 2020

SuvarnaMeenakshi Jul 9, 2020

abdosi Jul 9, 2020

qiluo-msft commented Jul 9, 2020 •

edited

Loading

SuvarnaMeenakshi commented Jul 20, 2020


		self.if_bpid_map = Namespace.dbs_get_bridge_port_map(self.db_conn, mibs.ASIC_DB)
		self.if_bpid_map = Namespace.dbs_get_bridge_port_map(self.db_conn)

[Namespace]: Fix to reduce time and fix counters related MIB #139

[Namespace]: Fix to reduce time and fix counters related MIB #139

Conversation

SuvarnaMeenakshi commented Jul 9, 2020

lgtm-com bot commented Jul 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft commented Jul 9, 2020 • edited Loading

SuvarnaMeenakshi commented Jul 20, 2020

qiluo-msft commented Jul 9, 2020 •

edited

Loading