From 4e48bfac5cf168945a1027e98de20fcf7d7bd161 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 12 Apr 2018 03:25:08 +0000 Subject: [PATCH 1/5] [minigraph] Fix parser on PNG DeviceInterfaceLink Bandwidth Signed-off-by: Qi Luo --- src/sonic-config-engine/minigraph.py | 24 +++++++++++++++---- .../tests/sample_output/ports.json | 2 +- .../tests/simple-sample-graph.xml | 14 ++++++++++- src/sonic-config-engine/tests/test_cfggen.py | 2 +- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 570f5de24188..e838bb89ce75 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -62,6 +62,7 @@ def parse_png(png, hname): console_port = '' mgmt_dev = '' mgmt_port = '' + port_speeds = {} for child in png: if child.tag == str(QName(ns, "DeviceInterfaceLinks")): for link in child.findall(str(QName(ns, "DeviceLinkBase"))): @@ -73,12 +74,18 @@ def parse_png(png, hname): endport = link.find(str(QName(ns, "EndPort"))).text startdevice = link.find(str(QName(ns, "StartDevice"))).text startport = link.find(str(QName(ns, "StartPort"))).text + bandwidth_node = link.find(str(QName(ns, "Bandwidth"))) + bandwidth = bandwidth_node.text if bandwidth_node is not None else None if enddevice == hname: + if bandwidth: + port_speeds[endport] = bandwidth if port_alias_map.has_key(endport): endport = port_alias_map[endport] neighbors[endport] = {'name': startdevice, 'port': startport} else: + if bandwidth: + port_speeds[startport] = bandwidth if port_alias_map.has_key(startport): startport = port_alias_map[startport] neighbors[startport] = {'name': enddevice, 'port': endport} @@ -106,7 +113,7 @@ def parse_png(png, hname): elif node.tag == str(QName(ns, "EndDevice")): mgmt_dev = node.text - return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port) + return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds) def parse_dpg(dpg, hname): @@ -369,6 +376,7 @@ def parse_xml(filename, platform=None, port_config_file=None): devices = None hostname = None port_speeds = {} + port_speeds1 = {} port_descriptions = {} syslog_servers = [] dhcp_servers = [] @@ -395,9 +403,9 @@ def parse_xml(filename, platform=None, port_config_file=None): elif child.tag == str(QName(ns, "CpgDec")): (bgp_sessions, bgp_asn, bgp_peers_with_range) = parse_cpg(child, hostname) elif child.tag == str(QName(ns, "PngDec")): - (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port) = parse_png(child, hostname) + (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds1) = parse_png(child, hostname) elif child.tag == str(QName(ns, "UngDec")): - (u_neighbors, u_devices, _, _, _, _) = parse_png(child, hostname) + (u_neighbors, u_devices, _, _, _, _, _) = parse_png(child, hostname) elif child.tag == str(QName(ns, "MetadataDeclaration")): (syslog_servers, dhcp_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id) = parse_meta(child, hostname) elif child.tag == str(QName(ns, "DeviceInfos")): @@ -444,8 +452,14 @@ def parse_xml(filename, platform=None, port_config_file=None): continue ports.setdefault(port_name, {})['speed'] = port_speeds[port_name] - if port_speeds[port_name] == '100000': - ports.setdefault(port_name, {})['fec'] = 'rs' + for port_name in port_speeds1: + ports.setdefault(port_name, {})['speed'] = port_speeds1[port_name] + for port_name, port in ports.items(): + if port.get('speed') == '100000': + port['fec'] = 'rs' + for port_name in port_descriptions: + ports.setdefault(port_name, {})['description'] = port_descriptions[port_name] + for port_name in port_descriptions: # ignore port not in port_config.ini if not ports.has_key(port_name): diff --git a/src/sonic-config-engine/tests/sample_output/ports.json b/src/sonic-config-engine/tests/sample_output/ports.json index 7533fb7377ef..3fa2e29a8b0a 100644 --- a/src/sonic-config-engine/tests/sample_output/ports.json +++ b/src/sonic-config-engine/tests/sample_output/ports.json @@ -1,7 +1,7 @@ [ { "PORT_TABLE:Ethernet8": { - "speed": "40000", + "speed": "1000", "description": "Interface description" }, "OP": "SET" diff --git a/src/sonic-config-engine/tests/simple-sample-graph.xml b/src/sonic-config-engine/tests/simple-sample-graph.xml index 33e8a6d616d9..e2240905a297 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph.xml @@ -181,7 +181,19 @@ - + + + DeviceInterfaceLink + true + 1000 + ARISTA01T1 + et1 + true + switch-t0 + Ethernet8 + true + + switch-t0 diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index fbec671aff56..25c4d9cc2cf9 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -148,7 +148,7 @@ def test_minigraph_deployment_id(self): def test_minigraph_ethernet_interfaces(self): argument = '-m "' + self.sample_graph_simple + '" -p "' + self.port_config + '" -v "PORT[\'Ethernet8\']"' output = self.run_script(argument) - self.assertEqual(output.strip(), "{'alias': 'fortyGigE0/8', 'lanes': '37,38,39,40', 'description': 'Interface description', 'speed': '40000'}") + self.assertEqual(output.strip(), "{'alias': 'fortyGigE0/8', 'lanes': '37,38,39,40', 'description': 'Interface description', 'speed': '1000'}") argument = '-m "' + self.sample_graph_simple + '" -p "' + self.port_config + '" -v "PORT[\'Ethernet12\']"' output = self.run_script(argument) self.assertEqual(output.strip(), "{'alias': 'fortyGigE0/12', 'lanes': '33,34,35,36', 'fec': 'rs', 'speed': '100000', 'description': 'Interface description'}") From 54a878a77e546ceaf22ebea6ec9d91728debe5d2 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 12 Apr 2018 19:19:47 +0000 Subject: [PATCH 2/5] Fix test case due to rebase Signed-off-by: Qi Luo --- src/sonic-config-engine/tests/test_cfggen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 25c4d9cc2cf9..901b882f7048 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -157,7 +157,7 @@ def test_minigraph_extra_ethernet_interfaces(self): argument = '-m "' + self.sample_graph_simple + '" -p "' + self.port_config + '" -v "PORT"' output = self.run_script(argument) self.assertEqual(output.strip(), \ - "{'Ethernet8': {'alias': 'fortyGigE0/8', 'lanes': '37,38,39,40', 'description': 'Interface description', 'speed': '40000'}, " + "{'Ethernet8': {'alias': 'fortyGigE0/8', 'lanes': '37,38,39,40', 'description': 'Interface description', 'speed': '1000'}, " "'Ethernet0': {'alias': 'fortyGigE0/0', 'lanes': '29,30,31,32', 'speed': '10000'}, " "'Ethernet4': {'alias': 'fortyGigE0/4', 'lanes': '25,26,27,28', 'speed': '25000'}, " "'Ethernet108': {'alias': 'fortyGigE0/108', 'lanes': '81,82,83,84'}, " From 8e0e2d620a65ccca791f1e48320f419740e06d72 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 12 Apr 2018 19:23:36 +0000 Subject: [PATCH 3/5] Rename variables Signed-off-by: Qi Luo --- src/sonic-config-engine/minigraph.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index e838bb89ce75..d298c52b206a 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -375,8 +375,8 @@ def parse_xml(filename, platform=None, port_config_file=None): neighbors = None devices = None hostname = None - port_speeds = {} - port_speeds1 = {} + port_speeds_default = {} + port_speed_png = {} port_descriptions = {} syslog_servers = [] dhcp_servers = [] @@ -403,13 +403,13 @@ def parse_xml(filename, platform=None, port_config_file=None): elif child.tag == str(QName(ns, "CpgDec")): (bgp_sessions, bgp_asn, bgp_peers_with_range) = parse_cpg(child, hostname) elif child.tag == str(QName(ns, "PngDec")): - (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds1) = parse_png(child, hostname) + (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speed_png) = parse_png(child, hostname) elif child.tag == str(QName(ns, "UngDec")): (u_neighbors, u_devices, _, _, _, _, _) = parse_png(child, hostname) elif child.tag == str(QName(ns, "MetadataDeclaration")): (syslog_servers, dhcp_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id) = parse_meta(child, hostname) elif child.tag == str(QName(ns, "DeviceInfos")): - (port_speeds, port_descriptions) = parse_deviceinfo(child, hwsku) + (port_speeds_default, port_descriptions) = parse_deviceinfo(child, hwsku) results = {} results['DEVICE_METADATA'] = {'localhost': { @@ -446,14 +446,14 @@ def parse_xml(filename, platform=None, port_config_file=None): results['VLAN_INTERFACE'] = vlan_intfs results['PORTCHANNEL_INTERFACE'] = pc_intfs - for port_name in port_speeds: + for port_name in port_speeds_default: # ignore port not in port_config.ini if not ports.has_key(port_name): continue - ports.setdefault(port_name, {})['speed'] = port_speeds[port_name] - for port_name in port_speeds1: - ports.setdefault(port_name, {})['speed'] = port_speeds1[port_name] + ports.setdefault(port_name, {})['speed'] = port_speeds_default[port_name] + for port_name in port_speed_png: + ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] for port_name, port in ports.items(): if port.get('speed') == '100000': port['fec'] = 'rs' From 3ebeddee568635340856dce36c10fd63996be416 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 12 Apr 2018 19:32:34 +0000 Subject: [PATCH 4/5] Fix StartPort name in test case Signed-off-by: Qi Luo --- src/sonic-config-engine/minigraph.py | 8 ++++---- src/sonic-config-engine/tests/simple-sample-graph.xml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index d298c52b206a..5423ee9d2825 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -78,17 +78,17 @@ def parse_png(png, hname): bandwidth = bandwidth_node.text if bandwidth_node is not None else None if enddevice == hname: - if bandwidth: - port_speeds[endport] = bandwidth if port_alias_map.has_key(endport): endport = port_alias_map[endport] neighbors[endport] = {'name': startdevice, 'port': startport} - else: if bandwidth: - port_speeds[startport] = bandwidth + port_speeds[endport] = bandwidth + else: if port_alias_map.has_key(startport): startport = port_alias_map[startport] neighbors[startport] = {'name': enddevice, 'port': endport} + if bandwidth: + port_speeds[startport] = bandwidth if child.tag == str(QName(ns, "Devices")): for device in child.findall(str(QName(ns, "Device"))): diff --git a/src/sonic-config-engine/tests/simple-sample-graph.xml b/src/sonic-config-engine/tests/simple-sample-graph.xml index e2240905a297..6e351e1cd003 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph.xml @@ -190,7 +190,7 @@ et1 true switch-t0 - Ethernet8 + fortyGigE0/8 true From 98d79c5720055ad53d23876299986c061bb9c5d5 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 12 Apr 2018 19:57:58 +0000 Subject: [PATCH 5/5] Remove redundant code, add comment Signed-off-by: Qi Luo --- src/sonic-config-engine/minigraph.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 5423ee9d2825..4ac7e9d68c92 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -452,13 +452,16 @@ def parse_xml(filename, platform=None, port_config_file=None): continue ports.setdefault(port_name, {})['speed'] = port_speeds_default[port_name] + for port_name in port_speed_png: + # if port_name is not in port_config.ini, still consider it. + # and later swss will pick up and behave on-demand port break-up. + # if on-deman port break-up is not supported on a specific platform, swss will return error. ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] + for port_name, port in ports.items(): if port.get('speed') == '100000': port['fec'] = 'rs' - for port_name in port_descriptions: - ports.setdefault(port_name, {})['description'] = port_descriptions[port_name] for port_name in port_descriptions: # ignore port not in port_config.ini