From 8155e8c592199b27d73b800f471447f27add8826 Mon Sep 17 00:00:00 2001
From: Stephen Worley <sworley@cumulusnetworks.com>
Date: Wed, 22 Jul 2020 13:45:47 -0400
Subject: [PATCH] zebra: add flag track released state of proto NHGS

Add a flag to track the released state of a proto-based NHG.

This flag is used to know whether the upper level proto has called
the *_del API. Typically, the NHG would just get removed and uninstalled
at this point but there is a chance we are being sent it while routes
are still being owned or we were sent it multiple times. This flag
and associated code handles that.

Ticket: CM-30369

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
---
 zebra/zebra_nhg.c | 23 +++++++++++++++++++++--
 zebra/zebra_nhg.h | 13 ++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 3d0835d272..6abce30e8e 100644
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -2794,6 +2794,17 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type,
 	zebra_nhg_install_kernel(new);
 
 	if (old) {
+		/*
+		 * Check to handle recving DEL while routes still in use then
+		 * a replace.
+		 *
+		 * In this case we would have decremented the refcnt already
+		 * but set the FLAG here. Go ahead and increment once to fix
+		 * the misordering we have been sent.
+		 */
+		if (CHECK_FLAG(old->flags, NEXTHOP_GROUP_PROTO_RELEASED))
+			zebra_nhg_increment_ref(old);
+
 		rib_handle_nhg_replace(old, new);
 
 		/* if this != 1 at this point, we have a bug */
@@ -2833,14 +2844,22 @@ struct nhg_hash_entry *zebra_nhg_proto_del(uint32_t id)
 	nhe = zebra_nhg_lookup_id(id);
 
 	if (!nhe) {
-		if (IS_ZEBRA_DEBUG_NHG_DETAIL)
+		if (IS_ZEBRA_DEBUG_NHG)
 			zlog_debug("%s: id %u, lookup failed", __func__, id);
 
 		return NULL;
 	}
 
+	if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_PROTO_RELEASED)) {
+		if (IS_ZEBRA_DEBUG_NHG)
+			zlog_debug("%s: id %u, already released", __func__, id);
+
+		return NULL;
+	}
+
+	SET_FLAG(nhe->flags, NEXTHOP_GROUP_PROTO_RELEASED);
+
 	if (nhe->refcnt > 1) {
-		/* TODO: should be warn? */
 		if (IS_ZEBRA_DEBUG_NHG)
 			zlog_debug(
 				"%s: id %u, still being used by routes refcnt %u",
diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
index 6c9f20d0a4..d8aafbf2ca 100644
--- a/zebra/zebra_nhg.h
+++ b/zebra/zebra_nhg.h
@@ -109,10 +109,21 @@ struct nhg_hash_entry {
  */
 #define NEXTHOP_GROUP_BACKUP (1 << 4)
 
+/*
+ * The NHG has been release by an upper level protocol via the
+ * `zebra_nhg_proto_del()` API.
+ *
+ * We use this flag to track this state in case the NHG is still being used
+ * by routes therefore holding their refcnts as well. Otherwise, the NHG will
+ * be removed and uninstalled.
+ *
+ */
+#define NEXTHOP_GROUP_PROTO_RELEASED (1 << 5)
+
 /*
  * Track FPM installation status..
  */
-#define NEXTHOP_GROUP_FPM (1 << 5)
+#define NEXTHOP_GROUP_FPM (1 << 6)
 };
 
 /* Was this one we created, either this session or previously? */