Fix show int transceiver EEPROM crash for for Backplane cartridge + enhance EEPROM CLI output (sonic-net#4020)

* Fix show int transceiver EEPROM crash for for Backplane cartridge + enhance EEPROM CLI output

Signed-off-by: Mihir Patel <[email protected]>

* Fixed SA warning

* Added None check for sfp_firmware_info_dict

* Added is_transceiver_c_cmis

* Added new line before get_data_map_sort_key

---------

Signed-off-by: Mihir Patel <[email protected]>

Author: mihirpat1

scripts/sfpshow

@@ -27,7 +27,9 @@ from utilities_common.sfp_helper import (
         CMIS_VDM_TO_LEGACY_STATUS_MAP,
         CCMIS_STATUS_MAP,
         CCMIS_VDM_TO_LEGACY_STATUS_MAP,
-        CCMIS_VDM_THRESHOLD_TO_LEGACY_DOM_THRESHOLD_MAP
+        CCMIS_VDM_THRESHOLD_TO_LEGACY_DOM_THRESHOLD_MAP,
+        get_data_map_sort_key,
+        get_transceiver_data_map
 )
 from utilities_common.sfp_helper import is_transceiver_cmis
 from tabulate import tabulate
@@ -317,15 +319,18 @@ class SFPShow(object):
         is_sfp_cmis = is_transceiver_cmis(sfp_info_dict)
         is_sfp_c_cmis = 'supported_max_tx_power' in sfp_info_dict
 
-        if is_sfp_c_cmis:
-            data_map = C_CMIS_DATA_MAP
-        elif is_sfp_cmis:
-            data_map = CMIS_DATA_MAP
-        else:
-            data_map = QSFP_DATA_MAP
+        # Get the appropriate data map for this transceiver type
+        data_map = get_transceiver_data_map(sfp_info_dict)
+
+        # Combine sfp_info_dict and sfp_firmware_info_dict for sorting
+        combined_dict = sfp_info_dict.copy()
+        if sfp_firmware_info_dict:
+            combined_dict.update(sfp_firmware_info_dict)
+
+        # Use the utility function to get sorted keys from combined dictionary
+        sorted_sfp_info_keys = sorted(combined_dict.keys(), key=get_data_map_sort_key(combined_dict, data_map))
 
-        sorted_data_map_keys = sorted(data_map, key=data_map.get)
-        for key in sorted_data_map_keys:
+        for key in sorted_sfp_info_keys:
             if key == 'cable_type':
                 output += '{}{}: {}\n'.format(indent, sfp_info_dict['cable_type'], sfp_info_dict['cable_length'])
             elif key == 'cable_length':
@@ -352,7 +357,10 @@ class SFPShow(object):
                 if key in sfp_firmware_info_dict:
                     output += '{}{}: {}\n'.format(indent, data_map[key], sfp_firmware_info_dict[key])
             else:
-                output += '{}{}: {}\n'.format(indent, data_map[key], sfp_info_dict[key])
+                # For both known and unknown keys
+                display_name = data_map.get(key, key)  # Use data_map name if available, otherwise use key
+                value = sfp_info_dict.get(key, sfp_firmware_info_dict.get(key, 'N/A') if sfp_firmware_info_dict else 'N/A')
+                output += '{}{}: {}\n'.format(indent, display_name, value)
 
         return output
 

sfputil/main.py

@@ -28,7 +28,7 @@
 )
 from utilities_common.sfp_helper import covert_application_advertisement_to_output_string
 from utilities_common.sfp_helper import QSFP_DATA_MAP
-from utilities_common.sfp_helper import is_transceiver_cmis
+from utilities_common.sfp_helper import is_transceiver_cmis, get_data_map_sort_key
 from tabulate import tabulate
 from utilities_common.general import load_db_config
 
@@ -338,8 +338,10 @@ def convert_sfp_info_to_output_string(sfp_info_dict):
     output = ''
     is_sfp_cmis = is_transceiver_cmis(sfp_info_dict)
     if is_sfp_cmis:
