Merge pull request #8650 from idryzhov/bgp-fix-redist

bgpd: fix redistribution in vrf

bgpd/bgp_main.c

@@ -314,9 +314,6 @@ static int bgp_vrf_enable(struct vrf *vrf)
 		bgp_vrf_link(bgp, vrf);
 
 		bgp_handle_socket(bgp, vrf, old_vrf_id, true);
-		/* Update any redistribution if vrf_id changed */
-		if (old_vrf_id != bgp->vrf_id)
-			bgp_redistribute_redo(bgp);
 		bgp_instance_up(bgp);
 		vpn_leak_zebra_vrf_label_update(bgp, AFI_IP);
 		vpn_leak_zebra_vrf_label_update(bgp, AFI_IP6);
@@ -336,7 +333,6 @@ static int bgp_vrf_enable(struct vrf *vrf)
 static int bgp_vrf_disable(struct vrf *vrf)
 {
 	struct bgp *bgp;
-	vrf_id_t old_vrf_id;
 
 	if (vrf->vrf_id == VRF_DEFAULT)
 		return 0;
@@ -358,15 +354,11 @@ static int bgp_vrf_disable(struct vrf *vrf)
 		vpn_leak_prechange(BGP_VPN_POLICY_DIR_FROMVPN, AFI_IP6,
 				   bgp_get_default(), bgp);
 
-		old_vrf_id = bgp->vrf_id;
 		bgp_handle_socket(bgp, vrf, VRF_UNKNOWN, false);
 		/* We have instance configured, unlink from VRF and make it
 		 * "down". */
-		bgp_vrf_unlink(bgp, vrf);
-		/* Delete any redistribute vrf bitmaps if the vrf_id changed */
-		if (old_vrf_id != bgp->vrf_id)
-			bgp_unset_redist_vrf_bitmaps(bgp, old_vrf_id);
 		bgp_instance_down(bgp);
+		bgp_vrf_unlink(bgp, vrf);
 	}
 
 	/* Note: This is a callback, the VRF will be deleted by the caller. */

bgpd/bgp_zebra.c

@@ -1700,6 +1700,9 @@ 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(
@@ -1909,22 +1912,6 @@ void bgp_redistribute_redo(struct bgp *bgp)
 	}
 }
 
-/* Unset redistribute vrf bitmap during triggers like
-   restart networking or delete VRFs */
-void bgp_unset_redist_vrf_bitmaps(struct bgp *bgp, vrf_id_t old_vrf_id)
-{
-	int i;
-	afi_t afi;
-
-	for (afi = AFI_IP; afi < AFI_MAX; afi++)
-		for (i = 0; i < ZEBRA_ROUTE_MAX; i++)
-			if (vrf_bitmap_check(zclient->redist[afi][i],
-					     old_vrf_id))
-				vrf_bitmap_unset(zclient->redist[afi][i],
-						 old_vrf_id);
-	return;
-}
-
 void bgp_zclient_reset(void)
 {
 	zclient_reset(zclient);

bgpd/bgpd.c

@@ -3438,6 +3438,46 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name,
 	return BGP_CREATED;
 }
 
+static void bgp_zclient_set_redist(afi_t afi, int type, unsigned short instance,
+				   vrf_id_t vrf_id, bool set)
+{
+	if (instance) {
+		if (set)
+			redist_add_instance(&zclient->mi_redist[afi][type],
+					    instance);
+		else
+			redist_del_instance(&zclient->mi_redist[afi][type],
+					    instance);
+	} else {
+		if (set)
+			vrf_bitmap_set(zclient->redist[afi][type], vrf_id);
+		else
+			vrf_bitmap_unset(zclient->redist[afi][type], vrf_id);
+	}
+}
+
+static void bgp_set_redist_vrf_bitmaps(struct bgp *bgp, bool set)
+{
+	afi_t afi;
+	int i;
+	struct list *red_list;
+	struct listnode *node;
+	struct bgp_redist *red;
+
+	for (afi = AFI_IP; afi < AFI_MAX; afi++) {
+		for (i = 0; i < ZEBRA_ROUTE_MAX; i++) {
+
+			red_list = bgp->redist[afi][i];
+			if (!red_list)
+				continue;
+
+			for (ALL_LIST_ELEMENTS_RO(red_list, node, red))
+				bgp_zclient_set_redist(afi, i, red->instance,
+						       bgp->vrf_id, set);
+		}
+	}
+}
+
 /*
  * Make BGP instance "up". Applies only to VRFs (non-default) and
  * implies the VRF has been learnt from Zebra.
@@ -3447,6 +3487,8 @@ void bgp_instance_up(struct bgp *bgp)
 	struct peer *peer;
 	struct listnode *node, *next;
 
+	bgp_set_redist_vrf_bitmaps(bgp, true);
+
 	/* Register with zebra. */
 	bgp_zebra_instance_register(bgp);
 
@@ -3491,6 +3533,10 @@ void bgp_instance_down(struct bgp *bgp)
 
 	/* Cleanup registered nexthops (flags) */
 	bgp_cleanup_nexthops(bgp);
+
+	bgp_zebra_instance_deregister(bgp);
+
+	bgp_set_redist_vrf_bitmaps(bgp, false);
 }
 
 /* Delete BGP instance. */

bgpd/bgpd.h

@@ -2386,8 +2386,6 @@ static inline bool bgp_in_graceful_shutdown(struct bgp *bgp)
 	        !!CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_SHUTDOWN));
 }
 
-extern void bgp_unset_redist_vrf_bitmaps(struct bgp *, vrf_id_t);
-
 /* For benefit of rfapi */
 extern struct peer *peer_new(struct bgp *bgp);
 

zebra/redistribute.c

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