Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
14 changes: 14 additions & 0 deletions sonic_platform_base/sfp_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ class SfpBase(device_base.DeviceBase):
# Device type definition. Note, this is a constant.
DEVICE_TYPE = "sfp"

# SFP port type. Please note this is the cage type rather than transceiver type.
SFP_PORT_TYPE_UNSPECIFIED = "UNSPECIFIED"
SFP_PORT_TYPE_SFP = "SFP"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about other cage types (OSFP, etc)? Is "SFP" the same as "SFP+"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the cage/form-factor type, while SFP/SFP+/SFP28 shares the same cage/form-factor type, I do not know much about OSFP, hence it's not declared here.

SFP_PORT_TYPE_QSFP = "QSFP"
SFP_PORT_TYPE_QSFPDD = "QSFP_DD"

# Generic error types definition
SFP_STATUS_INITIALIZING = 'Initializing'
SFP_STATUS_OK = 'OK'
Expand Down Expand Up @@ -59,6 +65,14 @@ def __init__(self):
self._xcvr_api_factory = XcvrApiFactory(self.read_eeprom, self.write_eeprom)
self._xcvr_api = None

def get_port_type(self):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i think you removed the code in xcvrd which check for cage type. can this now be removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, sure

"""
Retrieves the port/cage type of this SFP
Returns:
A string, the port/cage type of this SFP
"""
return self.SFP_PORT_TYPE_UNSPECIFIED

def get_num_thermals(self):
"""
Retrieves the number of thermals available on this SFP
Expand Down
302 changes: 299 additions & 3 deletions sonic_platform_base/sonic_xcvr/api/public/cmis.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
from ...fields import consts
from ..xcvr_api import XcvrApi

import ast
import logging
from ...codes.public.cmis import CmisCodes
from ...fields import consts
from ..xcvr_api import XcvrApi
from .cmisCDB import CmisCdbApi
Expand Down Expand Up @@ -135,8 +137,7 @@ def get_transceiver_info(self):
"specification_compliance": admin_info[consts.MEDIA_TYPE_FIELD],
"vendor_date": admin_info[consts.VENDOR_DATE_FIELD],
"vendor_oui": admin_info[consts.VENDOR_OUI_FIELD],
# TODO
"application_advertisement": "N/A",
"application_advertisement": admin_info.get(consts.APPLS_ADVT_FIELD, "N/A")
}
xcvr_info['host_electrical_interface'] = self.get_host_electrical_interface()
xcvr_info['media_interface_code'] = self.get_module_media_interface()
Expand Down Expand Up @@ -903,6 +904,9 @@ def set_lpmode(self, lpmode):

lpmode_val = self.xcvr_eeprom.read(consts.MODULE_LEVEL_CONTROL)
if lpmode_val is not None:
# Turn on software control mode
if self.xcvr_eeprom.read(consts.CMIS_MAJOR_REVISION) >= 4:
lpmode_val = lpmode_val & ~(1 << 6) # LowPwrAllowRequestHW
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LowPwrRequestSW override LowPwrAllowRequestHW, hence this change is NOT required:-

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not exactly, please refer to row 2 and 3, when LowPwrRequestSW is 0, the LowPwrState is determined by the LowPwrRequestHW (i.e. The LPMODE input pin on the board), and it's a known issue that some of the Accton SONIC platforms do not support manipulating LPMODE in its CPLD.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From the truth table, there is no operation being set for LowPwrAllowRequestHW. Your logic will set the bit 6 to zero always which is not needed when bit 4 is set 1.
image

I believe that we don't need to set bit 6 as you can see the logic from the above image.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're right, I'll remove it, thanks

if lpmode is True:
lpmode_val = lpmode_val | (1 << 4)
self.xcvr_eeprom.write(consts.MODULE_LEVEL_CONTROL, lpmode_val)
Expand All @@ -914,7 +918,7 @@ def set_lpmode(self, lpmode):
time.sleep(1)
lpmode = self.xcvr_eeprom.read(consts.TRANS_MODULE_STATUS_FIELD)
if lpmode is not None:
if lpmode.get('ModuleState') == 'ModuleReady':
if lpmode.get('ModuleState') in ['ModulePwrUp', 'ModuleReady']:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ModulePwrUp is a transient state, and there is NO guarantee for below transition:-

ModulePwrUp -> ModuleReady

Hence the API should fail ifthe state is NOT 'ModuleReady'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

return True
return False
return False
Expand Down Expand Up @@ -1745,4 +1749,296 @@ def get_transceiver_loopback(self):
for lane in range(1, self.NUM_CHANNELS+1):
trans_loopback['host_input_loopback_lane%d' % lane] = 'N/A'
return trans_loopback

def set_datapath_init(self, host_lanemask):
"""
Put the datapath into the initialized state

