Skip to content

[sfp] Fix issue: Application Advertisement is not well formatted #2491

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 7 commits into from
Nov 19, 2022
Merged

[sfp] Fix issue: Application Advertisement is not well formatted #2491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5cdba83
[sfp] Fix issue: Application Advertisement is not well formatted
Junchao-Mellanox Nov 8, 2022
6a908e8
Fix unit test issue
Junchao-Mellanox Nov 10, 2022
58ac8c6
Merge remote-tracking branch 'origin/master' into format-app-adv
Junchao-Mellanox Nov 11, 2022
52d6c81
Fix review comment
Junchao-Mellanox Nov 11, 2022
04bd602
Fix LGTM
Junchao-Mellanox Nov 11, 2022
d6b4a8f
Fix review comment
Junchao-Mellanox Nov 16, 2022
ea5452b
Merge branch 'sonic-net:master' into format-app-adv
Junchao-Mellanox Nov 18, 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
23 changes: 4 additions & 19 deletions scripts/sfpshow
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import click
from natsort import natsorted
from sonic_py_common.interface import front_panel_prefix, backplane_prefix, inband_prefix, recirc_prefix
from sonic_py_common import multi_asic
from utilities_common.sfp_helper import covert_application_advertisement_to_output_string
from utilities_common.sfp_helper import QSFP_DATA_MAP
from tabulate import tabulate

# Mock the redis DB for unit test purposes
Expand All @@ -38,25 +40,6 @@ from utilities_common import multi_asic as multi_asic_util
from utilities_common.platform_sfputil_helper import is_rj45_port, RJ45_PORT_TYPE

# TODO: We should share these maps and the formatting functions between sfputil and sfpshow
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this TODO is still valid? what is missing to close it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is still valid. Now show interface transceiver eeprom and sfputil show eeprom have very similar output. However, these two commands have their own implementation. I suppose they shall share the same implementation, but I did not do it in this PR because it would be a big change.

QSFP_DATA_MAP = {
'model': 'Vendor PN',
'vendor_oui': 'Vendor OUI',
'vendor_date': 'Vendor Date Code(YYYY-MM-DD Lot)',
'manufacturer': 'Vendor Name',
'vendor_rev': 'Vendor Rev',
'serial': 'Vendor SN',
'type': 'Identifier',
'ext_identifier': 'Extended Identifier',
'ext_rateselect_compliance': 'Extended RateSelect Compliance',
'cable_length': 'cable_length',
'cable_type': 'Length',
'nominal_bit_rate': 'Nominal Bit Rate(100Mbs)',
'specification_compliance': 'Specification compliance',
'encoding': 'Encoding',
'connector': 'Connector',
'application_advertisement': 'Application Advertisement'
}

