Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor fixes for reboot HLD #1847

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

vvolam
Copy link
Contributor

@vvolam vvolam commented Nov 7, 2024

  • Add more details for MODULE_REBOOT_SMARTSWITCH reboot type
  • Update DPU and Switch reboot sequence with PCI detach/reattach() vendor APIs
  • Remove module name from module APIs
  • Remove the tests which were missed removing from first HLD PR.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

No pipelines are associated with this pull request.

Copy link
Contributor

@rameshraghupathy rameshraghupathy left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@prgeor
Copy link
Contributor

prgeor commented Nov 9, 2024

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@vvolam
Copy link
Contributor Author

vvolam commented Nov 9, 2024

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@prgeor Once the system issues the reboot command, it is exiting the context based on my understanding. We can only check the switch state in our test scripts. Please correct me, if I missing any thing here?

@prgeor
Copy link
Contributor

prgeor commented Nov 15, 2024

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@prgeor Once the system issues the reboot command, it is exiting the context based on my understanding. We can only check the switch state in our test scripts. Please correct me, if I missing any thing here?

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@prgeor Once the system issues the reboot command, it is exiting the context based on my understanding. We can only check the switch state in our test scripts. Please correct me, if I missing any thing here?

@vvolam you don't need current context to check but there are different ways in which you can do.

  1. NPU can put a marker for each DPU and check if the marker exist while NPU comes up
  2. NPU can check the uptime of DPU before rebooting and know whether it rebooted

I think we can probably keep this for future

@vvolam
Copy link
Contributor Author

vvolam commented Nov 16, 2024

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@prgeor Once the system issues the reboot command, it is exiting the context based on my understanding. We can only check the switch state in our test scripts. Please correct me, if I missing any thing here?

@vvolam consider a case where NPU was rebooted do we have check in place to detect whether NPU actually rebooted once the NPU comes up?

@prgeor Once the system issues the reboot command, it is exiting the context based on my understanding. We can only check the switch state in our test scripts. Please correct me, if I missing any thing here?

@vvolam you don't need current context to check but there are different ways in which you can do.

  1. NPU can put a marker for each DPU and check if the marker exist while NPU comes up
  2. NPU can check the uptime of DPU before rebooting and know whether it rebooted

I think we can probably keep this for future
@prgeor Sure, I will keep a note on this to enhance the HLD for this. Thank you!

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@vvolam in the Test Plan section there is this rendered text with ### is that intentional?

###Graceful boot/reboot###

@vvolam
Copy link
Contributor Author

vvolam commented Nov 18, 2024

@vvolam in the Test Plan section there is this rendered text with ### is that intentional?

###Graceful boot/reboot###

It was a syntax error. Fixed it. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants