From 59bbe85d4bb0bf11f9c78aa4e7bee5023bc33400 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Tue, 6 Jun 2023 14:02:42 +0200
Subject: [PATCH 01/13] bgpd: revert no retain backend

Partially revert a486300b26 ("bgpd: implement retain route-target all
behaviour") in order to fix a memory consumption issue in the next
commit.

Fixes: a486300b26 ("bgpd: implement retain route-target all behaviour")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 bgpd/bgp_mplsvpn.c | 20 ++++++++------------
 bgpd/bgp_mplsvpn.h |  2 +-
 bgpd/bgp_route.c   | 19 ++-----------------
 3 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index dc9bd3cff5..447a21b5ea 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -2116,7 +2116,7 @@ static struct bgp *bgp_lookup_by_rd(struct bgp_path_info *bpi,
 	return NULL;
 }
 
-static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
+static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 					  struct bgp *from_bgp, /* from */
 					  struct bgp_path_info *path_vpn,
 					  struct prefix_rd *prd)
@@ -2146,7 +2146,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 				"%s: from vpn (%s) to vrf (%s), skipping: %s",
 				__func__, from_bgp->name_pretty,
 				to_bgp->name_pretty, debugmsg);
-		return false;
+		return;
 	}
 
 	/*
@@ -2173,7 +2173,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 			zlog_debug(
 				"from vpn (%s) to vrf (%s), skipping after no intersection of route targets",
 				from_bgp->name_pretty, to_bgp->name_pretty);
-		return false;
+		return;
 	}
 
 	rd_buf[0] = '\0';
@@ -2190,7 +2190,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 			zlog_debug(
 				"%s: skipping import, match RD (%s) of src VRF (%s) and the prefix (%pFX)",
 				__func__, rd_buf, to_bgp->name_pretty, p);
-		return false;
+		return;
 	}
 
 	if (debug)
@@ -2301,7 +2301,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 					to_bgp->vpn_policy[afi]
 						.rmap[BGP_VPN_POLICY_DIR_FROMVPN]
 						->name);
-			return false;
+			return;
 		}
 		/*
 		 * if route-map changed nexthop, don't nexthop-self on output
@@ -2363,17 +2363,14 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 			 num_labels, src_vrf, &nexthop_orig, nexthop_self_flag,
 			 debug))
 		bgp_dest_unlock_node(bn);
-
-	return true;
 }
 
-bool vpn_leak_to_vrf_update(struct bgp *from_bgp,
+void vpn_leak_to_vrf_update(struct bgp *from_bgp,
 			    struct bgp_path_info *path_vpn,
 			    struct prefix_rd *prd)
 {
 	struct listnode *mnode, *mnnode;
 	struct bgp *bgp;
-	bool leak_success = false;
 
 	int debug = BGP_DEBUG(vpn, VPN_LEAK_TO_VRF);
 
@@ -2385,11 +2382,10 @@ bool vpn_leak_to_vrf_update(struct bgp *from_bgp,
 
 		if (!path_vpn->extra
 		    || path_vpn->extra->bgp_orig != bgp) { /* no loop */
-			leak_success |= vpn_leak_to_vrf_update_onevrf(
-				bgp, from_bgp, path_vpn, prd);
+			vpn_leak_to_vrf_update_onevrf(bgp, from_bgp, path_vpn,
+						      prd);
 		}
 	}
-	return leak_success;
 }
 
 void vpn_leak_to_vrf_withdraw(struct bgp_path_info *path_vpn)
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 75758edcc2..3922fbcb11 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -59,7 +59,7 @@ extern void vpn_leak_to_vrf_withdraw_all(struct bgp *to_bgp, afi_t afi);
 extern void vpn_leak_to_vrf_update_all(struct bgp *to_bgp, struct bgp *from_bgp,
 				       afi_t afi);
 
-extern bool vpn_leak_to_vrf_update(struct bgp *from_bgp,
+extern void vpn_leak_to_vrf_update(struct bgp *from_bgp,
 				   struct bgp_path_info *path_vpn,
 				   struct prefix_rd *prd);
 
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 7737738d28..155aebc5f9 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4026,7 +4026,6 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 	afi_t nh_afi;
 	bool force_evpn_import = false;
 	safi_t orig_safi = safi;
-	bool leak_success = true;
 	int allowas_in = 0;
 
 	if (frrtrace_enabled(frr_bgp, process_update)) {
@@ -4713,7 +4712,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 		}
 		if ((SAFI_MPLS_VPN == safi)
 		    && (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) {
-			leak_success = vpn_leak_to_vrf_update(bgp, pi, prd);
+			vpn_leak_to_vrf_update(bgp, pi, prd);
 		}
 
 #ifdef ENABLE_BGP_VNC
@@ -4728,13 +4727,6 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 					   type, sub_type, NULL);
 		}
 #endif
-		if ((safi == SAFI_MPLS_VPN) &&
-		    !CHECK_FLAG(bgp->af_flags[afi][safi],
-				BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL) &&
-		    !leak_success) {
-			bgp_unlink_nexthop(pi);
-			bgp_path_info_delete(dest, pi);
-		}
 		return;
 	} // End of implicit withdraw
 
@@ -4876,7 +4868,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 	}
 	if ((SAFI_MPLS_VPN == safi)
 	    && (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) {
-		leak_success = vpn_leak_to_vrf_update(bgp, new, prd);
+		vpn_leak_to_vrf_update(bgp, new, prd);
 	}
 #ifdef ENABLE_BGP_VNC
 	if (SAFI_MPLS_VPN == safi) {
@@ -4890,13 +4882,6 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 				   sub_type, NULL);
 	}
 #endif
-	if ((safi == SAFI_MPLS_VPN) &&
-	    !CHECK_FLAG(bgp->af_flags[afi][safi],
-			BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL) &&
-	    !leak_success) {
-		bgp_unlink_nexthop(new);
-		bgp_path_info_delete(dest, new);
-	}
 
 	return;
 

From 3cc70b02a9e19a364f3144031f27f3e50f128c0c Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Tue, 6 Jun 2023 12:46:21 +0200
Subject: [PATCH 02/13] bgpd: fix memory usage of vpn no retain

By default, bgpd stores all MPLS VPN SAFI prefixes unless the "no bgp
retain route-target all" option is used to store only prefixes that are
imported into local VRFs. The "no retain" option temporarily uses too
much memory, as all prefixes are stored in memory before the deletion of
non-imported prefixes is done.

Filter out non-imported prefixes before they are set into the BGP adj
RIB out.

Fixes: a486300b26 ("bgpd: implement retain route-target all behaviour")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 bgpd/bgp_mplsvpn.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 bgpd/bgp_mplsvpn.h |  4 ++++
 bgpd/bgp_route.c   | 10 ++++++++++
 3 files changed, 60 insertions(+)

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 447a21b5ea..c6c1ef82cd 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -2365,6 +2365,52 @@ static void vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp,   /* to */
 		bgp_dest_unlock_node(bn);
 }
 
+bool vpn_leak_to_vrf_no_retain_filter_check(struct bgp *from_bgp,
+					    struct attr *attr, afi_t afi)
+{
+	struct ecommunity *ecom_route_target = bgp_attr_get_ecommunity(attr);
+	int debug = BGP_DEBUG(vpn, VPN_LEAK_TO_VRF);
+	struct listnode *node;
+	const char *debugmsg;
+	struct bgp *to_bgp;
+
+	/* Loop over BGP instances */
+	for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, to_bgp)) {
+		if (!vpn_leak_from_vpn_active(to_bgp, afi, &debugmsg)) {
+			if (debug)
+				zlog_debug(
+					"%s: from vpn (%s) to vrf (%s) afi %s, skipping: %s",
+					__func__, from_bgp->name_pretty,
+					to_bgp->name_pretty, afi2str(afi),
+					debugmsg);
+			continue;
+		}
+
+		/* Check for intersection of route targets */
+		if (!ecommunity_include(
+			    to_bgp->vpn_policy[afi]
+				    .rtlist[BGP_VPN_POLICY_DIR_FROMVPN],
+			    ecom_route_target)) {
+			if (debug)
+				zlog_debug(
+					"%s: from vpn (%s) to vrf (%s) afi %s %s, skipping after no intersection of route targets",
+					__func__, from_bgp->name_pretty,
+					to_bgp->name_pretty, afi2str(afi),
+					ecommunity_str(ecom_route_target));
+			continue;
+		}
+		return false;
+	}
+
+	if (debug)
+		zlog_debug(
+			"%s: from vpn (%s) afi %s %s, no import - must be filtered",
+			__func__, from_bgp->name_pretty, afi2str(afi),
+			ecommunity_str(ecom_route_target));
+
+	return true;
+}
+
 void vpn_leak_to_vrf_update(struct bgp *from_bgp,
 			    struct bgp_path_info *path_vpn,
 			    struct prefix_rd *prd)
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 3922fbcb11..5d9c9f6b6b 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -59,6 +59,10 @@ extern void vpn_leak_to_vrf_withdraw_all(struct bgp *to_bgp, afi_t afi);
 extern void vpn_leak_to_vrf_update_all(struct bgp *to_bgp, struct bgp *from_bgp,
 				       afi_t afi);
 
+extern bool vpn_leak_to_vrf_no_retain_filter_check(struct bgp *from_bgp,
+						   struct attr *attr,
+						   afi_t afi);
+
 extern void vpn_leak_to_vrf_update(struct bgp *from_bgp,
 				   struct bgp_path_info *path_vpn,
 				   struct prefix_rd *prd);
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 155aebc5f9..5285a221e2 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4169,6 +4169,16 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 		goto filtered;
 	}
 
+	if ((afi == AFI_IP || afi == AFI_IP6) && safi == SAFI_MPLS_VPN &&
+	    bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT &&
+	    !CHECK_FLAG(bgp->af_flags[afi][safi],
+			BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL) &&
+	    vpn_leak_to_vrf_no_retain_filter_check(bgp, attr, afi)) {
+		reason =
+			"no import. Filtered by no bgp retain route-target all";
+		goto filtered;
+	}
+
 	/* If the route has Node Target Extended Communities, check
 	 * if it's allowed to be installed locally.
 	 */

From af79038c4be1b5cec7da9f67b81280d6f14e2479 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Wed, 7 Jun 2023 12:46:30 +0200
Subject: [PATCH 03/13] bgpd: cleanup un-imported vpn prefix if no retain set

After some VRF imports are removed and "no bgp retain route-target all"
is set, prefixes that are not imported anymore remain in the BGP table.

Parse the BGP table and remove un-imported prefixes in such a case.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 bgpd/bgp_mplsvpn.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 bgpd/bgp_mplsvpn.h |  3 +++
 bgpd/bgp_vty.c     |  9 ++++++--
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index c6c1ef82cd..ea82591ced 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -2551,6 +2551,51 @@ void vpn_leak_to_vrf_withdraw_all(struct bgp *to_bgp, afi_t afi)
 	}
 }
 
+void vpn_leak_no_retain(struct bgp *to_bgp, struct bgp *vpn_from, afi_t afi)
+{
+	struct bgp_dest *pdest;
+	safi_t safi = SAFI_MPLS_VPN;
+
+	assert(vpn_from);
+
+	/*
+	 * Walk vpn table
+	 */
+	for (pdest = bgp_table_top(vpn_from->rib[afi][safi]); pdest;
+	     pdest = bgp_route_next(pdest)) {
+		struct bgp_table *table;
+		struct bgp_dest *bn;
+		struct bgp_path_info *bpi;
+
+		/* This is the per-RD table of prefixes */
+		table = bgp_dest_get_bgp_table_info(pdest);
+
+		if (!table)
+			continue;
+
+		for (bn = bgp_table_top(table); bn; bn = bgp_route_next(bn)) {
+			for (bpi = bgp_dest_get_bgp_path_info(bn); bpi;
+			     bpi = bpi->next) {
+
+				if (bpi->extra &&
+				    bpi->extra->bgp_orig == to_bgp)
+					continue;
+
+				if (bpi->sub_type != BGP_ROUTE_NORMAL)
+					continue;
+
+				if (!vpn_leak_to_vrf_no_retain_filter_check(
+					    vpn_from, bpi->attr, afi))
+					/* do not filter */
+					continue;
+
+				bgp_unlink_nexthop(bpi);
+				bgp_rib_remove(bn, bpi, bpi->peer, afi, safi);
+			}
+		}
+	}
+}
+
 void vpn_leak_to_vrf_update_all(struct bgp *to_bgp, struct bgp *vpn_from,
 				afi_t afi)
 {
@@ -3755,6 +3800,7 @@ void vpn_leak_postchange_all(void)
  */
 void bgp_vpn_leak_unimport(struct bgp *from_bgp)
 {
+	struct bgp *bgp_default = bgp_get_default();
 	struct bgp *to_bgp;
 	const char *tmp_name;
 	char *vname;
@@ -3833,6 +3879,17 @@ void bgp_vpn_leak_unimport(struct bgp *from_bgp)
 				}
 			}
 		}
+
+		if (bgp_default &&
+		    !CHECK_FLAG(bgp_default->af_flags[afi][SAFI_MPLS_VPN],
+				BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL)) {
+			/* 'from_bgp' instance will be deleted
+			 * so force to unset importation to update VPN labels
+			 */
+			UNSET_FLAG(from_bgp->af_flags[afi][SAFI_UNICAST],
+				   BGP_CONFIG_MPLSVPN_TO_VRF_IMPORT);
+			vpn_leak_no_retain(from_bgp, bgp_default, afi);
+		}
 	}
 	return;
 }
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 5d9c9f6b6b..23458caf53 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -56,6 +56,9 @@ extern void vpn_leak_from_vrf_update_all(struct bgp *to_bgp,
 
 extern void vpn_leak_to_vrf_withdraw_all(struct bgp *to_bgp, afi_t afi);
 
+extern void vpn_leak_no_retain(struct bgp *to_bgp, struct bgp *vpn_from,
+			       afi_t afi);
+
 extern void vpn_leak_to_vrf_update_all(struct bgp *to_bgp, struct bgp *from_bgp,
 				       afi_t afi);
 
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 6d7b745713..7b75c96e2f 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -9954,6 +9954,7 @@ DEFPY (bgp_imexport_vpn,
 	bool yes = true;
 	int flag;
 	enum vpn_policy_direction dir;
+	struct bgp *bgp_default = bgp_get_default();
 
 	if (argv_find(argv, argc, "no", &idx))
 		yes = false;
@@ -9989,14 +9990,18 @@ DEFPY (bgp_imexport_vpn,
 		SET_FLAG(bgp->af_flags[afi][safi], flag);
 		if (!previous_state) {
 			/* trigger export current vrf */
-			vpn_leak_postchange(dir, afi, bgp_get_default(), bgp);
+			vpn_leak_postchange(dir, afi, bgp_default, bgp);
 		}
 	} else {
 		if (previous_state) {
 			/* trigger un-export current vrf */
-			vpn_leak_prechange(dir, afi, bgp_get_default(), bgp);
+			vpn_leak_prechange(dir, afi, bgp_default, bgp);
 		}
 		UNSET_FLAG(bgp->af_flags[afi][safi], flag);
+		if (previous_state && bgp_default &&
+		    !CHECK_FLAG(bgp_default->af_flags[afi][SAFI_MPLS_VPN],
+				BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL))
+			vpn_leak_no_retain(bgp, bgp_default, afi);
 	}
 
 	hook_call(bgp_snmp_init_stats, bgp);

From a04d32b36649c0e14c76ec121d17a46d95db5697 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Wed, 7 Jun 2023 15:15:30 +0200
Subject: [PATCH 04/13] bgpd: fix incorrect json output in bgp_show_table_rd()

'{}' extra output may present JSON output from bgp_show_table_rd() when
no prefix are seen.

> {
>  "vrfId": 0,
>  "vrfName": "default",
>  "tableVersion": 0,
>  "routerId": "1.1.1.1",
>  "defaultLocPrf": 100,
>  "localAS": 65500,
>  "routes": {  "routeDistinguishers" : { "444:1" : {  }  }  }  }
> {}

Do not output '{}' when bgp_show_table() returns a valid JSON.
Tested without rd in config, bgp_show_table() returns nothing and the
JSON output is only '{}'

Fixes: 0224b3296c ("bgpd: Print empty JSON `{}` if no entries under `show bgp ipv4 vpn json`")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 bgpd/bgp_route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 5285a221e2..058f57177a 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -11783,7 +11783,7 @@ int bgp_show_table_rd(struct vty *vty, struct bgp *bgp, safi_t safi,
 				"\nDisplayed  %ld routes and %ld total paths\n",
 				output_cum, total_cum);
 	} else {
-		if (use_json && output_cum == 0)
+		if (use_json && output_cum == 0 && json_header_depth == 0)
 			vty_out(vty, "{}\n");
 	}
 	return CMD_SUCCESS;

From c2435c67f4a431d6ad9e52a7c8aa7ad2105f5b19 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Tue, 6 Jun 2023 17:00:18 +0200
Subject: [PATCH 05/13] topotests: cosmetic changes to bgp_vpnv4_noretain

Cosmetic changes to bgp_vpnv4_noretain to prepare the next commit.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 ...filtered.json => ipv4_vpn_routes_all.json} |  0
 ...on => ipv4_vpn_routes_no_retain_init.json} |  0
 .../test_bgp_vpnv4_noretain.py                | 81 ++++++++-----------
 3 files changed, 35 insertions(+), 46 deletions(-)
 rename tests/topotests/bgp_vpnv4_noretain/r1/{ipv4_vpn_routes_unfiltered.json => ipv4_vpn_routes_all.json} (100%)
 rename tests/topotests/bgp_vpnv4_noretain/r1/{ipv4_vpn_routes.json => ipv4_vpn_routes_no_retain_init.json} (100%)

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_unfiltered.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
similarity index 100%
rename from tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_unfiltered.json
rename to tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
similarity index 100%
rename from tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes.json
rename to tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index 9b8ae4b7e3..af6e47ab78 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -139,67 +139,56 @@ def router_json_cmp_exact_filter(router, cmd, expected):
     return topotest.json_cmp(json_output, expected, exact=True)
 
 
-def test_bgp_no_retain():
+def check_show_bgp_ipv4_vpn(rname, json_file):
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+    router = tgen.gears[rname]
+
+    logger.info("Checking VPNv4 routes for convergence on {}".format(rname))
+
+    json_file = "{}/{}/{}".format(CWD, router.name, json_file)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        router_json_cmp_exact_filter,
+        router,
+        "show bgp ipv4 vpn json",
+        expected,
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assertmsg = '"{}" JSON output mismatches'.format(router.name)
+    assert result is None, assertmsg
+
+
+def test_bgp_no_retain_step1():
     """
     Check bgp no retain route-target all on r1
     """
 
