Skip to content

Retrieve subport from CONFIG_DB to enable breakout support #342

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

Merged
merged 12 commits into from
Apr 4, 2023
Merged

Retrieve subport from CONFIG_DB to enable breakout support #342

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c6bb9a7
Retrieve channel from CONFIG_DB to enable breakout support
mihirpat1 Mar 7, 2023
c13de6a
Merge branch 'master' of https://github.com/sonic-net/sonic-platform-…
mihirpat1 Mar 7, 2023
ed772fc
Enhanced test cases
mihirpat1 Mar 7, 2023
cbc7382
Moved datapath init pending status check after ConfigSuccess read to …
mihirpat1 Mar 9, 2023
b90c00a
Addressed review comments
mihirpat1 Mar 10, 2023
845934d
Moved host_lanes assignment after detecting module type as CMIS
mihirpat1 Mar 10, 2023
d1af7f1
Simplified logic to find host_lane_mask
mihirpat1 Mar 16, 2023
3678685
1. Removed redundant calls to get_cmis_application_desired
mihirpat1 Mar 17, 2023
d9634d6
1. Added testcase for is_cmis_application_update_required
mihirpat1 Mar 21, 2023
5900348
Added more details in log for aplication miismatch on different lanes
mihirpat1 Mar 21, 2023
ab69aa6
Replaced channel with subport
mihirpat1 Mar 27, 2023
9b61a80
Modified comment to describe subport meaning
mihirpat1 Apr 4, 2023
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
46 changes: 46 additions & 0 deletions sonic-xcvrd/tests/test_xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from xcvrd.xcvrd_utilities.port_mapping import *
from xcvrd.xcvrd_utilities.sfp_status_helper import *
from xcvrd.xcvrd import *
import pytest
import copy
import os
import sys
Expand Down Expand Up @@ -653,6 +654,51 @@ def test_CmisManagerTask_task_run_stop(self, mock_chassis):
cmis_manager.join()
assert not cmis_manager.is_alive()

@pytest.mark.parametrize("host_lane_count, speed, channel, expected", [
(8, 400000, 0, 0xFF),
(4, 100000, 1, 0xF),
(4, 100000, 2, 0xF0),
(4, 100000, 0, 0xF),
(4, 100000, 9, 0x0),
(1, 50000, 2, 0x2),
(1, 200000, 2, 0x0)
])
def test_CmisManagerTask_get_cmis_host_lane_mask(self, host_lane_count, speed, channel, expected):
appl_advert_dict = {
1: {
'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)',
'module_media_interface_id': '400GBASE-DR4 (Cl 124)',
'media_lane_count': 4,
'host_lane_count': 8,
'host_lane_assignment_options': 1
},
2: {
'host_electrical_interface_id': 'CAUI-4 C2M (Annex 83E)',
'module_media_interface_id': 'Active Cable assembly with BER < 5x10^-5',
'media_lane_count': 4,
'host_lane_count': 4,
'host_lane_assignment_options': 17
},
3: {
'host_electrical_interface_id': '50GAUI-1 C2M',
'module_media_interface_id': '50GBASE-SR',
'media_lane_count': 1,
'host_lane_count': 1,
'host_lane_assignment_options': 255
}
}
mock_xcvr_api = MagicMock()
mock_xcvr_api.get_application_advertisement = MagicMock(return_value=appl_advert_dict)

def get_host_lane_assignment_option_side_effect(app):
return appl_advert_dict[app]['host_lane_assignment_options']
mock_xcvr_api.get_host_lane_assignment_option = MagicMock(side_effect=get_host_lane_assignment_option_side_effect)
port_mapping = PortMapping()
stop_event = threading.Event()
task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event)

assert task.get_cmis_host_lane_mask(mock_xcvr_api, host_lane_count, channel, speed) == expected

@patch('xcvrd.xcvrd.platform_chassis')
@patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_update_event', MagicMock(return_value=(None, None)))
@patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_update_event', MagicMock())
Expand Down
116 changes: 81 additions & 35 deletions sonic-xcvrd/xcvrd/xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,8 @@ def on_port_update_event(self, port_change_event):
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 'channel' in port_change_event.port_dict:
self.port_dict[lport]['channel'] = int(port_change_event.port_dict['channel'])

self.force_cmis_reinit(lport, 0)
else:
Expand Down Expand Up @@ -1058,31 +1060,24 @@ def get_interface_speed(self, ifname):
speed = 1000
return speed

def get_cmis_application_desired(self, api, channel, speed):
def get_cmis_application_desired(self, api, host_lane_count, speed):
"""
Get the CMIS application code that matches the specified host side configurations

Args:
api:
XcvrApi object
channel:
Integer, a bitmask of the lanes on the host side
e.g. 0x5 for lane 0 and lane 2.
host_lane_count:
Number of lanes on the host side
speed:
Integer, the port speed of the host interface

Returns:
Integer, the transceiver-specific application code
"""
if speed == 0 or channel == 0:
if speed == 0 or host_lane_count == 0:
return 0

host_lane_count = 0
for lane in range(self.CMIS_NUM_CHANNELS):
if ((1 << lane) & channel) == 0:
continue
host_lane_count += 1

appl_code = 0
appl_dict = api.get_application_advertisement()
for c in appl_dict.keys():
Expand All @@ -1102,13 +1097,63 @@ def get_cmis_dp_init_duration_secs(self, api):
def get_cmis_dp_deinit_duration_secs(self, api):
return api.get_datapath_deinit_duration()/1000

def is_cmis_application_update_required(self, api, channel, speed):
def get_cmis_host_lane_mask(self, api, host_lane_count, channel, speed):
"""
Retrieves mask of active host lanes based on host lane count, channel and speed

Args:
api:
XcvrApi object
host_lane_count:
Integer, number of lanes on the host side
channel:
Integer, channel id of the group which the host lanes belong to (1 based)
speed:
Integer, the port speed of the host interface

Returns:
Integer, a mask of the active lanes on the host side
e.g. 0x5 for lane 0 and lane 2.
"""
host_lane_mask = 0

if host_lane_count <= 0 or channel < 0 or speed <= 0:
self.log_error("Invalid input to get host lane mask - host_lane_count {} channel {} speed {}!".format(
host_lane_count, channel, speed))
return host_lane_mask

if channel != 0:
appl = self.get_cmis_application_desired(api, host_lane_count, speed)
if appl < 1:
self.log_error("Failed to get host lanes as no suitable app {} found host_lane_count {} speed {}".format(
appl, host_lane_count, speed))
return host_lane_mask

host_lane_assignment_option = api.get_host_lane_assignment_option(appl)
curr_channel_num = 0
for bit in range(8):
mask = 1 << bit
if host_lane_assignment_option & mask != 0:
curr_channel_num += 1
if curr_channel_num == channel:
for i in range(host_lane_count):
host_lane_mask |= mask
mask = mask << 1
break
else:
host_lane_mask = (1 << host_lane_count) - 1

return host_lane_mask

