Skip to content

[Smartswitch][pcied] Fix pcied handling for smartswitch during DPU detach #5#633

Merged
judyjoseph merged 3 commits intosonic-net:masterfrom
gpunathilell:fix_pcied
Jul 8, 2025
Merged

[Smartswitch][pcied] Fix pcied handling for smartswitch during DPU detach #5#633
judyjoseph merged 3 commits intosonic-net:masterfrom
gpunathilell:fix_pcied

Conversation

@gpunathilell
Copy link
Copy Markdown
Contributor

@gpunathilell gpunathilell commented Jun 26, 2025

Description

Two changes are being done in this PR:
Remove deletion of the PCIE_DETACH_INFO from the pcied (This should be handled by the power off/power on of the DPU from the module_base implementation)
The returned value from self.detach_info.get(key) in pcied is of the form tuple:
(True, [('bus_info', '0000:03:00.1'), ('dpu_state', 'detaching')]) This has to be handled correctly by the pcied (which was expecting a dictionary

Motivation and Context

This is required since the pcied should not delete the entries from the PCIE_DETACH_INFO table as opposed to the module_base implementation

How Has This Been Tested?

Unit tests:

tests/test_DaemonPcied.py::TestDaemonPcied::test_signal_handler PASSED   [  7%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_run PASSED              [ 14%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_del PASSED              [ 21%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_del_exception PASSED    [ 28%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_is_dpu_in_detaching_mode PASSED [ 35%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_check_pcie_devices PASSED [ 42%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_check_pcie_devices_update_aer PASSED [ 50%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_check_pcie_devices_detaching PASSED [ 57%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_update_pcie_devices_status_db PASSED [ 64%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_check_n_update_pcie_aer_stats PASSED [ 71%]
tests/test_DaemonPcied.py::TestDaemonPcied::test_update_aer_to_statedb PASSED [ 78%]
tests/test_pcied.py::test_main PASSED                                    [ 85%]
tests/test_pcied.py::test_read_id_file PASSED                            [ 92%]
tests/test_pcied.py::test_load_platform_pcieutil PASSED                  [100%]

Also tested manually on smart switch system,
Restarted pcied (supervisorctl restart pcied in pmon docker) and checked that the detach info table is not deleted)
Checked the logs to confirm that pcied is not crashing due to the additional change for processing of detach table

Additional Information (Optional)

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

@vvolam vvolam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vvolam vvolam requested a review from Copilot July 2, 2025 17:37
@vvolam
Copy link
Copy Markdown
Contributor

vvolam commented Jul 2, 2025

@gpunathilell could you update PR description with the testing info.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates how pcied handles DPU detach information by removing manual cleanup of detach entries and adapting to a tuple-based return format from detach_info.get.

  • Drop deletion of PCIE_DETACH_INFO entries in __del__
  • Convert the (exists, field_list) tuple from detach_info.get into a dict for correct field access
  • Update tests to reflect the new tuple format

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/test_DaemonPcied.py Removed assertions on detach_info._del; adjusted mock get() to return tuples
scripts/pcied Deleted cleanup code for detach_info; unpacked tuple return and built dict

@rameshraghupathy
Copy link
Copy Markdown
Contributor

LGTM

@gpunathilell
Copy link
Copy Markdown
Contributor Author

@gpunathilell could you update PR description with the testing info.

Updated

Copy link
Copy Markdown
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@judyjoseph judyjoseph merged commit fa41b9c into sonic-net:master Jul 8, 2025
5 checks passed
@mssonicbld
Copy link
Copy Markdown
Collaborator

Cherry-pick PR to 202505: #641

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants