Skip to content

Minigraph ECMP parsing support (cleaner format) #4985

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

Merged
abdosi merged 2 commits into from
Dec 30, 2020
Merged

Minigraph ECMP parsing support (cleaner format) #4985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
940dd94
Minigraph ECMP parsing support (cleaner format)
kktheballer Dec 23, 2020
13787ee
Adding revised test cases and changing test minigraph.xml content
kktheballer Dec 23, 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
162 changes: 153 additions & 9 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from lxml import etree as ET
from lxml.etree import QName


from portconfig import get_port_config
from sonic_py_common.multi_asic import get_asic_id_from_name
from sonic_py_common.interface import backplane_prefix
Expand Down Expand Up @@ -105,7 +106,73 @@ def parse_device(device):

return (lo_prefix, lo_prefix_v6, mgmt_prefix, name, hwsku, d_type, deployment_id)

def parse_png(png, hname):
def calculate_lcm_for_ecmp (nhdevices_bank_map, nhip_bank_map):
banks_enumerated = {}
lcm_array = []
for value in nhdevices_bank_map.values():
Copy link
Copy Markdown
Contributor

@anish-n anish-n Sep 10, 2020

Choose a reason for hiding this comment

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

IPv4 and IPv6 could have a different enumeration of banks and as a result a different lcm. I would also suggest moving this calculation of lcm to a helper function.

Copy link
Copy Markdown
Contributor Author

@kktheballer kktheballer Sep 11, 2020

Choose a reason for hiding this comment

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

ok will do this

for key in nhip_bank_map.keys():
if nhip_bank_map[key] == value:
if value not in banks_enumerated:
banks_enumerated[value] = 1
else:
banks_enumerated[value] = banks_enumerated[value] + 1
for bank_enumeration in banks_enumerated.values():
lcm_list = range(1, bank_enumeration+1)
lcm_comp = lcm_list[0]
for i in lcm_list[1:]:
lcm_comp = lcm_comp * i / calculate_gcd(lcm_comp, i)
lcm_array.append(lcm_comp)

LCM = sum(lcm_array)
return int(LCM)

def calculate_gcd(x, y):
while y != 0:
(x, y) = (y, x % y)
return int(x)

def formulate_fine_grained_ecmp(version, dpg_ecmp_content, port_device_map, port_alias_map):
family = ""
tag = ""
neigh_key = []
if version == "ipv4":
family = "IPV4"
tag = "fgnhg_v4"
elif version == "ipv6":
family = "IPV6"
tag = "fgnhg_v6"

port_nhip_map = dpg_ecmp_content['port_nhip_map']
nhgaddr = dpg_ecmp_content['nhgaddr']
nhg_int = dpg_ecmp_content['nhg_int']

nhip_device_map = {port_nhip_map[x]: port_device_map[x] for x in port_device_map
if x in port_nhip_map}
nhip_devices = sorted(list(set(nhip_device_map.values())))
nhdevices_ip_bank_map = {device: bank for bank, device in enumerate(nhip_devices)}
nhip_bank_map = {ip: nhdevices_ip_bank_map[device] for ip, device in nhip_device_map.items()}
LCM = calculate_lcm_for_ecmp(nhdevices_ip_bank_map, nhip_bank_map)

FG_NHG_MEMBER = {ip: {"FG_NHG": tag, "bank": bank} for ip, bank in nhip_bank_map.items()}
nhip_port_map = dict(zip(port_nhip_map.values(), port_nhip_map.keys()))



for nhip, memberinfo in FG_NHG_MEMBER.items():
if nhip in nhip_port_map:
memberinfo["link"] = port_alias_map[nhip_port_map[nhip]]
FG_NHG_MEMBER[nhip] = memberinfo

FG_NHG_PREFIX = {nhgaddr: {"FG_NHG": tag}}
FG_NHG = {tag: {"bucket_size": LCM}}
for ip in nhip_bank_map:
neigh_key.append(str(nhg_int + "|" + ip))
NEIGH = {neigh_key: {"family": family} for neigh_key in neigh_key}