def is_cmis_application_update_required(self, api, host_lane_count, channel, speed):
"""
Check if the CMIS application update is required

Args:
api:
XcvrApi object
host_lane_count:
Number of lanes on the host side
channel:
Integer, a bitmask of the lanes on the host side
e.g. 0x5 for lane 0 and lane 2.
Expand All @@ -1121,7 +1166,7 @@ def is_cmis_application_update_required(self, api, channel, speed):
if speed == 0 or channel == 0 or api.is_flat_memory():
return False

app_new = self.get_cmis_application_desired(api, channel, speed)
app_new = self.get_cmis_application_desired(api, host_lane_count, speed)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. current logic checks for all 8 host lanes whereas we should care for the requested host lanes only
  2. Also we should make use of the selected/desired application during init state and compare it with the current active application
  3. Also we should check for DP != Activated early in the function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have handled #2 now. As discussed, we are already handling #1 and #3 and have changed CMIS_NUM_CHANNELS to CMIS_MAX_HOST_LANES.

if app_new != 1:
self.log_notice("Non-default application is not supported")

Expand Down Expand Up @@ -1392,19 +1437,13 @@ def task_worker(self):
pport = int(info.get('index', "-1"))
speed = int(info.get('speed', "0"))
lanes = info.get('lanes', "").strip()
channel = info.get('channel', 0)
if pport < 0 or speed == 0 or len(lanes) < 1:
continue

# Desired port speed on the host side
host_speed = speed

# Convert the physical lane list into a logical lanemask
#
# TODO: Add dynamic port breakout support by checking the physical lane offset
host_lanes = 0
phys_lanes = lanes.split(',')
for i in range(len(phys_lanes)):
host_lanes |= (1 << i)
host_lane_count = len(lanes.split(','))

# double-check the HW presence before moving forward
sfp = platform_chassis.get_sfp(pport)
Expand Down Expand Up @@ -1432,6 +1471,13 @@ def task_worker(self):
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY
continue

host_lanes = self.get_cmis_host_lane_mask(api, host_lane_count, channel, speed)
if host_lanes == 0:
self.log_error("{}: Invalid lane mask received host_lane_count {} channel {} speed {}!".format(
lport, host_lane_count, channel, speed))
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED
continue

if api.is_coherent_module():
if 'tx_power' not in self.port_dict[lport]:
self.port_dict[lport]['tx_power'] = self.get_configured_tx_power_from_db(lport)
Expand Down Expand Up @@ -1477,13 +1523,13 @@ def task_worker(self):
else:
self.log_notice("{} Successfully configured Tx power = {}".format(lport, tx_power))

appl = self.get_cmis_application_desired(api, host_lanes, host_speed)
appl = self.get_cmis_application_desired(api, host_lane_count, host_speed)
if appl < 1:
self.log_error("{}: no suitable app for the port".format(lport))
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED
continue

need_update = self.is_cmis_application_update_required(api, host_lanes, host_speed)
need_update = self.is_cmis_application_update_required(api, host_lane_count, host_lanes, host_speed)

# For ZR module, Datapath needes to be re-initlialized on new channel selection
if api.is_coherent_module():
Expand All @@ -1510,11 +1556,11 @@ def task_worker(self):
self.port_dict[lport]['cmis_retries'] = retries + 1
continue

# TODO: Make sure this doesn't impact other datapaths
#Sets module to high power mode and doesn't impact datapath if module is already in high power mode
api.set_lpmode(False)
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF
dpDeinitDuration = self.get_cmis_dp_deinit_duration_secs(api)
self.log_notice("{} DpDeinit duration {} secs".format(lport, dpDeinitDuration))
self.log_notice("{}: DpDeinit duration {} secs".format(lport, dpDeinitDuration))
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=dpDeinitDuration)
elif state == self.CMIS_STATE_AP_CONF:
# TODO: Use fine grained time when the CMIS memory map is available
Expand All @@ -1540,7 +1586,7 @@ def task_worker(self):
self.log_notice("{} configured laser frequency {} GHz".format(lport, freq))

# D.1.3 Software Configuration and Initialization
appl = self.get_cmis_application_desired(api, host_lanes, host_speed)
appl = self.get_cmis_application_desired(api, host_lane_count, host_speed)
if appl < 1:
self.log_error("{}: no suitable app for the port".format(lport))
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED
Expand All @@ -1551,14 +1597,6 @@ def task_worker(self):
self.force_cmis_reinit(lport, retries + 1)
continue

if getattr(api, 'get_cmis_rev', None):
# 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):
self.log_notice("{}: datapath init not pending".format(lport))
self.force_cmis_reinit(lport, retries + 1)
continue

self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_INIT
elif state == self.CMIS_STATE_DP_INIT:
if not self.check_config_error(api, host_lanes, ['ConfigSuccess']):
Expand All @@ -1567,6 +1605,14 @@ def task_worker(self):
self.force_cmis_reinit(lport, retries + 1)
continue

if getattr(api, 'get_cmis_rev', None):
# 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):
self.log_notice("{}: datapath init not pending".format(lport))
self.force_cmis_reinit(lport, retries + 1)
continue

# Ensure the Datapath is NOT Activated unless the host Tx siganl is good.
# NOTE: Some CMIS compliant modules may have 'auto-squelch' feature where
# the module won't take datapaths to Activated state if host tries to enable
Expand All @@ -1579,7 +1625,7 @@ def task_worker(self):
# D.1.3 Software Configuration and Initialization
api.set_datapath_init(host_lanes)
dpInitDuration = self.get_cmis_dp_init_duration_secs(api)
self.log_notice("{} DpInit duration {} secs".format(lport, dpInitDuration))
self.log_notice("{}: DpInit duration {} secs".format(lport, dpInitDuration))
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=dpInitDuration)
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON
elif state == self.CMIS_STATE_DP_TXON:
Expand Down