Support running acl tests without any IPv4 management configuration#21155
Support running acl tests without any IPv4 management configuration#21155bingwang-ms merged 16 commits intosonic-net:masterfrom
Conversation
Signed-off-by: Will Rideout <wrideout@arista.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Commenter does not have sufficient privileges for PR 21155 in repo sonic-net/sonic-mgmt |
|
hi @wrideout-arista , looks like this PR has some conflict that causes CI unable to run. do you mind to help resolve it? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This should be good to go |
|
@r12f this looks ready for review |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
adding @bingwang-ms for review. |
| dst_ipv6 = config['dst_ipv6'] | ||
| source_ip = str(ip_interface(source_ip_str).ip) | ||
| source_ipv6 = str(ip_interface(source_ipv6_str).ip) | ||
| source_ip = str(ip_interface(source_ip_str).ip) if source_ip_str else None |
There was a problem hiding this comment.
Can you remind me why garp_service.py was changed? I remember it's only used in dualtor setup. In single-ToR setup, we are using arp_responder
There was a problem hiding this comment.
Prior to skipping all v4 cases when run on isolated-v6 testbeds, we found that the garp service was emitting a TypeError when trying to configure and send arp packets using a v4 address which was None. The changes here are meant to prevent this by selectively configuring and sending arp, or nd, or both, all depending on the configuration of the testbed.
Granted, these changes are less impactful due to the fact that the ip_version fixture in test/test_acl.py now skips v4 tests on isolated-v6 testbeds, but the bug still exists without them. With these changes to the service we no longer see a TypeError.
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155
|
@bingwang-ms is this good to go? |
Hi @wrideout-arista, thanks for sharing the rest result. I noticed there are still 4 test failures. Is that related to this change? |
|
hi @wrideout-arista do you mind to help xfail the 4 failing tests, we can follow them up later. thanks a lot! |
|
Looks like #18077 already exists to track those failures, so I will reference that in some new xfails. |
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com>
|
Cherry-pick PR to 202511: #22006 |
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: Priyansh Tratiya <ptratiya@microsoft.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: Yael Tzur <ytzur@nvidia.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: nnelluri-cisco <nnelluri@cisco.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: Raghavendran Ramanathan <rraghav@cisco.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: Zhuohui Tan <zhuohui.tan@amd.com>
Remove xfail because the test can pass on v6 topos after garp fix: #21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com> Signed-off-by: Mihut Aronovici <aronovic@cisco.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
<!-- Please make sure you've read and understood our contributing guidelines; https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md Please provide following information to help code review process a bit easier: --> ### Description of PR Remove xfail because the test can pass on v6 topos after garp fix: sonic-net/sonic-mgmt#21155 <!-- - Please include a summary of the change and which issue is fixed. - Please also include relevant motivation and context. Where should reviewer start? background context? - List any dependencies that are required for this change. --> Summary: Fixes # (issue) ### Type of change <!-- - Fill x for your type of change. - e.g. - [x] Bug fix --> - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [x] Test case improvement ### Back port request - [x] 202412 - [x] 202505 ### Approach #### What is the motivation for this PR? The test case can pass with the fix from sonic-net/sonic-mgmt#21155 #### How did you do it? Remove xfail in tests_mark_conditions.yaml #### How did you verify/test it? The test passed on v6 topo after removing xfail #### Any platform specific information? #### Supported testbed topology if it's a new test case? v6 topos ### Documentation <!-- (If it's a new feature, new test case) Did you update documentation/Wiki relevant to your implementation? Link to the wiki page? --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
Remove xfail because the test can pass on v6 topos after garp fix: #21155 Signed-off-by: markxiao <markxiao@arista.com>
…1064) <!-- Please make sure you've read and understood our contributing guidelines; https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md Please provide following information to help code review process a bit easier: --> ### Description of PR Remove xfail because the test can pass on v6 topos after garp fix: sonic-net/sonic-mgmt#21155 <!-- - Please include a summary of the change and which issue is fixed. - Please also include relevant motivation and context. Where should reviewer start? background context? - List any dependencies that are required for this change. --> Summary: Fixes # (issue) ### Type of change <!-- - Fill x for your type of change. - e.g. - [x] Bug fix --> - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [x] Test case improvement ### Back port request - [x] 202412 - [x] 202505 ### Approach #### What is the motivation for this PR? The test case can pass with the fix from sonic-net/sonic-mgmt#21155 #### How did you do it? Remove xfail in tests_mark_conditions.yaml #### How did you verify/test it? The test passed on v6 topo after removing xfail #### Any platform specific information? #### Supported testbed topology if it's a new test case? v6 topos ### Documentation <!-- (If it's a new feature, new test case) Did you update documentation/Wiki relevant to your implementation? Link to the wiki page? --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
…onic-net#21155) * Support running acl tests without any IPv4 management configuration. Signed-off-by: Will Rideout <wrideout@arista.com> Signed-off-by: Abhishek <abhishek@nexthop.ai>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com> Signed-off-by: Abhishek <abhishek@nexthop.ai>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com> Signed-off-by: Venkata Gouri Rajesh Etla <vrajeshe@cisco.com>
Remove xfail because the test can pass on v6 topos after garp fix: sonic-net#21155 Signed-off-by: markxiao <markxiao@arista.com>
Description of PR
Summary:
Fixes #18077
Modify the gratuitous arp service such that arp or neighbor-discovery packets are sent based on the L3 configuration of the testbed topology. For testbed topologies which do not configure IPv4 addressing, such as isolated-v6 testbeds, we skip the creation of arp packets and do not send them. In this case, only IPv6 neighbor-discovery packets are created and sent. The inverse is true as well when a testbed topology only specifies the use of IPv4 connections.
Type of change
Back port request
Approach
What is the motivation for this PR?
This change was made as a part of the isolated-v6 testbed qualification effort. Running acl tests on isolated-v6 testbeds was previously unsupported, and skipped via conditional mark, as the tests expected to receive IPv4 arp packets when no IPv4 connectivity was established.
How did you verify/test it?
The full suite of acl tests was run against a testbed where IPv4 and IPv6 connectivity was specified in the topology config, and no regressions were seen. The same suite of tests was run against a isolated-v6 testbed, and new test passes were observed: some new failures were also observed for tests which were previously skipped. However @r12f asked that we submit this change, and follow up with the remaining failing tests individually in subsequent changes as this change improves overall test coverage.
Any platform specific information?
None.