Merge pull request #5158 from opensourcerouting/72-bfdd-bug-fixes

[7.2] bfdd: pack of bug fixes

bfdd/bfd.c

@@ -213,9 +213,13 @@ void bfd_session_disable(struct bfd_session *bs)
 
 	/* Disable all timers. */
 	bfd_recvtimer_delete(bs);
-	bfd_echo_recvtimer_delete(bs);
 	bfd_xmttimer_delete(bs);
-	bfd_echo_xmttimer_delete(bs);
+	ptm_bfd_echo_stop(bs);
+	bs->vrf = NULL;
+	bs->ifp = NULL;
+
+	/* Set session down so it doesn't report UP and disabled. */
+	ptm_bfd_sess_dn(bs, BD_PATH_DOWN);
 }
 
 static uint32_t ptm_bfd_gen_ID(void)
@@ -328,7 +332,14 @@ void ptm_bfd_sess_dn(struct bfd_session *bfd, uint8_t diag)
 	bfd->demand_mode = 0;
 	monotime(&bfd->downtime);
 
-	ptm_bfd_snd(bfd, 0);
+	/*
+	 * Only attempt to send if we have a valid socket:
+	 * this function might be called by session disablers and in
+	 * this case we won't have a valid socket (i.e. interface was
+	 * removed or VRF doesn't exist anymore).
+	 */
+	if (bfd->sock != -1)
+		ptm_bfd_snd(bfd, 0);
 
 	/* Slow down the control packets, the connection is down. */
 	bs_set_slow_timers(bfd);
@@ -1208,19 +1219,10 @@ int bs_observer_add(struct bfd_session *bs)
 	struct bfd_session_observer *bso;
 
 	bso = XCALLOC(MTYPE_BFDD_SESSION_OBSERVER, sizeof(*bso));
-	bso->bso_isaddress = false;
 	bso->bso_bs = bs;
-	bso->bso_isinterface = !BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_MH);
+	bso->bso_addr.family = bs->key.family;
-	if (bso->bso_isinterface)
+	memcpy(&bso->bso_addr.u.prefix, &bs->key.local,
-		strlcpy(bso->bso_entryname, bs->key.ifname,
+	       sizeof(bs->key.local));
-			sizeof(bso->bso_entryname));
-	/* Handle socket binding failures caused by missing local addresses. */
-	if (bs->sock == -1) {
-		bso->bso_isaddress = true;
-		bso->bso_addr.family = bs->key.family;
-		memcpy(&bso->bso_addr.u.prefix, &bs->key.local,
-		       sizeof(bs->key.local));
-	}
 
 	TAILQ_INSERT_TAIL(&bglobal.bg_obslist, bso, bso_entry);
 
@@ -1434,7 +1436,7 @@ struct bfd_session *bfd_key_lookup(struct bfd_key key)
 	if (ctx.result) {
 		bsp = ctx.result;
 		log_debug(" peer %s found, but ifp"
-			  " and/or loc-addr params ignored");
+			  " and/or loc-addr params ignored", peer_buf);
 	}
 	return bsp;
 }
@@ -1700,6 +1702,15 @@ static int bfd_vrf_disable(struct vrf *vrf)
 	}
 
 	log_debug("VRF disable %s id %d", vrf->name, vrf->vrf_id);
+
+	/* Disable read/write poll triggering. */
+	THREAD_OFF(bvrf->bg_ev[0]);
+	THREAD_OFF(bvrf->bg_ev[1]);
+	THREAD_OFF(bvrf->bg_ev[2]);
+	THREAD_OFF(bvrf->bg_ev[3]);
+	THREAD_OFF(bvrf->bg_ev[4]);
+	THREAD_OFF(bvrf->bg_ev[5]);
+
 	/* Close all descriptors. */
 	socket_close(&bvrf->bg_echo);
 	socket_close(&bvrf->bg_shop);

bfdd/bfd.h

@@ -274,12 +274,8 @@ struct bfd_state_str_list {
 
 struct bfd_session_observer {
 	struct bfd_session *bso_bs;
-	bool bso_isinterface;
+	char bso_entryname[MAXNAMELEN];
-	bool bso_isaddress;
+	struct prefix bso_addr;
-	union {
-		char bso_entryname[MAXNAMELEN];
-		struct prefix bso_addr;
-	};
 
 	TAILQ_ENTRY(bfd_session_observer) bso_entry;
 };

bfdd/bfdd_northbound.c

@@ -58,10 +58,39 @@ static int bfd_session_create(enum nb_event event, const struct lyd_node *dnode,
 			      union nb_resource *resource, bool mhop)
 {
 	struct bfd_session *bs;
+	const char *ifname;
 	struct bfd_key bk;
+	struct in6_addr i6a;
 
 	switch (event) {
 	case NB_EV_VALIDATE:
+		/*
+		 * When `dest-addr` is IPv6 and link-local we must
+		 * require interface name, otherwise we can't figure
+		 * which interface to use to send the packets.
+		 *
+		 * `memset` `i6a` in case address is IPv4 or non
+		 * link-local IPv6, it should also avoid static
+		 * analyzer warning about unset memory read.
+		 */
+		memset(&i6a, 0, sizeof(i6a));
+		yang_dnode_get_ipv6(&i6a, dnode, "./dest-addr");
+
+		/*
+		 * To support old FRR versions we must allow empty
+		 * interface to be specified, however that should
+		 * change in the future.
+		 */
+		if (yang_dnode_exists(dnode, "./interface"))
+			ifname = yang_dnode_get_string(dnode, "./interface");
+		else
+			ifname = "";
+
+		if (IN6_IS_ADDR_LINKLOCAL(&i6a) && strlen(ifname) == 0) {
+			zlog_warn("%s: when using link-local you must specify "
+				  "an interface.", __func__);
+			return NB_ERR_VALIDATION;
+		}
 		break;
 
 	case NB_EV_PREPARE:

bfdd/ptm_adapter.c

@@ -576,8 +576,6 @@ static void bfdd_sessions_enable_interface(struct interface *ifp)
 
 	TAILQ_FOREACH(bso, &bglobal.bg_obslist, bso_entry) {
 		bs = bso->bso_bs;
-		if (bso->bso_isinterface == false)
-			continue;
 		/* Interface name mismatch. */
 		if (strcmp(ifp->name, bs->key.ifname))
 			continue;
@@ -602,10 +600,6 @@ static void bfdd_sessions_disable_interface(struct interface *ifp)
 	struct bfd_session *bs;
 
 	TAILQ_FOREACH(bso, &bglobal.bg_obslist, bso_entry) {
-		if (bso->bso_isinterface == false)
-			continue;
-
-		/* Interface name mismatch. */
 		bs = bso->bso_bs;
 		if (strcmp(ifp->name, bs->key.ifname))
 			continue;
@@ -613,7 +607,6 @@ static void bfdd_sessions_disable_interface(struct interface *ifp)
 		if (bs->sock == -1)
 			continue;
 
-		/* Try to enable it. */
 		bfd_session_disable(bs);
 
 	}
@@ -650,8 +643,6 @@ void bfdd_sessions_disable_vrf(struct vrf *vrf)
 	struct bfd_session *bs;
 
 	TAILQ_FOREACH(bso, &bglobal.bg_obslist, bso_entry) {
-		if (bso->bso_isinterface)
-			continue;
 		bs = bso->bso_bs;
 		if (bs->key.vrfname[0] &&
 		    strcmp(vrf->name, bs->key.vrfname))
@@ -660,7 +651,6 @@ void bfdd_sessions_disable_vrf(struct vrf *vrf)
 		if (bs->sock == -1)
 			continue;
 
-		/* Try to enable it. */
 		bfd_session_disable(bs);
 	}
 }
@@ -716,9 +706,6 @@ static void bfdd_sessions_enable_address(struct connected *ifc)
 	struct prefix prefix;
 
 	TAILQ_FOREACH(bso, &bglobal.bg_obslist, bso_entry) {
-		if (bso->bso_isaddress == false)
-			continue;
-
 		/* Skip enabled sessions. */
 		bs = bso->bso_bs;
 		if (bs->sock != -1)