Args:
host_lanemask: Integer, a bitmask of the lanes on the system/host side
e.g. 0x5 for lane 0 and lane 2.

Returns:
Boolean, true if success otherwise false
"""
cmis_major = self.xcvr_eeprom.read(consts.CMIS_MAJOR_REVISION)
data = self.xcvr_eeprom.read(consts.DATAPATH_DEINIT_FIELD)
for lane in range(self.NUM_CHANNELS):
if (1 << lane) & host_lanemask == 0:
continue
if cmis_major >= 4: # CMIS v4 onwards
data &= ~(1 << lane)
else: # CMIS v3
data |= (1 << lane)
self.xcvr_eeprom.write(consts.DATAPATH_DEINIT_FIELD, data)

def set_datapath_deinit(self, host_lanemask):
"""
Put the datapath into the de-initialized state

Args:
host_lanemask: Integer, a bitmask of the lanes on the system/host side
e.g. 0x5 for lane 0 and lane 2.

Returns:
Boolean, true if success otherwise false
"""
cmis_major = self.xcvr_eeprom.read(consts.CMIS_MAJOR_REVISION)
data = self.xcvr_eeprom.read(consts.DATAPATH_DEINIT_FIELD)
for lane in range(self.NUM_CHANNELS):
if (1 << lane) & host_lanemask == 0:
continue
if cmis_major >= 4: # CMIS v4 onwards
data |= (1 << lane)
else: # CMIS v3
data &= ~(1 << lane)
self.xcvr_eeprom.write(consts.DATAPATH_DEINIT_FIELD, data)

def get_host_speed(self, ifname):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we move out this function to Xcvrd as a helper function because this is NOT specific to CMIS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

"""
Get the port speed from the host interface name

Args:
ifname: String, host interface name

Returns:
Integer, the port speed if success otherwise 0
"""
# see HOST_ELECTRICAL_INTERFACE of sff8024.py
speed = 0
if '400G' in ifname:
speed = 400000
elif '200G' in ifname:
speed = 200000
elif '100G' in ifname or 'CAUI-4' in ifname:
speed = 100000
elif '50G' in ifname or 'LAUI-2' in ifname:
speed = 50000
elif '40G' in ifname or 'XLAUI' in ifname or 'XLPPI' in ifname:
speed = 40000
elif '25G' in ifname:
speed = 25000
elif '10G' in ifname or 'SFI' in ifname or 'XFI' in ifname:
speed = 10000
elif '1000BASE' in ifname:
speed = 1000
return speed

def get_cmis_state(self):
"""
Get the CMIS states

Returns:
Dictionary, the states of module, config error and datapath
"""
state = {
'module_state': self.get_module_state(),
'config_state': self.get_config_datapath_hostlane_status(),
'datapath_state': self.get_datapath_state()
}
return state

def get_cmis_application_selected(self, host_lane):
"""
Get the CMIS selected application code of a host lane

Args:
host_lane:
Integer, the lane id on the host/system side

Returns:
Integer, the transceiver-specific application code
"""
ap_code = 0
if host_lane in range(self.NUM_CHANNELS) and not self.is_flat_memory():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you bring the is_flat_memory() check first because otherwise the first if check will always run

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

field = "{}_{}_{}".format(consts.STAGED_CTRL_APSEL_FIELD, 0, host_lane + 1)
ap_code = self.xcvr_eeprom.read(field) >> 4

return (ap_code & 0xf)

def get_cmis_application_matched(self, host_speed, host_lanemask):
"""
Get the CMIS application code that matches the specified host side configurations

Args:
host_speed:
Integer, the port speed of the host interface
host_lanemask:
Integer, a bitmask of the lanes on the host side
e.g. 0x5 for lane 0 and lane 2.

Returns:
Integer, the transceiver-specific application code
"""
if host_speed == 0 or host_lanemask == 0:
return 0

host_lane_count = 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe that we have already have a API to get host lane count. can you please check?.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

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

appl_code = 0
appl_dict = self.xcvr_eeprom.read(consts.APPLS_ADVT_FIELD)
if appl_dict is None or appl_dict.strip() in ["None", "{}", ""]:
return 0
appl_dict = ast.literal_eval(appl_dict)
for c in appl_dict.keys():
d = appl_dict[c]
if d.get('host_lane_count') != host_lane_count:
continue
if self.get_host_speed(d.get('host_electrical_interface_id')) != host_speed:
continue
appl_code = c
break

return (appl_code & 0xf)

def has_cmis_application_update(self, host_speed, host_lanemask):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can this be rename to "is_cmis_application_update_required()" ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And it's moved to Xcvrd

"""
Check for CMIS application update and retrieve the new application code

