Merge pull request #8361 from rameshabhinay/change_1

bgpd: vrf route leaking related fixes

bgpd/bgp_ecommunity.c

@@ -1294,15 +1294,19 @@ bool ecommunity_del_val(struct ecommunity *ecom, struct ecommunity_val *eval)
 
 	/* Delete the selected value */
 	ecom->size--;
-	p = XMALLOC(MTYPE_ECOMMUNITY_VAL, ecom->size * ecom->unit_size);
-	if (c != 0)
-		memcpy(p, ecom->val, c * ecom->unit_size);
-	if ((ecom->size - c) != 0)
-		memcpy(p + (c)*ecom->unit_size,
-		       ecom->val + (c + 1) * ecom->unit_size,
-		       (ecom->size - c) * ecom->unit_size);
-	XFREE(MTYPE_ECOMMUNITY_VAL, ecom->val);
-	ecom->val = p;
+	if (ecom->size) {
+		p = XMALLOC(MTYPE_ECOMMUNITY_VAL, ecom->size * ecom->unit_size);
+		if (c != 0)
+			memcpy(p, ecom->val, c * ecom->unit_size);
+		if ((ecom->size - c) != 0)
+			memcpy(p + (c)*ecom->unit_size,
+			       ecom->val + (c + 1) * ecom->unit_size,
+			       (ecom->size - c) * ecom->unit_size);
+		XFREE(MTYPE_ECOMMUNITY_VAL, ecom->val);
+		ecom->val = p;
+	} else
+		ecom->val = NULL;
+
 	return true;
 }
 

bgpd/bgp_mplsvpn.c

@@ -540,6 +540,17 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
 	if (bpi) {
 		bool labelssame = labels_same(bpi, label, num_labels);
 
+		if (CHECK_FLAG(source_bpi->flags, BGP_PATH_REMOVED)
+		    && CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) {
+			if (debug) {
+				zlog_debug(
+					"%s: ->%s(s_flags: 0x%x b_flags: 0x%x): %pFX: Found route, being removed, not leaking",
+					__func__, bgp->name_pretty,
+					source_bpi->flags, bpi->flags, p);
+			}
+			return NULL;
+		}
+
 		if (attrhash_cmp(bpi->attr, new_attr) && labelssame
 		    && !CHECK_FLAG(bpi->flags, BGP_PATH_REMOVED)) {
 
@@ -613,6 +624,16 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
 		return bpi;
 	}
 
+	if (CHECK_FLAG(source_bpi->flags, BGP_PATH_REMOVED)) {
+		if (debug) {
+			zlog_debug(
+				"%s: ->%s(s_flags: 0x%x): %pFX: New route, being removed, not leaking",
+				__func__, bgp->name_pretty,
+				source_bpi->flags, p);
+		}
+		return NULL;
+	}
+
 	new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_IMPORTED, 0,
 		bgp->peer_self, new_attr, bn);
 
@@ -1027,6 +1048,8 @@ void vpn_leak_from_vrf_withdraw_all(struct bgp *bgp_vpn, /* to */
 					if (debug)
 						zlog_debug("%s: deleting it",
 							   __func__);
+					/* withdraw from leak-to vrfs as well */
+					vpn_leak_to_vrf_withdraw(bgp_vpn, bpi);
 					bgp_aggregate_decrement(
 						bgp_vpn,
 						bgp_dest_get_prefix(bn), bpi,
@@ -1101,7 +1124,10 @@ vpn_leak_to_vrf_update_onevrf(struct bgp *bgp_vrf,	    /* to */
 	if (!ecom_intersect(
 		    bgp_vrf->vpn_policy[afi].rtlist[BGP_VPN_POLICY_DIR_FROMVPN],
 		    path_vpn->attr->ecommunity)) {
-
+		if (debug)
+			zlog_debug(
+				"from vpn to vrf %s, skipping after no intersection of route targets",
+				bgp_vrf->name_pretty);
 		return;
 	}
 
@@ -1532,7 +1558,8 @@ void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw,
 				 bool is_config)
 {
 	afi_t afi;
-	int debug;
+	int debug = (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF)
+		     | BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF));
 	char *vname;
 	const char *export_name;
 	char buf[RD_ADDRSTRLEN];
@@ -1541,14 +1568,23 @@ void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw,
 	struct ecommunity *ecom;
 	vpn_policy_direction_t idir, edir;
 
+	/*
+	 * Router-id change that is not explicitly configured
+	 * (a change from zebra, frr restart for example)
+	 * should not replace a configured vpn RD/RT.
+	 */
+	if (!is_config) {
+		if (debug)
+			zlog_debug("%s: skipping non explicit router-id change",
+				   __func__);
+		return;
+	}
+
 	if (bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT
 	    && bgp->inst_type != BGP_INSTANCE_TYPE_VRF)
 		return;
 
 	export_name = bgp->name ? bgp->name : VRF_DEFAULT_NAME;
-	debug = (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF) |
-		     BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF));
-
 	idir = BGP_VPN_POLICY_DIR_FROMVPN;
 	edir = BGP_VPN_POLICY_DIR_TOVPN;
 
