Merge pull request #5528 from opensourcerouting/bmp-dns-fixing

BMP: improve active outbound connection details

bgpd/bgp_bmp.c

@@ -1662,34 +1662,31 @@ static void bmp_active_connect(struct bmp_active *ba)
 	bmp_active_setup(ba);
 }
 
-static void bmp_active_resolved(struct resolver_query *resq, int numaddrs,
-				union sockunion *addr)
+static void bmp_active_resolved(struct resolver_query *resq, const char *errstr,
+				int numaddrs, union sockunion *addr)
 {
 	struct bmp_active *ba = container_of(resq, struct bmp_active, resq);
 	unsigned i;
 
 	if (numaddrs <= 0) {
-		int ret;
-
+		zlog_warn("bmp[%s]: hostname resolution failed: %s",
+			  ba->hostname, errstr);
+		ba->last_err = errstr;
+		ba->curretry += ba->curretry / 2;
 		ba->addrpos = 0;
-		ba->addrtotal = 1;
-		ret = str2sockunion(ba->hostname, &ba->addrs[0]);
-		if (ret < 0) {
-			ba->addrtotal = 0;
-			ba->curretry += ba->curretry / 2;
-			bmp_active_setup(ba);
-			return;
-		}
-	} else {
-		if (numaddrs > (int)array_size(ba->addrs))
-			numaddrs = array_size(ba->addrs);
-
-		ba->addrpos = 0;
-		ba->addrtotal = numaddrs;
-		for (i = 0; i < ba->addrtotal; i++)
-			memcpy(&ba->addrs[i], &addr[i], sizeof(ba->addrs[0]));
+		ba->addrtotal = 0;
+		bmp_active_setup(ba);
+		return;
 	}
 
+	if (numaddrs > (int)array_size(ba->addrs))
+		numaddrs = array_size(ba->addrs);
+
+	ba->addrpos = 0;
+	ba->addrtotal = numaddrs;
+	for (i = 0; i < ba->addrtotal; i++)
+		memcpy(&ba->addrs[i], &addr[i], sizeof(ba->addrs[0]));
+
 	bmp_active_connect(ba);
 }
 
@@ -1706,6 +1703,8 @@ static int bmp_active_thread(struct thread *t)
 	THREAD_OFF(ba->t_read);
 	THREAD_OFF(ba->t_write);
 
+	ba->last_err = NULL;
+
 	if (ba->socket == -1) {
 		resolver_resolve(&ba->resq, AF_UNSPEC, ba->hostname,
 				 bmp_active_resolved);
@@ -1718,8 +1717,9 @@ static int bmp_active_thread(struct thread *t)
 
 	sockunion2str(&ba->addrs[ba->addrpos], buf, sizeof(buf));
 	if (ret < 0 || status != 0) {
-		zlog_warn("bmp[%s]: failed to connect to %s:%d",
-			  ba->hostname, buf, ba->port);
+		ba->last_err = strerror(status);
+		zlog_warn("bmp[%s]: failed to connect to %s:%d: %s",
+			  ba->hostname, buf, ba->port, ba->last_err);
 		goto out_next;
 	}
 
@@ -2079,9 +2079,12 @@ DEFPY(show_bmp,
 	struct bmp_bgp *bmpbgp;
 	struct bmp_targets *bt;
 	struct bmp_listener *bl;
+	struct bmp_active *ba;
 	struct bmp *bmp;
 	struct ttable *tt;
 	char buf[SU_ADDRSTRLEN];
+	char uptime[BGP_UPTIME_LEN];
+	char *out;
 
 	frr_each(bmp_bgph, &bmp_bgph, bmpbgp) {
 		vty_out(vty, "BMP state for BGP %s:\n\n",
@@ -2130,6 +2133,51 @@ DEFPY(show_bmp,
 					sockunion2str(&bl->addr, buf,
 						      SU_ADDRSTRLEN), bl->port);
 
+			vty_out(vty, "\n    Outbound connections:\n");
+			tt = ttable_new(&ttable_styles[TTSTYLE_BLANK]);
+			ttable_add_row(tt, "remote|state||timer");
+			ttable_rowseps(tt, 0, BOTTOM, true, '-');
+			frr_each (bmp_actives, &bt->actives, ba) {
+				const char *state_str = "?";
+
+				if (ba->bmp) {
+					peer_uptime(ba->bmp->t_up.tv_sec,
+						    uptime, sizeof(uptime),
+						    false, NULL);
+					ttable_add_row(tt, "%s:%d|Up|%s|%s",
+						       ba->hostname, ba->port,
+						       ba->bmp->remote, uptime);
+					continue;
+				}
+
+				uptime[0] = '\0';
+
+				if (ba->t_timer) {
+					long trem = thread_timer_remain_second(
+						ba->t_timer);
+
+					peer_uptime(monotime(NULL) - trem,
+						    uptime, sizeof(uptime),
+						    false, NULL);
+					state_str = "RetryWait";
+				} else if (ba->t_read) {
+					state_str = "Connecting";
+				} else if (ba->resq.callback) {
+					state_str = "Resolving";
+				}
+
+				ttable_add_row(tt, "%s:%d|%s|%s|%s",
+					       ba->hostname, ba->port,
+					       state_str,
+					       ba->last_err ? ba->last_err : "",
+					       uptime);
+				continue;
+			}
+			out = ttable_dump(tt, "\n");
+			vty_out(vty, "%s", out);
+			XFREE(MTYPE_TMP, out);
+			ttable_del(tt);
+
 			vty_out(vty, "\n    %zu connected clients:\n",
 					bmp_session_count(&bt->sessions));
 			tt = ttable_new(&ttable_styles[TTSTYLE_BLANK]);
@@ -2142,14 +2190,17 @@ DEFPY(show_bmp,
 
 				pullwr_stats(bmp->pullwr, &total, &q, &kq);
 
-				ttable_add_row(tt, "%s|-|%Lu|%Lu|%Lu|%Lu|%zu|%zu",
-					       bmp->remote,
+				peer_uptime(bmp->t_up.tv_sec, uptime,
+					    sizeof(uptime), false, NULL);
+
+				ttable_add_row(tt, "%s|%s|%Lu|%Lu|%Lu|%Lu|%zu|%zu",
+					       bmp->remote, uptime,
 					       bmp->cnt_update,
 					       bmp->cnt_mirror,
 					       bmp->cnt_mirror_overruns,
 					       total, q, kq);
 			}
-			char *out = ttable_dump(tt, "\n");
+			out = ttable_dump(tt, "\n");
 			vty_out(vty, "%s", out);
 			XFREE(MTYPE_TMP, out);
 			ttable_del(tt);

bgpd/bgp_bmp.h

@@ -182,6 +182,7 @@ struct bmp_active {
 	unsigned addrpos, addrtotal;
 	union sockunion addrs[8];
 	int socket;
+	const char *last_err;
 	struct thread *t_timer, *t_read, *t_write;
 };
 

lib/resolver.c

@@ -145,7 +145,8 @@ static void ares_address_cb(void *arg, int status, int timeouts,
 {
 	struct resolver_query *query = (struct resolver_query *)arg;
 	union sockunion addr[16];
-	void (*callback)(struct resolver_query *, int, union sockunion *);
+	void (*callback)(struct resolver_query *, const char *, int,
+			 union sockunion *);
 	size_t i;
 
 	callback = query->callback;
@@ -153,9 +154,10 @@ static void ares_address_cb(void *arg, int status, int timeouts,
 
 	if (status != ARES_SUCCESS) {
 		if (resolver_debug)
-			zlog_debug("[%p] Resolving failed", query);
+			zlog_debug("[%p] Resolving failed (%s)",
+				   query, ares_strerror(status));
 
-		callback(query, -1, NULL);
+		callback(query, ares_strerror(status), -1, NULL);
 		return;
 	}
 
@@ -177,14 +179,29 @@ static void ares_address_cb(void *arg, int status, int timeouts,
 	if (resolver_debug)
 		zlog_debug("[%p] Resolved with %d results", query, (int)i);
 
-	callback(query, i, &addr[0]);
+	callback(query, NULL, i, &addr[0]);
+}
+
+static int resolver_cb_literal(struct thread *t)
+{
+	struct resolver_query *query = THREAD_ARG(t);
+	void (*callback)(struct resolver_query *, const char *, int,
+			 union sockunion *);
+
+	callback = query->callback;
+	query->callback = NULL;
+
+	callback(query, ARES_SUCCESS, 1, &query->literal_addr);
+	return 0;
 }
 
 void resolver_resolve(struct resolver_query *query, int af,
 		      const char *hostname,
-		      void (*callback)(struct resolver_query *, int,
-				       union sockunion *))
+		      void (*callback)(struct resolver_query *, const char *,
+				       int, union sockunion *))
 {
+	int ret;
+
 	if (query->callback != NULL) {
 		flog_err(
 			EC_LIB_RESOLVER,
@@ -193,10 +210,26 @@ void resolver_resolve(struct resolver_query *query, int af,
 		return;
 	}
 
+	query->callback = callback;
+	query->literal_cb = NULL;
+
+	ret = str2sockunion(hostname, &query->literal_addr);
+	if (ret == 0) {
+		if (resolver_debug)
+			zlog_debug("[%p] Resolving '%s' (IP literal)",
+				   query, hostname);
+
+		/* for consistency with proper name lookup, don't call the
+		 * callback immediately; defer to thread loop
+		 */
+		thread_add_timer_msec(state.master, resolver_cb_literal,
+				      query, 0, &query->literal_cb);
+		return;
+	}
+
 	if (resolver_debug)
 		zlog_debug("[%p] Resolving '%s'", query, hostname);
 
-	query->callback = callback;
 	ares_gethostbyname(state.channel, hostname, af, ares_address_cb, query);
 	resolver_update_timeouts(&state);
 }

lib/resolver.h

@@ -14,12 +14,18 @@
 #include "sockunion.h"
 
 struct resolver_query {
-	void (*callback)(struct resolver_query *, int n, union sockunion *);
+	void (*callback)(struct resolver_query *, const char *errstr, int n,
+			 union sockunion *);
+
+	/* used to immediate provide the result if IP literal is passed in */
+	union sockunion literal_addr;
+	struct thread *literal_cb;
 };
 
 void resolver_init(struct thread_master *tm);
 void resolver_resolve(struct resolver_query *query, int af,
 		      const char *hostname, void (*cb)(struct resolver_query *,
-						       int, union sockunion *));
+						       const char *, int,
+						       union sockunion *));
 
 #endif /* _FRR_RESOLVER_H */

nhrpd/nhrp_nhs.c

@@ -238,8 +238,8 @@ nhrp_reg_by_nbma(struct nhrp_nhs *nhs, const union sockunion *nbma_addr)
 	return NULL;
 }
 
-static void nhrp_nhs_resolve_cb(struct resolver_query *q, int n,
-				union sockunion *addrs)
+static void nhrp_nhs_resolve_cb(struct resolver_query *q, const char *errstr,
+				int n, union sockunion *addrs)
 {
 	struct nhrp_nhs *nhs = container_of(q, struct nhrp_nhs, dns_resolve);
 	struct nhrp_interface *nifp = nhs->ifp->info;