Revert "bgpd: fix incorrect usage of slist in dampening"

This reverts commit 5054cfcbbc8c776a91e9390d36cff891d0890b4f.

2 files changed, 88 insertions, 26 deletions

diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c
index bf61d73588..7f1abb73c0 100644
--- a/bgpd/bgp_damp.c
+++ b/bgpd/bgp_damp.c
@@ -40,33 +40,72 @@
 static void bgp_reuselist_add(struct reuselist *list,
 			      struct bgp_damp_info *info)
 {
+	struct reuselist_node *new_node;
+
 	assert(info);
-	SLIST_INSERT_HEAD(list, info, entry);
+	new_node = XCALLOC(MTYPE_BGP_DAMP_REUSELIST, sizeof(*new_node));
+	new_node->info = info;
+	SLIST_INSERT_HEAD(list, new_node, entry);
 }
 
 static void bgp_reuselist_del(struct reuselist *list,
-			      struct bgp_damp_info *info)
+			      struct reuselist_node **node)
 {
-	assert(info);
-	SLIST_REMOVE(list, info, bgp_damp_info, entry);
+	if ((*node) == NULL)
+		return;
+	assert(list && node && *node);
+	SLIST_REMOVE(list, (*node), reuselist_node, entry);
+	XFREE(MTYPE_BGP_DAMP_REUSELIST, (*node));
+	*node = NULL;
 }
 
 static void bgp_reuselist_switch(struct reuselist *source,
-				 struct bgp_damp_info *info,
+				 struct reuselist_node *node,
 				 struct reuselist *target)
 {
-	assert(source && target && info);
-	SLIST_REMOVE(source, info, bgp_damp_info, entry);
-	SLIST_INSERT_HEAD(target, info, entry);
+	assert(source && target && node);
+	SLIST_REMOVE(source, node, reuselist_node, entry);
+	SLIST_INSERT_HEAD(target, node, entry);
+}
+
+static void bgp_reuselist_free(struct reuselist *list)
+{
+	struct reuselist_node *rn;
+
+	assert(list);
+	while ((rn = SLIST_FIRST(list)) != NULL)
+		bgp_reuselist_del(list, &rn);
+}
+
+static struct reuselist_node *bgp_reuselist_find(struct reuselist *list,
+						 struct bgp_damp_info *info)
+{
+	struct reuselist_node *rn;
+
+	assert(list && info);
+	SLIST_FOREACH (rn, list, entry) {
+		if (rn->info == info)
+			return rn;
+	}
+	return NULL;
 }
 
 static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi)
 {
+	struct reuselist_node *node;
+
 	assert(bdi && bdi->config);
-	if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX)
-		bgp_reuselist_del(&bdi->config->no_reuse_list, bdi);
-	else
-		bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], bdi);
+	if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) {
+		node = bgp_reuselist_find(&bdi->config->no_reuse_list, bdi);
+		if (node)
+			bgp_reuselist_del(&bdi->config->no_reuse_list, &node);
+	} else {
+		node = bgp_reuselist_find(&bdi->config->reuse_list[bdi->index],
+					  bdi);
+		if (node)
+			bgp_reuselist_del(&bdi->config->reuse_list[bdi->index],
+					  &node);
+	}
 	bdi->config = NULL;
 }
 
@@ -148,9 +187,19 @@ static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi,
 	bgp_reuselist_add(&bdc->no_reuse_list, bdi);
 }
 
-static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi)
+static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi,
+				     struct bgp_damp_config *bdc)
 {
-	bgp_damp_info_unclaim(bdi);
+	struct reuselist_node *rn;
+
+	assert(bdc && bdi);
+	if (bdi->config == NULL) {
+		bgp_damp_info_unclaim(bdi);
+		return;
+	}
+	bdi->config = NULL;
+	rn = bgp_reuselist_find(&bdc->no_reuse_list, bdi);
+	bgp_reuselist_del(&bdc->no_reuse_list, &rn);
 }
 
 /* Return decayed penalty value.  */
@@ -176,6 +225,7 @@ static int bgp_reuse_timer(struct thread *t)
 	struct bgp_damp_config *bdc = THREAD_ARG(t);
 	struct bgp_damp_info *bdi;
 	struct reuselist plist;
+	struct reuselist_node *node;
 	struct bgp *bgp;
 	time_t t_now, t_diff;
 
@@ -196,7 +246,8 @@ static int bgp_reuse_timer(struct thread *t)
 	assert(bdc->reuse_offset < bdc->reuse_list_size);
 
 	/* 3. if ( the saved list head pointer is non-empty ) */
-	while ((bdi = SLIST_FIRST(&plist)) != NULL) {
+	while ((node = SLIST_FIRST(&plist)) != NULL) {
+		bdi = node->info;
 		bgp = bdi->path->peer->bgp;
 
 		/* Set t-diff = t-now - t-updated.  */
@@ -227,19 +278,20 @@ static int bgp_reuse_timer(struct thread *t)
 			}
 
 			if (bdi->penalty <= bdc->reuse_limit / 2.0) {
-				bgp_reuselist_del(&plist, bdi);
 				bgp_damp_info_free(bdi, bdc, 1, bdi->afi,
 						   bdi->safi);
+				bgp_reuselist_del(&plist, &node);
 			} else {
-				bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX;
-				bgp_reuselist_switch(&plist, bdi,
+				node->info->index =
+					BGP_DAMP_NO_REUSE_LIST_INDEX;
+				bgp_reuselist_switch(&plist, node,
 						     &bdc->no_reuse_list);
 			}
 		} else {
 			/* Re-insert into another list (See RFC2439 Section
 			 * 4.8.6).  */
 			bdi->index = bgp_reuse_index(bdi->penalty, bdc);
-			bgp_reuselist_switch(&plist, bdi,
+			bgp_reuselist_switch(&plist, node,
 					     &bdc->reuse_list[bdi->index]);
 		}
 	}
@@ -335,7 +387,7 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest,
 	if (bdi->penalty >= bdc->suppress_value) {
 		bgp_path_info_set_flag(dest, path, BGP_PATH_DAMPED);
 		bdi->suppress_time = t_now;
-		bgp_no_reuse_list_delete(bdi);
+		bgp_no_reuse_list_delete(bdi, bdc);
 		bgp_reuse_list_add(bdi, bdc);
 	}
 	return BGP_DAMP_USED;
@@ -496,13 +548,15 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc,
 			 afi_t afi, safi_t safi)
 {
 	struct bgp_damp_info *bdi;
+	struct reuselist_node *rn;
 	struct reuselist *list;
 	unsigned int i;
 
 	bdc->reuse_offset = 0;
 	for (i = 0; i < bdc->reuse_list_size; ++i) {
 		list = &bdc->reuse_list[i];
-		while ((bdi = SLIST_FIRST(list)) != NULL) {
+		while ((rn = SLIST_FIRST(list)) != NULL) {
+			bdi = rn->info;
 			if (bdi->lastrecord == BGP_RECORD_UPDATE) {
 				bgp_aggregate_increment(bgp, &bdi->dest->p,
 							bdi->path, bdi->afi,
@@ -510,13 +564,14 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc,
 				bgp_process(bgp, bdi->dest, bdi->afi,
 					    bdi->safi);
 			}
-			bgp_reuselist_del(list, bdi);
+			bgp_reuselist_del(list, &rn);
 			bgp_damp_info_free(bdi, bdc, 1, afi, safi);
 		}
 	}
 
-	while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) {
-		bgp_reuselist_del(&bdc->no_reuse_list, bdi);
+	while ((rn = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) {
+		bdi = rn->info;
+		bgp_reuselist_del(&bdc->no_reuse_list, &rn);
 		bgp_damp_info_free(bdi, bdc, 1, afi, safi);
 	}
 
@@ -528,6 +583,10 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc,
 	XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->reuse_index);
 	bdc->reuse_index_size = 0;
 
+	/* Free reuse list array. */
+	for (i = 0; i < bdc->reuse_list_size; ++i)
+		bgp_reuselist_free(&bdc->reuse_list[i]);
+
 	XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->reuse_list);
 	bdc->reuse_list_size = 0;
 
diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h
index 2e41d2fbda..8367f291c7 100644
--- a/bgpd/bgp_damp.h
+++ b/bgpd/bgp_damp.h
@@ -61,11 +61,14 @@ struct bgp_damp_info {
 
 	afi_t afi;
 	safi_t safi;
+};
 
-	SLIST_ENTRY(bgp_damp_info) entry;
+struct reuselist_node {
+	SLIST_ENTRY(reuselist_node) entry;
+	struct bgp_damp_info *info;
 };
 
-SLIST_HEAD(reuselist, bgp_damp_info);
+SLIST_HEAD(reuselist, reuselist_node);
 
 /* Specified parameter set configuration. */
 struct bgp_damp_config {