ripd, ripngd: change how we keep track of redistribution configuration

ripd and ripngd were leveraging the zclient code to keep track of
the redistribute configuration, which is what most daemons do. The
problem, however, is that the zclient code uses VRF IDs to identify
VRFs, and VRF IDs are unknown until a VRF is enabled (information
received from zebra). This means we can't configure a redistribute
command on a RIP instance when the corresponding VRF is disabled
(doing so leads to a null-dereference crash right now in both ripd
and ripngd).

To fix this, change the rip/ripng data structures so that they
keep track of the full redistribute configuration and not only the
route-map and metric associated to each command. This is similar
to what bgpd and ospfd are doing to solve the same problem. In the
future the zclient code and all daemons need to be refactored to
consolidate the handling of redistribute configuration in a single
place to reduce code duplication. One of the most important changes
to do is to use VRF names and not VRF IDs to identify VRFs.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

4 files changed, 79 insertions, 43 deletions

diff --git a/ripngd/ripng_northbound.c b/ripngd/ripng_northbound.c
index 4b15a3aef..dda57a31f 100644
--- a/ripngd/ripng_northbound.c
+++ b/ripngd/ripng_northbound.c
@@ -411,6 +411,17 @@ static int ripngd_instance_redistribute_create(enum nb_event event,
 					       const struct lyd_node *dnode,
 					       union nb_resource *resource)
 {
+	struct ripng *ripng;
+	int type;
+
+	if (event != NB_EV_APPLY)
+		return NB_OK;
+
+	ripng = yang_dnode_get_entry(dnode, true);
+	type = yang_dnode_get_enum(dnode, "./protocol");
+
+	ripng->redist[type].enabled = true;
+
 	return NB_OK;
 }
 
@@ -426,7 +437,17 @@ static int ripngd_instance_redistribute_delete(enum nb_event event,
 	ripng = yang_dnode_get_entry(dnode, true);
 	type = yang_dnode_get_enum(dnode, "./protocol");
 
-	ripng_redistribute_conf_delete(ripng, type);
+	ripng->redist[type].enabled = false;
+	if (ripng->redist[type].route_map.name) {
+		free(ripng->redist[type].route_map.name);
+		ripng->redist[type].route_map.name = NULL;
+		ripng->redist[type].route_map.map = NULL;
+	}
+	ripng->redist[type].metric_config = false;
+	ripng->redist[type].metric = 0;
+
+	if (ripng->enabled)
+		ripng_redistribute_conf_delete(ripng, type);
 
 	return NB_OK;
 }
@@ -440,7 +461,8 @@ ripngd_instance_redistribute_apply_finish(const struct lyd_node *dnode)
 	ripng = yang_dnode_get_entry(dnode, true);
 	type = yang_dnode_get_enum(dnode, "./protocol");
 
