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

Ignore removing switch for mellanox platform due to known limitation #1216

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

Junchao-Mellanox
Copy link
Contributor

Why I did this?

Due to a limitation in mellanox platform, sonic ignores syncs graceful shutdown in syncd.sh previously. However, ignoring graceful shutdown in syncd.sh also ignores closing other resources such as removing all counters.
This PR is going to ignore remove_switch for mellanox platform when shutdown type is cold. It make sure other shutdown operations are properly executed in shutdown flow.

How I test this?

Running config reload on many platforms and check result.

@Junchao-Mellanox
Copy link
Contributor Author

Hi @kcudnik , could you please kindly review this?

@liat-grozovik
Copy link
Collaborator

@kcudnik @saiarcot895 can you please review and merge?

@saiarcot895 saiarcot895 merged commit acd57ae into sonic-net:master Mar 13, 2023
yxieca pushed a commit that referenced this pull request Mar 15, 2023
…1216)

* Ignore removing switch for mellanox platform due to known limitation

* Fix review comment

* Fix review comment
StormLiangMS pushed a commit that referenced this pull request Mar 19, 2023
…1216)

* Ignore removing switch for mellanox platform due to known limitation

* Fix review comment

* Fix review comment
@Junchao-Mellanox Junchao-Mellanox deleted the fix-syncd-stop branch April 24, 2023 02:49
Junchao-Mellanox added a commit to Junchao-Mellanox/sonic-sairedis that referenced this pull request Apr 24, 2023
Junchao-Mellanox added a commit to Junchao-Mellanox/sonic-sairedis that referenced this pull request Apr 24, 2023
Junchao-Mellanox added a commit to Junchao-Mellanox/sonic-sairedis that referenced this pull request Apr 24, 2023
yxieca pushed a commit that referenced this pull request Apr 24, 2023
kcudnik pushed a commit that referenced this pull request Apr 28, 2023
…itation (#1216)" (#1233)

This commit introduces a race condition with would cause syncd crash. The issue happens when:

Syncd.m_sn has been freed
SAI send a port state change notification which will access Syncd.m_sn ===> causes crash
qiluo-msft pushed a commit that referenced this pull request May 1, 2023
…itation (#1216)" (#1232)

What I did?

This reverts commit 62aa244.

Why I did this?

This commit introduces a race condition with would cause syncd crash. The issue happens when:

Syncd.m_sn has been freed
SAI send a port state change notification which will access Syncd.m_sn ===> causes crash
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.

7 participants