-        sorted_qsfp_data_map_keys = sorted(QSFP_DD_DATA_MAP, key=QSFP_DD_DATA_MAP.get)
-        for key in sorted_qsfp_data_map_keys:
+        # Use the utility function with the local QSFP_DD_DATA_MAP for CMIS transceivers
+        get_sort_key = get_data_map_sort_key(sfp_info_dict, QSFP_DD_DATA_MAP)
+        sorted_qsfp_dd_info_keys = sorted(sfp_info_dict.keys(), key=get_sort_key)
+        for key in sorted_qsfp_dd_info_keys:
             if key == 'cable_type':
                 output += '{}{}: {}\n'.format(indent, sfp_info_dict['cable_type'], sfp_info_dict['cable_length'])
             elif key == 'cable_length':
@@ -355,14 +357,15 @@ def convert_sfp_info_to_output_string(sfp_info_dict):
             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])
-                except (KeyError, ValueError) as e:
-                    output += '{}{}: N/A\n'.format(indent, QSFP_DD_DATA_MAP[key])
+                # For both known and unknown keys, use the data map display name if available
+                display_name = QSFP_DD_DATA_MAP.get(key, key)  # Use data_map name if available, otherwise use key
+                output += '{}{}: {}\n'.format(indent, display_name, sfp_info_dict.get(key, 'N/A'))
 
     else:
-        sorted_qsfp_data_map_keys = sorted(QSFP_DATA_MAP, key=QSFP_DATA_MAP.get)
-        for key in sorted_qsfp_data_map_keys:
+        # Use the utility function with QSFP_DATA_MAP for non-CMIS transceivers
+        get_sort_key = get_data_map_sort_key(sfp_info_dict, QSFP_DATA_MAP)
+        sorted_qsfp_info_keys = sorted(sfp_info_dict.keys(), key=get_sort_key)
+        for key in sorted_qsfp_info_keys:
             if key == 'cable_type':
                 output += '{}{}: {}\n'.format(indent, sfp_info_dict['cable_type'], sfp_info_dict['cable_length'])
             elif key == 'cable_length':
@@ -379,7 +382,9 @@ def convert_sfp_info_to_output_string(sfp_info_dict):
                 except ValueError as e:
                     output += '{}N/A\n'.format((indent * 2))
             else:
-                output += '{}{}: {}\n'.format(indent, QSFP_DATA_MAP[key], sfp_info_dict[key])
+                # For both known and unknown keys, use the data map display name if available
+                display_name = QSFP_DATA_MAP.get(key, key)  # Use data_map name if available, otherwise use key
+                output += '{}{}: {}\n'.format(indent, display_name, sfp_info_dict.get(key, 'N/A'))
 
     return output
 

tests/mock_tables/state_db.json

@@ -718,11 +718,9 @@
         "active_apsel_hostlane4": "N/A",
         "is_replaceable": "True",
         "application_advertisement": "{1: {'host_electrical_interface_id': 'IB NDR', 'module_media_interface_id': 'Copper cable', 'media_lane_count': 4, 'host_lane_count': 4, 'host_lane_assignment_options': 17}, 2: {'host_electrical_interface_id': 'IB SDR (Arch.Spec.Vol.2)', 'module_media_interface_id': 'Copper cable', 'media_lane_count': 4, 'host_lane_count': 4, 'host_lane_assignment_options': 17}}",
-        "host_electrical_interface": "N/A",
         "active_apsel_hostlane2": "N/A",
         "supported_min_tx_power": "N/A",
         "supported_max_tx_power": "N/A",
-        "host_lane_assignment_option": "17",
         "specification_compliance": "passive_copper_media_interface",
         "ext_identifier": "Power Class 1 (0.25W Max)",
         "active_apsel_hostlane8": "N/A",
@@ -736,7 +734,6 @@
         "type": "OSFP 8X Pluggable Transceiver",
         "cable_length": "1.0",
         "active_apsel_hostlane5": "N/A",
