Smart Counter Poll to allow counters to work properly on Broadcom platforms.#1755
Open
justin-wong-ce wants to merge 11 commits intosonic-net:202511from
Open
Smart Counter Poll to allow counters to work properly on Broadcom platforms.#1755justin-wong-ce wants to merge 11 commits intosonic-net:202511from
justin-wong-ce wants to merge 11 commits intosonic-net:202511from
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Justin Wong <jvwong@arista.com>
Signed-off-by: Justin Wong <jvwong@arista.com>
Builds are failing due to tests failing a check where every function needs a `SWSS_LOG_ENTER();`. Adding it to the functions missing it. Signed-off-by: Justin Wong <jvwong@arista.com>
justin-wong-ce
force-pushed
the
202511_counter_enhancement
branch
from
6d9b8d0 to
e7fdfd5
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixed code logic that does not account for a change in supported counters on an already initialized FlexCounter object. On an actual device, counter support does not change on an initialized FlexCounter object as it is hardware / SAI bound. Fixed mock function not returning a SAI_STATUS_FAILURE when a counter poll is supposed to fail. Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Author
|
WIP - adding unit test |
Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- reverted previous test mock function change with getStats as it was intended - changed some ordering of test value checking as ordering of counters have changed - added logic to handle removal of counters from counter groups - added logic to cleanup stale counter groups - added logic to account for behavior when different global flags are set, i.e. (use_sai_stats_capa_query, dont_clear_support_counter, always_check_supported_counters) - moved large duplicated code blocks into helper functions Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Change makeCounterGroupRef args to use size_t instead of uint64_t Signed-off-by: Justin Wong <jvwong@arista.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
@justin-wong-ce can you please share master pr for the same ? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
This is change is to address #1753 and the 202511 implementation of the HLD added in sonic-net/SONiC#2190.
Existing 202511 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 by dynamically discovering what each interface is capable of during initialization of
syncd.For more detials please refer to the above issue and HLD.
Testing
Testing is done on both a
Arista-7060X6-16PE-384C-B-O128S2andArista-7260CX3-D108C8on 202511 with the tests:Due to ongoing warm reboot test issues in 202511 (sonic-net/sonic-swss#4108), warm reboot tests (
sonic-mgmt/tests/platform_tests/test_advanced_reboot.py) are instead conducted on 202505 usingArista-7260CX3-D108C8with this change backported to it.A full
sonic-mgmttest suite run has also been ran. There are no notable fallout compared tosonic-mgmtruns without this change.Performance Impact:
All logic change is only done in the counter initialisation stage of FlexCounters.cpp - there is no polling logic change at all.
Therefore, any performance impact is limited to any new execution of
syncd- i.e. reboot /config reload/systemctl restart.Tested on several topologies:
This impact seems reasonable for what this offers.
The fastest operation that would cause a
syncdrestart is asystemctl restart swss, that is a operation that spans minutes. The worst real life scenario on a HwSKU with older CPU and many interfaces takes additional ~17 seconds. New, high interface-count HwSKUs only takes an extra ~2 seconds.As for memory usage - the impact is on the order of kilobytes, the impact is negligible as the system has GBs of RAM.