From 81048cd1fb63da9fdca567aa863c7d1d43156ed3 Mon Sep 17 00:00:00 2001 From: Mai Bui Date: Mon, 19 Jun 2023 17:41:22 -0400 Subject: [PATCH 01/12] add semgrep (#379) **Why I did it** [Semgrep](https://github.com/returntocorp/semgrep) is a static analysis tool to find security vulnerabilities. When opening a PR or commtting to PR, Semgrep performs a diff-aware scanning, which scans changed files in PRs. When merging PR, Semgrep performs a full scan on master branch and report all findings. Ref: - [Supported Language](https://semgrep.dev/docs/supported-languages/#language-maturity) - [Semgrep Rules](https://registry.semgrep.dev/rule) **How I did it** Integrate Semgrep into this repository by committing a job configuration file --- .github/workflows/semgrep.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/workflows/semgrep.yml diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 000000000..975769a50 --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,22 @@ +name: Semgrep + +on: + pull_request: {} + push: + branches: + - master + - '201[7-9][0-1][0-9]' + - '202[0-9][0-1][0-9]' + +jobs: + semgrep: + if: github.repository_owner == 'sonic-net' + name: Semgrep + runs-on: ubuntu-latest + container: + image: returntocorp/semgrep + steps: + - uses: actions/checkout@v3 + - run: semgrep ci + env: + SEMGREP_RULES: p/default From 66f981d22701b3ecef2a8e6b48981a940bee2223 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Sat, 8 Jul 2023 02:39:12 +0800 Subject: [PATCH 02/12] [PSU power threshold] Fix logic error: compare the system power with the PSU's power threshold (#367) * Adjust the logic to check PSU power threshold Check the system power instead of PSU power against a single PSU's threshold Signed-off-by: Stephen Sun * Log only once instead of logging per PSU Signed-off-by: Stephen Sun --------- Signed-off-by: Stephen Sun --- sonic-psud/scripts/psud | 24 +++++++++++++++++++----- sonic-psud/tests/test_DaemonPsud.py | 21 ++++++++++++--------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/sonic-psud/scripts/psud b/sonic-psud/scripts/psud index b52b98948..ce588589c 100644 --- a/sonic-psud/scripts/psud +++ b/sonic-psud/scripts/psud @@ -380,6 +380,7 @@ class DaemonPsud(daemon_base.DaemonBase): self.psu_tbl = None self.psu_chassis_info = None self.first_run = True + self.psu_threshold_exceeded_logged = False global platform_psuutil global platform_chassis @@ -458,6 +459,7 @@ class DaemonPsud(daemon_base.DaemonBase): if not platform_chassis: return + self.psu_threshold_exceeded_logged = False for index, psu in enumerate(platform_chassis.get_all_psus()): try: self._update_single_psu_data(index + 1, psu) @@ -535,25 +537,37 @@ class DaemonPsud(daemon_base.DaemonBase): power_warning_suppress_threshold = try_get(psu.get_psu_power_warning_suppress_threshold, NOT_AVAILABLE) power_critical_threshold = try_get(psu.get_psu_power_critical_threshold, NOT_AVAILABLE) if psu_status.check_psu_power_threshold: + # Calculate total power + system_power = float(power) + for _, other_psu in enumerate(platform_chassis.get_all_psus()): + if other_psu is psu: + # Skip the current PSU + continue + power_str = try_get(other_psu.get_power, NOT_AVAILABLE) + if power_str != NOT_AVAILABLE: + system_power += float(power_str) + if power_warning_suppress_threshold == NOT_AVAILABLE or power_critical_threshold == NOT_AVAILABLE: self.log_error("PSU power thresholds become invalid: threshold {} critical threshold {}".format(power_warning_suppress_threshold, power_critical_threshold)) psu_status.check_psu_power_threshold = False psu_status.power_exceeded_threshold = False elif psu_status.power_exceeded_threshold: # The failing threshold is the warning threshold - if power < power_warning_suppress_threshold: + if system_power < power_warning_suppress_threshold: # Clear alarm power_exceeded_threshold = False else: # The rising threshold is the critical threshold - if power >= power_critical_threshold: + if system_power >= power_critical_threshold: # Raise alarm power_exceeded_threshold = True - if psu_status.set_power_exceed_threshold(power_exceeded_threshold): + if psu_status.set_power_exceed_threshold(power_exceeded_threshold) and not self.psu_threshold_exceeded_logged: + # Since this is a system level PSU power exceeding check, we do not need to log it for each PSU log_on_status_changed(self, not psu_status.power_exceeded_threshold, - 'PSU power warning cleared: {} power {} is back to normal.'.format(name, power), - 'PSU power warning: {} power {} exceeds critical threshold {}.'.format(name, power, power_critical_threshold)) + 'PSU power warning cleared: system power {} is back to normal, below the warning suppress threshold {}.'.format(system_power, power_warning_suppress_threshold), + 'PSU power warning: system power {} exceeds the critical threshold {}.'.format(system_power, power_critical_threshold)) + self.psu_threshold_exceeded_logged = True if presence and psu_status.set_voltage(voltage, voltage_high_threshold, voltage_low_threshold): set_led = True diff --git a/sonic-psud/tests/test_DaemonPsud.py b/sonic-psud/tests/test_DaemonPsud.py index 482eb1cdd..805c76cc0 100644 --- a/sonic-psud/tests/test_DaemonPsud.py +++ b/sonic-psud/tests/test_DaemonPsud.py @@ -188,19 +188,22 @@ def test_power_threshold(self): psu = MockPsu('PSU 1', 0, True, 'Fake Model', '12345678', '1234') psud.platform_chassis = MockChassis() psud.platform_chassis._psu_list.append(psu) + another_psu = MockPsu('PSU 2', 0, True, 'Fake Model', '12345678', '1234') + another_psu.set_power(10.0) + psud.platform_chassis._psu_list.append(another_psu) daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER) daemon_psud.psu_tbl = mock.MagicMock() - psu.get_psu_power_critical_threshold = mock.MagicMock(return_value=120.0) - psu.get_psu_power_warning_suppress_threshold = mock.MagicMock(return_value=110.0) + psu.get_psu_power_critical_threshold = mock.MagicMock(return_value=130.0) + psu.get_psu_power_warning_suppress_threshold = mock.MagicMock(return_value=120.0) # Normal start. All good and all thresholds are supported # Power is in normal range (below warning threshold) daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(100.0, 110.0, 120.0, False) + expected_fvp = self._construct_expected_fvp(100.0, 120.0, 130.0, False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -213,7 +216,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(115.0, 110.0, 120.0, False) + expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -224,7 +227,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(125.0, 110.0, 120.0, True) + expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -235,7 +238,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(115.0, 110.0, 120.0, True) + expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -246,7 +249,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(105.0, 110.0, 120.0, False) + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() daemon_psud._update_led_color() @@ -257,7 +260,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(125.0, 110.0, 120.0, True) + expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() @@ -268,7 +271,7 @@ def test_power_threshold(self): daemon_psud._update_single_psu_data(1, psu) assert daemon_psud.psu_status_dict[1].check_psu_power_threshold assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold - expected_fvp = self._construct_expected_fvp(105.0, 110.0, 120.0, False) + expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False) daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp) daemon_psud._update_led_color() assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led() From 63bd9d8b7cbbdfec039aa236079c3b5d0c01112c Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Fri, 7 Jul 2023 13:38:09 -0700 Subject: [PATCH 03/12] [ycabled][active-active] no initialize Async Client, when no active-active cable type; fix names for all ycabled threads (#373) This PR intends to not start sonic-ycabled's Asynchronous client, when there is no active-active cable type. The Problem that we encounter is ycabled expects all threads to be working/running state all the time. Since there are no active-active cable_type Async client thread exits gracefully, but by design this needs to be working if added to monitor loop thread. So this PR fixes this problem by runnig the Async Client only if configuration is active-active for atleast a cable. This PR also has all the infrastructure changes UT for all Classes having infinite loops in ycabled, thus helping in picking up cases if changes break the code. Signed-off-by: vaibhav-dahiya --- sonic-ycabled/tests/test_y_cable_helper.py | 323 +++++++++++++++++- sonic-ycabled/tests/test_ycable.py | 60 +++- sonic-ycabled/ycable/ycable.py | 38 ++- .../ycable/ycable_utilities/y_cable_helper.py | 41 ++- 4 files changed, 428 insertions(+), 34 deletions(-) diff --git a/sonic-ycabled/tests/test_y_cable_helper.py b/sonic-ycabled/tests/test_y_cable_helper.py index 04ac6f7b5..84630eb48 100644 --- a/sonic-ycabled/tests/test_y_cable_helper.py +++ b/sonic-ycabled/tests/test_y_cable_helper.py @@ -1730,6 +1730,7 @@ def test_change_ports_status_for_y_cable_change_event(self, mock_swsscommon_tabl def mock_get_asic_id(mock_logical_port_name): return 0 + state_db = {} y_cable_presence = [True] logical_port_dict = {'Ethernet0': '1'} @@ -1745,7 +1746,7 @@ def mock_get_asic_id(mock_logical_port_name): patched_util.get_asic_id_for_logical_port.return_value = 0 rc = change_ports_status_for_y_cable_change_event( - logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stop_event=threading.Event()) + logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()) assert(rc == None) @@ -1764,6 +1765,7 @@ def mock_get_asic_id(mock_logical_port_name): y_cable_presence = [True] logical_port_dict = {'Ethernet0': '1'} + state_db = {} mock_table = MagicMock() mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) @@ -1777,7 +1779,7 @@ def mock_get_asic_id(mock_logical_port_name): patched_util.get_asic_id_for_logical_port.return_value = 0 rc = change_ports_status_for_y_cable_change_event( - logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl,stop_event=threading.Event()) + logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()) assert(rc == None) @@ -1794,6 +1796,7 @@ def mock_get_asic_id(mock_logical_port_name): y_cable_presence = [True] logical_port_dict = {'Ethernet0': '2'} + state_db = {} mock_table = MagicMock() mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) @@ -1806,7 +1809,7 @@ def mock_get_asic_id(mock_logical_port_name): patched_util.get_asic_id_for_logical_port.return_value = 0 rc = change_ports_status_for_y_cable_change_event( - logical_port_dict, y_cable_presence,port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stop_event=threading.Event()) + logical_port_dict, y_cable_presence,port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()) assert(rc == None) @@ -7141,3 +7144,317 @@ def test_ycable_graceful_client(self, channel, stub): read_side = 1 Y_cable_restart_client = GracefulRestartClient("Ethernet48", None, read_side) + +class TestYcableScriptExecution(object): + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + def test_ycable_helper_cli_worker(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + stop_event = threading.Event() + + asic_index = 0 + Y_cable_cli_task = YCableCliUpdateTask() + Y_cable_cli_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + + #Y_cable_cli_task.task_stopping_event.is_set = MagicMock(side_effect=False) + + expected_exception_start = None + expected_exception_join = None + #Y_cable_cli_task.start() + Y_cable_cli_task.task_cli_worker() + Y_cable_cli_task.task_stopping_event.clear() + + assert swsscommon.Select.select.call_count == 1 + #y_cable_helper.handle_show_hwmode_state_cmd_arg_tbl_notification.assert_called() + Y_cable_cli_task_n = YCableCliUpdateTask() + Y_cable_cli_task_n.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + + Y_cable_cli_task_n.task_cli_worker() + assert swsscommon.Select.select.call_count == 2 + + + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + def test_ycable_helper_cli_worker_execution(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[(False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False) ,('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), (None, None, None), (None, None, None)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + stop_event = threading.Event() + + asic_index = 0 + Y_cable_cli_task = YCableCliUpdateTask() + Y_cable_cli_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + Y_cable_cli_task.cli_table_helper.xcvrd_show_hwmode_dir_cmd_tbl[asic_index].return_value = mock_selectable + + #Y_cable_cli_task.task_stopping_event.is_set = MagicMock(side_effect=False) + + expected_exception_start = None + expected_exception_join = None + trace = None + """ + try: + #Y_cable_cli_task.start() + Y_cable_cli_task.task_cli_worker() + time.sleep(5) + Y_cable_cli_task.task_stopping_event.clear() + except Exception as e1: + expected_exception_start = e1 + trace = traceback.format_exc() + """ + + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + mock_table = MagicMock() + """mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + mock_table.get = MagicMock( + side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + mock_swsscommon_table.return_value = mock_table + """ + Y_cable_task.hw_mux_cable_tbl_keys = MagicMock(side_effect={0:["Ethernet0", "Ethernet4"]}) + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + def test_ycable_helper_table_worker_active_active(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), ('Ethernet0', swsscommon.SET_COMMAND, (('index', '1'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_stubs', MagicMock(return_value={})) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_channels', MagicMock(return_value={})) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.FieldValuePairs', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker_probe_active_active(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[(False, False, False), (False, False, False), (False, False, False), ('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + """ + mock_table = MagicMock() + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + mock_table.get = MagicMock( + side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + mock_swsscommon_table.return_value = mock_table + """ + + Y_cable_task.hw_mux_cable_tbl_keys = MagicMock(side_effect={0:["Ethernet0", "Ethernet4"]}) + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + + + + + + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_stubs', MagicMock(return_value={})) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_channels', MagicMock(return_value={})) + @patch('swsscommon.swsscommon.FieldValuePairs', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker_probe_active(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[(False, False, False), (False, False, False), ('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), ('command', 'probe'),)), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + """ + mock_table = MagicMock() + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + mock_table.get = MagicMock( + side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + mock_swsscommon_table.return_value = mock_table + """ + + Y_cable_task.hw_mux_cable_tbl_keys = MagicMock(side_effect={0:["Ethernet0", "Ethernet4"]}) + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + + + + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker_probe(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[(False, False, False), ('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + #mock_table = MagicMock() + #mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + swsscommon.Table.return_value.get.return_value = ( + True, {"read_side": "1", "state":"active"}) + #mock_table.get = MagicMock( + # side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + swsscommon.Table.return_value.getKeys.return_value = ( + ['Ethernet0', 'Ethernet4']) + #mock_swsscommon_table.return_value = mock_table + + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker_toggle(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), )), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + #mock_table = MagicMock() + #mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + swsscommon.Table.return_value.get.return_value = ( + True, {"read_side": "1", "state":"active"}) + #mock_table.get = MagicMock( + # side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + swsscommon.Table.return_value.getKeys.return_value = ( + ['Ethernet0', 'Ethernet4']) + #mock_swsscommon_table.return_value = mock_table + + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch('swsscommon.swsscommon.Select.addSelectable', MagicMock()) + @patch('swsscommon.swsscommon.Select.TIMEOUT', MagicMock(return_value=None)) + @patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj', MagicMock()) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_stubs', MagicMock(return_value={})) + @patch('ycable.ycable_utilities.y_cable_helper.grpc_port_channels', MagicMock(return_value={})) + @patch('swsscommon.swsscommon.FieldValuePairs', MagicMock()) + #@patch('swsscommon.swsscommon.CastSelectableToRedisSelectObj.getDbConnector', MagicMock()) + @patch('swsscommon.swsscommon.SubscriberStateTable') + @patch('swsscommon.swsscommon.Select.select') + #@patch('swsscommon.swsscommon.Table') + def test_ycable_helper_table_worker_toggle_active_active(self, mock_select, mock_sub_table): + + mock_selectable = MagicMock() + mock_selectable.pop = MagicMock( + side_effect=[('Ethernet0', swsscommon.SET_COMMAND, (('state', 'active'), ("command", "probe"),)), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False), (False, False, False)]) + mock_select.return_value = (swsscommon.Select.OBJECT, mock_selectable) + mock_sub_table.return_value = mock_selectable + + + Y_cable_task = YCableTableUpdateTask() + Y_cable_task.task_stopping_event.is_set = MagicMock(side_effect=[False, True]) + #mock_table = MagicMock() + #mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + swsscommon.Table.return_value.get.return_value = ( + True, {"read_side": "1", "state":"active"}) + #mock_table.get = MagicMock( + # side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + swsscommon.Table.return_value.getKeys.return_value = ( + ['Ethernet0', 'Ethernet4']) + #mock_swsscommon_table.return_value = mock_table + + Y_cable_task.task_worker() + assert swsscommon.Select.select.call_count == 1 + + diff --git a/sonic-ycabled/tests/test_ycable.py b/sonic-ycabled/tests/test_ycable.py index b45fea22e..1847ffd8d 100644 --- a/sonic-ycabled/tests/test_ycable.py +++ b/sonic-ycabled/tests/test_ycable.py @@ -83,6 +83,7 @@ def test_ycable_helper_class_run_loop(self): Y_cable_cli_task.start() Y_cable_cli_task.join() + @patch("swsscommon.swsscommon.Select", MagicMock()) @patch("swsscommon.swsscommon.Select.addSelectable", MagicMock()) def test_ycable_helper_class_run(self): @@ -307,13 +308,15 @@ def test_handle_state_update_task(self): port = "Ethernet0" fvp_dict = {} + state_db = {} y_cable_presence = False stopping_event = None port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl = {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {} - rc = handle_state_update_task(port, fvp_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stopping_event) + rc = handle_state_update_task(port, fvp_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stopping_event) assert(rc == None) + def wait_until(total_wait_time, interval, call_back, *args, **kwargs): wait_time = 0 while wait_time <= total_wait_time: @@ -353,20 +356,13 @@ def test_ycable_helper_class_run_loop_with_exception(self): except Exception as e2: expected_exception_join = e2 - """ - #Handy debug Helpers or else use import logging - #f = open("newfile", "w") - #f.write(format(e2)) - #f.write(format(m1)) - #f.write(trace) - """ - assert(type(expected_exception_start) == type(expected_exception_join)) assert(expected_exception_start.args == expected_exception_join.args) assert("NotImplementedError" in str(trace) and "effect" in str(trace)) assert("sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py" in str(trace)) assert("swsscommon.Select" in str(trace)) + class TestYcableAsyncScript(object): @patch("swsscommon.swsscommon.Select", MagicMock(side_effect=NotImplementedError)) @@ -400,3 +396,49 @@ def test_ycable_helper_async_client_run_loop_with_exception(self, sfputil): assert("sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py" in str(trace)) assert("setup_grpc_channel_for_port" in str(trace)) + +class TestYcableActiveActiveHelper(object): + + @patch("ycable.ycable.platform_sfputil") + def test_check_presence_for_active_active_cable_type(self, sfputil): + + + y_cable_tbl = {} + test_db = "TEST_DB" + status = True + asic_index = 0 + fvs = [('state', "auto"), ('read_side', 1), ('soc_ipv4', '192.168.0.1/32')] + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + + sfputil.logical = ["Ethernet0", "Ethernet4"] + sfputil.get_asic_id_for_logical_port = MagicMock(return_value=0) + rc = check_presence_for_active_active_cable_type(y_cable_tbl) + + assert(rc == False) + + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch("ycable.ycable.platform_sfputil") + def test_check_presence_for_active_active_cable_type(self, sfputil): + + + y_cable_tbl = {} + test_db = "TEST_DB" + status = True + asic_index = 0 + fvs = [('state', "auto"), ('read_side', 1), ('soc_ipv4', '192.168.0.1/32')] + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + + sfputil.logical = ["Ethernet0", "Ethernet4"] + sfputil.get_asic_id_for_logical_port = MagicMock(return_value=0) + rc = check_presence_for_active_active_cable_type(y_cable_tbl) + + assert(rc == True) + diff --git a/sonic-ycabled/ycable/ycable.py b/sonic-ycabled/ycable/ycable.py index 3618b56f2..bc85b04d9 100644 --- a/sonic-ycabled/ycable/ycable.py +++ b/sonic-ycabled/ycable/ycable.py @@ -70,6 +70,26 @@ # Helper functions ============================================================= # + +def check_presence_for_active_active_cable_type(port_tbl): + + + logical_port_list = platform_sfputil.logical + for logical_port_name in logical_port_list: + # Get the asic to which this port belongs + asic_index = platform_sfputil.get_asic_id_for_logical_port(logical_port_name) + if asic_index is None: + continue + + (status, cable_type) = y_cable_helper.check_mux_cable_port_type(logical_port_name, port_tbl, asic_index) + + if status and cable_type == "active-active": + return True + + return False + + + def detect_port_in_error_status(logical_port_name, status_tbl): rec, fvp = status_tbl.get(logical_port_name) if rec: @@ -81,13 +101,13 @@ def detect_port_in_error_status(logical_port_name, status_tbl): else: return False -def handle_state_update_task(port, fvp_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stopping_event): +def handle_state_update_task(port, fvp_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stopping_event): port_dict = {} port_dict[port] = fvp_dict.get('status', None) y_cable_helper.change_ports_status_for_y_cable_change_event( - port_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stopping_event) + port_dict, y_cable_presence, port_tbl, port_tbl_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stopping_event) # # Helper classes =============================================================== @@ -105,6 +125,7 @@ def __init__(self, y_cable_presence): self.task_stopping_event = threading.Event() self.y_cable_presence = y_cable_presence self.table_helper = y_cable_table_helper.YcableInfoUpdateTableHelper() + self.name = "YcableInfoUpdateTask" def task_worker(self, y_cable_presence): @@ -159,6 +180,7 @@ def __init__(self, sfp_error_event, y_cable_presence): self.sfp_error_event = sfp_error_event self.y_cable_presence = y_cable_presence self.table_helper = y_cable_table_helper.YcableStateUpdateTableHelper() + self.name = "YcableStateUpdateTask" def task_worker(self, stopping_event, sfp_error_event, y_cable_presence): @@ -208,7 +230,7 @@ def task_worker(self, stopping_event, sfp_error_event, y_cable_presence): continue # Check if all tables are created in table_helper - handle_state_update_task(port, fvp_dict, y_cable_presence, self.table_helper.get_port_tbl(), self.table_helper.port_table_keys, self.table_helper.get_loopback_tbl(), self.table_helper.loopback_keys, self.table_helper.get_hw_mux_cable_tbl(), self.table_helper.get_hw_mux_cable_tbl_peer(), self.table_helper.get_y_cable_tbl(), self.table_helper.get_static_tbl(), self.table_helper.get_mux_tbl(), self.table_helper.get_grpc_config_tbl(), self.table_helper.get_fwd_state_response_tbl(), stopping_event) + handle_state_update_task(port, fvp_dict, y_cable_presence, self.table_helper.get_port_tbl(), self.table_helper.port_table_keys, self.table_helper.get_loopback_tbl(), self.table_helper.loopback_keys, self.table_helper.get_hw_mux_cable_tbl(), self.table_helper.get_hw_mux_cable_tbl_peer(), self.table_helper.get_y_cable_tbl(), self.table_helper.get_static_tbl(), self.table_helper.get_mux_tbl(), self.table_helper.get_grpc_config_tbl(), self.table_helper.get_fwd_state_response_tbl(), self.table_helper.state_db, stopping_event) def run(self): if self.task_stopping_event.is_set(): @@ -242,6 +264,7 @@ def __init__(self, log_identifier): self.y_cable_presence = [False] self.table_helper = y_cable_table_helper.DaemonYcableTableHelper() self.threads = [] + self.name = "DaemonYcable" # Signal handler def signal_handler(self, sig, frame): @@ -389,9 +412,12 @@ def run(self): y_cable_cli_worker_update = y_cable_helper.YCableCliUpdateTask() y_cable_cli_worker_update.start() self.threads.append(y_cable_cli_worker_update) - y_cable_async_noti_worker = y_cable_helper.YCableAsyncNotificationTask() - y_cable_async_noti_worker.start() - self.threads.append(y_cable_async_noti_worker) + # enable async client only if there are active-active cables + active_active_cable_presence = check_presence_for_active_active_cable_type(self.table_helper.get_port_tbl()) + if active_active_cable_presence is True: + y_cable_async_noti_worker = y_cable_helper.YCableAsyncNotificationTask() + y_cable_async_noti_worker.start() + self.threads.append(y_cable_async_noti_worker) # Start main loop self.log_info("Start daemon main loop") diff --git a/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py b/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py index cfd6cadc3..f7a916bab 100644 --- a/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py +++ b/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py @@ -387,10 +387,10 @@ def apply_grpc_secrets_configuration(SECRETS_PATH, grpc_config): if grpc_client_config is not None: config = grpc_client_config.get("config", None) if config is not None: - type = config.get("type",None) + type_chan = config.get("type",None) auth_level = config.get("auth_level",None) log_level = config.get("log_level", None) - fvs_updated = swsscommon.FieldValuePairs([('type', type), + fvs_updated = swsscommon.FieldValuePairs([('type', type_chan), ('auth_level',auth_level ), ('log_level',log_level)]) grpc_config[asic_index].set('config', fvs_updated) @@ -407,7 +407,7 @@ def apply_grpc_secrets_configuration(SECRETS_PATH, grpc_config): grpc_config[asic_index].set('certs', fvs_updated) -def get_grpc_credentials(type, kvp): +def get_grpc_credentials(type_chan, kvp): root_file = kvp.get("ca_crt", None) if root_file is not None and os.path.isfile(root_file): @@ -416,7 +416,7 @@ def get_grpc_credentials(type, kvp): helper_logger.log_error("grpc credential channel setup no root file in config_db") return None - if type == "mutual": + if type_chan == "mutual": cert_file = kvp.get("client_crt", None) if cert_file is not None and os.path.isfile(cert_file): cert_chain = open(cert_file, 'rb').read() @@ -435,7 +435,7 @@ def get_grpc_credentials(type, kvp): root_certificates=root_cert, private_key=key, certificate_chain=cert_chain) - elif type == "server": + elif type_chan == "server": credential = grpc.ssl_channel_credentials( root_certificates=root_cert) else: @@ -458,7 +458,7 @@ def connect_channel(channel, stub, port): else: break -def create_channel(type, level, kvp, soc_ip, port, asic_index, fwd_state_response_tbl, is_async): +def create_channel(type_chan, level, kvp, soc_ip, port, asic_index, fwd_state_response_tbl, is_async): # Helper callback to get an channel connectivity state def wait_for_state_change(channel_connectivity): @@ -487,28 +487,33 @@ def wait_for_state_change(channel_connectivity): grpc_port_connectivity[port] = "SHUTDOWN" - if type == "secure": + if type_chan == "secure": credential = get_grpc_credentials(level, kvp) target_name = kvp.get("grpc_ssl_credential", None) if credential is None or target_name is None: return (None, None) - GRPC_CLIENT_OPTIONS.append(('grpc.ssl_target_name_override', '{}'.format(target_name))) if is_async: - channel = grpc.aio.secure_channel("{}:{}".format(soc_ip, GRPC_PORT), credential, options=GRPC_CLIENT_OPTIONS) + ASYNC_GRPC_CLIENT_OPTIONS = [] + ASYNC_GRPC_CLIENT_OPTIONS.append(('grpc.ssl_target_name_override', '{}'.format(target_name))) + channel = grpc.aio.secure_channel("{}:{}".format(soc_ip, GRPC_PORT), credential, options=ASYNC_GRPC_CLIENT_OPTIONS) + stub = linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub(channel) else: + GRPC_CLIENT_OPTIONS.append(('grpc.ssl_target_name_override', '{}'.format(target_name))) channel = grpc.secure_channel("{}:{}".format(soc_ip, GRPC_PORT), credential, options=GRPC_CLIENT_OPTIONS) + stub = linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub(channel) else: if is_async: - channel = grpc.aio.insecure_channel("{}:{}".format(soc_ip, GRPC_PORT), options=GRPC_CLIENT_OPTIONS) + channel = grpc.aio.insecure_channel("{}:{}".format(soc_ip, GRPC_PORT)) + stub = linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub(channel) else: channel = grpc.insecure_channel("{}:{}".format(soc_ip, GRPC_PORT), options=GRPC_CLIENT_OPTIONS) + stub = linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub(channel) - stub = linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub(channel) if not is_async and channel is not None: @@ -541,7 +546,7 @@ def setup_grpc_channel_for_port(port, soc_ip, asic_index, grpc_config, fwd_state #if no config from config DB, treat channel to be as insecure - type = "insecure" + type_chan = "insecure" level = "server" (status, fvs) = grpc_config[asic_index].get("config") @@ -550,12 +555,12 @@ def setup_grpc_channel_for_port(port, soc_ip, asic_index, grpc_config, fwd_state "Could not retreive fieldvalue pairs for {}, inside config_db table kvp config for {} for setting up channel type".format(port, grpc_config[asic_index].getTableName())) else: grpc_config_dict = dict(fvs) - type = grpc_config_dict.get("type", None) + type_chan = grpc_config_dict.get("type", None) level = grpc_config_dict.get("auth_level", None) kvp = {} - if type == "secure": + if type_chan == "secure": (status, fvs) = grpc_config[asic_index].get("certs") if status is False: helper_logger.log_warning( @@ -565,7 +570,7 @@ def setup_grpc_channel_for_port(port, soc_ip, asic_index, grpc_config, fwd_state kvp = dict(fvs) - channel, stub = create_channel(type, level, kvp, soc_ip, port, asic_index, fwd_state_response_tbl, is_async) + channel, stub = create_channel(type_chan, level, kvp, soc_ip, port, asic_index, fwd_state_response_tbl, is_async) if stub is None: helper_logger.log_warning("stub was not setup for gRPC soc ip {} port {}, no gRPC soc server running ?".format(soc_ip, port)) @@ -1384,7 +1389,7 @@ def init_ports_status_for_y_cable(platform_sfp, platform_chassis, y_cable_presen "Could not retreive port inside config_db PORT table {} for Y-Cable initiation".format(logical_port_name)) -def change_ports_status_for_y_cable_change_event(port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, stop_event=threading.Event()): +def change_ports_status_for_y_cable_change_event(port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()): global read_side delete_change_event = [False] @@ -3713,6 +3718,7 @@ def task_worker(self): handle_hw_mux_cable_table_grpc_notification( fvp, self.table_helper.get_hw_mux_cable_tbl(), asic_index, self.table_helper.get_mux_metrics_tbl(), False, port, self.table_helper.get_port_tbl(), self.table_helper.get_grpc_config_tbl(), self.table_helper.get_fwd_state_response_tbl()) + while True: (port_m, op_m, fvp_m) = self.table_helper.get_mux_cable_command_tbl()[asic_index].pop() @@ -3788,6 +3794,7 @@ def __init__(self): self.task_download_firmware_thread = {} self.task_stopping_event = threading.Event() self.cli_table_helper = y_cable_table_helper.YcableCliUpdateTableHelper() + self.name = "YCableCliUpdateTask" def task_cli_worker(self): @@ -3842,6 +3849,7 @@ def task_cli_worker(self): namespace = redisSelectObj.getDbConnector().getNamespace() asic_index = multi_asic.get_asic_index_from_namespace(namespace) + while True: (key, op_m, fvp_m) = self.cli_table_helper.xcvrd_log_tbl[asic_index].pop() @@ -4070,6 +4078,7 @@ def __init__(self): self.task_stopping_event = threading.Event() self.table_helper = y_cable_table_helper.YcableAsyncNotificationTableHelper() self.read_side = process_loopback_interface_and_get_read_side(self.table_helper.loopback_keys) + self.name = "YCableAsyncNotificationTask" async def task_worker(self): From 432602a288a81f9db30b2148adc9306f0aefb1b4 Mon Sep 17 00:00:00 2001 From: Michael Wang - TW <133941180+MichaelWangSmci@users.noreply.github.com> Date: Wed, 12 Jul 2023 21:36:43 +0800 Subject: [PATCH 04/12] =?UTF-8?q?Update=20active=20application=20selected?= =?UTF-8?q?=20code=20in=20transceiver=5Finfo=20table=20aft=E2=80=A6=20(#38?= =?UTF-8?q?1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update active application selected code in transceiver_info table after CMIS config finished successfully * code rearrangement * add tests for post_port_active_apsel_to_db() * also update host_lane_count and media_lane_count --- sonic-xcvrd/tests/test_xcvrd.py | 83 +++++++++++++++++++++++++++++++++ sonic-xcvrd/xcvrd/xcvrd.py | 31 ++++++++++++ 2 files changed, 114 insertions(+) diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 2581c7a0d..6123bef39 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -764,6 +764,89 @@ def get_host_lane_assignment_option_side_effect(app): appl = task.get_cmis_application_desired(mock_xcvr_api, host_lane_count, speed) assert task.get_cmis_host_lanes_mask(mock_xcvr_api, appl, host_lane_count, subport) == expected + def test_CmisManagerTask_post_port_active_apsel_to_db(self): + mock_xcvr_api = MagicMock() + mock_xcvr_api.get_active_apsel_hostlane = MagicMock(side_effect=[ + { + 'ActiveAppSelLane1': 1, + 'ActiveAppSelLane2': 1, + 'ActiveAppSelLane3': 1, + 'ActiveAppSelLane4': 1, + 'ActiveAppSelLane5': 1, + 'ActiveAppSelLane6': 1, + 'ActiveAppSelLane7': 1, + 'ActiveAppSelLane8': 1 + }, + { + 'ActiveAppSelLane1': 2, + 'ActiveAppSelLane2': 2, + 'ActiveAppSelLane3': 2, + 'ActiveAppSelLane4': 2, + 'ActiveAppSelLane5': 2, + 'ActiveAppSelLane6': 2, + 'ActiveAppSelLane7': 2, + 'ActiveAppSelLane8': 2 + }, + NotImplementedError + ]) + mock_xcvr_api.get_application_advertisement = MagicMock(side_effect=[ + { + 1: { + 'media_lane_count': 4, + 'host_lane_count': 8 + } + }, + { + 2: { + 'media_lane_count': 1, + 'host_lane_count': 2 + } + } + ]) + + int_tbl = Table("STATE_DB", TRANSCEIVER_INFO_TABLE) + + port_mapping = PortMapping() + stop_event = threading.Event() + task = CmisManagerTask(DEFAULT_NAMESPACE, port_mapping, stop_event) + task.xcvr_table_helper.get_intf_tbl = MagicMock(return_value=int_tbl) + + # case: partial lanes update + lport = "Ethernet0" + host_lanes_mask = 0xc + ret = task.post_port_active_apsel_to_db(mock_xcvr_api, lport, host_lanes_mask) + assert int_tbl.getKeys() == ["Ethernet0"] + assert dict(int_tbl.mock_dict["Ethernet0"]) == {'active_apsel_hostlane3': '1', + 'active_apsel_hostlane4': '1', + 'host_lane_count': '8', + 'media_lane_count': '4'} + # case: full lanes update + lport = "Ethernet8" + host_lanes_mask = 0xff + task.post_port_active_apsel_to_db(mock_xcvr_api, lport, host_lanes_mask) + assert int_tbl.getKeys() == ["Ethernet0", "Ethernet8"] + assert dict(int_tbl.mock_dict["Ethernet0"]) == {'active_apsel_hostlane3': '1', + 'active_apsel_hostlane4': '1', + 'host_lane_count': '8', + 'media_lane_count': '4'} + assert dict(int_tbl.mock_dict["Ethernet8"]) == {'active_apsel_hostlane1': '2', + 'active_apsel_hostlane2': '2', + 'active_apsel_hostlane3': '2', + 'active_apsel_hostlane4': '2', + 'active_apsel_hostlane5': '2', + 'active_apsel_hostlane6': '2', + 'active_apsel_hostlane7': '2', + 'active_apsel_hostlane8': '2', + 'host_lane_count': '2', + 'media_lane_count': '1'} + + # case: NotImplementedError + int_tbl = Table("STATE_DB", TRANSCEIVER_INFO_TABLE) # a new empty table + lport = "Ethernet0" + host_lanes_mask = 0xf + ret = task.post_port_active_apsel_to_db(mock_xcvr_api, lport, host_lanes_mask) + assert int_tbl.getKeys() == [] + @patch('xcvrd.xcvrd.platform_chassis') @patch('xcvrd.xcvrd_utilities.port_mapping.subscribe_port_update_event', MagicMock(return_value=(None, None))) @patch('xcvrd.xcvrd_utilities.port_mapping.handle_port_update_event', MagicMock()) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index f59cf6433..8a273e062 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1335,6 +1335,36 @@ def configure_laser_frequency(self, api, lport, freq, grid=75): self.log_error("{} Tuning in progress, subport selection may fail!".format(lport)) return api.set_laser_freq(freq, grid) + def post_port_active_apsel_to_db(self, api, lport, host_lanes_mask): + try: + act_apsel = api.get_active_apsel_hostlane() + appl_advt = api.get_application_advertisement() + except NotImplementedError: + helper_logger.log_error("Required feature is not implemented") + return + + tuple_list = [] + for lane in range(self.CMIS_MAX_HOST_LANES): + if ((1 << lane) & host_lanes_mask) == 0: + continue + act_apsel_lane = act_apsel.get('ActiveAppSelLane{}'.format(lane + 1), 'N/A') + tuple_list.append(('active_apsel_hostlane{}'.format(lane + 1), + str(act_apsel_lane))) + + # also update host_lane_count and media_lane_count + if len(tuple_list) > 0: + appl_advt_act = appl_advt.get(act_apsel_lane) + host_lane_count = appl_advt_act.get('host_lane_count', 'N/A') if appl_advt_act else 'N/A' + tuple_list.append(('host_lane_count', str(host_lane_count))) + media_lane_count = appl_advt_act.get('media_lane_count', 'N/A') if appl_advt_act else 'N/A' + tuple_list.append(('media_lane_count', str(media_lane_count))) + + asic_index = self.port_mapping.get_asic_id_for_logical_port(lport) + intf_tbl = self.xcvr_table_helper.get_intf_tbl(asic_index) + fvs = swsscommon.FieldValuePairs(tuple_list) + intf_tbl.set(lport, fvs) + self.log_notice("{}: updated TRANSCEIVER_INFO_TABLE {}".format(lport, tuple_list)) + def wait_for_port_config_done(self, namespace): # Connect to APPL_DB and subscribe to PORT table notifications appl_db = daemon_base.db_connect("APPL_DB", namespace=namespace) @@ -1641,6 +1671,7 @@ def task_worker(self): self.log_notice("{}: READY".format(lport)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_READY + self.post_port_active_apsel_to_db(api, lport, host_lanes_mask) except (NotImplementedError, AttributeError) as e: self.log_error("{}: internal errors due to {}".format(lport, e)) From d73808cead2aaf386ae033d9d571882c92f0377c Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan <93744978+assrinivasan@users.noreply.github.com> Date: Wed, 12 Jul 2023 10:58:39 -0700 Subject: [PATCH 05/12] Added PCIe transaction check for all peripherals on the bus (#331) * Added PCIe transaction check for all peripherals on the bus This is to determine if there are any unreachable devices on the PCIe bus. The gold list of PCI peripherals is a static file that varies depending on the platform. BDF information is parsed from this YAML file. Transcations are then carried out to determine the health of each peripheral. * Modified code to read pcie devices from the pre-configured YAML file Modified cmd to be executed to setpci (does pci transaction) Added exception handling * Formalized syslog error messages Added specific check for ASIC missing to differentiate from device mismatch Added yaml support * Made changes to the way setpci command is called per SA review comments * Added comments per prgeor review comments * Added relevant comments per prgeor review comments * Fixed a bug that was causing UnboundLocalError. * Added unit tests for the check_pcie_devices method --- sonic-pcied/scripts/pcied | 53 ++++++++++ sonic-pcied/tests/test_DaemonPcied.py | 138 ++++++++++++++++++++++++-- 2 files changed, 184 insertions(+), 7 deletions(-) diff --git a/sonic-pcied/scripts/pcied b/sonic-pcied/scripts/pcied index f88f159d1..e42748eca 100644 --- a/sonic-pcied/scripts/pcied +++ b/sonic-pcied/scripts/pcied @@ -9,6 +9,8 @@ import os import signal import sys import threading +import subprocess +import yaml from sonic_py_common import daemon_base, device_info, logger from swsscommon import swsscommon @@ -32,6 +34,7 @@ PCIED_MAIN_THREAD_SLEEP_SECS = 60 PCIEUTIL_CONF_FILE_ERROR = 1 PCIEUTIL_LOAD_ERROR = 2 +PLATFORM_PCIE_YAML_FILE = "/usr/share/sonic/platform/pcie.yaml" platform_pcieutil = None @@ -157,6 +160,56 @@ class DaemonPcied(daemon_base.DaemonBase): if self.resultInfo is None: return + # This block reads the PCIE YAML file, iterates through each device and + # for each device runs a transaction check to verify that the device ID + # matches the one on file. In the event of an invalid device ID such as + # '0xffff', it marks the device as missing as that is the only scenario + # in which such a value is returned. + + # Default, invalid BDF values to begin + bus = None + device = None + func = None + + try: + stream = open(PLATFORM_PCIE_YAML_FILE, 'r') + + # Load contents of the PCIe YAML file into a dictionary object + dictionary = yaml.safe_load(stream) + + # Iterate through each element in dict; extract the BDF information and device ID for that element + for elem in dictionary: + bus = int(elem.get('bus'), 16) + device = int(elem.get('dev'), 16) + func = int(elem.get('fn'), 16) + pcieDeviceID = elem.get('id') + + # Form the PCI device address from the BDF information above + pcie_device = ("{:02x}:{:02x}.{:01d}".format(bus, device, func)) + + # Form and run the command to read the device ID from the PCI device + cmd = ["setpci", "-s", pcie_device, "2.w"] + output = subprocess.check_output(cmd) + pcieDeviceQueryResult = output.decode('utf8').rstrip() + + # verify that the queried device ID matches what we have on file for the current PCI device + # If not, take appropriate action based on the queried value + if pcieDeviceQueryResult is None or "No devices selected" in pcieDeviceQueryResult: + self.log_error("Invalid PCIe Peripheral {:02x}:{:02x}.{:01d} - {}".format(bus, device, func, pcieDeviceQueryResult)) + elif pcieDeviceQueryResult != pcieDeviceID: + if pcieDeviceQueryResult == 'ffff': + self.log_error("PCIe device {:02x}:{:02x}.{:01d} missing.".format(bus, device, func)) + else: + self.log_error("PCIe device {:02x}:{:02x}.{:01d} ID mismatch. Expected {}, received {}".format(bus, device, func, pcieDeviceID, pcieDeviceQueryResult)) + + except Exception as ex: + # Verify that none of the BDF objects are None. If condition evals to False, do not mention BDF information in error log + if not [i for i in (bus, device, func) if i is None]: + self.log_error("PCIe Exception occurred for PCIe device {:02x}:{:02x}.{:01d}: {}".format(bus, device, func, ex)) + else: + self.log_error("PCIe Exception occurred: {}".format(ex)) + pass + for result in self.resultInfo: if result["result"] == "Failed": self.log_warning("PCIe Device: " + result["name"] + " Not Found") diff --git a/sonic-pcied/tests/test_DaemonPcied.py b/sonic-pcied/tests/test_DaemonPcied.py index 2c3c953e7..cfcbf33eb 100644 --- a/sonic-pcied/tests/test_DaemonPcied.py +++ b/sonic-pcied/tests/test_DaemonPcied.py @@ -59,18 +59,23 @@ pcie_check_result_no = [] -pcie_check_result_pass = \ -""" -[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, +pcie_check_result_pass = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, {'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'}, {'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Passed'}] -""" -pcie_check_result_fail = \ -""" -[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, + +pcie_check_result_fail = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, {'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'}, {'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Failed'}] + + +TEST_PLATFORM_PCIE_YAML_FILE = \ +""" +- bus: '00' + dev: '01' + fn: '0' + id: '9170' + name: 'PCI A' """ class TestDaemonPcied(object): @@ -143,6 +148,125 @@ def test_run(self): daemon_pcied.run() assert daemon_pcied.check_pcie_devices.call_count == 1 + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_yaml_file_open_error(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + pcied.platform_pcieutil.get_pcie_check = mock.MagicMock() + + daemon_pcied.check_pcie_devices() + + assert pcied.platform_pcieutil.get_pcie_check.called + + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_result_fail(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_fail) + + daemon_pcied.check_pcie_devices() + + assert pcied.platform_pcieutil.get_pcie_check.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_result_pass(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_pass) + + daemon_pcied.check_pcie_devices() + + assert pcied.platform_pcieutil.get_pcie_check.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_result_none(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=None) + + daemon_pcied.check_pcie_devices() + + assert pcied.platform_pcieutil.get_pcie_check.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_load_yaml_happy(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + + with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: + + class MockOutput: + def decode(self, encodingType): + return self + def rstrip(self): + return "9170" + + mock_output = MockOutput() + with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: + mock_check_output.return_value = mock_output + + daemon_pcied.check_pcie_devices() + + assert mock_check_output.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_load_yaml_mismatch(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + + with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: + + class MockOutput: + def decode(self, encodingType): + return self + def rstrip(self): + return "0123" + + mock_output = MockOutput() + with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: + mock_check_output.return_value = mock_output + + daemon_pcied.check_pcie_devices() + + assert mock_check_output.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_load_yaml_missing_device(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + + with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: + + class MockOutput: + def decode(self, encodingType): + return self + def rstrip(self): + return "ffff" + + mock_output = MockOutput() + with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: + mock_check_output.return_value = mock_output + + daemon_pcied.check_pcie_devices() + + assert mock_check_output.called + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) + def test_check_pcie_devices_load_yaml_invalid_device(self): + daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) + + with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: + + class MockOutput: + def decode(self, encodingType): + return self + def rstrip(self): + return "No devices selected" + + mock_output = MockOutput() + with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: + mock_check_output.return_value = mock_output + + daemon_pcied.check_pcie_devices() + + assert mock_check_output.called + + + @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) def test_check_pcie_devices(self): daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) From db6e340d6ed3f8d0e5dbfa4d587bab60281f52a3 Mon Sep 17 00:00:00 2001 From: MichaelWangSmci <133941180+MichaelWangSmci@users.noreply.github.com> Date: Sat, 15 Jul 2023 03:31:34 +0800 Subject: [PATCH 06/12] Fix index out of range in the error log of invalid media lane mask received (#386) Signed-off-by: Michael Wang --- sonic-xcvrd/xcvrd/xcvrd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 8a273e062..6fc59a00f 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -1530,8 +1530,8 @@ def task_worker(self): appl, lport, subport) if self.port_dict[lport]['media_lanes_mask'] <= 0: self.log_error("{}: Invalid media lane mask received - media_lane_count {} " - "media_lane_assignment_options {} lport{} subport {}" - " appl {}!".format(media_lane_count,media_lane_assignment_options,lport,subport,appl)) + "media_lane_assignment_options {} subport {}" + " appl {}!".format(lport, media_lane_count, media_lane_assignment_options, subport, appl)) self.port_dict[lport]['cmis_state'] = self.CMIS_STATE_FAILED continue media_lanes_mask = self.port_dict[lport]['media_lanes_mask'] From 94242c2465d521238535704347f0d01dfd0293c9 Mon Sep 17 00:00:00 2001 From: spilkey-cisco <110940806+spilkey-cisco@users.noreply.github.com> Date: Fri, 14 Jul 2023 16:40:07 -0700 Subject: [PATCH 07/12] Use vendor customizable fan speed threshold checks (#378) --- sonic-thermalctld/scripts/thermalctld | 79 ++++++++------------- sonic-thermalctld/tests/test_thermalctld.py | 68 ++++++------------ 2 files changed, 50 insertions(+), 97 deletions(-) diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 806bf5413..0abc0f4b0 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -119,57 +119,35 @@ class FanStatus(logger.Logger): self.status = status return True - def _check_speed_value_available(self, speed, target_speed, tolerance, current_status): - if speed == NOT_AVAILABLE or target_speed == NOT_AVAILABLE or tolerance == NOT_AVAILABLE: - if isinstance(tolerance, int) and (tolerance > 100 or tolerance < 0): - self.log_warning('Invalid tolerance value: {}'.format(tolerance)) - return False - - if current_status is True: - self.log_warning('Fan speed or target_speed or tolerance became unavailable, ' - 'speed={}, target_speed={}, tolerance={}'.format(speed, target_speed, tolerance)) - return False - return True - - def set_under_speed(self, speed, target_speed, tolerance): + def set_under_speed(self, is_under_speed): """ Set and cache Fan under speed status - :param speed: Fan speed - :param target_speed: Fan target speed - :param tolerance: Threshold between Fan speed and target speed + :param is_under_speed: Fan under speed threshold status :return: True if status changed else False """ - if not self._check_speed_value_available(speed, target_speed, tolerance, self.under_speed): - old_status = self.under_speed - self.under_speed = False - return old_status != self.under_speed + if is_under_speed == NOT_AVAILABLE: + if self.under_speed: + self.log_warning('Fan under speed threshold check became unavailable') + is_under_speed = False - status = speed < target_speed * (1 - float(tolerance) / 100) - if status == self.under_speed: - return False + old_status = self.under_speed + self.under_speed = is_under_speed + return old_status != self.under_speed - self.under_speed = status - return True - - def set_over_speed(self, speed, target_speed, tolerance): + def set_over_speed(self, is_over_speed): """ Set and cache Fan over speed status - :param speed: Fan speed - :param target_speed: Fan target speed - :param tolerance: Threshold between Fan speed and target speed + :param is_over_speed: Fan over speed threshold status :return: True if status changed else False """ - if not self._check_speed_value_available(speed, target_speed, tolerance, self.over_speed): - old_status = self.over_speed - self.over_speed = False - return old_status != self.over_speed + if is_over_speed == NOT_AVAILABLE: + if self.over_speed: + self.log_warning('Fan over speed threshold check became unavailable') + is_over_speed = False - status = speed > target_speed * (1 + float(tolerance) / 100) - if status == self.over_speed: - return False - - self.over_speed = status - return True + old_status = self.over_speed + self.over_speed = is_over_speed + return old_status != self.over_speed def is_ok(self): """ @@ -315,16 +293,18 @@ class FanUpdater(logger.Logger): fan_status = self.fan_status_dict[fan_name] speed = NOT_AVAILABLE - speed_tolerance = NOT_AVAILABLE speed_target = NOT_AVAILABLE + is_under_speed = NOT_AVAILABLE + is_over_speed = NOT_AVAILABLE fan_fault_status = NOT_AVAILABLE fan_direction = NOT_AVAILABLE is_replaceable = try_get(fan.is_replaceable, False) presence = try_get(fan.get_presence, False) if presence: speed = try_get(fan.get_speed) - speed_tolerance = try_get(fan.get_speed_tolerance) speed_target = try_get(fan.get_target_speed) + is_under_speed = try_get(fan.is_under_speed) + is_over_speed = try_get(fan.is_over_speed) fan_fault_status = try_get(fan.get_status, False) fan_direction = try_get(fan.get_direction) @@ -344,20 +324,20 @@ class FanUpdater(logger.Logger): 'Fan fault warning: {} is broken'.format(fan_name) ) - if presence and fan_status.set_under_speed(speed, speed_target, speed_tolerance): + if presence and fan_status.set_under_speed(is_under_speed): set_led = True self._log_on_status_changed(not fan_status.under_speed, 'Fan low speed warning cleared: {} speed is back to normal'.format(fan_name), - 'Fan low speed warning: {} current speed={}, target speed={}, tolerance={}'. - format(fan_name, speed, speed_target, speed_tolerance) + 'Fan low speed warning: {} current speed={}, target speed={}'. + format(fan_name, speed, speed_target) ) - if presence and fan_status.set_over_speed(speed, speed_target, speed_tolerance): + if presence and fan_status.set_over_speed(is_over_speed): set_led = True self._log_on_status_changed(not fan_status.over_speed, 'Fan high speed warning cleared: {} speed is back to normal'.format(fan_name), - 'Fan high speed warning: {} target speed={}, current speed={}, tolerance={}'. - format(fan_name, speed_target, speed, speed_tolerance) + 'Fan high speed warning: {} current speed={}, target speed={}'. + format(fan_name, speed, speed_target) ) # We don't set PSU led here, PSU led will be handled in psud @@ -376,8 +356,9 @@ class FanUpdater(logger.Logger): ('status', str(fan_fault_status)), ('direction', str(fan_direction)), ('speed', str(speed)), - ('speed_tolerance', str(speed_tolerance)), ('speed_target', str(speed_target)), + ('is_under_speed', str(is_under_speed)), + ('is_over_speed', str(is_over_speed)), ('is_replaceable', str(is_replaceable)), ('timestamp', datetime.now().strftime('%Y%m%d %H:%M:%S')) ]) diff --git a/sonic-thermalctld/tests/test_thermalctld.py b/sonic-thermalctld/tests/test_thermalctld.py index 3c8d14c89..6fe9ccbd1 100644 --- a/sonic-thermalctld/tests/test_thermalctld.py +++ b/sonic-thermalctld/tests/test_thermalctld.py @@ -68,30 +68,6 @@ class TestFanStatus(object): """ Test cases to cover functionality in FanStatus class """ - def test_check_speed_value_available(self): - fan_status = thermalctld.FanStatus() - - ret = fan_status._check_speed_value_available(30, 32, 5, True) - assert ret == True - assert fan_status.log_warning.call_count == 0 - - ret = fan_status._check_speed_value_available(thermalctld.NOT_AVAILABLE, 32, 105, True) - assert ret == False - assert fan_status.log_warning.call_count == 1 - fan_status.log_warning.assert_called_with('Invalid tolerance value: 105') - - # Reset - fan_status.log_warning.reset_mock() - - ret = fan_status._check_speed_value_available(thermalctld.NOT_AVAILABLE, 32, 5, False) - assert ret == False - assert fan_status.log_warning.call_count == 0 - - ret = fan_status._check_speed_value_available(thermalctld.NOT_AVAILABLE, 32, 5, True) - assert ret == False - assert fan_status.log_warning.call_count == 1 - fan_status.log_warning.assert_called_with('Fan speed or target_speed or tolerance became unavailable, speed=N/A, target_speed=32, tolerance=5') - def test_set_presence(self): fan_status = thermalctld.FanStatus() ret = fan_status.set_presence(True) @@ -104,52 +80,48 @@ def test_set_presence(self): def test_set_under_speed(self): fan_status = thermalctld.FanStatus() - ret = fan_status.set_under_speed(thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE) - assert not ret - - ret = fan_status.set_under_speed(thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE, 0) - assert not ret - ret = fan_status.set_under_speed(thermalctld.NOT_AVAILABLE, 0, 0) + ret = fan_status.set_under_speed(False) assert not ret - ret = fan_status.set_under_speed(0, 0, 0) - assert not ret - - ret = fan_status.set_under_speed(80, 100, 19) + ret = fan_status.set_under_speed(True) assert ret assert fan_status.under_speed assert not fan_status.is_ok() - ret = fan_status.set_under_speed(81, 100, 19) + ret = fan_status.set_under_speed(True) + assert not ret + + ret = fan_status.set_under_speed(False) assert ret assert not fan_status.under_speed assert fan_status.is_ok() - def test_set_over_speed(self): - fan_status = thermalctld.FanStatus() - ret = fan_status.set_over_speed(thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE) - assert not ret - - ret = fan_status.set_over_speed(thermalctld.NOT_AVAILABLE, thermalctld.NOT_AVAILABLE, 0) + ret = fan_status.set_under_speed(False) assert not ret - ret = fan_status.set_over_speed(thermalctld.NOT_AVAILABLE, 0, 0) - assert not ret + def test_set_over_speed(self): + fan_status = thermalctld.FanStatus() - ret = fan_status.set_over_speed(0, 0, 0) + ret = fan_status.set_over_speed(False) assert not ret - ret = fan_status.set_over_speed(120, 100, 19) + ret = fan_status.set_over_speed(True) assert ret assert fan_status.over_speed assert not fan_status.is_ok() - ret = fan_status.set_over_speed(120, 100, 21) + ret = fan_status.set_over_speed(True) + assert not ret + + ret = fan_status.set_over_speed(False) assert ret assert not fan_status.over_speed assert fan_status.is_ok() + ret = fan_status.set_over_speed(False) + assert not ret + class TestFanUpdater(object): """ @@ -251,7 +223,7 @@ def test_fan_under_speed(self): fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED assert fan_updater.log_warning.call_count == 1 - fan_updater.log_warning.assert_called_with('Fan low speed warning: FanDrawer 0 fan 1 current speed=1, target speed=2, tolerance=0') + fan_updater.log_warning.assert_called_with('Fan low speed warning: FanDrawer 0 fan 1 current speed=1, target speed=2') fan_list[0].make_normal_speed() fan_updater.update() @@ -267,7 +239,7 @@ def test_fan_over_speed(self): fan_list = chassis.get_all_fans() assert fan_list[0].get_status_led() == MockFan.STATUS_LED_COLOR_RED assert fan_updater.log_warning.call_count == 1 - fan_updater.log_warning.assert_called_with('Fan high speed warning: FanDrawer 0 fan 1 target speed=1, current speed=2, tolerance=0') + fan_updater.log_warning.assert_called_with('Fan high speed warning: FanDrawer 0 fan 1 current speed=2, target speed=1') fan_list[0].make_normal_speed() fan_updater.update() From 76baca3d71542dbeeeeac6bea45b762abab7166f Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan <93744978+assrinivasan@users.noreply.github.com> Date: Wed, 19 Jul 2023 12:02:47 -0700 Subject: [PATCH 08/12] Fixes for the issues uncovered by sonic-pcied unit tests (#389) * Fixes for the following issues: - Lack of getKeys() impl in mock swsscommon table class in sonic-pcied - Fixed a 'set' bug in pcied that was uncovered by new code flows * Removed mocked table instances per prgeor review comments --- sonic-pcied/scripts/pcied | 3 ++- .../tests/mocked_libs/swsscommon/swsscommon.py | 3 +++ sonic-pcied/tests/test_DaemonPcied.py | 11 ----------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/sonic-pcied/scripts/pcied b/sonic-pcied/scripts/pcied index e42748eca..64f898f7f 100644 --- a/sonic-pcied/scripts/pcied +++ b/sonic-pcied/scripts/pcied @@ -134,7 +134,8 @@ class DaemonPcied(daemon_base.DaemonBase): self.aer_stats = {} if Id is not None: - self.device_table.set(self.device_name, [('id', Id)]) + fvp = swsscommon.FieldValuePairs([('id', Id)]) + self.device_table.set(self.device_name, fvp) self.aer_stats = platform_pcieutil.get_pcie_aer_stats(bus=Bus, dev=Dev, func=Fn) self.update_aer_to_statedb() diff --git a/sonic-pcied/tests/mocked_libs/swsscommon/swsscommon.py b/sonic-pcied/tests/mocked_libs/swsscommon/swsscommon.py index e5aa2a2ba..ddb3cd686 100644 --- a/sonic-pcied/tests/mocked_libs/swsscommon/swsscommon.py +++ b/sonic-pcied/tests/mocked_libs/swsscommon/swsscommon.py @@ -26,6 +26,9 @@ def get(self, key): def get_size(self): return (len(self.mock_dict)) + def getKeys(self): + return list(self.mock_dict.keys()) + class FieldValuePairs: fv_dict = {} diff --git a/sonic-pcied/tests/test_DaemonPcied.py b/sonic-pcied/tests/test_DaemonPcied.py index cfcbf33eb..6deba44c3 100644 --- a/sonic-pcied/tests/test_DaemonPcied.py +++ b/sonic-pcied/tests/test_DaemonPcied.py @@ -282,22 +282,18 @@ def test_check_pcie_devices(self): @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) def test_update_pcie_devices_status_db(self): daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - daemon_pcied.status_table = mock.MagicMock() daemon_pcied.log_info = mock.MagicMock() daemon_pcied.log_error = mock.MagicMock() # test for pass resultInfo daemon_pcied.update_pcie_devices_status_db(0) - assert daemon_pcied.status_table.set.call_count == 1 assert daemon_pcied.log_info.call_count == 1 assert daemon_pcied.log_error.call_count == 0 - daemon_pcied.status_table.set.reset_mock() daemon_pcied.log_info.reset_mock() # test for resultInfo with 1 device failed to detect daemon_pcied.update_pcie_devices_status_db(1) - assert daemon_pcied.status_table.set.call_count == 1 assert daemon_pcied.log_info.call_count == 0 assert daemon_pcied.log_error.call_count == 1 @@ -306,20 +302,17 @@ def test_update_pcie_devices_status_db(self): @mock.patch('pcied.read_id_file') def test_check_n_update_pcie_aer_stats(self, mock_read): daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - daemon_pcied.device_table = mock.MagicMock() daemon_pcied.update_aer_to_statedb = mock.MagicMock() pcied.platform_pcieutil.get_pcie_aer_stats = mock.MagicMock() mock_read.return_value = None daemon_pcied.check_n_update_pcie_aer_stats(0,1,0) assert daemon_pcied.update_aer_to_statedb.call_count == 0 - assert daemon_pcied.device_table.set.call_count == 0 assert pcied.platform_pcieutil.get_pcie_aer_stats.call_count == 0 mock_read.return_value = '1714' daemon_pcied.check_n_update_pcie_aer_stats(0,1,0) assert daemon_pcied.update_aer_to_statedb.call_count == 1 - assert daemon_pcied.device_table.set.call_count == 1 assert pcied.platform_pcieutil.get_pcie_aer_stats.call_count == 1 @@ -327,7 +320,6 @@ def test_check_n_update_pcie_aer_stats(self, mock_read): def test_update_aer_to_statedb(self): daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) daemon_pcied.log_debug = mock.MagicMock() - daemon_pcied.device_table = mock.MagicMock() daemon_pcied.device_name = mock.MagicMock() daemon_pcied.aer_stats = pcie_aer_stats_no_err @@ -344,6 +336,3 @@ def test_update_aer_to_statedb(self): daemon_pcied.update_aer_to_statedb() assert daemon_pcied.log_debug.call_count == 0 - assert daemon_pcied.device_table.set.call_count == 1 - - daemon_pcied.device_table.set.reset_mock() From f3c26319c423c38c8f8aa16e112c1ea768ea4497 Mon Sep 17 00:00:00 2001 From: Ashwin Srinivasan <93744978+assrinivasan@users.noreply.github.com> Date: Tue, 8 Aug 2023 15:50:36 -0700 Subject: [PATCH 09/12] Revert pcied enhancements (#392) * Revert "Fixes for the issues uncovered by sonic-pcied unit tests (#389)" This reverts commit 76baca3d71542dbeeeeac6bea45b762abab7166f. * Revert "Added PCIe transaction check for all peripherals on the bus (#331)" This reverts commit d73808cead2aaf386ae033d9d571882c92f0377c. * Fixes for the issues uncovered by sonic-pcied unit tests (#389) * Fixes for the following issues: - Lack of getKeys() impl in mock swsscommon table class in sonic-pcied - Fixed a 'set' bug in pcied that was uncovered by new code flows * Removed mocked table instances per prgeor review comments --- sonic-pcied/scripts/pcied | 53 ---------- sonic-pcied/tests/test_DaemonPcied.py | 138 ++------------------------ 2 files changed, 7 insertions(+), 184 deletions(-) diff --git a/sonic-pcied/scripts/pcied b/sonic-pcied/scripts/pcied index 64f898f7f..fb59fff3f 100644 --- a/sonic-pcied/scripts/pcied +++ b/sonic-pcied/scripts/pcied @@ -9,8 +9,6 @@ import os import signal import sys import threading -import subprocess -import yaml from sonic_py_common import daemon_base, device_info, logger from swsscommon import swsscommon @@ -34,7 +32,6 @@ PCIED_MAIN_THREAD_SLEEP_SECS = 60 PCIEUTIL_CONF_FILE_ERROR = 1 PCIEUTIL_LOAD_ERROR = 2 -PLATFORM_PCIE_YAML_FILE = "/usr/share/sonic/platform/pcie.yaml" platform_pcieutil = None @@ -161,56 +158,6 @@ class DaemonPcied(daemon_base.DaemonBase): if self.resultInfo is None: return - # This block reads the PCIE YAML file, iterates through each device and - # for each device runs a transaction check to verify that the device ID - # matches the one on file. In the event of an invalid device ID such as - # '0xffff', it marks the device as missing as that is the only scenario - # in which such a value is returned. - - # Default, invalid BDF values to begin - bus = None - device = None - func = None - - try: - stream = open(PLATFORM_PCIE_YAML_FILE, 'r') - - # Load contents of the PCIe YAML file into a dictionary object - dictionary = yaml.safe_load(stream) - - # Iterate through each element in dict; extract the BDF information and device ID for that element - for elem in dictionary: - bus = int(elem.get('bus'), 16) - device = int(elem.get('dev'), 16) - func = int(elem.get('fn'), 16) - pcieDeviceID = elem.get('id') - - # Form the PCI device address from the BDF information above - pcie_device = ("{:02x}:{:02x}.{:01d}".format(bus, device, func)) - - # Form and run the command to read the device ID from the PCI device - cmd = ["setpci", "-s", pcie_device, "2.w"] - output = subprocess.check_output(cmd) - pcieDeviceQueryResult = output.decode('utf8').rstrip() - - # verify that the queried device ID matches what we have on file for the current PCI device - # If not, take appropriate action based on the queried value - if pcieDeviceQueryResult is None or "No devices selected" in pcieDeviceQueryResult: - self.log_error("Invalid PCIe Peripheral {:02x}:{:02x}.{:01d} - {}".format(bus, device, func, pcieDeviceQueryResult)) - elif pcieDeviceQueryResult != pcieDeviceID: - if pcieDeviceQueryResult == 'ffff': - self.log_error("PCIe device {:02x}:{:02x}.{:01d} missing.".format(bus, device, func)) - else: - self.log_error("PCIe device {:02x}:{:02x}.{:01d} ID mismatch. Expected {}, received {}".format(bus, device, func, pcieDeviceID, pcieDeviceQueryResult)) - - except Exception as ex: - # Verify that none of the BDF objects are None. If condition evals to False, do not mention BDF information in error log - if not [i for i in (bus, device, func) if i is None]: - self.log_error("PCIe Exception occurred for PCIe device {:02x}:{:02x}.{:01d}: {}".format(bus, device, func, ex)) - else: - self.log_error("PCIe Exception occurred: {}".format(ex)) - pass - for result in self.resultInfo: if result["result"] == "Failed": self.log_warning("PCIe Device: " + result["name"] + " Not Found") diff --git a/sonic-pcied/tests/test_DaemonPcied.py b/sonic-pcied/tests/test_DaemonPcied.py index 6deba44c3..f3e343654 100644 --- a/sonic-pcied/tests/test_DaemonPcied.py +++ b/sonic-pcied/tests/test_DaemonPcied.py @@ -59,23 +59,18 @@ pcie_check_result_no = [] -pcie_check_result_pass = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, +pcie_check_result_pass = \ +""" +[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, {'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'}, {'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Passed'}] +""" - -pcie_check_result_fail = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, +pcie_check_result_fail = \ +""" +[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'}, {'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'}, {'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Failed'}] - - -TEST_PLATFORM_PCIE_YAML_FILE = \ -""" -- bus: '00' - dev: '01' - fn: '0' - id: '9170' - name: 'PCI A' """ class TestDaemonPcied(object): @@ -148,125 +143,6 @@ def test_run(self): daemon_pcied.run() assert daemon_pcied.check_pcie_devices.call_count == 1 - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_yaml_file_open_error(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - pcied.platform_pcieutil.get_pcie_check = mock.MagicMock() - - daemon_pcied.check_pcie_devices() - - assert pcied.platform_pcieutil.get_pcie_check.called - - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_result_fail(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_fail) - - daemon_pcied.check_pcie_devices() - - assert pcied.platform_pcieutil.get_pcie_check.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_result_pass(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_pass) - - daemon_pcied.check_pcie_devices() - - assert pcied.platform_pcieutil.get_pcie_check.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_result_none(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=None) - - daemon_pcied.check_pcie_devices() - - assert pcied.platform_pcieutil.get_pcie_check.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_load_yaml_happy(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - - with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: - - class MockOutput: - def decode(self, encodingType): - return self - def rstrip(self): - return "9170" - - mock_output = MockOutput() - with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: - mock_check_output.return_value = mock_output - - daemon_pcied.check_pcie_devices() - - assert mock_check_output.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_load_yaml_mismatch(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - - with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: - - class MockOutput: - def decode(self, encodingType): - return self - def rstrip(self): - return "0123" - - mock_output = MockOutput() - with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: - mock_check_output.return_value = mock_output - - daemon_pcied.check_pcie_devices() - - assert mock_check_output.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_load_yaml_missing_device(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - - with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: - - class MockOutput: - def decode(self, encodingType): - return self - def rstrip(self): - return "ffff" - - mock_output = MockOutput() - with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: - mock_check_output.return_value = mock_output - - daemon_pcied.check_pcie_devices() - - assert mock_check_output.called - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) - def test_check_pcie_devices_load_yaml_invalid_device(self): - daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) - - with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd: - - class MockOutput: - def decode(self, encodingType): - return self - def rstrip(self): - return "No devices selected" - - mock_output = MockOutput() - with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output: - mock_check_output.return_value = mock_output - - daemon_pcied.check_pcie_devices() - - assert mock_check_output.called - - - @mock.patch('pcied.load_platform_pcieutil', mock.MagicMock()) def test_check_pcie_devices(self): daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER) From 816059be4de5ef0c80a9f6330fe2561bf756dc4f Mon Sep 17 00:00:00 2001 From: Anoop Kamath <115578705+AnoopKamath@users.noreply.github.com> Date: Wed, 16 Aug 2023 11:57:03 -0700 Subject: [PATCH 10/12] Apply custom Si settings via CMIS: SONiC xcvrd platform daemon changes (#385) * Optics SI settings changes for platform daemon --- sonic-xcvrd/tests/optics_si_settings.json | 222 ++++++++++++++++++ sonic-xcvrd/tests/test_xcvrd.py | 42 ++++ sonic-xcvrd/xcvrd/xcvrd.py | 32 ++- .../xcvrd/xcvrd_utilities/optics_si_parser.py | 137 +++++++++++ 4 files changed, 430 insertions(+), 3 deletions(-) create mode 100644 sonic-xcvrd/tests/optics_si_settings.json create mode 100644 sonic-xcvrd/xcvrd/xcvrd_utilities/optics_si_parser.py diff --git a/sonic-xcvrd/tests/optics_si_settings.json b/sonic-xcvrd/tests/optics_si_settings.json new file mode 100644 index 000000000..2b9defc81 --- /dev/null +++ b/sonic-xcvrd/tests/optics_si_settings.json @@ -0,0 +1,222 @@ +{ + "GLOBAL_MEDIA_SETTINGS":{ + "0-31":{ + "100G_SPEED":{ + "CREDO-CAC82X321M2MC0HW":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":0, + "OutputAmplitudeTargetRx2":0, + "OutputAmplitudeTargetRx3":0, + "OutputAmplitudeTargetRx4":0, + "OutputAmplitudeTargetRx5":0, + "OutputAmplitudeTargetRx6":0, + "OutputAmplitudeTargetRx7":0, + "OutputAmplitudeTargetRx8":0 + } + } + } + } + }, + "PORT_MEDIA_SETTINGS":{ + "0":{ + "100G_SPEED":{ + "CREDO-CAC82X321M2MC0HW":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":0, + "OutputAmplitudeTargetRx2":0, + "OutputAmplitudeTargetRx3":0, + "OutputAmplitudeTargetRx4":0, + "OutputAmplitudeTargetRx5":0, + "OutputAmplitudeTargetRx6":0, + "OutputAmplitudeTargetRx7":0, + "OutputAmplitudeTargetRx8":0 + } + } + } + }, + "1":{ + "100G_SPEED":{ + "Default":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":1, + "OutputAmplitudeTargetRx2":1, + "OutputAmplitudeTargetRx3":1, + "OutputAmplitudeTargetRx4":1, + "OutputAmplitudeTargetRx5":1, + "OutputAmplitudeTargetRx6":1, + "OutputAmplitudeTargetRx7":1, + "OutputAmplitudeTargetRx8":1 + } + } + } + }, + "10":{ + "100G_SPEED":{ + "Default":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":1, + "OutputAmplitudeTargetRx2":1, + "OutputAmplitudeTargetRx3":1, + "OutputAmplitudeTargetRx4":1, + "OutputAmplitudeTargetRx5":1, + "OutputAmplitudeTargetRx6":1, + "OutputAmplitudeTargetRx7":1, + "OutputAmplitudeTargetRx8":1 + } + } + } + }, + "11":{ + "100G_SPEED":{ + "Default":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":1, + "OutputAmplitudeTargetRx2":1, + "OutputAmplitudeTargetRx3":1, + "OutputAmplitudeTargetRx4":1, + "OutputAmplitudeTargetRx5":1, + "OutputAmplitudeTargetRx6":1, + "OutputAmplitudeTargetRx7":1, + "OutputAmplitudeTargetRx8":1 + } + } + } + }, + "12":{ + "100G_SPEED":{ + "Default":{ + "OutputEqPreCursorTargetRx":{ + "OutputEqPreCursorTargetRx1":3, + "OutputEqPreCursorTargetRx2":3, + "OutputEqPreCursorTargetRx3":3, + "OutputEqPreCursorTargetRx4":3, + "OutputEqPreCursorTargetRx5":3, + "OutputEqPreCursorTargetRx6":3, + "OutputEqPreCursorTargetRx7":3, + "OutputEqPreCursorTargetRx8":3 + }, + "OutputEqPostCursorTargetRx":{ + "OutputEqPostCursorTargetRx1":0, + "OutputEqPostCursorTargetRx2":0, + "OutputEqPostCursorTargetRx3":0, + "OutputEqPostCursorTargetRx4":0, + "OutputEqPostCursorTargetRx5":0, + "OutputEqPostCursorTargetRx6":0, + "OutputEqPostCursorTargetRx7":0, + "OutputEqPostCursorTargetRx8":0 + }, + "OutputAmplitudeTargetRx":{ + "OutputAmplitudeTargetRx1":1, + "OutputAmplitudeTargetRx2":1, + "OutputAmplitudeTargetRx3":1, + "OutputAmplitudeTargetRx4":1, + "OutputAmplitudeTargetRx5":1, + "OutputAmplitudeTargetRx6":1, + "OutputAmplitudeTargetRx7":1, + "OutputAmplitudeTargetRx8":1 + } + } + } + } + } +} diff --git a/sonic-xcvrd/tests/test_xcvrd.py b/sonic-xcvrd/tests/test_xcvrd.py index 6123bef39..a3adf5823 100644 --- a/sonic-xcvrd/tests/test_xcvrd.py +++ b/sonic-xcvrd/tests/test_xcvrd.py @@ -1,6 +1,7 @@ #from unittest.mock import DEFAULT from xcvrd.xcvrd_utilities.port_mapping import * from xcvrd.xcvrd_utilities.sfp_status_helper import * +from xcvrd.xcvrd_utilities.optics_si_parser import * from xcvrd.xcvrd import * import pytest import copy @@ -41,6 +42,14 @@ global_media_settings = media_settings_with_comma_dict['GLOBAL_MEDIA_SETTINGS'].pop('1-32') media_settings_with_comma_dict['GLOBAL_MEDIA_SETTINGS']['1-5,6,7-20,21-32'] = global_media_settings +with open(os.path.join(test_path, 'optics_si_settings.json'), 'r') as fn: + optics_si_settings_dict = json.load(fn) +port_optics_si_settings = {} +optics_si_settings_with_comma_dict = copy.deepcopy(optics_si_settings_dict) +global_optics_si_settings = optics_si_settings_with_comma_dict['GLOBAL_MEDIA_SETTINGS'].pop('0-31') +port_optics_si_settings['PORT_MEDIA_SETTINGS'] = optics_si_settings_with_comma_dict.pop('PORT_MEDIA_SETTINGS') +optics_si_settings_with_comma_dict['GLOBAL_MEDIA_SETTINGS']['0-5,6,7-20,21-31'] = global_optics_si_settings + class TestXcvrdThreadException(object): @patch('xcvrd.xcvrd.platform_chassis', MagicMock()) @@ -470,6 +479,39 @@ def _check_notify_media_setting(self, index): port_mapping.handle_port_change_event(port_change_event) notify_media_setting(logical_port_name, xcvr_info_dict, app_port_tbl, port_mapping) + @patch('xcvrd.xcvrd_utilities.optics_si_parser.g_optics_si_dict', optics_si_settings_dict) + @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) + def test_fetch_optics_si_setting(self): + self._check_fetch_optics_si_setting(1) + + @patch('xcvrd.xcvrd_utilities.optics_si_parser.g_optics_si_dict', optics_si_settings_with_comma_dict) + @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) + def test_fetch_optics_si_setting_with_comma(self): + self._check_fetch_optics_si_setting(1) + self._check_fetch_optics_si_setting(6) + + @patch('xcvrd.xcvrd_utilities.optics_si_parser.g_optics_si_dict', port_optics_si_settings) + @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) + def test_fetch_optics_si_setting_with_port(self): + self._check_fetch_optics_si_setting(1) + + @patch('xcvrd.xcvrd._wrapper_get_presence', MagicMock(return_value=True)) + @patch('xcvrd.xcvrd_utilities.optics_si_parser.get_module_vendor_key', MagicMock(return_value=('CREDO-CAC82X321M','CREDO'))) + def _check_fetch_optics_si_setting(self, index): + port = 1 + lane_speed = 100 + mock_sfp = MagicMock() + optics_si_parser.fetch_optics_si_setting(port, lane_speed, mock_sfp) + + def test_get_module_vendor_key(self): + mock_sfp = MagicMock() + mock_xcvr_api = MagicMock() + mock_sfp.get_xcvr_api = MagicMock(return_value=mock_xcvr_api) + mock_xcvr_api.get_manufacturer = MagicMock(return_value='Credo ') + mock_xcvr_api.get_model = MagicMock(return_value='CAC82X321HW') + result = get_module_vendor_key(1, mock_sfp) + assert result == ('CREDO-CAC82X321HW','CREDO') + def test_detect_port_in_error_status(self): class MockTable: def get(self, key): diff --git a/sonic-xcvrd/xcvrd/xcvrd.py b/sonic-xcvrd/xcvrd/xcvrd.py index 6fc59a00f..ab06b9ab1 100644 --- a/sonic-xcvrd/xcvrd/xcvrd.py +++ b/sonic-xcvrd/xcvrd/xcvrd.py @@ -27,6 +27,7 @@ from .xcvrd_utilities import sfp_status_helper from .xcvrd_utilities import port_mapping + from .xcvrd_utilities import optics_si_parser except ImportError as e: raise ImportError(str(e) + " - required module not found") @@ -1591,6 +1592,11 @@ def task_worker(self): self.port_dict[lport]['cmis_expired'] = now + datetime.timedelta(seconds = max(modulePwrUpDuration, dpDeinitDuration)) elif state == self.CMIS_STATE_AP_CONF: + # Explicit control bit to apply custom Host SI settings. + # It will be set to 1 and applied via set_application if + # custom SI settings is applicable + ec = 0 + # TODO: Use fine grained time when the CMIS memory map is available if not self.check_module_state(api, ['ModuleReady']): if (expired is not None) and (expired <= now): @@ -1613,9 +1619,28 @@ def task_worker(self): else: self.log_notice("{} configured laser frequency {} GHz".format(lport, freq)) + # Stage custom SI settings + if optics_si_parser.optics_si_present(): + optics_si_dict = {} + # Apply module SI settings if applicable + lane_speed = int(speed/1000)//host_lane_count + optics_si_dict = optics_si_parser.fetch_optics_si_setting(pport, lane_speed, sfp) + + if optics_si_dict: + self.log_notice("{}: Apply Optics SI found for Vendor: {} PN: {} lane speed: {}G". + format(lport, api.get_manufacturer(), api.get_model(), lane_speed)) + if not api.stage_custom_si_settings(host_lanes_mask, optics_si_dict): + self.log_notice("{}: unable to stage custom SI settings ".format(lport)) + self.force_cmis_reinit(lport, retries + 1) + continue + + # Set Explicit control bit to apply Custom Host SI settings + ec = 1 + # D.1.3 Software Configuration and Initialization - if not api.set_application(host_lanes_mask, appl): - self.log_notice("{}: unable to set application".format(lport)) + api.set_application(host_lanes_mask, appl, ec) + if not api.scs_apply_datapath_init(host_lanes_mask): + self.log_notice("{}: unable to set application and stage DP init".format(lport)) self.force_cmis_reinit(lport, retries + 1) continue @@ -2450,9 +2475,10 @@ def init(self): self.xcvr_table_helper = XcvrTableHelper(self.namespaces) if is_fast_reboot_enabled(): - self.log_info("Skip loading media_settings.json in case of fast-reboot") + self.log_info("Skip loading media_settings.json and optics_si_settings.json in case of fast-reboot") else: self.load_media_settings() + optics_si_parser.load_optics_si_settings() # Make sure this daemon started after all port configured self.log_notice("XCVRD INIT: Wait for port config is done") diff --git a/sonic-xcvrd/xcvrd/xcvrd_utilities/optics_si_parser.py b/sonic-xcvrd/xcvrd/xcvrd_utilities/optics_si_parser.py new file mode 100644 index 000000000..fff418563 --- /dev/null +++ b/sonic-xcvrd/xcvrd/xcvrd_utilities/optics_si_parser.py @@ -0,0 +1,137 @@ +import json +import os + +from sonic_py_common import device_info, logger +from xcvrd import xcvrd + +g_optics_si_dict = {} + +SYSLOG_IDENTIFIER = "xcvrd" +helper_logger = logger.Logger(SYSLOG_IDENTIFIER) + +def get_optics_si_settings_value(physical_port, lane_speed, key, vendor_name_str): + GLOBAL_MEDIA_SETTINGS_KEY = 'GLOBAL_MEDIA_SETTINGS' + PORT_MEDIA_SETTINGS_KEY = 'PORT_MEDIA_SETTINGS' + DEFAULT_KEY = 'Default' + SPEED_KEY = str(lane_speed) + 'G_SPEED' + RANGE_SEPARATOR = '-' + COMMA_SEPARATOR = ',' + default_dict = {} + optics_si_dict = {} + + # Keys under global media settings can be a list or range or list of ranges + # of physical port numbers. Below are some examples + # 1-32 + # 1,2,3,4,5 + # 1-4,9-12 + + if GLOBAL_MEDIA_SETTINGS_KEY in g_optics_si_dict: + for keys in g_optics_si_dict[GLOBAL_MEDIA_SETTINGS_KEY]: + if COMMA_SEPARATOR in keys: + port_list = keys.split(COMMA_SEPARATOR) + for port in port_list: + if RANGE_SEPARATOR in port: + if xcvrd.check_port_in_range(port, physical_port): + optics_si_dict = g_optics_si_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] + break + elif str(physical_port) == port: + optics_si_dict = g_optics_si_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] + break + + elif RANGE_SEPARATOR in keys: + if xcvrd.check_port_in_range(keys, physical_port): + optics_si_dict = g_optics_si_dict[GLOBAL_MEDIA_SETTINGS_KEY][keys] + + key_dict = {} + if SPEED_KEY in optics_si_dict: + if key in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + return key_dict[key] + elif vendor_name_str in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + return key_dict[vendor_name_str] + elif DEFAULT_KEY in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + default_dict = key_dict[DEFAULT_KEY] + + optics_si_dict = {} + + if PORT_MEDIA_SETTINGS_KEY in g_optics_si_dict: + for keys in g_optics_si_dict[PORT_MEDIA_SETTINGS_KEY]: + if int(keys) == physical_port: + optics_si_dict = g_optics_si_dict[PORT_MEDIA_SETTINGS_KEY][keys] + break + if len(optics_si_dict) == 0: + if len(default_dict) != 0: + return default_dict + else: + helper_logger.log_error("Error: No values for physical port '{}'".format(physical_port)) + return {} + + key_dict = {} + if SPEED_KEY in optics_si_dict: + if key in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + return key_dict[key] + elif vendor_name_str in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + return key_dict[vendor_name_str] + elif DEFAULT_KEY in optics_si_dict[SPEED_KEY]: + key_dict = optics_si_dict[SPEED_KEY] + default_dict = key_dict[DEFAULT_KEY] + elif len(default_dict) != 0: + return default_dict + + return default_dict + +def get_module_vendor_key(physical_port, sfp): + api = sfp.get_xcvr_api() + if api is None: + helper_logger.log_info("Module {} xcvrd api not found".format(physical_port)) + return None + + vendor_name = api.get_manufacturer() + if vendor_name is None: + helper_logger.log_info("Module {} vendor name not found".format(physical_port)) + return None + + vendor_pn = api.get_model() + if vendor_pn is None: + helper_logger.log_info("Module {} vendor part number not found".format(physical_port)) + return None + + return vendor_name.upper().strip() + '-' + vendor_pn.upper().strip(), vendor_name.upper().strip() + +def fetch_optics_si_setting(physical_port, lane_speed, sfp): + if not g_optics_si_dict: + return + + optics_si = {} + + if not xcvrd._wrapper_get_presence(physical_port): + helper_logger.log_info("Module {} presence not detected during notify".format(physical_port)) + return optics_si + vendor_key, vendor_name = get_module_vendor_key(physical_port, sfp) + if vendor_key is None or vendor_name is None: + helper_logger.log_error("Error: No Vendor Key found for port '{}'".format(logical_port_name)) + return optics_si + optics_si = get_optics_si_settings_value(physical_port, lane_speed, vendor_key, vendor_name) + return optics_si + +def load_optics_si_settings(): + global g_optics_si_dict + (platform_path, _) = device_info.get_paths_to_platform_and_hwsku_dirs() + + optics_si_settings_file_path = os.path.join(platform_path, "optics_si_settings.json") + if not os.path.isfile(optics_si_settings_file_path): + helper_logger.log_info("No optics SI file exists") + return {} + + with open(optics_si_settings_file_path, "r") as optics_si_file: + g_optics_si_dict = json.load(optics_si_file) + +def optics_si_present(): + if g_optics_si_dict: + return True + return False + From 82506ce78aa08d38d0415be370c17b1fa311694a Mon Sep 17 00:00:00 2001 From: vdahiya12 <67608553+vdahiya12@users.noreply.github.com> Date: Thu, 17 Aug 2023 13:35:50 -0700 Subject: [PATCH 11/12] [ycable] Curb log messages in active-active by changing verbosity level; fix missing namespaces in delete event handle (#391) * [ycabled][active-active] Correct the behavior when no active-active cable type Signed-off-by: vaibhav dahiya --- sonic-ycabled/tests/test_y_cable_helper.py | 301 ++++++++++++++++++ .../ycable/ycable_utilities/y_cable_helper.py | 43 +-- 2 files changed, 323 insertions(+), 21 deletions(-) diff --git a/sonic-ycabled/tests/test_y_cable_helper.py b/sonic-ycabled/tests/test_y_cable_helper.py index 84630eb48..4b441fc46 100644 --- a/sonic-ycabled/tests/test_y_cable_helper.py +++ b/sonic-ycabled/tests/test_y_cable_helper.py @@ -1624,6 +1624,7 @@ def test_check_identifier_presence_and_update_mux_table_entry_module_microsoft_y state_db, port_tbl, y_cable_tbl, static_tbl, mux_tbl, asic_index, logical_port_name, y_cable_presence) assert(rc == None) + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) def test_check_identifier_presence_and_delete_mux_table_entry(self): @@ -1660,6 +1661,8 @@ def test_check_identifier_presence_and_delete_mux_table_entry(self): rc = check_identifier_presence_and_delete_mux_table_entry( state_db, port_tbl, asic_index, logical_port_name, y_cable_presence, delete_change_event, y_cable_tbl, static_tbl, mux_tbl) assert(rc == None) + + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_chassis') @patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') @@ -1783,6 +1786,107 @@ def mock_get_asic_id(mock_logical_port_name): assert(rc == None) + + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch('ycable.ycable_utilities.y_cable_helper.check_identifier_presence_and_setup_channel', MagicMock(return_value=(None))) + @patch('ycable.ycable_utilities.y_cable_helper.process_loopback_interface_and_get_read_side',MagicMock(return_value=0)) + @patch('swsscommon.swsscommon.Table') + def test_change_ports_status_for_y_cable_change_event_sfp_removed_with_false(self, mock_swsscommon_table): + + mock_logical_port_name = [""] + + def mock_get_asic_id(mock_logical_port_name): + return 0 + + y_cable_presence = [True] + logical_port_dict = {'Ethernet0': '0'} + state_db = {} + + mock_table = MagicMock() + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + mock_table.get = MagicMock( + side_effect=[(True, (('index', 1), )), (True, (('index', 2), ))]) + mock_swsscommon_table.return_value = mock_table + + fvs = [('state', "auto"), ('read_side', 1)] + test_db = "TEST_DB" + port_tbl = {} + asic_index = 0 + status = True + port_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "PORT_INFO_TABLE") + port_tbl[asic_index].get.return_value = (status, fvs) + + port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl = {}, {}, {}, {}, {}, {}, {}, {}, {}, {} + port_table_keys[0] = ['Ethernet0'] + + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') as patched_util: + + patched_util.get_asic_id_for_logical_port.return_value = 0 + rc = change_ports_status_for_y_cable_change_event( + logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()) + + assert(rc == None) + + + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) + @patch('ycable.ycable_utilities.y_cable_helper.check_mux_cable_port_type', MagicMock(return_value=(True,"active-active"))) + @patch('ycable.ycable_utilities.y_cable_helper.check_identifier_presence_and_setup_channel', MagicMock(return_value=(None))) + @patch('ycable.ycable_utilities.y_cable_helper.process_loopback_interface_and_get_read_side',MagicMock(return_value=0)) + @patch('swsscommon.swsscommon.Table') + def test_change_ports_status_for_y_cable_change_event_sfp_removed_with_removal(self, mock_swsscommon_table): + + mock_logical_port_name = [""] + + def mock_get_asic_id(mock_logical_port_name): + return 0 + + y_cable_presence = [True] + logical_port_dict = {'Ethernet0': '0'} + state_db = {} + + mock_table = MagicMock() + mock_table.getKeys = MagicMock(return_value=['Ethernet0', 'Ethernet4']) + mock_table.get = MagicMock( + side_effect=[(True, (('index', 1), ('state', "auto"), ('read_side', 1))), (True, (('index', 2), ('state', "auto"), ('read_side', 1)))]) + mock_swsscommon_table.return_value = mock_table + + fvs = [('state', "auto"), ('read_side', 1)] + test_db = "TEST_DB" + port_tbl ,y_cable_tbl = {}, {} + asic_index = 0 + status = True + port_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "PORT_INFO_TABLE") + port_tbl[asic_index].get.return_value = (status, fvs) + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "PORT_INFO_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + + port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl = {}, {}, {}, {}, {}, {}, {}, {}, {} + port_table_keys[0] = ['Ethernet0'] + + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') as patched_util: + + patched_util.get_asic_id_for_logical_port.return_value = 0 + rc = change_ports_status_for_y_cable_change_event( + logical_port_dict, y_cable_presence, port_tbl, port_table_keys, loopback_tbl, loopback_keys, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, y_cable_tbl, static_tbl, mux_tbl, grpc_client, fwd_state_response_tbl, state_db, stop_event=threading.Event()) + + assert(rc == None) + + + + + + + + + + + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) @patch('ycable.ycable_utilities.y_cable_helper.process_loopback_interface_and_get_read_side',MagicMock(return_value=0)) @@ -1891,6 +1995,43 @@ def test_check_identifier_presence_and_update_mux_info_entry(self): assert(rc == None) + + def test_check_identifier_presence_and_update_mux_info_entry_with_false(self): + asic_index = 0 + logical_port_name = "Ethernet0" + + state_db = {} + test_db = "TEST_DB" + status = False + mux_tbl = {} + y_cable_tbl = {} + static_tbl = {} + fvs = [('state', "auto"), ('read_side', 1)] + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + static_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "STATIC_TABLE") + static_tbl[asic_index].get.return_value = (status, fvs) + + + mux_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], MUX_CABLE_INFO_TABLE) + + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') as patched_util: + + patched_util.logical.return_value = ['Ethernet0', 'Ethernet4'] + rc = check_identifier_presence_and_update_mux_info_entry( + state_db, mux_tbl, asic_index, logical_port_name, y_cable_tbl, static_tbl) + + assert(rc == None) + + + + + + + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_instances') @patch('swsscommon.swsscommon.Table') def test_get_firmware_dict(self, port_instance, mock_swsscommon_table): @@ -2119,6 +2260,81 @@ def get_nic_temperature(self): assert(rc['mux_direction'] == 'self') assert(rc['internal_voltage'] == 0.5) + + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') + def test_get_muxcable_info_with_false(self, platform_sfputil): + physical_port = 20 + + logical_port_name = "Ethernet20" + swsscommon.Table.return_value.get.return_value = ( + False, {"read_side": "1"}) + platform_sfputil.get_asic_id_for_logical_port = 0 + asic_index = 0 + y_cable_tbl = {} + mux_tbl = {} + test_db = "TEST_DB" + status = False + fvs = [('state', "auto"), ('read_side', 1)] + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_instances') as patched_util: + + class PortInstanceHelper(): + def __init__(self): + self.EEPROM_ERROR = -1 + self.TARGET_NIC = 1 + self.TARGET_TOR_A = 1 + self.TARGET_TOR_B = 1 + self.FIRMWARE_DOWNLOAD_STATUS_INPROGRESS = 1 + self.FIRMWARE_DOWNLOAD_STATUS_FAILED = 2 + self.download_firmware_status = 0 + self.MUX_TOGGLE_STATUS_INPROGRESS = 1 + self.MUX_TOGGLE_STATUS_FAILED = 2 + self.MUX_TOGGLE_STATUS_NOT_INITIATED_OR_FINISHED = 2 + self.mux_toggle_status = 0 + self.SWITCH_COUNT_MANUAL = "manual" + self.SWITCH_COUNT_AUTO = "auto" + + def get_active_linked_tor_side(self): + return 1 + + def get_mux_direction(self): + return 1 + + def get_switch_count_total(self, switch_count): + return 1 + + def get_eye_heights(self, tgt_tor): + return 500 + + def is_link_active(self, tgt_nic): + return True + + def get_local_temperature(self): + return 22.75 + + def get_local_voltage(self): + return 0.5 + + def get_nic_voltage(self): + return 2.7 + + def get_nic_temperature(self): + return 20 + + patched_util.get.return_value = PortInstanceHelper() + + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') as patched_util: + patched_util.get_asic_id_for_logical_port.return_value = 0 + + rc = get_muxcable_info(physical_port, logical_port_name, mux_tbl, asic_index, y_cable_tbl) + + assert(rc == -1) + + @patch('ycable.ycable_utilities.y_cable_helper.y_cable_port_locks', MagicMock(return_value=[0])) @patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') def test_get_muxcable_info_peer_side(self, platform_sfputil): @@ -6402,6 +6618,43 @@ def test_check_identifier_presence_and_setup_channel(self): assert(rc == None) + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) + def test_check_identifier_presence_and_setup_channel_with_false_status(self): + + status = False + fvs = [('state', "auto"), ('read_side', 1), ('cable_type','active-active'), ('soc_ipv4','192.168.0.1')] + + state_db = {} + test_db = "TEST_DB" + y_cable_tbl = {} + static_tbl = {} + mux_tbl = {} + hw_mux_cable_tbl = {} + hw_mux_cable_tbl_peer = {} + port_tbl = {} + read_side = 0 + asic_index = 0 + y_cable_presence = [True] + delete_change_event = [True] + swsscommon.Table.return_value.get.return_value = ( + False, {"config": "1"}) + + port_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "PORT_INFO_TABLE") + port_tbl[asic_index].get.return_value = (status, fvs) + hw_mux_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "HW_TABLE1") + hw_mux_cable_tbl_peer[asic_index] = swsscommon.Table( + test_db[asic_index], "HW_TABLE2") + grpc_client , fwd_state_response_tbl = {}, {} + mux_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "MUX_INFO_TABLE") + + rc = check_identifier_presence_and_setup_channel("Ethernet0", port_tbl, hw_mux_cable_tbl, hw_mux_cable_tbl_peer, asic_index, read_side, mux_tbl, y_cable_presence, grpc_client, fwd_state_response_tbl) + + assert(rc == None) + + @patch('ycable.ycable_utilities.y_cable_helper.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) @patch('ycable.ycable_utilities.y_cable_helper.setup_grpc_channel_for_port', MagicMock(return_value=(None, None))) def test_check_identifier_presence_and_setup_channel_with_mock(self): @@ -6501,6 +6754,33 @@ def test_setup_grpc_channel_for_port(self): assert(stub == True) assert(channel != None) + @patch('proto_out.linkmgr_grpc_driver_pb2_grpc.DualToRActiveStub', MagicMock(return_value=True)) + def test_setup_grpc_channel_for_port_get_false(self): + + status = False + fvs = [('state', "auto"), ('read_side', 1), ('cable_type','active-standby'), ('soc_ipv4','192.168.0.1')] + grpc_client , fwd_state_response_tbl = {}, {} + asic_index = 0 + Table = MagicMock() + Table.get.return_value = (status, fvs) + #swsscommon.Table.return_value.get.return_value = ( + # True, { 'config', {'type': 'secure'}}) + swsscommon.Table.return_value.get.return_value = ( + False, {"config": "1"}) + test_db = "TEST_DB" + asic_index = 0 + grpc_client[asic_index] = swsscommon.Table( + test_db[asic_index], "PORT_INFO_TABLE") + with patch('ycable.ycable_utilities.y_cable_helper.y_cable_platform_sfputil') as patched_util: + + patched_util.get_asic_id_for_logical_port.return_value = 0 + (channel, stub) = setup_grpc_channel_for_port("Ethernet0", "192.168.0.1", asic_index, grpc_client, fwd_state_response_tbl, False) + + assert(stub == True) + assert(channel != None) + + + def test_connect_channel(self): with patch('grpc.channel_ready_future') as patched_util: @@ -7126,6 +7406,27 @@ def test_get_muxcable_info_for_active_active(self): assert(rc['mux_direction_probe_count'] == 'unknown') assert(rc['peer_mux_direction_probe_count'] == 'unknown') + def test_get_muxcable_info_for_active_active_with_false(self): + physical_port = 20 + + logical_port_name = "Ethernet20" + swsscommon.Table.return_value.get.return_value = ( + False, {"read_side": "1"}) + asic_index = 0 + y_cable_tbl = {} + mux_tbl = {} + test_db = "TEST_DB" + status = True + fvs = [('state', "auto"), ('read_side', 1)] + y_cable_tbl[asic_index] = swsscommon.Table( + test_db[asic_index], "Y_CABLE_TABLE") + y_cable_tbl[asic_index].get.return_value = (status, fvs) + + rc = get_muxcable_info_for_active_active(physical_port, logical_port_name, mux_tbl, asic_index, y_cable_tbl) + + assert(rc != None) + + @patch("grpc.aio.secure_channel") diff --git a/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py b/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py index f7a916bab..3fd1c6786 100644 --- a/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py +++ b/sonic-ycabled/ycable/ycable_utilities/y_cable_helper.py @@ -296,7 +296,7 @@ def check_mux_cable_port_type(logical_port_name, port_tbl, asic_index): (status, fvs) = port_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) return (False, None) @@ -353,7 +353,7 @@ def retry_setup_grpc_channel_for_port(port, asic_index, port_tbl, grpc_client, f (status, fvs) = port_tbl[asic_index].get(port) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(port, port_tbl[asic_index].getTableName())) return False @@ -551,7 +551,7 @@ def setup_grpc_channel_for_port(port, soc_ip, asic_index, grpc_config, fwd_state (status, fvs) = grpc_config[asic_index].get("config") if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table kvp config for {} for setting up channel type".format(port, grpc_config[asic_index].getTableName())) else: grpc_config_dict = dict(fvs) @@ -563,7 +563,7 @@ def setup_grpc_channel_for_port(port, soc_ip, asic_index, grpc_config, fwd_state if type_chan == "secure": (status, fvs) = grpc_config[asic_index].get("certs") if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table kvp certs for {} for setting up channel type".format(port, grpc_config[asic_index].getTableName())) #if type is secure, must have certs defined return (None, None) @@ -647,7 +647,7 @@ def check_identifier_presence_and_setup_channel(logical_port_name, port_tbl, hw_ (status, fvs) = port_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) return @@ -1160,7 +1160,7 @@ def check_identifier_presence_and_update_mux_table_entry(state_db, port_tbl, y_c global y_cable_port_locks (status, fvs) = port_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) return @@ -1302,7 +1302,7 @@ def check_identifier_presence_and_delete_mux_table_entry(state_db, port_tbl, asi (status, fvs) = port_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) return @@ -1315,7 +1315,7 @@ def check_identifier_presence_and_delete_mux_table_entry(state_db, port_tbl, asi #We dont delete the values here, rather just update the values in state DB (status, fvs) = y_cable_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {} while deleting mux entry".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {} while deleting mux entry".format( logical_port_name, y_cable_tbl[asic_index].getTableName())) mux_port_dict = dict(fvs) read_side = mux_port_dict.get("read_side", None) @@ -1444,8 +1444,9 @@ def change_ports_status_for_y_cable_change_event(port_dict, y_cable_presence, po if y_cable_presence[0] is True and delete_change_event[0] is True: y_cable_presence[:] = [False] - state_db = {}, + state_db = {} yc_hw_mux_cable_table = {} + namespaces = multi_asic.get_front_end_namespaces() for namespace in namespaces: asic_id = multi_asic.get_asic_index_from_namespace( namespace) @@ -1515,7 +1516,7 @@ def check_identifier_presence_and_update_mux_info_entry(state_db, mux_tbl, asic_ (cable_status, cable_type) = check_mux_cable_port_type(logical_port_name, port_tbl, asic_index) if status is False: - helper_logger.log_info("Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, port_tbl[asic_index].getTableName())) return elif cable_status is True: @@ -1555,7 +1556,7 @@ def get_firmware_dict(physical_port, port_instance, target, side, mux_info_dict, (status, fvs) = mux_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, mux_tbl[asic_index].getTableName())) + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, mux_tbl[asic_index].getTableName())) mux_info_dict[("version_{}_active".format(side))] = "N/A" mux_info_dict[("version_{}_inactive".format(side))] = "N/A" mux_info_dict[("version_{}_next".format(side))] = "N/A" @@ -1695,7 +1696,7 @@ def get_muxcable_info_for_active_active(physical_port, port, mux_tbl, asic_index (status, fvs) = y_cable_tbl[asic_index].get(port) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, y_cable_tbl[asic_index].getTableName())) + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, y_cable_tbl[asic_index].getTableName())) return -1 mux_port_dict = dict(fvs) @@ -1830,7 +1831,7 @@ def get_muxcable_info(physical_port, logical_port_name, mux_tbl, asic_index, y_c (status, fvs) = y_cable_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, y_cable_tbl[asic_index].getTableName())) + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format(logical_port_name, y_cable_tbl[asic_index].getTableName())) return -1 mux_port_dict = dict(fvs) @@ -2078,7 +2079,7 @@ def get_muxcable_static_info(physical_port, logical_port_name, y_cable_tbl): (status, fvs) = y_cable_tbl[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( logical_port_name, y_cable_tbl[asic_index].getTableName())) return -1 @@ -2183,7 +2184,7 @@ def post_port_mux_info_to_db(logical_port_name, mux_tbl, asic_index, y_cable_tbl if not y_cable_wrapper_get_presence(physical_port) or cable_type == 'pseudo-cable': mux_info_dict = get_muxcable_info_without_presence() elif cable_type == 'active-active': - helper_logger.log_warning("Error: trying to post mux info without presence of port {}".format(logical_port_name)) + helper_logger.log_debug("Error: trying to post mux info without presence of port {}".format(logical_port_name)) mux_info_dict = get_muxcable_info_for_active_active(physical_port, logical_port_name, mux_tbl, asic_index, y_cable_tbl) if mux_info_dict is not None and mux_info_dict != -1: fvs = swsscommon.FieldValuePairs( @@ -3265,7 +3266,7 @@ def handle_show_hwmode_state_cmd_arg_tbl_notification(fvp, port_tbl, xcvrd_show_ (status, fv) = hw_mux_cable_tbl[asic_index].get(port) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table while responding to cli cmd show mux status {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table while responding to cli cmd show mux status {}".format( port, hw_mux_cable_tbl[asic_index].getTableName())) set_result_and_delete_port('state', 'unknown', xcvrd_show_hwmode_dir_cmd_sts_tbl[asic_index], xcvrd_show_hwmode_dir_rsp_tbl[asic_index], port) return -1 @@ -3397,7 +3398,7 @@ def handle_fwd_state_command_grpc_notification(fvp_m, hw_mux_cable_tbl, fwd_stat helper_logger.log_debug("Y_CABLE_DEBUG:processing the notification fwd_state port {}".format(port)) (status, fv) = hw_mux_cable_tbl[asic_index].get(port) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( port, hw_mux_cable_tbl[asic_index].getTableName())) return False mux_port_dict = dict(fv) @@ -3469,7 +3470,7 @@ def handle_hw_mux_cable_table_grpc_notification(fvp, hw_mux_cable_tbl, asic_inde (status, fvs) = hw_mux_cable_tbl[asic_index].get(port) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( port, hw_mux_cable_tbl[asic_index].getTableName())) return helper_logger.log_debug("Y_CABLE_DEBUG processing the notification mux hw state port {}".format(port)) @@ -3559,7 +3560,7 @@ def handle_ycable_active_standby_probe_notification(cable_type, fvp_dict, appl_d (status, fv) = hw_mux_cable_tbl[asic_index].get(port_m) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( port_m, hw_mux_cable_tbl[asic_index].getTableName())) return False @@ -3683,7 +3684,7 @@ def task_worker(self): requested_status = new_status (status, fvs) = self.table_helper.get_hw_mux_cable_tbl()[asic_index].get(port) if status is False: - helper_logger.log_warning("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( + helper_logger.log_debug("Could not retreive fieldvalue pairs for {}, inside state_db table {}".format( port, self.table_helper.get_hw_mux_cable_tbl()[asic_index].getTableName())) continue mux_port_dict = dict(fvs) @@ -4093,7 +4094,7 @@ async def task_worker(self): asic_index = y_cable_platform_sfputil.get_asic_id_for_logical_port(logical_port_name) (status, fvs) = self.table_helper.get_port_tbl()[asic_index].get(logical_port_name) if status is False: - helper_logger.log_warning( + helper_logger.log_debug( "Could not retreive fieldvalue pairs for {}, inside config_db table {}".format(logical_port_name, self.table_helper.get_port_tbl()[asic_index].getTableName())) continue From c1c43f686ce29c005de8e7e87085c440bc7af998 Mon Sep 17 00:00:00 2001 From: vganesan-nokia <67648637+vganesan-nokia@users.noreply.github.com> Date: Fri, 1 Sep 2023 01:06:42 -0400 Subject: [PATCH 12/12] [pmon][chassis][voq] Chassis DB cleanup when module is down (#394) Description Following changes are done in this PR - In line card pmon:chassisd: - line card's hostname and number of asics is pushed to a new table CHASSIS_MODULE_TABLE in chassis state db in redis_chassis server. The hostname and number of asics are required to clean up chassis add db. 'hostname' and asic name are used to construct key for chassis app db entries. - In supervisor pomon:chassisd: - When midplane connectivity loss is deteced, error syslog is generated. No chassis app db clean up is done for midplane connectivity loss. - When a module goes down and if it is in down state for more than 30 minutes, chassis app db clean up is done for all the asics of the module that went down. As part of the clean up entries created by all the asics of the down module are deleted from the following tables in chassis app db in redis_chassis server in supervisor. (1) SYSTEM_NEIGH (2) SYSTEM_INTERFACE (3) SYSTEM_LAG_MEMBER_TABLE (4) SYSTEM_LAG_TABLE The LAG IDs used by the asics of the down module are also de-allocated from SYSTEM_LAG_ID_TABLE and SYSTEM_LAG_ID_SET. Motivation and Context In an operational system, if a line card is brought down the entries created by the down line card are still present in the chassis db and hence the corresponding voq system entries (such as system interface, systen neighbor and so on ) in all other line cards. These stale entries may affect the accuracy of the current entries. To fix this, we cleanup the chassis db for all asics of a given line card that is detected to be down for a long period of time. How Has This Been Tested? After the chassis is up with a line card (1) Pull out the line card. Notice the error log message indicating line card down. Re-insert the line card/Bring up the line card within 30 minutes and observe that the chassis db entries created by this line card are not cleaned. (2) Pull out the line card. Notice the erroe log message indicating line card down. After more than 30 minutes, observe that the chassis db does not have entries created by the removed line card. --- sonic-chassisd/scripts/chassisd | 152 ++++++++++++++++++- sonic-chassisd/tests/mock_swsscommon.py | 19 ++- sonic-chassisd/tests/test_chassis_db_init.py | 2 + sonic-chassisd/tests/test_chassisd.py | 116 +++++++++++++- 4 files changed, 283 insertions(+), 6 deletions(-) diff --git a/sonic-chassisd/scripts/chassisd b/sonic-chassisd/scripts/chassisd index 64206297b..807fec7be 100644 --- a/sonic-chassisd/scripts/chassisd +++ b/sonic-chassisd/scripts/chassisd @@ -9,11 +9,14 @@ try: import os + import re import signal + import subprocess import sys import threading + import time - from sonic_py_common import daemon_base, logger + from sonic_py_common import daemon_base, logger, device_info from sonic_py_common.task_base import ProcessTaskBase # If unit testing is occurring, mock swsscommon and module_base @@ -63,7 +66,11 @@ CHASSIS_MIDPLANE_INFO_NAME_FIELD = 'name' CHASSIS_MIDPLANE_INFO_IP_FIELD = 'ip_address' CHASSIS_MIDPLANE_INFO_ACCESS_FIELD = 'access' +CHASSIS_MODULE_HOSTNAME_TABLE = 'CHASSIS_MODULE_TABLE' +CHASSIS_MODULE_INFO_HOSTNAME_FIELD = 'hostname' + CHASSIS_INFO_UPDATE_PERIOD_SECS = 10 +CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD = 30 # Minutes CHASSIS_LOAD_ERROR = 1 CHASSIS_NOT_SUPPORTED = 2 @@ -189,7 +196,11 @@ class ModuleUpdater(logger.Logger): else: self.asic_table = swsscommon.Table(self.chassis_state_db, CHASSIS_ASIC_INFO_TABLE) -# + + self.hostname_table = swsscommon.Table(self.chassis_state_db, CHASSIS_MODULE_HOSTNAME_TABLE) + self.down_modules = {} + self.chassis_app_db_clean_sha = None + self.midplane_initialized = try_get(chassis.init_midplane_switch, default=False) if not self.midplane_initialized: self.log_error("Chassisd midplane intialization failed") @@ -251,9 +262,33 @@ class ModuleUpdater(logger.Logger): (CHASSIS_MODULE_INFO_SERIAL_FIELD, module_info_dict[CHASSIS_MODULE_INFO_SERIAL_FIELD])]) self.module_table.set(key, fvs) + # Construct key for down_modules dict. Example down_modules key format: LINE-CARD0| + fvs = self.hostname_table.get(key) + if isinstance(fvs, list) and fvs[0] is True: + fvs = dict(fvs[-1]) + hostname = fvs[CHASSIS_MODULE_INFO_HOSTNAME_FIELD] + down_module_key = key+'|'+hostname + else: + down_module_key = key+'|' + if module_info_dict[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] != str(ModuleBase.MODULE_STATUS_ONLINE): notOnlineModules.append(key) + # Record the time when the module down was detected to track the + # module down time. Used for chassis db cleanup for all asics of the module if the module is down for a + # long time like 30 mins. + # All down modules including supervisor are added to the down modules dictionary. This is to help + # identifying module operational status change. But the clean up will not be attempted for supervisor + if down_module_key not in self.down_modules: + self.log_warning("Module {} went off-line!".format(key)) + self.down_modules[down_module_key] = {} + self.down_modules[down_module_key]['down_time'] = time.time() + self.down_modules[down_module_key]['cleaned'] = False continue + else: + # Module is operational. Remove it from down time tracking. + if down_module_key in self.down_modules: + self.log_notice("Module {} recovered on-line!".format(key)) + del self.down_modules[down_module_key] for asic_id, asic in enumerate(module_info_dict[CHASSIS_MODULE_INFO_ASICS]): asic_global_id, asic_pci_addr = asic @@ -266,6 +301,16 @@ class ModuleUpdater(logger.Logger): (CHASSIS_ASIC_ID_IN_MODULE_FIELD, str(asic_id))]) self.asic_table.set(asic_key, asic_fvs) + # In line card push the hostname of the module and num_asics to the chassis state db. + # The hostname is used as key to access chassis app db entries + if not self._is_supervisor(): + hostname_key = "{}{}".format(ModuleBase.MODULE_TYPE_LINE, int(self.my_slot) - 1) + hostname = try_get(device_info.get_hostname, default="None") + hostname_fvs = swsscommon.FieldValuePairs([(CHASSIS_MODULE_INFO_SLOT_FIELD, self.my_slot), + (CHASSIS_MODULE_INFO_HOSTNAME_FIELD, hostname), + (CHASSIS_MODULE_INFO_NUM_ASICS_FIELD, str(len(module_info_dict[CHASSIS_MODULE_INFO_ASICS])))]) + self.hostname_table.set(hostname_key, hostname_fvs) + # Asics that are on the "not online" modules need to be cleaned up asics = list(self.asic_table.getKeys()) for asic in asics: @@ -329,11 +374,113 @@ class ModuleUpdater(logger.Logger): midplane_ip = try_get(module.get_midplane_ip, default=INVALID_IP) midplane_access = try_get(module.is_midplane_reachable, default=False) + # Generate syslog for the loss of midplane connectivity when midplane connectivity + # loss is detected for the first time + current_midplane_state = 'False' + fvs = self.midplane_table.get(module_key) + if isinstance(fvs, list) and fvs[0] is True: + fvs = dict(fvs[-1]) + current_midplane_state = fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] + + if midplane_access is False and current_midplane_state == 'True': + self.log_warning("Module {} lost midplane connectivity".format(module_key)) + elif midplane_access is True and current_midplane_state == 'False': + self.log_notice("Module {} midplane connectivity is up".format(module_key)) + # Update db with midplane information fvs = swsscommon.FieldValuePairs([(CHASSIS_MIDPLANE_INFO_IP_FIELD, midplane_ip), (CHASSIS_MIDPLANE_INFO_ACCESS_FIELD, str(midplane_access))]) self.midplane_table.set(module_key, fvs) + def _cleanup_chassis_app_db(self, module_host): + + if self.chassis_app_db_clean_sha is None: + self.chassis_app_db = daemon_base.db_connect("CHASSIS_APP_DB") + self.chassis_app_db_pipe = swsscommon.RedisPipeline(self.chassis_app_db) + + # Lua script for chassis db cleanup for a specific asic + # The clean up operation is required to delete only those entries created by + # the asic that lost connection. Entries from the following tables are deleted + # (1) SYSTEM_NEIGH + # (2) SYSTEM_INTERFACE + # (3) SYSTEM_LAG_MEMBER_TABLE + # (4) SYSTEM_LAG_TABLE + # (5) The corresponding LAG IDs of the entries from SYSTEM_LAG_TABLE + # SYSTEM_LAG_ID_TABLE and SYSTEM_LAG_ID_SET are adjusted appropriately + + script = "local host = string.gsub(ARGV[1], '%-', '%%-')\n\ + local dev = ARGV[2]\n\ + local tables = {'SYSTEM_NEIGH*', 'SYSTEM_INTERFACE*', 'SYSTEM_LAG_MEMBER_TABLE*'}\n\ + for i = 1, table.getn(tables) do\n\ + local ps = tables[i] .. '|' .. host .. '|' .. dev\n\ + local keylist = redis.call('KEYS', tables[i])\n\ + for j,key in ipairs(keylist) do\n\ + if string.match(key, ps) ~= nil then\n\ + redis.call('DEL', key)\n\ + end\n\ + end\n\ + end\n\ + local ps = 'SYSTEM_LAG_TABLE*|' .. '(' .. host .. '|' .. dev ..'.*' .. ')'\n\ + local keylist = redis.call('KEYS', 'SYSTEM_LAG_TABLE*')\n\ + for j,key in ipairs(keylist) do\n\ + local lagname = string.match(key, ps)\n\ + if lagname ~= nil then\n\ + redis.call('DEL', key)\n\ + local lagid = redis.call('HGET', 'SYSTEM_LAG_ID_TABLE', lagname)\n\ + redis.call('SREM', 'SYSTEM_LAG_ID_SET', lagid)\n\ + redis.call('HDEL', 'SYSTEM_LAG_ID_TABLE', lagname)\n\ + end\n\ + end\n\ + return" + self.chassis_app_db_clean_sha = self.chassis_app_db_pipe.loadRedisScript(script) + + # Chassis app db cleanup of all asics of the module + + # Get the module key and host name from down_modules key + module, lc = re.split('\|', module_host) + + if lc == '': + # Host name is not available for this module. No clean up is needed + self.log_notice("Host name is not available for Module {}. Chassis db clean up not done!".format(module)) + return + + # Get number of asics in the module + fvs = self.hostname_table.get(module) + if isinstance(fvs, list) and fvs[0] is True: + fvs = dict(fvs[-1]) + num_asics = int(fvs[CHASSIS_MODULE_INFO_NUM_ASICS_FIELD]) + else: + num_asics = 0 + + for asic_id in range(0, num_asics): + asic = CHASSIS_ASIC+str(asic_id) + + # Cleanup the chassis app db entries using lua script + redis_cmd = 'redis-cli -h redis_chassis.server -p 6380 -n 12 EVALSHA ' + self.chassis_app_db_clean_sha + ' 0 ' + lc + ' ' + asic + try: + subp = subprocess.Popen(redis_cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + subp.communicate() + self.log_notice("Cleaned up chassis app db entries for {}({})/{}".format(module, lc, asic)) + except Exception: + self.log_error("Failed to clean up chassis app db entries for {}({})/{}".format(module, lc, asic)) + + + def module_down_chassis_db_cleanup(self): + if self._is_supervisor() == False: + return + time_now = time.time() + for module in self.down_modules: + if self.down_modules[module]['cleaned'] == False: + down_time = self.down_modules[module]['down_time'] + delta = (time_now - down_time) / 60 + if delta >= CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD: + if module.startswith(ModuleBase.MODULE_TYPE_LINE): + # Module is down for more than 30 minutes. Do the chassis clean up + self.log_notice("Module {} is down for long time. Initiating chassis app db clean up".format(module)) + self._cleanup_chassis_app_db(module) + self.down_modules[module]['cleaned'] = True + + # # Config Manager task ======================================================== # @@ -449,6 +596,7 @@ class ChassisdDaemon(daemon_base.DaemonBase): while not self.stop.wait(CHASSIS_INFO_UPDATE_PERIOD_SECS): self.module_updater.module_db_update() self.module_updater.check_midplane_reachability() + self.module_updater.module_down_chassis_db_cleanup() self.log_info("Stop daemon main loop") diff --git a/sonic-chassisd/tests/mock_swsscommon.py b/sonic-chassisd/tests/mock_swsscommon.py index 2da88e1e4..3922a0a93 100644 --- a/sonic-chassisd/tests/mock_swsscommon.py +++ b/sonic-chassisd/tests/mock_swsscommon.py @@ -2,8 +2,9 @@ class Table: - def __init__(self, db, table_name): - self.table_name = table_name + def __init__(self, *argv): + self.db_or_pipe = argv[0] + self.table_name = argv[1] self.mock_dict = {} def _del(self, key): @@ -17,7 +18,10 @@ def set(self, key, fvs): def get(self, key): if key in self.mock_dict: - return self.mock_dict[key] + rv = [] + rv.append(True) + rv.append(tuple(self.mock_dict[key].items())) + return rv return None def getKeys(self): @@ -45,3 +49,12 @@ def select(self, timeout=-1, interrupt_on_signal=False): class SubscriberStateTable(Table): pass + +class RedisPipeline: + def __init__(self, db): + self.db = db + + def loadRedisScript(self, script): + self.script = script + self.script_mock_sha = 'd79033d1cab85249929e8c069f6784474d71cc43' + return self.script_mock_sha diff --git a/sonic-chassisd/tests/test_chassis_db_init.py b/sonic-chassisd/tests/test_chassis_db_init.py index e9a9560ba..e0ce2caab 100644 --- a/sonic-chassisd/tests/test_chassis_db_init.py +++ b/sonic-chassisd/tests/test_chassis_db_init.py @@ -33,6 +33,8 @@ def test_provision_db(): chassis_table = provision_db(chassis, log) fvs = chassis_table.get(CHASSIS_INFO_KEY_TEMPLATE.format(1)) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert serial == fvs[CHASSIS_INFO_SERIAL_FIELD] assert model == fvs[CHASSIS_INFO_MODEL_FIELD] assert revision == fvs[CHASSIS_INFO_REV_FIELD] diff --git a/sonic-chassisd/tests/test_chassisd.py b/sonic-chassisd/tests/test_chassisd.py index 5b073af50..d61bf7c65 100644 --- a/sonic-chassisd/tests/test_chassisd.py +++ b/sonic-chassisd/tests/test_chassisd.py @@ -70,6 +70,8 @@ def test_moduleupdater_check_valid_fields(): module.supervisor_slot) module_updater.module_db_update() fvs = module_updater.module_table.get(name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert desc == fvs[CHASSIS_MODULE_INFO_DESC_FIELD] assert slot == int(fvs[CHASSIS_MODULE_INFO_SLOT_FIELD]) assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] @@ -118,6 +120,8 @@ def test_moduleupdater_check_status_update(): module.supervisor_slot) module_updater.module_db_update() fvs = module_updater.module_table.get(name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) print('Initial DB-entry {}'.format(fvs)) assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] @@ -125,15 +129,21 @@ def test_moduleupdater_check_status_update(): status = ModuleBase.MODULE_STATUS_OFFLINE module.set_oper_status(status) fvs = module_updater.module_table.get(name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) print('Not updated DB-entry {}'.format(fvs)) assert status != fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] # Update status and db module_updater.module_db_update() fvs = module_updater.module_table.get(name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) print('Updated DB-entry {}'.format(fvs)) assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] + # Run chassis db clean up from LC. + module_updater.module_down_chassis_db_cleanup() def test_moduleupdater_check_deinit(): chassis = MockChassis() @@ -155,6 +165,8 @@ def test_moduleupdater_check_deinit(): module_updater.modules_num_update() module_updater.module_db_update() fvs = module_updater.module_table.get(name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] module_table = module_updater.module_table @@ -255,6 +267,8 @@ def test_configupdater_check_num_modules(): chassis.module_list.append(module) module_updater.modules_num_update() fvs = module_updater.chassis_table.get(CHASSIS_INFO_KEY_TEMPLATE.format(1)) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert chassis.get_num_modules() == int(fvs[CHASSIS_INFO_CARD_NUM_FIELD]) module_updater.deinit() @@ -313,14 +327,28 @@ def test_midplane_presence_modules(): name = "LINE-CARD0" fvs = midplane_table.get(name) assert fvs != None + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert module.get_midplane_ip() == fvs[CHASSIS_MIDPLANE_INFO_IP_FIELD] assert str(module.is_midplane_reachable()) == fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] - #Set access of line-card to down + #Set access of line-card to Up (midplane connectivity is down initially) + module.set_midplane_reachable(True) + module_updater.check_midplane_reachability() + fvs = midplane_table.get(name) + assert fvs != None + if isinstance(fvs, list): + fvs = dict(fvs[-1]) + assert module.get_midplane_ip() == fvs[CHASSIS_MIDPLANE_INFO_IP_FIELD] + assert str(module.is_midplane_reachable()) == fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] + + #Set access of line-card to Down (to mock midplane connectivity state change) module.set_midplane_reachable(False) module_updater.check_midplane_reachability() fvs = midplane_table.get(name) assert fvs != None + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert module.get_midplane_ip() == fvs[CHASSIS_MIDPLANE_INFO_IP_FIELD] assert str(module.is_midplane_reachable()) == fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] @@ -381,6 +409,8 @@ def test_midplane_presence_supervisor(): name = "SUPERVISOR0" fvs = midplane_table.get(name) assert fvs != None + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert supervisor.get_midplane_ip() == fvs[CHASSIS_MIDPLANE_INFO_IP_FIELD] assert str(supervisor.is_midplane_reachable()) == fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] @@ -389,6 +419,8 @@ def test_midplane_presence_supervisor(): module_updater.check_midplane_reachability() fvs = midplane_table.get(name) assert fvs != None + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert supervisor.get_midplane_ip() == fvs[CHASSIS_MIDPLANE_INFO_IP_FIELD] assert str(supervisor.is_midplane_reachable()) == fvs[CHASSIS_MIDPLANE_INFO_ACCESS_FIELD] @@ -449,6 +481,8 @@ def test_asic_presence(): def verify_fabric_asic(asic_name, asic_pci_address, module_name, asic_id_in_module): fvs = fabric_asic_table.get(asic_name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) assert fvs[CHASSIS_ASIC_PCI_ADDRESS_FIELD] == asic_pci_address assert fvs[CHASSIS_MODULE_INFO_NAME_FIELD] == module_name assert fvs[CHASSIS_ASIC_ID_IN_MODULE_FIELD] == asic_id_in_module @@ -543,3 +577,83 @@ def test_daemon_run_linecard(): with patch.object(sonic_platform.platform.Chassis, 'get_my_slot') as mock: mock.return_value = sonic_platform.platform.Platform().get_chassis().get_supervisor_slot() + 1 daemon_chassisd.run() + +def test_chassis_db_cleanup(): + chassis = MockChassis() + + #Supervisor + index = 0 + sup_name = "SUPERVISOR0" + desc = "Supervisor card" + sup_slot = 16 + serial = "RP1000101" + module_type = ModuleBase.MODULE_TYPE_SUPERVISOR + supervisor = MockModule(index, sup_name, desc, module_type, sup_slot, serial) + supervisor.set_midplane_ip() + chassis.module_list.append(supervisor) + + #Linecard 0. Host name will be pushed for this to make clean up happen + index = 1 + lc_name = "LINE-CARD0" + desc = "36 port 400G card" + lc_slot = 1 + serial = "LC1000101" + module_type = ModuleBase.MODULE_TYPE_LINE + module = MockModule(index, lc_name, desc, module_type, lc_slot, serial) + module.set_midplane_ip() + chassis.module_list.append(module) + + #Linecard 1. Host name will not be pushed for this so that clean up will not happen + index = 2 + lc2_name = "LINE-CARD1" + desc = "36 port 400G card" + lc2_slot = 2 + serial = "LC2000101" + module_type = ModuleBase.MODULE_TYPE_LINE + module2 = MockModule(index, lc2_name, desc, module_type, lc2_slot, serial) + module2.set_midplane_ip() + chassis.module_list.append(module2) + + # Supervisor ModuleUpdater + sup_module_updater = ModuleUpdater(SYSLOG_IDENTIFIER, chassis, sup_slot, sup_slot) + sup_module_updater.modules_num_update() + # Mock hostname table update for the line card LINE-CARD0 + hostname = "lc1-host-00" + num_asics = 1 + hostname_fvs = swsscommon.FieldValuePairs([(CHASSIS_MODULE_INFO_SLOT_FIELD, str(lc_slot)), + (CHASSIS_MODULE_INFO_HOSTNAME_FIELD, hostname), + (CHASSIS_MODULE_INFO_NUM_ASICS_FIELD, str(num_asics))]) + sup_module_updater.hostname_table.set(lc_name, hostname_fvs) + + # Set linecard initial state to ONLINE + status = ModuleBase.MODULE_STATUS_ONLINE + module.set_oper_status(status) + sup_module_updater.module_db_update() + + fvs = sup_module_updater.module_table.get(lc_name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) + assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] + + # Change linecard module status to OFFLINE + status = ModuleBase.MODULE_STATUS_OFFLINE + module.set_oper_status(status) + sup_module_updater.module_db_update() + + fvs = sup_module_updater.module_table.get(lc_name) + if isinstance(fvs, list): + fvs = dict(fvs[-1]) + assert status == fvs[CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] + + # Mock >= CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD module down period for LINE-CARD0 + down_module_key = lc_name+"|"+hostname + module_down_time = sup_module_updater.down_modules[down_module_key]["down_time"] + sup_module_updater.down_modules[down_module_key]["down_time"] = module_down_time - ((CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD+10)*60) + + # Mock >= CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD module down period for LINE-CARD1 + down_module_key = lc2_name+"|" + module_down_time = sup_module_updater.down_modules[down_module_key]["down_time"] + sup_module_updater.down_modules[down_module_key]["down_time"] = module_down_time - ((CHASSIS_DB_CLEANUP_MODULE_DOWN_PERIOD+10)*60) + + # Run module database update from supervisor to run chassis db cleanup + sup_module_updater.module_down_chassis_db_cleanup() \ No newline at end of file