From 64c93a15fe18660ff2e7be0f61fec74717ecca1c Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi <50386592+SuvarnaMeenakshi@users.noreply.github.com> Date: Mon, 16 Nov 2020 11:09:20 -0800 Subject: [PATCH] [RFC4292][Namespace]: Fix implementation of RouteUpdater for multi-asic platform (#176) * [RFC4292][Namespace]: Fix implementation of RouteUpdater for multi-asic platform. For multi-asic platforms, the default route was getting retrieved from only the last namespace as the result dictionary was being written with the same key. To avoid this, modify implementation to retrieve default routes from all front end asic namespaces.Update implementation to ensure that only external routes are displayed in the result. Add test case to test multiple namespaces. --- src/sonic_ax_impl/mibs/ietf/rfc4292.py | 56 ++++--- tests/mock_tables/asic0/config_db.json | 39 +++++ tests/mock_tables/asic1/config_db.json | 36 +++++ tests/mock_tables/asic2/appl_db.json | 4 + tests/mock_tables/asic2/config_db.json | 51 ++++++ tests/mock_tables/multi_asic.py | 56 +++++++ tests/namespace/test_forward.py | 206 +++++++++++++++++++++++++ tests/test_forward.py | 1 + 8 files changed, 429 insertions(+), 20 deletions(-) create mode 100644 tests/mock_tables/multi_asic.py create mode 100644 tests/namespace/test_forward.py diff --git a/src/sonic_ax_impl/mibs/ietf/rfc4292.py b/src/sonic_ax_impl/mibs/ietf/rfc4292.py index a103360b1938..0766778f0556 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc4292.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc4292.py @@ -5,6 +5,7 @@ from ax_interface import MIBMeta, ValueType, MIBUpdater, SubtreeMIBEntry from ax_interface.util import ip2tuple_v4 from bisect import bisect_right +from sonic_py_common import multi_asic class RouteUpdater(MIBUpdater): def __init__(self): @@ -49,26 +50,41 @@ def update_data(self): self.route_dest_list.append(sub_id) self.route_dest_map[sub_id] = self.loips[loip].packed - route_entries = Namespace.dbs_keys(self.db_conn, mibs.APPL_DB, "ROUTE_TABLE:*") - if not route_entries: - return - - for route_entry in route_entries: - routestr = route_entry - ipnstr = routestr[len("ROUTE_TABLE:"):] - if ipnstr == "0.0.0.0/0": - ipn = ipaddress.ip_network(ipnstr) - ent = Namespace.dbs_get_all(self.db_conn, mibs.APPL_DB, routestr, blocking=True) - nexthops = ent["nexthop"] - ifnames = ent["ifname"] - for nh, ifn in zip(nexthops.split(','), ifnames.split(',')): - ## Ignore non front panel interfaces - ## TODO: non front panel interfaces should not be in APPL_DB at very beginning - ## This is to workaround the bug in current sonic-swss implementation - if ifn == "eth0" or ifn == "lo" or ifn == "docker0": continue - sub_id = ip2tuple_v4(ipn.network_address) + ip2tuple_v4(ipn.netmask) + (self.tos,) + ip2tuple_v4(nh) - self.route_dest_list.append(sub_id) - self.route_dest_map[sub_id] = ipn.network_address.packed + # Get list of front end asic namespaces for multi-asic platform. + # This list will be empty for single asic platform. + front_ns = multi_asic.get_all_namespaces()['front_ns'] + ipnstr = "0.0.0.0/0" + ipn = ipaddress.ip_network(ipnstr) + route_str = "ROUTE_TABLE:0.0.0.0/0" + + for db_conn in Namespace.get_non_host_dbs(self.db_conn): + # For multi-asic platform, proceed to get routes only for + # front end namespaces. + # For single-asic platform, front_ns will be empty list. + if front_ns and db_conn.namespace not in front_ns: + continue + port_table = multi_asic.get_port_table(db_conn.namespace) + ent = db_conn.get_all(mibs.APPL_DB, route_str, blocking=False) + if ent is None: + continue + nexthops = ent["nexthop"] + ifnames = ent["ifname"] + for nh, ifn in zip(nexthops.split(','), ifnames.split(',')): + ## Ignore non front panel interfaces + ## TODO: non front panel interfaces should not be in APPL_DB at very beginning + ## This is to workaround the bug in current sonic-swss implementation + if ifn == "eth0" or ifn == "lo" or ifn == "docker0": + continue + # Ignore internal asic routes + if multi_asic.is_port_channel_internal(ifn, db_conn.namespace): + continue + if (ifn in port_table and + multi_asic.ROLE in port_table[ifn] and + port_table[ifn][multi_asic.ROLE] == multi_asic.INTERNAL_PORT): + continue + sub_id = ip2tuple_v4(ipn.network_address) + ip2tuple_v4(ipn.netmask) + (self.tos,) + ip2tuple_v4(nh) + self.route_dest_list.append(sub_id) + self.route_dest_map[sub_id] = ipn.network_address.packed self.route_dest_list.sort() diff --git a/tests/mock_tables/asic0/config_db.json b/tests/mock_tables/asic0/config_db.json index 2c63c0851048..f82c04d5a32c 100644 --- a/tests/mock_tables/asic0/config_db.json +++ b/tests/mock_tables/asic0/config_db.json @@ -1,2 +1,41 @@ { + "DEVICE_METADATA|localhost": { + "chassis_serial_number": "SAMPLETESTSN", + "sub_role": "FrontEnd" + }, + "PORT_TABLE:Ethernet0": { + "description": "snowflake", + "alias": "etp1", + "role": "Ext", + "speed": 100000 + }, + "PORT_TABLE:Ethernet4": { + "description": "snowflake", + "alias": "etp2", + "role": "Ext", + "speed": 100000 + }, + "PORT_TABLE:Ethernet-BP0": { + "description": "snowflake", + "alias": "etp3", + "role": "Int", + "speed": 100000 + }, + "PORT_TABLE:Ethernet-BP4": { + "description": "snowflake", + "alias": "etp4", + "role": "Int", + "speed": 100000 + }, + "LAG_MEMBER_TABLE:PortChannel01:Ethernet-BP0": { + "status": "enabled" + }, + "LAG_MEMBER_TABLE:PortChannel01:Ethernet-BP4": { + "status": "enabled" + }, + "LAG_TABLE:PortChannel01": { + "admin_status": "up", + "oper_status": "up", + "mtu": "9216" + } } diff --git a/tests/mock_tables/asic1/config_db.json b/tests/mock_tables/asic1/config_db.json index 2c63c0851048..0c9578211fdc 100644 --- a/tests/mock_tables/asic1/config_db.json +++ b/tests/mock_tables/asic1/config_db.json @@ -1,2 +1,38 @@ { + "DEVICE_METADATA|localhost": { + "chassis_serial_number": "SAMPLETESTSN", + "sub_role": "FrontEnd" + }, + "PORT_TABLE:Ethernet8": { + "description": "snowflake", + "alias": "etp5", + "role": "Ext", + "speed": 100000 + }, + "PORT_TABLE:Ethernet12": { + "speed": 1000, + "role": "Ext", + "alias": "etp6" + }, + "PORT_TABLE:Ethernet-BP8": { + "role": "Int", + "alias": "etp7" + }, + "PORT_TABLE:Ethernet-BP12": { + "description": "snowflake", + "role": "Int", + "alias": "etp8", + "speed": 1000 + }, + "LAG_MEMBER_TABLE:PortChannel02:Ethernet-BP08": { + "status": "enabled" + }, + "LAG_MEMBER_TABLE:PortChannel02:Ethernet-BP12": { + "status": "enabled" + }, + "LAG_TABLE:PortChannel02": { + "admin_status": "up", + "oper_status": "up", + "mtu": "9216" + } } diff --git a/tests/mock_tables/asic2/appl_db.json b/tests/mock_tables/asic2/appl_db.json index 37aad20d620e..87a07cf314eb 100644 --- a/tests/mock_tables/asic2/appl_db.json +++ b/tests/mock_tables/asic2/appl_db.json @@ -36,6 +36,10 @@ "nexthop": "", "ifname": "lo" }, + "ROUTE_TABLE:0.0.0.0/0": { + "ifname": "PortChannel03,PortChannel04", + "nexthop": "10.10.0.0,10.10.0.5" + }, "LAG_MEMBER_TABLE:PortChannel03:Ethernet-BP16": { "status": "enabled" }, diff --git a/tests/mock_tables/asic2/config_db.json b/tests/mock_tables/asic2/config_db.json index 2c63c0851048..dddff2a9cb14 100644 --- a/tests/mock_tables/asic2/config_db.json +++ b/tests/mock_tables/asic2/config_db.json @@ -1,2 +1,53 @@ { + "DEVICE_METADATA|localhost": { + "chassis_serial_number": "SAMPLETESTSN", + "sub_role": "BackEnd" + }, + "PORT_TABLE:Ethernet-BP16": { + "description": "backplane", + "alias": "etp9", + "speed": 100000 + }, + "PORT_TABLE:Ethernet-BP20": { + "description": "backplane", + "alias": "etp10", + "speed": 100000 + }, + "PORT_TABLE:Ethernet-BP24": { + "description": "backplane", + "alias": "etp11", + "speed": 100000 + }, + "PORT_TABLE:Ethernet-BP28": { + "description": "backplane", + "alias": "etp12", + "speed": 100000 + }, + "LAG_MEMBER_TABLE:PortChannel03:Ethernet-BP16": { + "status": "enabled" + }, + "LAG_MEMBER_TABLE:PortChannel03:Ethernet-BP20": { + "status": "enabled" + }, + "LAG_MEMBER_TABLE:PortChannel04:Ethernet-BP24": { + "status": "enabled" + }, + "LAG_MEMBER_TABLE:PortChannel04:Ethernet-BP28": { + "status": "enabled" + }, + "LAG_TABLE:PortChannel03": { + "admin_status": "up", + "oper_status": "up", + "mtu": "9216" + }, + "LAG_TABLE:PortChannel04": { + "admin_status": "up", + "oper_status": "up", + "mtu": "9216" + }, + "LAG_TABLE:PortChannel_Temp": { + "admin_status": "up", + "oper_status": "up", + "mtu": "9216" + } } diff --git a/tests/mock_tables/multi_asic.py b/tests/mock_tables/multi_asic.py new file mode 100644 index 000000000000..ba0a2f75b1fb --- /dev/null +++ b/tests/mock_tables/multi_asic.py @@ -0,0 +1,56 @@ +import os +import json + +import tests.mock_tables.dbconnector +from sonic_py_common import multi_asic +from swsssdk import SonicDBConfig + +INPUT_DIR = os.path.dirname(os.path.abspath(__file__)) +int_port_channel = ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04'] + +def mock_get_num_asics(): + ns_list = SonicDBConfig.get_ns_list() + if len(ns_list) > 1: + return(len(ns_list) - 1) + else: + return 1 + + +def mock_is_multi_asic(): + if mock_get_num_asics() > 1: + return True + else: + return False + +def mock_get_all_namespaces(): + if mock_get_num_asics() == 1: + return {'front_ns': [], 'back_ns': []} + else: + return {'front_ns': ['asic0', 'asic1'], 'back_ns': ['asic2']} + +def mock_is_port_channel_internal(port_channel, namespace=None): + if (mock_get_num_asics() == 1): + return False + else: + return True if port_channel in int_port_channel else False + +def mock_get_port_table(namespace=None): + if namespace is not None: + fname = os.path.join(INPUT_DIR, namespace, 'config_db.json') + else: + fname = os.path.join(INPUT_DIR, 'config_db.json') + port_table = {} + db = {} + with open(fname) as f: + db = json.load(f) + for k in db: + if 'PORT_TABLE' in db: + new_key = k[len('PORT_TABLE:'):] + port_table[new_key] = db[k] + return port_table + +multi_asic.get_num_asics = mock_get_num_asics +multi_asic.is_multi_asic = mock_is_multi_asic +multi_asic.get_all_namespaces = mock_get_all_namespaces +multi_asic.is_port_channel_internal = mock_is_port_channel_internal +multi_asic.get_port_table = mock_get_port_table diff --git a/tests/namespace/test_forward.py b/tests/namespace/test_forward.py new file mode 100644 index 000000000000..362f801d7e26 --- /dev/null +++ b/tests/namespace/test_forward.py @@ -0,0 +1,206 @@ +import os +import sys +import ipaddress +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 + +import tests.mock_tables.dbconnector +import tests.mock_tables.multi_asic + +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.mibs.ietf import rfc4292 +from sonic_ax_impl.main import SonicMIB + +class TestForwardMIB(TestCase): + @classmethod + def setUpClass(cls): + tests.mock_tables.dbconnector.load_namespace_config() + importlib.reload(rfc4292) + cls.lut = MIBTable(rfc4292.IpCidrRouteTable) + for updater in cls.lut.updater_instances: + updater.update_data() + updater.reinit_data() + updater.update_data() + + def test_update(self): + for updater in self.lut.updater_instances: + updater.update_data() + updater.reinit_data() + updater.update_data() + + def test_network_order(self): + ip = ipaddress.ip_address("0.1.2.3") + ipb = ip.packed + ips = ".".join(str(int(x)) for x in list(ipb)) + self.assertEqual(ips, "0.1.2.3") + + def test_getnextpdu_first_default(self): + # oid.include = 1 + oid = ObjectIdentifier(10, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1)) + 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) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.IP_ADDRESS) + self.assertEqual(str(value0.name), '.1.3.6.1.2.1.4.24.4.1.1.0.0.0.0.0.0.0.0.0.10.0.0.1') + self.assertEqual(str(value0.data), ipaddress.ip_address("0.0.0.0").packed.decode()) + + def test_getpdu(self): + oid = ObjectIdentifier(24, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 0, 0, 9)) + 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.IP_ADDRESS) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(str(value0.data), ipaddress.ip_address("0.0.0.0").packed.decode()) + + def test_getnextpdu(self): + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=( + ObjectIdentifier(21, 0, 0, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 1, 0)), + ) + ) + + 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.IP_ADDRESS) + self.assertEqual(str(value0.data), ipaddress.ip_address("0.0.0.0").packed.decode()) + + def test_getnextpdu_exactmatch(self): + oid = ObjectIdentifier(24, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 0, 0, 3)) + 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.IP_ADDRESS) + print("test_getnextpdu_exactmatch: ", str(oid)) + self.assertEqual(str(value0.name), str(oid)) + self.assertEqual(str(value0.data), ipaddress.ip_address("0.0.0.0").packed.decode()) + + 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, 4, 24, 4, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1)), + ) + ) + + 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) + + #test to ensure internal asic route is not present in SNMP output + def test_getpdu_internal_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, 4, 24, 4, 1, 1, 0, 0, 0, 0, 0, 10, 10, 0, 5)), + ) + ) + + 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(12, 0, 0, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 1, 255)), + ) + ) + + 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) + + def test_getpdu_loopback_status(self): + loip_tuple = (10, 1, 0, 32) # ref: appl_db.json + lomask_tuple = (255, 255, 255, 255) + emptyip_tuple = (0, 0, 0, 0) + + oid = ObjectIdentifier(24, 0, 1, 0 + , (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 16) + loip_tuple + lomask_tuple + (0,) + emptyip_tuple + ) + 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) + + 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_first_default_status(self): + oid = ObjectIdentifier(10, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 24, 4, 1, 16)) + 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) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.INTEGER) + self.assertEqual(str(value0.name), '.1.3.6.1.2.1.4.24.4.1.16.0.0.0.0.0.0.0.0.0.10.0.0.1') + self.assertEqual(value0.data, 1) + + @classmethod + def tearDownClass(cls): + tests.mock_tables.dbconnector.clean_up_config() diff --git a/tests/test_forward.py b/tests/test_forward.py index 3fb9e1271929..57d876134a2b 100644 --- a/tests/test_forward.py +++ b/tests/test_forward.py @@ -8,6 +8,7 @@ from unittest import TestCase import tests.mock_tables.dbconnector +import tests.mock_tables.multi_asic from ax_interface.mib import MIBTable from ax_interface.pdu import PDUHeader