-        "media_lane_assignment_option": "N/A",
         "vendor_rev": "A3",
         "active_apsel_hostlane6": "N/A",
         "encoding": "N/A",
@@ -749,7 +746,6 @@
         "vendor_oui": "some-oui",
         "connector": "No separable connector",
         "supported_max_laser_freq": "N/A",
-        "media_interface_code": "Copper cable",
         "active_apsel_hostlane3": "N/A",
         "ext_rateselect_compliance": "N/A",
         "model": "some-model    ",

tests/sfp_test.py

@@ -198,6 +198,8 @@
         Vendor PN: some-model    
         Vendor Rev: A3
         Vendor SN: serial1   
+        dom_capability: N/A
+        is_replaceable: True
         ChannelMonitorValues:
                 RX1Power: 0.5dBm
                 RX2Power: 0.3dBm

tests/sfputil_test.py

@@ -38,8 +38,7 @@
     'cable_length': '3',
     'application_advertisement': 'N/A',
     'specification_compliance': "{'10/40G Ethernet Compliance Code': '40GBASE-CR4'}",
-    'dom_capability': "{'Tx_power_support': 'no', 'Rx_power_support': 'no',\
-                        'Voltage_support': 'no', 'Temp_support': 'no'}",
+    'dom_capability': "N/A",
     'nominal_bit_rate': '255'
 }
 FLAT_MEMORY_MODULE_EEPROM = """Ethernet16: SFP EEPROM detected
@@ -59,6 +58,8 @@
         Vendor PN: MCP1600-C003
         Vendor Rev: A2
         Vendor SN: MT1636VS10561
+        dom_capability: N/A
+        type_abbrv_name: QSFP28
 """
 EMPTY_DOM_VALUES = """        ChannelMonitorValues:
         ChannelThresholdValues:
@@ -150,7 +151,7 @@ def test_format_dict_value_to_string(self):
                 'cable_length': '3',
                 'application_advertisement': 'N/A',
                 'specification_compliance': "{'10/40G Ethernet Compliance Code': '40GBASE-CR4'}",
-                'dom_capability': "{'Tx_power_support': 'no', 'Rx_power_support': 'no', 'Voltage_support': 'no', 'Temp_support': 'no'}",
+                'dom_capability': "N/A",
                 'nominal_bit_rate': '255'
             },
             # expected_output
@@ -170,6 +171,8 @@ def test_format_dict_value_to_string(self):
             "        Vendor PN: MCP1600-C003\n"
             "        Vendor Rev: A2\n"
             "        Vendor SN: MT1636VS10561\n"
+            "        dom_capability: N/A\n"
+            "        type_abbrv_name: QSFP28\n"
         ),
         # CMIS compliant module
         (
@@ -197,15 +200,11 @@ def test_format_dict_value_to_string(self):
                                                   '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'}",
+                'dom_capability': "N/A",
                 'nominal_bit_rate': '0',
                 'hardware_rev': '0.0',
-                'media_interface_code': '400ZR, DWDM, amplified',
-                'host_electrical_interface': '400GAUI-8 C2M (Annex 120E)',
                 'host_lane_count': 8,
                 'media_lane_count': 1,
-                'host_lane_assignment_option': 1,
-                'media_lane_assignment_option': 1,
                 'active_apsel_hostlane1': 1,
                 'active_apsel_hostlane2': 1,
                 'active_apsel_hostlane3': 1,
@@ -238,14 +237,10 @@ def test_format_dict_value_to_string(self):
             "        Extended Identifier: Power Class 8 (18.0W Max)\n"
             "        Extended RateSelect Compliance: N/A\n"
             "        Hardware Revision: 0.0\n"
-            "        Host Electrical Interface: 400GAUI-8 C2M (Annex 120E)\n"
-            "        Host Lane Assignment Options: 1\n"
             "        Host Lane Count: 8\n"
             "        Identifier: QSFP-DD Double Density 8X Pluggable Transceiver\n"
             "        Length Cable Assembly(m): 0\n"
-            "        Media Interface Code: 400ZR, DWDM, amplified\n"
             "        Media Interface Technology: C-band tunable laser\n"
-            "        Media Lane Assignment Options: 1\n"
             "        Media Lane Count: 1\n"
             "        Nominal Bit Rate(100Mbs): 0\n"
             "        Specification compliance: sm_media_interface\n"
@@ -259,6 +254,8 @@ def test_format_dict_value_to_string(self):
             "        Vendor PN: def\n"
             "        Vendor Rev: ghi\n"
             "        Vendor SN: jkl\n"
+            "        dom_capability: N/A\n"
+            "        type_abbrv_name: QSFP-DD\n"
         ),
     ])
     def test_convert_sfp_info_to_output_string(self, sfp_info_dict, expected_output):

utilities_common/sfp_helper.py

@@ -437,3 +437,78 @@ def is_transceiver_cmis(sfp_info_dict):
     if sfp_info_dict is None:
         return False
     return 'cmis_rev' in sfp_info_dict
+
+
+def is_transceiver_c_cmis(sfp_info_dict):
+    """
+    Check if the transceiver is C-CMIS compliant.
+    If the sfp_info_dict is None, return False.
+    If 'supported_max_tx_power' is present in the dictionary, return True.
+    Otherwise, return False.
+    """
+    if sfp_info_dict is None:
+        return False
+    return 'supported_max_tx_power' in sfp_info_dict
+
+
+def get_data_map_sort_key(sfp_info_dict, data_map=None):
+    """
+    Create a sorting key function for SFP info keys based on the transceiver type.
+
+    This function returns a key function that can be used with sorted() to order
+    SFP info dictionary keys. Known keys (those present in the appropriate data map)
+    are given priority 0 and sorted by their display name, while unknown keys are
+    given priority 1 and sorted alphabetically by their original key name.
+
+    Args:
+        sfp_info_dict (dict): The SFP info dictionary to determine transceiver type
+        data_map (dict, optional): Custom data map to use. If not provided, will determine
+                                   automatically based on transceiver type.
+
+    Returns:
+        function: A key function that can be used with sorted()
+
+    Example:
+        sorted_keys = sorted(sfp_info_dict.keys(), key=get_data_map_sort_key(sfp_info_dict))
+        # Or with custom data map:
+        sorted_keys = sorted(sfp_info_dict.keys(), key=get_data_map_sort_key(sfp_info_dict, CUSTOM_DATA_MAP))
+    """
+    if data_map is None:
+        data_map = get_transceiver_data_map(sfp_info_dict)
+
+    def get_sort_key(key):
+        """
+        Sort key function that prioritizes known fields over unknown ones.
+        Known fields are sorted by their display names, unknown fields by their key names.
+        """
+        if key in data_map:
+            return (0, data_map[key])  # Priority 0 for known keys, use data_map value
+        else:
+            return (1, key)  # Priority 1 for unknown keys, use key name
+
+    return get_sort_key
+
+
+def get_transceiver_data_map(sfp_info_dict):
+    """
+    Get the appropriate data map based on the transceiver type.
+
+    Args:
+        sfp_info_dict (dict): The SFP info dictionary to determine transceiver type
+
+    Returns:
+        dict: The appropriate data map (C_CMIS_DATA_MAP, CMIS_DATA_MAP, or QSFP_DATA_MAP)
+              Returns QSFP_DATA_MAP as default if sfp_info_dict is None or invalid
+    """
+    if sfp_info_dict is None or not isinstance(sfp_info_dict, dict):
+        return QSFP_DATA_MAP  # Default fallback
+
+    is_sfp_cmis = is_transceiver_cmis(sfp_info_dict)
+    is_sfp_c_cmis = is_transceiver_c_cmis(sfp_info_dict)
+
+    if is_sfp_c_cmis:
+        return C_CMIS_DATA_MAP
+    elif is_sfp_cmis:
+        return CMIS_DATA_MAP
+    else:
+        return QSFP_DATA_MAP