diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 1adcaa224..103f01483 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/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') @@ -998,6 +1003,10 @@ def test_CmisManagerTask_handle_port_change_event(self): port_mapping = PortMapping() stop_event = threading.Event() task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) + task.xcvr_table_helper = MagicMock() + status_tbl = MagicMock() + status_tbl.delete = MagicMock() + task.xcvr_table_helper.get_status_tbl = MagicMock(return_value=status_tbl) assert not task.isPortConfigDone port_change_event = PortChangeEvent('PortConfigDone', -1, 0, PortChangeEvent.PORT_SET) @@ -1012,13 +1021,17 @@ def test_CmisManagerTask_handle_port_change_event(self): task.on_port_update_event(port_change_event) assert len(task.port_dict) == 0 - port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL) + 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 - port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_SET) + port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_DEL, {'index':1}) task.on_port_update_event(port_change_event) - assert len(task.port_dict) == 1 + assert len(task.port_dict) == 0 @patch('xcvrd.xcvrd.XcvrTableHelper') @@ -2421,11 +2434,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: diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index ec9523830..50cc9dce3 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/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,41 @@ 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 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 +1313,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 +1582,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): diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/media_settings_parser.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/media_settings_parser.py index 9e79e6c7b..eaa4eb58a 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/media_settings_parser.py +++ b/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))) diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/port_mapping.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/port_mapping.py index 788dc8134..faabb7ebe 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/port_mapping.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/port_mapping.py @@ -80,7 +80,11 @@ def get_logical_to_physical(self, port_name): def get_physical_to_logical(self, physical_port: int): assert isinstance(physical_port, int), "{} is NOT integer".format(physical_port) - return self.physical_to_logical.get(physical_port) + logical_port_list = self.physical_to_logical.get(physical_port) + if logical_port_list: + # Sorting based on the number in port_name eg: 128 in Ethernet128 + logical_port_list.sort(key=lambda intf: int(intf[8:])) + return logical_port_list def logical_port_name_to_physical_port_list(self, port_name): try: @@ -115,8 +119,8 @@ def subscribe_port_update_event(namespaces, logger): """ port_tbl_map = [ {'CONFIG_DB': swsscommon.CFG_PORT_TABLE_NAME}, - {'STATE_DB': 'TRANSCEIVER_INFO'}, {'STATE_DB': 'PORT_TABLE', 'FILTER': ['host_tx_ready']}, + {'STATE_DB': 'TRANSCEIVER_STATUS', 'FILTER': ['status']}, ] sel = swsscommon.Select() @@ -141,7 +145,7 @@ def apply_filter_to_fvp(filter, fvp): if key not in (set(filter) | set({'index', 'key', 'asic_id', 'op'})): del fvp[key] -def handle_port_update_event(sel, asic_context, stop_event, logger, port_change_event_handler): +def handle_port_update_event(sel, asic_context, stop_event, logger, port_mapping, port_change_event_handler): """ Select PORT update events, notify the observers upon a port update in CONFIG_DB or a XCVR insertion/removal in STATE_DB @@ -163,8 +167,42 @@ def handle_port_update_event(sel, asic_context, stop_event, logger, port_change_ if not validate_port(key): continue fvp = dict(fvp) if fvp is not None else {} - logger.log_warning("$$$ {} handle_port_update_event() : op={} DB:{} Table:{} fvp {}".format( - key, op, port_tbl.db_name, port_tbl.table_name, fvp)) + logger.log_debug("handle_port_update_event key:{} op:{} DB:{} Table:{} fvp:{}"\ + .format(key, op, port_tbl.db_name, port_tbl.table_name, fvp)) + if port_tbl.db_name == "CONFIG_DB" and\ + port_tbl.table_name == swsscommon.CFG_PORT_TABLE_NAME: + if op == swsscommon.SET_COMMAND and\ + 'index' in fvp: + new_physical_index = int(fvp['index']) + if not port_mapping.is_logical_port(key): + # New logical port created + port_change_event = PortChangeEvent(key,\ + new_physical_index,\ + asic_context[port_tbl],\ + PortChangeEvent.PORT_ADD) + port_mapping.handle_port_change_event(port_change_event) + else: + current_physical_index = port_mapping.get_logical_to_physical(key)[0] + if current_physical_index != new_physical_index: + port_change_event = PortChangeEvent(key, + current_physical_index, + asic_context[port_tbl], + PortChangeEvent.PORT_REMOVE) + port_mapping.handle_port_change_event(port_change_event) + + port_change_event = PortChangeEvent(key,\ + new_physical_index,\ + asic_context[port_tbl],\ + PortChangeEvent.PORT_ADD) + port_mapping.handle_port_change_event(port_change_event) + elif op == swsscommon.DEL_COMMAND and\ + port_mapping.is_logical_port(key): + fvp['index'] = port_mapping.get_logical_to_physical(key)[0] + port_change_event = PortChangeEvent(key,\ + port_mapping.get_logical_to_physical(key)[0],\ + asic_context[port_tbl],\ + PortChangeEvent.PORT_REMOVE) + port_mapping.handle_port_change_event(port_change_event) if 'index' not in fvp: fvp['index'] = '-1' @@ -185,30 +223,29 @@ def handle_port_update_event(sel, asic_context, stop_event, logger, port_change_ apply_filter_to_fvp(filter, fvp) if key in PortChangeEvent.PORT_EVENT: - diff = dict(set(fvp.items()) - set(PortChangeEvent.PORT_EVENT[key].items())) - # Ignore duplicate events - if not diff: - PortChangeEvent.PORT_EVENT[key] = fvp - continue + diff = dict(set(fvp.items()) - set(PortChangeEvent.PORT_EVENT[key].items())) + # Ignore duplicate events + if not diff: + PortChangeEvent.PORT_EVENT[key] = fvp + continue PortChangeEvent.PORT_EVENT[key] = fvp if fvp['op'] == swsscommon.SET_COMMAND: - port_change_event = PortChangeEvent(fvp['key'], - port_index, - fvp['asic_id'], - PortChangeEvent.PORT_SET, - fvp) + port_change_event = PortChangeEvent(fvp['key'], + port_index, + fvp['asic_id'], + PortChangeEvent.PORT_SET, + fvp) elif fvp['op'] == swsscommon.DEL_COMMAND: - port_change_event = PortChangeEvent(fvp['key'], - port_index, - fvp['asic_id'], - PortChangeEvent.PORT_DEL, - fvp) + port_change_event = PortChangeEvent(fvp['key'], + port_index, + fvp['asic_id'], + PortChangeEvent.PORT_DEL, + fvp) # This is the final event considered for processing - logger.log_warning("*** {} handle_port_update_event() fvp {}".format( - key, fvp)) + logger.log_debug("handle_port_update_event Key={} fvp {}".format(key, fvp)) if port_change_event is not None: - port_change_event_handler(port_change_event) + port_change_event_handler(port_change_event) def handle_port_config_change(sel, asic_context, stop_event, port_mapping, logger, port_change_event_handler):