[202605] Smart Counter Poll to allow counters to work properly on Broadcom platforms.#1956
[202605] Smart Counter Poll to allow counters to work properly on Broadcom platforms.#1956justin-wong-ce wants to merge 5 commits into
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
Signed-off-by: Justin Wong <jvwong@arista.com>
5e06b50 to
ece0295
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw run |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1152093: ✅Stage BuildTrixie:
|
<!-- Please make sure you've read and understood our contributing guidelines; https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md Please provide following information to help code review process a bit easier: --> ### Description of PR <!-- - Please include a summary of the change and which issue is fixed. - Please also include relevant motivation and context. Where should reviewer start? background context? - List any dependencies that are required for this change. --> Summary: Casting workaround for N/A counters on management ports on Broadcom platforms. This impacts all hwsku-topo combos that uses management ports (including TH5). Casting from: Azure/sonic-sairedis.msft#69 Proper fix under review at: master: sonic-net#1774 202511: sonic-net#1862 202605: sonic-net#1956 Fixes sonic-net#1753 (temporary fix) ### Type of change <!-- - Fill x for your type of change. - e.g. - [x] Bug fix --> - [x] Bug fix - [ ] New feature - [ ] Refactor / cleanup - [ ] Documentation update - [ ] Test improvement ### Approach #### What is the motivation for this PR? To fix `N/A` counters on management ports on Broadcom platforms. ##### Work item tracking - Microsoft ADO **(number only)**: #### How did you do it? Temporarily fill in `0` values for counters that are supported in the usual interfaces but not supported in the management ports. #### How did you verify/test it? `show interface counters` no longer show `N/A` counters. #### Any platform specific information? Broadcom only. ### Documentation <!-- (If it's a new feature, new test case) Did you update documentation/Wiki relevant to your implementation? Link to the wiki page? --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
<!-- Please make sure you've read and understood our contributing guidelines; https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md Please provide following information to help code review process a bit easier: --> ### Description of PR <!-- - Please include a summary of the change and which issue is fixed. - Please also include relevant motivation and context. Where should reviewer start? background context? - List any dependencies that are required for this change. --> Summary: Casting workaround for N/A counters on management ports on Broadcom platforms. This impacts all hwsku-topo combos that uses management ports (including TH5). Casting from: Azure/sonic-sairedis.msft#69 Proper fix under review at: master: #1774 202511: #1862 202605: #1956 Fixes #1753 (temporary fix) ### Type of change <!-- - Fill x for your type of change. - e.g. - [x] Bug fix --> - [x] Bug fix - [ ] New feature - [ ] Refactor / cleanup - [ ] Documentation update - [ ] Test improvement ### Approach #### What is the motivation for this PR? To fix `N/A` counters on management ports on Broadcom platforms. ##### Work item tracking - Microsoft ADO **(number only)**: #### How did you do it? Temporarily fill in `0` values for counters that are supported in the usual interfaces but not supported in the management ports. #### How did you verify/test it? `show interface counters` no longer show `N/A` counters. #### Any platform specific information? Broadcom only. ### Documentation <!-- (If it's a new feature, new test case) Did you update documentation/Wiki relevant to your implementation? Link to the wiki page? --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
List of changes made: - Added tracking of free indicies for reuse - Added m_failedPolls cleanup when object is removed - Improved m_failedPolls lookup logic to only use a single lookup - Added addGroup helper - Used const& refs for sets instead of copies where applicable - Removed getSortedCGRef() for to avoid performance impact from the SWSS_LOG_ENTER() requirement in functions - Better clarity by changing find() -> std::find() - Simplied erase-emplace logic by using built in [] operator logic - Normalized key check method when dealing with maps - Indentation fixes - Minor optimization when iterating through counter groups in setupBaseCounterGroup() - Added warning comments on parts of code where more care is required (consuming of const refs) - Added unit test for m_failedPolls logic Signed-off-by: Justin Wong <jvwong@arista.com>
Signed-off-by: Justin Wong <jvwong@arista.com>
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Justin Wong <jvwong@arista.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
lolyu
left a comment
There was a problem hiding this comment.
why we need to manually cherry-pick this into 202605? is there any conflict?
Oh I was not able to cleanly cherry-pick to origin/202605 locally, but I had a slightly desync'd fork. I wil also update the test results on both PR threads when it is available. |
Description of PR
Summary:
This is change is to address #1753 and to implement the HLD logic in sonic-net/SONiC#2190 to 202605.
Currently, the code assumes all ports support the same counter capabilities. This causes issues on most Broadcom platform switches as there are different types of ports on a switch that does not support the same set of counters.
Fix this issue by dynamically discovering what each interface is capable of during initialization of
syncd.For more details, please refer to the referenced issue and HLD.
Fixes #1753
Type of change
Approach
What is the motivation for this PR?
Counters are not working on management ports on Broadcom platforms.
Work item tracking
How did you do it?
Implement the HLD logic from sonic-net/SONiC#2190.
How did you verify/test it?
Testing is done on both a
Arista-7060X6-16PE-384C-B-O128S2andArista-7260CX3-D108C8on 202511 with the tests:A full
sonic-mgmttest suite run has also been ran. There are no notable fallout compared tosonic-mgmtruns without this change. This is ran on both XGS and DNX Broadcom platforms.Any platform specific information?
Broadcom platforms only.
Documentation