From 113430368169b0c50896e70339c24ed70f6e6c41 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Wed, 13 Jan 2021 14:05:24 +0200 Subject: [PATCH 1/6] Fix for PortChannel member of VLAN on fast-reboot Fix of IP parsing. Signed-off-by: Shlomi Bitton --- scripts/fast-reboot-dump.py | 49 ++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/scripts/fast-reboot-dump.py b/scripts/fast-reboot-dump.py index 3c655381f4..9f83616b19 100644 --- a/scripts/fast-reboot-dump.py +++ b/scripts/fast-reboot-dump.py @@ -39,7 +39,7 @@ def generate_neighbor_entries(filename, all_available_macs): } arp_output.append(obj) - ip_addr = key.split(':')[2] + ip_addr = key.split(':', 2)[2] if ipaddress.ip_interface(str(ip_addr)).ip.version != 4: #This is ipv6 address ip_addr = key.replace(key.split(':')[0] + ':' + key.split(':')[1] + ':', '') @@ -80,23 +80,45 @@ def get_bridge_port_id_2_port_id(db): return bridge_port_id_2_port_id -def get_map_port_id_2_iface_name(db): +def get_lag_by_member(member_name, app_db): + keys = app_db.keys(app_db.APPL_DB, 'LAG_MEMBER_TABLE:*') + keys = [] if keys is None else keys + for key in keys: + _, lag_name, lag_member_name = key.split(":") + if lag_member_name == member_name: + return lag_name + return None + +def get_map_port_id_2_iface_name(asic_db, app_db): port_id_2_iface = {} - keys = db.keys(db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF:oid:*') + keys = asic_db.keys(asic_db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF:oid:*') keys = [] if keys is None else keys for key in keys: - value = db.get_all(db.ASIC_DB, key) + value = asic_db.get_all(asic_db.ASIC_DB, key) if value['SAI_HOSTIF_ATTR_TYPE'] != 'SAI_HOSTIF_TYPE_NETDEV': continue port_id = value['SAI_HOSTIF_ATTR_OBJ_ID'] iface_name = value['SAI_HOSTIF_ATTR_NAME'] port_id_2_iface[port_id] = iface_name + keys = asic_db.keys(asic_db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER:oid:*') + keys = [] if keys is None else keys + for key in keys: + value = asic_db.get_all(asic_db.ASIC_DB, key) + lag_id = value['SAI_LAG_MEMBER_ATTR_LAG_ID'] + if lag_id in port_id_2_iface: + continue + member_id = value['SAI_LAG_MEMBER_ATTR_PORT_ID'] + member_name = port_id_2_iface[member_id] + lag_name = get_lag_by_member(member_name, app_db) + if lag_name is not None: + port_id_2_iface[lag_id] = lag_name + return port_id_2_iface -def get_map_bridge_port_id_2_iface_name(db): - bridge_port_id_2_port_id = get_bridge_port_id_2_port_id(db) - port_id_2_iface = get_map_port_id_2_iface_name(db) +def get_map_bridge_port_id_2_iface_name(asic_db, app_db): + bridge_port_id_2_port_id = get_bridge_port_id_2_port_id(asic_db) + port_id_2_iface = get_map_port_id_2_iface_name(asic_db, app_db) bridge_port_id_2_iface_name = {} @@ -160,10 +182,12 @@ def get_fdb(db, vlan_name, vlan_id, bridge_id_2_iface): def generate_fdb_entries(filename): fdb_entries = [] - db = swsssdk.SonicV2Connector(host='127.0.0.1') - db.connect(db.ASIC_DB, False) # Make one attempt only + asic_db = swsssdk.SonicV2Connector(host='127.0.0.1') + app_db = swsssdk.SonicV2Connector(host='127.0.0.1') + asic_db.connect(asic_db.ASIC_DB, False) # Make one attempt only + app_db.connect(app_db.APPL_DB, False) # Make one attempt only - bridge_id_2_iface = get_map_bridge_port_id_2_iface_name(db) + bridge_id_2_iface = get_map_bridge_port_id_2_iface_name(asic_db, app_db) vlan_ifaces = get_vlan_ifaces() @@ -171,11 +195,12 @@ def generate_fdb_entries(filename): map_mac_ip_per_vlan = {} for vlan in vlan_ifaces: vlan_id = int(vlan.replace('Vlan', '')) - fdb_entry, available_macs, map_mac_ip_per_vlan[vlan] = get_fdb(db, vlan, vlan_id, bridge_id_2_iface) + fdb_entry, available_macs, map_mac_ip_per_vlan[vlan] = get_fdb(asic_db, vlan, vlan_id, bridge_id_2_iface) all_available_macs |= available_macs fdb_entries.extend(fdb_entry) - db.close(db.ASIC_DB) + asic_db.close(asic_db.ASIC_DB) + app_db.close(app_db.APPL_DB) with open(filename, 'w') as fp: json.dump(fdb_entries, fp, indent=2, separators=(',', ': ')) From 90742b19bd72fe38c300fdc2bb07e16050b051c6 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Mon, 8 Feb 2021 18:18:34 +0200 Subject: [PATCH 2/6] remove redundant ip parsing --- scripts/fast-reboot-dump.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/fast-reboot-dump.py b/scripts/fast-reboot-dump.py index 4c5047adf7..084d52db50 100644 --- a/scripts/fast-reboot-dump.py +++ b/scripts/fast-reboot-dump.py @@ -40,9 +40,6 @@ def generate_neighbor_entries(filename, all_available_macs): arp_output.append(obj) ip_addr = key.split(':', 2)[2] - if ipaddress.ip_interface(str(ip_addr)).ip.version != 4: - #This is ipv6 address - ip_addr = key.replace(key.split(':')[0] + ':' + key.split(':')[1] + ':', '') neighbor_entries.append((vlan_name, mac, ip_addr)) syslog.syslog(syslog.LOG_INFO, "Neighbor entry: [Vlan: %s, Mac: %s, Ip: %s]" % (vlan_name, mac, ip_addr)) From a8e4a29f7a02be4c924d73209ddfac8dd58a06bb Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Wed, 17 Feb 2021 12:55:47 +0000 Subject: [PATCH 3/6] Add unit-test for Vlan PortChannel member scenario Adapt the fast-reboot-dump script for unit testing --- scripts/fast-reboot-dump.py | 27 ++++++----- tests/fast_reboot_dump_dbs/APPL_DB.json | 10 ++++ tests/fast_reboot_dump_dbs/ASIC_DB.json | 62 +++++++++++++++++++++++++ tests/fast_reboot_dump_test.py | 49 +++++++++++++++++++ 4 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 tests/fast_reboot_dump_dbs/APPL_DB.json create mode 100644 tests/fast_reboot_dump_dbs/ASIC_DB.json create mode 100644 tests/fast_reboot_dump_test.py diff --git a/scripts/fast-reboot-dump.py b/scripts/fast-reboot-dump.py index 084d52db50..1f5b90bbf3 100644 --- a/scripts/fast-reboot-dump.py +++ b/scripts/fast-reboot-dump.py @@ -177,24 +177,14 @@ def get_fdb(db, vlan_name, vlan_id, bridge_id_2_iface): return fdb_entries, available_macs, map_mac_ip def generate_fdb_entries(filename): - fdb_entries = [] - asic_db = swsssdk.SonicV2Connector(host='127.0.0.1') app_db = swsssdk.SonicV2Connector(host='127.0.0.1') asic_db.connect(asic_db.ASIC_DB, False) # Make one attempt only app_db.connect(app_db.APPL_DB, False) # Make one attempt only - bridge_id_2_iface = get_map_bridge_port_id_2_iface_name(asic_db, app_db) - vlan_ifaces = get_vlan_ifaces() - all_available_macs = set() - map_mac_ip_per_vlan = {} - for vlan in vlan_ifaces: - vlan_id = int(vlan.replace('Vlan', '')) - fdb_entry, available_macs, map_mac_ip_per_vlan[vlan] = get_fdb(asic_db, vlan, vlan_id, bridge_id_2_iface) - all_available_macs |= available_macs - fdb_entries.extend(fdb_entry) + fdb_entries, all_available_macs, map_mac_ip_per_vlan = generate_fdb_entries_logic(asic_db, app_db, vlan_ifaces) asic_db.close(asic_db.ASIC_DB) app_db.close(app_db.APPL_DB) @@ -204,6 +194,21 @@ def generate_fdb_entries(filename): return all_available_macs, map_mac_ip_per_vlan +def generate_fdb_entries_logic(asic_db, app_db, vlan_ifaces): + fdb_entries = [] + all_available_macs = set() + map_mac_ip_per_vlan = {} + + bridge_id_2_iface = get_map_bridge_port_id_2_iface_name(asic_db, app_db) + + for vlan in vlan_ifaces: + vlan_id = int(vlan.replace('Vlan', '')) + fdb_entry, available_macs, map_mac_ip_per_vlan[vlan] = get_fdb(asic_db, vlan, vlan_id, bridge_id_2_iface) + all_available_macs |= available_macs + fdb_entries.extend(fdb_entry) + + return fdb_entries, all_available_macs, map_mac_ip_per_vlan + def get_if(iff, cmd): s = socket.socket() ifreq = ioctl(s, cmd, struct.pack("16s16x",bytes(iff.encode()))) diff --git a/tests/fast_reboot_dump_dbs/APPL_DB.json b/tests/fast_reboot_dump_dbs/APPL_DB.json new file mode 100644 index 0000000000..cb7ebd1f29 --- /dev/null +++ b/tests/fast_reboot_dump_dbs/APPL_DB.json @@ -0,0 +1,10 @@ +{ + "LAG_MEMBER_TABLE:PortChannel0001:Ethernet128": { + "expireat": 1613562033.6011732, + "ttl": -0.001, + "type": "hash", + "value": { + "status": "enabled" + } + } +} diff --git a/tests/fast_reboot_dump_dbs/ASIC_DB.json b/tests/fast_reboot_dump_dbs/ASIC_DB.json new file mode 100644 index 0000000000..c6ff380ac8 --- /dev/null +++ b/tests/fast_reboot_dump_dbs/ASIC_DB.json @@ -0,0 +1,62 @@ +{ + "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000001724": { + "expireat": 1613562026.636173, + "ttl": -0.001, + "type": "hash", + "value": { + "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true", + "SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW", + "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x20000000016ea", + "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT" + } + }, + "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF:oid:0xd000000000eea": { + "expireat": 1613562026.59113, + "ttl": -0.001, + "type": "hash", + "value": { + "SAI_HOSTIF_ATTR_NAME": "Ethernet128", + "SAI_HOSTIF_ATTR_OBJ_ID": "oid:0x1000000000ec1", + "SAI_HOSTIF_ATTR_OPER_STATUS": "true", + "SAI_HOSTIF_ATTR_TYPE": "SAI_HOSTIF_TYPE_NETDEV", + "SAI_HOSTIF_ATTR_VLAN_TAG": "SAI_HOSTIF_VLAN_TAG_KEEP" + } + }, + "ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER:oid:0x1b0000000016ee": { + "expireat": 1613562026.634684, + "ttl": -0.001, + "type": "hash", + "value": { + "SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE": "false", + "SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE": "false", + "SAI_LAG_MEMBER_ATTR_LAG_ID": "oid:0x20000000016ea", + "SAI_LAG_MEMBER_ATTR_PORT_ID": "oid:0x1000000000ec1" + } + }, + "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\"oid:0x260000000016f3\",\"mac\":\"52:54:00:5D:FC:B7\",\"switch_id\":\"oid:0x21000000000000\"}": { + "expireat": 1613562026.593537, + "ttl": -0.001, + "type": "hash", + "value": { + "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000001724", + "SAI_FDB_ENTRY_ATTR_PACKET_ACTION": "SAI_PACKET_ACTION_FORWARD", + "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC" + } + }, + "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:oid:0x26000000000013": { + "expireat": 1613562026.598609, + "ttl": -0.001, + "type": "hash", + "value": { + "NULL": "NULL" + } + }, + "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:oid:0x260000000016f3": { + "expireat": 1613562026.589369, + "ttl": -0.001, + "type": "hash", + "value": { + "SAI_VLAN_ATTR_VLAN_ID": "2" + } + } +} diff --git a/tests/fast_reboot_dump_test.py b/tests/fast_reboot_dump_test.py new file mode 100644 index 0000000000..1da19df235 --- /dev/null +++ b/tests/fast_reboot_dump_test.py @@ -0,0 +1,49 @@ +import json +import os +from deepdiff import DeepDiff +from utilities_common.db import Db +import importlib +fast_reboot_dump = importlib.import_module("scripts.fast-reboot-dump") + +class TestFastRebootDump(object): + + @classmethod + def setup_class(cls): + print("SETUP") + + test_db_dumps_directory = os.getcwd() + '/fast_reboot_dump_dbs' + asic_db_object = Db() + app_db_object = Db() + asic_db = asic_db_object.db + app_db = app_db_object.db + populate_db(asic_db, test_db_dumps_directory, 'ASIC_DB.json') + populate_db(app_db, test_db_dumps_directory, 'APPL_DB.json') + + cls.asic_db = asic_db + cls.app_db = app_db + + def test_generate_fdb_entries_vlan_portcahnnel_member(self): + vlan_ifaces = ['Vlan2'] + + fdb_entries, all_available_macs, map_mac_ip_per_vlan = fast_reboot_dump.generate_fdb_entries_logic(self.asic_db, self.app_db, vlan_ifaces) + + expectd_fdb_entries = [{'FDB_TABLE:Vlan2:52-54-00-5D-FC-B7': {'type': 'dynamic', 'port': 'PortChannel0001'}, 'OP': 'SET'}] + assert not DeepDiff(fdb_entries, expectd_fdb_entries, ignore_order=True) + + expectd_all_available_macs = {('Vlan2', '52:54:00:5d:fc:b7')} + assert not DeepDiff(all_available_macs, expectd_all_available_macs, ignore_order=True) + + expectd_map_mac_ip_per_vlan = {'Vlan2': {'52:54:00:5d:fc:b7': 'PortChannel0001'}} + assert not DeepDiff(map_mac_ip_per_vlan, expectd_map_mac_ip_per_vlan, ignore_order=True) + + @classmethod + def teardown_class(cls): + print("TEARDOWN") + +def populate_db(dbconn, test_db_dumps_directory, db_dump_filename): + db = getattr(dbconn, db_dump_filename.replace('.json','')) + with open(test_db_dumps_directory + '/' + db_dump_filename) as DB: + db_dump = json.load(DB) + for table, fields in db_dump.items(): + for key, value in fields['value'].items(): + dbconn.set(db, table, key, value) From efd8022f61937e0f4555fd4f8a64cb4802709824 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Wed, 17 Feb 2021 15:47:56 +0000 Subject: [PATCH 4/6] Add dependency to setup.py --- setup.py | 1 + tests/fast_reboot_dump_test.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 2af6829029..70be706dd3 100644 --- a/setup.py +++ b/setup.py @@ -161,6 +161,7 @@ 'swsssdk>=2.0.1', 'tabulate==0.8.2', 'xmltodict==0.12.0', + 'deepdiff==5.2.3', ], setup_requires= [ 'pytest-runner', diff --git a/tests/fast_reboot_dump_test.py b/tests/fast_reboot_dump_test.py index 1da19df235..5bf44b2c5e 100644 --- a/tests/fast_reboot_dump_test.py +++ b/tests/fast_reboot_dump_test.py @@ -11,7 +11,7 @@ class TestFastRebootDump(object): def setup_class(cls): print("SETUP") - test_db_dumps_directory = os.getcwd() + '/fast_reboot_dump_dbs' + test_db_dumps_directory = os.getcwd() + '/tests/fast_reboot_dump_dbs' asic_db_object = Db() app_db_object = Db() asic_db = asic_db_object.db From cab258aad03ae7d02b0b7060eff63df674e237a5 Mon Sep 17 00:00:00 2001 From: shlomibitton <60430976+shlomibitton@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:57:11 +0200 Subject: [PATCH 5/6] Add test description --- tests/fast_reboot_dump_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fast_reboot_dump_test.py b/tests/fast_reboot_dump_test.py index 5bf44b2c5e..92b3b55340 100644 --- a/tests/fast_reboot_dump_test.py +++ b/tests/fast_reboot_dump_test.py @@ -22,6 +22,7 @@ def setup_class(cls): cls.asic_db = asic_db cls.app_db = app_db + #Test fast-reboot-dump script to generate all required objects when there is a VLAN interface with a PortChannel member. def test_generate_fdb_entries_vlan_portcahnnel_member(self): vlan_ifaces = ['Vlan2'] From e6fd007d5d3022ff2a32f08df6ffbb82771962f2 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Mon, 15 Mar 2021 12:43:50 +0200 Subject: [PATCH 6/6] refactor the code to be more readable --- scripts/fast-reboot-dump.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/scripts/fast-reboot-dump.py b/scripts/fast-reboot-dump.py index 1f5b90bbf3..54550fb8bc 100644 --- a/scripts/fast-reboot-dump.py +++ b/scripts/fast-reboot-dump.py @@ -86,8 +86,8 @@ def get_lag_by_member(member_name, app_db): return lag_name return None -def get_map_port_id_2_iface_name(asic_db, app_db): - port_id_2_iface = {} +def get_map_host_port_id_2_iface_name(asic_db): + host_port_id_2_iface = {} keys = asic_db.keys(asic_db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF:oid:*') keys = [] if keys is None else keys for key in keys: @@ -96,20 +96,33 @@ def get_map_port_id_2_iface_name(asic_db, app_db): continue port_id = value['SAI_HOSTIF_ATTR_OBJ_ID'] iface_name = value['SAI_HOSTIF_ATTR_NAME'] - port_id_2_iface[port_id] = iface_name + host_port_id_2_iface[port_id] = iface_name + + return host_port_id_2_iface +def get_map_lag_port_id_2_portchannel_name(asic_db, app_db, host_port_id_2_iface): + lag_port_id_2_iface = {} keys = asic_db.keys(asic_db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_LAG_MEMBER:oid:*') keys = [] if keys is None else keys for key in keys: value = asic_db.get_all(asic_db.ASIC_DB, key) lag_id = value['SAI_LAG_MEMBER_ATTR_LAG_ID'] - if lag_id in port_id_2_iface: + if lag_id in lag_port_id_2_iface: continue member_id = value['SAI_LAG_MEMBER_ATTR_PORT_ID'] - member_name = port_id_2_iface[member_id] + member_name = host_port_id_2_iface[member_id] lag_name = get_lag_by_member(member_name, app_db) if lag_name is not None: - port_id_2_iface[lag_id] = lag_name + lag_port_id_2_iface[lag_id] = lag_name + + return lag_port_id_2_iface + +def get_map_port_id_2_iface_name(asic_db, app_db): + port_id_2_iface = {} + host_port_id_2_iface = get_map_host_port_id_2_iface_name(asic_db) + port_id_2_iface.update(host_port_id_2_iface) + lag_port_id_2_iface = get_map_lag_port_id_2_portchannel_name(asic_db, app_db, host_port_id_2_iface) + port_id_2_iface.update(lag_port_id_2_iface) return port_id_2_iface