-	ripng_redistribute_conf_update(ripng, type);
+	if (ripng->enabled)
+		ripng_redistribute_conf_update(ripng, type);
 }
 
 /*
@@ -462,10 +484,10 @@ ripngd_instance_redistribute_route_map_modify(enum nb_event event,
 	type = yang_dnode_get_enum(dnode, "../protocol");
 	rmap_name = yang_dnode_get_string(dnode, NULL);
 
-	if (ripng->route_map[type].name)
-		free(ripng->route_map[type].name);
-	ripng->route_map[type].name = strdup(rmap_name);
-	ripng->route_map[type].map = route_map_lookup_by_name(rmap_name);
+	if (ripng->redist[type].route_map.name)
+		free(ripng->redist[type].route_map.name);
+	ripng->redist[type].route_map.name = strdup(rmap_name);
+	ripng->redist[type].route_map.map = route_map_lookup_by_name(rmap_name);
 
 	return NB_OK;
 }
@@ -483,9 +505,9 @@ ripngd_instance_redistribute_route_map_delete(enum nb_event event,
 	ripng = yang_dnode_get_entry(dnode, true);
 	type = yang_dnode_get_enum(dnode, "../protocol");
 
-	free(ripng->route_map[type].name);
-	ripng->route_map[type].name = NULL;
-	ripng->route_map[type].map = NULL;
+	free(ripng->redist[type].route_map.name);
+	ripng->redist[type].route_map.name = NULL;
+	ripng->redist[type].route_map.map = NULL;
 
 	return NB_OK;
 }
@@ -509,8 +531,8 @@ ripngd_instance_redistribute_metric_modify(enum nb_event event,
 	type = yang_dnode_get_enum(dnode, "../protocol");
 	metric = yang_dnode_get_uint8(dnode, NULL);
 
-	ripng->route_map[type].metric_config = true;
-	ripng->route_map[type].metric = metric;
+	ripng->redist[type].metric_config = true;
+	ripng->redist[type].metric = metric;
 
 	return NB_OK;
 }
@@ -528,8 +550,8 @@ ripngd_instance_redistribute_metric_delete(enum nb_event event,
 	ripng = yang_dnode_get_entry(dnode, true);
 	type = yang_dnode_get_enum(dnode, "../protocol");
 
-	ripng->route_map[type].metric_config = false;
-	ripng->route_map[type].metric = 0;
+	ripng->redist[type].metric_config = false;
+	ripng->redist[type].metric = 0;
 
 	return NB_OK;
 }
diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c
index 913ac5191..e2dcc0238 100644
--- a/ripngd/ripng_zebra.c
+++ b/ripngd/ripng_zebra.c
@@ -165,24 +165,28 @@ void ripng_redistribute_conf_delete(struct ripng *ripng, int type)
 
 int ripng_redistribute_check(struct ripng *ripng, int type)
 {
-	return vrf_bitmap_check(zclient->redist[AFI_IP6][type],
-				ripng->vrf->vrf_id);
+	return ripng->redist[type].enabled;
 }
 
-void ripng_redistribute_clean(struct ripng *ripng)
+void ripng_redistribute_enable(struct ripng *ripng)
 {
 	for (int i = 0; i < ZEBRA_ROUTE_MAX; i++) {
-		if (!vrf_bitmap_check(zclient->redist[AFI_IP6][i],
-				      ripng->vrf->vrf_id))
+		if (!ripng_redistribute_check(ripng, i))
 			continue;
 
-		if (zclient->sock > 0)
-			zebra_redistribute_send(ZEBRA_REDISTRIBUTE_DELETE,
-						zclient, AFI_IP6, i, 0,
-						ripng->vrf->vrf_id);
+		zebra_redistribute_send(ZEBRA_REDISTRIBUTE_ADD, zclient,
+					AFI_IP6, i, 0, ripng->vrf->vrf_id);
+	}
+}
 
-		vrf_bitmap_unset(zclient->redist[AFI_IP6][i],
-				 ripng->vrf->vrf_id);
+void ripng_redistribute_disable(struct ripng *ripng)
+{
+	for (int i = 0; i < ZEBRA_ROUTE_MAX; i++) {
+		if (!ripng_redistribute_check(ripng, i))
+			continue;
+
+		zebra_redistribute_send(ZEBRA_REDISTRIBUTE_DELETE, zclient,
+					AFI_IP6, i, 0, ripng->vrf->vrf_id);
 	}
 }
 
@@ -192,8 +196,7 @@ void ripng_redistribute_write(struct vty *vty, struct ripng *ripng)
 
 	for (i = 0; i < ZEBRA_ROUTE_MAX; i++) {
 		if (i == zclient->redist_default
-		    || !vrf_bitmap_check(zclient->redist[AFI_IP6][i],
-					 ripng->vrf->vrf_id))
+		    || !ripng_redistribute_check(ripng, i))
 			continue;
 
 		vty_out(vty, "    %s", zebra_route_string(i));
diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c
index 951c73bc4..c3aa9d8db 100644
--- a/ripngd/ripngd.c
+++ b/ripngd/ripngd.c
@@ -1682,10 +1682,11 @@ void ripng_output_process(struct interface *ifp, struct sockaddr_in6 *to,
 			}
 
 			/* Redistribute route-map. */