-    tgen = get_topogen()
-    if tgen.routers_have_failure():
-        pytest.skip(tgen.errors)
-
-    # Check IPv4 VPN routing tables on r1
-    logger.info("Checking VPNv4 routes for convergence on r1")
-    router = tgen.gears["r1"]
-    json_file = "{}/{}/ipv4_vpn_routes.json".format(CWD, router.name)
-    if not os.path.isfile(json_file):
-        logger.info("skipping file {}".format(json_file))
-        assert 0, "{} file not found".format(json_file)
-        return
-
-    expected = json.loads(open(json_file).read())
-    test_func = partial(
-        router_json_cmp_exact_filter,
-        router,
-        "show bgp ipv4 vpn json",
-        expected,
-    )
-    _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
-    assertmsg = '"{}" JSON output mismatches'.format(router.name)
-    assert result is None, assertmsg
+    rname = "r1"
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
 
 
-def test_bgp_retain():
+def test_bgp_retain_step2():
     """
     Apply and check bgp retain route-target all on r1
     """
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500
+ address-family ipv4 vpn
+  bgp retain route-target all
+"""
 
     tgen = get_topogen()
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
-    # Check IPv4 VPN routing tables on r1
-    logger.info("Checking VPNv4 routes on r1 after bgp no retain")
-    router = tgen.gears["r1"]
-    router.vtysh_cmd(
-        "configure\nrouter bgp 65500\naddress-family ipv4 vpn\nbgp retain route-target all\n"
-    )
-    json_file = "{}/{}/ipv4_vpn_routes_unfiltered.json".format(CWD, router.name)
-    if not os.path.isfile(json_file):
-        logger.info("skipping file {}".format(json_file))
-        assert 0, "{} file not found".format(json_file)
-        return
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
 
-    expected = json.loads(open(json_file).read())
-    test_func = partial(
-        router_json_cmp_exact_filter,
-        router,
-        "show bgp ipv4 vpn json",
-        expected,
-    )
-    _, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
-    assertmsg = '"{}" JSON output mismatches'.format(router.name)
-    assert result is None, assertmsg
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_all.json")
 
 
 def test_memory_leak():

From bf636d371281f491256d88121cb207553be658bf Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Tue, 6 Jun 2023 17:52:11 +0200
Subject: [PATCH 06/13] topotests: apply style to bgp_vpnv4_noretain

Apply style cosmetic changes to bgp_vpnv4_noretain with "black".

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../test_bgp_vpnv4_noretain.py                | 54 ++++++++++---------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index af6e47ab78..979cebf6ae 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -34,6 +34,7 @@ from lib.topolog import logger
 
 pytestmark = [pytest.mark.bgpd]
 
+
 def build_topo(tgen):
     "Build function"
 
@@ -53,41 +54,42 @@ def build_topo(tgen):
     switch = tgen.add_switch("s4")
     switch.add_link(tgen.gears["r2"])
 
-        
+
 def _populate_iface():
     tgen = get_topogen()
     cmds_list = [
-        'modprobe mpls_router',
-        'echo 100000 > /proc/sys/net/mpls/platform_labels',
-        'ip link add vrf1 type vrf table 10',
-        'ip link set dev vrf1 up',
-        'ip link set dev {0}-eth1 master vrf1',
-        'echo 1 > /proc/sys/net/mpls/conf/vrf1/input',
+        "modprobe mpls_router",
+        "echo 100000 > /proc/sys/net/mpls/platform_labels",
+        "ip link add vrf1 type vrf table 10",
+        "ip link set dev vrf1 up",
+        "ip link set dev {0}-eth1 master vrf1",
+        "echo 1 > /proc/sys/net/mpls/conf/vrf1/input",
     ]
     cmds_list_extra = [
-        'ip link add vrf2 type vrf table 20',
-        'ip link set dev vrf2 up',
-        'ip link set dev {0}-eth2 master vrf2',
-        'echo 1 > /proc/sys/net/mpls/conf/vrf2/input',
+        "ip link add vrf2 type vrf table 20",
+        "ip link set dev vrf2 up",
+        "ip link set dev {0}-eth2 master vrf2",
+        "echo 1 > /proc/sys/net/mpls/conf/vrf2/input",
     ]
-    
-    for cmd in cmds_list:
-        input = cmd.format('r1', '1', '2')
-        logger.info('input: ' + cmd)
-        output = tgen.net['r1'].cmd(cmd.format('r1', '1', '2'))
-        logger.info('output: ' + output)
 
     for cmd in cmds_list:
-        input = cmd.format('r2', '2', '1')
-        logger.info('input: ' + cmd)
-        output = tgen.net['r2'].cmd(cmd.format('r2', '2', '1'))
-        logger.info('output: ' + output)
+        input = cmd.format("r1", "1", "2")
+        logger.info("input: " + cmd)
+        output = tgen.net["r1"].cmd(cmd.format("r1", "1", "2"))
+        logger.info("output: " + output)
+
+    for cmd in cmds_list:
+        input = cmd.format("r2", "2", "1")
+        logger.info("input: " + cmd)
+        output = tgen.net["r2"].cmd(cmd.format("r2", "2", "1"))
+        logger.info("output: " + output)
 
     for cmd in cmds_list_extra:
-        input = cmd.format('r2', '2', '1')
-        logger.info('input: ' + cmd)
-        output = tgen.net['r2'].cmd(cmd.format('r2', '2', '1'))
-        logger.info('output: ' + output)
+        input = cmd.format("r2", "2", "1")
+        logger.info("input: " + cmd)
+        output = tgen.net["r2"].cmd(cmd.format("r2", "2", "1"))
+        logger.info("output: " + output)
+
 
 def setup_module(mod):
     "Sets up the pytest environment"
@@ -96,7 +98,7 @@ def setup_module(mod):
     tgen.start_topology()
 
     router_list = tgen.routers()
-    _populate_iface() 
+    _populate_iface()
 
     for rname, router in router_list.items():
         router.load_config(

From fe993cb25ff2498e33102f9922b313e33f46a566 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 12:27:18 +0200
Subject: [PATCH 07/13] topotests: router relative variables in
 bgp_vpnv4_noretain

Change IP addresses, label, RD and RT to match the router number
and VRF number.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../topotests/bgp_vpnv4_noretain/r1/bgpd.conf | 10 +++----
 .../r1/ipv4_vpn_routes_all.json               | 26 +++++++++----------
 .../r1/ipv4_vpn_routes_no_retain_init.json    | 20 +++++++-------
 .../bgp_vpnv4_noretain/r1/isisd.conf          |  2 +-
 .../bgp_vpnv4_noretain/r1/zebra.conf          |  8 +++---
 .../topotests/bgp_vpnv4_noretain/r2/bgpd.conf | 22 ++++++++--------
 .../bgp_vpnv4_noretain/r2/isisd.conf          |  2 +-
 .../bgp_vpnv4_noretain/r2/zebra.conf          | 14 +++++-----
 8 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
index 3d8773b8bf..ba68e61ddf 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
@@ -1,5 +1,5 @@
 router bgp 65500
- bgp router-id 1.1.1.1
+ bgp router-id 192.0.2.1
  neighbor 10.125.0.2 remote-as 65500
  address-family ipv4 unicast
   no neighbor 10.125.0.2 activate
@@ -10,13 +10,13 @@ router bgp 65500
  exit-address-family
 !
 router bgp 65500 vrf vrf1
- bgp router-id 1.1.1.1
+ bgp router-id 192.0.2.1
  address-family ipv4 unicast
   redistribute connected
   label vpn export 101
-  rd vpn export 444:1
-  rt vpn import 51:100 52:100
-  rt vpn export 51:100
+  rd vpn export 192.0.2.1:1
+  rt vpn import 192.0.2.1:100 192.0.2.2:100
+  rt vpn export 192.0.2.1:100
   export vpn
   import vpn
  exit-address-family
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
index 45f4acce6f..1926e86a8c 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
@@ -1,21 +1,21 @@
 {
    "vrfId":0,
    "vrfName":"default",
-   "routerId":"1.1.1.1",
+   "routerId":"192.0.2.1",
    "defaultLocPrf":100,
    "localAS":65500,
    "routes":{
       "routeDistinguishers":{
-         "444:1":{
-            "10.201.0.0/24":[
+         "192.0.2.1:1":{
+            "10.101.0.0/24":[
                {
                   "valid":true,
                   "bestpath":true,
                   "selectionReason":"First path received",
                   "pathFrom":"external",
-                  "prefix":"10.201.0.0",
+                  "prefix":"10.101.0.0",
                   "prefixLen":24,
-                  "network":"10.201.0.0\/24",
+                  "network":"10.101.0.0\/24",
                   "metric":0,
                   "weight":32768,
                   "peerId":"(unspec)",
@@ -34,16 +34,16 @@
                }
             ]
          },
-         "444:2":{
-            "10.200.0.0/24":[
+         "192.0.2.2:1":{
+            "10.201.0.0/24":[
                {
                   "valid":true,
                   "bestpath":true,
                   "selectionReason":"First path received",
                   "pathFrom":"internal",
-                  "prefix":"10.200.0.0",
+                  "prefix":"10.201.0.0",
                   "prefixLen":24,
-                  "network":"10.200.0.0\/24",
+                  "network":"10.201.0.0\/24",
                   "metric":0,
                   "locPrf":100,
                   "weight":0,
@@ -61,16 +61,16 @@
                }
             ]
          },
-         "444:3":{
-            "10.210.0.0/24":[
+         "192.0.2.2:2":{
+            "10.202.0.0/24":[
                {
                   "valid":true,
                   "bestpath":true,
                   "selectionReason":"First path received",
                   "pathFrom":"internal",
-                  "prefix":"10.210.0.0",
+                  "prefix":"10.202.0.0",
                   "prefixLen":24,
-                  "network":"10.210.0.0\/24",
+                  "network":"10.202.0.0\/24",
                   "metric":0,
                   "locPrf":100,
                   "weight":0,
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
index 28e153e3de..261dec9f9f 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
@@ -1,21 +1,21 @@
 {
    "vrfId":0,
    "vrfName":"default",
-   "routerId":"1.1.1.1",
+   "routerId":"192.0.2.1",
    "defaultLocPrf":100,
    "localAS":65500,
    "routes":{
       "routeDistinguishers":{
-         "444:1":{
-            "10.201.0.0/24":[
+         "192.0.2.1:1":{
+            "10.101.0.0/24":[
                {
                   "valid":true,
                   "bestpath":true,
                   "selectionReason":"First path received",
                   "pathFrom":"external",
-                  "prefix":"10.201.0.0",
+                  "prefix":"10.101.0.0",
                   "prefixLen":24,
-                  "network":"10.201.0.0\/24",
+                  "network":"10.101.0.0\/24",
                   "metric":0,
                   "weight":32768,
                   "peerId":"(unspec)",
@@ -34,16 +34,16 @@
                }
             ]
          },
-         "444:2":{
-            "10.200.0.0/24":[
+         "192.0.2.2:1":{
+            "10.201.0.0/24":[
                {
                   "valid":true,
                   "bestpath":true,
                   "selectionReason":"First path received",
                   "pathFrom":"internal",
-                  "prefix":"10.200.0.0",
+                  "prefix":"10.201.0.0",
                   "prefixLen":24,
-                  "network":"10.200.0.0\/24",
+                  "network":"10.201.0.0\/24",
                   "metric":0,
                   "locPrf":100,
                   "weight":0,
@@ -61,7 +61,7 @@
                }
             ]
          },
-         "444:3":{
+         "192.0.2.2:2":{
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/isisd.conf b/tests/topotests/bgp_vpnv4_noretain/r1/isisd.conf
index 6f5cb6ec68..233a6473b3 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/isisd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/isisd.conf
@@ -10,5 +10,5 @@ router isis 1
  is-type level-1
  net 49.0002.0000.1994.00
  segment-routing on
- segment-routing prefix 1.1.1.1/32 index 11
+ segment-routing prefix 192.0.2.1/32 index 11
 !
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf b/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
index 5b8b1e8ffb..4f641e512e 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
@@ -1,13 +1,13 @@
 log stdout
 interface lo
- ip address 1.1.1.1/32
+ ip address 192.0.2.1/32
 !
 interface r1-gre0
  ip address 192.168.0.1/24
 !
-interface r1-eth1 vrf vrf1
- ip address 10.201.0.1/24
-!
 interface r1-eth0
  ip address 10.125.0.1/24
 !
+interface r1-eth1 vrf vrf1
+ ip address 10.101.0.1/24
+!
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
index 235fb31177..13de0e520f 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
@@ -1,5 +1,5 @@
 router bgp 65500
- bgp router-id 2.2.2.2
+ bgp router-id 192.0.2.2
  neighbor 10.125.0.1 remote-as 65500
  address-family ipv4 unicast
   no neighbor 10.125.0.1 activate
@@ -10,25 +10,25 @@ router bgp 65500
  exit-address-family
 !
 router bgp 65500 vrf vrf1
- bgp router-id 2.2.2.2
+ bgp router-id 192.0.2.2
  address-family ipv4 unicast
   redistribute connected
-  label vpn export 102
-  rd vpn export 444:2
-  rt vpn import 53:100 52:100 51:100
-  rt vpn export 52:100
+  label vpn export 201
+  rd vpn export 192.0.2.2:1
+  rt vpn import 192.0.2.1:100 192.0.2.2:100 192.0.2.2:200
+  rt vpn export 192.0.2.2:100
   export vpn
   import vpn
  exit-address-family
 !
 router bgp 65500 vrf vrf2
- bgp router-id 2.2.2.2
+ bgp router-id 192.0.2.2
  address-family ipv4 unicast
   redistribute connected
-  label vpn export 102
-  rd vpn export 444:3
-  rt vpn both 53:100 52:100 51:100
-  rt vpn both 53:100
+  label vpn export 202
+  rd vpn export 192.0.2.2:2
+  rt vpn both 192.0.2.1:100 192.0.2.2:100 192.0.2.2:200
+  rt vpn both 192.0.2.2:200
   export vpn
   import vpn
  exit-address-family
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/isisd.conf b/tests/topotests/bgp_vpnv4_noretain/r2/isisd.conf
index cbec8c3674..547d10f2bc 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/isisd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/isisd.conf
@@ -10,5 +10,5 @@ router isis 1
  is-type level-1
  net 49.0002.0000.1995.00
  segment-routing on
- segment-routing prefix 2.2.2.2/32 index 22
+ segment-routing prefix 192.0.2.2/32 index 22
 !
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
index 7ec644ac2a..72cc10b6c2 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
@@ -1,16 +1,16 @@
 log stdout
 interface lo
- ip address 2.2.2.2/32
+ ip address 192.0.2.2/32
 !
 interface r2-gre0
  ip address 192.168.0.2/24
 !
-interface r2-eth1 vrf vrf1
- ip address 10.200.0.2/24
-!
-interface r2-eth2 vrf vrf2
- ip address 10.210.0.2/24
-!
 interface r2-eth0
  ip address 10.125.0.2/24
 !
+interface r2-eth1 vrf vrf1
+ ip address 10.201.0.2/24
+!
+interface r2-eth2 vrf vrf2
+ ip address 10.202.0.2/24
+!

From a92a2683a316e9029d6f858f961a281a6caf6696 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 12:52:44 +0200
Subject: [PATCH 08/13] topotests: filter out void RD in bgp_vpnv4_noretain

Filter out route-distinguishers that have no prefix in "show bgp table
json"

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../r1/ipv4_vpn_routes_no_retain_init.json                 | 2 --
 .../bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py          | 7 +++++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
index 261dec9f9f..855ae1c2c9 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
@@ -60,8 +60,6 @@
                   ]
                }
             ]
-         },
-         "192.0.2.2:2":{
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index 979cebf6ae..9d48daa588 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -17,6 +17,7 @@ import os
 import sys
 import json
 from functools import partial
+from copy import deepcopy
 import pytest
 
 # Save the Current Working Directory to find configuration files.
@@ -138,6 +139,12 @@ def router_json_cmp_exact_filter(router, cmd, expected):
                 if "version" in attr:
                     attr.pop("version")
 
+    # filter out RD with no data (e.g. "444:3": {})
+    json_tmp = deepcopy(json_output)
+    for rd, data in json_tmp["routes"]["routeDistinguishers"].items():
+        if len(data.keys()) == 0:
+            json_output["routes"]["routeDistinguishers"].pop(rd)
+
     return topotest.json_cmp(json_output, expected, exact=True)
 
 

From 9aa2e5b1cd341f9d2cbf24484bf632193a0f196e Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 14:27:12 +0200
Subject: [PATCH 09/13] topotests: do not import r1 vrf1 to itself
 bgp_vpnv4_noretain

Do not import r1 vrf1 to itself in order to check that r1 vrf1 prefixes
are only in VPN table because it is needed for advertising the prefixes
to other routers.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
index ba68e61ddf..0c5a75c6d7 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
@@ -15,7 +15,7 @@ router bgp 65500 vrf vrf1
   redistribute connected
   label vpn export 101
   rd vpn export 192.0.2.1:1
-  rt vpn import 192.0.2.1:100 192.0.2.2:100
+  rt vpn import 192.0.2.2:100
   rt vpn export 192.0.2.1:100
   export vpn
   import vpn

From 1a2b340892dba73444d121c1fabcbed8305c25e2 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 14:34:22 +0200
Subject: [PATCH 10/13] topotests: check r2 vpn table in bgp_vpnv4_noretain

Remove no retain in r2 and check that r2 VPN table remains the same
after r1 configuration changes.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../topotests/bgp_vpnv4_noretain/r2/bgpd.conf |  5 +-
 .../r2/ipv4_vpn_routes_all.json               | 94 +++++++++++++++++++
 .../test_bgp_vpnv4_noretain.py                |  2 +
 3 files changed, 98 insertions(+), 3 deletions(-)
 create mode 100644 tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json

diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
index 13de0e520f..1ffbfe8b69 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
@@ -6,7 +6,6 @@ router bgp 65500
  exit-address-family
  address-family ipv4 vpn
   neighbor 10.125.0.1 activate
-  no bgp retain route-target all
  exit-address-family
 !
 router bgp 65500 vrf vrf1
@@ -27,8 +26,8 @@ router bgp 65500 vrf vrf2
   redistribute connected
   label vpn export 202
   rd vpn export 192.0.2.2:2
-  rt vpn both 192.0.2.1:100 192.0.2.2:100 192.0.2.2:200
-  rt vpn both 192.0.2.2:200
+  rt vpn import 192.0.2.1:100 192.0.2.2:100 192.0.2.2:200
+  rt vpn export 192.0.2.2:200
   export vpn
   import vpn
  exit-address-family
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
new file mode 100644
index 0000000000..79304a5e55
--- /dev/null
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
@@ -0,0 +1,94 @@
+{
+  "vrfId": 0,
+  "vrfName": "default",
+  "routerId":"192.0.2.2",
+  "defaultLocPrf": 100,
+  "localAS": 65500,
+  "routes": {
+    "routeDistinguishers": {
+      "192.0.2.1:1": {
+        "10.101.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "internal",
+            "prefix": "10.101.0.0",
+            "prefixLen": 24,
+            "network": "10.101.0.0/24",
+            "metric": 0,
+            "locPrf": 100,
+            "weight": 0,
+            "peerId": "10.125.0.1",
+            "path": "",
+            "origin": "incomplete",
+            "nexthops": [
+              {
+                "ip": "10.125.0.1",
+                "hostname": "r1",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
+      },
+      "192.0.2.2:1": {
+        "10.201.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "external",
+            "network": "10.201.0.0/24",
+            "prefixLen": 24,
+            "prefix": "10.201.0.0",
+            "metric": 0,
+            "weight": 32768,
+            "peerId": "(unspec)",
+            "path": "",
+            "origin": "incomplete",
+            "announceNexthopSelf": true,
+            "nhVrfName": "vrf1",
+            "nexthops": [
+              {
+                "ip": "0.0.0.0",
+                "hostname": "r2",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
+      },
+      "192.0.2.2:2": {
+        "10.202.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "external",
+            "network": "10.202.0.0/24",
+            "prefixLen": 24,
+            "prefix": "10.202.0.0",
+            "metric": 0,
+            "weight": 32768,
+            "peerId": "(unspec)",
+            "path": "",
+            "origin": "incomplete",
+            "announceNexthopSelf": true,
+            "nhVrfName": "vrf2",
+            "nexthops": [
+              {
+                "ip": "0.0.0.0",
+                "hostname": "r2",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
+      }
+    }
+  }
+}
diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index 9d48daa588..994582fc62 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -176,6 +176,7 @@ def test_bgp_no_retain_step1():
 
     rname = "r1"
     check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
 
 
 def test_bgp_retain_step2():
@@ -198,6 +199,7 @@ router bgp 65500
     router.vtysh_cmd(cfg)
 
     check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_all.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
 
 
 def test_memory_leak():

From d629c2f41988ac7c9b57415fdc0bcacecab429e0 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Wed, 14 Jun 2023 11:05:01 +0200
Subject: [PATCH 11/13] topotests: check protocol convergence at init in
 bgp_vpnv4_noretain

Check that the BGP session is Established before starting the tests.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../r2/ipv4_vpn_summary.json                  | 17 +++++++++++++
 .../test_bgp_vpnv4_noretain.py                | 24 +++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_summary.json

diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_summary.json b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_summary.json
new file mode 100644
index 0000000000..a4408f1915
--- /dev/null
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_summary.json
@@ -0,0 +1,17 @@
+{
+  "routerId":"192.0.2.2",
+  "as":65500,
+  "vrfId":0,
+  "vrfName":"default",
+  "peerCount":1,
+  "peers":{
+    "10.125.0.1":{
+      "remoteAs":65500,
+      "localAs":65500,
+      "version":4,
+      "state":"Established",
+      "peerState":"OK"
+    }
+  },
+  "totalPeers":1
+}
diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index 994582fc62..94acd62f6d 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -169,6 +169,30 @@ def check_show_bgp_ipv4_vpn(rname, json_file):
     assert result is None, assertmsg
 
 
+def test_protocols_convergence_step0():
+    """
+    Assert that all protocols have converged
+    """
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    # check that r2 peerings are ok
+    logger.info("Checking BGP ipv4 vpn summary for r2")
+    router = tgen.gears["r2"]
+    json_file = "{}/{}/ipv4_vpn_summary.json".format(CWD, router.name)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        topotest.router_json_cmp,
+        router,
+        "show bgp ipv4 vpn summary json",
+        expected,
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=20, wait=0.5)
+    assertmsg = '"{}" JSON output mismatches'.format(router.name)
+    assert result is None, assertmsg
+
+
 def test_bgp_no_retain_step1():
     """
     Check bgp no retain route-target all on r1

From 22dfa04b78d80184af2cd695276947da0f007ff0 Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 13:42:30 +0200
Subject: [PATCH 12/13] topotests: more tests in bgp_vpnv4_noretain

Add more VRFs in bgp_vpnv4_noretain and test combinations of no-retain
/ retain and import / un-import.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../topotests/bgp_vpnv4_noretain/r1/bgpd.conf |  20 +-
 .../r1/ipv4_vpn_routes_all.json               |  55 ++++
 .../r1/ipv4_vpn_routes_no_retain_init.json    |  28 ++
 ...pn_routes_no_retain_init_plus_r2_vrf2.json | 121 ++++++++
 ...pn_routes_no_retain_init_plus_r2_vrf3.json | 121 ++++++++
 .../bgp_vpnv4_noretain/r1/zebra.conf          |   3 +
 .../topotests/bgp_vpnv4_noretain/r2/bgpd.conf |  10 +
 .../r2/ipv4_vpn_routes_all.json               |  55 ++++
 .../bgp_vpnv4_noretain/r2/zebra.conf          |   3 +
 .../test_bgp_vpnv4_noretain.py                | 271 +++++++++++++++++-
 10 files changed, 678 insertions(+), 9 deletions(-)
 create mode 100644 tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
 create mode 100644 tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
index 0c5a75c6d7..a30caf5c9d 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
@@ -21,4 +21,22 @@ router bgp 65500 vrf vrf1
   import vpn
  exit-address-family
 !
-
+router bgp 65500 vrf vrf3
+ bgp router-id 192.0.2.1
+ address-family ipv4 unicast
+  redistribute connected
+  label vpn export 103
+  rd vpn export 192.0.2.1:3
+  rt vpn export 192.0.2.1:300
+  export vpn
+ exit-address-family
+!
+router bgp 65500 vrf vrf4
+ bgp router-id 192.0.2.1
+ address-family ipv4 unicast
+  label vpn export 104
+  rd vpn export 192.0.2.1:4
+  rt vpn import 192.0.2.1:300
+  import vpn
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
index 1926e86a8c..e2fed4ef82 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
@@ -34,6 +34,34 @@
                }
             ]
          },
+         "192.0.2.1:3":{
+            "10.103.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.103.0.0",
+                  "prefixLen":24,
+                  "network":"10.103.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf3",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
          "192.0.2.2:1":{
             "10.201.0.0/24":[
                {
@@ -87,6 +115,33 @@
                   ]
                }
             ]
+         },
+         "192.0.2.2:3":{
+            "10.203.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.203.0.0",
+                  "prefixLen":24,
+                  "network":"10.203.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
index 855ae1c2c9..a6f789282c 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
@@ -34,6 +34,34 @@
                }
             ]
          },
+         "192.0.2.1:3":{
+            "10.103.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.103.0.0",
+                  "prefixLen":24,
+                  "network":"10.103.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf3",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
          "192.0.2.2:1":{
             "10.201.0.0/24":[
                {
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
new file mode 100644
index 0000000000..4877bebb32
--- /dev/null
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
@@ -0,0 +1,121 @@
+{
+   "vrfId":0,
+   "vrfName":"default",
+   "routerId":"192.0.2.1",
+   "defaultLocPrf":100,
+   "localAS":65500,
+   "routes":{
+      "routeDistinguishers":{
+         "192.0.2.1:1":{
+            "10.101.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.101.0.0",
+                  "prefixLen":24,
+                  "network":"10.101.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf1",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.1:3":{
+            "10.103.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.103.0.0",
+                  "prefixLen":24,
+                  "network":"10.103.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf3",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.2:1":{
+            "10.201.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.201.0.0",
+                  "prefixLen":24,
+                  "network":"10.201.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.2:2":{
+            "10.202.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.202.0.0",
+                  "prefixLen":24,
+                  "network":"10.202.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         }
+      }
+   }
+}
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json
new file mode 100644
index 0000000000..f05c12b42f
--- /dev/null
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json
@@ -0,0 +1,121 @@
+{
+   "vrfId":0,
+   "vrfName":"default",
+   "routerId":"192.0.2.1",
+   "defaultLocPrf":100,
+   "localAS":65500,
+   "routes":{
+      "routeDistinguishers":{
+         "192.0.2.1:1":{
+            "10.101.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.101.0.0",
+                  "prefixLen":24,
+                  "network":"10.101.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf1",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.1:3":{
+            "10.103.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"external",
+                  "prefix":"10.103.0.0",
+                  "prefixLen":24,
+                  "network":"10.103.0.0\/24",
+                  "metric":0,
+                  "weight":32768,
+                  "peerId":"(unspec)",
+                  "path":"",
+                  "origin":"incomplete",
+                  "announceNexthopSelf":true,
+                  "nhVrfName":"vrf3",
+                  "nexthops":[
+                     {
+                        "ip":"0.0.0.0",
+                        "hostname":"r1",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.2:1":{
+            "10.201.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.201.0.0",
+                  "prefixLen":24,
+                  "network":"10.201.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         },
+         "192.0.2.2:3":{
+            "10.203.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.203.0.0",
+                  "prefixLen":24,
+                  "network":"10.203.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
+         }
+      }
+   }
+}
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf b/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
index 4f641e512e..f99cfafe32 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/zebra.conf
@@ -11,3 +11,6 @@ interface r1-eth0
 interface r1-eth1 vrf vrf1
  ip address 10.101.0.1/24
 !
+interface r1-eth3 vrf vrf3
+ ip address 10.103.0.1/24
+!
\ No newline at end of file
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
index 1ffbfe8b69..a5c13e0343 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
@@ -32,3 +32,13 @@ router bgp 65500 vrf vrf2
   import vpn
  exit-address-family
 !
+router bgp 65500 vrf vrf3
+ bgp router-id 192.0.2.2
+ address-family ipv4 unicast
+  redistribute connected
+  label vpn export 203
+  rd vpn export 192.0.2.2:3
+  rt vpn export 192.0.2.2:300
+  export vpn
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
index 79304a5e55..f94bea2e22 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
@@ -33,6 +33,33 @@
           }
         ]
       },
+      "192.0.2.1:3": {
+        "10.103.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "internal",
+            "prefix": "10.103.0.0",
+            "prefixLen": 24,
+            "network": "10.103.0.0/24",
+            "metric": 0,
+            "locPrf": 100,
+            "weight": 0,
+            "peerId": "10.125.0.1",
+            "path": "",
+            "origin": "incomplete",
+            "nexthops": [
+              {
+                "ip": "10.125.0.1",
+                "hostname": "r1",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
+      },
       "192.0.2.2:1": {
         "10.201.0.0/24": [
           {
@@ -88,6 +115,34 @@
             ]
           }
         ]
+      },
+      "192.0.2.2:3": {
+        "10.203.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "external",
+            "prefix": "10.203.0.0",
+            "prefixLen": 24,
+            "network": "10.203.0.0/24",
+            "metric": 0,
+            "weight": 32768,
+            "peerId": "(unspec)",
+            "path": "",
+            "origin": "incomplete",
+            "announceNexthopSelf": true,
+            "nhVrfName": "vrf3",
+            "nexthops": [
+              {
+                "ip": "0.0.0.0",
+                "hostname": "r2",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
       }
     }
   }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
index 72cc10b6c2..057a3614c7 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
@@ -14,3 +14,6 @@ interface r2-eth1 vrf vrf1
 interface r2-eth2 vrf vrf2
  ip address 10.202.0.2/24
 !
+interface r2-eth3 vrf vrf3
+ ip address 10.203.0.1/24
+!
diff --git a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
index 94acd62f6d..f665040f7f 100644
--- a/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
+++ b/tests/topotests/bgp_vpnv4_noretain/test_bgp_vpnv4_noretain.py
@@ -55,6 +55,18 @@ def build_topo(tgen):
     switch = tgen.add_switch("s4")
     switch.add_link(tgen.gears["r2"])
 
+    switch = tgen.add_switch("s5")
+    switch.add_link(tgen.gears["r1"])
+
+    switch = tgen.add_switch("s6")
+    switch.add_link(tgen.gears["r2"])
+
+    switch = tgen.add_switch("s7")
+    switch.add_link(tgen.gears["r1"])
+
+    switch = tgen.add_switch("s8")
+    switch.add_link(tgen.gears["r2"])
+
 
 def _populate_iface():
     tgen = get_topogen()
@@ -65,12 +77,18 @@ def _populate_iface():
         "ip link set dev vrf1 up",
         "ip link set dev {0}-eth1 master vrf1",
         "echo 1 > /proc/sys/net/mpls/conf/vrf1/input",
-    ]
-    cmds_list_extra = [
         "ip link add vrf2 type vrf table 20",
         "ip link set dev vrf2 up",
         "ip link set dev {0}-eth2 master vrf2",
         "echo 1 > /proc/sys/net/mpls/conf/vrf2/input",
+        "ip link add vrf3 type vrf table 30",
+        "ip link set dev vrf3 up",
+        "ip link set dev {0}-eth3 master vrf3",
+        "echo 1 > /proc/sys/net/mpls/conf/vrf3/input",
+        "ip link add vrf4 type vrf table 40",
+        "ip link set dev vrf4 up",
+        "ip link set dev {0}-eth4 master vrf4",
+        "echo 1 > /proc/sys/net/mpls/conf/vrf4/input",
     ]
 
     for cmd in cmds_list:
@@ -85,12 +103,6 @@ def _populate_iface():
         output = tgen.net["r2"].cmd(cmd.format("r2", "2", "1"))
         logger.info("output: " + output)
 
-    for cmd in cmds_list_extra:
-        input = cmd.format("r2", "2", "1")
-        logger.info("input: " + cmd)
-        output = tgen.net["r2"].cmd(cmd.format("r2", "2", "1"))
-        logger.info("output: " + output)
-
 
 def setup_module(mod):
     "Sets up the pytest environment"
@@ -226,6 +238,249 @@ router bgp 65500
     check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
 
 
+def test_bgp_no_retain_step3():
+    """
+    Apply and check no bgp retain route-target all on r1
+    """
+    rname = "r1"
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears["r1"]
+    router.vtysh_cmd(
+        "configure\nrouter bgp 65500\naddress-family ipv4 vpn\nno bgp retain route-target all\n"
+    )
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_add_vrf2_step4():
+    """
+    Add vrf2 on r1 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ bgp router-id 192.0.2.1
+ address-family ipv4 unicast
+  redistribute connected
+  label vpn export 102
+  rd vpn export 192.0.2.1:200
+  rt vpn import 192.0.2.2:200
+  import vpn
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_unimport_vrf2_step5():
+    """
+    Unimport to vrf2 on r1 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  no import vpn
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_import_vrf2_step6():
+    """
+    Re-import to vrf2 on r1 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  import vpn
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_import_vrf1_step7():
+    """
+    Import r2 vrf1 into r1 vrf2 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  rt vpn import 192.0.2.1:100
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_import_vrf3_step8():
+    """
+    Import r2 vrf3 into r1 vrf2 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  rt vpn import 192.0.2.2:300
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_unimport_vrf3_step9():
+    """
+    Un-import r2 vrf3 into r1 vrf2 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  no rt vpn import 192.0.2.2:300
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_import_vrf3_step10():
+    """
+    Import r2 vrf3 into r1 vrf2 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500 vrf vrf2
+ address-family ipv4 unicast
+  rt vpn import 192.0.2.2:300
+ exit-address-family
+!
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_no_retain_remove_vrf2_step11():
+    """
+    Import r2 vrf3 into r1 vrf2 and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+no router bgp 65500 vrf vrf2
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_no_retain_init.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
+def test_bgp_retain_step12():
+    """
+    Configure retain and check bgp vpnv4 table
+    """
+
+    rname = "r1"
+    cfg = """
+configure
+router bgp 65500
+ address-family ipv4 vpn
+  bgp retain route-target all
+"""
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears[rname]
+    router.vtysh_cmd(cfg)
+
+    check_show_bgp_ipv4_vpn(rname, "ipv4_vpn_routes_all.json")
+    check_show_bgp_ipv4_vpn("r2", "ipv4_vpn_routes_all.json")
+
+
 def test_memory_leak():
     "Run the memory leak test and report results."
     tgen = get_topogen()

From e3d2aeac17bb1ffa33711324ee2aa947eac4228a Mon Sep 17 00:00:00 2001
From: Louis Scalbert <louis.scalbert@6wind.com>
Date: Thu, 8 Jun 2023 13:42:30 +0200
Subject: [PATCH 13/13] topotests: import vpn to default vrf in
 bgp_vpnv4_noretain

Test that importing VPN prefixes into the default VRF is possible with
the no retain feature.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
---
 .../topotests/bgp_vpnv4_noretain/r1/bgpd.conf |  4 +++
 .../r1/ipv4_vpn_routes_all.json               | 27 ++++++++++++++++++
 .../r1/ipv4_vpn_routes_no_retain_init.json    | 27 ++++++++++++++++++
 ...pn_routes_no_retain_init_plus_r2_vrf2.json | 27 ++++++++++++++++++
 ...pn_routes_no_retain_init_plus_r2_vrf3.json | 27 ++++++++++++++++++
 .../topotests/bgp_vpnv4_noretain/r2/bgpd.conf | 10 +++++++
 .../r2/ipv4_vpn_routes_all.json               | 28 +++++++++++++++++++
 .../bgp_vpnv4_noretain/r2/zebra.conf          |  3 ++
 8 files changed, 153 insertions(+)

diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
index a30caf5c9d..0709e43edf 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/bgpd.conf
@@ -3,6 +3,10 @@ router bgp 65500
  neighbor 10.125.0.2 remote-as 65500
  address-family ipv4 unicast
   no neighbor 10.125.0.2 activate
+  label vpn export 100
+  rd vpn export 192.0.2.1:0
+  rt vpn import 192.0.2.2:400
+  import vpn
  exit-address-family
  address-family ipv4 vpn
   neighbor 10.125.0.2 activate
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
index e2fed4ef82..648bf854ba 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_all.json
@@ -142,6 +142,33 @@
                   ]
                }
             ]
+         },
+         "192.0.2.2:4":{
+            "10.204.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.204.0.0",
+                  "prefixLen":24,
+                  "network":"10.204.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
index a6f789282c..f01607ac4e 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init.json
@@ -88,6 +88,33 @@
                   ]
                }
             ]
