[action] [PR:14856] [Snappi]: New base config function to accomodate mixed-speed ingress and egress tests. #16030
Conversation
This file contains 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
…and egress tests. (sonic-net#14856) Description of PR Summary: Existing snappi_dut_base_config in tests/common/snappi_tests/snappi_fixtures.py has an assert in case, mixed-speed ingress and egress interfaces are selected. Since the interface speeds were same, the L1 configuration was done ONLY once. With mixed-speed interfaces being used as ingress and egress, the assert needs to be removed. Second issue with existing snappi_multi_base_config was that speed was set to ONLY one of the interfaces being used for the test. This was incorrect for mixed speed interfaces, causing Snappi API itself to crash. Fixes # (issue) sonic-net#12966 Approach What is the motivation for this PR? Existing snappi_dut_base_config asserts when ingress and egress interface speeds are different. Furthermore, snappi framework itself did not support mixed-speed interfaces for the test and crashed (Please see issue sonic-net#12966 ) for the same. How did you do it? Added a new function - snappi_sys_base_config which replaces the assert with info level log indicating that interfaces are of different speeds. The L1 configuration is done for all the snappi_ports and set appropriate speed for all the snappi_ports. Ideally, existing snappi_dut_base_config could be modified with additional argument mixed-speed=NONE, and then selectively run the code for the mixed-speed=TRUE. However, this being frequently used function, I will keep it as is, and add a new function to ensure, existing function is not broken. How did you verify/test it? Ran on the local clone with mixed and same speed interfaces. No issues seen. Any platform specific information? Supported testbed topology if it's a new test case? Documentation co-authorized by: jianquanye@microsoft.com
|
/azp run |
Merged
8 tasks
|
Original PR: #14856 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of PR
Summary:
Existing snappi_dut_base_config in tests/common/snappi_tests/snappi_fixtures.py has an assert in case, mixed-speed ingress and egress interfaces are selected.
Since the interface speeds were same, the L1 configuration was done ONLY once.
With mixed-speed interfaces being used as ingress and egress, the assert needs to be removed.
Second issue with existing snappi_multi_base_config was that speed was set to ONLY one of the interfaces being used for the test. This was incorrect for mixed speed interfaces, causing Snappi API itself to crash.
Fixes # (issue)
#12966
Type of change
Back port request
Approach
What is the motivation for this PR?
Existing snappi_dut_base_config asserts when ingress and egress interface speeds are different.
Furthermore, snappi framework itself did not support mixed-speed interfaces for the test and crashed (Please see issue #12966 ) for the same.
How did you do it?
Added a new function - snappi_sys_base_config which replaces the assert with info level log indicating that interfaces are of different speeds.
The L1 configuration is done for all the snappi_ports and set appropriate speed for all the snappi_ports. Ideally, existing snappi_dut_base_config could be modified with additional argument mixed-speed=NONE, and then selectively run the code for the mixed-speed=TRUE. However, this being frequently used function, I will keep it as is, and add a new function to ensure, existing function is not broken.
How did you verify/test it?
Ran on the local clone with mixed and same speed interfaces. No issues seen.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation