From 1e9044be8d4325fa82f01d72eb6c8581dcd6fd06 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Wed, 23 Jun 2021 16:35:44 +0200
Subject: [PATCH 1/7] *: clean up ifp-by-local-address function(s)

Most users of if_lookup_address_exact only cared about whether the
address is any local address.  Split that off into a separate function.

For the users that actually need the ifp - which I'm about to add a few
of - change it to prefer returning interfaces that are UP.

(Function name changed due to slight change in behavior re. UP state, to
avoid possible bugs from this change.)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 lib/if.c                | 43 +++++++++++++++++++++++++++--------------
 lib/if.h                |  8 +++++++-
 pimd/pim_bsm.c          |  4 ++--
 pimd/pim_igmp.c         |  2 +-
 pimd/pim_igmp_mtrace.c  |  4 ++--
 pimd/pim_register.c     |  2 +-
 pimd/pim_rp.c           |  9 ---------
 ripd/rip_snmp.c         |  2 +-
 staticd/static_routes.c |  8 ++++----
 staticd/static_zebra.c  |  8 ++------
 10 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/lib/if.c b/lib/if.c
index a40f04f5a2..6bfbdf9147 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -455,36 +455,51 @@ static struct interface *if_lookup_by_index_all_vrf(ifindex_t ifindex)
 	return NULL;
 }
 
