Fix CMIS and preemphasis lane mapping issues

sonic-xcvrd/tests/test_xcvrd.py

@@ -179,7 +179,7 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
         task.get_cfg_port_tbl = MagicMock()
         task.xcvr_table_helper.get_status_tbl.return_value = mock_get_status_tbl
         port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET,
-                                            {'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
+                                            {'index':1, 'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
 
         # Case 1: get_xcvr_api() raises an exception
         task.on_port_update_event(port_change_event)
@@ -193,6 +193,8 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
         mock_sfp.get_xcvr_api = MagicMock(return_value=mock_xcvr_api)
         mock_xcvr_api.is_flat_memory = MagicMock(side_effect=AttributeError)
         task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
+        port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET,
+                                            {'index':1, 'speed':'200000', 'lanes':'1,2,3,4,5,6,7,8'})
         task.on_port_update_event(port_change_event)
         task.task_worker()
         assert get_cmis_state_from_state_db('Ethernet0', task.xcvr_table_helper.get_status_tbl(task.port_mapping.get_asic_id_for_logical_port('Ethernet0'))) == CMIS_STATE_READY
@@ -202,6 +204,8 @@ def test_CmisManagerTask_get_xcvr_api_exception(self, mock_platform_chassis, moc
         mock_xcvr_api.is_coherent_module = MagicMock(return_value=False)
         mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD')
         task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, True])
+        port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET,
+                                            {'index':1, 'speed':'400000', 'lanes':'1,2,3,4,5,6,7,8'})
         task.on_port_update_event(port_change_event)
         task.get_cmis_host_lanes_mask = MagicMock()
         task.task_worker()
@@ -618,6 +622,7 @@ def test_get_media_settings_key(self, mock_is_cmis_api, mock_chassis):
         mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
         mock_api = MagicMock()
         mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api)
+        mock_sfp.get_platform_media_key = MagicMock(side_effect=NotImplementedError)
         mock_is_cmis_api.return_value = False
 
         xcvr_info_dict = {
@@ -890,7 +895,7 @@ def test_handle_port_update_event(self, mock_select, mock_sub_table):
         stop_event = threading.Event()
         stop_event.is_set = MagicMock(return_value=False)
         handle_port_update_event(sel, asic_context, stop_event,
-                                  logger, port_mapping.handle_port_change_event)
+                                  logger, port_mapping, port_mapping.handle_port_change_event)
 
     @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock())
     @patch('swsscommon.swsscommon.SubscriberStateTable')
@@ -1014,10 +1019,14 @@ def test_CmisManagerTask_handle_port_change_event(self):
 
         port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL)
         task.on_port_update_event(port_change_event)
-        assert len(task.port_dict) == 1
+        assert len(task.port_dict) == 0
 
         port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET)
         task.on_port_update_event(port_change_event)
+        assert len(task.port_dict) == 0
+
+        port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET, {'index':1})
+        task.on_port_update_event(port_change_event)
         assert len(task.port_dict) == 1
 
 
@@ -2421,11 +2430,13 @@ def test_get_media_val_str(self):
         num_logical_ports = 1
         lane_dict = {'lane0': '1', 'lane1': '2', 'lane2': '3', 'lane3': '4'}
         logical_idx = 1
-        media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx)
+        lane_count = 4
+        media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx, lane_count)
         assert media_str == '1,2,3,4'
         num_logical_ports = 2
         logical_idx = 1
-        media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx)
+        lane_count = 2
+        media_str = get_media_val_str(num_logical_ports, lane_dict, logical_idx, lane_count)
         assert media_str == '3,4'
 
     class MockPortMapping:

sonic-xcvrd/xcvrd/xcvrd.py

@@ -827,6 +827,9 @@ def log_debug(self, message):
     def log_notice(self, message):
         helper_logger.log_notice("CMIS: {}".format(message))
 
+    def log_warning(self, message):
+        helper_logger.log_warning("CMIS: {}".format(message))
+
     def log_error(self, message):
         helper_logger.log_error("CMIS: {}".format(message))
 