fine_grained_content = {"FG_NHG_MEMBER": FG_NHG_MEMBER, "FG_NHG": FG_NHG, "FG_NHG_PREFIX": FG_NHG_PREFIX, "NEIGH": NEIGH}
return fine_grained_content

def parse_png(png, hname, dpg_ecmp_content = None):
neighbors = {}
devices = {}
console_dev = ''
Expand All @@ -116,6 +183,13 @@ def parse_png(png, hname):
console_ports = {}
mux_cable_ports = {}
is_storage_device = False
port_device_map = {}
png_ecmp_content = {}
FG_NHG_MEMBER = {}
FG_NHG_PREFIX = {}
FG_NHG = {}
NEIGH = {}

for child in png:
if child.tag == str(QName(ns, "DeviceInterfaceLinks")):
for link in child.findall(str(QName(ns, "DeviceLinkBase"))):
Expand All @@ -141,6 +215,11 @@ def parse_png(png, hname):
}
continue

if linktype == "DeviceInterfaceLink":
endport = link.find(str(QName(ns, "EndPort"))).text
startdevice = link.find(str(QName(ns, "StartDevice"))).text
port_device_map[endport] = startdevice

Copy link
Copy Markdown
Contributor

@anish-n anish-n Sep 8, 2020

Choose a reason for hiding this comment

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

Suggest moving this logic to line 211, and utilizing the endport and startdevice values already generated on line 194 and 195.

Copy link
Copy Markdown
Contributor Author

@kktheballer kktheballer Sep 11, 2020

Choose a reason for hiding this comment

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

I believe I had written like this to work around the already in-place lines of code in 190-191

if linktype != "DeviceInterfaceLink" and linktype != "UnderlayInterfaceLink":
continue

Expand Down Expand Up @@ -196,6 +275,7 @@ def parse_png(png, hname):
elif node.tag == str(QName(ns, "EndDevice")):
mgmt_dev = node.text


if child.tag == str(QName(ns, "DeviceInterfaceLinks")):
for link in child.findall(str(QName(ns, 'DeviceLinkBase'))):
if link.find(str(QName(ns, "ElementType"))).text == "LogicalLink":
Expand All @@ -205,7 +285,19 @@ def parse_png(png, hname):

mux_cable_ports[intf_name] = "true"

return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds, console_ports, mux_cable_ports, is_storage_device)
if (len(dpg_ecmp_content)):
for version, content in dpg_ecmp_content.items(): # version is ipv4 or ipv6
fine_grained_content = formulate_fine_grained_ecmp(version, content, port_device_map, port_alias_map) # port_alias_map
FG_NHG_MEMBER.update(fine_grained_content['FG_NHG_MEMBER'])
FG_NHG_PREFIX.update(fine_grained_content['FG_NHG_PREFIX'])
FG_NHG.update(fine_grained_content['FG_NHG'])
NEIGH.update(fine_grained_content['NEIGH'])

png_ecmp_content = {"FG_NHG_PREFIX": FG_NHG_PREFIX, "FG_NHG_MEMBER": FG_NHG_MEMBER, "FG_NHG": FG_NHG,
"NEIGH": NEIGH}

return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds, console_ports, mux_cable_ports, is_storage_device, png_ecmp_content)


def parse_asic_external_link(link, asic_name, hostname):
neighbors = {}
Expand Down Expand Up @@ -295,6 +387,8 @@ def parse_asic_png(png, asic_name, hostname):
if lo_prefix_v6:
device_data['lo_addr_v6']= lo_prefix_v6
devices[name] = device_data


return (neighbors, devices, port_speeds)

def parse_loopback_intf(child):
Expand Down Expand Up @@ -339,12 +433,13 @@ def parse_dpg(dpg, hname):

ipintfs = child.find(str(QName(ns, "IPInterfaces")))
intfs = {}
ip_intfs_map = {}
for ipintf in ipintfs.findall(str(QName(ns, "IPInterface"))):
intfalias = ipintf.find(str(QName(ns, "AttachTo"))).text
intfname = port_alias_map.get(intfalias, intfalias)
ipprefix = ipintf.find(str(QName(ns, "Prefix"))).text
intfs[(intfname, ipprefix)] = {}

ip_intfs_map[ipprefix] = intfalias
lo_intfs = parse_loopback_intf(child)

mvrfConfigs = child.find(str(QName(ns, "MgmtVrfConfigs")))
Expand Down Expand Up @@ -381,7 +476,46 @@ def parse_dpg(dpg, hname):
pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75)))}
else:
pcs[pcintfname] = {'members': pcmbr_list, 'min_links': str(int(math.ceil(len(pcmbr_list) * 0.75)))}

port_nhipv4_map = {}
port_nhipv6_map = {}
nhgaddr = ["", ""]
nhg_int = ""
nhportlist = []
dpg_ecmp_content = {}
ipnhs = child.find(str(QName(ns, "IPNextHops")))
if ipnhs is not None:
for ipnh in ipnhs.findall(str(QName(ns, "IPNextHop"))):
Copy link
Copy Markdown
Contributor

@anish-n anish-n Sep 8, 2020

Choose a reason for hiding this comment

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

Lets add a check for IPNextHop type FineGrainedECMPGroupMember

if ipnh.find(str(QName(ns, "Type"))).text == 'FineGrainedECMPGroupMember':
ipnhfmbr = ipnh.find(str(QName(ns, "AttachTo"))).text
ipnhaddr = ipnh.find(str(QName(ns, "Address"))).text
nhportlist.append(ipnhfmbr)
if "." in ipnhaddr:
port_nhipv4_map[ipnhfmbr] = ipnhaddr
elif ":" in ipnhaddr:
port_nhipv6_map[ipnhfmbr] = ipnhaddr

if port_nhipv4_map is not None and port_nhipv6_map is not None:
subnet_check_ip = list(port_nhipv4_map.values())[0]
for subnet_range in ip_intfs_map:
if ("." in subnet_range):
a = ipaddress.ip_address(UNICODE_TYPE(subnet_check_ip))
n = list(ipaddress.ip_network(UNICODE_TYPE(subnet_range), False).hosts())
if a in n:
nhg_int = ip_intfs_map[subnet_range]
Copy link
Copy Markdown
Contributor

@anish-n anish-n Dec 11, 2020

Choose a reason for hiding this comment

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

Assumes a single interface for all IPs, though it may be addressed in a future PR: it is better to not to assume single interface associated with all IPs.

dwnstrms = child.find(str(QName(ns, "DownstreamSummarySet")))
for dwnstrm in dwnstrms.findall(str(QName(ns, "DownstreamSummary"))):
dwnstrmentry = str(ET.tostring(dwnstrm))
if ("FineGrainedECMPGroupDestination" in dwnstrmentry):
subnet_ip = dwnstrm.find(str(QName(ns1, "Subnet"))).text
truncsubnet_ip = subnet_ip.split("/")[0]
Copy link
Copy Markdown
Contributor

@anish-n anish-n Nov 20, 2020

Choose a reason for hiding this comment

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

Why do we need to truncate it here for the IPv4/IPv6 check?

if "." in (truncsubnet_ip):
nhgaddr[0] = subnet_ip
elif ":" in (truncsubnet_ip):
nhgaddr[1] = subnet_ip
ipv4_content = {"port_nhip_map": port_nhipv4_map, "nhgaddr": nhgaddr[0], "nhg_int": nhg_int}
ipv6_content = {"port_nhip_map": port_nhipv6_map, "nhgaddr": nhgaddr[1], "nhg_int": nhg_int}
dpg_ecmp_content['ipv4'] = ipv4_content
dpg_ecmp_content['ipv6'] = ipv6_content
vlanintfs = child.find(str(QName(ns, "VlanInterfaces")))
vlan_intfs = []
vlans = {}
Expand Down Expand Up @@ -503,6 +637,7 @@ def parse_dpg(dpg, hname):
except:
print("Warning: Ignoring Control Plane ACL %s without type" % aclname, file=sys.stderr)