+         },
+         "192.0.2.2:4":{
+            "10.204.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.204.0.0",
+                  "prefixLen":24,
+                  "network":"10.204.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
index 4877bebb32..6df6c69b8f 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf2.json
@@ -115,6 +115,33 @@
                   ]
                }
             ]
+         },
+         "192.0.2.2:4":{
+            "10.204.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.204.0.0",
+                  "prefixLen":24,
+                  "network":"10.204.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json
index f05c12b42f..7a17ff0f16 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r1/ipv4_vpn_routes_no_retain_init_plus_r2_vrf3.json
@@ -115,6 +115,33 @@
                   ]
                }
             ]
+         },
+         "192.0.2.2:4":{
+            "10.204.0.0/24":[
+               {
+                  "valid":true,
+                  "bestpath":true,
+                  "selectionReason":"First path received",
+                  "pathFrom":"internal",
+                  "prefix":"10.204.0.0",
+                  "prefixLen":24,
+                  "network":"10.204.0.0\/24",
+                  "metric":0,
+                  "locPrf":100,
+                  "weight":0,
+                  "peerId":"10.125.0.2",
+                  "path":"",
+                  "origin":"incomplete",
+                  "nexthops":[
+                     {
+                        "ip":"10.125.0.2",
+                        "hostname":"r2",
+                        "afi":"ipv4",
+                        "used":true
+                     }
+                  ]
+               }
+            ]
          }
       }
    }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
index a5c13e0343..729daef2bc 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/bgpd.conf
@@ -42,3 +42,13 @@ router bgp 65500 vrf vrf3
   export vpn
  exit-address-family
 !
+router bgp 65500 vrf vrf4
+ bgp router-id 192.0.2.2
+ address-family ipv4 unicast
+  redistribute connected
+  label vpn export 204
+  rd vpn export 192.0.2.2:4
+  rt vpn export 192.0.2.2:400
+  export vpn
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
index f94bea2e22..d8b8e88d93 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/ipv4_vpn_routes_all.json
@@ -143,6 +143,34 @@
             ]
           }
         ]
+      },
+      "192.0.2.2:4": {
+        "10.204.0.0/24": [
+          {
+            "valid": true,
+            "bestpath": true,
+            "selectionReason": "First path received",
+            "pathFrom": "external",
+            "prefix": "10.204.0.0",
+            "prefixLen": 24,
+            "network": "10.204.0.0/24",
+            "metric": 0,
+            "weight": 32768,
+            "peerId": "(unspec)",
+            "path": "",
+            "origin": "incomplete",
+            "announceNexthopSelf": true,
+            "nhVrfName": "vrf4",
+            "nexthops": [
+              {
+                "ip": "0.0.0.0",
+                "hostname": "r2",
+                "afi": "ipv4",
+                "used": true
+              }
+            ]
+          }
+        ]
       }
     }
   }
diff --git a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
index 057a3614c7..f19ad9d3e8 100644
--- a/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
+++ b/tests/topotests/bgp_vpnv4_noretain/r2/zebra.conf
@@ -17,3 +17,6 @@ interface r2-eth2 vrf vrf2
 interface r2-eth3 vrf vrf3
  ip address 10.203.0.1/24
 !
+interface r2-eth4 vrf vrf4
+ ip address 10.204.0.1/24
+!