Args:
host_speed:
Integer, the port speed of the host interface
host_lanemask:
Integer, a bitmask of the lanes on the host side
e.g. 0x5 for lane 0 and lane 2.

Returns:
(has_update, new_appl)
"""
if host_speed == 0 or host_lanemask == 0 or self.is_flat_memory():
return (False, 1)

app_new = self.get_cmis_application_matched(host_speed, host_lanemask)
if app_new != 1 or host_lanemask != (1 << self.NUM_CHANNELS) - 1:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do you think the default application should ALWAYS support 8 lanes - this is not realistic assumption

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, this additional checker is removed

logger.info("Non-default application is not supported")
return (False, 1)

app_old = 0
for lane in range(self.NUM_CHANNELS):
if (1 << lane) & host_lanemask == 0:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you add extra bracket?

if ((1 << lane) & host_lanemask) == 0:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

continue
if app_old == 0:
app_old = self.get_cmis_application_selected(lane)
elif app_old != self.get_cmis_application_selected(lane):
logger.info("Not all the lanes are in the same application mode")
logger.info("Forcing application update...")
return (True, app_new)

if app_old == app_new:
skip = True
dp_state = self.get_datapath_state()
conf_state = self.get_config_datapath_hostlane_status()
for lane in range(self.NUM_CHANNELS):
if (1 << lane) & host_lanemask == 0:
continue
name = "DP{}State".format(lane + 1)
if dp_state[name] != CmisCodes.DATAPATH_STATE[4]:
skip = False
break
name = "ConfigStatusLane{}".format(lane + 1)
if conf_state[name] != CmisCodes.CONFIG_STATUS[1]:
skip = False
break
if skip:
return (False, app_old)

return (True, app_new)

def set_cmis_application_stop(self, host_lanemask):
"""
Deinitialize the datapath and turn off Tx power to the associated line lanes

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

Returns:
Boolean, true if success otherwise false
"""
# D.2.2 Software Deinitialization
self.set_datapath_deinit(host_lanemask)
self.set_lpmode(True)

# D.1.3 Software Configuration and Initialization
self.tx_disable_channel(host_lanemask, True)
self.set_lpmode(False)
return True

def set_cmis_application_apsel(self, host_lanemask, appl_code):
"""
Update the selected application code to the specified host lanes

Args:
host_lanemask:
Integer, a bitmask of the lanes on the host side
e.g. 0x5 for lane 0 and lane 2.
appl_code:
Integer, the desired application code

Returns:
Boolean, true if success otherwise false
"""
# Update the application selection
for lane in range(self.NUM_CHANNELS):
if (1 << lane) & host_lanemask == 0:
continue
addr = "{}_{}_{}".format(consts.STAGED_CTRL_APSEL_FIELD, 0, lane + 1)
data = appl_code << 4
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should not hardcode datapath id and explicit control as ZERO. see previous comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, will update the memory map for stage controls

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do you assume, the datapath ID will be always ZERO and EXPLICIT control will be ZERO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. In the case of AVAGO Optic, the EXPLICIT needs to be ZERO for 4x100G
  2. In the case of INNOLIGHT#T-DP4CNT-N00, the EXPLICIT needs to be 1 for 4x100G
  3. The first lane mark is now added to the latest commit

self.xcvr_eeprom.write(addr, data)

# Apply DataPathInit
return self.xcvr_eeprom.write("%s_%d" % (consts.STAGED_CTRL_APPLY_DPINIT_FIELD, 0), host_lanemask)

def set_cmis_application_start(self, host_lanemask):
"""
Initialize the datapath associated with the specified host lanes, while the Tx power
state of the line side will not be updated.

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

Returns:
Boolean, true if success otherwise false
"""
return self.set_datapath_init(host_lanemask)

def set_cmis_application_txon(self, host_lanemask):
"""
Turn on Tx power of the lanes on the line side associated with the specified host lanes

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