-/* Lookup interface by IP address. */
-struct interface *if_lookup_exact_address(const void *src, int family,
+/* Lookup interface by IP address.
+ *
+ * supersedes if_lookup_exact_address(), which didn't care about up/down
+ * state.  but all users we have either only care if the address is local
+ * (=> use if_address_is_local() please), or care about UP interfaces before
+ * anything else
+ *
+ * to accept only UP interfaces, check if_is_up() on the returned ifp.
+ */
+struct interface *if_lookup_address_local(const void *src, int family,
 					  vrf_id_t vrf_id)
 {
 	struct vrf *vrf = vrf_lookup_by_id(vrf_id);
 	struct listnode *cnode;
-	struct interface *ifp;
+	struct interface *ifp, *best_down = NULL;
 	struct prefix *p;
 	struct connected *c;
 
+	if (family != AF_INET && family != AF_INET6)
+		return NULL;
+
 	FOR_ALL_INTERFACES (vrf, ifp) {
 		for (ALL_LIST_ELEMENTS_RO(ifp->connected, cnode, c)) {
 			p = c->address;
 
-			if (p && (p->family == family)) {
-				if (family == AF_INET) {
-					if (IPV4_ADDR_SAME(
-						    &p->u.prefix4,
+			if (!p || p->family != family)
+				continue;
+
+			if (family == AF_INET) {
+				if (!IPV4_ADDR_SAME(&p->u.prefix4,
 						    (struct in_addr *)src))
-						return ifp;
-				} else if (family == AF_INET6) {
-					if (IPV6_ADDR_SAME(
-						    &p->u.prefix6,
+					continue;
+			} else if (family == AF_INET6) {
+				if (!IPV6_ADDR_SAME(&p->u.prefix6,
 						    (struct in6_addr *)src))
-						return ifp;
-				}
+					continue;
 			}
+
+			if (if_is_up(ifp))
+				return ifp;
+			if (!best_down)
+				best_down = ifp;
 		}
 	}
-	return NULL;
+	return best_down;
 }
 
 /* Lookup interface by IP address. */
diff --git a/lib/if.h b/lib/if.h
index 1012bf5557..5f0dc27555 100644
--- a/lib/if.h
+++ b/lib/if.h
@@ -511,7 +511,7 @@ extern void if_update_to_new_vrf(struct interface *, vrf_id_t vrf_id);
 extern struct interface *if_lookup_by_index(ifindex_t, vrf_id_t vrf_id);
 extern struct interface *if_vrf_lookup_by_index_next(ifindex_t ifindex,
 						     vrf_id_t vrf_id);
-extern struct interface *if_lookup_exact_address(const void *matchaddr,
+extern struct interface *if_lookup_address_local(const void *matchaddr,
 						 int family, vrf_id_t vrf_id);
 extern struct connected *if_lookup_address(const void *matchaddr, int family,
 					   vrf_id_t vrf_id);
@@ -520,6 +520,12 @@ extern struct interface *if_lookup_prefix(const struct prefix *prefix,
 size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz,
 			   struct interface ***result, vrf_id_t vrf_id);
 
+static inline bool if_address_is_local(const void *matchaddr, int family,
+				       vrf_id_t vrf_id)
+{
+	return if_lookup_address_local(matchaddr, family, vrf_id) != NULL;
+}
+
 struct vrf;
 extern struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf);
 extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id);
diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c
index a3a3426f39..6c4c7eeb5a 100644
--- a/pimd/pim_bsm.c
+++ b/pimd/pim_bsm.c
@@ -1323,8 +1323,8 @@ int pim_bsm_process(struct interface *ifp, struct ip *ip_hdr, uint8_t *buf,
 				return -1;
 			}
 		}
-	} else if (if_lookup_exact_address(&ip_hdr->ip_dst, AF_INET,
-					   pim->vrf->vrf_id)) {
+	} else if (if_address_is_local(&ip_hdr->ip_dst, AF_INET,
+				       pim->vrf->vrf_id)) {
 		/* Unicast BSM received - if ucast bsm not enabled on
 		 * the interface, drop it
 		 */
diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c
index 795c96c838..cd905b3cbd 100644
--- a/pimd/pim_igmp.c
+++ b/pimd/pim_igmp.c
@@ -324,7 +324,7 @@ static int igmp_recv_query(struct igmp_sock *igmp, int query_version,
 		return 0;
 	}
 
-	if (if_lookup_exact_address(&from, AF_INET, ifp->vrf_id)) {
+	if (if_address_is_local(&from, AF_INET, ifp->vrf_id)) {
 		if (PIM_DEBUG_IGMP_PACKETS)
 			zlog_debug("Recv IGMP query on interface: %s from ourself %s",
 				   ifp->name, from_str);
diff --git a/pimd/pim_igmp_mtrace.c b/pimd/pim_igmp_mtrace.c
index 73af44fc46..42101bc48e 100644
--- a/pimd/pim_igmp_mtrace.c
+++ b/pimd/pim_igmp_mtrace.c
@@ -596,8 +596,8 @@ int igmp_mtrace_recv_qry_req(struct igmp_sock *igmp, struct ip *ip_hdr,
 	 * if applicable
 	 */
 	if (!IPV4_CLASS_DE(ntohl(ip_hdr->ip_dst.s_addr)))
-		if (!if_lookup_exact_address(&ip_hdr->ip_dst, AF_INET,
-					     pim->vrf->vrf_id))
+		if (!if_address_is_local(&ip_hdr->ip_dst, AF_INET,
+					 pim->vrf->vrf_id))
 			return mtrace_forward_packet(pim, ip_hdr);
 
 	if (igmp_msg_len < (int)sizeof(struct igmp_mtrace)) {
diff --git a/pimd/pim_register.c b/pimd/pim_register.c
index e2538da36f..cc0dace7c2 100644
--- a/pimd/pim_register.c
+++ b/pimd/pim_register.c
@@ -327,7 +327,7 @@ int pim_register_recv(struct interface *ifp, struct in_addr dest_addr,
 #define PIM_MSG_REGISTER_BIT_RESERVED_LEN 4
 	ip_hdr = (struct ip *)(tlv_buf + PIM_MSG_REGISTER_BIT_RESERVED_LEN);
 
-	if (!pim_rp_check_is_my_ip_address(pim, dest_addr)) {
+	if (!if_address_is_local(&dest_addr, AF_INET, pim->vrf->vrf_id)) {
 		if (PIM_DEBUG_PIM_REG) {
 			char dest[INET_ADDRSTRLEN];
 
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index f2a969e04a..1651387384 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -1178,15 +1178,6 @@ int pim_rp_config_write(struct pim_instance *pim, struct vty *vty,
 	return count;
 }
 
-bool pim_rp_check_is_my_ip_address(struct pim_instance *pim,
-				   struct in_addr dest_addr)
-{
-	if (if_lookup_exact_address(&dest_addr, AF_INET, pim->vrf->vrf_id))
-		return true;
-
-	return false;
-}
-
 void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj)
 {
 	struct rp_info *rp_info;
diff --git a/ripd/rip_snmp.c b/ripd/rip_snmp.c
index 824cbd8cf1..436dc4de0e 100644
--- a/ripd/rip_snmp.c
+++ b/ripd/rip_snmp.c
@@ -257,7 +257,7 @@ static struct interface *rip2IfLookup(struct variable *v, oid name[],
 
 		oid2in_addr(name + v->namelen, sizeof(struct in_addr), addr);
 
-		return if_lookup_exact_address((void *)addr, AF_INET,
+		return if_lookup_address_local((void *)addr, AF_INET,
 					       VRF_DEFAULT);
 	} else {
 		len = *length - v->namelen;
diff --git a/staticd/static_routes.c b/staticd/static_routes.c
index 60f384e517..45c42ddcef 100644
--- a/staticd/static_routes.c
+++ b/staticd/static_routes.c
@@ -199,14 +199,14 @@ bool static_add_nexthop_validate(const char *nh_vrf_name,
 	switch (type) {
 	case STATIC_IPV4_GATEWAY:
 	case STATIC_IPV4_GATEWAY_IFNAME:
-		if (if_lookup_exact_address(&ipaddr->ipaddr_v4, AF_INET,
-					    vrf->vrf_id))
+		if (if_address_is_local(&ipaddr->ipaddr_v4, AF_INET,
+					vrf->vrf_id))
 			return false;
 		break;
 	case STATIC_IPV6_GATEWAY:
 	case STATIC_IPV6_GATEWAY_IFNAME:
-		if (if_lookup_exact_address(&ipaddr->ipaddr_v6, AF_INET6,
-					    vrf->vrf_id))
+		if (if_address_is_local(&ipaddr->ipaddr_v6, AF_INET6,
+					vrf->vrf_id))
 			return false;
 		break;
 	default:
diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c
index 538fae28aa..38b3c93d76 100644
--- a/staticd/static_zebra.c
+++ b/staticd/static_zebra.c
@@ -162,14 +162,10 @@ static bool
 static_nexthop_is_local(vrf_id_t vrfid, struct prefix *addr, int family)
 {
 	if (family == AF_INET) {
-		if (if_lookup_exact_address(&addr->u.prefix4,
-					AF_INET,
-					vrfid))
+		if (if_address_is_local(&addr->u.prefix4, AF_INET, vrfid))
 			return true;
 	} else if (family == AF_INET6) {
-		if (if_lookup_exact_address(&addr->u.prefix6,
-					AF_INET6,
-					vrfid))
+		if (if_address_is_local(&addr->u.prefix6, AF_INET6, vrfid))
 			return true;
 	}
 	return false;

From 1ee6df336315cfe47a54fa95579812dfb845bc7f Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Wed, 22 Sep 2021 11:22:12 +0200
Subject: [PATCH 2/7] pimd: actually return msec in timer_remain_msec()

... really old TODO sitting there.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 pimd/pim_time.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/pimd/pim_time.c b/pimd/pim_time.c
index c88ee7554b..93aaffb6a4 100644
--- a/pimd/pim_time.c
+++ b/pimd/pim_time.c
@@ -171,9 +171,7 @@ void pim_time_uptime_begin(char *buf, int buf_size, int64_t now, int64_t begin)
 
 long pim_time_timer_remain_msec(struct thread *t_timer)
 {
-	/* FIXME: Actually fetch msec resolution from thread */
-
 	/* no timer thread running means timer has expired: return 0 */
 
-	return t_timer ? 1000 * thread_timer_remain_second(t_timer) : 0;
+	return t_timer ? thread_timer_remain_msec(t_timer) : 0;
 }

From 4efdb9c6282485e48ffd387e10b0a0e116de0bb9 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Fri, 25 Jun 2021 10:53:26 +0200
Subject: [PATCH 3/7] pimd: clean up BSR NHT & fix parallel links

The Bootstrap message RX path needs a RPF check for the BSR address,
and this is implemented both incorrectly as well as quite ugly.

Clean up and fix case when we have multiple interfaces to the same LAN
and/or ECMP nexthops (both would cause message duplication, the former
can even cause BSM forwarding loops.)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 pimd/pim_bsm.c      |  95 ++++-----------------
 pimd/pim_cmd.c      |  14 +---
 pimd/pim_nht.c      | 199 ++++++++++++++++++++++++++++++--------------
 pimd/pim_nht.h      |  21 +++--
 pimd/pim_rp.c       |  20 ++---
 pimd/pim_rpf.c      |   2 +-
 pimd/pim_upstream.c |   2 +-
 pimd/pim_vxlan.c    |   4 +-
 8 files changed, 184 insertions(+), 173 deletions(-)

diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c
index 6c4c7eeb5a..b7207aa212 100644
--- a/pimd/pim_bsm.c
+++ b/pimd/pim_bsm.c
@@ -162,8 +162,6 @@ static int pim_on_bs_timer(struct thread *t)
 	struct bsm_scope *scope;
 	struct bsgrp_node *bsgrp_node;
 	struct bsm_rpinfo *bsrp;
-	struct prefix nht_p;
-	bool is_bsr_tracking = true;
 
 	scope = THREAD_ARG(t);
 	THREAD_OFF(scope->bs_timer);
@@ -172,15 +170,7 @@ static int pim_on_bs_timer(struct thread *t)
 		zlog_debug("%s: Bootstrap Timer expired for scope: %d",
 			   __func__, scope->sz_id);
 
-	/* Remove next hop tracking for the bsr */
-	nht_p.family = AF_INET;
-	nht_p.prefixlen = IPV4_MAX_BITLEN;
-	nht_p.u.prefix4 = scope->current_bsr;
-	if (PIM_DEBUG_BSM)
-		zlog_debug("%s: Deregister BSR addr %pFX with Zebra NHT",
-			   __func__, &nht_p);
-	pim_delete_tracked_nexthop(scope->pim, &nht_p, NULL, NULL,
-				   is_bsr_tracking);
+	pim_nht_bsr_del(scope->pim, scope->current_bsr);
 
 	/* Reset scope zone data */
 	scope->accept_nofwd_bsm = false;
@@ -543,35 +533,6 @@ static void pim_instate_pend_list(struct bsgrp_node *bsgrp_node)
 	pim_bsm_rpinfos_free(bsgrp_node->partial_bsrp_list);
 }
 
-static bool pim_bsr_rpf_check(struct pim_instance *pim, struct in_addr bsr,
-			      struct in_addr ip_src_addr)
-{
-	struct pim_nexthop nexthop;
-	int result;
-
-	memset(&nexthop, 0, sizeof(nexthop));
-
-	/* New BSR recived */
-	if (bsr.s_addr != pim->global_scope.current_bsr.s_addr) {
-		result = pim_nexthop_match(pim, bsr, ip_src_addr);
-
-		/* Nexthop lookup pass for the new BSR address */
-		if (result)
-			return true;
-
-		if (PIM_DEBUG_BSM) {
-			char bsr_str[INET_ADDRSTRLEN];
-
-			pim_inet4_dump("<bsr?>", bsr, bsr_str, sizeof(bsr_str));
-			zlog_debug("%s : No route to BSR address %s", __func__,
-				   bsr_str);
-		}
-		return false;
-	}
-
-	return pim_nexthop_match_nht_cache(pim, bsr, ip_src_addr);
-}
-
 static bool is_preferred_bsr(struct pim_instance *pim, struct in_addr bsr,
 			     uint32_t bsr_prio)
 {
@@ -594,35 +555,11 @@ static bool is_preferred_bsr(struct pim_instance *pim, struct in_addr bsr,
 static void pim_bsm_update(struct pim_instance *pim, struct in_addr bsr,
 			   uint32_t bsr_prio)
 {
-	struct pim_nexthop_cache pnc;
-
 	if (bsr.s_addr != pim->global_scope.current_bsr.s_addr) {
-		struct prefix nht_p;
-		bool is_bsr_tracking = true;
+		if (pim->global_scope.current_bsr.s_addr)
+			pim_nht_bsr_del(pim, pim->global_scope.current_bsr);
+		pim_nht_bsr_add(pim, bsr);
 
-		/* De-register old BSR and register new BSR with Zebra NHT */
-		nht_p.family = AF_INET;
-		nht_p.prefixlen = IPV4_MAX_BITLEN;
-
-		if (pim->global_scope.current_bsr.s_addr != INADDR_ANY) {
-			nht_p.u.prefix4 = pim->global_scope.current_bsr;
-			if (PIM_DEBUG_BSM)
-				zlog_debug(
-					"%s: Deregister BSR addr %pFX with Zebra NHT",
-					__func__, &nht_p);
-			pim_delete_tracked_nexthop(pim, &nht_p, NULL, NULL,
-						   is_bsr_tracking);
-		}
-
-		nht_p.u.prefix4 = bsr;
-		if (PIM_DEBUG_BSM)
-			zlog_debug(
-				"%s: NHT Register BSR addr %pFX with Zebra NHT",
-				__func__, &nht_p);
-
-		memset(&pnc, 0, sizeof(struct pim_nexthop_cache));
-		pim_find_or_track_nexthop(pim, &nht_p, NULL, NULL,
-					  is_bsr_tracking, &pnc);
 		pim->global_scope.current_bsr = bsr;
 		pim->global_scope.current_bsr_first_ts =
 			pim_time_monotonic_sec();
@@ -1310,18 +1247,20 @@ int pim_bsm_process(struct interface *ifp, struct ip *ip_hdr, uint8_t *buf,
 		}
 	}
 
-	/* Mulicast BSM received */
 	if (ip_hdr->ip_dst.s_addr == qpim_all_pim_routers_addr.s_addr) {
-		if (!no_fwd) {
-			if (!pim_bsr_rpf_check(pim, bshdr->bsr_addr.addr,
-					       ip_hdr->ip_src)) {
-				if (PIM_DEBUG_BSM)
-					zlog_debug(
-						"%s : RPF check fail for BSR address %s",
-						__func__, bsr_str);
-				pim->bsm_dropped++;
-				return -1;
-			}
+		/* Multicast BSMs are only accepted if source interface & IP
+		 * match RPF towards the BSR's IP address, or they have
+		 * no-forward set
+		 */
+		if (!no_fwd
+		    && !pim_nht_bsr_rpf_check(pim, bshdr->bsr_addr.addr, ifp,
+					      ip_hdr->ip_src)) {
+			if (PIM_DEBUG_BSM)
+				zlog_debug(
+					"BSM check: RPF to BSR %s is not %pI4%%%s",
+					bsr_str, &ip_hdr->ip_src, ifp->name);
+			pim->bsm_dropped++;
+			return -1;
 		}
 	} else if (if_address_is_local(&ip_hdr->ip_dst, AF_INET,
 				       pim->vrf->vrf_id)) {
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index e5ee7a82ad..518fcb00e1 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -4061,17 +4061,9 @@ static void clear_pim_bsr_db(struct pim_instance *pim)
 	struct rp_info *rp_all;
 	struct pim_upstream *up;
 	struct rp_info *rp_info;
-	bool is_bsr_tracking = true;
 
-	/* Remove next hop tracking for the bsr */
-	nht_p.family = AF_INET;
-	nht_p.prefixlen = IPV4_MAX_BITLEN;
-	nht_p.u.prefix4 = pim->global_scope.current_bsr;
-	if (PIM_DEBUG_BSM) {
-		zlog_debug("%s: Deregister BSR addr %pFX with Zebra NHT",
-			   __func__, &nht_p);
-	}
-	pim_delete_tracked_nexthop(pim, &nht_p, NULL, NULL, is_bsr_tracking);
+	if (pim->global_scope.current_bsr.s_addr)
+		pim_nht_bsr_del(pim, pim->global_scope.current_bsr);
 
 	/* Reset scope zone data */
 	pim->global_scope.accept_nofwd_bsm = false;
@@ -4119,7 +4111,7 @@ static void clear_pim_bsr_db(struct pim_instance *pim)
 				   __func__, &nht_p);
 		}
 
-		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info, false);
+		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info);
 
 		if (!str2prefix("224.0.0.0/4", &g_all))
 			return;
diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c
index 50cfc297d4..b5f26d3612 100644
--- a/pimd/pim_nht.c
+++ b/pimd/pim_nht.c
@@ -110,22 +110,11 @@ static struct pim_nexthop_cache *pim_nexthop_cache_add(struct pim_instance *pim,
 	return pnc;
 }
 
-/*
- * pim_find_or_track_nexthop
- *
- * This API is used to Register an address with Zebra
- *
- * 1 -> Success
- * 0 -> Failure
- */
-int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
-			      struct pim_upstream *up, struct rp_info *rp,
-			      bool bsr_track_needed,
-			      struct pim_nexthop_cache *out_pnc)
+static struct pim_nexthop_cache *pim_nht_get(struct pim_instance *pim,
+					     struct prefix *addr)
 {
 	struct pim_nexthop_cache *pnc = NULL;
 	struct pim_rpf rpf;
-	struct listnode *ch_node = NULL;
 	struct zclient *zclient = NULL;
 
 	zclient = pim_zebra_zclient_get();
@@ -145,6 +134,23 @@ int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
 				__func__, addr, pim->vrf->name);
 	}
 
+	return pnc;
+}
+
+/* TBD: this does several distinct things and should probably be split up.
+ * (checking state vs. returning pnc vs. adding upstream vs. adding rp)
+ */
+int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
+			      struct pim_upstream *up, struct rp_info *rp,
+			      struct pim_nexthop_cache *out_pnc)
+{
+	struct pim_nexthop_cache *pnc;
+	struct listnode *ch_node = NULL;
+
+	pnc = pim_nht_get(pim, addr);
+
+	assertf(up || rp, "addr=%pFX", addr);
+
 	if (rp != NULL) {
 		ch_node = listnode_lookup(pnc->rp_list, rp);
 		if (ch_node == NULL)
@@ -154,9 +160,6 @@ int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
 	if (up != NULL)
 		hash_get(pnc->upstream_hash, up, hash_alloc_intern);
 
-	if (bsr_track_needed)
-		pnc->bsr_tracking = true;
-
 	if (CHECK_FLAG(pnc->flags, PIM_NEXTHOP_VALID)) {
 		if (out_pnc)
 			memcpy(out_pnc, pnc, sizeof(struct pim_nexthop_cache));
@@ -166,72 +169,146 @@ int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
 	return 0;
 }
 
+void pim_nht_bsr_add(struct pim_instance *pim, struct in_addr addr)
+{
+	struct pim_nexthop_cache *pnc;
+	struct prefix pfx;
+
+	pfx.family = AF_INET;
+	pfx.prefixlen = IPV4_MAX_BITLEN;
+	pfx.u.prefix4 = addr;
+
+	pnc = pim_nht_get(pim, &pfx);
+
+	pnc->bsr_count++;
+}
+
+static void pim_nht_drop_maybe(struct pim_instance *pim,
+			       struct pim_nexthop_cache *pnc)
+{
+	if (PIM_DEBUG_PIM_NHT)
+		zlog_debug(
+			"%s: NHT %pFX(%s) rp_list count:%d upstream count:%ld BSR count:%u",
+			__func__, &pnc->rpf.rpf_addr, pim->vrf->name,
+			pnc->rp_list->count, pnc->upstream_hash->count,
+			pnc->bsr_count);
+
+	if (pnc->rp_list->count == 0 && pnc->upstream_hash->count == 0
+	    && pnc->bsr_count == 0) {
+		struct zclient *zclient = pim_zebra_zclient_get();
+
+		pim_sendmsg_zebra_rnh(pim, zclient, pnc,
+				      ZEBRA_NEXTHOP_UNREGISTER);
+
+		list_delete(&pnc->rp_list);
+		hash_free(pnc->upstream_hash);
+
+		hash_release(pim->rpf_hash, pnc);
+		if (pnc->nexthop)
+			nexthops_free(pnc->nexthop);
+		XFREE(MTYPE_PIM_NEXTHOP_CACHE, pnc);
+	}
+}
+
 void pim_delete_tracked_nexthop(struct pim_instance *pim, struct prefix *addr,
-				struct pim_upstream *up, struct rp_info *rp,
-				bool del_bsr_tracking)
+				struct pim_upstream *up, struct rp_info *rp)
 {
 	struct pim_nexthop_cache *pnc = NULL;
 	struct pim_nexthop_cache lookup;
-	struct zclient *zclient = NULL;
 	struct pim_upstream *upstream = NULL;
 
-	zclient = pim_zebra_zclient_get();
-
 	/* Remove from RPF hash if it is the last entry */
 	lookup.rpf.rpf_addr = *addr;
 	pnc = hash_lookup(pim->rpf_hash, &lookup);
-	if (pnc) {
-		if (rp) {
-			/* Release the (*, G)upstream from pnc->upstream_hash,
-			 * whose Group belongs to the RP getting deleted
-			 */
-			frr_each (rb_pim_upstream, &pim->upstream_head,
-				  upstream) {
-				struct prefix grp;
-				struct rp_info *trp_info;
+	if (!pnc) {
+		zlog_warn("attempting to delete nonexistent NHT entry %pFX",
+			  addr);
+		return;
+	}
 
-				if (upstream->sg.src.s_addr != INADDR_ANY)
-					continue;
+	if (rp) {
+		/* Release the (*, G)upstream from pnc->upstream_hash,
+		 * whose Group belongs to the RP getting deleted
+		 */
+		frr_each (rb_pim_upstream, &pim->upstream_head, upstream) {
+			struct prefix grp;
+			struct rp_info *trp_info;
 
-				grp.family = AF_INET;
-				grp.prefixlen = IPV4_MAX_BITLEN;
-				grp.u.prefix4 = upstream->sg.grp;
+			if (upstream->sg.src.s_addr != INADDR_ANY)
+				continue;
 
-				trp_info = pim_rp_find_match_group(pim, &grp);
-				if (trp_info == rp)
-					hash_release(pnc->upstream_hash,
-						     upstream);
-			}
-			listnode_delete(pnc->rp_list, rp);
+			grp.family = AF_INET;
+			grp.prefixlen = IPV4_MAX_BITLEN;
+			grp.u.prefix4 = upstream->sg.grp;
+
+			trp_info = pim_rp_find_match_group(pim, &grp);
+			if (trp_info == rp)
+				hash_release(pnc->upstream_hash, upstream);
 		}
+		listnode_delete(pnc->rp_list, rp);
+	}
 
-		if (up)
-			hash_release(pnc->upstream_hash, up);
+	if (up)
+		hash_release(pnc->upstream_hash, up);
 
-		if (del_bsr_tracking)
-			pnc->bsr_tracking = false;
+	pim_nht_drop_maybe(pim, pnc);
+}
 
-		if (PIM_DEBUG_PIM_NHT)
-			zlog_debug(
-				"%s: NHT %pFX(%s) rp_list count:%d upstream count:%ld",
-				__func__, addr, pim->vrf->name,
-				pnc->rp_list->count, pnc->upstream_hash->count);
+void pim_nht_bsr_del(struct pim_instance *pim, struct in_addr addr)
+{
+	struct pim_nexthop_cache *pnc = NULL;
+	struct pim_nexthop_cache lookup;
 
-		if (pnc->rp_list->count == 0
-		    && pnc->upstream_hash->count == 0
-		    && pnc->bsr_tracking == false) {
-			pim_sendmsg_zebra_rnh(pim, zclient, pnc,
-					      ZEBRA_NEXTHOP_UNREGISTER);
+	lookup.rpf.rpf_addr.family = AF_INET;
+	lookup.rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN;
+	lookup.rpf.rpf_addr.u.prefix4 = addr;
 
-			list_delete(&pnc->rp_list);
-			hash_free(pnc->upstream_hash);
+	pnc = hash_lookup(pim->rpf_hash, &lookup);
 
-			hash_release(pim->rpf_hash, pnc);
-			if (pnc->nexthop)
-				nexthops_free(pnc->nexthop);
-			XFREE(MTYPE_PIM_NEXTHOP_CACHE, pnc);
+	if (!pnc) {
+		zlog_warn("attempting to delete nonexistent NHT BSR entry %pI4",
+			  &addr);
+		return;
+	}
+
+	assertf(pnc->bsr_count > 0, "addr=%pI4", &addr);
+	pnc->bsr_count--;
+
+	pim_nht_drop_maybe(pim, pnc);
+}
+
+bool pim_nht_bsr_rpf_check(struct pim_instance *pim, struct in_addr bsr_addr,
+			   struct interface *src_ifp, struct in_addr src_ip)
+{
+	struct pim_nexthop_cache *pnc = NULL;
+	struct pim_nexthop_cache lookup;
+	struct nexthop *nh;
+
+	lookup.rpf.rpf_addr.family = AF_INET;
+	lookup.rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN;
+	lookup.rpf.rpf_addr.u.prefix4 = bsr_addr;
+
+	pnc = hash_lookup(pim->rpf_hash, &lookup);
+	if (!pnc)
+		return false;
+	if (!CHECK_FLAG(pnc->flags, PIM_NEXTHOP_VALID))
+		return false;
+
+	/* if we accept BSMs from more than one ECMP nexthop, this will cause
+	 * BSM message "multiplication" for each ECMP hop.  i.e. if you have
+	 * 4-way ECMP and 4 hops you end up with 256 copies of each BSM
+	 * message.
+	 *
+	 * so...  only accept the first (IPv4) nexthop as source.
+	 */
+
+	for (nh = pnc->nexthop; nh; nh = nh->next) {
+		if (nh->type == NEXTHOP_TYPE_IPV4_IFINDEX) {
+			return nh->ifindex == src_ifp->ifindex
+			       && nh->gate.ipv4.s_addr == src_ip.s_addr;
 		}
 	}
+	return false;
 }
 
 /* Given a source address and a neighbor address, check if the neighbor is one
diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h
index 12dbf167d1..e7d6825409 100644
--- a/pimd/pim_nht.h
+++ b/pimd/pim_nht.h
@@ -45,22 +45,20 @@ struct pim_nexthop_cache {
 
 	struct list *rp_list;
 	struct hash *upstream_hash;
-	/* Ideally this has to be list of scope zone. But for now we can just
-	 * have as a bool variable to say bsr_tracking.
-	 * Later this variable can be changed as a list of scope zones for
-	 * tracking same bsr for multiple scope zones.
+
+	/* bsr_count won't currently go above 1 as we only have global_scope,
+	 * but if anyone adds scope support multiple scopes may NHT-track the
+	 * same BSR
 	 */
-	bool bsr_tracking;
+	uint32_t bsr_count;
 };
 
 int pim_parse_nexthop_update(ZAPI_CALLBACK_ARGS);
 int pim_find_or_track_nexthop(struct pim_instance *pim, struct prefix *addr,
 			      struct pim_upstream *up, struct rp_info *rp,
-			      bool bsr_track_needed,
 			      struct pim_nexthop_cache *out_pnc);
 void pim_delete_tracked_nexthop(struct pim_instance *pim, struct prefix *addr,
-				struct pim_upstream *up, struct rp_info *rp,
-				bool del_bsr_tracking);
+				struct pim_upstream *up, struct rp_info *rp);
 struct pim_nexthop_cache *pim_nexthop_cache_find(struct pim_instance *pim,
 						 struct pim_rpf *rpf);
 uint32_t pim_compute_ecmp_hash(struct prefix *src, struct prefix *grp);
@@ -77,4 +75,11 @@ bool pim_nexthop_match(struct pim_instance *pim, struct in_addr addr,
 bool pim_nexthop_match_nht_cache(struct pim_instance *pim, struct in_addr addr,
 				 struct in_addr ip_src);
 
+/* for RPF check on BSM message receipt */
+void pim_nht_bsr_add(struct pim_instance *pim, struct in_addr bsr_addr);
+void pim_nht_bsr_del(struct pim_instance *pim, struct in_addr bsr_addr);
+/* RPF(bsr_addr) == src_ip%src_ifp? */
+bool pim_nht_bsr_rpf_check(struct pim_instance *pim, struct in_addr bsr_addr,
+			   struct interface *src_ifp, struct in_addr src_ip);
+
 #endif
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 1651387384..7ad95d9f6a 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -392,7 +392,7 @@ void pim_upstream_update(struct pim_instance *pim, struct pim_upstream *up)
 			zlog_debug(
 				"%s: Deregister upstream %s addr %pFX with Zebra NHT",
 				__func__, up->sg_str, &nht_p);
-		pim_delete_tracked_nexthop(pim, &nht_p, up, NULL, false);
+		pim_delete_tracked_nexthop(pim, &nht_p, up, NULL);
 	}
 
 	/* Update the upstream address */
@@ -555,7 +555,7 @@ int pim_rp_new(struct pim_instance *pim, struct in_addr rp_addr,
 			pim_rp_check_interfaces(pim, rp_all);
 			pim_rp_refresh_group_to_rp_mapping(pim);
 			pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_all,
-						  false, NULL);
+						  NULL);
 
 			if (!pim_ecmp_nexthop_lookup(pim,
 						     &rp_all->rp.source_nexthop,
@@ -649,7 +649,7 @@ int pim_rp_new(struct pim_instance *pim, struct in_addr rp_addr,
 	if (PIM_DEBUG_PIM_NHT_RP)
 		zlog_debug("%s: NHT Register RP addr %pFX grp %pFX with Zebra ",
 			   __func__, &nht_p, &rp_info->group);
-	pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, false, NULL);
+	pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL);
 	if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, &nht_p,
 				     &rp_info->group, 1))
 		return PIM_RP_NO_PATH;
@@ -757,7 +757,7 @@ int pim_rp_del(struct pim_instance *pim, struct in_addr rp_addr,
 	if (PIM_DEBUG_PIM_NHT_RP)
 		zlog_debug("%s: Deregister RP addr %pFX with Zebra ", __func__,
 			   &nht_p);
-	pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info, false);
+	pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info);
 
 	if (!str2prefix("224.0.0.0/4", &g_all))
 		return PIM_RP_BAD_ADDRESS;
@@ -886,7 +886,7 @@ int pim_rp_change(struct pim_instance *pim, struct in_addr new_rp_addr,
 		if (PIM_DEBUG_PIM_NHT_RP)
 			zlog_debug("%s: Deregister RP addr %pFX with Zebra ",
 				   __func__, &nht_p);
-		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info, false);
+		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info);
 	}
 
 	pim_rp_nexthop_del(rp_info);
@@ -919,7 +919,7 @@ int pim_rp_change(struct pim_instance *pim, struct in_addr new_rp_addr,
 		zlog_debug("%s: NHT Register RP addr %pFX grp %pFX with Zebra ",
 			   __func__, &nht_p, &rp_info->group);
 
-	pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, false, NULL);
+	pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL);
 	if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop, &nht_p,
 				     &rp_info->group, 1)) {
 		route_unlock_node(rn);
@@ -949,8 +949,7 @@ void pim_rp_setup(struct pim_instance *pim)
 		nht_p.prefixlen = IPV4_MAX_BITLEN;
 		nht_p.u.prefix4 = rp_info->rp.rpf_addr.u.prefix4;
 
-		pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, false,
-					  NULL);
+		pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL);
 		if (!pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop,
 					     &nht_p, &rp_info->group, 1))
 			if (PIM_DEBUG_PIM_NHT_RP)
@@ -1097,8 +1096,7 @@ struct pim_rpf *pim_rp_g(struct pim_instance *pim, struct in_addr group)
 			zlog_debug(
 				"%s: NHT Register RP addr %pFX grp %pFX with Zebra",
 				__func__, &nht_p, &rp_info->group);
-		pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, false,
-					  NULL);
+		pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info, NULL);
 		pim_rpf_set_refresh_time(pim);
 		(void)pim_ecmp_nexthop_lookup(pim, &rp_info->rp.source_nexthop,
 					      &nht_p, &rp_info->group, 1);
@@ -1328,7 +1326,7 @@ void pim_resolve_rp_nh(struct pim_instance *pim, struct pim_neighbor *nbr)
 		nht_p.u.prefix4 = rp_info->rp.rpf_addr.u.prefix4;
 		memset(&pnc, 0, sizeof(struct pim_nexthop_cache));
 		if (!pim_find_or_track_nexthop(pim, &nht_p, NULL, rp_info,
-					       false, &pnc))
+					       &pnc))
 			continue;
 
 		for (nh_node = pnc.nexthop; nh_node; nh_node = nh_node->next) {
diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c
index b93f85e48c..aa89431d32 100644
--- a/pimd/pim_rpf.c
+++ b/pimd/pim_rpf.c
@@ -262,7 +262,7 @@ enum pim_rpf_result pim_rpf_update(struct pim_instance *pim,
 	if ((up->sg.src.s_addr == INADDR_ANY && I_am_RP(pim, up->sg.grp)) ||
 	    PIM_UPSTREAM_FLAG_TEST_FHR(up->flags))
 		neigh_needed = false;
-	pim_find_or_track_nexthop(pim, &nht_p, up, NULL, false, NULL);
+	pim_find_or_track_nexthop(pim, &nht_p, up, NULL, NULL);
 	if (!pim_ecmp_nexthop_lookup(pim, &rpf->source_nexthop, &src, &grp,
 				neigh_needed)) {
 		/* Route is Deleted in Zebra, reset the stored NH data */
diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c
index d0d120523d..6f22937de8 100644
--- a/pimd/pim_upstream.c
+++ b/pimd/pim_upstream.c
@@ -271,7 +271,7 @@ struct pim_upstream *pim_upstream_del(struct pim_instance *pim,
 			zlog_debug(
 				"%s: Deregister upstream %s addr %pFX with Zebra NHT",
 				__func__, up->sg_str, &nht_p);
-		pim_delete_tracked_nexthop(pim, &nht_p, up, NULL, false);
+		pim_delete_tracked_nexthop(pim, &nht_p, up, NULL);
 	}
 
 	XFREE(MTYPE_PIM_UPSTREAM, up);
diff --git a/pimd/pim_vxlan.c b/pimd/pim_vxlan.c
index 5d5ea1bfe6..e24f647923 100644
--- a/pimd/pim_vxlan.c
+++ b/pimd/pim_vxlan.c
@@ -357,8 +357,8 @@ static void pim_vxlan_orig_mr_up_add(struct pim_vxlan_sg *vxlan_sg)
 			nht_p.family = AF_INET;
 			nht_p.prefixlen = IPV4_MAX_BITLEN;
 			nht_p.u.prefix4 = up->upstream_addr;
-			pim_delete_tracked_nexthop(vxlan_sg->pim,
-				&nht_p, up, NULL, false);
+			pim_delete_tracked_nexthop(vxlan_sg->pim, &nht_p, up,
+						   NULL);
 		}
 		/* We are acting FHR; clear out use_rpt setting if any */
 		pim_upstream_update_use_rpt(up, false /*update_mroute*/);

From b09bd804ac625dea4dee4e2a58c3b167ef39bca7 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Sun, 25 Jul 2021 15:48:03 +0200
Subject: [PATCH 4/7] pimd: move BSM clear into BSM code

... where it actually belongs.  And make a bunch of stuff static, since
it's no longer used across files now.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 pimd/pim_bsm.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++--
 pimd/pim_bsm.h |   6 +--
 pimd/pim_cmd.c | 119 +---------------------------------------------
 3 files changed, 123 insertions(+), 128 deletions(-)

diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c
index b7207aa212..2078a859e5 100644
--- a/pimd/pim_bsm.c
+++ b/pimd/pim_bsm.c
@@ -70,7 +70,7 @@ static void pim_bsm_rpinfo_free(struct bsm_rpinfo *bsrp_info)
 	XFREE(MTYPE_PIM_BSRP_INFO, bsrp_info);
 }
 
-void pim_bsm_rpinfos_free(struct bsm_rpinfos_head *head)
+static void pim_bsm_rpinfos_free(struct bsm_rpinfos_head *head)
 {
 	struct bsm_rpinfo *bsrp_info;
 
@@ -78,14 +78,14 @@ void pim_bsm_rpinfos_free(struct bsm_rpinfos_head *head)
 		pim_bsm_rpinfo_free(bsrp_info);
 }
 
-void pim_free_bsgrp_data(struct bsgrp_node *bsgrp_node)
+static void pim_free_bsgrp_data(struct bsgrp_node *bsgrp_node)
 {
 	pim_bsm_rpinfos_free(bsgrp_node->bsrp_list);
 	pim_bsm_rpinfos_free(bsgrp_node->partial_bsrp_list);
 	XFREE(MTYPE_PIM_BSGRP_NODE, bsgrp_node);
 }
 
-void pim_free_bsgrp_node(struct route_table *rt, struct prefix *grp)
+static void pim_free_bsgrp_node(struct route_table *rt, struct prefix *grp)
 {
 	struct route_node *rn;
 
@@ -102,7 +102,7 @@ static void pim_bsm_frag_free(struct bsm_frag *bsfrag)
 	XFREE(MTYPE_PIM_BSM_FRAG, bsfrag);
 }
 
-void pim_bsm_frags_free(struct bsm_scope *scope)
+static void pim_bsm_frags_free(struct bsm_scope *scope)
 {
 	struct bsm_frag *bsfrag;
 
@@ -202,7 +202,7 @@ static int pim_on_bs_timer(struct thread *t)
 	return 0;
 }
 
-void pim_bs_timer_stop(struct bsm_scope *scope)
+static void pim_bs_timer_stop(struct bsm_scope *scope)
 {
 	if (PIM_DEBUG_BSM)
 		zlog_debug("%s : BS timer being stopped of sz: %d", __func__,
@@ -569,6 +569,122 @@ static void pim_bsm_update(struct pim_instance *pim, struct in_addr bsr,
 	pim->global_scope.current_bsr_last_ts = pim_time_monotonic_sec();
 }
 
+void pim_bsm_clear(struct pim_instance *pim)
+{
+	struct route_node *rn;
+	struct route_node *rpnode;
+	struct bsgrp_node *bsgrp;
+	struct prefix nht_p;
+	struct prefix g_all;
+	struct rp_info *rp_all;
+	struct pim_upstream *up;
+	struct rp_info *rp_info;
+
+	if (pim->global_scope.current_bsr.s_addr)
+		pim_nht_bsr_del(pim, pim->global_scope.current_bsr);
+
+	/* Reset scope zone data */
+	pim->global_scope.accept_nofwd_bsm = false;
+	pim->global_scope.state = ACCEPT_ANY;
+	pim->global_scope.current_bsr.s_addr = INADDR_ANY;
+	pim->global_scope.current_bsr_prio = 0;
+	pim->global_scope.current_bsr_first_ts = 0;
+	pim->global_scope.current_bsr_last_ts = 0;
+	pim->global_scope.bsm_frag_tag = 0;
+	pim_bsm_frags_free(&pim->global_scope);
+
+	pim_bs_timer_stop(&pim->global_scope);
+
+	for (rn = route_top(pim->global_scope.bsrp_table); rn;
+	     rn = route_next(rn)) {
+		bsgrp = rn->info;
+		if (!bsgrp)
+			continue;
+
+		rpnode = route_node_lookup(pim->rp_table, &bsgrp->group);
+
+		if (!rpnode) {
+			pim_free_bsgrp_node(bsgrp->scope->bsrp_table,
+					    &bsgrp->group);
+			pim_free_bsgrp_data(bsgrp);
+			continue;
+		}
+
+		rp_info = (struct rp_info *)rpnode->info;
+
+		if ((!rp_info) || (rp_info->rp_src != RP_SRC_BSR)) {
+			pim_free_bsgrp_node(bsgrp->scope->bsrp_table,
+					    &bsgrp->group);
+			pim_free_bsgrp_data(bsgrp);
+			continue;
+		}
+
+		/* Deregister addr with Zebra NHT */
+		nht_p.family = AF_INET;
+		nht_p.prefixlen = IPV4_MAX_BITLEN;
+		nht_p.u.prefix4 = rp_info->rp.rpf_addr.u.prefix4;
+
+		if (PIM_DEBUG_PIM_NHT_RP) {
+			zlog_debug("%s: Deregister RP addr %pFX with Zebra ",
+				   __func__, &nht_p);
+		}
+
+		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info);
+
+		if (!str2prefix("224.0.0.0/4", &g_all))
+			return;
+
+		rp_all = pim_rp_find_match_group(pim, &g_all);
+
+		if (rp_all == rp_info) {
+			rp_all->rp.rpf_addr.family = AF_INET;
+			rp_all->rp.rpf_addr.u.prefix4.s_addr = INADDR_NONE;
+			rp_all->i_am_rp = 0;
+		} else {
+			/* Delete the rp_info from rp-list */
+			listnode_delete(pim->rp_list, rp_info);
+
+			/* Delete the rp node from rp_table */
+			rpnode->info = NULL;
+			route_unlock_node(rpnode);
+			route_unlock_node(rpnode);
+			XFREE(MTYPE_PIM_RP, rp_info);
+		}
+
+		pim_free_bsgrp_node(bsgrp->scope->bsrp_table, &bsgrp->group);
+		pim_free_bsgrp_data(bsgrp);
+	}
+	pim_rp_refresh_group_to_rp_mapping(pim);
+
+
+	frr_each (rb_pim_upstream, &pim->upstream_head, up) {
+		/* Find the upstream (*, G) whose upstream address is same as
+		 * the RP
+		 */
+		if (up->sg.src.s_addr != INADDR_ANY)
+			continue;
+
+		struct prefix grp;
+		struct rp_info *trp_info;
+
+		grp.family = AF_INET;
+		grp.prefixlen = IPV4_MAX_BITLEN;
+		grp.u.prefix4 = up->sg.grp;
+
+		trp_info = pim_rp_find_match_group(pim, &grp);
+
+		/* RP not found for the group grp */
+		if (pim_rpf_addr_is_inaddr_none(&trp_info->rp)) {
+			pim_upstream_rpf_clear(pim, up);
+			pim_rp_set_upstream_addr(pim, &up->upstream_addr,
+						 up->sg.src, up->sg.grp);
+		} else {
+			/* RP found for the group grp */
+			pim_upstream_update(pim, up);
+		}
+	}
+}
+
 static bool pim_bsm_send_intf(uint8_t *buf, int len, struct interface *ifp,
 			      struct in_addr dst_addr)
 {
diff --git a/pimd/pim_bsm.h b/pimd/pim_bsm.h
index dbfeeceec8..a536b50688 100644
--- a/pimd/pim_bsm.h
+++ b/pimd/pim_bsm.h
@@ -205,6 +205,7 @@ struct bsmmsg_rpinfo {
 /* API */
 void pim_bsm_proc_init(struct pim_instance *pim);
 void pim_bsm_proc_free(struct pim_instance *pim);
+void pim_bsm_clear(struct pim_instance *pim);
 void pim_bsm_write_config(struct vty *vty, struct interface *ifp);
 int  pim_bsm_process(struct interface *ifp,
 		     struct ip *ip_hdr,
@@ -214,9 +215,4 @@ int  pim_bsm_process(struct interface *ifp,
 bool pim_bsm_new_nbr_fwd(struct pim_neighbor *neigh, struct interface *ifp);
 struct bsgrp_node *pim_bsm_get_bsgrp_node(struct bsm_scope *scope,
 					  struct prefix *grp);
-void pim_bs_timer_stop(struct bsm_scope *scope);
-void pim_bsm_frags_free(struct bsm_scope *scope);
-void pim_bsm_rpinfos_free(struct bsm_rpinfos_head *head);
-void pim_free_bsgrp_data(struct bsgrp_node *bsgrp_node);
-void pim_free_bsgrp_node(struct route_table *rt, struct prefix *grp);
 #endif
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index 518fcb00e1..82627e5e84 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -4051,123 +4051,6 @@ DEFUN (clear_ip_pim_oil,
 	return CMD_SUCCESS;
 }
 
-static void clear_pim_bsr_db(struct pim_instance *pim)
-{
-	struct route_node *rn;
-	struct route_node *rpnode;
-	struct bsgrp_node *bsgrp;
-	struct prefix nht_p;
-	struct prefix g_all;
-	struct rp_info *rp_all;
-	struct pim_upstream *up;
-	struct rp_info *rp_info;
-
-	if (pim->global_scope.current_bsr.s_addr)
-		pim_nht_bsr_del(pim, pim->global_scope.current_bsr);
-
-	/* Reset scope zone data */
-	pim->global_scope.accept_nofwd_bsm = false;
-	pim->global_scope.state = ACCEPT_ANY;
-	pim->global_scope.current_bsr.s_addr = INADDR_ANY;
-	pim->global_scope.current_bsr_prio = 0;
-	pim->global_scope.current_bsr_first_ts = 0;
-	pim->global_scope.current_bsr_last_ts = 0;
-	pim->global_scope.bsm_frag_tag = 0;
-	pim_bsm_frags_free(&pim->global_scope);
-
-	pim_bs_timer_stop(&pim->global_scope);
-
-	for (rn = route_top(pim->global_scope.bsrp_table); rn;
-	     rn = route_next(rn)) {
-		bsgrp = rn->info;
-		if (!bsgrp)
-			continue;
-
-		rpnode = route_node_lookup(pim->rp_table, &bsgrp->group);
-
-		if (!rpnode) {
-			pim_free_bsgrp_node(bsgrp->scope->bsrp_table,
-					    &bsgrp->group);
-			pim_free_bsgrp_data(bsgrp);
-			continue;
-		}
-
-		rp_info = (struct rp_info *)rpnode->info;
-
-		if ((!rp_info) || (rp_info->rp_src != RP_SRC_BSR)) {
-			pim_free_bsgrp_node(bsgrp->scope->bsrp_table,
-					    &bsgrp->group);
-			pim_free_bsgrp_data(bsgrp);
-			continue;
-		}
-
-		/* Deregister addr with Zebra NHT */
-		nht_p.family = AF_INET;
-		nht_p.prefixlen = IPV4_MAX_BITLEN;
-		nht_p.u.prefix4 = rp_info->rp.rpf_addr.u.prefix4;
-
-		if (PIM_DEBUG_PIM_NHT_RP) {
-			zlog_debug("%s: Deregister RP addr %pFX with Zebra ",
-				   __func__, &nht_p);
-		}
-
-		pim_delete_tracked_nexthop(pim, &nht_p, NULL, rp_info);
-
-		if (!str2prefix("224.0.0.0/4", &g_all))
-			return;
-
-		rp_all = pim_rp_find_match_group(pim, &g_all);
-
-		if (rp_all == rp_info) {
-			rp_all->rp.rpf_addr.family = AF_INET;
-			rp_all->rp.rpf_addr.u.prefix4.s_addr = INADDR_NONE;
-			rp_all->i_am_rp = 0;
-		} else {
-			/* Delete the rp_info from rp-list */
-			listnode_delete(pim->rp_list, rp_info);
-
-			/* Delete the rp node from rp_table */
-			rpnode->info = NULL;
-			route_unlock_node(rpnode);
-			route_unlock_node(rpnode);
-			XFREE(MTYPE_PIM_RP, rp_info);
-		}
-
-		pim_free_bsgrp_node(bsgrp->scope->bsrp_table, &bsgrp->group);
-		pim_free_bsgrp_data(bsgrp);
-	}
-	pim_rp_refresh_group_to_rp_mapping(pim);
-
-
-	frr_each (rb_pim_upstream, &pim->upstream_head, up) {
-		/* Find the upstream (*, G) whose upstream address is same as
-		 * the RP
-		 */
-		if (up->sg.src.s_addr != INADDR_ANY)
-			continue;
-
-		struct prefix grp;
-		struct rp_info *trp_info;
-
-		grp.family = AF_INET;
-		grp.prefixlen = IPV4_MAX_BITLEN;
-		grp.u.prefix4 = up->sg.grp;
-
-		trp_info = pim_rp_find_match_group(pim, &grp);
-
-		/* RP not found for the group grp */
-		if (pim_rpf_addr_is_inaddr_none(&trp_info->rp)) {
-			pim_upstream_rpf_clear(pim, up);
-			pim_rp_set_upstream_addr(pim, &up->upstream_addr,
-						 up->sg.src, up->sg.grp);
-		} else {
-			/* RP found for the group grp */
-			pim_upstream_update(pim, up);
-		}
-	}
-}
-
-
 DEFUN (clear_ip_pim_bsr_db,
        clear_ip_pim_bsr_db_cmd,
        "clear ip pim [vrf NAME] bsr-data",
@@ -4183,7 +4066,7 @@ DEFUN (clear_ip_pim_bsr_db,
 	if (!vrf)
 		return CMD_WARNING;
 
-	clear_pim_bsr_db(vrf->info);
+	pim_bsm_clear(vrf->info);
 
 	return CMD_SUCCESS;
 }

From caef8f7961016da632c7a58b21bd3cc7a72df624 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Mon, 26 Jul 2021 10:47:21 +0200
Subject: [PATCH 5/7] pimd: add back blocking RPF for BSM

NHT won't have a result yet when we get the first BSM from a new BSR.
Hence, the first packet(s) are lost, since their RPF validation fails.

Re-add the blocking RPF check that was there before (though in a much
more sensible manner.)

Also nuke the now-unused pim_nexthop_match* functions.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 pimd/pim_nht.c | 238 ++++++++++++++++---------------------------------
 pimd/pim_nht.h |   4 -
 2 files changed, 79 insertions(+), 163 deletions(-)

diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c
index b5f26d3612..cd6f4c45fa 100644
--- a/pimd/pim_nht.c
+++ b/pimd/pim_nht.c
@@ -282,15 +282,66 @@ bool pim_nht_bsr_rpf_check(struct pim_instance *pim, struct in_addr bsr_addr,
 {
 	struct pim_nexthop_cache *pnc = NULL;
 	struct pim_nexthop_cache lookup;
+	struct pim_neighbor *nbr = NULL;
 	struct nexthop *nh;
+	struct interface *ifp;
 
 	lookup.rpf.rpf_addr.family = AF_INET;
 	lookup.rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN;
 	lookup.rpf.rpf_addr.u.prefix4 = bsr_addr;
 
 	pnc = hash_lookup(pim->rpf_hash, &lookup);
-	if (!pnc)
+	if (!pnc || !CHECK_FLAG(pnc->flags, PIM_NEXTHOP_ANSWER_RECEIVED)) {
+		/* BSM from a new freshly registered BSR - do a synchronous
+		 * zebra query since otherwise we'd drop the first packet,
+		 * leading to additional delay in picking up BSM data
+		 */
+
+		/* FIXME: this should really be moved into a generic NHT
+		 * function that does "add and get immediate result" or maybe
+		 * "check cache or get immediate result." But until that can
+		 * be worked in, here's a copy of the code below :(
+		 */
+		struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM];
+		ifindex_t i;
+		struct interface *ifp = NULL;
+		int num_ifindex;
+
+		memset(nexthop_tab, 0, sizeof(nexthop_tab));
+		num_ifindex = zclient_lookup_nexthop(pim, nexthop_tab,
+						     MULTIPATH_NUM, bsr_addr,
+						     PIM_NEXTHOP_LOOKUP_MAX);
+
+		if (num_ifindex <= 0)
+			return false;
+
+		for (i = 0; i < num_ifindex; i++) {
+			struct pim_zlookup_nexthop *znh = &nexthop_tab[i];
+
+			/* pim_zlookup_nexthop has no ->type */
+
+			/* 1:1 match code below with znh instead of nh */
+			ifp = if_lookup_by_index(znh->ifindex,
+						 pim->vrf->vrf_id);
+
+			if (!ifp || !ifp->info)
+				continue;
+
+			if (if_is_loopback(ifp) && if_is_loopback(src_ifp))
+				return true;
+
+			nbr = pim_neighbor_find(ifp,
+						znh->nexthop_addr.u.prefix4);
+			if (!nbr)
+				continue;
+
+			return znh->ifindex == src_ifp->ifindex
+			       && znh->nexthop_addr.u.prefix4.s_addr
+					  == src_ip.s_addr;
+		}
 		return false;
+	}
+
 	if (!CHECK_FLAG(pnc->flags, PIM_NEXTHOP_VALID))
 		return false;
 
@@ -299,176 +350,45 @@ bool pim_nht_bsr_rpf_check(struct pim_instance *pim, struct in_addr bsr_addr,
 	 * 4-way ECMP and 4 hops you end up with 256 copies of each BSM
 	 * message.
 	 *
-	 * so...  only accept the first (IPv4) nexthop as source.
+	 * so...  only accept the first (IPv4) valid nexthop as source.
 	 */
 
 	for (nh = pnc->nexthop; nh; nh = nh->next) {
-		if (nh->type == NEXTHOP_TYPE_IPV4_IFINDEX) {
-			return nh->ifindex == src_ifp->ifindex
-			       && nh->gate.ipv4.s_addr == src_ip.s_addr;
-		}
-	}
-	return false;
-}
+		struct in_addr nhaddr;
 
-/* Given a source address and a neighbor address, check if the neighbor is one
- * of the next hop to reach the source. search from zebra route database
- */
-bool pim_nexthop_match(struct pim_instance *pim, struct in_addr addr,
-		       struct in_addr ip_src)
-{
-	struct pim_zlookup_nexthop nexthop_tab[MULTIPATH_NUM];
-	int i = 0;
-	ifindex_t first_ifindex = 0;
-	struct interface *ifp = NULL;
-	struct pim_neighbor *nbr = NULL;
-	int num_ifindex;
-
-	if (addr.s_addr == INADDR_NONE)
-		return false;
-
-	memset(nexthop_tab, 0,
-	       sizeof(struct pim_zlookup_nexthop) * MULTIPATH_NUM);
-	num_ifindex = zclient_lookup_nexthop(pim, nexthop_tab, MULTIPATH_NUM,
-					     addr, PIM_NEXTHOP_LOOKUP_MAX);
-	if (num_ifindex < 1) {
-		char addr_str[INET_ADDRSTRLEN];
-
-		pim_inet4_dump("<addr?>", addr, addr_str, sizeof(addr_str));
-		zlog_warn(
-			"%s %s: could not find nexthop ifindex for address %s",
-			__FILE__, __func__, addr_str);
-		return false;
-	}
-
-	while (i < num_ifindex) {
-		first_ifindex = nexthop_tab[i].ifindex;
-
-		ifp = if_lookup_by_index(first_ifindex, pim->vrf->vrf_id);
-		if (!ifp) {
-			if (PIM_DEBUG_ZEBRA) {
-				char addr_str[INET_ADDRSTRLEN];
-
-				pim_inet4_dump("<addr?>", addr, addr_str,
-					       sizeof(addr_str));
-				zlog_debug(
-					"%s %s: could not find interface for ifindex %d (address %s)",
-					__FILE__, __func__, first_ifindex,
-					addr_str);
-			}
-			i++;
-			continue;
-		}
-
-		if (!ifp->info) {
-			if (PIM_DEBUG_ZEBRA) {
-				char addr_str[INET_ADDRSTRLEN];
-
-				pim_inet4_dump("<addr?>", addr, addr_str,
-					       sizeof(addr_str));
-				zlog_debug(
-					"%s: multicast not enabled on input interface %s (ifindex=%d, RPF for source %s)",
-					__func__, ifp->name, first_ifindex,
-					addr_str);
-			}
-			i++;
-			continue;
-		}
-
-		if (!pim_if_connected_to_source(ifp, addr)) {
-			nbr = pim_neighbor_find(
-				ifp, nexthop_tab[i].nexthop_addr.u.prefix4);
-			if (PIM_DEBUG_PIM_TRACE_DETAIL)
-				zlog_debug("ifp name: %s, pim nbr: %p",
-					   ifp->name, nbr);
-			if (!nbr && !if_is_loopback(ifp)) {
-				i++;
+		switch (nh->type) {
+		case NEXTHOP_TYPE_IPV4:
+			if (nh->ifindex == IFINDEX_INTERNAL)
 				continue;
-			}
+
+			/* fallthru */
+		case NEXTHOP_TYPE_IPV4_IFINDEX:
+			nhaddr = nh->gate.ipv4;
+			break;
+
+		case NEXTHOP_TYPE_IFINDEX:
+			nhaddr = bsr_addr;
+			break;
+
+		default:
+			continue;
 		}
 
-		if (nexthop_tab[i].nexthop_addr.u.prefix4.s_addr
-		    == ip_src.s_addr)
+		ifp = if_lookup_by_index(nh->ifindex, pim->vrf->vrf_id);
+		if (!ifp || !ifp->info)
+			continue;
+
+		if (if_is_loopback(ifp) && if_is_loopback(src_ifp))
 			return true;
 
-		i++;
-	}
-
-	return false;
-}
-
-/* Given a source address and a neighbor address, check if the neighbor is one
- * of the next hop to reach the source. search from pim next hop cache
- */
-bool pim_nexthop_match_nht_cache(struct pim_instance *pim, struct in_addr addr,
-				 struct in_addr ip_src)
-{
-	struct pim_rpf rpf;
-	ifindex_t first_ifindex;
-	struct interface *ifp = NULL;
-	uint8_t nh_iter = 0;
-	struct pim_neighbor *nbr = NULL;
-	struct nexthop *nh_node = NULL;
-	struct pim_nexthop_cache *pnc = NULL;
-
-	memset(&rpf, 0, sizeof(struct pim_rpf));
-	rpf.rpf_addr.family = AF_INET;
-	rpf.rpf_addr.prefixlen = IPV4_MAX_BITLEN;
-	rpf.rpf_addr.u.prefix4 = addr;
-
-	pnc = pim_nexthop_cache_find(pim, &rpf);
-	if (!pnc || !pnc->nexthop_num)
-		return false;
-
-	for (nh_node = pnc->nexthop; nh_node; nh_node = nh_node->next) {
-		first_ifindex = nh_node->ifindex;
-		ifp = if_lookup_by_index(first_ifindex, pim->vrf->vrf_id);
-		if (!ifp) {
-			if (PIM_DEBUG_PIM_NHT) {
-				char addr_str[INET_ADDRSTRLEN];
-
-				pim_inet4_dump("<addr?>", addr, addr_str,
-					       sizeof(addr_str));
-				zlog_debug(
-					"%s %s: could not find interface for ifindex %d (address %s(%s))",
-					__FILE__, __func__, first_ifindex,
-					addr_str, pim->vrf->name);
-			}
-			nh_iter++;
+		/* MRIB (IGP) may be pointing at a router where PIM is down */
+		nbr = pim_neighbor_find(ifp, nhaddr);
+		if (!nbr)
 			continue;
-		}
-		if (!ifp->info) {
-			if (PIM_DEBUG_PIM_NHT) {
-				char addr_str[INET_ADDRSTRLEN];
 
-				pim_inet4_dump("<addr?>", addr, addr_str,
-					       sizeof(addr_str));
-				zlog_debug(
-					"%s: multicast not enabled on input interface %s(%s) (ifindex=%d, RPF for source %s)",
-					__func__, ifp->name, pim->vrf->name,
-					first_ifindex, addr_str);
-			}
-			nh_iter++;
-			continue;
-		}
-
-		if (!pim_if_connected_to_source(ifp, addr)) {
-			nbr = pim_neighbor_find(ifp, nh_node->gate.ipv4);
-			if (!nbr && !if_is_loopback(ifp)) {
-				if (PIM_DEBUG_PIM_NHT)
-					zlog_debug(
-						"%s: pim nbr not found on input interface %s(%s)",
-						__func__, ifp->name,
-						pim->vrf->name);
-				nh_iter++;
-				continue;
-			}
-		}
-
-		if (nh_node->gate.ipv4.s_addr == ip_src.s_addr)
-			return true;
+		return nh->ifindex == src_ifp->ifindex
+		       && nhaddr.s_addr == src_ip.s_addr;
 	}
-
 	return false;
 }
 
diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h
index e7d6825409..568c2eb232 100644
--- a/pimd/pim_nht.h
+++ b/pimd/pim_nht.h
@@ -70,10 +70,6 @@ void pim_sendmsg_zebra_rnh(struct pim_instance *pim, struct zclient *zclient,
 int pim_ecmp_fib_lookup_if_vif_index(struct pim_instance *pim,
 				     struct prefix *src, struct prefix *grp);
 void pim_rp_nexthop_del(struct rp_info *rp_info);
-bool pim_nexthop_match(struct pim_instance *pim, struct in_addr addr,
-		       struct in_addr ip_src);
-bool pim_nexthop_match_nht_cache(struct pim_instance *pim, struct in_addr addr,
-				 struct in_addr ip_src);
 
 /* for RPF check on BSM message receipt */
 void pim_nht_bsr_add(struct pim_instance *pim, struct in_addr bsr_addr);

From 56be7c7ed1384127c7caa5317e6de90c0f4c223f Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Thu, 23 Sep 2021 15:04:24 +0200
Subject: [PATCH 6/7] tests: add one more BSR check to pim_bsmp_01

This is implicitly checked by the "verify mroute" below, but it's much
more helpful to explicitly check in advance.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 .../multicast_pim_bsm_topo1/test_mcast_pim_bsmp_01.py         | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/topotests/multicast_pim_bsm_topo1/test_mcast_pim_bsmp_01.py b/tests/topotests/multicast_pim_bsm_topo1/test_mcast_pim_bsmp_01.py
index a94dcb505a..38560c06e4 100644
--- a/tests/topotests/multicast_pim_bsm_topo1/test_mcast_pim_bsmp_01.py
+++ b/tests/topotests/multicast_pim_bsm_topo1/test_mcast_pim_bsmp_01.py
@@ -848,6 +848,10 @@ def test_new_router_fwd_p0(request):
     assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result)
     do_countdown(5)
 
+    step("Verify again if BSR is installed from bsm forwarded by i1")
+    result = verify_pim_bsr(tgen, topo, "l1", bsr_ip)
+    assert result is True, "Testcase {} :Failed \n Error {}".format(tc_name, result)
+
     # Verify ip mroute populated again
     step("Verify mroute again on l1 (lhr)")
     result = verify_ip_mroutes(tgen, "l1", src_addr, GROUP_ADDRESS, iif, oil)

From 43038bd5ef313382a9c8da73342ab193bdb178c7 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Thu, 22 Jul 2021 11:49:08 +0200
Subject: [PATCH 7/7] pimd: correctly process rp-count==0 BSMs

rp-count==0 isn't a broken BSM, it just means the BSR no longer has any
Candidate RPs for the group range.  Previous behavior is badly mistaken
since it stops processing the entire packet.

Fix to correctly remove group range on rp-count==0 and continue
processing remainder of the packet.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---
 pimd/pim_bsm.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c
index 2078a859e5..4689b27034 100644
--- a/pimd/pim_bsm.c
+++ b/pimd/pim_bsm.c
@@ -1168,18 +1168,6 @@ static bool pim_bsm_parse_install_g2rp(struct bsm_scope *scope, uint8_t *buf,
 		buf += sizeof(struct bsmmsg_grpinfo);
 		offset += sizeof(struct bsmmsg_grpinfo);
 
-		if (grpinfo.rp_count == 0) {
-			if (PIM_DEBUG_BSM) {
-				char grp_str[INET_ADDRSTRLEN];
-
-				pim_inet4_dump("<Group?>", grpinfo.group.addr,
-					       grp_str, sizeof(grp_str));
-				zlog_debug("%s, Rp count is zero for group: %s",
-					   __func__, grp_str);
-			}
-			return false;
-		}
-
 		group.family = AF_INET;
 		if (grpinfo.group.mask > IPV4_MAX_BITLEN) {
 			if (PIM_DEBUG_BSM)
@@ -1194,6 +1182,32 @@ static bool pim_bsm_parse_install_g2rp(struct bsm_scope *scope, uint8_t *buf,
 		/* Get the Group node for the BSM rp table */
 		bsgrp = pim_bsm_get_bsgrp_node(scope, &group);
 
+		if (grpinfo.rp_count == 0) {
+			struct bsm_rpinfo *old_rpinfo;
+
+			/* BSR explicitly no longer has RPs for this group */
+			if (!bsgrp)
+				continue;
+
+			if (PIM_DEBUG_BSM) {
+				char grp_str[INET_ADDRSTRLEN];
+
+				pim_inet4_dump("<Group?>", grpinfo.group.addr,
+					       grp_str, sizeof(grp_str));
+				zlog_debug("%s, Rp count is zero for group: %s",
+					   __func__, grp_str);
+			}
+
+			old_rpinfo = bsm_rpinfos_first(bsgrp->bsrp_list);
+			if (old_rpinfo)
+				pim_rp_del(scope->pim, old_rpinfo->rp_address,
+					   group, NULL, RP_SRC_BSR);
+
+			pim_free_bsgrp_node(scope->bsrp_table, &bsgrp->group);
+			pim_free_bsgrp_data(bsgrp);
+			continue;
+		}
+
 		if (!bsgrp) {
 			if (PIM_DEBUG_BSM)
 				zlog_debug("%s, Create new  BSM Group node.",