Skip to content

Fix xcvrd to support 400G ZR optic #293

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
prgeor merged 9 commits into from
Oct 20, 2022
Merged

Fix xcvrd to support 400G ZR optic #293

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ccbad28
Fix xcvrd to support 400G ZR/DR optics
abohanyang Sep 16, 2022
f7f910b
Fix xcvrd to support 400G ZR/DR optics
abohanyang Sep 16, 2022
97a6780
Revert changes in DomInfoUpdateTask::on_port_config_change
abohanyang Sep 19, 2022
96b025d
Fix xcvrd test after modifying on_port_config_change
abohanyang Sep 19, 2022
8fd66f6
Call get_datapath_init_duration to get the init expiration time.
abohanyang Sep 29, 2022
3a3b96a
Address comments
abohanyang Oct 1, 2022
8b52afc
1. Revert changes in on_port_update_event
abohanyang Oct 4, 2022
88641db
Add code coverage
abohanyang Oct 10, 2022
ea29589
Add log for DpInit/DpDeinit duration
abohanyang Oct 19, 2022
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
13 changes: 13 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 @@ -514,6 +514,19 @@ def test_CmisManagerTask_task_worker(self, mock_chassis):
mock_xcvr_api.get_tx_config_power = MagicMock(return_value=0)
mock_xcvr_api.get_laser_config_freq = MagicMock(return_value=0)
mock_xcvr_api.get_module_type_abbreviation = MagicMock(return_value='QSFP-DD')
mock_xcvr_api.get_datapath_init_duration = MagicMock(return_value=60000.0)
mock_xcvr_api.get_datapath_deinit_duration = MagicMock(return_value=600000.0)
mock_xcvr_api.get_cmis_rev = MagicMock(return_value='5.0')
mock_xcvr_api.get_dpinit_pending = MagicMock(return_value={
'DPInitPending1': True,
'DPInitPending2': True,
'DPInitPending3': True,
'DPInitPending4': True,
'DPInitPending5': True,
'DPInitPending6': True,
'DPInitPending7': True,
'DPInitPending8': True
})
mock_xcvr_api.get_application_advertisement = MagicMock(return_value={
1: {
'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)',
Expand Down
47 changes: 42 additions & 5 deletions sonic-xcvrd/xcvrd/xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,12 @@ def get_cmis_application_desired(self, api, channel, speed):

return (appl_code & 0xf)

def get_cmis_dp_init_duration_secs(self, api):
return api.get_datapath_init_duration()/1000

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):
"""
Check if the CMIS application update is required
Expand Down Expand Up @@ -1176,6 +1182,32 @@ def check_config_error(self, api, channel, states):

return done

def check_datapath_init_pending(self, api, channel):
"""
Check if the CMIS datapath init is pending

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

Returns:
Boolean, true if all lanes are pending datapath init, otherwise false
"""
pending = True
dpinit_pending_dict = api.get_dpinit_pending()
for lane in range(self.CMIS_NUM_CHANNELS):
if ((1 << lane) & channel) == 0:
continue
key = "DPInitPending{}".format(lane + 1)
if not dpinit_pending_dict[key]:
pending = False
break

return pending

Comment on lines +1201 to +1210
Copy link
Copy Markdown
Collaborator

@prgeor prgeor Oct 4, 2022

Choose a reason for hiding this comment

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

I think we should check if DpInitPending is set for ALL host lanes and return TRUE. Return False if any of the lanes has DpInitPending=0

image

Copy link
Copy Markdown

@shyam77git shyam77git Oct 5, 2022

Choose a reason for hiding this comment

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

On further thought and checking , please look further into this.
should this be interpreted as: as long as one of the lanes is found in DPInitPending state, outcome is pending (i.e. true). Caller of this function on getting 'true', would perform force_cmis_reinit() instead of setting state to CMIS_STATE_DP_INIT (& proceeding with normal workflow)

def check_datapath_state(self, api, channel, states):
"""
Check if the CMIS datapath states are in the specified state
Expand Down Expand Up @@ -1458,7 +1490,7 @@ def task_worker(self):
# TODO: Make sure this doesn't impact other datapaths
api.set_lpmode(False)
self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_AP_CONF
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED)
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_deinit_duration_secs(api))
Copy link
Copy Markdown
Collaborator

@prgeor prgeor Oct 18, 2022

Choose a reason for hiding this comment

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

can you log the deinit duration?

Copy link
Copy Markdown
Contributor Author

@abohanyang abohanyang Oct 19, 2022

Choose a reason for hiding this comment

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

Sure. I just update the code and the summary of this PR.

elif state == self.CMIS_STATE_AP_CONF:
# TODO: Use fine grained time when the CMIS memory map is available
if not self.check_module_state(api, ['ModuleReady']):
Expand Down Expand Up @@ -1494,8 +1526,14 @@ def task_worker(self):
self.force_cmis_reinit(lport, retries + 1)
continue

# TODO: Use fine grained time when the CMIS memory map is available
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED)
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 @@ -1515,8 +1553,7 @@ def task_worker(self):

# D.1.3 Software Configuration and Initialization
api.set_datapath_init(host_lanes)
# TODO: Use fine grained timeout when the CMIS memory map is available
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.CMIS_DEF_EXPIRED)
self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds=self.get_cmis_dp_init_duration_secs(api))
Copy link
Copy Markdown
Collaborator

@prgeor prgeor Oct 18, 2022

Choose a reason for hiding this comment

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

can you please log the dp_init pending duration?

Copy link
Copy Markdown
Contributor Author

@abohanyang abohanyang Oct 19, 2022

Choose a reason for hiding this comment

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

Oct 19 19:10:24.470226 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=INSERTED, retries=0
Oct 19 19:10:24.584837 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: force Datapath reinit
Oct 19 19:10:25.589603 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_DEINIT, retries=0
Oct 19 19:10:26.621621 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256 DpDeinit duration 1.0 secs
Oct 19 19:10:27.627613 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=AP_CONFIGURED, retries=0
Oct 19 19:10:28.711402 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_INIT, retries=0
Oct 19 19:10:28.729570 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256 DpInit duration 300.0 secs
Oct 19 19:10:29.735655 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_TXON, retries=0
Oct 19 19:11:26.497042 cmp206-4 NOTICE pmon#xcvrd[38]: message repeated 56 times: [ CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_TXON, retries=0]
Oct 19 19:11:26.497149 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: Turning ON tx power
Oct 19 19:11:27.502474 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_ACTIVATION, retries=0
Oct 19 19:11:48.910446 cmp206-4 NOTICE pmon#xcvrd[38]: message repeated 21 times: [ CMIS: Ethernet256: 400G, lanemask=0xff, state=DP_ACTIVATION, retries=0]
Oct 19 19:11:48.910446 cmp206-4 NOTICE pmon#xcvrd[38]: CMIS: Ethernet256: READY

self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_DP_TXON
elif state == self.CMIS_STATE_DP_TXON:
if not self.check_datapath_state(api, host_lanes, ['DataPathInitialized']):
Expand Down