From 5eab69a4aae3a90a55ae88c51958e57920931b33 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Mon, 15 Jun 2020 21:07:58 -0700 Subject: [PATCH 1/8] Fix errors introduced. Add namespace test case for fdb related MIB. Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/__init__.py | 2 +- src/sonic_ax_impl/mibs/ieee802_1ab.py | 14 +-- src/sonic_ax_impl/mibs/ietf/rfc2737.py | 16 +-- tests/mock_tables/asic0/asic_db.json | 25 ++++ tests/mock_tables/asic1/asic_db.json | 9 ++ tests/namespace/test_fdb.py | 156 +++++++++++++++++++++++++ 6 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 tests/namespace/test_fdb.py diff --git a/src/sonic_ax_impl/mibs/__init__.py b/src/sonic_ax_impl/mibs/__init__.py index c9154c7f7..6e9839335 100644 --- a/src/sonic_ax_impl/mibs/__init__.py +++ b/src/sonic_ax_impl/mibs/__init__.py @@ -587,6 +587,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) diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index fa4d2b7e2..4ddd93e4b 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -222,18 +222,18 @@ 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, inst): """ 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() + if not self.pubsub[inst]: + redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) + db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) + self.pubsub[inst] = redis_client.pubsub() pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) while True: - data, interface, if_id = poll_lldp_entry_updates(pubsub) + data, interface, if_id = poll_lldp_entry_updates(self.pubsub[inst]) if not data: break @@ -243,7 +243,7 @@ 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]) + self._update_per_namespace_data(i) def local_port_num(self, sub_id): if len(sub_id) == 0: diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2737.py b/src/sonic_ax_impl/mibs/ietf/rfc2737.py index a0134cf99..4f5a7eb74 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2737.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2737.py @@ -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, inst): """ Update cache. Here we listen to changes in STATE_DB TRANSCEIVER_INFO table @@ -190,14 +190,14 @@ 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)) + if not self.pubsub[inst]: + 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)) while True: - msg = pubsub.get_message() + msg = self.pubsub[inst].get_message() if not msg: break @@ -234,7 +234,7 @@ def _update_per_namespace_data(self, statedb, pubsub): def update_data(self): for i in range(len(self.statedb)): - self._update_per_namespace_data(self.statedb[i], self.pubsub[i]) + self._update_per_namespace_data(i) def _update_transceiver_cache(self, interface): """ diff --git a/tests/mock_tables/asic0/asic_db.json b/tests/mock_tables/asic0/asic_db.json index 2c63c0851..590918051 100644 --- a/tests/mock_tables/asic0/asic_db.json +++ b/tests/mock_tables/asic0/asic_db.json @@ -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" + } } diff --git a/tests/mock_tables/asic1/asic_db.json b/tests/mock_tables/asic1/asic_db.json index 2c63c0851..70dedc520 100644 --- a/tests/mock_tables/asic1/asic_db.json +++ b/tests/mock_tables/asic1/asic_db.json @@ -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" + } } diff --git a/tests/namespace/test_fdb.py b/tests/namespace/test_fdb.py new file mode 100644 index 000000000..ee95bd5c4 --- /dev/null +++ b/tests/namespace/test_fdb.py @@ -0,0 +1,156 @@ +import os +import sys +import importlib + +modules_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.join(modules_path, 'src')) + +from unittest import TestCase + +# noinspection PyUnresolvedReferences +import tests.mock_tables.dbconnector + +from ax_interface.mib import MIBTable +from ax_interface.pdu import PDUHeader +from ax_interface.pdu_implementations import GetPDU, GetNextPDU +from ax_interface import ValueType +from ax_interface.encodings import ObjectIdentifier +from ax_interface.constants import PduTypes +from sonic_ax_impl.mibs.ietf import rfc4363 +from sonic_ax_impl.main import SonicMIB +import sonic_ax_impl + +class TestSonicMIB(TestCase): + @classmethod + def setUpClass(cls): + tests.mock_tables.dbconnector.load_namespace_config() + importlib.reload(rfc4363) + + cls.lut = MIBTable(rfc4363.QBridgeMIBObjects) + for updater in cls.lut.updater_instances: + updater.update_data() + updater.reinit_data() + updater.update_data() + + def test_getpdu(self): + oid = ObjectIdentifier(20, 0, 0, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 124, 254, 144, 128, 159, 4)) + get_pdu = GetPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(value0.data, 1) + + def test_getnextpdu(self): + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=( + ObjectIdentifier(20, 0, 0, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2)), + ) + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + self.assertEqual(value0.data, 1) + + def test_getnextpdu_exactmatch(self): + # oid.include = 1 + oid = ObjectIdentifier(20, 0, 1, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 124, 254, 144, 128, 159, 4)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + print("test_getnextpdu_exactmatch: ", str(oid)) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(value0.data, 1) + + def test_getnextpdu_exactmatch_asic0(self): + # oid.include = 1 + oid = ObjectIdentifier(20, 0, 1, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 124, 254, 144, 128, 159, 6)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + print("test_getnextpdu_exactmatch: ", str(oid)) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(value0.data, 9000) + + def test_getnextpdu_exactmatch_asic1(self): + # oid.include = 1 + oid = ObjectIdentifier(20, 0, 1, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 124, 254, 144, 128, 159, 8)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + print("test_getnextpdu_exactmatch: ", str(oid)) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(value0.data, 9008) + + def test_getpdu_noinstance(self): + get_pdu = GetPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=( + ObjectIdentifier(20, 0, 0, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 100001)), + ) + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.NO_SUCH_INSTANCE) + + def test_getnextpdu_empty(self): + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=( + ObjectIdentifier(20, 0, 0, 0, (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 3)), + ) + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.END_OF_MIB_VIEW) + From 1dced6e497b95a8bde631346a682961ef8a9b862 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Tue, 16 Jun 2020 10:51:16 -0700 Subject: [PATCH 2/8] 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. Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/__init__.py | 22 ++--------------- src/sonic_ax_impl/mibs/ieee802_1ab.py | 24 ++++++++++++------- src/sonic_ax_impl/mibs/ietf/rfc1213.py | 7 +++++- src/sonic_ax_impl/mibs/ietf/rfc2737.py | 4 +++- src/sonic_ax_impl/mibs/ietf/rfc2863.py | 3 +++ src/sonic_ax_impl/mibs/ietf/rfc4292.py | 1 + src/sonic_ax_impl/mibs/ietf/rfc4363.py | 3 +++ .../mibs/vendor/cisco/ciscoPfcExtMIB.py | 2 ++ .../mibs/vendor/cisco/ciscoSwitchQosMIB.py | 2 ++ tests/namespace/test_mibs.py | 2 ++ tests/test_mibs.py | 2 ++ 11 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/sonic_ax_impl/mibs/__init__.py b/src/sonic_ax_impl/mibs/__init__.py index 6e9839335..788f54079 100644 --- a/src/sonic_ax_impl/mibs/__init__.py +++ b/src/sonic_ax_impl/mibs/__init__.py @@ -160,10 +160,6 @@ def init_mgmt_interface_tables(db_conn): :param db_conn: db connector :return: tuple of mgmt name to oid map and mgmt name to alias map """ - - db_conn.connect(CONFIG_DB) - db_conn.connect(STATE_DB) - mgmt_ports_keys = db_conn.keys(CONFIG_DB, mgmt_if_entry_table(b'*')) if not mgmt_ports_keys: @@ -190,9 +186,6 @@ def init_sync_d_interface_tables(db_conn): :return: tuple(if_name_map, if_id_map, oid_map, if_alias_map) """ - # Make sure we're connected to COUNTERS_DB - db_conn.connect(COUNTERS_DB) - # { if_name (SONiC) -> sai_id } # ex: { "Ethernet76" : "1000000000023" } if_name_map, if_id_map = port_util.get_interface_oid_map(db_conn) @@ -232,8 +225,6 @@ def init_sync_d_interface_tables(db_conn): .format(port_util.SONIC_ETHERNET_RE_PATTERN)) logger.warning("Port name map:\n" + pprint.pformat(if_name_map, indent=2)) - db_conn.connect(APPL_DB) - if_alias_map = dict() for if_name in if_name_map: @@ -259,8 +250,6 @@ def init_sync_d_lag_tables(db_conn): # { OID -> lag_name (SONiC) } oid_lag_name_map = {} - db_conn.connect(APPL_DB) - lag_entries = db_conn.keys(APPL_DB, b"LAG_TABLE:*") if not lag_entries: @@ -293,10 +282,6 @@ def init_sync_d_queue_tables(db_conn): Initializes queue maps for SyncD-connected MIB(s). :return: tuple(port_queues_map, queue_stat_map) """ - - # Make sure we're connected to COUNTERS_DB - db_conn.connect(COUNTERS_DB) - # { Port index : Queue index (SONiC) -> sai_id } # ex: { "1:2" : "1000000000023" } queue_name_map = db_conn.get_all(COUNTERS_DB, COUNTERS_QUEUE_NAME_MAP, blocking=True) @@ -349,7 +334,6 @@ def get_device_metadata(db_conn): """ DEVICE_METADATA = "DEVICE_METADATA|localhost" - db_conn.connect(db_conn.STATE_DB) device_metadata = db_conn.get_all(db_conn.STATE_DB, DEVICE_METADATA) return device_metadata @@ -393,7 +377,8 @@ class RedisOidTreeUpdater(MIBUpdater): def __init__(self, prefix_str): super().__init__() - self.db_conn = Namespace.init_namespace_dbs() + self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, SNMP_OVERLAY_DB) if prefix_str.startswith('.'): prefix_str = prefix_str[1:] self.prefix_str = prefix_str @@ -464,7 +449,6 @@ def dbs_keys(dbs, db_name, pattern='*'): """ result_keys=[] for db_conn in dbs: - db_conn.connect(db_name) keys = db_conn.keys(db_name, pattern) if keys is not None: result_keys.extend(keys) @@ -477,7 +461,6 @@ def dbs_get_all(dbs, db_name, _hash, *args, **kwargs): """ result = {} for db_conn in dbs: - db_conn.connect(db_name) if(db_conn.exists(db_name, _hash)): ns_result = db_conn.get_all(db_name, _hash, *args, **kwargs) if ns_result is not None: @@ -586,7 +569,6 @@ def dbs_get_bridge_port_map(dbs, db_name): @staticmethod 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_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) diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index 4ddd93e4b..dd92fc717 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/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) self.loc_chassis_data = {} def reinit_data(self): @@ -143,6 +144,8 @@ def __init__(self): self.db_conn = Namespace.init_namespace_dbs() # establish connection to application database. Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_name_map = {} self.if_alias_map = {} self.if_id_map = {} @@ -383,6 +386,9 @@ def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_name_map = {} self.if_alias_map = {} self.if_id_map = {} @@ -491,6 +497,8 @@ def __init__(self): self.db_conn = Namespace.init_namespace_dbs() # establish connection to application database. Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_range = [] self.mgmt_ips = {} self.oid_name_map = {} @@ -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): + def _update_per_namespace_data(self, inst): """ 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'*'))) + if not self.pubsub[inst]: + redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) + db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) + self.pubsub[inst] = redis_client.pubsub() + self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) while True: - data, interface, if_index = poll_lldp_entry_updates(pubsub) + data, interface, if_index = poll_lldp_entry_updates(self.pubsub[inst]) if not data: break @@ -561,7 +569,7 @@ 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]) + self._update_per_namespace_data(i) def reinit_data(self): diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index c7dc2848f..c2c3fe8ec 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -95,6 +95,7 @@ class NextHopUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.nexthop_map = {} self.route_list = [] @@ -152,7 +153,11 @@ class InterfacesUpdater(MIBUpdater): def __init__(self): super().__init__() - self.db_conn = Namespace.init_namespace_dbs() + self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.STATE_DB) self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2737.py b/src/sonic_ax_impl/mibs/ietf/rfc2737.py index 4f5a7eb74..79dc9425c 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2737.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2737.py @@ -122,6 +122,8 @@ def __init__(self): self.statedb = Namespace.init_namespace_dbs() Namespace.connect_all_dbs(self.statedb, mibs.STATE_DB) + Namespace.connect_all_dbs(self.statedb, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.statedb, mibs.APPL_DB) self.if_alias_map = {} @@ -153,7 +155,7 @@ def reinit_data(self): # update interface maps _, self.if_alias_map, _, _, _ = \ - Namespace.init_namespace_sync_d_interface_tables(Namespace.init_namespace_dbs()) + Namespace.init_namespace_sync_d_interface_tables(self.statedb) device_metadata = mibs.get_device_metadata(self.statedb[0]) chassis_sub_id = (self.CHASSIS_ID, ) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2863.py b/src/sonic_ax_impl/mibs/ietf/rfc2863.py index ec575e7d0..6853ab952 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2863.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2863.py @@ -49,6 +49,9 @@ def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} diff --git a/src/sonic_ax_impl/mibs/ietf/rfc4292.py b/src/sonic_ax_impl/mibs/ietf/rfc4292.py index c004338bb..b1b16d7df 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc4292.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc4292.py @@ -11,6 +11,7 @@ def __init__(self): super().__init__() self.tos = 0 # ipCidrRouteTos self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.route_dest_map = {} self.route_dest_list = [] ## loopback ip string -> ip address object diff --git a/src/sonic_ax_impl/mibs/ietf/rfc4363.py b/src/sonic_ax_impl/mibs/ietf/rfc4363.py index e9aa46af6..00d2fc2be 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc4363.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc4363.py @@ -11,6 +11,9 @@ class FdbUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.ASIC_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) self.if_name_map = {} self.if_alias_map = {} diff --git a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py index 7661016c4..8eb8c732b 100644 --- a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py +++ b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py @@ -13,6 +13,8 @@ class PfcUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.if_name_map = {} self.if_alias_map = {} diff --git a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py index 274b71695..5d3cb4616 100644 --- a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py +++ b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py @@ -46,6 +46,8 @@ def __init__(self): """ super().__init__() self.db_conn = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) + Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} self.oid_lag_name_map = {} diff --git a/tests/namespace/test_mibs.py b/tests/namespace/test_mibs.py index b461d09ed..99e9bc6cb 100644 --- a/tests/namespace/test_mibs.py +++ b/tests/namespace/test_mibs.py @@ -17,6 +17,8 @@ def setUpClass(cls): def test_init_namespace_sync_d_lag_tables(self): dbs = Namespace.init_namespace_dbs() + Namespace.connect_all_dbs(dbs, mibs.APPL_DB) + Namespace.connect_all_dbs(dbs, mibs.COUNTERS_DB) lag_name_if_name_map, \ if_name_lag_name_map, \ diff --git a/tests/test_mibs.py b/tests/test_mibs.py index 3c28a89e5..ec7b3f0bc 100644 --- a/tests/test_mibs.py +++ b/tests/test_mibs.py @@ -17,6 +17,8 @@ def setUpClass(cls): def test_init_sync_d_lag_tables(self): db_conn = mibs.init_db() + db_conn.connect(mibs.APPL_DB) + db_conn.connect(mibs.COUNTERS_DB) lag_name_if_name_map, \ if_name_lag_name_map, \ From ffe4d38996167caf2813564de3a9c43f4a7a1a2b Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Tue, 16 Jun 2020 11:09:16 -0700 Subject: [PATCH 3/8] Minor fix Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/ieee802_1ab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index dd92fc717..89677cc34 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -233,7 +233,7 @@ def _update_per_namespace_data(self, inst): redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) self.pubsub[inst] = redis_client.pubsub() - pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) + self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) while True: data, interface, if_id = poll_lldp_entry_updates(self.pubsub[inst]) From 20dd2e256088a70b8d29dcbdbec535e82ab96d05 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Tue, 16 Jun 2020 11:10:58 -0700 Subject: [PATCH 4/8] Add mock asic_db. Signed-off-by: SuvarnaMeenakshi --- tests/mock_tables/global_db/asic_db.json | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tests/mock_tables/global_db/asic_db.json diff --git a/tests/mock_tables/global_db/asic_db.json b/tests/mock_tables/global_db/asic_db.json new file mode 100644 index 000000000..2c63c0851 --- /dev/null +++ b/tests/mock_tables/global_db/asic_db.json @@ -0,0 +1,2 @@ +{ +} From 50dd045d98327e2762bf2bcae6de1125dd08b5bc Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Tue, 16 Jun 2020 12:45:15 -0700 Subject: [PATCH 5/8] Remove trailing whitespace. Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/__init__.py | 6 +++--- src/sonic_ax_impl/mibs/ieee802_1ab.py | 6 +++--- src/sonic_ax_impl/mibs/ietf/rfc3433.py | 4 ++-- src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/sonic_ax_impl/mibs/__init__.py b/src/sonic_ax_impl/mibs/__init__.py index 788f54079..00584f9a1 100644 --- a/src/sonic_ax_impl/mibs/__init__.py +++ b/src/sonic_ax_impl/mibs/__init__.py @@ -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 @@ -404,7 +404,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 = [] @@ -413,7 +413,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: diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index 89677cc34..043c02c52 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -160,7 +160,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): """ @@ -310,7 +310,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: @@ -494,7 +494,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) Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc3433.py b/src/sonic_ax_impl/mibs/ietf/rfc3433.py index 4c0754f99..4e97b2bab 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc3433.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc3433.py @@ -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 @@ -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. """ diff --git a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py index 8eb8c732b..ce1859bdf 100644 --- a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py +++ b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py @@ -193,7 +193,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: From 1eb43dfc14bcd585720a2dd84bfa8081974f995a Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Wed, 24 Jun 2020 11:31:06 -0700 Subject: [PATCH 6/8] Revert "Make sure that db connections are done in MIB class init" This reverts commit 1dced6e497b95a8bde631346a682961ef8a9b862. --- src/sonic_ax_impl/mibs/__init__.py | 22 +++++++++++++++-- src/sonic_ax_impl/mibs/ieee802_1ab.py | 24 +++++++------------ src/sonic_ax_impl/mibs/ietf/rfc1213.py | 7 +----- src/sonic_ax_impl/mibs/ietf/rfc2737.py | 4 +--- src/sonic_ax_impl/mibs/ietf/rfc2863.py | 3 --- src/sonic_ax_impl/mibs/ietf/rfc4292.py | 1 - src/sonic_ax_impl/mibs/ietf/rfc4363.py | 3 --- .../mibs/vendor/cisco/ciscoPfcExtMIB.py | 2 -- .../mibs/vendor/cisco/ciscoSwitchQosMIB.py | 2 -- tests/namespace/test_mibs.py | 2 -- tests/test_mibs.py | 2 -- 11 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/sonic_ax_impl/mibs/__init__.py b/src/sonic_ax_impl/mibs/__init__.py index 00584f9a1..18e522ef8 100644 --- a/src/sonic_ax_impl/mibs/__init__.py +++ b/src/sonic_ax_impl/mibs/__init__.py @@ -160,6 +160,10 @@ def init_mgmt_interface_tables(db_conn): :param db_conn: db connector :return: tuple of mgmt name to oid map and mgmt name to alias map """ + + db_conn.connect(CONFIG_DB) + db_conn.connect(STATE_DB) + mgmt_ports_keys = db_conn.keys(CONFIG_DB, mgmt_if_entry_table(b'*')) if not mgmt_ports_keys: @@ -186,6 +190,9 @@ def init_sync_d_interface_tables(db_conn): :return: tuple(if_name_map, if_id_map, oid_map, if_alias_map) """ + # Make sure we're connected to COUNTERS_DB + db_conn.connect(COUNTERS_DB) + # { if_name (SONiC) -> sai_id } # ex: { "Ethernet76" : "1000000000023" } if_name_map, if_id_map = port_util.get_interface_oid_map(db_conn) @@ -225,6 +232,8 @@ def init_sync_d_interface_tables(db_conn): .format(port_util.SONIC_ETHERNET_RE_PATTERN)) logger.warning("Port name map:\n" + pprint.pformat(if_name_map, indent=2)) + db_conn.connect(APPL_DB) + if_alias_map = dict() for if_name in if_name_map: @@ -250,6 +259,8 @@ def init_sync_d_lag_tables(db_conn): # { OID -> lag_name (SONiC) } oid_lag_name_map = {} + db_conn.connect(APPL_DB) + lag_entries = db_conn.keys(APPL_DB, b"LAG_TABLE:*") if not lag_entries: @@ -282,6 +293,10 @@ def init_sync_d_queue_tables(db_conn): Initializes queue maps for SyncD-connected MIB(s). :return: tuple(port_queues_map, queue_stat_map) """ + + # Make sure we're connected to COUNTERS_DB + db_conn.connect(COUNTERS_DB) + # { Port index : Queue index (SONiC) -> sai_id } # ex: { "1:2" : "1000000000023" } queue_name_map = db_conn.get_all(COUNTERS_DB, COUNTERS_QUEUE_NAME_MAP, blocking=True) @@ -334,6 +349,7 @@ def get_device_metadata(db_conn): """ DEVICE_METADATA = "DEVICE_METADATA|localhost" + db_conn.connect(db_conn.STATE_DB) device_metadata = db_conn.get_all(db_conn.STATE_DB, DEVICE_METADATA) return device_metadata @@ -377,8 +393,7 @@ class RedisOidTreeUpdater(MIBUpdater): def __init__(self, prefix_str): super().__init__() - self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, SNMP_OVERLAY_DB) + self.db_conn = Namespace.init_namespace_dbs() if prefix_str.startswith('.'): prefix_str = prefix_str[1:] self.prefix_str = prefix_str @@ -449,6 +464,7 @@ def dbs_keys(dbs, db_name, pattern='*'): """ result_keys=[] for db_conn in dbs: + db_conn.connect(db_name) keys = db_conn.keys(db_name, pattern) if keys is not None: result_keys.extend(keys) @@ -461,6 +477,7 @@ def dbs_get_all(dbs, db_name, _hash, *args, **kwargs): """ result = {} for db_conn in dbs: + db_conn.connect(db_name) if(db_conn.exists(db_name, _hash)): ns_result = db_conn.get_all(db_name, _hash, *args, **kwargs) if ns_result is not None: @@ -569,6 +586,7 @@ def dbs_get_bridge_port_map(dbs, db_name): @staticmethod 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_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) diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index 043c02c52..0d0feab57 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -104,7 +104,6 @@ def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.loc_chassis_data = {} def reinit_data(self): @@ -144,8 +143,6 @@ def __init__(self): self.db_conn = Namespace.init_namespace_dbs() # establish connection to application database. Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_name_map = {} self.if_alias_map = {} self.if_id_map = {} @@ -386,9 +383,6 @@ def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_name_map = {} self.if_alias_map = {} self.if_id_map = {} @@ -497,8 +491,6 @@ def __init__(self): self.db_conn = Namespace.init_namespace_dbs() # establish connection to application database. Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.if_range = [] self.mgmt_ips = {} self.oid_name_map = {} @@ -544,18 +536,18 @@ def update_rem_if_mgmt(self, if_oid, if_name): return self.if_range.sort() - def _update_per_namespace_data(self, inst): + def _update_per_namespace_data(self, db_conn, pubsub): """ Listen to updates in APP DB, update local cache """ - if not self.pubsub[inst]: - redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) - db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) - self.pubsub[inst] = redis_client.pubsub() - self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) + 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(self.pubsub[inst]) + data, interface, if_index = poll_lldp_entry_updates(pubsub) if not data: break @@ -569,7 +561,7 @@ def _update_per_namespace_data(self, inst): def update_data(self): for i in range(len(self.db_conn)): - self._update_per_namespace_data(i) + self._update_per_namespace_data(self.db_conn[i], self.pubsub[i]) def reinit_data(self): diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index c2c3fe8ec..c7dc2848f 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -95,7 +95,6 @@ class NextHopUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.nexthop_map = {} self.route_list = [] @@ -153,11 +152,7 @@ class InterfacesUpdater(MIBUpdater): def __init__(self): super().__init__() - self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.STATE_DB) + self.db_conn = Namespace.init_namespace_dbs() self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2737.py b/src/sonic_ax_impl/mibs/ietf/rfc2737.py index 79dc9425c..4f5a7eb74 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2737.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2737.py @@ -122,8 +122,6 @@ def __init__(self): self.statedb = Namespace.init_namespace_dbs() Namespace.connect_all_dbs(self.statedb, mibs.STATE_DB) - Namespace.connect_all_dbs(self.statedb, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.statedb, mibs.APPL_DB) self.if_alias_map = {} @@ -155,7 +153,7 @@ def reinit_data(self): # update interface maps _, self.if_alias_map, _, _, _ = \ - Namespace.init_namespace_sync_d_interface_tables(self.statedb) + Namespace.init_namespace_sync_d_interface_tables(Namespace.init_namespace_dbs()) device_metadata = mibs.get_device_metadata(self.statedb[0]) chassis_sub_id = (self.CHASSIS_ID, ) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2863.py b/src/sonic_ax_impl/mibs/ietf/rfc2863.py index 6853ab952..ec575e7d0 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2863.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2863.py @@ -49,9 +49,6 @@ def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.CONFIG_DB) self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} diff --git a/src/sonic_ax_impl/mibs/ietf/rfc4292.py b/src/sonic_ax_impl/mibs/ietf/rfc4292.py index b1b16d7df..c004338bb 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc4292.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc4292.py @@ -11,7 +11,6 @@ def __init__(self): super().__init__() self.tos = 0 # ipCidrRouteTos self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.route_dest_map = {} self.route_dest_list = [] ## loopback ip string -> ip address object diff --git a/src/sonic_ax_impl/mibs/ietf/rfc4363.py b/src/sonic_ax_impl/mibs/ietf/rfc4363.py index 00d2fc2be..e9aa46af6 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc4363.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc4363.py @@ -11,9 +11,6 @@ class FdbUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.ASIC_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) self.if_name_map = {} self.if_alias_map = {} diff --git a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py index ce1859bdf..77ee7f88e 100644 --- a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py +++ b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py @@ -13,8 +13,6 @@ class PfcUpdater(MIBUpdater): def __init__(self): super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.if_name_map = {} self.if_alias_map = {} diff --git a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py index 5d3cb4616..274b71695 100644 --- a/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py +++ b/src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py @@ -46,8 +46,6 @@ def __init__(self): """ super().__init__() self.db_conn = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(self.db_conn, mibs.COUNTERS_DB) - Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB) self.lag_name_if_name_map = {} self.if_name_lag_name_map = {} self.oid_lag_name_map = {} diff --git a/tests/namespace/test_mibs.py b/tests/namespace/test_mibs.py index 99e9bc6cb..b461d09ed 100644 --- a/tests/namespace/test_mibs.py +++ b/tests/namespace/test_mibs.py @@ -17,8 +17,6 @@ def setUpClass(cls): def test_init_namespace_sync_d_lag_tables(self): dbs = Namespace.init_namespace_dbs() - Namespace.connect_all_dbs(dbs, mibs.APPL_DB) - Namespace.connect_all_dbs(dbs, mibs.COUNTERS_DB) lag_name_if_name_map, \ if_name_lag_name_map, \ diff --git a/tests/test_mibs.py b/tests/test_mibs.py index ec7b3f0bc..3c28a89e5 100644 --- a/tests/test_mibs.py +++ b/tests/test_mibs.py @@ -17,8 +17,6 @@ def setUpClass(cls): def test_init_sync_d_lag_tables(self): db_conn = mibs.init_db() - db_conn.connect(mibs.APPL_DB) - db_conn.connect(mibs.COUNTERS_DB) lag_name_if_name_map, \ if_name_lag_name_map, \ From 3442ff96abb84a61b32bc94d0b760db36c78f282 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Wed, 24 Jun 2020 11:33:30 -0700 Subject: [PATCH 7/8] 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. Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/ieee802_1ab.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index 0d0feab57..78c125e52 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -536,18 +536,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): + def _update_per_namespace_data(self, inst): """ 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'*'))) + if not self.pubsub[inst]: + redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) + db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) + self.pubsub[inst] = redis_client.pubsub() + self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) while True: - data, interface, if_index = poll_lldp_entry_updates(pubsub) + data, interface, if_index = poll_lldp_entry_updates(self.pubsub[inst]) if not data: break @@ -561,7 +561,7 @@ 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]) + self._update_per_namespace_data(i) def reinit_data(self): From ea91aca896a9e7a4e1ec80d2d288dd75f541cda0 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi Date: Thu, 25 Jun 2020 00:21:35 -0700 Subject: [PATCH 8/8] Fix as per review comments. Signed-off-by: SuvarnaMeenakshi --- src/sonic_ax_impl/mibs/__init__.py | 7 ++++++ src/sonic_ax_impl/mibs/ieee802_1ab.py | 31 ++++++++++---------------- src/sonic_ax_impl/mibs/ietf/rfc2737.py | 17 +++++++------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/sonic_ax_impl/mibs/__init__.py b/src/sonic_ax_impl/mibs/__init__.py index 18e522ef8..e9353861d 100644 --- a/src/sonic_ax_impl/mibs/__init__.py +++ b/src/sonic_ax_impl/mibs/__init__.py @@ -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__() diff --git a/src/sonic_ax_impl/mibs/ieee802_1ab.py b/src/sonic_ax_impl/mibs/ieee802_1ab.py index 78c125e52..6b36ff866 100644 --- a/src/sonic_ax_impl/mibs/ieee802_1ab.py +++ b/src/sonic_ax_impl/mibs/ieee802_1ab.py @@ -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() @@ -222,18 +221,12 @@ def get_next(self, sub_id): return None return self.if_range[right] - def _update_per_namespace_data(self, inst): + def _update_per_namespace_data(self, pubsub): """ Listen to updates in APP DB, update local cache """ - if not self.pubsub[inst]: - redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) - db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) - self.pubsub[inst] = redis_client.pubsub() - self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) - while True: - data, interface, if_id = poll_lldp_entry_updates(self.pubsub[inst]) + data, interface, if_id = poll_lldp_entry_updates(pubsub) if not data: break @@ -243,7 +236,10 @@ def _update_per_namespace_data(self, inst): def update_data(self): for i in range(len(self.db_conn)): - self._update_per_namespace_data(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: @@ -536,18 +532,12 @@ def update_rem_if_mgmt(self, if_oid, if_name): return self.if_range.sort() - def _update_per_namespace_data(self, inst): + def _update_per_namespace_data(self, pubsub): """ Listen to updates in APP DB, update local cache """ - if not self.pubsub[inst]: - redis_client = self.db_conn[inst].get_redis_client(self.db_conn[inst].APPL_DB) - db = self.db_conn[inst].get_dbid(self.db_conn[inst].APPL_DB) - self.pubsub[inst] = redis_client.pubsub() - self.pubsub[inst].psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*'))) - while True: - data, interface, if_index = poll_lldp_entry_updates(self.pubsub[inst]) + data, interface, if_index = poll_lldp_entry_updates(pubsub) if not data: break @@ -561,7 +551,10 @@ def _update_per_namespace_data(self, inst): def update_data(self): for i in range(len(self.db_conn)): - self._update_per_namespace_data(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): diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2737.py b/src/sonic_ax_impl/mibs/ietf/rfc2737.py index 4f5a7eb74..b503967d1 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2737.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2737.py @@ -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, inst): + def _update_per_namespace_data(self, pubsub): """ Update cache. Here we listen to changes in STATE_DB TRANSCEIVER_INFO table @@ -190,14 +190,8 @@ def _update_per_namespace_data(self, inst): # This code is not executed in unit test, since mockredis # does not support pubsub - if not self.pubsub[inst]: - 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)) - while True: - msg = self.pubsub[inst].get_message() + msg = pubsub.get_message() if not msg: break @@ -233,8 +227,13 @@ def _update_per_namespace_data(self, inst): 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(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): """