bgpd, topotests: apply route-map after rt vpn export

A route-map can be programmed to remove the route-target which
has been set with 'rt vpn export' command, but fails to remove
it.

Fix this by applying the route-map, then considering the resulting
extended community-list.
Add some tests to catch this issue.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index d6dc4860ae59628e9c68f2ec98f9c0487999545e..1e25e1b6a7009ffd2a20b368f84089b0eb35bfc7 100644 (file)
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -1573,32 +1573,6 @@ void vpn_leak_from_vrf_update(struct bgp *to_bgp,             /* to */
        /* shallow copy */
        static_attr = *path_vrf->attr;
 
-       /*
-        * route map handling
-        */
-       if (from_bgp->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_TOVPN]) {
-               struct bgp_path_info info;
-               route_map_result_t ret;
-
-               memset(&info, 0, sizeof(info));
-               info.peer = to_bgp->peer_self;
-               info.attr = &static_attr;
-               ret = route_map_apply(from_bgp->vpn_policy[afi]
-                                             .rmap[BGP_VPN_POLICY_DIR_TOVPN],
-                                     p, &info);
-               if (RMAP_DENYMATCH == ret) {
-                       bgp_attr_flush(&static_attr); /* free any added parts */
-                       if (debug)
-                               zlog_debug(
-                                       "%s: vrf %s route map \"%s\" says DENY, returning",
-                                       __func__, from_bgp->name_pretty,
-                                       from_bgp->vpn_policy[afi]
-                                               .rmap[BGP_VPN_POLICY_DIR_TOVPN]
-                                               ->name);
-                       return;
-               }
-       }
-
        if (debug && bgp_attr_get_ecommunity(&static_attr)) {
                char *s = ecommunity_ecom2str(
                        bgp_attr_get_ecommunity(&static_attr),
@@ -1629,12 +1603,37 @@ void vpn_leak_from_vrf_update(struct bgp *to_bgp,            /* to */
        } else if (rtlist_ecom) {
                new_ecom = ecommunity_dup(rtlist_ecom);
        } else {
-               if (debug)
-                       zlog_debug("%s: %s skipping: waiting for a non empty export rt list.",
-                                  __func__, from_bgp->name_pretty);
-               return;
+               new_ecom = NULL;
        }
 
+       bgp_attr_set_ecommunity(&static_attr, new_ecom);
+
+       /*
+        * route map handling
+        */
+       if (from_bgp->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_TOVPN]) {
+               struct bgp_path_info info;
+               route_map_result_t ret;
+
+               memset(&info, 0, sizeof(info));
+               info.peer = to_bgp->peer_self;
+               info.attr = &static_attr;
+               ret = route_map_apply(from_bgp->vpn_policy[afi]
+                                             .rmap[BGP_VPN_POLICY_DIR_TOVPN],
+                                     p, &info);
+               if (RMAP_DENYMATCH == ret) {
+                       bgp_attr_flush(&static_attr); /* free any added parts */
+                       if (debug)
+                               zlog_debug("%s: vrf %s route map \"%s\" says DENY, returning",
+                                          __func__, from_bgp->name_pretty,
+                                          from_bgp->vpn_policy[afi]
+                                                  .rmap[BGP_VPN_POLICY_DIR_TOVPN]
+                                                  ->name);
+                       return;
+               }
+       }
+
+       new_ecom = bgp_attr_get_ecommunity(&static_attr);
        if (!ecommunity_has_route_target(new_ecom)) {
                ecommunity_free(&new_ecom);
                if (debug)
@@ -1643,8 +1642,6 @@ void vpn_leak_from_vrf_update(struct bgp *to_bgp,      /* to */
                return;
        }
 
-       bgp_attr_set_ecommunity(&static_attr, new_ecom);
-
        if (debug && bgp_attr_get_ecommunity(&static_attr)) {
                char *s = ecommunity_ecom2str(
                        bgp_attr_get_ecommunity(&static_attr),

diff --git a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py
index 57b9f307a90fc97e4c16867d6a4c36c85710742f..8b2e674aadf33eb056f1a52df275a6274d94a680 100644 (file)
--- a/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py
+++ b/tests/topotests/bgp_vpnv4_ebgp/test_bgp_vpnv4_ebgp.py
@@ -299,6 +299,83 @@ def test_export_route_target_with_routemap_without_export_route_target():
     assert success, "{}, vpnv4 update {} still present".format(router.name, prefix)
 
 
+def test_export_route_target_with_default_command():
+    """
+    Add back route target with 'rt vpn export' command
+    """
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+    router = tgen.gears["r1"]
+    logger.info("r1, detach route-map and re-add route target vpn export")
+    router.vtysh_cmd(
+        "configure terminal\nrouter bgp 65500 vrf vrf1\naddress-family ipv4 unicast\nrt vpn export 52:100\n"
+    )
+    prefix = "172.31.0.1/32"
+    logger.info("r1, check that exported prefix {} is added back".format(prefix))
+    test_func = partial(
+        check_show_bgp_vpn_prefix_found,
+        router,
+        "ipv4",
+        prefix,
+        "444:1",
+    )
+    success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert success, "{}, vpnv4 update {} still not present".format(router.name, prefix)
+
+
+def test_export_suppress_route_target_with_route_map_command():
+    """
+    Add back route target with 'rt vpn export' command
+    """
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+    router = tgen.gears["r1"]
+    logger.info("r1, add an extended comm-list to delete 52:100")
+
+    router.vtysh_cmd("configure terminal\nbgp extcommunity-list 1 permit rt 52:100\n")
+    router.vtysh_cmd(
+        "configure terminal\nroute-map rmap permit 1\nset extended-comm-list 1 delete\n"
+    )
+    prefix = "172.31.0.1/32"
+    logger.info("r1, check that exported prefix {} is removed".format(prefix))
+    test_func = partial(
+        check_show_bgp_vpn_prefix_not_found,
+        router,
+        "ipv4",
+        prefix,
+        "444:1",
+    )
+    success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert success, "{}, vpnv4 update {} still present".format(router.name, prefix)
+
+
+def test_export_add_route_target_to_route_map_command():
+    """
+    Add route target with route-map so that route is added back
+    """
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+    router = tgen.gears["r1"]
+    logger.info("r1, add an additional set extcommunity 52:101")
+    router.vtysh_cmd(
+        "configure terminal\nroute-map rmap permit 1\nset extcommunity rt 52:101\n"
+    )
+    prefix = "172.31.0.1/32"
+    logger.info("r1, check that exported prefix {} is added back".format(prefix))
+    test_func = partial(
+        check_show_bgp_vpn_prefix_found,
+        router,
+        "ipv4",
+        prefix,
+        "444:1",
+    )
+    success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert success, "{}, vpnv4 update {} still not present".format(router.name, prefix)
+
+
 def test_memory_leak():
     "Run the memory leak test and report results."
     tgen = get_topogen()