Skip to content

Commit f3c2631

Browse files
authored
Revert pcied enhancements (sonic-net#392)
* Revert "Fixes for the issues uncovered by sonic-pcied unit tests (sonic-net#389)" This reverts commit 76baca3. * Revert "Added PCIe transaction check for all peripherals on the bus (sonic-net#331)" This reverts commit d73808c. * Fixes for the issues uncovered by sonic-pcied unit tests (sonic-net#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
1 parent 76baca3 commit f3c2631

2 files changed

Lines changed: 7 additions & 184 deletions

File tree

sonic-pcied/scripts/pcied

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import os
99
import signal
1010
import sys
1111
import threading
12-
import subprocess
13-
import yaml
1412

1513
from sonic_py_common import daemon_base, device_info, logger
1614
from swsscommon import swsscommon
@@ -34,7 +32,6 @@ PCIED_MAIN_THREAD_SLEEP_SECS = 60
3432

3533
PCIEUTIL_CONF_FILE_ERROR = 1
3634
PCIEUTIL_LOAD_ERROR = 2
37-
PLATFORM_PCIE_YAML_FILE = "/usr/share/sonic/platform/pcie.yaml"
3835

3936
platform_pcieutil = None
4037

@@ -161,56 +158,6 @@ class DaemonPcied(daemon_base.DaemonBase):
161158
if self.resultInfo is None:
162159
return
163160

164-
# This block reads the PCIE YAML file, iterates through each device and
165-
# for each device runs a transaction check to verify that the device ID
166-
# matches the one on file. In the event of an invalid device ID such as
167-
# '0xffff', it marks the device as missing as that is the only scenario
168-
# in which such a value is returned.
169-
170-
# Default, invalid BDF values to begin
171-
bus = None
172-
device = None
173-
func = None
174-
175-
try:
176-
stream = open(PLATFORM_PCIE_YAML_FILE, 'r')
177-
178-
# Load contents of the PCIe YAML file into a dictionary object
179-
dictionary = yaml.safe_load(stream)
180-
181-
# Iterate through each element in dict; extract the BDF information and device ID for that element
182-
for elem in dictionary:
183-
bus = int(elem.get('bus'), 16)
184-
device = int(elem.get('dev'), 16)
185-
func = int(elem.get('fn'), 16)
186-
pcieDeviceID = elem.get('id')
187-
188-
# Form the PCI device address from the BDF information above
189-
pcie_device = ("{:02x}:{:02x}.{:01d}".format(bus, device, func))
190-
191-
# Form and run the command to read the device ID from the PCI device
192-
cmd = ["setpci", "-s", pcie_device, "2.w"]
193-
output = subprocess.check_output(cmd)
194-
pcieDeviceQueryResult = output.decode('utf8').rstrip()
195-
196-
# verify that the queried device ID matches what we have on file for the current PCI device
197-
# If not, take appropriate action based on the queried value
198-
if pcieDeviceQueryResult is None or "No devices selected" in pcieDeviceQueryResult:
199-
self.log_error("Invalid PCIe Peripheral {:02x}:{:02x}.{:01d} - {}".format(bus, device, func, pcieDeviceQueryResult))
200-
elif pcieDeviceQueryResult != pcieDeviceID:
201-
if pcieDeviceQueryResult == 'ffff':
202-
self.log_error("PCIe device {:02x}:{:02x}.{:01d} missing.".format(bus, device, func))
203-
else:
204-
self.log_error("PCIe device {:02x}:{:02x}.{:01d} ID mismatch. Expected {}, received {}".format(bus, device, func, pcieDeviceID, pcieDeviceQueryResult))
205-
206-
except Exception as ex:
207-
# Verify that none of the BDF objects are None. If condition evals to False, do not mention BDF information in error log
208-
if not [i for i in (bus, device, func) if i is None]:
209-
self.log_error("PCIe Exception occurred for PCIe device {:02x}:{:02x}.{:01d}: {}".format(bus, device, func, ex))
210-
else:
211-
self.log_error("PCIe Exception occurred: {}".format(ex))
212-
pass
213-
214161
for result in self.resultInfo:
215162
if result["result"] == "Failed":
216163
self.log_warning("PCIe Device: " + result["name"] + " Not Found")

sonic-pcied/tests/test_DaemonPcied.py

Lines changed: 7 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -59,23 +59,18 @@
5959

6060
pcie_check_result_no = []
6161

62-
pcie_check_result_pass = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'},
62+
pcie_check_result_pass = \
63+
"""
64+
[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'},
6365
{'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'},
6466
{'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Passed'}]
67+
"""
6568

66-
67-
pcie_check_result_fail = [{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'},
69+
pcie_check_result_fail = \
70+
"""
71+
[{'bus': '00', 'dev': '01', 'fn': '0', 'id': '1f10', 'name': 'PCI A', 'result': 'Passed'},
6872
{'bus': '00', 'dev': '02', 'fn': '0', 'id': '1f11', 'name': 'PCI B', 'result': 'Passed'},
6973
{'bus': '00', 'dev': '03', 'fn': '0', 'id': '1f12', 'name': 'PCI C', 'result': 'Failed'}]
70-
71-
72-
TEST_PLATFORM_PCIE_YAML_FILE = \
73-
"""
74-
- bus: '00'
75-
dev: '01'
76-
fn: '0'
77-
id: '9170'
78-
name: 'PCI A'
7974
"""
8075

8176
class TestDaemonPcied(object):
@@ -148,125 +143,6 @@ def test_run(self):
148143
daemon_pcied.run()
149144
assert daemon_pcied.check_pcie_devices.call_count == 1
150145

151-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
152-
def test_check_pcie_devices_yaml_file_open_error(self):
153-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
154-
pcied.platform_pcieutil.get_pcie_check = mock.MagicMock()
155-
156-
daemon_pcied.check_pcie_devices()
157-
158-
assert pcied.platform_pcieutil.get_pcie_check.called
159-
160-
161-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
162-
def test_check_pcie_devices_result_fail(self):
163-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
164-
pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_fail)
165-
166-
daemon_pcied.check_pcie_devices()
167-
168-
assert pcied.platform_pcieutil.get_pcie_check.called
169-
170-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
171-
def test_check_pcie_devices_result_pass(self):
172-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
173-
pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=pcie_check_result_pass)
174-
175-
daemon_pcied.check_pcie_devices()
176-
177-
assert pcied.platform_pcieutil.get_pcie_check.called
178-
179-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
180-
def test_check_pcie_devices_result_none(self):
181-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
182-
pcied.platform_pcieutil.get_pcie_check = mock.MagicMock(return_value=None)
183-
184-
daemon_pcied.check_pcie_devices()
185-
186-
assert pcied.platform_pcieutil.get_pcie_check.called
187-
188-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
189-
def test_check_pcie_devices_load_yaml_happy(self):
190-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
191-
192-
with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd:
193-
194-
class MockOutput:
195-
def decode(self, encodingType):
196-
return self
197-
def rstrip(self):
198-
return "9170"
199-
200-
mock_output = MockOutput()
201-
with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output:
202-
mock_check_output.return_value = mock_output
203-
204-
daemon_pcied.check_pcie_devices()
205-
206-
assert mock_check_output.called
207-
208-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
209-
def test_check_pcie_devices_load_yaml_mismatch(self):
210-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
211-
212-
with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd:
213-
214-
class MockOutput:
215-
def decode(self, encodingType):
216-
return self
217-
def rstrip(self):
218-
return "0123"
219-
220-
mock_output = MockOutput()
221-
with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output:
222-
mock_check_output.return_value = mock_output
223-
224-
daemon_pcied.check_pcie_devices()
225-
226-
assert mock_check_output.called
227-
228-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
229-
def test_check_pcie_devices_load_yaml_missing_device(self):
230-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
231-
232-
with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd:
233-
234-
class MockOutput:
235-
def decode(self, encodingType):
236-
return self
237-
def rstrip(self):
238-
return "ffff"
239-
240-
mock_output = MockOutput()
241-
with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output:
242-
mock_check_output.return_value = mock_output
243-
244-
daemon_pcied.check_pcie_devices()
245-
246-
assert mock_check_output.called
247-
248-
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
249-
def test_check_pcie_devices_load_yaml_invalid_device(self):
250-
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)
251-
252-
with mock.patch('builtins.open', new_callable=mock.mock_open, read_data=TEST_PLATFORM_PCIE_YAML_FILE) as mock_fd:
253-
254-
class MockOutput:
255-
def decode(self, encodingType):
256-
return self
257-
def rstrip(self):
258-
return "No devices selected"
259-
260-
mock_output = MockOutput()
261-
with mock.patch('subprocess.check_output', mock.MagicMock()) as mock_check_output:
262-
mock_check_output.return_value = mock_output
263-
264-
daemon_pcied.check_pcie_devices()
265-
266-
assert mock_check_output.called
267-
268-
269-
270146
@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
271147
def test_check_pcie_devices(self):
272148
daemon_pcied = pcied.DaemonPcied(SYSLOG_IDENTIFIER)

0 commit comments

Comments
 (0)