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

add test plan for SONIC vrf feature #392 #409

Merged
merged 4 commits into from
Oct 9, 2019
Merged

add test plan for SONIC vrf feature #392 #409

merged 4 commits into from
Oct 9, 2019

Conversation

shine4chen
Copy link
Contributor

It includes ansible test cases and virtual switch test cases.

Signed-off-by: shine.chen shine.chen@nephosinc.com

shine.chen and others added 3 commits June 20, 2019 00:48
@preetham-singh
Copy link
Contributor

Can you please add test cases for below features in VRF context:

  1. Test Link Local IPv6 address in VRF, ping, forwarding & BGP neighborship.
  2. In Loopback interface test, check loopback interface IP can be used as a source in ping/ping6, BGP/BGP+ update-source and router-id for the VRF.
  3. BGP multihop peer over IGP(static) which has ECMP in VRF. Failover of a nhop in a ECMP should not impact BGP-TCP connection in kernel. This will ensure control plane ECMP in VRF.

@prsunny
Copy link
Contributor

prsunny commented Jul 19, 2019

At a higher level, the test coverage looks good to me. Consider adding few cases (routes, nexthops) to check CRM resources with VRF. You can check the values before and after config. Also suggest to combine test cases wherever possible instead of separate test cases for each scenario. Scalability can be given as a configurable option.

@shine4chen
Copy link
Contributor Author

@preetham-singh Thanks for your suggestion.

  1. In the existing sonic there are some issues about local-link address. So I suggest we can add local-link address test case after fixing these issues.
  2. Yes, it is the default behavior of FRR BGP template session. If no explicit configured BGP session always chooses the first loop-back device address on the specified vrf as its route-id and chooses the second loop-back device address as update-source. We can add a test case to cover loop-back address as ping/ping6 source.
  3. Control plane tcp connection is handled by linux kernel either in global or specified vrf. I think it may beyond vrf test scope.

@shine4chen
Copy link
Contributor Author

@prsunny agree, we can rerun some crm ansible test case in vrf environment.

@freeman386
Update vrf-ansible-test-plan.md
61838d0 
1. remove vrf attributes test
2. add loopback interface as bgp update-source test
3. modify bgp/acl redirect configuration
@dawnbeauty dawnbeauty mentioned this pull request Aug 12, 2019
Merged
1 task
@dawnbeauty
Copy link
Contributor

@prsunny @shine4chen Currently we do not add testcases about CRM with VRF. Since i dont think there is any particularly difference of CRM usage between global and VRF. If CRM ansible testcases passed (which means CRM works as normal without VRF), then it also means CRM works with VRF.

@lguohan lguohan merged commit a9ee08e into sonic-net:master Oct 9, 2019
@vboykox
Copy link
Member

vboykox commented May 18, 2022

@prsunny @shine4chen @lguohan Hi

Depending on the test cases PTF will verify the packet is arrived or dropped. For vrf
"src_mac" option test, PTF will analyze ip packet dst_mac after L3 forwarding
through vrf and do L3 forwarding only when ip packet's dst_mac is matched with
configed vrf "src_mac".

It looks that this contradicts the current SONiC behaviour - RIFs' src mac do not
get updated on VRF's src mac change.
Please see this issue:
sonic-net/sonic-swss#1833

So what behaviour for VRF/RIF src mac update is expected here by SONiC design?
SAI does not impose a requirement to "auto-update" RIF's src macs on VRF change.
I guess either the auto-update should be handled by SONiC, or testplan and
related test should be changed appropriately to not count on it.

Thanks in advance

@vboykox vboykox mentioned this pull request May 23, 2022
Merged
5 tasks
yejianquan pushed a commit to sonic-net/sonic-mgmt that referenced this pull request May 23, 2022
@vboykox
[test_vrf1_neigh_with_new_router_mac] increase socket size to receive…
147b978 
… jumbogram (#5669)

Signed-off-by: Volodymyr Boyko <volodymyrx.boiko@intel.com>

Description of PR
Approach
What is the motivation for this PR?
vrf test sends packets of size 9k; need to increase packet socket recv size accordingly, otherwise received packets get truncated to 4096 (default receive size)
How did you verify/test it?
Verified by manually checking if the packet received during test_vrf1_neigh_with_new_router_mac.
With this change it fails with:
"Exception: Pkt sent from 30.0.0.1 to 192.168.0.2 on port 2 was rcvd pkt on 1 which is one of the expected ports, but the src mac doesn't match, expected 00:12:34:56:78:9a, got 00:90:fb:5e:d6:be",
instead of no packet received.
The reason why the test still fails and ongoing discussion in these issues:
sonic-net/SONiC#409 (comment)
sonic-net/sonic-swss#1833
@vboykox
Copy link
Member

vboykox commented May 24, 2022

@prsunny @shine4chen @lguohan Hi

Depending on the test cases PTF will verify the packet is arrived or dropped. For vrf
"src_mac" option test, PTF will analyze ip packet dst_mac after L3 forwarding
through vrf and do L3 forwarding only when ip packet's dst_mac is matched with
configed vrf "src_mac".

It looks that this contradicts the current SONiC behaviour - RIFs' src mac do not get updated on VRF's src mac change. Please see this issue: Azure/sonic-swss#1833

So what behaviour for VRF/RIF src mac update is expected here by SONiC design? SAI does not impose a requirement to "auto-update" RIF's src macs on VRF change. I guess either the auto-update should be handled by SONiC, or testplan and related test should be changed appropriately to not count on it.

Thanks in advance

@prsunny kindly reminder

@shine4chen
Copy link
Contributor Author

@vboykox I prefer to revise the related test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prsunny prsunny prsunny approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@shine4chen @preetham-singh @prsunny @dawnbeauty @vboykox @lguohan @freeman386