From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Mon, 24 Aug 2020 19:14:44 +0000 (-0400)
Subject: pbrd: When multiple items share the pnhc do the right thing
X-Git-Tag: frr-7.5.1~90^2~1
X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=9d961247f51f6a9a05ad919de25c59f06056ab95;p=mirror_frr.git

pbrd: When multiple items share the pnhc do the right thing

We had multiple pnhc cache entries with the same nexthop
pointer.  This causes some large amount of confusion.
Fixup the code to handle this situation better.

Ticket: CM-31044
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---

diff --git a/pbrd/pbr_map.c b/pbrd/pbr_map.c
index b0d1e235f..cce97b5df 100644
--- a/pbrd/pbr_map.c
+++ b/pbrd/pbr_map.c
@@ -779,6 +779,7 @@ void pbr_map_check_vrf_nh_group_change(const char *nh_group,
 	struct pbr_map_sequence *pbrms;
 	struct listnode *node;
 
+
 	RB_FOREACH (pbrm, pbr_map_entry_head, &pbr_maps) {
 		for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, node, pbrms)) {
 			if (pbrms->nhgrp_name)
diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c
index 4d0682300..9722300d9 100644
--- a/pbrd/pbr_nht.c
+++ b/pbrd/pbr_nht.c
@@ -707,6 +707,7 @@ struct pbr_nht_individual {
 	struct interface *ifp;
 	struct pbr_vrf *pbr_vrf;
 	struct pbr_nexthop_cache *pnhc;
+	vrf_id_t old_vrf_id;
 
 	uint32_t valid;
 
@@ -897,22 +898,80 @@ void pbr_nht_nexthop_update(struct zapi_route *nhr)
 	hash_iterate(pbr_nhg_hash, pbr_nht_nexthop_update_lookup, nhr);
 }
 
-static void pbr_nht_individual_nexthop_vrf_handle(struct hash_bucket *b,
-						  void *data)
+struct nhrc_vrf_info {
+	struct pbr_vrf *pbr_vrf;
+	uint32_t old_vrf_id;
+	struct nhrc *nhrc;
+};
+
+static int pbr_nht_nhrc_vrf_change(struct hash_bucket *b, void *data)
+{
+	struct nhrc *nhrc = b->data;
+	struct nhrc_vrf_info *nhrcvi = data;
+
+	if (nhrc->nexthop.vrf_id == nhrcvi->old_vrf_id) {
+		nhrcvi->nhrc = nhrc;
+		return HASHWALK_ABORT;
+	}
+
+	return HASHWALK_CONTINUE;
+}
+
+static int pbr_nht_individual_nexthop_vrf_handle(struct hash_bucket *b,
+						 void *data)
 {
 	struct pbr_nexthop_cache *pnhc = b->data;
 	struct pbr_nht_individual *pnhi = data;
 
+	if (pnhc->looked_at == true)
+		return HASHWALK_CONTINUE;
+
 	if (pnhc->nexthop->vrf_id == VRF_DEFAULT)
-		return;
+		return HASHWALK_CONTINUE;
 
-	if ((strncmp(pnhc->vrf_name, pbr_vrf_name(pnhi->pbr_vrf),
-		     sizeof(pnhc->vrf_name))
-	     == 0)
-	    && pnhc->nexthop->vrf_id != pbr_vrf_id(pnhi->pbr_vrf)) {
+	if (strncmp(pnhc->vrf_name, pbr_vrf_name(pnhi->pbr_vrf),
+		    sizeof(pnhc->vrf_name))
+	    == 0) {
 		pnhi->pnhc = pnhc;
-		pnhi->nhr_matched = true;
+
+		if (pnhc->nexthop->vrf_id != pbr_vrf_id(pnhi->pbr_vrf)) {
+			struct nhrc_vrf_info nhrcvi;
+
+			memset(&nhrcvi, 0, sizeof(nhrcvi));
+			nhrcvi.pbr_vrf = pnhi->pbr_vrf;
+			nhrcvi.old_vrf_id = pnhc->nexthop->vrf_id;
+
+			pnhi->nhr_matched = true;
+			pnhi->old_vrf_id = pnhc->nexthop->vrf_id;
+
+			do {
+				nhrcvi.nhrc = NULL;
+				hash_walk(pbr_nhrc_hash,
+					  pbr_nht_nhrc_vrf_change, &nhrcvi);
+				if (nhrcvi.nhrc) {
+					hash_release(pbr_nhrc_hash,
+						     nhrcvi.nhrc);
+					nhrcvi.nhrc->nexthop.vrf_id =
+						pbr_vrf_id(pnhi->pbr_vrf);
+					hash_get(pbr_nhrc_hash, nhrcvi.nhrc,
+						 hash_alloc_intern);
+					pbr_send_rnh(&nhrcvi.nhrc->nexthop, true);
+				}
+			} while (nhrcvi.nhrc);
+		}
+
+		pnhc->looked_at = true;
+		return HASHWALK_ABORT;
 	}
+
+	return HASHWALK_CONTINUE;
+}
+
+static void pbr_nht_clear_looked_at(struct hash_bucket *b, void *data)
+{
+	struct pbr_nexthop_cache *pnhc = b->data;
+
+	pnhc->looked_at = false;
 }
 
 static void pbr_nht_nexthop_vrf_handle(struct hash_bucket *b, void *data)
@@ -920,34 +979,33 @@ static void pbr_nht_nexthop_vrf_handle(struct hash_bucket *b, void *data)
 	struct pbr_nexthop_group_cache *pnhgc = b->data;
 	struct pbr_vrf *pbr_vrf = data;
 	struct pbr_nht_individual pnhi = {};
-	struct nhrc *nhrc;
-	uint32_t old_vrf_id;
 
+	zlog_debug("pnhgc iterating");
+	hash_iterate(pnhgc->nhh, pbr_nht_clear_looked_at, NULL);
+	memset(&pnhi, 0, sizeof(pnhi));
+	pnhi.pbr_vrf = pbr_vrf;
 	do {
-		memset(&pnhi, 0, sizeof(pnhi));
-		pnhi.pbr_vrf = pbr_vrf;
-		hash_iterate(pnhgc->nhh, pbr_nht_individual_nexthop_vrf_handle,
-			     &pnhi);
+		struct pbr_nexthop_cache *pnhc;
+
+		pnhi.pnhc = NULL;
+		hash_walk(pnhgc->nhh, pbr_nht_individual_nexthop_vrf_handle,
+			  &pnhi);
 
 		if (!pnhi.pnhc)
 			continue;
 
+		pnhc = pnhi.pnhc;
+		pnhc->nexthop->vrf_id = pnhi.old_vrf_id;
 		pnhi.pnhc = hash_release(pnhgc->nhh, pnhi.pnhc);
-		old_vrf_id = pnhi.pnhc->nexthop->vrf_id;
+		if (pnhi.pnhc) {
+			pnhi.pnhc->nexthop->vrf_id = pbr_vrf_id(pbr_vrf);
 
-		nhrc = hash_lookup(pbr_nhrc_hash, pnhi.pnhc->nexthop);
-		if (nhrc) {
-			hash_release(pbr_nhrc_hash, nhrc);
-			nhrc->nexthop.vrf_id = pbr_vrf_id(pbr_vrf);
-			hash_get(pbr_nhrc_hash, nhrc, hash_alloc_intern);
-			pbr_send_rnh(pnhi.pnhc->nexthop, true);
-		}
-		pnhi.pnhc->nexthop->vrf_id = pbr_vrf_id(pbr_vrf);
-
-		hash_get(pnhgc->nhh, pnhi.pnhc, hash_alloc_intern);
+			hash_get(pnhgc->nhh, pnhi.pnhc, hash_alloc_intern);
+		} else
+			pnhc->nexthop->vrf_id = pbr_vrf_id(pbr_vrf);
 
 		pbr_map_check_vrf_nh_group_change(pnhgc->name, pbr_vrf,
-						  old_vrf_id);
+						  pnhi.old_vrf_id);
 	} while (pnhi.pnhc);
 }
 
diff --git a/pbrd/pbr_nht.h b/pbrd/pbr_nht.h
index a1099a00c..b58486323 100644
--- a/pbrd/pbr_nht.h
+++ b/pbrd/pbr_nht.h
@@ -53,6 +53,7 @@ struct pbr_nexthop_cache {
 
 	struct nexthop *nexthop;
 
+	bool looked_at;
 	bool valid;
 };