@@ -841,6 +844,15 @@ def update_port_transceiver_status_table_sw_cmis_state(self, lport, cmis_state_t
         fvs = swsscommon.FieldValuePairs([('cmis_state', cmis_state_to_set)])
         status_table.set(lport, fvs)
 
+    def delete_port_transceiver_status_table_sw_cmis_state(self, lport):
+        asic_index = self.port_mapping.get_asic_id_for_logical_port(lport)
+        status_table = self.xcvr_table_helper.get_status_tbl(asic_index)
+        if status_table is None:
+            helper_logger.log_error("status_table is None while deleting "
+                                    "sw CMIS state for lport {}".format(lport))
+            return
+        status_table.delete(lport)
+
     def on_port_update_event(self, port_change_event):
         if port_change_event.event_type not in [port_change_event.PORT_SET, port_change_event.PORT_DEL]:
             return
@@ -866,33 +878,56 @@ def on_port_update_event(self, port_change_event):
 
         # Skip if the port/cage type is not a CMIS
         # 'index' can be -1 if STATE_DB|PORT_TABLE
-        if lport not in self.port_dict:
-            self.port_dict[lport] = {}
 
         if port_change_event.port_dict is None:
             return
 
         if port_change_event.event_type == port_change_event.PORT_SET:
+            force_reinit = False
+
             if pport >= 0:
-                self.port_dict[lport]['index'] = pport
-            if 'speed' in port_change_event.port_dict and port_change_event.port_dict['speed'] != 'N/A':
-                self.port_dict[lport]['speed'] = port_change_event.port_dict['speed']
-            if 'lanes' in port_change_event.port_dict:
-                self.port_dict[lport]['lanes'] = port_change_event.port_dict['lanes']
-            if 'host_tx_ready' in port_change_event.port_dict:
-                self.port_dict[lport]['host_tx_ready'] = port_change_event.port_dict['host_tx_ready']
-            if 'admin_status' in port_change_event.port_dict:
-                self.port_dict[lport]['admin_status'] = port_change_event.port_dict['admin_status']
-            if 'laser_freq' in port_change_event.port_dict:
-                self.port_dict[lport]['laser_freq'] = int(port_change_event.port_dict['laser_freq'])
-            if 'tx_power' in port_change_event.port_dict:
-                self.port_dict[lport]['tx_power'] = float(port_change_event.port_dict['tx_power'])
-            if 'subport' in port_change_event.port_dict:
-                self.port_dict[lport]['subport'] = int(port_change_event.port_dict['subport'])
-
-            self.force_cmis_reinit(lport, 0)
+                if lport not in self.port_dict:
+                    self.port_dict[lport] = {'index': pport}
+
+            #Skip if STATE_DB update is received without update from CONFIG_DB
+            if lport not in self.port_dict:
+                return
+
+            for key in port_change_event.port_dict.keys():
+                if key in ['host_tx_ready', 'status'] or pport >= 0:
+                    if key in self.port_dict[lport]:
+                        if self.port_dict[lport][key] != port_change_event.port_dict[key]:
+                            self.port_dict[lport][key] = port_change_event.port_dict[key]
+                            # Trigger reinit only when there is change
+                            # Only CONFIG_DB has the field 'index' as > 0 value
+                            force_reinit = True
+                    else:
+                        self.port_dict[lport][key] = port_change_event.port_dict[key]
+                        # Trigger reinit only when there is change
+                        # Only CONFIG_DB has the field 'index' as > 0 value
+                        force_reinit = True
+
+            if 'alias' in port_change_event.port_dict:
+                try:
+                    subport_idx = int(port_change_event.port_dict['alias'].split('/')[-1])
+                except ValueError:
+                    subport_idx = 1
+                num_lanes = len(self.port_dict[lport]['lanes'].split(','))
+                subport_num = int((subport_idx / num_lanes) + (subport_idx % num_lanes))
+                if 'subport' in self.port_dict[lport]:
+                    if self.port_dict[lport]['subport'] != subport_num:
+                        self.port_dict[lport]['subport'] = subport_num
+                        force_reinit = True
+                else:
+                    self.port_dict[lport]['subport'] = subport_num
+                    force_reinit = True
+
+            if force_reinit:
+                self.force_cmis_reinit(lport, 0)
         else:
-            self.update_port_transceiver_status_table_sw_cmis_state(lport, CMIS_STATE_REMOVED)
+            if pport >= 0:
+                self.port_dict.pop(lport)
+                self.delete_port_transceiver_status_table_sw_cmis_state(lport)
 
     def get_cmis_dp_init_duration_secs(self, api):
         return api.get_datapath_init_duration()/1000
@@ -1293,6 +1328,7 @@ def task_worker(self):
                                                   asic_context,
                                                   self.task_stopping_event,
                                                   helper_logger,
+                                                  self.port_mapping,
                                                   self.on_port_update_event)
 
             for lport, info in self.port_dict.items():
@@ -1561,7 +1597,7 @@ def task_worker(self):
                                 self.force_cmis_reinit(lport, retries + 1)
                             continue
 
-                        if hasattr(api, 'get_cmis_rev'):
+                        if hasattr(api, 'get_cmis_rev') and False:
                             # Check datapath init pending on module that supports CMIS 5.x
                             majorRev = int(api.get_cmis_rev().split('.')[0])
                             if majorRev >= 5 and not self.check_datapath_init_pending(api, host_lanes_mask):

sonic-xcvrd/xcvrd/xcvrd_utilities/media_settings_parser.py

@@ -79,8 +79,16 @@ def get_media_settings_key(physical_port, transceiver_dict, port_speed, lane_cou
     media_key = ''
     media_compliance_dict = {}
 
+    sfp = xcvrd.platform_chassis.get_sfp(physical_port)
+
+    try:
+        media_key = sfp.get_platform_media_key(transceiver_dict[physical_port], int(port_speed), lane_count)
+    except NotImplementedError:
+        pass
+    else:
+        return media_key
+
     try:
-        sfp = xcvrd.platform_chassis.get_sfp(physical_port)
         api = sfp.get_xcvr_api()
         if xcvrd.is_cmis_api(api):
             media_compliance_code = media_compliance_dict_str
@@ -140,7 +148,7 @@ def get_media_val_str_from_dict(media_dict):
     return media_str
 
 
-def get_media_val_str(num_logical_ports, lane_dict, logical_idx):
+def get_media_val_str(num_logical_ports, lane_dict, logical_idx, lane_count):
     LANE_STR = 'lane'
 
     logical_media_dict = {}
@@ -149,9 +157,10 @@ def get_media_val_str(num_logical_ports, lane_dict, logical_idx):
     # The physical ports has more than one logical port meaning it is
     # in breakout mode. So fetch the corresponding lanes from the file
     media_val_str = ''
-    if (num_logical_ports > 1) and \
+    if ((num_logical_ports > 1) or (num_lanes_on_port != lane_count)) and \
        (num_lanes_on_port >= num_logical_ports):
-        num_lanes_per_logical_port = num_lanes_on_port//num_logical_ports
+        # Assuming the lanes are split evenly
+        num_lanes_per_logical_port = lane_count
         start_lane = logical_idx * num_lanes_per_logical_port
 
         for lane_idx in range(start_lane, start_lane +
@@ -338,7 +347,8 @@ def notify_media_setting(logical_port_name, transceiver_dict,
             if type(media_dict[media_key]) is dict:
                 media_val_str = get_media_val_str(num_logical_ports,
                                                   media_dict[media_key],
-                                                  logical_idx)
+                                                  logical_idx,
+                                                  lane_count)
             else:
                 media_val_str = media_dict[media_key]
             helper_logger.log_debug("{}:({},{}) ".format(index, str(media_key), str(media_val_str)))