Re-introduce the original everflow PR to support T2#6958
Re-introduce the original everflow PR to support T2#6958judyjoseph wants to merge 3 commits intosonic-net:masterfrom
Conversation
This reverts commit 1422b9b.
|
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
|
This pull request fixes 1 alert when merging 22d87d6 into 945eac8 - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
|
@judyjoseph - could you please include the attached patch as well. This changes setup_recycle_port fixture scope from 'package' to 'module'. With the introduction of 'core_dump_and_config_check' fixture in tests/conftest.py, we would do a 'config reload' at the end of the first module which would delete the configured recycle port IP addresses. |
|
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
updated @sanmalho-git - thanks ! |
|
@StormLiangMS - have identified the fix and now the everflow tests pass for T0 topology as well. |
| an=asic.asic_index + 1) | ||
| logging.info(cmd) | ||
| duthost.command(cmd) | ||
| duthost.command("sudo config save -y") |
There was a problem hiding this comment.
Is it necessary to do config save?
There was a problem hiding this comment.
Now that fixture is module scope it should be safe to remove.
|
|
||
| if 't2' in topo: | ||
| for duthost in duthosts.frontend_nodes: | ||
| duthost.command("mkdir -p {}".format(DUT_RUN_DIR)) |
There was a problem hiding this comment.
Is it necessary to put duthost.command("mkdir -p {}".format(DUT_RUN_DIR)) in the for loop?
There was a problem hiding this comment.
Can be changed to [setup_information['t3_duthost'], setup_information['t1_duthost']]
|
|
||
| BaseEverflowTest.apply_mirror_config(duthost, session_info, config_method) | ||
| for duthost in duthost_list: | ||
| session_info = BaseEverflowTest.mirror_session_info("test_session_1", duthost.facts["asic_type"]) |
There was a problem hiding this comment.
Can we put session_info = BaseEverflowTest.mirror_session_info("test_session_1", duthost.facts["asic_type"]) outside the for loop?
| if setup_info['topo'] == "t2": | ||
| # Further route add will not work this way because of recycle port. This newly added ECMP route may be | ||
| # used since recycle port sends back into datapath. | ||
| return |
There was a problem hiding this comment.
Please use pytest.skip or pytest_require to skip the test case on T2
| pass | ||
| else: | ||
| if "LLDP" in ptf.packet.Ether(received_packet).summary(): | ||
| logging.info("LLDP packet received, not mirror test packet.") |
There was a problem hiding this comment.
Do we need to ignore LACP as well?
|
|
||
| @pytest.fixture(scope="class") | ||
| def policer_mirror_session(self, duthosts, rand_one_dut_hostname, config_method): | ||
| def policer_mirror_session(self, duthosts, enum_rand_one_per_hwsku_frontend_hostname, config_method, setup_info): |
There was a problem hiding this comment.
@judyjoseph this name "enum_rand_one_per_hwsku_frontend_hostname" sounds like for multi asic or T2 only, could we use a common name like "rand_one_dut_hostname"?
|
this PR need to be refactor on top of #6945 for VOQ Chassis or we can close this one and have another new PR. |
|
Closing this as there is another PR which takes the changes here + support for chassis packet architecture merged via #6945 |
6 participants
Description of PR
Re-introduce the original everflow PR (#6225) to support T2
commit 22d87d6 in this PR
everflow test case fix #6561
[everflow] address testbed setup type checking issue #6609
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
How did you do it?
How did you verify/test it?
Ran tests with T0 topology
Ran against T2 topology
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation