Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CMIS and preemphasis lane mapping issues #508

Open
wants to merge 4 commits into
base: 202311
Choose a base branch
from
+139 −56
Open

Fix CMIS and preemphasis lane mapping issues #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29de90a
Fix CMIS and preemphasis lane mapping issues
jemifdo Jul 3, 2024
02eca1c
CMIS will be triggered on optics OIR
jemifdo Jul 5, 2024
094e843
Remove 'subport' derivation from 'alias'
jemifdo Jul 8, 2024
894a51c
Add UT coverage
jemifdo Jul 10, 2024
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
29 changes: 22 additions & 7 deletions sonic-xcvrd/tests/test_xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand All @@ -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')
Expand Down Expand Up @@ -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:
Expand Down
63 changes: 42 additions & 21 deletions sonic-xcvrd/xcvrd/xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Expand Down
20 changes: 15 additions & 5 deletions sonic-xcvrd/xcvrd/xcvrd_utilities/media_settings_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {}
Expand All @@ -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 +
Expand Down Expand Up @@ -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)))
Expand Down
Loading
Loading