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.

Sign up for GitHub

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

Jump to bottom

[fastboot] Notify SAI that fastboot is done #1396

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

[fastboot] Notify SAI that fastboot is done #1396

wants to merge 7 commits into from
+77 −5

Conversation

Junchao-Mellanox
Copy link
Contributor

Why I did this

Notify SAI that fastboot is done

How I did this

Set SAI_SWITCH_ATTR_FAST_API_ENABLE to false when fastboot is done

How I verify this

Manual test

@Junchao-Mellanox
Copy link
Contributor Author

Hi @yxieca , @vaibhavhd , could you please kindly review it?

kcudnik
kcudnik previously approved these changes Jun 17, 2024
View reviewed changes
@kcudnik
Copy link
Collaborator

kcudnik commented Jun 17, 2024

Please fix errors

@Junchao-Mellanox
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yxieca yxieca requested a review from adyeung June 20, 2024 22:57
@yxieca
Copy link
Contributor

yxieca commented Jun 20, 2024

@adyeung can you get someone from Broadcom to assess this change?

@Junchao-Mellanox Junchao-Mellanox marked this pull request as draft June 24, 2024 03:35
@adyeung
Copy link

adyeung commented Jun 26, 2024 •
edited
Loading

@tushar-ty @vishnushetty please help review

@Junchao-Mellanox
Copy link
Contributor Author

Hi, please hod on the review for a moment. I will need to update this PR.

@Junchao-Mellanox Junchao-Mellanox marked this pull request as ready for review July 4, 2024 07:31
@Junchao-Mellanox
Copy link
Contributor Author

Hi @tushar-ty @vishnushetty , could you please kindly provide comment?

@Junchao-Mellanox
Copy link
Contributor Author

Hi @adyeung, @tushar-ty, @vishnushetty, kindly reminder.

@adyeung
Copy link

adyeung commented Jul 17, 2024

Adding @Prasanth-KV to help review

Prasanth-KV
Prasanth-KV suggested changes Jul 22, 2024
View reviewed changes
syncd/Syncd.cpp Outdated Show resolved Hide resolved
@liat-grozovik
Copy link
Collaborator

@Junchao-Mellanox can you check please the PR checker coverage issue?

@Junchao-Mellanox
Copy link
Contributor Author

@Junchao-Mellanox can you check please the PR checker coverage issue?

Sure, working on it

@Junchao-Mellanox
Copy link
Contributor Author

The new test case passed locally, but failed in github checker, I will investigate.

@Junchao-Mellanox
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Contributor Author

Hi @kcudnik , could you please review and merge?

@liat-grozovik
Copy link
Collaborator

@saiarcot895 could you please help to merge?

@dprital
Copy link
Collaborator

dprital commented Jul 30, 2024

Should be merged only after upstream of SAI version 2405.29.0.0.
@Junchao-Mellanox , please mention it on PR description

@Junchao-Mellanox
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stepanblyschak stepanblyschak mentioned this pull request Aug 21, 2024
Open
@liat-grozovik
Copy link
Collaborator

@kcudnik can you please help to merge?

@liat-grozovik
Copy link
Collaborator

@saiarcot895 @kcudnik kindly reminder to merge this one

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

@kcudnik kcudnik kcudnik approved these changes

@stepanblyschak stepanblyschak stepanblyschak approved these changes

@Prasanth-KV Prasanth-KV Prasanth-KV approved these changes

@yxieca yxieca Awaiting requested review from yxieca

@vaibhavhd vaibhavhd Awaiting requested review from vaibhavhd

@adyeung adyeung Awaiting requested review from adyeung

Assignees
No one assigned
Labels
Request for 202405 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Junchao-Mellanox @kcudnik @mssonicbld @yxieca @adyeung @liat-grozovik @dprital @stepanblyschak @Prasanth-KV