Fix undefined variable and warning message #134

SuvarnaMeenakshi · 2020-06-16T17:58:18Z

- What I did

Fix undefined variable introduced in [Multi-asic]: Namespace support for LLDP and Sensor tables #131
Add fdb unit-test for namespace changes
Fix update_data() LocPortUpdater, LLDPRemManAddrUpdater and PhysicalTableMIBUpdater caused due to Namespace changes. The pubsub channel should not be created everytime on update-data and should be created only if it is not previously created. This causes a warning message and does not get any new changes in the subscribed key pattern.

sonic WARNING snmp#snmp-subagent [sonic_ax_impl] WARNING: Invalid interface name in TRANSCEIVER_INFO|*                      in STATE_DB, skipping
snmp#snmp-subagent [sonic_ax_impl] WARNING: Invalid interface name in *' in APP_DB, skipping

- How I did it

Added fdb unit-test for namespace changes
Made sure that the pubsub channel for each Namespace DB is created only once.

- How to verify it

Tested out on single asic and multiple asic platforms.
Make sure that warning messages are not seen in syslog.

- Description for the changelog

Add namespace test case for fdb related MIB. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

functions. Multiple connect is causing an increase in run time causing agentx connection to close. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

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

qiluo-msft · 2020-06-18T01:34:34Z

src/sonic_ax_impl/mibs/ieee802_1ab.py

@@ -536,18 +544,18 @@ def update_rem_if_mgmt(self, if_oid, if_name):
 return
 self.if_range.sort()

- def _update_per_namespace_data(self, db_conn, pubsub):


_update_per_namespace_data [](start = 8, length = 26)

Original code seems better. Why change? #Closed

Before namespace changes, self.pubsub was getting updated in update_data(). After namespace change, self.pubsub array was not updated as we pass the instance. By passing the array index, we can ensure that the array is directly getting udpated. So this fix is to make sure the code is as it was before namespace changes.

Updated description with the issue and why this fix is required.

I see the bug in #131.

To make code cleaner, could you try add

db_conn = self.db_conn[inst] pubsub = self.pubsub[inst] if not pubsub: ... pubsub = redis_client.pubsub() self.pubsub[inst] = pubsub

In reply to: 441919551 [](ancestors = 441919551)

Fixed as per comments.

qiluo-msft · 2020-06-18T01:38:24Z

src/sonic_ax_impl/mibs/ietf/rfc2737.py

+ redis_client = self.statedb[inst].get_redis_client(self.statedb[inst].STATE_DB)
+ db = self.statedb[inst].get_dbid(self.statedb[inst].STATE_DB)
+ self.pubsub[inst] = redis_client.pubsub()
+ self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, self.TRANSCEIVER_KEY_PATTERN))


Duplicated code. Could you reuse? #Closed

Duplicated with ieee802_1ab.py

In reply to: 441920582 [](ancestors = 441920582)

Fixed as per review comments.

qiluo-msft · 2020-06-18T01:45:56Z

src/sonic_ax_impl/mibs/ieee802_1ab.py

@@ -104,6 +104,7 @@ def __init__(self):
 super().__init__()

 self.db_conn = Namespace.init_namespace_dbs()
+ Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB)


Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) [](start = 8, length = 53)

Original code could tolerate temporary or initial Redis not offline and recover whenever online. But the new code could not. Can you improve and test?
#Closed

I do see that if redis-server process is stopped and started, the MIB walk does not get affected, as the connect is persistent_connect. But, I have reverted this change, I will raise a new PR to provide a clean fix to reduce runtime.

qiluo-msft · 2020-06-18T01:48:02Z

tests/mock_tables/asic0/asic_db.json

+ "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
+ "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000005",
+ "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
+ }
 }


If not related to the PR subject, could you submit another PR just for test cases? #Closed

This test case is to test the code path where the undefined variable is fixed in this PR.

qiluo-msft

As comments

This reverts commit 1dced6e.

not created everytime in update_data but is created only if the channel was not created previously. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

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

* Fix errors introduced. Add namespace test case for fdb related MIB. * Make sure that db connections are done in MIB class init functions. Multiple connect is causing an increase in run time causing agentx connection to close. * Minor fix * Add mock asic_db. * Remove trailing whitespace. * Revert "Make sure that db connections are done in MIB class init" * Fix to make sure pubsub channel in LLDPRemManAddrUpdater is not created everytime in update_data but is created only if the channel was not created previously. * Fix as per review comments.

SuvarnaMeenakshi added 2 commits June 15, 2020 21:07

Fix errors introduced.

5eab69a

Add namespace test case for fdb related MIB. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Make sure that db connections are done in MIB class init

1dced6e

functions. Multiple connect is causing an increase in run time causing agentx connection to close. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi requested review from qiluo-msft and abdosi June 16, 2020 18:03

SuvarnaMeenakshi added 2 commits June 16, 2020 11:09

Minor fix

ffe4d38

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

Add mock asic_db.

20dd2e2

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

SuvarnaMeenakshi added the Request for 201911 Branch label Jun 16, 2020

Remove trailing whitespace.

50dd045

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

qiluo-msft reviewed Jun 18, 2020

View reviewed changes

qiluo-msft requested changes Jun 18, 2020

View reviewed changes

SuvarnaMeenakshi added 2 commits June 24, 2020 11:31

Revert "Make sure that db connections are done in MIB class init"

1eb43df

This reverts commit 1dced6e.

Fix to make sure pubsub channel in LLDPRemManAddrUpdater is

3442ff9

not created everytime in update_data but is created only if the channel was not created previously. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi added the Bug label Jun 24, 2020

SuvarnaMeenakshi changed the title ~~Fix undefined variable and reduce runtime~~ Fix undefined variable and warning message Jun 24, 2020

Fix as per review comments.

ea91aca

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

qiluo-msft approved these changes Jun 25, 2020

View reviewed changes

qiluo-msft merged commit eba1408 into sonic-net:master Jun 25, 2020

abdosi added the Included in 201911 Branch label Jul 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix undefined variable and warning message #134

Fix undefined variable and warning message #134

SuvarnaMeenakshi commented Jun 16, 2020 •

edited

Loading

qiluo-msft Jun 18, 2020 •

edited

Loading

SuvarnaMeenakshi Jun 23, 2020

SuvarnaMeenakshi Jun 24, 2020

qiluo-msft Jun 25, 2020 •

edited

Loading

SuvarnaMeenakshi Jun 25, 2020

qiluo-msft Jun 18, 2020 •

edited

Loading

qiluo-msft Jun 25, 2020

SuvarnaMeenakshi Jun 25, 2020

qiluo-msft Jun 18, 2020 •

edited

Loading

SuvarnaMeenakshi Jun 24, 2020

qiluo-msft Jun 18, 2020 •

edited

Loading

SuvarnaMeenakshi Jun 23, 2020

qiluo-msft left a comment

Fix undefined variable and warning message #134

Fix undefined variable and warning message #134

Conversation

SuvarnaMeenakshi commented Jun 16, 2020 • edited Loading

qiluo-msft Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 23, 2020

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 24, 2020

Choose a reason for hiding this comment

qiluo-msft Jun 25, 2020 • edited Loading

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 25, 2020

Choose a reason for hiding this comment

qiluo-msft Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Jun 25, 2020

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 25, 2020

Choose a reason for hiding this comment

qiluo-msft Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 24, 2020

Choose a reason for hiding this comment

qiluo-msft Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

SuvarnaMeenakshi Jun 23, 2020

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

SuvarnaMeenakshi commented Jun 16, 2020 •

edited

Loading

qiluo-msft Jun 18, 2020 •

edited

Loading

qiluo-msft Jun 25, 2020 •

edited

Loading

qiluo-msft Jun 18, 2020 •

edited

Loading

qiluo-msft Jun 18, 2020 •

edited

Loading

qiluo-msft Jun 18, 2020 •

edited

Loading