Returns:
Boolean, true if success otherwise false
"""
self.tx_disable_channel(host_lanemask, False)

def get_error_description(self):
dp_state = self.get_datapath_state()
conf_state = self.get_config_datapath_hostlane_status()
for lane in range(self.NUM_CHANNELS):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you define a function (get_cmis_num_channels()) to get the count?. Optics like DR4 has only 4 channels. we can't assume 8 as default number of channels.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, but please note these are host lanes connected to the MAC/ASIC, hence it's 8 lanes in DR4, as to QSFP+ CMIS, it's most likely 4 lanes.

e.g. Here is an example of INNOLIGHT 400G DR4, the ConfigError reside at 0x94a, while DPState are at 0x900

root@sonic:~# hexdump -C -n 256 -s 0x880 /sys/bus/i2c/devices/40-0050/eeprom
00000880  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000890  00 21 21 25 25 29 29 2d  2d ff 00 00 33 33 33 33  |.!!%%))--...3333|
000008a0  ff ff 22 22 22 22 00 00  00 00 22 22 22 22 00 00  |..""""....""""..|
000008b0  00 00 00 00 10 10 10 10  10 10 10 10 ff 00 00 33  |...............3|
000008c0  33 33 33 ff ff 22 22 22  22 00 00 00 00 22 22 22  |333..""""...."""|
000008d0  22 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |"...............|
000008e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000900  44 44 44 44 00 00 00 00  00 00 00 00 00 00 00 00  |DDDD............|
00000910  00 00 00 00 00 00 00 00  00 00 3e db 3e 51 3c ca  |..........>.>Q<.|
00000920  3d 09 00 00 00 00 00 00  00 00 9c 40 9c 40 9c 40  |=..........@.@.@|
00000930  9c 40 00 00 00 00 00 00  00 00 3a 7c 56 23 31 23  |.@........:|V#1#|
00000940  32 7e 00 00 00 00 00 00  00 00 11 11 11 11 21 21  |2~............!!|
00000950  25 25 29 29 2d 2d ff 00  00 33 33 33 33 ff ff 22  |%%))--...3333.."|
00000960  22 22 22 00 00 00 00 22  22 22 22 00 00 00 00 00  |"""...."""".....|
00000970  11 12 13 14 00 00 00 00  11 12 13 14 00 00 00 00  |................|

name = "DP{}State".format(lane + 1)
if dp_state[name] != CmisCodes.DATAPATH_STATE[4]:
return dp_state[name]

name = "ConfigStatusLane{}".format(lane + 1)
if conf_state[name] != CmisCodes.CONFIG_STATUS[1]:
return conf_state[name]

state = self.get_module_state()
if state != CmisCodes.MODULE_STATE[3]:
return state

return None

# TODO: other XcvrApi methods
2 changes: 1 addition & 1 deletion sonic_platform_base/sonic_xcvr/api/public/cmisCDB.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def __init__(self, xcvr_eeprom):
super(CmisCdbApi, self).__init__(xcvr_eeprom)
self.cdb_instance_supported = self.xcvr_eeprom.read(consts.CDB_SUPPORT)
self.failed_status_dict = self.xcvr_eeprom.mem_map.codes.CDB_FAIL_STATUS
assert self.cdb_instance_supported != 0
#assert self.cdb_instance_supported != 0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This leads to a crash on my 400G DR4 optic (CMIS v4, AVAGO#AFCT-93DRPHZ-AZ2)
i.e.
admin@sonic:$ sudo sfputil show eeprom -p Ethernet0
Traceback (most recent call last):
File "/usr/local/bin/sfputil", line 8, in
sys.exit(cli())
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 764, in call
return self.main(*args, **kwargs)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 717, in main
rv = self.invoke(ctx)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 956, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 555, in invoke
return callback(*args, **kwargs)
File "/usr/local/lib/python3.7/dist-packages/sfputil/main.py", line 562, in eeprom
xcvr_info = platform_chassis.get_sfp(physical_port).get_transceiver_info()
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sonic_xcvr/sfp_optoe_base.py", line 23, in get_transceiver_info
api = self.get_xcvr_api()
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sfp_base.py", line 451, in get_xcvr_api
self.refresh_xcvr_api()
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sfp_base.py", line 441, in refresh_xcvr_api
self._xcvr_api = self._xcvr_api_factory.create_xcvr_api()
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sonic_xcvr/xcvr_api_factory.py", line 47, in create_xcvr_api
api = CmisApi(xcvr_eeprom)
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sonic_xcvr/api/public/cmis.py", line 29, in init
self.cdb = CmisCdbApi(xcvr_eeprom)
File "/usr/local/lib/python3.7/dist-packages/sonic_platform_base/sonic_xcvr/api/public/cmisCDB.py", line 29, in init
assert self.cdb_instance_supported != 0
AssertionError
admin@sonic:
$


def cdb1_chkflags(self):
'''
Expand Down
Loading