Skip all admin down ports for non-Mellanox device #6433
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…om's device Signed-off-by: xuliping <[email protected]>
|
This pull request introduces 1 alert when merging c36013a into c41f842 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging e8ee2b5 into 1620b7d - view on LGTM.com new alerts:
|
Blueve
requested changes
Sep 30, 2022
| @@ -2481,15 +2481,8 @@ def _check_port_buffer_info_and_return(duthost, table, ids, port, expected_profi | |||
| else: | |||
| if is_mellanox_device(duthost): | |||
| buffer_items_to_check = buffer_items_to_check_dict["down"][key_name] | |||
| elif is_broadcom_device(duthost) and (asic_type in ['td2', 'td3'] or speed <= '10000'): | |||
There was a problem hiding this comment.
Why remove is_broadcom_device condition? We also have other platform such as marvell , seems like this change will also cause skipping happening on other platforms, not just broadcom.
There was a problem hiding this comment.
The special handling is needed only for mellanox. buffermgr will not attach any profiles for admin down ports on other platforms
There was a problem hiding this comment.
@lipxu could you update your PR title to Skip all admin down ports for non-Mellanox device?
There was a problem hiding this comment.
Sure, @Blueve , have updated the title.
Updated the commit last time, but forgot to update the title, sorry for the confusion.
neethajohn
approved these changes
Sep 30, 2022
lipxu
changed the title
Blueve
approved these changes
Oct 2, 2022
Azarack
pushed a commit
to Azarack/sonic-mgmt
that referenced
this pull request
Oct 17, 2022
* remove asic type and speed check, skip all admin down ports on broadcom's device What is the motivation for this PR? There was no BUFFER_PROFILE_TABLE for admin down ports How did you do it? Skip all admin down ports for Broadcom device How did you verify/test it? Verify the original failure case Signed-off-by: xuliping <[email protected]>
wangxin
pushed a commit
that referenced
this pull request
Oct 21, 2022
* remove asic type and speed check, skip all admin down ports on broadcom's device What is the motivation for this PR? There was no BUFFER_PROFILE_TABLE for admin down ports How did you do it? Skip all admin down ports for Broadcom device How did you verify/test it? Verify the original failure case Signed-off-by: xuliping <[email protected]>
wangxin
pushed a commit
that referenced
this pull request
Oct 21, 2022
* remove asic type and speed check, skip all admin down ports on broadcom's device What is the motivation for this PR? There was no BUFFER_PROFILE_TABLE for admin down ports How did you do it? Skip all admin down ports for Broadcom device How did you verify/test it? Verify the original failure case Signed-off-by: xuliping <[email protected]>
allen-xf
pushed a commit
to allen-xf/sonic-mgmt
that referenced
this pull request
Oct 28, 2022
* remove asic type and speed check, skip all admin down ports on broadcom's device What is the motivation for this PR? There was no BUFFER_PROFILE_TABLE for admin down ports How did you do it? Skip all admin down ports for Broadcom device How did you verify/test it? Verify the original failure case Signed-off-by: xuliping <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: xuliping [email protected]
Description of PR
Summary:
Fixes # (issue)
Skip all admin down ports for Broadcom device
Type of change
Back port request
Approach
What is the motivation for this PR?
There was no BUFFER_PROFILE_TABLE for admin down ports
How did you do it?
Skip all admin down ports for Broadcom device
How did you verify/test it?
Verify the original failure case
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation