Skip to content
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
206 changes: 139 additions & 67 deletions src/sonic-config-engine/portconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
INTF_KEY = "interfaces"
OPTIONAL_HWSKU_ATTRIBUTES = ["fec", "autoneg"]

BRKOUT_PATTERN = r'(\d{1,3})x(\d{1,3}G)(\[(\d{1,3}G,?)*\])?(\((\d{1,3})\))?'
BRKOUT_PATTERN = r'(\d{1,6})x(\d{1,6}G?)(\[(\d{1,6}G?,?)*\])?(\((\d{1,6})\))?'
Copy link
Collaborator

@qiluo-msft qiluo-msft Dec 16, 2021

Choose a reason for hiding this comment

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

G?

Why allow missing G? What is the default unit? Is there a document for the default unit? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To allow to configure speed in megabytes:

                "1x100000[50G,40000,25000,10G]": ["Eth36"],
                "2x50G": ["Eth36/1", "Eth36/2"],
                "4x25G[10000]": ["Eth36/1", "Eth36/2", "Eth36/3", "Eth36/4"],
                "2x25000(2)+1x50G(2)": ["Eth36/1", "Eth36/2", "Eth36/3"],
                "1x50000(2)+2x25G(2)": ["Eth36/1", "Eth36/2", "Eth36/3"]

BRKOUT_PATTERN_GROUPS = 6

#
# Helper Functions
Expand Down Expand Up @@ -172,85 +173,156 @@ def parse_port_config_file(port_config_file):
port_alias_asic_map[data['alias']] = data['asic_port_name'].strip()
return (ports, port_alias_map, port_alias_asic_map)

# Generate configs (i.e. alias, lanes, speed, index) for port
def gen_port_config(ports, parent_intf_id, index, alias_list, lanes, k, offset):
if k is not None:
num_lane_used, speed, alt_speed, _, _ , assigned_lane = k[0], k[1], k[2], k[3], k[4], k[5]

# In case of symmetric mode
if assigned_lane is None:
assigned_lane = len(lanes.split(","))

parent_intf_id = int(offset)+int(parent_intf_id)
alias_position = 0 + int(offset)//int(num_lane_used)
lanes_start = 0 + int(offset)

step = int(assigned_lane)//int(num_lane_used)
for i in range(0,int(assigned_lane), step):
intf_name = PORT_STR + str(parent_intf_id)
ports[intf_name] = {}
ports[intf_name]['alias'] = alias_list[alias_position]
ports[intf_name]['lanes'] = ','.join(lanes.split(",")[lanes_start:lanes_start+step])
if speed:
speed_pat = re.search("^((\d+)G|\d+)$", speed.upper())
if speed_pat is None:
raise Exception('{} speed is not Supported...'.format(speed))
speed_G, speed_orig = speed_pat.group(2), speed_pat.group(1)
if speed_G:
conv_speed = int(speed_G)*1000
else:
conv_speed = int(speed_orig)
ports[intf_name]['speed'] = str(conv_speed)
class BreakoutCfg(object):

class BreakoutModeEntry:
def __init__(self, num_ports, default_speed, supported_speed, num_assigned_lanes=None):
self.num_ports = int(num_ports)
self.default_speed = self._speed_to_int(default_speed)
self.supported_speed = set((self.default_speed, ))
self._parse_supported_speed(supported_speed)
self.num_assigned_lanes = self._parse_num_assigned_lanes(num_assigned_lanes)

@classmethod
def _speed_to_int(cls, speed):
try:
if speed.endswith('G'):
return int(speed.replace('G', '')) * 1000

return int(speed)
except ValueError:
raise RuntimeError("Unsupported speed format '{}'".format(speed))

def _parse_supported_speed(self, speed):
if not speed:
return

if not speed.startswith('[') and not speed.endswith(']'):
raise RuntimeError("Unsupported port breakout format!")

for s in speed[1:-1].split(','):
self.supported_speed.add(self._speed_to_int(s.strip()))

def _parse_num_assigned_lanes(self, num_assigned_lanes):
if not num_assigned_lanes:
return

if isinstance(num_assigned_lanes, int):
return num_assigned_lanes

if not num_assigned_lanes.startswith('(') and not num_assigned_lanes.endswith(')'):
raise RuntimeError("Unsupported port breakout format!")

return int(num_assigned_lanes[1:-1])

def __eq__(self, other):
if isinstance(other, BreakoutCfg.BreakoutModeEntry):
if self.num_ports != other.num_ports:
return False
if self.supported_speed != other.supported_speed:
return False
if self.num_assigned_lanes != other.num_assigned_lanes:
return False
return True
else:
raise Exception('Regex return for speed is None...')
return False

