bgpd: Use synchronous way to get labels from Zebra

Both the label manager and table manager zapi code send data requests via zapi
to zebra and then immediately listen for a response from zebra. The problem here
is of course that the listen part is throwing away any zapi command that is not
the one it is looking for.

ISIS/OSPF and PIM all have synchronous abilities via zapi, which they all
do through a special zapi connection to zebra. BGP needs to follow this model
as well. Additionally the new zclient_sync connection that should be created,
a once a second timer should wake up and read any data on the socket to
prevent problems too much data accumulating in the socket.

```
r3# sh bgp labelpool summary
Labelpool Summary
-----------------
Ledger:       3
InUse:        3
Requests:     0
LabelChunks:  1
Pending:      128
Reconnects:   1
r3# sh bgp labelpool inuse
Prefix                Label
---------------------------
10.0.0.1/32           16
192.168.31.0/24       17
192.168.32.0/24       18
r3#
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd/bgp_labelpool.c

@@ -15,7 +15,6 @@
 #include "linklist.h"
 #include "skiplist.h"
 #include "workqueue.h"
-#include "zclient.h"
 #include "mpls.h"
 
 #include "bgpd/bgpd.h"
@@ -32,11 +31,6 @@
 #include "bgpd/bgp_labelpool_clippy.c"
 
 
-/*
- * Definitions and external declarations.
- */
-extern struct zclient *zclient;
-
 #if BGP_LABELPOOL_ENABLE_TESTS
 static void lptest_init(void);
 static void lptest_finish(void);
@@ -223,6 +217,8 @@ void bgp_lp_finish(void)
 {
 	struct lp_fifo *lf;
 	struct work_queue_item *item, *titem;
+	struct listnode *node;
+	struct lp_chunk *chunk;
 
 #if BGP_LABELPOOL_ENABLE_TESTS
 	lptest_finish();
@@ -236,6 +232,9 @@ void bgp_lp_finish(void)
 	skiplist_free(lp->inuse);
 	lp->inuse = NULL;
 
+	for (ALL_LIST_ELEMENTS_RO(lp->chunks, node, chunk))
+		bgp_zebra_release_label_range(chunk->first, chunk->last);
+
 	list_delete(&lp->chunks);
 
 	while ((lf = lp_fifo_pop(&lp->requests))) {
@@ -448,15 +447,13 @@ void bgp_lp_get(
 	lp_fifo_add_tail(&lp->requests, lf);
 
 	if (lp_fifo_count(&lp->requests) > lp->pending_count) {
-		if (!zclient || zclient->sock < 0)
+		if (!bgp_zebra_request_label_range(MPLS_LABEL_BASE_ANY,
+						   lp->next_chunksize))
 			return;
-		if (zclient_send_get_label_chunk(zclient, 0, lp->next_chunksize,
+
-						 MPLS_LABEL_BASE_ANY) !=
+		lp->pending_count += lp->next_chunksize;
-		    ZCLIENT_SEND_FAILURE) {
+		if ((lp->next_chunksize << 1) <= LP_CHUNK_SIZE_MAX)
-			lp->pending_count += lp->next_chunksize;
+			lp->next_chunksize <<= 1;
-			if ((lp->next_chunksize << 1) <= LP_CHUNK_SIZE_MAX)
-				lp->next_chunksize <<= 1;
-		}
 	}
 }
 
@@ -503,46 +500,12 @@ void bgp_lp_release(
 	}
 }
 
-/*
+static void bgp_sync_label_manager(struct event *e)
- * zebra response giving us a chunk of labels
- */
-void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
 {
-	struct lp_chunk *chunk;
 	int debug = BGP_DEBUG(labelpool, LABELPOOL);
 	struct lp_fifo *lf;
-	uint32_t labelcount;
 
-	if (last < first) {
+	while ((lf = lp_fifo_pop(&lp->requests))) {
-		flog_err(EC_BGP_LABEL,
-			 "%s: zebra label chunk invalid: first=%u, last=%u",
-			 __func__, first, last);
-		return;
-	}
-
-	chunk = XCALLOC(MTYPE_BGP_LABEL_CHUNK, sizeof(struct lp_chunk));
-
-	labelcount = last - first + 1;
-
-	chunk->first = first;
-	chunk->last = last;
-	chunk->nfree = labelcount;
-	bf_init(chunk->allocated_map, labelcount);
-
-	/*
-	 * Optimize for allocation by adding the new (presumably larger)
-	 * chunk at the head of the list so it is examined first.
-	 */
-	listnode_add_head(lp->chunks, chunk);
-
-	lp->pending_count -= labelcount;
-
-	if (debug) {
-		zlog_debug("%s: %zu pending requests", __func__,
-			lp_fifo_count(&lp->requests));
-	}
-
-	while (labelcount && (lf = lp_fifo_first(&lp->requests))) {
 
 		struct lp_lcb *lcb;
 		void *labelid = lf->lcb.labelid;
@@ -588,8 +551,6 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
 			break;
 		}
 
-		labelcount -= 1;
-
 		/*
 		 * we filled the request from local pool.
 		 * Enqueue response work item with new label.
@@ -610,9 +571,41 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
 		work_queue_add(lp->callback_q, q);
 
 finishedrequest:
-		lp_fifo_del(&lp->requests, lf);
 		XFREE(MTYPE_BGP_LABEL_FIFO, lf);
+}
+
+event_add_timer(bm->master, bgp_sync_label_manager, NULL, 1,
+		&bm->t_bgp_label_manager);
+}
+
+void bgp_lp_event_chunk(uint32_t first, uint32_t last)
+{
+	struct lp_chunk *chunk;
+	uint32_t labelcount;
+
+	if (last < first) {
+		flog_err(EC_BGP_LABEL,
+			 "%s: zebra label chunk invalid: first=%u, last=%u",
+			 __func__, first, last);
+		return;
 	}
+
+	chunk = XCALLOC(MTYPE_BGP_LABEL_CHUNK, sizeof(struct lp_chunk));
+
+	labelcount = last - first + 1;
+
+	chunk->first = first;
+	chunk->last = last;
+	chunk->nfree = labelcount;
+	bf_init(chunk->allocated_map, labelcount);
+
+	/*
+	 * Optimize for allocation by adding the new (presumably larger)
+	 * chunk at the head of the list so it is examined first.
+	 */
+	listnode_add_head(lp->chunks, chunk);
+
+	lp->pending_count -= labelcount;
 }
 
 /*
@@ -634,7 +627,6 @@ void bgp_lp_event_zebra_up(void)
 	unsigned int chunks_needed;
 	void *labelid;
 	struct lp_lcb *lcb;
-	int lm_init_ok;
 
 	lp->reconnect_count++;
 	/*
@@ -654,22 +646,16 @@ void bgp_lp_event_zebra_up(void)
 	chunks_needed = (labels_needed / lp->next_chunksize) + 1;
 	labels_needed = chunks_needed * lp->next_chunksize;
 
-	lm_init_ok = lm_label_manager_connect(zclient, 1) == 0;
-
-	if (!lm_init_ok) {
-		zlog_err("%s: label manager connection error", __func__);
-		return;
-	}
-
-	zclient_send_get_label_chunk(zclient, 0, labels_needed,
-				     MPLS_LABEL_BASE_ANY);
-	lp->pending_count = labels_needed;
-
 	/*
 	 * Invalidate current list of chunks
 	 */
 	list_delete_all_node(lp->chunks);
 
+	if (!bgp_zebra_request_label_range(MPLS_LABEL_BASE_ANY, labels_needed))
+		return;
+
+	lp->pending_count = labels_needed;
+
 	/*
 	 * Invalidate any existing labels and requeue them as requests
 	 */
@@ -712,6 +698,9 @@ void bgp_lp_event_zebra_up(void)
 
 		skiplist_delete_first(lp->inuse);
 	}
+
+	event_add_timer(bm->master, bgp_sync_label_manager, NULL, 1,
+			&bm->t_bgp_label_manager);
 }
 
 DEFUN(show_bgp_labelpool_summary, show_bgp_labelpool_summary_cmd,

bgpd/bgp_labelpool.h

@@ -38,7 +38,7 @@ extern void bgp_lp_finish(void);
 extern void bgp_lp_get(int type, void *labelid,
 	int (*cbfunc)(mpls_label_t label, void *labelid, bool allocated));
 extern void bgp_lp_release(int type, void *labelid, mpls_label_t label);
-extern void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last);
+extern void bgp_lp_event_chunk(uint32_t first, uint32_t last);
 extern void bgp_lp_event_zebra_down(void);
 extern void bgp_lp_event_zebra_up(void);
 extern void bgp_lp_vty_init(void);

bgpd/bgp_zebra.c

@@ -55,6 +55,7 @@
 
 /* All information about zebra. */
 struct zclient *zclient = NULL;
+struct zclient *zclient_sync = NULL;
 
 /* hook to indicate vrf status change for SNMP */
 DEFINE_HOOK(bgp_vrf_status_changed, (struct bgp *bgp, struct interface *ifp),
@@ -2854,9 +2855,6 @@ static void bgp_zebra_connected(struct zclient *zclient)
 
 	bgp_zebra_instance_register(bgp);
 
-	/* tell label pool that zebra is connected */
-	bgp_lp_event_zebra_up();
-
 	/* TODO - What if we have peers and networks configured, do we have to
 	 * kick-start them?
 	 */
@@ -3161,54 +3159,6 @@ static int bgp_zebra_process_local_ip_prefix(ZAPI_CALLBACK_ARGS)
 	return 0;
 }
 
-static int bgp_zebra_process_label_chunk(ZAPI_CALLBACK_ARGS)
-{
-	struct stream *s = NULL;
-	uint8_t response_keep;
-	uint32_t first;
-	uint32_t last;
-	uint8_t proto;
-	unsigned short instance;
-
-	s = zclient->ibuf;
-	STREAM_GETC(s, proto);
-	STREAM_GETW(s, instance);
-	STREAM_GETC(s, response_keep);
-	STREAM_GETL(s, first);
-	STREAM_GETL(s, last);
-
-	if (zclient->redist_default != proto) {
-		flog_err(EC_BGP_LM_ERROR, "Got LM msg with wrong proto %u",
-			 proto);
-		return 0;
-	}
-	if (zclient->instance != instance) {
-		flog_err(EC_BGP_LM_ERROR, "Got LM msg with wrong instance %u",
-			 proto);
-		return 0;
-	}
-
-	if (first > last ||
-		first < MPLS_LABEL_UNRESERVED_MIN ||
-		last > MPLS_LABEL_UNRESERVED_MAX) {
-
-		flog_err(EC_BGP_LM_ERROR, "%s: Invalid Label chunk: %u - %u",
-			 __func__, first, last);
-		return 0;
-	}
-	if (BGP_DEBUG(zebra, ZEBRA)) {
-		zlog_debug("Label Chunk assign: %u - %u (%u) ",
-			first, last, response_keep);
-	}
-
-	bgp_lp_event_chunk(response_keep, first, last);
-
-	return 0;
-
-stream_failure:		/* for STREAM_GETX */
-	return -1;
-}
-
 extern struct zebra_privs_t bgpd_privs;
 
 static int bgp_ifp_create(struct interface *ifp)
@@ -3427,7 +3377,6 @@ static zclient_handler *const bgp_handlers[] = {
 	[ZEBRA_L3VNI_DEL] = bgp_zebra_process_local_l3vni,
 	[ZEBRA_IP_PREFIX_ROUTE_ADD] = bgp_zebra_process_local_ip_prefix,
 	[ZEBRA_IP_PREFIX_ROUTE_DEL] = bgp_zebra_process_local_ip_prefix,
-	[ZEBRA_GET_LABEL_CHUNK] = bgp_zebra_process_label_chunk,
 	[ZEBRA_RULE_NOTIFY_OWNER] = rule_notify_owner,
 	[ZEBRA_IPSET_NOTIFY_OWNER] = ipset_notify_owner,
 	[ZEBRA_IPSET_ENTRY_NOTIFY_OWNER] = ipset_entry_notify_owner,
@@ -3464,8 +3413,45 @@ void bgp_if_init(void)
 	hook_register_prio(if_del, 0, bgp_if_delete_hook);
 }
 
+static void bgp_zebra_label_manager_connect(void)
+{
+	/* Connect to label manager. */
+	if (zclient_socket_connect(zclient_sync) < 0) {
+		zlog_warn("%s: failed connecting synchronous zclient!",
+			  __func__);
+		return;
+	}
+	/* make socket non-blocking */
+	set_nonblocking(zclient_sync->sock);
+
+	/* Send hello to notify zebra this is a synchronous client */
+	if (zclient_send_hello(zclient_sync) == ZCLIENT_SEND_FAILURE) {
+		zlog_warn("%s: failed sending hello for synchronous zclient!",
+			  __func__);
+		close(zclient_sync->sock);
+		zclient_sync->sock = -1;
+		return;
+	}
+
+	/* Connect to label manager */
+	if (lm_label_manager_connect(zclient_sync, 0) != 0) {
+		zlog_warn("%s: failed connecting to label manager!", __func__);
+		if (zclient_sync->sock > 0) {
+			close(zclient_sync->sock);
+			zclient_sync->sock = -1;
+		}
+		return;
+	}
+
+	/* tell label pool that zebra is connected */
+	bgp_lp_event_zebra_up();
+}
+
 void bgp_zebra_init(struct event_loop *master, unsigned short instance)
 {
+	struct zclient_options options = zclient_options_default;
+
+	options.synchronous = true;
 	zclient_num_connects = 0;
 
 	if_zapi_callbacks(bgp_ifp_create, bgp_ifp_up,
@@ -3477,6 +3463,16 @@ void bgp_zebra_init(struct event_loop *master, unsigned short instance)
 	zclient_init(zclient, ZEBRA_ROUTE_BGP, 0, &bgpd_privs);
 	zclient->zebra_connected = bgp_zebra_connected;
 	zclient->instance = instance;
+
+	/* Initialize special zclient for synchronous message exchanges. */
+	zclient_sync = zclient_new(master, &options, NULL, 0);
+	zclient_sync->sock = -1;
+	zclient_sync->redist_default = ZEBRA_ROUTE_BGP;
+	zclient_sync->instance = instance;
+	zclient_sync->session_id = 1;
+	zclient_sync->privs = &bgpd_privs;
+
+	bgp_zebra_label_manager_connect();
 }
 
 void bgp_zebra_destroy(void)
@@ -3486,6 +3482,12 @@ void bgp_zebra_destroy(void)
 	zclient_stop(zclient);
 	zclient_free(zclient);
 	zclient = NULL;
+
+	if (zclient_sync == NULL)
+		return;
+	zclient_stop(zclient_sync);
+	zclient_free(zclient_sync);
+	zclient_sync = NULL;
 }
 
 int bgp_zebra_num_connects(void)
@@ -3951,3 +3953,42 @@ void bgp_zebra_send_nexthop_label(int cmd, mpls_label_t label,
 	/* vrf_id is DEFAULT_VRF */
 	zebra_send_mpls_labels(zclient, cmd, &zl);
 }
+
+bool bgp_zebra_request_label_range(uint32_t base, uint32_t chunk_size)
+{
+	int ret;
+	uint32_t start, end;
+
+	if (!zclient_sync || zclient_sync->sock < 0)
+		return false;
+
+	ret = lm_get_label_chunk(zclient_sync, 0, base, chunk_size, &start,
+				 &end);
+	if (ret < 0) {
+		zlog_warn("%s: error getting label range!", __func__);
+		return false;
+	}
+
+	if (start > end || start < MPLS_LABEL_UNRESERVED_MIN ||
+	    end > MPLS_LABEL_UNRESERVED_MAX) {
+		flog_err(EC_BGP_LM_ERROR, "%s: Invalid Label chunk: %u - %u",
+			 __func__, start, end);
+		return false;
+	}
+
+	bgp_lp_event_chunk(start, end);
+
+	return true;
+}
+
+void bgp_zebra_release_label_range(uint32_t start, uint32_t end)
+{
+	int ret;
+
+	if (!zclient_sync || zclient_sync->sock < 0)
+		return;
+
+	ret = lm_release_label_chunk(zclient_sync, start, end);
+	if (ret < 0)
+		zlog_warn("%s: error releasing label range!", __func__);
+}

bgpd/bgp_zebra.h

@@ -123,4 +123,6 @@ extern void bgp_zebra_send_nexthop_label(int cmd, mpls_label_t label,
 					 enum lsp_types_t ltype,
 					 struct prefix *p, uint32_t num_labels,
 					 mpls_label_t out_labels[]);
+extern bool bgp_zebra_request_label_range(uint32_t base, uint32_t chunk_size);
+extern void bgp_zebra_release_label_range(uint32_t start, uint32_t end);
 #endif /* _QUAGGA_BGP_ZEBRA_H */

bgpd/bgpd.c

@@ -8062,6 +8062,7 @@ void bgp_master_init(struct event_loop *master, const int buffer_size,
 	bm->tcp_dscp = IPTOS_PREC_INTERNETCONTROL;
 	bm->inq_limit = BM_DEFAULT_Q_LIMIT;
 	bm->outq_limit = BM_DEFAULT_Q_LIMIT;
+	bm->t_bgp_label_manager = NULL;
 
 	bgp_mac_init();
 	/* init the rd id space.
@@ -8308,6 +8309,7 @@ void bgp_terminate(void)
 		list_delete(&bm->listen_sockets);
 
 	EVENT_OFF(bm->t_rmap_update);
+	EVENT_OFF(bm->t_bgp_label_manager);
 
 	bgp_mac_finish();
 }

bgpd/bgpd.h

@@ -165,6 +165,8 @@ struct bgp_master {
 	uint32_t inq_limit;
 	uint32_t outq_limit;
 
+	struct event *t_bgp_label_manager;
+
 	QOBJ_FIELDS;
 };
 DECLARE_QOBJ_TYPE(bgp_master);