bgpd, doc, ldpd, lib, tests, zebra: LM fixes

Corrections so that the BGP daemon can work with the label manager properly
through a label-manager proxy. Details:

- Correction so the BGP daemon behind a proxy label manager gets the range
  correctly (-I added to the BGP daemon, to set the daemon instance id)
- For the BGP case, added an asynchronous label manager connect command so
  the labels get recycled in case of a BGP daemon reconnection. With this,
  BGPd and LDPd would behave similarly.

Signed-off-by: F. Aragon <paco@voltanet.io>

bgpd/bgp_labelpool.c

@@ -530,6 +530,7 @@ void bgp_lp_event_zebra_up(void)
 	int chunks_needed;
 	void *labelid;
 	struct lp_lcb *lcb;
+	int lm_init_ok;
 
 	/*
 	 * Get label chunk allocation request dispatched to zebra
@@ -541,6 +542,11 @@ void bgp_lp_event_zebra_up(void)
 	chunks_needed = (labels_needed / LP_CHUNK_SIZE) + 1;
 	labels_needed = chunks_needed * LP_CHUNK_SIZE;
 
+	lm_init_ok = lm_label_manager_connect(zclient, 1) == 0;
+
+	if (!lm_init_ok)
+		zlog_err("%s: label manager connection error", __func__);
+
 	zclient_send_get_label_chunk(zclient, 0, labels_needed);
 	lp->pending_count = labels_needed;
 

bgpd/bgp_main.c

@@ -77,6 +77,7 @@ static const struct option longopts[] = {
 	{"no_kernel", no_argument, NULL, 'n'},
 	{"skip_runas", no_argument, NULL, 'S'},
 	{"ecmp", required_argument, NULL, 'e'},
+	{"int_num", required_argument, NULL, 'I'},
 	{0}};
 
 /* signal definitions */
@@ -371,15 +372,17 @@ int main(int argc, char **argv)
 	char *bgp_address = NULL;
 	int no_fib_flag = 0;
 	int skip_runas = 0;
+	int instance = 0;
 
 	frr_preinit(&bgpd_di, argc, argv);
 	frr_opt_add(
-		"p:l:Sne:" DEPRECATED_OPTIONS, longopts,
+		"p:l:Sne:I:" DEPRECATED_OPTIONS, longopts,
 		"  -p, --bgp_port     Set BGP listen port number (0 means do not listen).\n"
 		"  -l, --listenon     Listen on specified address (implies -n)\n"
 		"  -n, --no_kernel    Do not install route to kernel.\n"
 		"  -S, --skip_runas   Skip capabilities checks, and changing user and group IDs.\n"
-		"  -e, --ecmp         Specify ECMP to use.\n");
+		"  -e, --ecmp         Specify ECMP to use.\n"
+		"  -I, --int_num      Set instance number (label-manager)\n");
 
 	/* Command line argument treatment. */
 	while (1) {
@@ -426,6 +429,12 @@ int main(int argc, char **argv)
 		case 'S':
 			skip_runas = 1;
 			break;
+		case 'I':
+			instance = atoi(optarg);
+			if (instance > (unsigned short)-1)
+				zlog_err("Instance %i out of range (0..%u)",
+					 instance, (unsigned short)-1);
+			break;
 		default:
 			frr_help_exit(1);
 			break;
@@ -448,7 +457,7 @@ int main(int argc, char **argv)
 	bgp_vrf_init();
 
 	/* BGP related initialization.  */
-	bgp_init();
+	bgp_init((unsigned short)instance);
 
 	snprintf(bgpd_di.startinfo, sizeof(bgpd_di.startinfo), ", bgp@%s:%d",
 		 (bm->address ? bm->address : "<all>"), bm->port);

bgpd/bgp_zebra.c

@@ -2528,7 +2528,7 @@ stream_failure:		/* for STREAM_GETX */
 
 extern struct zebra_privs_t bgpd_privs;
 
-void bgp_zebra_init(struct thread_master *master)
+void bgp_zebra_init(struct thread_master *master, unsigned short instance)
 {
 	zclient_num_connects = 0;
 
@@ -2567,6 +2567,7 @@ void bgp_zebra_init(struct thread_master *master)
 	zclient->ipset_notify_owner = ipset_notify_owner;
 	zclient->ipset_entry_notify_owner = ipset_entry_notify_owner;
 	zclient->iptable_notify_owner = iptable_notify_owner;
+	zclient->instance = instance;
 }
 
 void bgp_zebra_destroy(void)

bgpd/bgp_zebra.h

@@ -23,7 +23,8 @@
 
 #include "vxlan.h"
 
-extern void bgp_zebra_init(struct thread_master *master);
+extern void bgp_zebra_init(struct thread_master *master,
+			   unsigned short instance);
 extern void bgp_zebra_init_tm_connect(struct bgp *bgp);
 extern uint32_t bgp_zebra_tm_get_id(void);
 extern bool bgp_zebra_tm_chunk_obtained(void);

bgpd/bgpd.c

@@ -7787,7 +7787,7 @@ void bgp_pthreads_finish()
 	frr_pthread_finish();
 }
 