ports[intf_name]['index'] = index.split(",")[alias_position]
def __ne__(self, other):
return not self == other

parent_intf_id += step
alias_position += 1
lanes_start += step
def __hash__(self):
return hash((self.num_ports, tuple(self.supported_speed), self.num_assigned_lanes))
Copy link
Collaborator

Choose a reason for hiding this comment

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

what exactly are we checking here?

Copy link
Collaborator Author

@oleksandrivantsiv oleksandrivantsiv Dec 15, 2021

Choose a reason for hiding this comment

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

BreakoutModeEntry objects are used to compare breakout modes based on their capabilities with __eq__ and __ne__ operators.
In Python when the class implements __eq__ operator it is recommended to implement __hash__ operator as well. __hash__ operator returns integer hash id of the object that can be used to put an object in a dictionary (as a key) or set. Here hash id is calculated based on capabilities that define breakout mode.

My initial implementation didn't include this method implementation. I added to fix LGTM alerts:

This pull request introduces 2 alerts when merging 04be47c into fc91dae - view on LGTM.com

new alerts:

1 for Inconsistent equality and inequality
1 for Inconsistent equality and hashing

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the clarification, @oleksandrivantsiv.


def __init__(self, name, bmode, properties):
self._interface_base_id = int(name.replace(PORT_STR, ''))
self._properties = properties
self._lanes = properties ['lanes'].split(',')
self._indexes = properties ['index'].split(',')
self._breakout_mode_entry = self._str_to_entries(bmode)
self._breakout_capabilities = None

# Find specified breakout mode in port breakout mode capabilities
for supported_mode in self._properties['breakout_modes']:
if self._breakout_mode_entry == self._str_to_entries(supported_mode):
self._breakout_capabilities = self._properties['breakout_modes'][supported_mode]
break
Copy link
Collaborator

Choose a reason for hiding this comment

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

why did you use break here?
what will be case when self._breakout_mode_entry != self._str_to_entries(supported_mode)? Should we address such cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here we are trying to find whether the given breakout mode is supported. We are iterating over supported modes and checking if the given breakout mode is equal to one of them. If the breakout mode is not equal to any of the supported modes we will raise an exception with an appropriated message at line 250.

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM!


if not self._breakout_capabilities:
raise RuntimeError("Unsupported breakout mode {}!".format(bmode))

def _re_group_to_entry(self, group):
if len(group) != BRKOUT_PATTERN_GROUPS:
raise RuntimeError("Unsupported breakout mode format!")

num_ports, default_speed, supported_speed, _, num_assigned_lanes, _ = group
if not num_assigned_lanes:
num_assigned_lanes = len(self._lanes)

return BreakoutCfg.BreakoutModeEntry(num_ports, default_speed, supported_speed, num_assigned_lanes)

def _str_to_entries(self, bmode):
"""
Example of match_list for some breakout_mode using regex
Breakout Mode -------> Match_list
-----------------------------
2x25G(2)+1x50G(2) ---> [('2', '25G', None, '(2)', '2'), ('1', '50G', None, '(2)', '2')]
1x50G(2)+2x25G(2) ---> [('1', '50G', None, '(2)', '2'), ('2', '25G', None, '(2)', '2')]
1x100G[40G] ---------> [('1', '100G', '[40G]', None, None)]
2x50G ---------------> [('2', '50G', None, None, None)]
"""

try:
groups_list = [re.match(BRKOUT_PATTERN, i).groups() for i in bmode.split("+")]
except Exception:
raise RuntimeError('Breakout mode "{}" validation failed!'.format(bmode))

return [self._re_group_to_entry(group) for group in groups_list]

def get_config(self):
# Ensure that we have corret number of configured lanes
lanes_used = 0
for entry in self._breakout_mode_entry:
lanes_used += entry.num_assigned_lanes

if lanes_used > len(self._lanes):
raise RuntimeError("Assigned lines count is more that available!")

ports = {}

lane_id = 0
alias_id = 0

for entry in self._breakout_mode_entry:
lanes_per_port = entry.num_assigned_lanes // entry.num_ports

for port in range(entry.num_ports):
interface_name = PORT_STR + str(self._interface_base_id + lane_id)

lanes = self._lanes[lane_id:lane_id + lanes_per_port]

ports[interface_name] = {
'alias': self._breakout_capabilities[alias_id],
'lanes': ','.join(lanes),
'speed': str(entry.default_speed),
'index': self._indexes[lane_id]
}

lane_id += lanes_per_port
alias_id += 1

return ports

offset = int(assigned_lane) + int(offset)
return offset
else:
raise Exception('Regex return for k is None...')

