Skip to content

Commit

Permalink
Fix undefined variable and warning message (#134)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
SuvarnaMeenakshi authored and abdosi committed Jul 8, 2020
1 parent 323d6b0 commit c70df53
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 35 deletions.
15 changes: 11 additions & 4 deletions src/sonic_ax_impl/mibs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def init_db():
"""
# SyncD database connector. THIS MUST BE INITIALIZED ON A PER-THREAD BASIS.
# Redis PubSub objects (such as those within swsssdk) are NOT thread-safe.
db_conn = SonicV2Connector(**redis_kwargs)
db_conn = SonicV2Connector(**redis_kwargs)

return db_conn

Expand Down Expand Up @@ -389,6 +389,13 @@ def get_transceiver_sensor_sub_id(ifindex, sensor):
transceiver_oid, = get_transceiver_sub_id(ifindex)
return (transceiver_oid + SENSOR_PART_ID_MAP[sensor], )

def get_redis_pubsub(db_conn, db_name, pattern):
redis_client = db_conn.get_redis_client(db_name)
db = db_conn.get_dbid(db_name)
pubsub = redis_client.pubsub()
pubsub.psubscribe("__keyspace@{}__:{}".format(db, pattern))
return pubsub

class RedisOidTreeUpdater(MIBUpdater):
def __init__(self, prefix_str):
super().__init__()
Expand Down Expand Up @@ -419,7 +426,7 @@ def update_data(self):
self.oid_list = []
self.oid_map = {}

keys = Namespace.dbs_keys(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*')
keys = Namespace.dbs_keys(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*')
# TODO: fix db_conn.keys to return empty list instead of None if there is no match
if keys is None:
keys = []
Expand All @@ -428,7 +435,7 @@ def update_data(self):
key = key.decode()
oid = oid2tuple(key, dot_prefix=False)
self.oid_list.append(oid)
value = Namespace.dbs_get_all(self.db_conn, SNMP_OVERLAY_DB, key)
value = Namespace.dbs_get_all(self.db_conn, SNMP_OVERLAY_DB, key)
if value[b'type'] in [b'COUNTER_32', b'COUNTER_64']:
self.oid_map[oid] = int(value[b'data'])
else:
Expand Down Expand Up @@ -587,6 +594,6 @@ def dbs_get_bridge_port_map(dbs, db_name):
def dbs_get_vlan_id_from_bvid(dbs, bvid):
for db_conn in Namespace.get_non_host_dbs(dbs):
db_conn.connect('ASIC_DB')
vlan_obj = db.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
vlan_obj = db_conn.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
if vlan_obj is not None:
return port_util.get_vlan_id_from_bvid(db_conn, bvid)
33 changes: 13 additions & 20 deletions src/sonic_ax_impl/mibs/ieee802_1ab.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class ManAddrConst:
"""
man_addr_oid = (1, 3, 6, 1, 2, 1, 2, 2, 1, 1)


def poll_lldp_entry_updates(pubsub):
ret = None, None, None
msg = pubsub.get_message()
Expand Down Expand Up @@ -157,7 +156,7 @@ def __init__(self):
# cache of port data
# { if_name -> { 'key': 'value' } }
self.loc_port_data = {}
self.pubsub = [None] * len(self.db_conn)
self.pubsub = [None] * len(self.db_conn)

def reinit_data(self):
"""
Expand Down Expand Up @@ -222,16 +221,10 @@ def get_next(self, sub_id):
return None
return self.if_range[right]

def _update_per_namespace_data(self, db_conn, pubsub):
def _update_per_namespace_data(self, pubsub):
"""
Listen to updates in APP DB, update local cache
"""
if not pubsub:
redis_client = db_conn.get_redis_client(db_conn.APPL_DB)
db = db_conn.get_dbid(db_conn.APPL_DB)
pubsub = redis_client.pubsub()
pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*')))

while True:
data, interface, if_id = poll_lldp_entry_updates(pubsub)

Expand All @@ -243,7 +236,10 @@ def _update_per_namespace_data(self, db_conn, pubsub):

def update_data(self):
for i in range(len(self.db_conn)):
self._update_per_namespace_data(self.db_conn[i], self.pubsub[i])
if not self.pubsub[i]:
pattern = mibs.lldp_entry_table(b'*')
self.pubsub[i] = mibs.get_redis_pubsub(self.db_conn[i], self.db_conn[i].APPL_DB, pattern)
self._update_per_namespace_data(self.pubsub[i])

def local_port_num(self, sub_id):
if len(sub_id) == 0:
Expand Down Expand Up @@ -307,7 +303,7 @@ def reinit_data(self):
self.mgmt_ip_str = None

# establish connection to application database.
self.db_conn.connect(mibs.APPL_DB)
self.db_conn.connect(mibs.APPL_DB)
mgmt_ip_bytes = self.db_conn.get(mibs.APPL_DB, mibs.LOC_CHASSIS_TABLE, b'lldp_loc_man_addr')

if not mgmt_ip_bytes:
Expand Down Expand Up @@ -488,7 +484,7 @@ class LLDPRemManAddrUpdater(MIBUpdater):
def __init__(self):
super().__init__()

self.db_conn = Namespace.init_namespace_dbs()
self.db_conn = Namespace.init_namespace_dbs()
# establish connection to application database.
Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB)
self.if_range = []
Expand Down Expand Up @@ -536,16 +532,10 @@ def update_rem_if_mgmt(self, if_oid, if_name):
return
self.if_range.sort()

def _update_per_namespace_data(self, db_conn, pubsub):
def _update_per_namespace_data(self, pubsub):
"""
Listen to updates in APP DB, update local cache
"""
if not pubsub:
redis_client = db_conn.get_redis_client(db_conn.APPL_DB)
db = db_conn.get_dbid(db_conn.APPL_DB)
pubsub = redis_client.pubsub()
pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*')))

while True:
data, interface, if_index = poll_lldp_entry_updates(pubsub)

Expand All @@ -561,7 +551,10 @@ def _update_per_namespace_data(self, db_conn, pubsub):

def update_data(self):
for i in range(len(self.db_conn)):
self._update_per_namespace_data(self.db_conn[i], self.pubsub[i])
if not self.pubsub[i]:
pattern = mibs.lldp_entry_table(b'*')
self.pubsub[i] = mibs.get_redis_pubsub(self.db_conn[i], self.db_conn[i].APPL_DB, pattern)
self._update_per_namespace_data(self.pubsub[i])


def reinit_data(self):
Expand Down
15 changes: 7 additions & 8 deletions src/sonic_ax_impl/mibs/ietf/rfc2737.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def reinit_data(self):
interface = transceiver_entry.split(mibs.TABLE_NAME_SEPARATOR_VBAR)[-1]
self._update_transceiver_cache(interface)

def _update_per_namespace_data(self, statedb, pubsub):
def _update_per_namespace_data(self, pubsub):
"""
Update cache.
Here we listen to changes in STATE_DB TRANSCEIVER_INFO table
Expand All @@ -190,12 +190,6 @@ def _update_per_namespace_data(self, statedb, pubsub):

# This code is not executed in unit test, since mockredis
# does not support pubsub
if not pubsub:
redis_client = statedb.get_redis_client(statedb.STATE_DB)
db = statedb.get_dbid(statedb.STATE_DB)
pubsub = redis_client.pubsub()
pubsub.psubscribe("__keyspace@{}__:{}".format(db, self.TRANSCEIVER_KEY_PATTERN))

while True:
msg = pubsub.get_message()

Expand Down Expand Up @@ -233,8 +227,13 @@ def _update_per_namespace_data(self, statedb, pubsub):
self.physical_entities.remove(sub_id)

def update_data(self):
# This code is not executed in unit test, since mockredis
# does not support pubsub
for i in range(len(self.statedb)):
self._update_per_namespace_data(self.statedb[i], self.pubsub[i])
if not self.pubsub[i]:
pattern = self.TRANSCEIVER_KEY_PATTERN
self.pubsub[i] = mibs.get_redis_pubsub(self.statedb[i], self.statedb[i].STATE_DB, pattern)
self._update_per_namespace_data(self.pubsub[i])

def _update_transceiver_cache(self, interface):
"""
Expand Down
4 changes: 2 additions & 2 deletions src/sonic_ax_impl/mibs/ietf/rfc3433.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def __init__(self):

super().__init__()

self.statedb = Namespace.init_namespace_dbs()
self.statedb = Namespace.init_namespace_dbs()
Namespace.connect_all_dbs(self.statedb, mibs.STATE_DB)

# list of available sub OIDs
Expand Down Expand Up @@ -378,7 +378,7 @@ def get_ent_physical_sensor_precision(self, sub_id):
"""
Get sensor precision value based on sub OID
:param sub_id: sub ID of the sensor
:return: sensor precision in range (-8, 9)
:return: sensor precision in range (-8, 9)
or None if sub_id not in the cache.
"""

Expand Down
2 changes: 1 addition & 1 deletion src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def requests_per_priority(self, sub_id):
"""
:param sub_id: The 0-based sub-identifier query.
:return: the counter for the respective sub_id/table.
"""
"""
port_oid = ''
queue_index = ''
try:
Expand Down
25 changes: 25 additions & 0 deletions tests/mock_tables/asic0/asic_db.json
Original file line number Diff line number Diff line change
@@ -1,2 +1,27 @@
{
"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:04\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000608",
"SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
},
"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:06\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000616",
"SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
},
"ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\"oid:0x26000000000016\",\"mac\":\"7C:FE:90:80:9F:08\",\"switch_id\":\"oid:0x21000000000000\"}": {
"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000620",
"SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
},
"ASIC_STATE:SAI_OBJECT_TYPE_VLAN:oid:0x26000000000016": {
"SAI_VLAN_ATTR_VLAN_ID": "1000"
},
"ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000608": {
"SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
"SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000003",
"SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
},
"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:0x1000000000005",
"SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
}
}
9 changes: 9 additions & 0 deletions tests/mock_tables/asic1/asic_db.json
Original file line number Diff line number Diff line change
@@ -1,2 +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_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
},
"ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000620": {
"SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
"SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000010",
"SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
}
}
2 changes: 2 additions & 0 deletions tests/mock_tables/global_db/asic_db.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Loading

0 comments on commit c70df53

Please sign in to comment.