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

Add test case for trap flow counter feature #4456

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

Junchao-Mellanox
Copy link
Contributor

Change-Id: I7f6b77dcb320fce1d9ea0be021486bb6b8e3ec87

Description of PR

Summary:
Add test case for trap flow counter feature

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911

Approach

What is the motivation for this PR?

Add test case for trap flow counter feature.

How did you do it?

This new test case is verifying the packet send number and packet rate. Test flow is like:

  1. Enable and clear trap flow counter
  2. Send packets via ptf
  3. Verify the send number is equal to the counter value, and the send rate is close to counter value

How did you verify/test it?

Manually run the test

Any platform specific information?

Any platform that support trap flow coutner

Supported testbed topology if it's a new test case?

ptf32/ptf64

Documentation

@Junchao-Mellanox
Copy link
Contributor Author

@yxieca could you assign someone to review this?

trap_type = protocol.lower()

# wait until the trap counter is enabled
assert wait_until(10, 1, _check_trap_counter_enabled, duthost, trap_type), 'counter is not created for {}'.format(trap_type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The wait_until interface has been changed in PR #4516. This function call needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do a rebase.

Change-Id: I7f6b77dcb320fce1d9ea0be021486bb6b8e3ec87
Change-Id: I6e8491cf88330e141b5e0d8c809488c10a0be79f
@Junchao-Mellanox
Copy link
Contributor Author

/azpw run

2 similar comments
@Junchao-Mellanox
Copy link
Contributor Author

/azpw run

@keboliu
Copy link
Contributor

keboliu commented Nov 3, 2021

/azpw run

@Junchao-Mellanox
Copy link
Contributor Author

Hi @wangxin , could you help trigger the build again?

@wangxin
Copy link
Collaborator

wangxin commented Nov 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@roy-sror
Copy link
Contributor

Hi @wangxin - could you please approve the PR

@wangxin wangxin merged commit c630ea1 into sonic-net:master Nov 16, 2021
@neethajohn
Copy link
Contributor

@wangxin, this feature is not yet merged into master. sonic-net/SONiC#858
The newly added testcases are failing. should we revert this?

yejianquan added a commit that referenced this pull request Nov 19, 2021
Approach
What is the motivation for this PR?
Enable test_copp on t0, eliminate test gap #4588

How did you do it?
Modify prepare logic to make it suitable to both t0 and t1 topologies.
Install another required package.
How did you verify/test it?
Run it on physical testbeds, including:

3 t0 testbeds with different platforms
1 t1 testbed to make sure it doesn't influence existing testbed
1 t1 backend testbed
All of them passed the test_policer and test_no_policer test functions,
but the new test function test_counter brought by #4456 raised errors due to the command mismatch of the bin file, it should be the issue of the new test case so we can ignore it for now.

Supported testbed topology if it's a new test case?
t0, t0-64, t0-52, t0-116

Co-authored-by: Jianquan Ye <jianquanye@microsoft.com>
neethajohn added a commit that referenced this pull request Nov 22, 2021
neethajohn added a commit that referenced this pull request Nov 23, 2021
This reverts commit c630ea1

Reverts #4456

This feature has not yet been reviewed sonic-net/SONiC#858

Affecting nighty run results because of this new testcase without feature support
AntonHryshchuk pushed a commit to AntonHryshchuk/sonic-mgmt that referenced this pull request Jan 4, 2022
What is the motivation for this PR?
Add test case for trap flow counter feature.

How did you do it?
This new test case is verifying the packet send number and packet rate. Test flow is like:
* Enable and clear trap flow counter
* Send packets via ptf
* Verify the send number equals to the counter value, and the send rate is close to counter value

How did you verify/test it?
Manually run the test

Any platform specific information?
Any platform that support trap flow counter
AntonHryshchuk pushed a commit to AntonHryshchuk/sonic-mgmt that referenced this pull request Jan 4, 2022
Approach
What is the motivation for this PR?
Enable test_copp on t0, eliminate test gap sonic-net#4588

How did you do it?
Modify prepare logic to make it suitable to both t0 and t1 topologies.
Install another required package.
How did you verify/test it?
Run it on physical testbeds, including:

3 t0 testbeds with different platforms
1 t1 testbed to make sure it doesn't influence existing testbed
1 t1 backend testbed
All of them passed the test_policer and test_no_policer test functions,
but the new test function test_counter brought by sonic-net#4456 raised errors due to the command mismatch of the bin file, it should be the issue of the new test case so we can ignore it for now.

Supported testbed topology if it's a new test case?
t0, t0-64, t0-52, t0-116

Co-authored-by: Jianquan Ye <jianquanye@microsoft.com>
AntonHryshchuk pushed a commit to AntonHryshchuk/sonic-mgmt that referenced this pull request Jan 4, 2022
…sonic-net#4747)

This reverts commit c630ea1

Reverts sonic-net#4456

This feature has not yet been reviewed sonic-net/SONiC#858

Affecting nighty run results because of this new testcase without feature support
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.

5 participants