@@ -1574,26 +1610,12 @@ void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw,
 				if (!bgp_import)
 					continue;
 
-				ecommunity_del_val(bgp_import->vpn_policy[afi].
-						   rtlist[idir],
+				ecommunity_del_val(
+					bgp_import->vpn_policy[afi]
+						.rtlist[idir],
 					(struct ecommunity_val *)ecom->val);
-
 			}
 		} else {
-			/*
-			 * Router-id changes that are not explicit config
-			 * changes should not replace configured RD/RT.
-			 */
-			if (!is_config) {
-				if (CHECK_FLAG(bgp->vpn_policy[afi].flags,
-					       BGP_VPN_POLICY_TOVPN_RD_SET)) {
-					if (debug)
-						zlog_debug("%s: auto router-id change skipped",
-							   __func__);
-					goto postchange;
-				}
-			}
-
 			/* New router-id derive auto RD and RT and export
 			 * to VPN
 			 */
@@ -1624,10 +1646,8 @@ void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw,
 				else
 					bgp_import->vpn_policy[afi].rtlist[idir]
 						= ecommunity_dup(ecom);
-
 			}
 
-postchange:
 			/* Update routes to VPN */
 			vpn_leak_postchange(BGP_VPN_POLICY_DIR_TOVPN,
 					    afi, bgp_get_default(),

bgpd/bgp_nb_config.c

@@ -123,7 +123,12 @@ int bgp_router_create(struct nb_cb_create_args *args)
 		if (is_new_bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT)
 			vpn_leak_postchange_all();
 
-		if (inst_type == BGP_INSTANCE_TYPE_VRF)
+		/*
+		 * Check if we need to export to other VRF(s).
+		 * Leak the routes to importing bgp vrf instances,
+		 * only when new bgp vrf instance is configured.
+		 */
+		if (ret != BGP_INSTANCE_EXISTS)
 			bgp_vpn_leak_export(bgp);
 
 		UNSET_FLAG(bgp->vrf_flags, BGP_VRF_AUTO);

bgpd/bgp_zebra.c

@@ -1700,9 +1700,6 @@ int bgp_redistribute_set(struct bgp *bgp, afi_t afi, int type,
 
 		redist_add_instance(&zclient->mi_redist[afi][type], instance);
 	} else {
-		if (vrf_bitmap_check(zclient->redist[afi][type], bgp->vrf_id))
-			return CMD_WARNING;
-
 #ifdef ENABLE_BGP_VNC
 		if (EVPN_ENABLED(bgp) && type == ZEBRA_ROUTE_VNC_DIRECT) {
 			vnc_export_bgp_enable(

bgpd/bgpd.c

@@ -3402,7 +3402,7 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name,
 		return ret;
 	case BGP_SUCCESS:
 		if (*bgp_val)
-			return ret;
+			return BGP_INSTANCE_EXISTS;
 	}
 
 	bgp = bgp_create(as, name, inst_type);

bgpd/bgpd.h

@@ -1844,6 +1844,7 @@ enum bgp_clear_type {
 /* BGP error codes.  */
 #define BGP_SUCCESS                               0
 #define BGP_CREATED                               1
+#define BGP_INSTANCE_EXISTS                       2
 #define BGP_ERR_INVALID_VALUE                    -1
 #define BGP_ERR_INVALID_FLAG                     -2
 #define BGP_ERR_INVALID_AS                       -3

zebra/redistribute.c

@@ -347,17 +347,12 @@ void zebra_redistribute_add(ZAPI_HANDLER_ARGS)
 					   zvrf_id(zvrf), afi);
 		}
 	} else {
-		if (!vrf_bitmap_check(client->redist[afi][type],
-				      zvrf_id(zvrf))) {
-			if (IS_ZEBRA_DEBUG_EVENT)
-				zlog_debug(
-					"%s: setting vrf %s(%u) redist bitmap",
-					__func__, VRF_LOGNAME(zvrf->vrf),
-					zvrf_id(zvrf));
-			vrf_bitmap_set(client->redist[afi][type],
-				       zvrf_id(zvrf));
-			zebra_redistribute(client, type, 0, zvrf_id(zvrf), afi);
-		}
+		if (IS_ZEBRA_DEBUG_EVENT)
+			zlog_debug("%s: setting vrf %s(%u) redist bitmap",
+				   __func__, VRF_LOGNAME(zvrf->vrf),
+				   zvrf_id(zvrf));
+		vrf_bitmap_set(client->redist[afi][type], zvrf_id(zvrf));
+		zebra_redistribute(client, type, 0, zvrf_id(zvrf), afi);
 	}
 
 stream_failure: