Enhance decommission logic on per-logical-port basis

Author: longhuan-cisco

sonic-xcvrd/xcvrd/xcvrd.py

@@ -477,7 +477,6 @@ class CmisManagerTask(threading.Thread):
     CMIS_MODULE_TYPES    = ['QSFP-DD', 'QSFP_DD', 'OSFP', 'OSFP-8X', 'QSFP+C']
     CMIS_MAX_HOST_LANES    = 8
     CMIS_EXPIRATION_BUFFER_MS = 2
-    ALL_LANES_MASK = 0xff
 
     def __init__(self, namespaces, port_mapping, main_thread_stop_event, skip_cmis_mgr=False):
         threading.Thread.__init__(self)
@@ -698,98 +697,217 @@ def get_cmis_media_lanes_mask(self, api, appl, lport, subport):
 
     def clear_decomm_pending(self, lport):
         """
-        Clear the decommission pending status for the entire physical port this logical port belongs to.
+        Clear the decommission pending status for this logical port.
 
         Args:
             lport:
                 String, logical port name
         """
-        self.decomm_pending_dict.pop(self.port_dict.get(lport, {}).get('index'), None)
+        physical_port_idx = self.port_dict.get(lport, {}).get('index')
+        if physical_port_idx not in self.decomm_pending_dict:
+            return
+        self.decomm_pending_dict[physical_port_idx].pop(lport, None)
+        # If there are no more logical ports pending decommission on this physical port,
+        # remove the physical port entry from the decomm_pending_dict
+        if not self.decomm_pending_dict[physical_port_idx]:
+            self.decomm_pending_dict.pop(physical_port_idx)
 
-    def set_decomm_pending(self, lport):
+    def set_decomm_pending(self, lport, api):
         """
-        Set the decommission pending status.
+        Set the decommission pending status for this logical port to start decommissioning.
+
+        Decommissioning can be done on a per-logical-port basis: for each logical port,
+        only decommission the minimal set of host lanes to allow the logical port to
+        be able to apply the new appl code without config errors.
+
+        decomm_pending_dict stores the host lanes pending to be decommissioned for a logical
+        port.
+        i.e. self.decomm_pending_dict[physical_port_idx][lport] = lanes_mask_requiring_decomm
+        lanes_mask_requiring_decomm of a logical port can be wider than the logical port itself.
 
         Args:
             lport:
                 String, logical port name
+            api:
+                XcvrApi object
+        Returns:
+            Boolean, True if skip the rest of the processing of the current CMIS state
         """
-        physical_port_idx = self.port_dict[lport]['index']
-        if physical_port_idx in self.decomm_pending_dict:
-            # only one logical port can be the lead logical port doing the
-            # decommission state machine.
-            return
-        self.decomm_pending_dict[physical_port_idx] = lport
-        self.log_notice("{}: DECOMMISSION: setting decomm_pending for physical port "
-                        "{}".format(lport, physical_port_idx))
+        skip_rest_processing = False
+
+        lport_host_lanes_mask = self.port_dict[lport]['host_lanes_mask']
+        lanes_mask_requiring_decomm = self.get_host_lanes_mask_requiring_decomm(lport, api)
+
+        # Check if other lports are doing decommission on the lanes overlapping with this lport
+        total_affected_lanes_mask = lport_host_lanes_mask | lanes_mask_requiring_decomm
+        if total_affected_lanes_mask & self.get_decomm_pending_host_lanes_mask(lport, exclude_lports=[lport]):
+            self.clear_decomm_pending(lport)
+
+            if total_affected_lanes_mask & self.get_decomm_failed_host_lanes_mask(lport):
+                # Fail this lport if any of its host lanes are in decommissioning failed state
+                self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_FAILED)
+                decomm_status_str = "failed"
+            else:
+                decomm_status_str = "waiting for completion"
+
+            self.log_notice("{}: DECOMM: decommission initiated by other lports is still in progress on "
+                            "host lanes {:#x}, {}".format(lport, total_affected_lanes_mask, decomm_status_str))
+
+            skip_rest_processing = True
+        elif lanes_mask_requiring_decomm:
+            self.decomm_pending_dict.setdefault(self.port_dict[lport]['index'], {})[lport] = lanes_mask_requiring_decomm
+            self.log_notice("{}: DECOMM: setting decomm_pending for host lanes "
+                            "{:#x}".format(lport, lanes_mask_requiring_decomm))
+
+            self.port_dict[lport]['appl'] = 0
+            self.port_dict[lport]['host_lanes_mask'] = lanes_mask_requiring_decomm
+            self.port_dict[lport]['media_lanes_mask'] = lanes_mask_requiring_decomm
+            self.log_notice("{}: DECOMM: setting appl={} and host_lanes_mask/media_lanes_mask="
+                            "{:#x}".format(lport, self.port_dict[lport]['appl'], lanes_mask_requiring_decomm))
+
+            # Skip rest of the deinit/pre-init when this is the logical port doing decommission
+            self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_DP_DEINIT)
+            skip_rest_processing = True
+        else:  # For the case of lanes_mask_requiring_decomm == 0x0:
+            # If lport was previously marked as pending, then decommissioning is no more needed, clear its status
+            self.clear_decomm_pending(lport)
+
+        return skip_rest_processing
 
-    def is_decomm_lead_lport(self, lport):
+    def is_decomm_pending(self, lport):
         """
-        Check if this is the lead logical port doing the decommission state machine.
+        Check if this logical port is in middle of decommissioning.
 
         Args:
             lport:
                 String, logical port name
         Returns:
             Boolean, True if decommission pending, False otherwise
         """
-        return self.decomm_pending_dict.get(self.port_dict[lport]['index']) == lport
+        return lport in self.decomm_pending_dict.get(self.port_dict[lport]['index'], {})
 
-    def is_decomm_pending(self, lport):
+    def get_decomm_pending_host_lanes_mask(self, lport, exclude_lports=[]):
         """
-        Get the decommission pending status for the physical port the given logical port belongs to.
+        Get the host lanes in decommission pending status for the entire
+        physical port the given logical port belongs to.
 
         Args:
             lport:
                 String, logical port name
+            exclude_lports:
+                List of logical ports to exclude from the mask
         Returns:
-            Boolean, True if decommission pending, False otherwise
+            Integer, bitmask of host lanes that are decommission pending
         """
-        return self.port_dict[lport]['index'] in self.decomm_pending_dict
+        physical_port_idx = self.port_dict[lport]['index']
+        decomm_ports = self.decomm_pending_dict.get(physical_port_idx, {})
+        if not decomm_ports:
+            return 0
 
-    def is_decomm_failed(self, lport):
+        decomm_pending_mask = 0
+        for logical_port, mask in decomm_ports.items():
+            if logical_port in exclude_lports:
+                continue
+            decomm_pending_mask |= mask
+
+        return decomm_pending_mask
+
+    def get_decomm_failed_host_lanes_mask(self, lport):
         """
-        Get the decommission failed status for the physical port the given logical port belongs to.
+        Get the host lanes in decommissioning failed state for the entire
+        physical port the given logical port belongs to.
 
         Args:
             lport:
                 String, logical port name
         Returns:
-            Boolean, True if decommission failed, False otherwise
+            Integer, bitmask of host lanes in decommissioning failed state
         """
+        failed_mask = 0
+
         physical_port_idx = self.port_dict[lport]['index']
-        lead_logical_port = self.decomm_pending_dict.get(physical_port_idx)
-        if lead_logical_port is None:
-            return False
-        return (
-            get_cmis_state_from_state_db(
-                lead_logical_port,
-                self.xcvr_table_helper.get_status_sw_tbl(
-                    self.get_asic_id(lead_logical_port)
-                )
-            )
-            == CMIS_STATE_FAILED
-        )
+        if physical_port_idx not in self.decomm_pending_dict:
+            return failed_mask
 
-    def is_decommission_required(self, api, app_new):
+        for logical_port, mask in self.decomm_pending_dict[physical_port_idx].items():
+            if get_cmis_state_from_state_db(
+                logical_port, self.xcvr_table_helper.get_status_sw_tbl(self.get_asic_id(logical_port))
+            ) != CMIS_STATE_FAILED:
+                continue
+            failed_mask |= mask
+
+        return failed_mask
+
+    def get_host_lanes_mask_requiring_decomm(self, lport, api):
         """
-        Check if the CMIS decommission (i.e. reset appl code to 0 for all lanes
-        of the entire physical port) is required
+        Get the minimal set of host lanes that require decommissioning to allow
+        the given logical port to apply its new appl code successfully.
 
         Args:
+            lport:
+                String, logical port name
             api:
                 XcvrApi object
-            app_new:
-                Integer, the new desired appl code
         Returns:
-            True, if decommission is required
-            False, if decommission is not required
+            Integer, bitmask of host lanes that require decommissioning
         """
-        for lane in range(self.CMIS_MAX_HOST_LANES):
-            app_cur = api.get_application(lane)
-            if app_cur != 0 and app_cur != app_new:
-                return True
-        return False
+        def get_data_path_mask(app_advt, app, lane_idx):
+            """
+            Get the host lane mask for the entire data path based on the appl code on one of its lanes.
+
+            Args:
+                app_advt: The application advertisement dictionary
+                app: The application code
+                lane_idx: The index of the lane this appl code is assigned to
+            Returns:
+                Integer, the host lane mask for this data path
+            """
+            host_lane_assignment_options = app_advt.get(app, {}).get('host_lane_assignment_options')
+            host_lane_count = app_advt.get(app, {}).get('host_lane_count')
+
+            if not host_lane_assignment_options or not host_lane_count:
+                return 0
+
+            mask_for_single_lane = 1 << lane_idx
+
+            for start_lane_idx in range(self.CMIS_MAX_HOST_LANES):
+                if not (host_lane_assignment_options & (1 << start_lane_idx)):
+                    continue
+                data_path_mask = ((1 << host_lane_count) - 1) << start_lane_idx
+                if data_path_mask & mask_for_single_lane:
+                    return data_path_mask
+
+            return 0
+
+        lport_host_lanes_mask = self.port_dict[lport]['host_lanes_mask']
+        app_advt = api.get_application_advertisement()
+        active_app_dict = api.get_active_apsel_hostlane()
+        app_new = self.port_dict[lport]['appl']
+
+        conflicting_data_paths_host_lanes_mask = 0
+        # Identify the configured data paths that share host lanes with this logical port
+        # and have conflicting appl codes
+        for lane_idx in range(self.CMIS_MAX_HOST_LANES):
+            if not (1 << lane_idx & lport_host_lanes_mask):
+                continue
+            app_cur = active_app_dict.get('ActiveAppSelLane{}'.format(lane_idx + 1), 0)
+            if app_cur == 0 or app_cur == app_new:
+                continue
+            conflicting_data_paths_host_lanes_mask |= get_data_path_mask(app_advt, app_cur, lane_idx)
+
+        # If conflicting_data_paths_host_lanes_mask is covered by current lport's mask,
+        # then new appl code can be applied directly without decommissioning
+        host_lanes_mask_requiring_decomm = (
+            0 if not (conflicting_data_paths_host_lanes_mask & ~lport_host_lanes_mask)
+            else conflicting_data_paths_host_lanes_mask
+        )
+
+        log_func = self.log_debug if not host_lanes_mask_requiring_decomm else self.log_notice
+        log_func("{}: DECOMM: based on ActiveAppSel(lane 8->1) {}, to apply appl {} on {:#010b}, "
+                  "host lanes requiring decomm is {:#010b}".format(
+                      lport, list(reversed(active_app_dict.values())), app_new, lport_host_lanes_mask,
+                      host_lanes_mask_requiring_decomm))
+        return host_lanes_mask_requiring_decomm
 
     def is_cmis_application_update_required(self, api, app_new, host_lanes_mask):
         """
@@ -1211,16 +1329,15 @@ def task_worker(self):
                 host_lanes_mask = self.port_dict[lport].get('host_lanes_mask', 0)
                 appl = self.port_dict[lport].get('appl', 0)
                 # appl can be 0 if this lport is in decommission state machine, which should not be considered as failed case.
-                if state != CMIS_STATE_INSERTED and not self.is_decomm_lead_lport(lport) and (host_lanes_mask <= 0 or appl < 1):
+                if state != CMIS_STATE_INSERTED and not self.is_decomm_pending(lport) and (host_lanes_mask <= 0 or appl < 1):
                     self.log_error("{}: Unexpected value for host_lanes_mask {} or appl {} in "
                                     "{} state".format(lport, host_lanes_mask, appl, state))
                     self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_FAILED)
                     continue
 
                 self.log_notice("{}: {}G, lanemask=0x{:x}, CMIS state={}{}, Module state={}, DP state={}, appl {} host_lane_count {} "
                                 "retries={}".format(lport, int(speed/1000), host_lanes_mask, state,
-                                "(decommission" + ("*" if self.is_decomm_lead_lport(lport) else "") + ")"
-                                    if self.is_decomm_pending(lport) else "",
+                                "(decommission)" if self.is_decomm_pending(lport) else "",
                                 api.get_module_state(), api.get_datapath_state(), appl, host_lane_count, retries))
                 if retries > self.CMIS_MAX_RETRIES:
                     self.log_error("{}: FAILED".format(lport))
@@ -1264,27 +1381,7 @@ def task_worker(self):
                         media_lanes_mask = self.port_dict[lport]['media_lanes_mask']
                         self.log_notice("{}: Setting media_lanemask=0x{:x}".format(lport, media_lanes_mask))
 
-                        if self.is_decommission_required(api, appl):
-                            self.set_decomm_pending(lport)
-
-                        if self.is_decomm_lead_lport(lport):
-                            # Set all the DP lanes AppSel to unused(0) when non default app code needs to be configured
-                            self.port_dict[lport]['appl'] = appl = 0
-                            self.port_dict[lport]['host_lanes_mask'] = host_lanes_mask = self.ALL_LANES_MASK
-                            self.port_dict[lport]['media_lanes_mask'] = self.ALL_LANES_MASK
-                            self.log_notice("{}: DECOMMISSION: setting appl={} and "
-                                            "host_lanes_mask/media_lanes_mask={:#x}".format(lport, appl, self.ALL_LANES_MASK))
-                            # Skip rest of the deinit/pre-init when this is the lead logical port for decommission
-                            self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_DP_DEINIT)
-                            continue
-                        elif self.is_decomm_pending(lport):
-                            if self.is_decomm_failed(lport):
-                                self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_FAILED)
-                                decomm_status_str = "failed"
-                            else:
-                                decomm_status_str = "waiting for completion"
-                            self.log_notice("{}: DECOMMISSION: decommission has already started for this physical port, "
-                                            "{}".format(lport, decomm_status_str))
+                        if self.set_decomm_pending(lport, api):
                             continue
 
                         if self.port_dict[lport]['host_tx_ready'] != 'true' or \
@@ -1444,7 +1541,7 @@ def task_worker(self):
 
                         # Clear decommission status and invoke CMIS reinit so that normal CMIS initialization can begin
                         if self.is_decomm_pending(lport):
-                            self.log_notice("{}: DECOMMISSION: done for physical port {}".format(lport, self.port_dict[lport]['index']))
+                            self.log_notice("{}: DECOMM: decommission done for host lanes {:#x}".format(lport, self.port_dict[lport]['host_lanes_mask']))
                             self.clear_decomm_pending(lport)
                             self.force_cmis_reinit(lport)
                             continue