[sonic-config-engine]: Improve comparison between default and supported breakout modes. (#9278)

Closes #7958 

#### Why I did it
The previous implementation of sonic-cfggen did a simple comparison between default breakout mode in
hwsku.json and supported modes in platform.json. To set a different default speed in hwsku.json
it was required to add one more entry to supported modes in platfrom.json file:

1x10G[100G,50G] vs 1x100G[50G,10G]

The new implementation does more intelligent parsing and analysis of supported and default modes. It
allows changing default speed without adding a new entry to platform.json.

#### How I did it
Add more intelligent parsing and analysis of supported and default modes.

#### How to verify it
Run sonic-config-engine unit tests from sonic-config-engine/tests directory

src/sonic-config-engine/portconfig.py

@@ -39,7 +39,8 @@ CUR_BRKOUT_MODE = "brkout_mode"
 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})\))?'
+BRKOUT_PATTERN_GROUPS = 6
 
 #
 # Helper Functions
@@ -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
+class BreakoutCfg(object):
-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
+    class BreakoutModeEntry:
-        if assigned_lane is None:
+        def __init__(self, num_ports, default_speed, supported_speed, num_assigned_lanes=None):
-            assigned_lane = len(lanes.split(","))
+            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)
 
-        parent_intf_id = int(offset)+int(parent_intf_id)
+        @classmethod
-        alias_position = 0 + int(offset)//int(num_lane_used)
+        def _speed_to_int(cls, speed):
-        lanes_start = 0 + int(offset)
+            try:
+                if speed.endswith('G'):
+                    return int(speed.replace('G', '')) * 1000
 
-        step = int(assigned_lane)//int(num_lane_used)
+                return int(speed)
-        for i in range(0,int(assigned_lane), step):
+            except ValueError:
-            intf_name = PORT_STR + str(parent_intf_id)
+                raise RuntimeError("Unsupported speed format '{}'".format(speed))
-            ports[intf_name] = {}
+
-            ports[intf_name]['alias'] = alias_list[alias_position]
+        def _parse_supported_speed(self, speed):
-            ports[intf_name]['lanes'] = ','.join(lanes.split(",")[lanes_start:lanes_start+step])
+            if not speed:
-            if speed:
+                return
-                speed_pat = re.search("^((\d+)G|\d+)$", speed.upper())
+
-                if speed_pat is None:
+            if not speed.startswith('[') and not speed.endswith(']'):
-                    raise Exception('{} speed is not Supported...'.format(speed))
+                raise RuntimeError("Unsupported port breakout format!")
-                speed_G, speed_orig = speed_pat.group(2), speed_pat.group(1)
+
-                if speed_G:
+            for s in speed[1:-1].split(','):
-                    conv_speed = int(speed_G)*1000
+                self.supported_speed.add(self._speed_to_int(s.strip()))
-                else:
+
-                    conv_speed = int(speed_orig)
+        def _parse_num_assigned_lanes(self, num_assigned_lanes):
-                ports[intf_name]['speed'] = str(conv_speed)
+            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
+        def __hash__(self):
-            alias_position += 1
+            return hash((self.num_ports, tuple(self.supported_speed), self.num_assigned_lanes))
-            lanes_start += step
+
+    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
+
+        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']
+    mode_handler = BreakoutCfg(interface, breakout_mode, port_dict[INTF_KEY][interface])
-    alias_list = port_dict[INTF_KEY][interface]['breakout_modes'][breakout_mode]
-    lanes = port_dict[INTF_KEY][interface]['lanes']
 
-    """
+    return mode_handler.get_config()
-    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()]
-
-    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
 
 def parse_platform_json_file(hwsku_json_file, platform_json_file):
     ports = {}

src/sonic-config-engine/tests/sample_hwsku.json

@@ -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]"
         }
     }
 }

src/sonic-config-engine/tests/sample_output/platform_output.json

@@ -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"
   }
 }

src/sonic-config-engine/tests/sample_platform.json

@@ -350,6 +350,61 @@
                 "2x25G(2)+1x50G(2)": ["Eth32/1", "Eth32/2", "Eth32/3"],
                 "1x50G(2)+2x25G(2)": ["Eth32/1", "Eth32/2", "Eth32/3"]
             }
+        },
+        "Ethernet128": {
+            "index": "33,33,33,33",
+            "lanes": "128,129,130,131",
+            "breakout_modes": {
+                "1x100G[40G]": ["Eth33"],
+                "2x50G": ["Eth33/1", "Eth33/2"],
+                "4x25G[10G]": ["Eth33/1", "Eth33/2", "Eth33/3", "Eth33/4"],
+                "2x25G(2)+1x50G(2)": ["Eth33/1", "Eth33/2", "Eth33/3"],
+                "1x50G(2)+2x25G(2)": ["Eth33/1", "Eth33/2", "Eth33/3"]
+            }
+        },
+        "Ethernet132": {
+            "index": "34,34,34,34",
+            "lanes": "132,133,134,135",
+            "breakout_modes": {
+                "1x100G[50G,40G,25G,10G]": ["Eth34"],
+                "2x50G": ["Eth34/1", "Eth34/2"],
+                "4x25G[10G]": ["Eth34/1", "Eth34/2", "Eth34/3", "Eth34/4"],
+                "2x25G(2)+1x50G(2)": ["Eth34/1", "Eth34/2", "Eth34/3"],
+                "1x50G(2)+2x25G(2)": ["Eth34/1", "Eth34/2", "Eth34/3"]
+            }
+        },
+        "Ethernet136": {
+            "index": "35,35,35,35",
+            "lanes": "136,137,138,139",
+            "breakout_modes": {
+                "1x100G[50G,40G,25G,10G]": ["Eth35"],
+                "2x50G": ["Eth35/1", "Eth35/2"],
+                "4x25G[10G]": ["Eth35/1", "Eth35/2", "Eth35/3", "Eth35/4"],
+                "2x25G(2)+1x50G(2)": ["Eth35/1", "Eth35/2", "Eth35/3"],
+                "1x50G(2)+2x25G(2)": ["Eth35/1", "Eth35/2", "Eth35/3"]
+            }
+        },
+        "Ethernet140": {
+            "index": "36,36,36,36",
+            "lanes": "140,141,142,143",
+            "breakout_modes": {
+                "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"]
+            }
+        },
+        "Ethernet144": {
+            "index": "37,37,37,37",
+            "lanes": "144,145,146,147",
+            "breakout_modes": {
+                "1x100G[50G,40G,25G,10G]": ["Eth37"],
+                "2x50G": ["Eth37/1", "Eth37/2"],
+                "4x25G[10000]": ["Eth37/1", "Eth37/2", "Eth37/3", "Eth37/4"],
+                "2x25000(2)+1x50G(2)": ["Eth37/1", "Eth37/2", "Eth37/3"],
+                "1x50000(2)+2x25G(2)": ["Eth37/1", "Eth37/2", "Eth37/3"]
+            }
         }
     }
 }

src/sonic-config-engine/tests/test_cfggen_platformJson.py

@@ -58,9 +58,20 @@ class TestCfgGenPlatformJson(TestCase):
         argument = '-m "' + self.platform_sample_graph + '" -p "' + self.platform_json + '" -S "' + self.hwsku_json + '" -v "PORT.keys()|list"'
         output = self.run_script(argument)
         self.maxDiff = None
-        expected = "['Ethernet8', 'Ethernet9', 'Ethernet36', 'Ethernet98', 'Ethernet0', 'Ethernet6', 'Ethernet4', 'Ethernet109', 'Ethernet108', 'Ethernet18', 'Ethernet100', 'Ethernet34', 'Ethernet104', 'Ethernet106', 'Ethernet94', 'Ethernet126', 'Ethernet96', 'Ethernet124', 'Ethernet90', 'Ethernet91', 'Ethernet92', 'Ethernet93', 'Ethernet50', 'Ethernet51', 'Ethernet52', 'Ethernet53', 'Ethernet54', 'Ethernet99', 'Ethernet56', 'Ethernet113', 'Ethernet76', 'Ethernet74', 'Ethernet39', 'Ethernet72', 'Ethernet73', 'Ethernet70', 'Ethernet71', 'Ethernet32', 'Ethernet33', 'Ethernet16', 'Ethernet111', 'Ethernet10', 'Ethernet11', 'Ethernet12', 'Ethernet13', 'Ethernet58', 'Ethernet19', 'Ethernet59', 'Ethernet38', 'Ethernet78', 'Ethernet68', 'Ethernet14', 'Ethernet89', 'Ethernet88', 'Ethernet118', 'Ethernet119', 'Ethernet116', 'Ethernet114', 'Ethernet80', 'Ethernet112', 'Ethernet86', 'Ethernet110', 'Ethernet84', 'Ethernet31', 'Ethernet49', 'Ethernet48', 'Ethernet46', 'Ethernet30', 'Ethernet29', 'Ethernet40', 'Ethernet120', 'Ethernet28', 'Ethernet66', 'Ethernet60', 'Ethernet64', 'Ethernet44', 'Ethernet20', 'Ethernet79', 'Ethernet69', 'Ethernet24', 'Ethernet26']"
 
-        self.assertEqual(sorted(output.strip()), sorted(expected))
+        expected = [
+            'Ethernet0', 'Ethernet4', 'Ethernet6', 'Ethernet8', 'Ethernet9', 'Ethernet10', 'Ethernet11', 'Ethernet12', 'Ethernet13', 'Ethernet14', 'Ethernet16',
+            'Ethernet18', 'Ethernet19', 'Ethernet20', 'Ethernet24', 'Ethernet26', 'Ethernet28', 'Ethernet29', 'Ethernet30', 'Ethernet31', 'Ethernet32', 'Ethernet33',
+            'Ethernet34', 'Ethernet36', 'Ethernet38', 'Ethernet39', 'Ethernet40', 'Ethernet44', 'Ethernet46', 'Ethernet48', 'Ethernet49', 'Ethernet50', 'Ethernet51',
+            'Ethernet52', 'Ethernet53', 'Ethernet54', 'Ethernet56', 'Ethernet58', 'Ethernet59', 'Ethernet60', 'Ethernet64', 'Ethernet66', 'Ethernet68', 'Ethernet69',
+            'Ethernet70', 'Ethernet71', 'Ethernet72', 'Ethernet73', 'Ethernet74', 'Ethernet76', 'Ethernet78', 'Ethernet79', 'Ethernet80', 'Ethernet84', 'Ethernet86',
+            'Ethernet88', 'Ethernet89', 'Ethernet90', 'Ethernet91', 'Ethernet92', 'Ethernet93', 'Ethernet94', 'Ethernet96', 'Ethernet98', 'Ethernet99', 'Ethernet100',
+            'Ethernet104', 'Ethernet106', 'Ethernet108', 'Ethernet109', 'Ethernet110', 'Ethernet111', 'Ethernet112', 'Ethernet113', 'Ethernet114', 'Ethernet116',
+            'Ethernet118', 'Ethernet119', 'Ethernet120', 'Ethernet124', 'Ethernet126', 'Ethernet128', 'Ethernet132', 'Ethernet136', 'Ethernet137', 'Ethernet138',
+            'Ethernet139', 'Ethernet140', 'Ethernet141', 'Ethernet142', 'Ethernet144'
+            ]
+
+        self.assertEqual(sorted(eval(output.strip())), sorted(expected))
 
     # Check specific Interface with it's proper configuration as per platform.json
     def test_platform_json_specific_ethernet_interfaces(self):
@@ -83,6 +94,7 @@ class TestCfgGenPlatformJson(TestCase):
         expected = "{'index': '2', 'lanes': '4,5', 'description': 'Eth2/1', 'admin_status': 'up', 'mtu': '9100', 'alias': 'Eth2/1', 'pfc_asym': 'off', 'speed': '50000', 'tpid': '0x8100'}"
         print(output.strip())
         self.assertEqual(utils.to_dict(output.strip()), utils.to_dict(expected))
+
     # Check all Interface with it's proper configuration as per platform.json
     def test_platform_json_all_ethernet_interfaces(self):
         argument = '-m "' + self.platform_sample_graph + '" -p "' + self.platform_json + '" -S "' + self.hwsku_json  + '" -v "PORT"'