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.

Sign up for GitHub

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

Jump to bottom

[orchagent] Adds swss support for drop counters #1075

Merged
merged 30 commits into from
Nov 19, 2019
Merged

[orchagent] Adds swss support for drop counters #1075

merged 30 commits into from
Nov 19, 2019

Conversation

daall
Copy link
Contributor

@daall daall commented Sep 30, 2019 •
edited
Loading

Adds swss support for drop counters

  • Creates a new orchestrator for managing debug counters
  • Adds utility functions for managing debug counters

How I verified it
A set of vswitch tests is included.

=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.15+, pytest-3.3.0, py-1.8.0, pluggy-0.6.0 -- /usr/bin/python2
cachedir: .cache
rootdir: /home/daall/dev/sonic-swss/tests, inifile:
collected 14 items                                                                                                                                                                                                

test_drop_counters.py::TestDropCounters::test_deviceCapabilitiesTablePopulated remove extra link dummy
PASSED                                                                                                                       [  7%]
test_drop_counters.py::TestDropCounters::test_flexCounterGroupInitialized PASSED                                                                                                                            [ 14%]
test_drop_counters.py::TestDropCounters::test_createAndRemoveDropCounterBasic PASSED                                                                                                                        [ 21%]
test_drop_counters.py::TestDropCounters::test_createAndRemoveDropCounterReversed PASSED                                                                                                                     [ 28%]
test_drop_counters.py::TestDropCounters::test_createCounterWithInvalidCounterType PASSED                                                                                                                    [ 35%]
test_drop_counters.py::TestDropCounters::test_createCounterWithInvalidDropReason PASSED                                                                                                                     [ 42%]
test_drop_counters.py::TestDropCounters::test_addReasonToInitializedCounter PASSED                                                                                                                          [ 50%]
test_drop_counters.py::TestDropCounters::test_removeReasonFromInitializedCounter PASSED                                                                                                                     [ 57%]
test_drop_counters.py::TestDropCounters::test_addDropReasonMultipleTimes PASSED                                                                                                                             [ 64%]
test_drop_counters.py::TestDropCounters::test_addInvalidDropReason PASSED                                                                                                                                   [ 71%]
test_drop_counters.py::TestDropCounters::test_removeDropReasonMultipleTimes PASSED                                                                                                                          [ 78%]
test_drop_counters.py::TestDropCounters::test_removeNonexistentDropReason PASSED                                                                                                                            [ 85%]
test_drop_counters.py::TestDropCounters::test_removeInvalidDropReason PASSED                                                                                                                                [ 92%]
test_drop_counters.py::TestDropCounters::test_createAndDeleteMultipleCounters PASSED                                                                                                                        [100%]

=========================================================================================== 14 passed in 113.65 seconds ===========================================================================================

Details if related
Depends on:

Signed-off-by: Danny Allen daall@microsoft.com

@daall daall mentioned this pull request Oct 1, 2019
Merged
@daall daall marked this pull request as ready for review October 3, 2019 22:08
@daall daall force-pushed the drop_counters branch 2 times, most recently from 7ba0a4d to 7cbe631 Compare October 12, 2019 01:59
This was referenced Oct 12, 2019
Merged
Closed
@daall daall self-assigned this Oct 18, 2019
dev-aviz
dev-aviz reviewed Oct 22, 2019
View reviewed changes
utilities/debug_counter/debug_counter.cpp Outdated
this->counter_id = debug_counter_id;
}

void DebugCounter::removeDebugCounterFromSAI()
Copy link

Choose a reason for hiding this comment

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

How do handle warm reboot case?
Can the SWSS reset the counters? or reconcile the debug counters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll have to verify but yes, the counters should be reconciled upon warm reboot.

qiluo-msft
qiluo-msft reviewed Oct 29, 2019
View reviewed changes
utilities/Makefile.am Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed Oct 29, 2019
View reviewed changes
configure.ac Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed Oct 29, 2019
View reviewed changes
orchagent/Makefile.am Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft requested changes Oct 29, 2019
View reviewed changes
Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

Please also resolve conflicts and fix vs tests.

@daall
[orchagent] Adds swss support for drop counters
f298f1f 
- Creates a new orchestrator for managing debug counters
- Adds utility functions for managing flex counters
- Adds utility functions for managing debug counters

Signed-off-by: Danny Allen <daall@microsoft.com>
@daall
Fix build issues
0787c6e
@daall
Fix log levels
11e540b
@yxieca
Copy link
Contributor

yxieca commented Nov 15, 2019

My comments are mostly cosmetic issues. Overall the change looks pretty good.

Thanks for adding these comments in source code to tell tricky choices. These comments helps a lot.

Add some comments to state the purpose of test steps would also help readers.

I found that you used quite a bit 'this->' to specify scope in your code. This is not wrong per-se. Just curious why are you doing compiler's job? :-)

qiluo-msft
qiluo-msft approved these changes Nov 18, 2019
View reviewed changes
Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

Looks good to me. Please also check others' comments

@daall
Copy link
Contributor Author

daall commented Nov 19, 2019

retest this please

@yxieca
Copy link
Contributor

yxieca commented Nov 19, 2019

retest this please

@yxieca yxieca merged commit c3b8fe1 into sonic-net:master Nov 19, 2019
@daall daall deleted the drop_counters branch December 18, 2019 01:37
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
@vasant17
Update Command-Reference.md (sonic-net#1075)
cf314cf 
Aligned single back quotes in .md file
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
@vaibhavhd
Add SAI_OBJECT_TYPE_TUNNEL object to break-before-make list (sonic-ne…
9652ea4 
…t#1075)

SAI_OBJECT_TYPE_TUNNEL has fewer attributes in 201811 than in 202012. These new attributes are CREATE only, and can't be added using just SET oper. Hence old object needs to be removed, and new object needs to be added.
Moreover existing sequence (make before break) causes SAI errors in removing VXLAN tunnel (as part of CPA teardown).

Adding SAI_OBJECT_TYPE_TUNNEL to break before make to avoid creating a new object before removing existing ones.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dev-aviz dev-aviz dev-aviz left review comments

@yxieca yxieca yxieca left review comments

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@wendani wendani Awaiting requested review from wendani

@prsunny prsunny Awaiting requested review from prsunny prsunny is a code owner

@lguohan lguohan Awaiting requested review from lguohan

@marian-pritsak marian-pritsak Awaiting requested review from marian-pritsak

Assignees

@daall daall

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@daall @yxieca @qiluo-msft @dev-aviz