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] 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)