Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fast-reboot] Fix dump script to support PortChannels in a VLAN group #1393

Merged
merged 7 commits into from
Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 64 additions & 24 deletions scripts/fast-reboot-dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ def generate_neighbor_entries(filename, all_available_macs):
}
arp_output.append(obj)

ip_addr = key.split(':')[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] + ':', '')
ip_addr = key.split(':', 2)[2]
shlomibitton marked this conversation as resolved.
Show resolved Hide resolved
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))

Expand Down Expand Up @@ -80,23 +77,58 @@ 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):
port_id_2_iface = {}
keys = db.keys(db.ASIC_DB, 'ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF:oid:*')
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:
value = db.get_all(db.ASIC_DB, key)
_, lag_name, lag_member_name = key.split(":")
if lag_member_name == member_name:
return lag_name
return None

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:
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
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:*')
tahmed-dev marked this conversation as resolved.
Show resolved Hide resolved
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 lag_port_id_2_iface:
continue
member_id = value['SAI_LAG_MEMBER_ATTR_PORT_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:
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

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 = {}

Expand Down Expand Up @@ -158,29 +190,37 @@ 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')
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swsssdk [](start = 14, length = 7)

swsssdk is deprecated and the import change is alreay merged.

I tested the master latest image plus master latest sonic-utilities

admin@str-msn2700-20:~$ show platform summary
Platform: x86_64-mlnx_msn2700-r0
HwSKU: Mellanox-SN2700-D40C8S8
ASIC: mellanox
ASIC Count: 1
admin@str-msn2700-20:~$ sudo fast-reboot
Failed to run fast-reboot-dump.py. Exit code: 2
admin@str-msn2700-20:~$ sudo fast-reboot-dump.py
admin@str-msn2700-20:~$ echo $?
2

Apr  3 21:59:03.838231 str-msn2700-20 ERR fast-reboot-dump: Got an exception name 'swsssdk' is not defined: Traceback: Traceback (most recent call last):#012  File "/usr/local/bin/fast-reboot-dump.py", line 341, in <module>#012    res = main()#012  File "/usr/local/bin/fast-reboot-dump.py", line 331, in main#012    all_available_macs, map_mac_ip_per_vlan = generate_fdb_entries(root_dir + '/fdb.json')#012  File "/usr/local/bin/fast-reboot-dump.py", line 193, in generate_fdb_entries#012    asic_db = swsssdk.SonicV2Connector(host='127.0.0.1')#012NameError: name 'swsssdk' is not defined
Apr  3 21:59:03.850487 str-msn2700-20 NOTICE admin: fast-reboot failure (12) cleanup ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can we avoid this in the future? for example, can we remove swsssdk as a build or test dependency so that the unit test will automatically fail?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case does not cover the problematic code, so the exception is not triggered.

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

db = SonicV2Connector(use_unix_socket_path=False)
db.connect(db.ASIC_DB, False) # Make one attempt only
vlan_ifaces = get_vlan_ifaces()

bridge_id_2_iface = get_map_bridge_port_id_2_iface_name(db)
fdb_entries, all_available_macs, map_mac_ip_per_vlan = generate_fdb_entries_logic(asic_db, app_db, vlan_ifaces)

vlan_ifaces = get_vlan_ifaces()
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=(',', ': '))

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(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)

with open(filename, 'w') as fp:
json.dump(fdb_entries, fp, indent=2, separators=(',', ': '))

return all_available_macs, map_mac_ip_per_vlan
return fdb_entries, all_available_macs, map_mac_ip_per_vlan

def get_if(iff, cmd):
s = socket.socket()
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
'swsssdk>=2.0.1',
'tabulate==0.8.2',
'xmltodict==0.12.0',
'deepdiff==5.2.3',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deepdiff [](start = 9, length = 8)

This is tests_require?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like it is

],
setup_requires= [
'pytest-runner',
Expand Down
10 changes: 10 additions & 0 deletions tests/fast_reboot_dump_dbs/APPL_DB.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
liat-grozovik marked this conversation as resolved.
Show resolved Hide resolved
"LAG_MEMBER_TABLE:PortChannel0001:Ethernet128": {
liat-grozovik marked this conversation as resolved.
Show resolved Hide resolved
"expireat": 1613562033.6011732,
"ttl": -0.001,
"type": "hash",
"value": {
"status": "enabled"
}
}
}
62 changes: 62 additions & 0 deletions tests/fast_reboot_dump_dbs/ASIC_DB.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
50 changes: 50 additions & 0 deletions tests/fast_reboot_dump_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
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() + '/tests/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

#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']

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)

liat-grozovik marked this conversation as resolved.
Show resolved Hide resolved
@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)