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.

Sign up for GitHub

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

Jump to bottom

Common functions for Multi ASIC CLIs #4973

Merged
merged 24 commits into from
Aug 14, 2020
Merged

Common functions for Multi ASIC CLIs #4973

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0c7d22f
Changes for identifing the cli object's role
arlakshm Jul 15, 2020
0620982
Fix lgtm warning
arlakshm Jul 15, 2020
00e34d0
address review comment
arlakshm Jul 16, 2020
b133dc1
Fixed review comments
arlakshm Jul 16, 2020
3c671c1
Add index to port_config and minor fixes
arlakshm Jul 20, 2020
51adf86
Merge remote-tracking branch 'arlakshm/master' into masic_port_role
arlakshm Jul 27, 2020
4985d2d
move common functions to sonic_py_common package
arlakshm Jul 28, 2020
525b6b4
fix compilation issue
arlakshm Jul 30, 2020
2af6005
Address review comments
arlakshm Aug 5, 2020
3028347
reorganize imports
arlakshm Aug 5, 2020
c249599
Merge remote-tracking branch 'arlakshm/master' into masic_port_role
arlakshm Aug 6, 2020
c69a6c3
Fix compilation
arlakshm Aug 6, 2020
ea16670
fix lgtm warnings
arlakshm Aug 6, 2020
2e5bc12
Merge remote-tracking branch 'arlakshm/master' into masic_port_role
arlakshm Aug 7, 2020
413b06d
use TCP instead of Unix socket
arlakshm Aug 7, 2020
6a3009c
address review commit
arlakshm Aug 7, 2020
7628bbd
change the port in port_config to Ext\Int
arlakshm Aug 7, 2020
b4837ff
Merge remote-tracking branch 'arlakshm/master' into masic_port_role
arlakshm Aug 10, 2020
cabc2f3
revert the changes in device_info.py
arlakshm Aug 10, 2020
789a548
address review comments
arlakshm Aug 12, 2020
b65012e
For cli get the namespace list from linux
arlakshm Aug 12, 2020
f1814c5
Merge remote-tracking branch 'arlakshm/master' into masic_port_role
arlakshm Aug 13, 2020
f1a4f07
fix import spacing
arlakshm Aug 13, 2020
0b0cfd5
address review comments
arlakshm Aug 13, 2020
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
80 changes: 80 additions & 0 deletions src/sonic-config-engine/sonic_device_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from natsort import natsorted
import glob
from swsssdk import ConfigDBConnector, SonicDBConfig
from portconfig import get_port_config

DOCUMENTATION = '''
---
Expand All @@ -26,6 +27,13 @@
ASIC_CONF_FILENAME = 'asic.conf'
FRONTEND_ASIC_SUB_ROLE = 'FrontEnd'
BACKEND_ASIC_SUB_ROLE = 'BackEnd'
EXTERNAL_PORT = 'E'
INTERNAL_PORT = 'I'
PORT_CHANNEL_CFG_DB_TABLE = 'PORTCHANNEL'
PORT_CFG_DB_TABLE = 'PORT'
BGP_NEIGH_CFG_DB_TABLE = 'BGP_NEIGHBOR'
NEIGH_DEVICE_METADATA_CFG_DB_TABLE = "DEVICE_NEIGHBOR_METADATA"

def get_machine_info():
if not os.path.isfile('/host/machine.conf'):
return None
Expand Down Expand Up @@ -125,6 +133,78 @@ def get_all_namespaces():

return {'front_ns':front_ns, 'back_ns':back_ns}

def get_port_config_from_all_asics():
if not is_multi_npu():
return get_port_config()

platform = get_platform()
hwsku = get_hwsku()
all_ports = {}
ns_list = get_namespaces()

for ns in ns_list:
asic_id = get_npu_id_from_name(ns)
(ports, _, _) = get_port_config(hwsku=hwsku, platform=platform, asic=asic_id)
all_ports.update(ports)

return all_ports

def get_port_role(port_name):
ports_config = get_port_config_from_all_asics()
if not ports_config[port_name].has_key('role'):
return EXTERNAL_PORT

role = ports_config[port_name]['role']
return role

def is_port_internal(port_name):
role = get_port_role(port_name)
if role is INTERNAL_PORT:
return True
return False

def is_port_channel_internal(port_channel, namespace=None):
if not is_multi_npu():
return False
if namespace is None:
ns_list = get_namespaces()
else:
ns_list = [namespace]
Copy link
Contributor

Choose a reason for hiding this comment

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

check of valid namepsace it not already checked. Raise exception if invalid namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, the namespace validation should be done by the caller ( CLI handler)


for ns in ns_list:
config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=ns)
config_db.connect()
#import pdb; pdb.set_trace()
port_channels = config_db.get_table(PORT_CHANNEL_CFG_DB_TABLE)
if not port_channels.has_key(port_channel):
continue
members = port_channels[port_channel]['members']
for member in members:
if is_port_internal(member):
return True
Copy link
Contributor

@smaheshm smaheshm Jul 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This logic will be intensive for 'external' ports. It'll call 'get_port_config_from_all_asics()' for all member ports. Any reason why you are checking all members for 'external' role and only one member for 'internal' role.

will this work:

return is_port_internal(members[0])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the latest commit

return False

def is_bgp_session_internal(bgp_neigh_ip, namespace=None):
if not is_multi_npu():
return False

if namespace is None:
ns_list = get_namespaces()
else:
ns_list = [namespace]
Copy link
Contributor

Choose a reason for hiding this comment

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

validate namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, the namespace validation should be done by the caller ( CLI handler)


for ns in ns_list:
config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=ns)
config_db.connect()
bgp_sessions = config_db.get_table(BGP_NEIGH_CFG_DB_TABLE)
if not bgp_sessions.has_key(bgp_neigh_ip):
continue
bgp_neigh_name = bgp_sessions[bgp_neigh_ip]['name']
neighbor_metadata = config_db.get_table(NEIGH_DEVICE_METADATA_CFG_DB_TABLE)
if neighbor_metadata[bgp_neigh_name]['type'].lower() == 'asic':
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

else:
return False?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest commit

return False

def get_platform_info(machine_info):
if machine_info != None:
if machine_info.has_key('onie_platform'):
Expand Down
18 changes: 9 additions & 9 deletions src/sonic-config-engine/tests/multi_npu_data/sample_port_config-0.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# name lanes alias asic_port_name
Ethernet0 33,34,35,36 Ethernet1/1 Eth0-ASIC0
Ethernet4 29,30,31,32 Ethernet1/2 Eth1-ASIC0
Ethernet8 41,42,43,44 Ethernet1/3 Eth2-ASIC0
Ethernet12 37,38,39,40 Ethernet1/4 Eth3-ASIC0
Ethernet-BP0 13,14,15,16 Ethernet-BP0 Eth4-ASIC0
Ethernet-BP4 17,18,19,20 Ethernet-BP4 Eth5-ASIC0
Ethernet-BP8 21,22,23,24 Ethernet-BP8 Eth6-ASIC0
Ethernet-BP12 25,26,27,28 Ethernet-BP12 Eth7-ASIC0
# name lanes alias asic_port_name role
Ethernet0 33,34,35,36 Ethernet1/1 Eth0-ASIC0 E
Copy link
Contributor

Choose a reason for hiding this comment

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

I talked to Joe, as per him it would be good to add the "index" field also in port_config.ini files. There could be API's which use that.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the usage or significance of index here? There is a way an index is computed based on interface name, which is used in SNMP mainly.
https://github.com/Azure/sonic-py-swsssdk/blob/master/src/swsssdk/port_util.py#L26
Is the interface index compute in port_util same as index here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@judyjoseph, @SuvarnaMeenakshi.
I have added port_index in the port_config.ini in the latest commit. The port index is a running number, there is no significance to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks ! To mention the significance of "index" .. there are some places where the code check for presence of "index" in the port_config.ini eg: https://github.com/Azure/sonic-platform-common/blob/be1cc2475e65b764f7aab44785a779cc3d011ee9/sonic_platform_base/sonic_sfp/sfputilbase.py#L458

So it would be safer to have the index field.... as there are logic below which checks for len(line.split()) >= 4: etc. This will change for us as we add more columns now.

Also we would need to have separate index numbering for front-panel and back panel interfaces ...

Ethernet0 33,34,35,36 Ethernet1/1 0 Eth0-ASIC0 E
Ethernet4 29,30,31,32 Ethernet1/2 1 Eth1-ASIC0 E
...
Ethernet-BP0 13,14,15,16 Ethernet-BP0 0 Eth4-ASIC0 I
Ethernet-BP4 17,18,19,20 Ethernet-BP4 1 Eth5-ASIC0 I
....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also we would need to have separate index numbering for front-panel and back panel interfaces ...

@judyjoseph, can you elaborate why we need separate index numbering for front-panel and back panel interface ?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the platform code ( could be other places as well ) we are making a logical list of all the interfaces, and we are looking for only front panel interfaces which is significant. Currently when there is no "index" field in port_config.ini, the way it calculates the index is from the name "Ethernet0"(here it is 0/4 ==> 0), "Ethernet4" (here it is 4/4 ==> 1) etc.

So It would be ok to start the index from 0 separately per interface type and it will follow the interface naming/numbering convention also. Do you think there could be an issue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@judyjoseph, Changed the port_config.ini in the latest commit

Ethernet4 29,30,31,32 Ethernet1/2 Eth1-ASIC0 E
Ethernet8 41,42,43,44 Ethernet1/3 Eth2-ASIC0 E
Ethernet12 37,38,39,40 Ethernet1/4 Eth3-ASIC0 E
Ethernet-BP0 13,14,15,16 Ethernet-BP0 Eth4-ASIC0 I
Ethernet-BP4 17,18,19,20 Ethernet-BP4 Eth5-ASIC0 I
Ethernet-BP8 21,22,23,24 Ethernet-BP8 Eth6-ASIC0 I
Ethernet-BP12 25,26,27,28 Ethernet-BP12 Eth7-ASIC0 I
18 changes: 9 additions & 9 deletions src/sonic-config-engine/tests/multi_npu_data/sample_port_config-1.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# name lanes alias asic_port_name
Ethernet16 33,34,35,36 Ethernet1/5 Eth0-ASIC1
Ethernet20 29,30,31,32 Ethernet1/6 Eth1-ASIC1
Ethernet24 41,42,43,44 Ethernet1/7 Eth2-ASIC1
Ethernet28 37,38,39,40 Ethernet1/8 Eth3-ASIC1
Ethernet-BP16 13,14,15,16 Ethernet-BP16 Eth4-ASIC1
Ethernet-BP20 17,18,19,20 Ethernet-BP20 Eth5-ASIC1
Ethernet-BP24 21,22,23,24 Ethernet-BP24 Eth6-ASIC1
Ethernet-BP28 25,26,27,28 Ethernet-BP28 Eth7-ASIC1
# name lanes alias asic_port_name role
Ethernet16 33,34,35,36 Ethernet1/5 Eth0-ASIC1 E
Ethernet20 29,30,31,32 Ethernet1/6 Eth1-ASIC1 E
Ethernet24 41,42,43,44 Ethernet1/7 Eth2-ASIC1 E
Ethernet28 37,38,39,40 Ethernet1/8 Eth3-ASIC1 E
Ethernet-BP16 13,14,15,16 Ethernet-BP16 Eth4-ASIC1 I
Ethernet-BP20 17,18,19,20 Ethernet-BP20 Eth5-ASIC1 I
Ethernet-BP24 21,22,23,24 Ethernet-BP24 Eth6-ASIC1 I
Ethernet-BP28 25,26,27,28 Ethernet-BP28 Eth7-ASIC1 I
18 changes: 9 additions & 9 deletions src/sonic-config-engine/tests/multi_npu_data/sample_port_config-2.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# name lanes alias asic_port_name
Ethernet-BP256 61,62,63,64 Ethernet-BP256 Eth0-ASIC2
Ethernet-BP260 57,58,59,60 Ethernet-BP260 Eth1-ASIC2
Ethernet-BP264 53,54,55,56 Ethernet-BP264 Eth2-ASIC2
Ethernet-BP268 49,50,51,52 Ethernet-BP268 Eth3-ASIC2
Ethernet-BP272 45,46,47,48 Ethernet-BP272 Eth4-ASIC2
Ethernet-BP276 41,42,43,44 Ethernet-BP276 Eth5-ASIC2
Ethernet-BP280 37,38,39,40 Ethernet-BP280 Eth6-ASIC2
Ethernet-BP284 33,34,35,36 Ethernet-BP284 Eth7-ASIC2
# name lanes alias asic_port_name role
Ethernet-BP256 61,62,63,64 Ethernet-BP256 Eth0-ASIC2 I
Ethernet-BP260 57,58,59,60 Ethernet-BP260 Eth1-ASIC2 I
Ethernet-BP264 53,54,55,56 Ethernet-BP264 Eth2-ASIC2 I
Ethernet-BP268 49,50,51,52 Ethernet-BP268 Eth3-ASIC2 I
Ethernet-BP272 45,46,47,48 Ethernet-BP272 Eth4-ASIC2 I
Ethernet-BP276 41,42,43,44 Ethernet-BP276 Eth5-ASIC2 I
Ethernet-BP280 37,38,39,40 Ethernet-BP280 Eth6-ASIC2 I
Ethernet-BP284 33,34,35,36 Ethernet-BP284 Eth7-ASIC2 I
18 changes: 9 additions & 9 deletions src/sonic-config-engine/tests/multi_npu_data/sample_port_config-3.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# name lanes alias asic_port_name
Ethernet-BP384 29,30,31,32 Ethernet-BP384 Eth0-ASIC3
Ethernet-BP388 25,26,27,28 Ethernet-BP388 Eth1-ASIC3
Ethernet-BP392 21,22,23,24 Ethernet-BP392 Eth2-ASIC3
Ethernet-BP396 17,18,19,20 Ethernet-BP396 Eth3-ASIC3
Ethernet-BP400 13,14,15,16 Ethernet-BP400 Eth4-ASIC3
Ethernet-BP404 9,10,11,12 Ethernet-BP404 Eth5-ASIC3
Ethernet-BP408 5,6,7,8 Ethernet-BP408 Eth6-ASIC3
Ethernet-BP412 1,2,3,4 Ethernet-BP412 Eth7-ASIC3
# name lanes alias asic_port_name role
Ethernet-BP384 29,30,31,32 Ethernet-BP384 Eth0-ASIC3 I
Ethernet-BP388 25,26,27,28 Ethernet-BP388 Eth1-ASIC3 I
Ethernet-BP392 21,22,23,24 Ethernet-BP392 Eth2-ASIC3 I
Ethernet-BP396 17,18,19,20 Ethernet-BP396 Eth3-ASIC3 I
Ethernet-BP400 13,14,15,16 Ethernet-BP400 Eth4-ASIC3 I
Ethernet-BP404 9,10,11,12 Ethernet-BP404 Eth5-ASIC3 I
Ethernet-BP408 5,6,7,8 Ethernet-BP408 Eth6-ASIC3 I
Ethernet-BP412 1,2,3,4 Ethernet-BP412 Eth7-ASIC3 I
13 changes: 13 additions & 0 deletions src/sonic-config-engine/tests/test_multinpu_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,19 @@ def test_backend_asic_portchannel_intf(self):
self.assertListEqual(output.keys(), \
['PortChannel4013', 'PortChannel4013|10.1.0.2/31', 'PortChannel4014', 'PortChannel4014|10.1.0.6/31'])

def test_frontend_asic_ports(self):
argument = "-m {} -p {} -n asic0 --var-json \"PORT\"".format(self.sample_graph, self.port_config[0])
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, \
{
"Ethernet0": {"admin_status": "up", "alias": "Ethernet1/1", "asic_port_name": "Eth0-ASIC0", "description": "01T2:Ethernet1", "lanes": "33,34,35,36", "mtu": "9100", "pfc_asym": "off", "role": "E", "speed": "40000" },
"Ethernet4": {"admin_status": "up", "alias": "Ethernet1/2", "asic_port_name": "Eth1-ASIC0", "description": "01T2:Ethernet2", "lanes": "29,30,31,32", "mtu": "9100", "pfc_asym": "off", "role": "E", "speed": "40000" },
"Ethernet8": {"alias": "Ethernet1/3", "asic_port_name": "Eth2-ASIC0", "description": "Ethernet1/3", "lanes": "41,42,43,44", "mtu": "9100", "pfc_asym": "off", "role": "E", "speed": "40000"},
"Ethernet12": {"alias": "Ethernet1/4", "asic_port_name": "Eth3-ASIC0", "description": "Ethernet1/4", "lanes": "37,38,39,40", "mtu": "9100", "pfc_asym": "off", "role": "E", "speed": "40000"},
"Ethernet-BP0": {"admin_status": "up", "alias": "Ethernet-BP0", "asic_port_name": "Eth4-ASIC0", "description": "ASIC2:Eth0-ASIC2", "lanes": "13,14,15,16", "mtu": "9100", "pfc_asym": "off", "role": "I", "speed": "40000"},
"Ethernet-BP4": {"admin_status": "up", "alias": "Ethernet-BP4", "asic_port_name": "Eth5-ASIC0", "description": "ASIC2:Eth1-ASIC2", "lanes": "17,18,19,20", "mtu": "9100", "pfc_asym": "off", "role": "I", "speed": "40000"},
"Ethernet-BP8": {"admin_status": "up", "alias": "Ethernet-BP8", "asic_port_name": "Eth6-ASIC0", "description": "ASIC3:Eth0-ASIC3", "lanes": "21,22,23,24", "mtu": "9100", "pfc_asym": "off", "role": "I", "speed": "40000"},
"Ethernet-BP12": {"admin_status": "up", "alias": "Ethernet-BP12", "asic_port_name": "Eth7-ASIC0", "description": "ASIC3:Eth1-ASIC3", "lanes": "25,26,27,28", "mtu": "9100", "pfc_asym": "off", "role": "I", "speed": "40000"}})
def test_frontend_asic_device_neigh(self):
argument = "-m {} -p {} -n asic0 --var-json \"DEVICE_NEIGHBOR\"".format(self.sample_graph, self.port_config[0])
output = json.loads(self.run_script(argument))
Expand Down