"""
Given a port and breakout mode, this method returns
the list of child ports using platform_json file
"""
def get_child_ports(interface, breakout_mode, platform_json_file):
child_ports = {}

port_dict = readJson(platform_json_file)

index = port_dict[INTF_KEY][interface]['index']
alias_list = port_dict[INTF_KEY][interface]['breakout_modes'][breakout_mode]
lanes = port_dict[INTF_KEY][interface]['lanes']

"""
Example of match_list for some breakout_mode using regex
Breakout Mode -------> Match_list
-----------------------------
2x25G(2)+1x50G(2) ---> [('2', '25G', None, '(2)', '2'), ('1', '50G', None, '(2)', '2')]
1x50G(2)+2x25G(2) ---> [('1', '50G', None, '(2)', '2'), ('2', '25G', None, '(2)', '2')]
1x100G[40G] ---------> [('1', '100G', '[40G]', None, None)]
2x50G ---------------> [('2', '50G', None, None, None)]
"""
# Asymmetric breakout mode
if re.search("\+",breakout_mode) is not None:
breakout_parts = breakout_mode.split("+")
match_list = [re.match(BRKOUT_PATTERN, i).groups() for i in breakout_parts]

# Symmetric breakout mode
else:
match_list = [re.match(BRKOUT_PATTERN, breakout_mode).groups()]
mode_handler = BreakoutCfg(interface, breakout_mode, port_dict[INTF_KEY][interface])

offset = 0
parent_intf_id = int(re.search("Ethernet(\d+)", interface).group(1))
for k in match_list:
offset = gen_port_config(child_ports, parent_intf_id, index, alias_list, lanes, k, offset)
return child_ports
return mode_handler.get_config()

def parse_platform_json_file(hwsku_json_file, platform_json_file):
ports = {}
Expand Down
15 changes: 15 additions & 0 deletions src/sonic-config-engine/tests/sample_hwsku.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@
},
"Ethernet124": {
"default_brkout_mode": "2x50G"
},
"Ethernet128": {
"default_brkout_mode": "1x40G[100G]"
},
"Ethernet132": {
"default_brkout_mode": "1x25G[100G,50G,40G,10G]"
},
"Ethernet136": {
"default_brkout_mode": "4x10G[25G]"
},
"Ethernet140": {
"default_brkout_mode": "2x25G(2)+1x50000(2)"
},
"Ethernet144": {
"default_brkout_mode": "1x100000[50G,40000,25G,10000]"
}
}
}
101 changes: 101 additions & 0 deletions src/sonic-config-engine/tests/sample_output/platform_output.json
Original file line number Diff line number Diff line change
Expand Up @@ -819,5 +819,106 @@
"pfc_asym": "off",
"speed": "50000",
"tpid": "0x8100"
},
"Ethernet128": {
"index": "33",
"lanes": "128,129,130,131",
"description": "Eth33",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth33",
"pfc_asym": "off",
"speed": "40000"
},
"Ethernet132": {
"index": "34",
"lanes": "132,133,134,135",
"description": "Eth34",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth34",
"pfc_asym": "off",
"speed": "25000"
},
"Ethernet136": {
"index": "35",
"lanes": "136",
"description": "Eth35/1",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth35/1",
"pfc_asym": "off",
"speed": "10000"
},
"Ethernet137": {
"index": "35",
"lanes": "137",
"description": "Eth35/2",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth35/2",
"pfc_asym": "off",
"speed": "10000"
},
"Ethernet138": {
"index": "35",
"lanes": "138",
"description": "Eth35/3",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth35/3",
"pfc_asym": "off",
"speed": "10000"
},
"Ethernet139": {
"index": "35",
"lanes": "139",
"description": "Eth35/4",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth35/4",
"pfc_asym": "off",
"speed": "10000"
},
"Ethernet140": {
"index": "36",
"lanes": "140",
"description": "Eth36/1",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth36/1",
"pfc_asym": "off",
"speed": "25000"
},
"Ethernet141": {
"index": "36",
"lanes": "141",
"description": "Eth36/2",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth36/2",
"pfc_asym": "off",
"speed": "25000"
},
"Ethernet142": {
"index": "36",
"lanes": "142,143",
"description": "Eth36/3",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth36/3",
"pfc_asym": "off",
"speed": "50000"
},
"Ethernet144": {
"index": "37",
"lanes": "144,145,146,147",
"description": "Eth37",
"tpid": "0x8100",
"mtu": "9100",
"alias": "Eth37",
"pfc_asym": "off",
"speed": "100000",
"fec": "rs"
}
}
Loading