Update of BGPVPN Route Targets breaks connectivity

[Łukasz Rajewski]

Setting or unsetting import/export targets breaks BGPVPN connectivity event thought that its properties are valid after update.

Scenario

From Horizon:
1. Create Network A (10.100.1.0/24) and B (10.100.2.0/24)
2. Create server 1 in network A
3. Create server 2 in network B
4. Create L3 BGPVPN with RT=64512:1
5. Associate Network A and B to created BGPVPN <bgpvpn_id>
6. SSH to Server 1
7. Ping from Server 1 to Server 2 works (OK)
With Openstack CLI:
8. bgpvpn set --export-target '64512:2' --import-target '64512:1' <bgpvpn_id>
9. Ping from Server 1 to Server 2 does not work (OK/NOT OK)
10. bgpvpn unset --export-target '64512:2' --import-target '64512:1' <bgpvpn_id>
11. Ping from Server 1 to Server 2 does not work (NOT OK)

[Thomas Morin (tmmorin-orange)]

Łukasz, if you build a tempest scenario for this it will be doubly useful: everyone will have tempest job logs to look at and diagnose (without having to reproduce locally), and we will have a basis upon which checking that the bug is solved.

[Łukasz Rajewski (rajewluk)]

Tempest tests to reproduce that bug can be found in following change:

https://review.openstack.org/#/c/504198/

3 tests with update of route targtes can be found in this change.

One test fails always (test_bgpvpn_route_targets_update_common_target) and other fail randomly. Moreover, also tests already existing before sometimes fail because of update of BGPVPN (even though they do not do any BGPBVPN update). That randomicity must be related with order of tests execution. After first update of BGPVPN something wrong must happen that breaks connectivity.

Observed both lack of ping e.g. in test test_bgpvpn_route_targets_update_common_target as well as unexpected ping response in test test_bgpvpn_negative_disjoint_import_export (from change https://review.openstack.org/#/c/501714/)

[Thomas Morin (tmmorin-orange)]

I have found out the place where the issue happens: the update of RTs results in a route update, that (wrongly) translates to adding state for the prefix in the dataplane then removing it.

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.267 DEBUG networking_bagpipe.bagpipe_bgp.vpn.ipvpn.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.VRF method _on_event called with arguments ([RouteEvent(replaces one route): Advertise [RouteEntry: ipv4/mpls-vpn 10.100.2.20/32 label [ 127 ] next-hop 192.168.122.192 rd 192.168.122.192:130 nh:192.168.122.192 extended-community [ target:64512:1 target:64512:2 encap:GRE ] from:VRF-2] VRF-2],) {} from (pid=8316) wrapper /opt/stack/networking-bagpipe/networking_bagpipe/bagpipe_bgp/common/log_decorator.py:39

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.268 DEBUG networking_bagpipe.bagpipe_bgp.vpn.ipvpn.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.VRF method _new_best_route called with arguments ('10.100.2.20/32', [RouteEntry: ipv4/mpls-vpn 10.100.2.20/32 label [ 127 ] next-hop 192.168.122.192 rd 192.168.122.192:130 nh:192.168.122.192 extended-community [ target:64512:1 target:64512:2 encap:GRE ]]) {} from (pid=8316) wrapper /opt/stack/networking-bagpipe/networking_bagpipe/bagpipe_bgp/common/log_decorator.py:39

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.269 INFO networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.MPLSOVSVRFDataplane method setup_dataplane_for_remote_endpoint called with arguments ('10.100.2.20/32', '192.168.122.192', 127, 10.100.2.20/32 label [ 127 ] next-hop 192.168.122.192 rd 192.168.122.192:130, set([encap:GRE]), 0) {}

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.269 DEBUG networking_bagpipe.bagpipe_bgp.vpn.ipvpn.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.VRF method _best_route_removed called with arguments ('10.100.2.20/32', [RouteEntry: ipv4/mpls-vpn 10.100.2.20/32 label [ 127 ] next-hop 192.168.122.192 rd 192.168.122.192:130 nh:192.168.122.192 extended-community [ target:64512:1 encap:GRE ]], False) {} from (pid=8316) wrapper /opt/stack/networking-bagpipe/networking_bagpipe/bagpipe_bgp/common/log_decorator.py:39

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.271 INFO networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.MPLSOVSVRFDataplane method remove_dataplane_for_remote_endpoint called with arguments ('10.100.2.20/32', '192.168.122.192', 127, 10.100.2.20/32 label [ 127 ] next-hop 192.168.122.192 rd 192.168.122.192:130, set([encap:GRE]), 0) {}

Sep 21 09:50:48 tm-devstack-master-01 bagpipe-bgp[8316]: 2017-09-21 09:50:48.271 DEBUG networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.1 [-] networking_bagpipe.bagpipe_bgp.vpn.ipvpn.mpls_ovs_dataplane.MPLSOVSVRFDataplane method _ovs_flow_del called with arguments ('reg0=1,ip,reg1=0,nw_dst=10.100.2.20/32', 3) {'return_flow': True} from (pid=8316) wrapper /opt/stack/networking-bagpipe/networking_bagpipe/bagpipe_bgp/common/log_deco...

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/506707

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/506707
Committed: https://git.openstack.org/cgit/openstack/networking-bagpipe/commit/?id=f6ef1bf62e97499232cc8150d34fdc554d8a6dfd
Submitter: Jenkins
Branch: master

commit f6ef1bf62e97499232cc8150d34fdc554d8a6dfd
Author: Thomas Morin <email address hidden>
Date: Fri Sep 22 12:01:42 2017 +0200

    bagpipe-bgp: fix dataplane updates on some route updates

    When a route event results in a call to best_route_removed and
    would result in removing state for the dataplane, we need to check
    first if there is not, in the best routes for the same destination,
    a route mapping to the same dataplane state.

    Change-Id: I26177cbfbf3658640227270ff6bc399bc62510ec
    Closes-Bug: 1716893

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/networking-bagpipe 8.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/504198
Committed: https://git.openstack.org/cgit/openstack/networking-bgpvpn/commit/?id=ecdc24bf33bd41e0d2485fbe9144ce42f9853da8
Submitter: Zuul
Branch: master

commit ecdc24bf33bd41e0d2485fbe9144ce42f9853da8
Author: Łukasz Rajewski <email address hidden>
Date: Thu Sep 14 19:57:09 2017 +0200

    Tempest tests to check L3 BGPVPN RTs update

    Added complex tempest tests that verify update of L3 BGPVPN route
    targets in different conditions
    - Change of RT to iRT<>eRT and again to only RT
    - Change of RT to, RT with iRT<>eRT and again only RT
    - Change of eRT<>iRT to RT with sequentional network association

    Tests should check that update doesn't break connectivity when route
    targets are defined properly and in opposite, when route targets
    don't assure proper connectivity - it shouldn't be possible

    Related-Bug: 1716893
    Related-Bug: 1719822
    Depends-On: Ia69e63d5e60ab539c40a2de373b5ffe4b46548f8
    Change-Id: Ice15324e704048945e4371f4a7eb2e24744fefd7