From 53639ded1ec5a5d534f8f571dd7d7aa664ba086c Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Wed, 23 Jun 2021 01:28:08 +0800 Subject: [PATCH] [xcvrd] Add bitmap support for SFP error event (#184) Support SFP error bitmap. Currently, SONiC use a single value to represent SFP error, however, multiple SFP errors could exist at the same time. This PR is aimed to support it Signed-off-by: Stephen Sun --- sonic-xcvrd/tests/test_xcvrd.py | 41 +++++-- sonic-xcvrd/xcvrd/xcvrd.py | 109 ++++++++---------- .../xcvrd_utilities/sfp_status_helper.py | 37 ++++++ .../xcvrd/xcvrd_utilities/y_cable_helper.py | 34 ++---- 4 files changed, 128 insertions(+), 93 deletions(-) create mode 100644 sonic-xcvrd/xcvrd/xcvrd_utilities/sfp_status_helper.py diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 11f759f01802..507890162d17 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -1,10 +1,7 @@ import os import sys -import subprocess -import pytest import unittest -from imp import load_source if sys.version_info >= (3, 3): from unittest.mock import MagicMock, patch else: @@ -12,6 +9,7 @@ from sonic_py_common import daemon_base from swsscommon import swsscommon +from sonic_platform_base.sfp_base import SfpBase from .mock_swsscommon import Table @@ -24,13 +22,12 @@ test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) scripts_path = os.path.join(modules_path, "xcvrd") -helper_file_path = os.path.join(scripts_path, "xcvrd_utilities"+"/y_cable_helper.py") sys.path.insert(0, modules_path) os.environ["XCVRD_UNIT_TESTING"] = "1" -load_source('y_cable_helper', scripts_path + '/xcvrd_utilities/y_cable_helper.py') -from y_cable_helper import * from xcvrd.xcvrd import * +from xcvrd.xcvrd_utilities.y_cable_helper import * +from xcvrd.xcvrd_utilities.sfp_status_helper import * class TestXcvrdScript(object): @@ -219,9 +216,9 @@ def test_init_port_sfp_status_tbl(self): init_port_sfp_status_tbl(stop_event) @patch('xcvrd.xcvrd_utilities.y_cable_helper.y_cable_platform_sfputil', MagicMock(return_value=[0])) - @patch('y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) - @patch('y_cable_helper._wrapper_get_presence', MagicMock(return_value=True)) - @patch('y_cable_helper.get_muxcable_info', MagicMock(return_value={'tor_active': 'self', + @patch('xcvrd.xcvrd_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) + @patch('xcvrd.xcvrd_utilities.y_cable_helper._wrapper_get_presence', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd_utilities.y_cable_helper.get_muxcable_info', MagicMock(return_value={'tor_active': 'self', 'mux_direction': 'self', 'manual_switch_count': '7', 'auto_switch_count': '71', @@ -258,9 +255,9 @@ def test_post_port_mux_info_to_db(self): assert(rc != -1) @patch('xcvrd.xcvrd_utilities.y_cable_helper.y_cable_platform_sfputil', MagicMock(return_value=[0])) - @patch('y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) - @patch('y_cable_helper._wrapper_get_presence', MagicMock(return_value=True)) - @patch('y_cable_helper.get_muxcable_static_info', MagicMock(return_value={'read_side': 'self', + @patch('xcvrd.xcvrd_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) + @patch('xcvrd.xcvrd_utilities.y_cable_helper._wrapper_get_presence', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd_utilities.y_cable_helper.get_muxcable_static_info', MagicMock(return_value={'read_side': 'self', 'nic_lane1_precursor1': '1', 'nic_lane1_precursor2': '-7', 'nic_lane1_maincursor': '-1', @@ -318,3 +315,23 @@ def test_get_media_settings_key(self): result = get_media_settings_key(0, xcvr_info_dict) assert result == ['MOLEX-1064141421', 'QSFP+-*'] # TODO: Ensure that error message was logged + + def test_detect_port_in_error_status(self): + class MockTable: + def get(self, key): + pass + + status_tbl = MockTable() + status_tbl.get = MagicMock(return_value=(True, {'error': 'N/A'})) + assert not detect_port_in_error_status(None, status_tbl) + + status_tbl.get = MagicMock(return_value=(True, {'error': SfpBase.SFP_ERROR_DESCRIPTION_BLOCKING})) + assert detect_port_in_error_status(None, status_tbl) + + def test_is_error_sfp_status(self): + error_values = [7, 11, 19, 35] + for error_value in error_values: + assert is_error_block_eeprom_reading(error_value) + + assert not is_error_block_eeprom_reading(int(SFP_STATUS_INSERTED)) + assert not is_error_block_eeprom_reading(int(SFP_STATUS_REMOVED)) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index e17237f45f8b..563145a8e956 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -15,11 +15,11 @@ import threading import time - from enum import Enum from sonic_py_common import daemon_base, device_info, logger from sonic_py_common import multi_asic from swsscommon import swsscommon + from .xcvrd_utilities import sfp_status_helper from .xcvrd_utilities import y_cable_helper except ImportError as e: raise ImportError(str(e) + " - required module not found") @@ -43,18 +43,6 @@ TIME_FOR_SFP_READY_SECS = 1 XCVRD_MAIN_THREAD_SLEEP_SECS = 60 -# SFP status definition, shall be aligned with the definition in get_change_event() of ChassisBase -SFP_STATUS_REMOVED = '0' -SFP_STATUS_INSERTED = '1' - -# SFP error code enum, new elements can be added to the enum if new errors need to be supported. -SFP_STATUS_ERR_ENUM = Enum('SFP_STATUS_ERR_ENUM', ['SFP_STATUS_ERR_I2C_STUCK', 'SFP_STATUS_ERR_BAD_EEPROM', - 'SFP_STATUS_ERR_UNSUPPORTED_CABLE', 'SFP_STATUS_ERR_HIGH_TEMP', - 'SFP_STATUS_ERR_BAD_CABLE'], start=2) - -# Convert the error code to string and store them in a set for convenience -errors_block_eeprom_reading = set(str(error_code.value) for error_code in SFP_STATUS_ERR_ENUM) - EVENT_ON_ALL_SFP = '-1' # events definition SYSTEM_NOT_READY = 'system_not_ready' @@ -188,11 +176,13 @@ def _wrapper_get_transceiver_change_event(timeout): if platform_chassis is not None: try: status, events = platform_chassis.get_change_event(timeout) - sfp_events = events['sfp'] - return status, sfp_events + sfp_events = events.get('sfp') + sfp_errors = events.get('sfp_error') + return status, sfp_events, sfp_errors except NotImplementedError: pass - return platform_sfputil.get_transceiver_change_event(timeout) + status, events = platform_sfputil.get_transceiver_change_event(timeout) + return status, events, None def _wrapper_get_sfp_type(physical_port): @@ -203,6 +193,14 @@ def _wrapper_get_sfp_type(physical_port): pass return None + +def _wrapper_get_sfp_error_description(physical_port): + if platform_chassis: + try: + return platform_chassis.get_sfp(physical_port).get_error_description() + except NotImplementedError: + pass + return None # Remove unnecessary unit from the raw data @@ -553,7 +551,7 @@ def recover_missing_sfp_table_entries(sfp_util, int_tbl, status_tbl, stop_event) continue keys = int_tbl[asic_index].getKeys() - if logical_port_name not in keys and not detect_port_in_error_status(logical_port_name, status_tbl[asic_index]): + if logical_port_name not in keys and not sfp_status_helper.detect_port_in_error_status(logical_port_name, status_tbl[asic_index]): post_port_sfp_info_to_db(logical_port_name, int_tbl[asic_index], transceiver_dict, stop_event) @@ -791,30 +789,17 @@ def waiting_time_compensation_with_sleep(time_start, time_to_wait): # Update port SFP status table on receiving SFP change event -def update_port_transceiver_status_table(logical_port_name, status_tbl, status): - fvs = swsscommon.FieldValuePairs([('status', status)]) +def update_port_transceiver_status_table(logical_port_name, status_tbl, status, error_descriptions='N/A'): + fvs = swsscommon.FieldValuePairs([('status', status), ('error', error_descriptions)]) status_tbl.set(logical_port_name, fvs) + # Delete port from SFP status table def delete_port_from_status_table(logical_port_name, status_tbl): status_tbl._del(logical_port_name) -# Check whether port in error status - - -def detect_port_in_error_status(logical_port_name, status_tbl): - rec, fvp = status_tbl.get(logical_port_name) - if rec: - status_dict = dict(fvp) - if status_dict['status'] in errors_block_eeprom_reading: - return True - else: - return False - else: - return False - # Init TRANSCEIVER_STATUS table @@ -844,16 +829,16 @@ def init_port_sfp_status_tbl(stop_event=threading.Event()): physical_port_list = logical_port_name_to_physical_port_list(logical_port_name) if physical_port_list is None: helper_logger.log_error("No physical ports found for logical port '{}'".format(logical_port_name)) - update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], SFP_STATUS_REMOVED) + update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], sfp_status_helper.SFP_STATUS_REMOVED) for physical_port in physical_port_list: if stop_event.is_set(): break if not _wrapper_get_presence(physical_port): - update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], SFP_STATUS_REMOVED) + update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], sfp_status_helper.SFP_STATUS_REMOVED) else: - update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], SFP_STATUS_INSERTED) + update_port_transceiver_status_table(logical_port_name, status_tbl[asic_index], sfp_status_helper.SFP_STATUS_INSERTED) # # Helper classes =============================================================== @@ -892,7 +877,7 @@ def task_worker(self, y_cable_presence): logger.log_warning("Got invalid asic index for {}, ignored".format(logical_port_name)) continue - if not detect_port_in_error_status(logical_port_name, status_tbl[asic_index]): + if not sfp_status_helper.detect_port_in_error_status(logical_port_name, status_tbl[asic_index]): post_port_dom_info_to_db(logical_port_name, dom_tbl[asic_index], self.task_stopping_event) post_port_dom_threshold_info_to_db(logical_port_name, dom_tbl[asic_index], self.task_stopping_event) if y_cable_presence[0] is True: @@ -1035,7 +1020,7 @@ def task_worker(self, stopping_event, sfp_error_event, y_cable_presence): while not stopping_event.is_set(): next_state = state time_start = time.time() - status, port_dict = _wrapper_get_transceiver_change_event(timeout) + status, port_dict, error_dict = _wrapper_get_transceiver_change_event(timeout) if not port_dict: continue helper_logger.log_debug("Got event {} {} in state {}".format(status, port_dict, state)) @@ -1095,11 +1080,11 @@ def task_worker(self, stopping_event, sfp_error_event, y_cable_presence): logger.log_warning("Got invalid asic index for {}, ignored".format(logical_port)) continue - if value == SFP_STATUS_INSERTED: + if value == sfp_status_helper.SFP_STATUS_INSERTED: helper_logger.log_info("Got SFP inserted event") # A plugin event will clear the error state. update_port_transceiver_status_table( - logical_port, status_tbl[asic_index], SFP_STATUS_INSERTED) + logical_port, status_tbl[asic_index], sfp_status_helper.SFP_STATUS_INSERTED) helper_logger.log_info("receive plug in and update port sfp status table.") rc = post_port_sfp_info_to_db(logical_port, int_tbl[asic_index], transceiver_dict) # If we didn't get the sfp info, assuming the eeprom is not ready, give a try again. @@ -1111,28 +1096,36 @@ def task_worker(self, stopping_event, sfp_error_event, y_cable_presence): post_port_dom_threshold_info_to_db(logical_port, dom_tbl[asic_index]) notify_media_setting(logical_port, transceiver_dict, app_port_tbl[asic_index]) transceiver_dict.clear() - elif value == SFP_STATUS_REMOVED: + elif value == sfp_status_helper.SFP_STATUS_REMOVED: helper_logger.log_info("Got SFP removed event") update_port_transceiver_status_table( - logical_port, status_tbl[asic_index], SFP_STATUS_REMOVED) - helper_logger.log_info("receive plug out and pdate port sfp status table.") + logical_port, status_tbl[asic_index], sfp_status_helper.SFP_STATUS_REMOVED) + helper_logger.log_info("receive plug out and update port sfp status table.") del_port_sfp_dom_info_from_db(logical_port, int_tbl[asic_index], dom_tbl[asic_index]) - elif value in errors_block_eeprom_reading: - helper_logger.log_info("Got SFP Error event") - # Add port to error table to stop accessing eeprom of it - # If the port already in the error table, the stored error code will - # be updated to the new one. - update_port_transceiver_status_table(logical_port, status_tbl[asic_index], value) - helper_logger.log_info("receive error update port sfp status table.") - # In this case EEPROM is not accessible, so remove the DOM info - # since it will be outdated if long time no update. - # but will keep the interface info in the DB since it static. - del_port_sfp_dom_info_from_db(logical_port, None, dom_tbl[asic_index]) - else: - # SFP return unkown event, just ignore for now. - helper_logger.log_warning("Got unknown event {}, ignored".format(value)) - continue + try: + error_bits = int(value) + helper_logger.log_info("Got SFP error event {}".format(value)) + + error_descriptions = sfp_status_helper.fetch_generic_error_description(error_bits) + + if sfp_status_helper.has_vendor_specific_error(error_bits): + if error_dict: + vendor_specific_error_description = error_dict.get(key) + else: + vendor_specific_error_description = _wrapper_get_sfp_error_description(key) + error_descriptions.append(vendor_specific_error_description) + + # Add error info to database + # Any existing error will be replaced by the new one. + update_port_transceiver_status_table(logical_port, status_tbl[asic_index], value, '|'.join(error_descriptions)) + helper_logger.log_info("Receive error update port sfp status table.") + # In this case EEPROM is not accessible. The DOM info will be removed since it can be out-of-date. + # The interface info remains in the DB since it is static. + if sfp_status_helper.is_error_block_eeprom_reading(error_bits): + del_port_sfp_dom_info_from_db(logical_port, None, dom_tbl[asic_index]) + except (TypeError, ValueError) as e: + logger.log_error("Got unrecognized event {}, ignored".format(value)) # Since ports could be connected to a mux cable, if there is a change event process the change for being on a Y cable Port y_cable_helper.change_ports_status_for_y_cable_change_event( diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/sfp_status_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/sfp_status_helper.py new file mode 100644 index 000000000000..789b761e42c9 --- /dev/null +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/sfp_status_helper.py @@ -0,0 +1,37 @@ +from sonic_platform_base.sfp_base import SfpBase + +# SFP status definition, shall be aligned with the definition in get_change_event() of ChassisBase +SFP_STATUS_REMOVED = '0' +SFP_STATUS_INSERTED = '1' + +# SFP error code dictinary, new elements can be added if new errors need to be supported. +SFP_ERRORS_BLOCKING_MASK = 0x02 +SFP_ERRORS_GENERIC_MASK = 0x0000FFFE +SFP_ERRORS_VENDOR_SPECIFIC_MASK = 0xFFFF0000 + +def is_error_block_eeprom_reading(error_bits): + return 0 != (error_bits & SFP_ERRORS_BLOCKING_MASK) + + +def has_vendor_specific_error(error_bits): + return 0 != (error_bits & SFP_ERRORS_VENDOR_SPECIFIC_MASK) + + +def fetch_generic_error_description(error_bits): + generic_error_bits = (error_bits & SFP_ERRORS_GENERIC_MASK) + error_descriptions = [] + if generic_error_bits: + for error_bit, error_description in SfpBase.SFP_ERROR_BIT_TO_DESCRIPTION_DICT.items(): + if error_bit & generic_error_bits: + error_descriptions.append(error_description) + return error_descriptions + + +def detect_port_in_error_status(logical_port_name, status_tbl): + rec, fvp = status_tbl.get(logical_port_name) + if rec: + status_dict = dict(fvp) + error = status_dict.get('error') + return SfpBase.SFP_ERROR_DESCRIPTION_BLOCKING in error + return False + diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py index ac2fc4e0dbe3..0857d8323a49 100644 --- a/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/y_cable_helper.py @@ -10,6 +10,7 @@ from sonic_py_common import multi_asic from sonic_y_cable import y_cable from swsscommon import swsscommon +from . import sfp_status_helper SELECT_TIMEOUT = 1000 @@ -21,27 +22,6 @@ helper_logger = logger.Logger(SYSLOG_IDENTIFIER) - -# SFP status definition, shall be aligned with the definition in get_change_event() of ChassisBase -SFP_STATUS_REMOVED = '0' -SFP_STATUS_INSERTED = '1' - -# SFP error codes, stored as strings. Can add more as needed. -SFP_STATUS_ERR_I2C_STUCK = '2' -SFP_STATUS_ERR_BAD_EEPROM = '3' -SFP_STATUS_ERR_UNSUPPORTED_CABLE = '4' -SFP_STATUS_ERR_HIGH_TEMP = '5' -SFP_STATUS_ERR_BAD_CABLE = '6' - -# Store the error codes in a set for convenience -errors_block_eeprom_reading = { - SFP_STATUS_ERR_I2C_STUCK, - SFP_STATUS_ERR_BAD_EEPROM, - SFP_STATUS_ERR_UNSUPPORTED_CABLE, - SFP_STATUS_ERR_HIGH_TEMP, - SFP_STATUS_ERR_BAD_CABLE -} - Y_CABLE_STATUS_NO_TOR_ACTIVE = 0 Y_CABLE_STATUS_TORA_ACTIVE = 1 Y_CABLE_STATUS_TORB_ACTIVE = 2 @@ -438,15 +418,23 @@ def change_ports_status_for_y_cable_change_event(port_dict, y_cable_presence, st continue if logical_port_name in port_table_keys[asic_index]: - if value == SFP_STATUS_INSERTED: + if value == sfp_status_helper.SFP_STATUS_INSERTED: helper_logger.log_info("Got SFP inserted event") check_identifier_presence_and_update_mux_table_entry( state_db, port_tbl, y_cable_tbl, static_tbl, mux_tbl, asic_index, logical_port_name, y_cable_presence) - elif value == SFP_STATUS_REMOVED or value in errors_block_eeprom_reading: + elif value == sfp_status_helper.SFP_STATUS_REMOVED: check_identifier_presence_and_delete_mux_table_entry( state_db, port_tbl, asic_index, logical_port_name, y_cable_presence, delete_change_event) else: + try: + # Now that the value is in bitmap format, let's convert it to number + event_bits = int(value) + if sfp_status_helper.is_error_block_eeprom_reading(event_bits): + check_identifier_presence_and_delete_mux_table_entry( + state_db, port_tbl, asic_index, logical_port_name, y_cable_presence, delete_change_event) + except: + pass # SFP return unkown event, just ignore for now. helper_logger.log_warning("Got unknown event {}, ignored".format(value)) continue