mg_tunnels = child.find(str(QName(ns, "TunnelInterfaces")))
if mg_tunnels is not None:
table_key_to_mg_key_map = {"encap_ecn_mode": "EcnEncapsulationMode",
Expand All @@ -521,7 +656,7 @@ def parse_dpg(dpg, hname):
if mg_key in mg_tunnel.attrib:
tunnelintfs[tunnel_type][tunnel_name][table_key] = mg_tunnel.attrib[mg_key]

return intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnelintfs
return intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnelintfs, dpg_ecmp_content
return None, None, None, None, None, None, None, None, None, None

def parse_host_loopback(dpg, hname):
Expand Down Expand Up @@ -927,6 +1062,8 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
bgp_monitors = []
bgp_asn = None
intfs = None
dpg_ecmp_content = {}
png_ecmp_content = {}
vlan_intfs = None
pc_intfs = None
tunnel_intfs = None
Expand Down Expand Up @@ -988,13 +1125,13 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
for child in root:
if asic_name is None:
if child.tag == str(QName(ns, "DpgDec")):
(intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnel_intfs) = parse_dpg(child, hostname)
(intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnel_intfs, dpg_ecmp_content) = parse_dpg(child, hostname)
elif child.tag == str(QName(ns, "CpgDec")):
(bgp_sessions, bgp_internal_sessions, bgp_asn, bgp_peers_with_range, bgp_monitors) = parse_cpg(child, hostname)
elif child.tag == str(QName(ns, "PngDec")):
(neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speed_png, console_ports, mux_cable_ports, is_storage_device) = parse_png(child, hostname)
(neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speed_png, console_ports, mux_cable_ports, is_storage_device, png_ecmp_content) = parse_png(child, hostname, dpg_ecmp_content)
elif child.tag == str(QName(ns, "UngDec")):
(u_neighbors, u_devices, _, _, _, _, _, _) = parse_png(child, hostname)
(u_neighbors, u_devices, _, _, _, _, _, _) = parse_png(child, hostname, None)
elif child.tag == str(QName(ns, "MetadataDeclaration")):
(syslog_servers, dhcp_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype, resource_type) = parse_meta(child, hostname)
elif child.tag == str(QName(ns, "LinkMetadataDeclaration")):
Expand All @@ -1003,7 +1140,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
(port_speeds_default, port_descriptions) = parse_deviceinfo(child, hwsku)
else:
if child.tag == str(QName(ns, "DpgDec")):
(intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnel_intfs) = parse_dpg(child, asic_name)
(intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni, tunnel_intfs, dpg_ecmp_content) = parse_dpg(child, asic_name)
host_lo_intfs = parse_host_loopback(child, hostname)
elif child.tag == str(QName(ns, "CpgDec")):
(bgp_sessions, bgp_internal_sessions, bgp_asn, bgp_peers_with_range, bgp_monitors) = parse_cpg(child, asic_name, local_devices)
Expand Down Expand Up @@ -1305,6 +1442,13 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
'client_crt_cname': 'client.restapi.sonic'
}
}

if len(png_ecmp_content):
results['FG_NHG_MEMBER'] = png_ecmp_content['FG_NHG_MEMBER']
results['FG_NHG_PREFIX'] = png_ecmp_content['FG_NHG_PREFIX']
results['FG_NHG'] = png_ecmp_content['FG_NHG']
results['NEIGH'] = png_ecmp_content['NEIGH']

# Do not configure the minigraph's mirror session, which is currently unused
# mirror_sessions = {}
# if erspan_dst:
Expand Down
Loading