SFP_DOM_CHANNEL_MONITOR_MAP = {
'rx1power': 'RXPower',
'tx1bias': 'TXBias',
Expand Down Expand Up @@ -287,6 +270,8 @@ class SFPShow(object):
output += '{}{}: {}\n'.format((indent * 2), compliance_key, spec_compliance_dict[compliance_key])
except ValueError as e:
output += '{}N/A\n'.format((indent * 2))
elif key == 'application_advertisement':
output += covert_application_advertisement_to_output_string(indent, sfp_info_dict)
else:
output += '{}{}: {}\n'.format(indent, QSFP_DATA_MAP[key], sfp_info_dict[key])

Expand Down
43 changes: 14 additions & 29 deletions sfputil/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from swsscommon.swsscommon import SonicV2Connector
from natsort import natsorted
from sonic_py_common import device_info, logger, multi_asic
from utilities_common.sfp_helper import covert_application_advertisement_to_output_string
from utilities_common.sfp_helper import QSFP_DATA_MAP
from tabulate import tabulate

VERSION = '3.0'
Expand Down Expand Up @@ -50,25 +52,6 @@
SFF8472_A0_SIZE = 256

# TODO: We should share these maps and the formatting functions between sfputil and sfpshow
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

QSFP_DATA_MAP = {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is still used by QSFP28/and QSFP+

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not remove it. I just move it to a common place. See sfp_helper.py.

'model': 'Vendor PN',
'vendor_oui': 'Vendor OUI',
'vendor_date': 'Vendor Date Code(YYYY-MM-DD Lot)',
'manufacturer': 'Vendor Name',
'vendor_rev': 'Vendor Rev',
'serial': 'Vendor SN',
'type': 'Identifier',
'ext_identifier': 'Extended Identifier',
'ext_rateselect_compliance': 'Extended RateSelect Compliance',
'cable_length': 'cable_length',
'cable_type': 'Length',
'nominal_bit_rate': 'Nominal Bit Rate(100Mbs)',
'specification_compliance': 'Specification compliance',
'encoding': 'Encoding',
'connector': 'Connector',
'application_advertisement': 'Application Advertisement'
}

QSFP_DD_DATA_MAP = {
'model': 'Vendor PN',
'vendor_oui': 'Vendor OUI',
Expand Down Expand Up @@ -350,6 +333,8 @@ def convert_sfp_info_to_output_string(sfp_info_dict):
elif key == 'supported_max_laser_freq' or key == 'supported_min_laser_freq':
if key in sfp_info_dict: # C-CMIS compliant / coherent modules
output += '{}{}: {}GHz\n'.format(indent, QSFP_DD_DATA_MAP[key], sfp_info_dict[key])
elif key == 'application_advertisement':
output += covert_application_advertisement_to_output_string(indent, sfp_info_dict)
else:
try:
output += '{}{}: {}\n'.format(indent, QSFP_DD_DATA_MAP[key], sfp_info_dict[key])
Expand Down Expand Up @@ -1358,8 +1343,8 @@ def download_firmware(port_name, filepath):
def run(port_name, mode):
"""Run the firmware with default mode=1"""

if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
sys.exit(EXIT_FAIL)

if not is_sfp_present(port_name):
Expand All @@ -1379,8 +1364,8 @@ def run(port_name, mode):
def commit(port_name):
"""Commit the running firmware"""

if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
sys.exit(EXIT_FAIL)

if not is_sfp_present(port_name):
Expand All @@ -1403,8 +1388,8 @@ def upgrade(port_name, filepath):

physical_port = logical_port_to_physical_port_index(port_name)

if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
sys.exit(EXIT_FAIL)

if not is_sfp_present(port_name):
Expand Down Expand Up @@ -1441,8 +1426,8 @@ def upgrade(port_name, filepath):
def download(port_name, filepath):
"""Download firmware on the transceiver"""

if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
sys.exit(EXIT_FAIL)

if not is_sfp_present(port_name):
Expand All @@ -1469,8 +1454,8 @@ def unlock(port_name, password):
physical_port = logical_port_to_physical_port_index(port_name)
sfp = platform_chassis.get_sfp(physical_port)

if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
if is_port_type_rj45(port_name):
click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
sys.exit(EXIT_FAIL)

if not is_sfp_present(port_name):
Expand Down
26 changes: 24 additions & 2 deletions tests/mock_tables/state_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"manufacturer": "INNOLIGHT",
"model": "C-DQ8FNM010-N00",
"vendor_oui": "44-7c-7f",
"vendor_date": "2020-05-22 ",
"vendor_date": "2020-05-22",
"connector": "No separable connector",
"encoding": "Not supported for CMIS cables",
"ext_identifier": "Power Class 1(10.0W Max)",
Expand All @@ -83,7 +83,7 @@
"cable_length": "10",
"specification_compliance": "Not supported for CMIS cables",
"nominal_bit_rate": "Not supported for CMIS cables",
"application_advertisement": "400GAUI-8 C2M (Annex 120E) - Active Cable assembly with BER < 2.6x10^-4\n\t\t\t\t IB EDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 5x10^-5\n\t\t\t\t IB QDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 10^-12\n\t\t\t\t "
"application_advertisement": "400GAUI-8 C2M (Annex 120E) - Active Cable assembly with BER < 2.6x10^-4\n\t\t\t\t IB EDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 5x10^-5\n\t\t\t\t IB QDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 10^-12"
},
"TRANSCEIVER_DOM_SENSOR|Ethernet8": {
"temperature": "44.9883",
Expand Down Expand Up @@ -205,6 +205,24 @@
"nominal_bit_rate": "N/A",
"application_advertisement": "N/A"
},
"TRANSCEIVER_INFO|Ethernet40": {
"type": "QSFP-DD Double Density 8X Pluggable Transceiver",
"vendor_rev": "2A",
"serial": "INKAO2900002A",
"manufacturer": "INNOLIGHT",
"model": "C-DQ8FNM010-N00",
"vendor_oui": "44-7c-7f",
"vendor_date": "2020-05-22",
"connector": "No separable connector",
"encoding": "Not supported for CMIS cables",
"ext_identifier": "Power Class 1(10.0W Max)",
"ext_rateselect_compliance": "Not supported for CMIS cables",
"cable_type": "Length Cable Assembly(m)",
"cable_length": "10",
"specification_compliance": "Not supported for CMIS cables",
"nominal_bit_rate": "Not supported for CMIS cables",
"application_advertisement": "{1: {'host_electrical_interface_id': '400G CR8', 'module_media_interface_id': 'Copper cable', 'media_lane_count': 8, 'host_lane_count': 8, 'host_lane_assignment_options': 1, 'media_lane_assignment_options': 2}, 2: {'host_electrical_interface_id': '200GBASE-CR4 (Clause 136)'}}"
},
"TRANSCEIVER_STATUS|Ethernet0": {
"status": "67",
"error": "Blocking Error|High temperature"
Expand Down Expand Up @@ -233,6 +251,10 @@
"status": "255",
"error": "Unknown"
},
"TRANSCEIVER_STATUS|Ethernet40": {
"status": "0",
"error": "N/A"
},
"CHASSIS_INFO|chassis 1": {
"psu_num": "2"
},
Expand Down
53 changes: 39 additions & 14 deletions tests/sfp_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
Application Advertisement: 400GAUI-8 C2M (Annex 120E) - Active Cable assembly with BER < 2.6x10^-4
IB EDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 5x10^-5
IB QDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 10^-12

Connector: No separable connector
Encoding: Not supported for CMIS cables
Extended Identifier: Power Class 1(10.0W Max)
Expand All @@ -75,13 +74,13 @@
Length Cable Assembly(m): 10
Nominal Bit Rate(100Mbs): Not supported for CMIS cables
Specification compliance: Not supported for CMIS cables
Vendor Date Code(YYYY-MM-DD Lot): 2020-05-22
Vendor Date Code(YYYY-MM-DD Lot): 2020-05-22
Vendor Name: INNOLIGHT
Vendor OUI: 44-7c-7f
Vendor PN: C-DQ8FNM010-N00
Vendor Rev: 2A
Vendor SN: INKAO2900002A
ChannelMonitorValues:
ChannelMonitorValues:
RX1Power: -3.8595dBm
RX2Power: 8.1478dBm
RX3Power: -22.9243dBm
Expand All @@ -98,15 +97,15 @@
TX3Power: 1.175dBm
TX4Bias: 0.0000mA
TX4Power: 1.175dBm
TX5Bias: 0.0000mAmA
TX5Bias: 0.0000mA
TX5Power: 1.175dBm
TX6Bias: 8.2240mAmA
TX6Bias: 8.2240mA
TX6Power: 1.175dBm
TX7Bias: 8.2240mAmA
TX7Bias: 8.2240mA
TX7Power: 1.175dBm
TX8Bias: 8.2240mAmA
TX8Bias: 8.2240mA
TX8Power: 1.175dBm
ChannelThresholdValues:
ChannelThresholdValues:
RxPowerHighAlarm : 6.9999dBm
RxPowerHighWarning: 4.9999dBm
RxPowerLowAlarm : -11.9044dBm
Expand All @@ -119,10 +118,10 @@
TxPowerHighWarning: 4.9999dBm
TxPowerLowAlarm : -10.5012dBm
TxPowerLowWarning : -7.5007dBm
ModuleMonitorValues:
ModuleMonitorValues:
Temperature: 44.9883C
Vcc: 3.2999Volts
ModuleThresholdValues:
ModuleThresholdValues:
TempHighAlarm : 80.0000C
TempHighWarning: 75.0000C
TempLowAlarm : -10.0000C
Expand Down Expand Up @@ -158,7 +157,6 @@
Application Advertisement: 400GAUI-8 C2M (Annex 120E) - Active Cable assembly with BER < 2.6x10^-4
IB EDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 5x10^-5
IB QDR (Arch.Spec.Vol.2) - Active Cable assembly with BER < 10^-12

Connector: No separable connector
Encoding: Not supported for CMIS cables
Extended Identifier: Power Class 1(10.0W Max)
Expand All @@ -167,7 +165,27 @@
Length Cable Assembly(m): 10
Nominal Bit Rate(100Mbs): Not supported for CMIS cables
Specification compliance: Not supported for CMIS cables
Vendor Date Code(YYYY-MM-DD Lot): 2020-05-22
Vendor Date Code(YYYY-MM-DD Lot): 2020-05-22
Vendor Name: INNOLIGHT
Vendor OUI: 44-7c-7f
Vendor PN: C-DQ8FNM010-N00
Vendor Rev: 2A
Vendor SN: INKAO2900002A
"""

test_qsfp_dd_eeprom_adv_app_output = """\
Ethernet40: SFP EEPROM detected
Application Advertisement: 400G CR8 - Host Assign (0x1) - Copper cable - Media Assign (0x2)
200GBASE-CR4 (Clause 136) - Host Assign (Unknown) - Unknown - Media Assign (Unknown)
Connector: No separable connector
Encoding: Not supported for CMIS cables
Extended Identifier: Power Class 1(10.0W Max)
Extended RateSelect Compliance: Not supported for CMIS cables
Identifier: QSFP-DD Double Density 8X Pluggable Transceiver
Length Cable Assembly(m): 10
Nominal Bit Rate(100Mbs): Not supported for CMIS cables
Specification compliance: Not supported for CMIS cables
Vendor Date Code(YYYY-MM-DD Lot): 2020-05-22
Vendor Name: INNOLIGHT
Vendor OUI: 44-7c-7f
Vendor PN: C-DQ8FNM010-N00
Expand Down Expand Up @@ -390,7 +408,7 @@ def test_qsfp_dd_eeprom_with_dom(self):
runner = CliRunner()
result = runner.invoke(show.cli.commands["interfaces"].commands["transceiver"].commands["eeprom"], ["Ethernet8 -d"])
assert result.exit_code == 0
assert "result.output == test_qsfp_dd_eeprom_with_dom_output"
assert result.output == test_qsfp_dd_eeprom_with_dom_output

def test_sfp_eeprom(self):
runner = CliRunner()
Expand All @@ -407,7 +425,14 @@ def test_qsfp_dd_eeprom(self):
runner = CliRunner()
result = runner.invoke(show.cli.commands["interfaces"].commands["transceiver"].commands["eeprom"], ["Ethernet8"])
assert result.exit_code == 0
assert "result.output == test_qsfp_dd_eeprom_output"
assert result.output == test_qsfp_dd_eeprom_output

def test_qsfp_dd_eeprom_adv_app(self):
runner = CliRunner()
result = runner.invoke(show.cli.commands["interfaces"].commands["transceiver"].commands["eeprom"], ["Ethernet40"])
assert result.exit_code == 0
print(result.output)
assert result.output == test_qsfp_dd_eeprom_adv_app_output

def test_rj45_eeprom(self):
runner = CliRunner()
Expand Down
19 changes: 15 additions & 4 deletions tests/sfputil_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def test_format_dict_value_to_string(self):
sfputil.QSFP_DOM_CHANNEL_MONITOR_MAP,
sfputil.DOM_VALUE_UNIT_MAP)
assert output == expected_output

@pytest.mark.parametrize("sfp_info_dict, expected_output",[
# Non-CMIS module
(
Expand Down Expand Up @@ -140,7 +141,13 @@ def test_format_dict_value_to_string(self):
'ext_rateselect_compliance': 'N/A',
'cable_type': 'Length Cable Assembly(m)',
'cable_length': '0',
'application_advertisement': 'N/A',
'application_advertisement': "{1: {'host_electrical_interface_id': '400G CR8', \
'module_media_interface_id': 'Copper cable', \
'media_lane_count': 8, \
'host_lane_count': 8, \
'host_lane_assignment_options': 1, \
'media_lane_assignment_options': 2}, \
2: {'host_electrical_interface_id': '200GBASE-CR4 (Clause 136)'}}",
'specification_compliance': "sm_media_interface",
'dom_capability': "{'Tx_power_support': 'no', 'Rx_power_support': 'no', 'Voltage_support': 'no', 'Temp_support': 'no'}",
'nominal_bit_rate': '0',
Expand Down Expand Up @@ -178,7 +185,8 @@ def test_format_dict_value_to_string(self):
" Active App Selection Host Lane 7: 1\n"
" Active App Selection Host Lane 8: 1\n"
" Active Firmware Version: 0.1\n"
" Application Advertisement: N/A\n"
" Application Advertisement: 400G CR8 - Host Assign (0x1) - Copper cable - Media Assign (0x2)\n"
" 200GBASE-CR4 (Clause 136) - Host Assign (Unknown) - Unknown - Media Assign (Unknown)\n"
" CMIS Revision: 5.0\n"
" Connector: LC\n"
" Encoding: N/A\n"
Expand Down Expand Up @@ -279,7 +287,8 @@ def test_error_status_from_db(self):
['Ethernet12', 'Unknown state: 255'],
['Ethernet16', 'Unplugged'],
['Ethernet28', 'Unplugged'],
['Ethernet36', 'Unknown']]
['Ethernet36', 'Unknown'],
['Ethernet40', 'Unplugged']]
output = sfputil.fetch_error_status_from_state_db(None, db.db)
assert output == expected_output

Expand All @@ -296,7 +305,8 @@ def test_error_status_from_db_RJ45(self):
['Ethernet12', 'N/A'],
['Ethernet16', 'N/A'],
['Ethernet28', 'N/A'],
['Ethernet36', 'N/A']]
['Ethernet36', 'N/A'],
['Ethernet40', 'N/A']]
output = sfputil.fetch_error_status_from_state_db(None, db.db)
assert output == expected_output

Expand Down Expand Up @@ -382,6 +392,7 @@ def test_show_error_status(self):
Ethernet16 Unplugged
Ethernet28 Unplugged
Ethernet36 Unknown
Ethernet40 Unplugged
"""
assert result.output == expected_output

Expand Down
Loading