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

Fix for delay flex counters flow #1739

Closed
wants to merge 2 commits into from
Closed

Fix for delay flex counters flow #1739

wants to merge 2 commits into from

Conversation

shlomibitton
Copy link
Contributor

Signed-off-by: Shlomi Bitton shlomibi@nvidia.com

What I did
Exclude installing flex counter for CPU interface (mgmt) for proper query counter capability.
Fixing this PR: #1646

Why I did it
The CPU interface does not suppose to be inserted to flex counter DB.
In addition, if we query the counter statistics capability by this interface it will cause all port counters id lists to be empty and break lua port rates functionality.

How I verified it
Enable all counters group types and observe all ID lists for ports populated correctly.

Details if related

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
Copy link
Collaborator

@liat-grozovik liat-grozovik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing unit test to make sure that when CPU port is the first it is working as expected. so then you can add the original fix which is now expected to be reverted.

@shlomibitton
Copy link
Contributor Author

@qiluo-msft @lguohan Can you please review this?
This is the fix for the issue introduced from PR: #1646 #1736
I added another unit test to verify this case.
To merge this fix properly we need to merge again the previous PR's and then merge this fix to both branches.

@qiluo-msft
Copy link
Contributor

Could you included previous code into this PR so we can merge this PR atomically?

@shlomibitton
Copy link
Contributor Author

Could you included previous code into this PR so we can merge this PR atomically?

@qiluo-msft I created 2 new PR's with the previous code, issue fix and new unit test case for both branches.
master: #1749
202012: #1750

I am closing this PR since it is not relevant

@shlomibitton shlomibitton deleted the shlomi_fix_delay_counters branch July 27, 2021 16:32
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants