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 <[email protected]>
pguibert6WIND · Nov 21, 2023 · fc1177f · fc1177f
1 parent 3dd3b7f
commit fc1177f
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 32 deletions.
diff --git 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
@@ -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()