[test]: Update BGP speaker test by volodymyrsamotiy · Pull Request #236 · sonic-net/sonic-mgmt

volodymyrsamotiy · 2017-08-03T09:04:40Z

Add logic to test BGP-MP feature
Add logic to test traffic trough the advertised routes

* Add logic to test BGP-MP feature * Add logic to test traffic trough the advertised routes

lguohan · 2017-08-09T07:12:52Z

sihuihan88 · 2017-08-16T18:14:25Z

ansible/roles/test/templates/bgp_speaker_route.j2

+{% elif addr_family == 'ipv4' %}
+0.0.0.0/0 {% for portchannel, v in minigraph_portchannels.iteritems() %}[{% for member in v.members %}{{ '%d' % minigraph_port_indices[member]}}{% if not loop.last %} {% endif %}{% endfor %}]{% if not loop.last %} {% endif %}{% endfor %}
+
+{{announce_prefix}} {% for vlan, v in minigraph_vlans.iteritems() %}{% for member in v.members %}[{{ '%d' % minigraph_port_indices[member]}}]{% if not loop.last %} {% endif %}{% endfor %}{% if not loop.last %} {% endif %}{% endfor %}


Is the nexthop vlan or portchannel?

Portchannel. Fixed it.

It doesn't sound right to me. We have announced the next-hop of the route as VLAN. Shouldn't expect packets on Portchannels. We might need to change both ipv4 and ipv6 to VLAN here.

I tested it locally, I think we also need to config the ips of two mux on PTF host to make sure it will not use default route.

one example: In in exabgp/start.sh template, to config one mux_ip: ifconfig eth{{ '%d' % (minigraph_vlans[minigraph_vlan_interfaces[0]['attachto']]['members'][1] | replace("Ethernet", "") | int / 4)}} {{item.mux_ip_1}}

@volodymyrsamotiy

sihuihan88 · 2017-08-16T18:15:57Z

ansible/roles/test/tasks/bgp_speaker.yml

+      - testbed_type='{{testbed_type}}'
+      - router_mac='{{ansible_Ethernet0['macaddress']}}'
+      - fib_info='/root/bgp_speaker_route.txt'
+      - ipv4={{ ipv4|default(false) }}


default true for ipv4?

Yes and default value is set to IPv4 in line 80.

lguohan · 2017-08-25T17:36:36Z

ansible/roles/test/tasks/bgp_speaker.yml

+
+- name: Activate IPv6 address family
+  shell: vtysh -c "conf t" -c "router bgp {{ minigraph_bgp_asn }}" -c "address-family ipv6" -c "neighbor BGPSLBPassive activate"
+  when: addr_family == 'ipv6'


please also add this to the bgpd.conf.j2 in buildimage.

It is already in buildimage (hash commit 127a73a), so removed these changes.

* Fix comments

lguohan · 2017-09-11T21:03:16Z

@sihuihan88, are all comments from you addressed?

sihuihan88 · 2017-09-11T23:40:39Z

No. There is one comment haven't been replied. Please revise according to previous comments. Copy here in case you didn't see it. @volodymyrsamotiy
As above in ansible/roles/test/templates/bgp_speaker_route.j2

It doesn't sound right to me. We have announced the next-hop of the route as VLAN. Shouldn't expect packets on Portchannels. We might need to change both ipv4 and ipv6 to VLAN here.


I tested it locally, I think we also need to config the ips of two mux on PTF host to make sure it will not use default route.

one example: In in exabgp/start.sh template, to config one mux_ip: ifconfig eth{{ '%d' % (minigraph_vlans[minigraph_vlan_interfaces[0]['attachto']]['members'][1] | replace("Ethernet", "") | int / 4)}} {{item.mux_ip_1}}```

* Fix comments

sihuihan88 · 2017-09-22T00:37:41Z

ansible/roles/test/templates/bgp_speaker_route.j2

@@ -0,0 +1,8 @@
+{% if addr_family == 'ipv6' %}
+{{announce_prefix}} [{% for member in minigraph_portchannels['PortChannel01'].members %}{{ '%d' % minigraph_port_indices[member]}}{% if not loop.last %} {% endif %}{% endfor %}]{% if not loop.last %} {% endif %}


Can we avoid hard code "PortChannel01" here? It will fail easily if the name of the port channel changes.

I investigated it and looks like we cannot avoid hard code if we want to use only one specific port-channel.
In case of using all port-channels (like FIB test does) we can just iterate through the all LAGs and so avoid hard code.
But, as we discussed earlier, we should use only one port-channel as a next-hop to avoid ECMP.

Shall we consider use a j2 template to select one ipv6 portchannel, and keep track of its name to avoid hardcoding in both places?

sihuihan88 · 2017-09-22T00:42:02Z

ansible/roles/test/tasks/bgp_speaker.yml

+- set_fact: addr_family='ipv4'
+  when: addr_family is not defined
+
+- set_fact: mux_ip_1="{% if addr_family=='ipv4' %}{{vlan_ips[1]}}{% else %}{{minigraph_portchannel_interfaces[1]['peer_addr']}}{% endif %}"


Is it possible not to hard code minigraph_portchanel_interfaces[1] here?

Please see above comment.

Can you check whether it's feasible to avoid hardcode portchannel name by using j2 template to select ipv6 portchannel and store the name?

* Fix comments

* msft_github/master: Update README.testbed.Setup.md call test by test case name (sonic-net#362) [testbed] fanout add rootfanout connect when deploying fanout switch (sonic-net#368) [lag_2] enable lag_2 test on t0-116 topology (sonic-net#390) [deployment] add missing quotation marks that breaks (sonic-net#386) Finalize T0-116 topology (sonic-net#381) Directed broadcast Test (sonic-net#377) [change_mac]: Change MAC address formatting (sonic-net#380) add github template helping report issues and submit pull request (sonic-net#378) Fixing logic to identify proper 'port_config.ini' file. (sonic-net#376) [sensors] add sensor definitions for arista 7260/7060 (sonic-net#371) [pfc_wd]: Rework for P2 (sonic-net#363) [test]: Update BGP speaker test (sonic-net#236) Add vm_resument which restore paused VMs (sonic-net#297) Copy acsbase files to ptf before ptf_runner (sonic-net#366) Modify dhcp_servers IPs in order to fix DHCP relay test (sonic-net#364) [templates]: Remove trailing space in t1-lag minigraph template (sonic-net#360) [fastreboot test]: Add "fanout_listener" for MLNX fanout switches (sonic-net#353) patch t0 portchannel name to 15 chars (sonic-net#358) [process_checker]: Add the process_checker playbook (sonic-net#349) fix issue 354 (sonic-net#355) Update build instruction for docker-ptf.gz and docker-sonic-mgmt.gz [upgrade_sonic]: Add upgrade_sonic as a convenience script to upgrade the device with an image (sonic-net#338) [acl test]: Generate switch info file. (sonic-net#346) fix t0 templates for generate minigraph (sonic-net#344) [loganalyzer_end.yml]: Rename expected_matches (sonic-net#345) [minigraph_facts]: Add the missing brace (sonic-net#342) [Arista7260cx3] update topology t0-116 to include port 17 and 19 (sonic-net#340) [fdb test] help switch populate mac table before starting the test (sonic-net#341) [arista7260cx3] include port 18 and 20 as 50G breakout ports (sonic-net#339) Loganalyzer enhancements (sonic-net#321) [plugin]: Adding Dell platform in switch connection plugin (sonic-net#333) modified files to get lag members from minigraph (sonic-net#327) [minigraph] modify script to fit the latest configuration method (sonic-net#331) [minigraph_fact] add port mapping for 5 INGRASYS switches (sonic-net#336) [sensors] add sensor data for 5 INGRASYS switches (sonic-net#335) [minigraph-template]: Add ACL L3 and mirroring tables to minigraph template. (sonic-net#330) [acltb-test]: Addopt ACL test to acl-loader. (sonic-net#299) [everflow test]: Addopt Everflow testbed test to acl-loader. (sonic-net#300) Remove hard-coded bgp speaker asn number, fetch from DUT (sonic-net#329) Port alias mapping added for Arista-7060CX-32S-C32 (sonic-net#328) Adding if case for S6100 platform to get port details (sonic-net#314) [interface_facts] improve interface facts checking performance (sonic-net#326) [fdb] Hard code port layout to enable fdb test on t0-116 topology (sonic-net#322) (sonic-net#320) [fib] Hard code port layout to enable fib test on t0-116 topology (sonic-net#322) [fib, fdb] Allow fib and fdb test info files to contain empty lines and comments (sonic-net#323) [lag_test]: Correct typo error for lag tests (sonic-net#325) [link flap test] improve performance of link flap test (sonic-net#319)

…hold based buffer allocation (sonic-net#236) Cherry-pick (sonic-net#16885) What is the motivation for this PR? Verifying HeadroomPoolsize test for broadcom-dnx chassis How did you verify/test it? Executed qos test for headroomPool size and verify the results

cb3ddf5 [pmon][xcvrd] xcvrd process show backtrace on the internal port. Port PR233 (sonic-net#236) 5b4c9e1 Fix python wheels path downloaded from vs official build. (sonic-net#244)

…utomatically (sonic-net#22570) #### Why I did it src/sonic-host-services ``` * 9082af7 - (HEAD -> 202405, origin/202405) [kdump-config] update kdump default config with pre-defined value in /proc/cmdline (installer.conf) (cherry-pick from master to 202405) (sonic-net#236) (4 days ago) [Marty Y. Lok] ``` #### How I did it #### How to verify it #### Description for the changelog

[test]: Update BGP speaker test

2b60a19

* Add logic to test BGP-MP feature * Add logic to test traffic trough the advertised routes

msftclas added the cla-already-signed label Aug 3, 2017

stcheng requested a review from sihuihan88 August 3, 2017 18:10

stcheng added the Enhancement label Aug 3, 2017

sihuihan88 previously requested changes Aug 16, 2017

View reviewed changes

lguohan reviewed Aug 25, 2017

View reviewed changes

[test]: Update BGP speaker test

ce7735e

* Fix comments

[test]: Update BGP speaker test

7f529e5

* Fix comments

sihuihan88 reviewed Sep 22, 2017

View reviewed changes

msftgits removed the cla-already-signed label Sep 26, 2017

sonic-net deleted a comment from msftclas Sep 27, 2017

[test]: Update BGP speaker test

ab40a6a

* Fix comments

sihuihan88 approved these changes Nov 20, 2017

View reviewed changes

Merge branch 'master' into master

6f0fa6c

lguohan merged commit fff94a3 into sonic-net:master Nov 23, 2017

		@@ -0,0 +1,8 @@
		{% if addr_family == 'ipv6' %}
		{{announce_prefix}} [{% for member in minigraph_portchannels['PortChannel01'].members %}{{ '%d' % minigraph_port_indices[member]}}{% if not loop.last %} {% endif %}{% endfor %}]{% if not loop.last %} {% endif %}

Conversation

volodymyrsamotiy commented Aug 3, 2017

Uh oh!

lguohan commented Aug 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sihuihan88 Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan commented Sep 11, 2017

Uh oh!

sihuihan88 commented Sep 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sihuihan88 Sep 1, 2017 •

edited

Loading

lguohan Aug 25, 2017 •

edited

Loading

sihuihan88 commented Sep 11, 2017 •

edited

Loading