-void bgp_init(void)
+void bgp_init(unsigned short instance)
 {
 
 	/* allocates some vital data structures used by peer commands in
@@ -7797,7 +7797,7 @@ void bgp_init(void)
 	bgp_pthreads_init();
 
 	/* Init zebra. */
-	bgp_zebra_init(bm->master);
+	bgp_zebra_init(bm->master, instance);
 
 #if ENABLE_BGP_VNC
 	vnc_zebra_init(bm->master);

bgpd/bgpd.h

@@ -1518,7 +1518,7 @@ extern int bgp_config_write(struct vty *);
 
 extern void bgp_master_init(struct thread_master *master);
 
-extern void bgp_init(void);
+extern void bgp_init(unsigned short instance);
 extern void bgp_pthreads_run(void);
 extern void bgp_pthreads_finish(void);
 extern void bgp_route_map_init(void);

doc/manpages/bgpd.rst

@@ -21,6 +21,13 @@ OPTIONS available for the |DAEMON| command:
 
 .. include:: common-options.rst
 
+LABEL MANAGER
+-------------
+
+.. option:: -I, --int_num
+
+   Set zclient id. This is required when using Zebra label manager in proxy mode.
+
 FILES
 =====
 

ldpd/lde.c

@@ -1641,7 +1641,7 @@ static void zclient_sync_init(unsigned short instance)
 	sock_set_nonblock(zclient_sync->sock);
 
 	/* Connect to label manager */
-	while (lm_label_manager_connect(zclient_sync) != 0) {
+	while (lm_label_manager_connect(zclient_sync, 0) != 0) {
 		log_warnx("Error connecting to label manager!");
 		sleep(1);
 	}

lib/log.c

@@ -945,6 +945,7 @@ static const struct zebra_desc_table command_types[] = {
 	DESC_ENTRY(ZEBRA_MPLS_LABELS_DELETE),
 	DESC_ENTRY(ZEBRA_IPMR_ROUTE_STATS),
 	DESC_ENTRY(ZEBRA_LABEL_MANAGER_CONNECT),
+	DESC_ENTRY(ZEBRA_LABEL_MANAGER_CONNECT_ASYNC),
 	DESC_ENTRY(ZEBRA_GET_LABEL_CHUNK),
 	DESC_ENTRY(ZEBRA_RELEASE_LABEL_CHUNK),
 	DESC_ENTRY(ZEBRA_ADVERTISE_ALL_VNI),

lib/zclient.c

@@ -1839,24 +1839,29 @@ static int zclient_read_sync_response(struct zclient *zclient,
  * immediately reads the answer from the input buffer.
  *
  * @param zclient Zclient used to connect to label manager (zebra)
+ * @param async Synchronous (0) or asynchronous (1) operation
  * @result Result of response
  */
-int lm_label_manager_connect(struct zclient *zclient)
+int lm_label_manager_connect(struct zclient *zclient, int async)
 {
 	int ret;
 	struct stream *s;
 	uint8_t result;
+	uint16_t cmd = async ? ZEBRA_LABEL_MANAGER_CONNECT_ASYNC :
+			       ZEBRA_LABEL_MANAGER_CONNECT;
 
 	if (zclient_debug)
 		zlog_debug("Connecting to Label Manager (LM)");
 
-	if (zclient->sock < 0)
+	if (zclient->sock < 0) {
+		zlog_debug("%s: invalid zclient socket", __func__);
 		return -1;
+	}
 
 	/* send request */
 	s = zclient->obuf;
 	stream_reset(s);
-	zclient_create_header(s, ZEBRA_LABEL_MANAGER_CONNECT, VRF_DEFAULT);
+	zclient_create_header(s, cmd, VRF_DEFAULT);
 
 	/* proto */
 	stream_putc(s, zclient->redist_default);
@@ -1882,8 +1887,11 @@ int lm_label_manager_connect(struct zclient *zclient)
 	if (zclient_debug)
 		zlog_debug("LM connect request sent (%d bytes)", ret);
 
+	if (async)
+		return 0;
+
 	/* read response */
-	if (zclient_read_sync_response(zclient, ZEBRA_LABEL_MANAGER_CONNECT)
+	if (zclient_read_sync_response(zclient, cmd)
 	    != 0)
 		return -1;
 

lib/zclient.h

@@ -112,6 +112,7 @@ typedef enum {
 	ZEBRA_MPLS_LABELS_DELETE,
 	ZEBRA_IPMR_ROUTE_STATS,
 	ZEBRA_LABEL_MANAGER_CONNECT,
+	ZEBRA_LABEL_MANAGER_CONNECT_ASYNC,
 	ZEBRA_GET_LABEL_CHUNK,
 	ZEBRA_RELEASE_LABEL_CHUNK,
 	ZEBRA_FEC_REGISTER,
@@ -572,7 +573,7 @@ extern int zclient_send_get_label_chunk(
 	uint8_t		keep,
 	uint32_t	chunk_size);
 
-extern int lm_label_manager_connect(struct zclient *zclient);
+extern int lm_label_manager_connect(struct zclient *zclient, int async);
 extern int lm_get_label_chunk(struct zclient *zclient, uint8_t keep,
 			      uint32_t chunk_size, uint32_t *start,
 			      uint32_t *end);

tests/test_lblmgr.c

@@ -59,7 +59,7 @@ static int zebra_send_label_manager_connect()
 
 	printf("Connect to Label Manager\n");
 
-	ret = lm_label_manager_connect(zclient);
+	ret = lm_label_manager_connect(zclient, 0);
 	printf("Label Manager connection result: %u \n", ret);
 	if (ret != 0) {
 		fprintf(stderr, "Error %d connecting to Label Manager %s\n",

zebra/label_manager.c

@@ -228,8 +228,9 @@ int zread_relay_label_manager_request(int cmd, struct zserv *zserv,
 	zserv->proto = proto;
 
 	/* in case there's any incoming message enqueued, read and forward it */
-	while (ret == 0)
-		ret = relay_response_back();
+	if (zserv->is_synchronous)
+		while (ret == 0)
+			ret = relay_response_back();
 
 	/* get the msg buffer used toward the 'master' Label Manager */
 	dst = zclient->obuf;

zebra/zapi_msg.c

@@ -1829,7 +1829,8 @@ static void zread_label_manager_connect(struct zserv *client,
 		flog_err(EC_ZEBRA_TM_WRONG_PROTO,
 			 "client %d has wrong protocol %s", client->sock,
 			 zebra_route_string(proto));
-		zsend_label_manager_connect_response(client, vrf_id, 1);
+		if (client->is_synchronous)
+			zsend_label_manager_connect_response(client, vrf_id, 1);
 		return;
 	}
 	zlog_notice("client %d with vrf %u instance %u connected as %s",
@@ -1847,33 +1848,12 @@ static void zread_label_manager_connect(struct zserv *client,
 		" Label Manager client connected: sock %d, proto %s, vrf %u instance %u",
 		client->sock, zebra_route_string(proto), vrf_id, instance);
 	/* send response back */
-	zsend_label_manager_connect_response(client, vrf_id, 0);
+	if (client->is_synchronous)
+		zsend_label_manager_connect_response(client, vrf_id, 0);
 
 stream_failure:
 	return;
 }
-static int msg_client_id_mismatch(const char *op, struct zserv *client,
-				  uint8_t proto, unsigned int instance)
-{
-	if (proto != client->proto) {
-		flog_err(EC_ZEBRA_PROTO_OR_INSTANCE_MISMATCH,
-			 "%s: msg vs client proto mismatch, client=%u msg=%u",
-			 op, client->proto, proto);
-		/* TODO: fail when BGP sets proto and instance */
-		/* return 1; */
-	}
-
-	if (instance != client->instance) {
-		flog_err(
-			EC_ZEBRA_PROTO_OR_INSTANCE_MISMATCH,
-			"%s: msg vs client instance mismatch, client=%u msg=%u",
-			op, client->instance, instance);
-		/* TODO: fail when BGP sets proto and instance */
-		/* return 1; */
-	}
-
-	return 0;
-}
 
 static void zread_get_label_chunk(struct zserv *client, struct stream *msg,
 				  vrf_id_t vrf_id)
@@ -1894,21 +1874,16 @@ static void zread_get_label_chunk(struct zserv *client, struct stream *msg,
 	STREAM_GETC(s, keep);
 	STREAM_GETL(s, size);
 
-	/* detect client vs message (proto,instance) mismatch */
-	if (msg_client_id_mismatch("Get-label-chunk", client, proto, instance))
-		return;
-
-	lmc = assign_label_chunk(client->proto, client->instance, keep, size);
+	lmc = assign_label_chunk(proto, instance, keep, size);
 	if (!lmc)
 		flog_err(
 			EC_ZEBRA_LM_CANNOT_ASSIGN_CHUNK,
 			"Unable to assign Label Chunk of size %u to %s instance %u",
-			size, zebra_route_string(client->proto),
-			client->instance);
+			size, zebra_route_string(proto), instance);
 	else
 		zlog_debug("Assigned Label Chunk %u - %u to %s instance %u",
 			   lmc->start, lmc->end,
-			   zebra_route_string(client->proto), client->instance);
+			   zebra_route_string(proto), instance);
 	/* send response back */
 	zsend_assign_label_chunk_response(client, vrf_id, lmc);
 
@@ -1932,12 +1907,7 @@ static void zread_release_label_chunk(struct zserv *client, struct stream *msg)
 	STREAM_GETL(s, start);
 	STREAM_GETL(s, end);
 
-	/* detect client vs message (proto,instance) mismatch */
-	if (msg_client_id_mismatch("Release-label-chunk", client, proto,
-				   instance))
-		return;
-
-	release_label_chunk(client->proto, client->instance, start, end);
+	release_label_chunk(proto, instance, start, end);
 
 stream_failure:
 	return;
@@ -1945,8 +1915,8 @@ stream_failure:
 static void zread_label_manager_request(ZAPI_HANDLER_ARGS)
 {
 	/* to avoid sending other messages like ZERBA_INTERFACE_UP */
-	if (hdr->command == ZEBRA_LABEL_MANAGER_CONNECT)
-		client->is_synchronous = 1;
+	client->is_synchronous = hdr->command ==
+				 ZEBRA_LABEL_MANAGER_CONNECT;
 
 	/* external label manager */
 	if (lm_is_external)
@@ -1954,16 +1924,10 @@ static void zread_label_manager_request(ZAPI_HANDLER_ARGS)
 						  zvrf_id(zvrf));
 	/* this is a label manager */
 	else {
-		if (hdr->command == ZEBRA_LABEL_MANAGER_CONNECT)
+		if (hdr->command == ZEBRA_LABEL_MANAGER_CONNECT ||
+		    hdr->command == ZEBRA_LABEL_MANAGER_CONNECT_ASYNC)
 			zread_label_manager_connect(client, msg, zvrf_id(zvrf));
 		else {
-			/* Sanity: don't allow 'unidentified' requests */
-			if (!client->proto) {
-				flog_err(
-					EC_ZEBRA_LM_ALIENS,
-					"Got label request from an unidentified client");
-				return;
-			}
 			if (hdr->command == ZEBRA_GET_LABEL_CHUNK)
 				zread_get_label_chunk(client, msg,
 						      zvrf_id(zvrf));
@@ -2448,6 +2412,7 @@ void (*zserv_handlers[])(ZAPI_HANDLER_ARGS) = {
 	[ZEBRA_MPLS_LABELS_DELETE] = zread_mpls_labels,
 	[ZEBRA_IPMR_ROUTE_STATS] = zebra_ipmr_route_stats,
 	[ZEBRA_LABEL_MANAGER_CONNECT] = zread_label_manager_request,
+	[ZEBRA_LABEL_MANAGER_CONNECT_ASYNC] = zread_label_manager_request,
 	[ZEBRA_GET_LABEL_CHUNK] = zread_label_manager_request,
 	[ZEBRA_RELEASE_LABEL_CHUNK] = zread_label_manager_request,
 	[ZEBRA_FEC_REGISTER] = zread_fec_register,