-			if (ripng->route_map[rinfo->type].name) {
-				ret = route_map_apply(
-					ripng->route_map[rinfo->type].map,
-					(struct prefix *)p, RMAP_RIPNG, rinfo);
+			if (ripng->redist[rinfo->type].route_map.name) {
+				ret = route_map_apply(ripng->redist[rinfo->type]
+							      .route_map.map,
+						      (struct prefix *)p,
+						      RMAP_RIPNG, rinfo);
 
 				if (ret == RMAP_DENYMATCH) {
 					if (IS_RIPNG_DEBUG_PACKET)
@@ -1700,10 +1701,10 @@ void ripng_output_process(struct interface *ifp, struct sockaddr_in6 *to,
 			/* When the route-map does not set metric. */
 			if (!rinfo->metric_set) {
 				/* If the redistribute metric is set. */
-				if (ripng->route_map[rinfo->type].metric_config
+				if (ripng->redist[rinfo->type].metric_config
 				    && rinfo->metric != RIPNG_METRIC_INFINITY) {
 					rinfo->metric_out =
-						ripng->route_map[rinfo->type]
+						ripng->redist[rinfo->type]
 							.metric;
 				} else {
 					/* If the route is not connected or
@@ -2531,8 +2532,8 @@ void ripng_clean(struct ripng *ripng)
 		ripng_instance_disable(ripng);
 
 	for (int i = 0; i < ZEBRA_ROUTE_MAX; i++)
-		if (ripng->route_map[i].name)
-			free(ripng->route_map[i].name);
+		if (ripng->redist[i].route_map.name)
+			free(ripng->redist[i].route_map.name);
 
 	agg_table_finish(ripng->table);
 	list_delete(&ripng->peer_list);
@@ -2548,7 +2549,6 @@ void ripng_clean(struct ripng *ripng)
 	vector_free(ripng->passive_interface);
 	list_delete(&ripng->offset_list_master);
 	ripng_interface_clean(ripng);
-	ripng_redistribute_clean(ripng);
 
 	RB_REMOVE(ripng_instance_head, &ripng_instances, ripng);
 	XFREE(MTYPE_RIPNG_VRF_NAME, ripng->vrf_name);
@@ -2598,9 +2598,10 @@ void ripng_if_rmap_update_interface(struct interface *ifp)
 static void ripng_routemap_update_redistribute(struct ripng *ripng)
 {
 	for (int i = 0; i < ZEBRA_ROUTE_MAX; i++) {
-		if (ripng->route_map[i].name)
-			ripng->route_map[i].map = route_map_lookup_by_name(
-				ripng->route_map[i].name);
+		if (ripng->redist[i].route_map.name)
+			ripng->redist[i].route_map.map =
+				route_map_lookup_by_name(
+					ripng->redist[i].route_map.name);
 	}
 }
 
@@ -2652,6 +2653,9 @@ static void ripng_instance_enable(struct ripng *ripng, struct vrf *vrf,
 	ripng_vrf_link(ripng, vrf);
 	ripng->enabled = true;
 
+	/* Resend all redistribute requests. */
+	ripng_redistribute_enable(ripng);
+
 	/* Create read and timer thread. */
 	ripng_event(ripng, RIPNG_READ, ripng->sock);
 	ripng_event(ripng, RIPNG_UPDATE_EVENT, 1);
@@ -2694,6 +2698,9 @@ static void ripng_instance_disable(struct ripng *ripng)
 		}
 	}
 
+	/* Flush all redistribute requests. */
+	ripng_redistribute_disable(ripng);
+
 	/* Cancel the RIPng timers */
 	RIPNG_TIMER_OFF(ripng->t_update);
 	RIPNG_TIMER_OFF(ripng->t_triggered_update);
diff --git a/ripngd/ripngd.h b/ripngd/ripngd.h
index d6c4394c6..b38e6b3a9 100644
--- a/ripngd/ripngd.h
+++ b/ripngd/ripngd.h
@@ -149,13 +149,16 @@ struct ripng {
 	/* RIPng ECMP flag */
 	bool ecmp;
 
-	/* For redistribute route map. */
+	/* RIPng redistribute configuration. */
 	struct {
-		char *name;
-		struct route_map *map;
+		bool enabled;
+		struct {
+			char *name;
+			struct route_map *map;
+		} route_map;
 		bool metric_config;
 		uint8_t metric;
-	} route_map[ZEBRA_ROUTE_MAX];
+	} redist[ZEBRA_ROUTE_MAX];
 
 	/* For distribute-list container */
 	struct distribute_ctx *distribute_ctx;
@@ -447,7 +450,8 @@ extern void ripng_if_rmap_update_interface(struct interface *);
 extern void ripng_zebra_ipv6_add(struct ripng *ripng, struct agg_node *node);
 extern void ripng_zebra_ipv6_delete(struct ripng *ripng, struct agg_node *node);
 
-extern void ripng_redistribute_clean(struct ripng *ripng);
+extern void ripng_redistribute_enable(struct ripng *ripng);
+extern void ripng_redistribute_disable(struct ripng *ripng);
 extern int ripng_redistribute_check(struct ripng *ripng, int type);
 extern void ripng_redistribute_write(struct vty *vty, struct ripng *ripng);