From f533be73f6e73e7c6c2f2093f4bec59862c08dd6 Mon Sep 17 00:00:00 2001 From: paco Date: Thu, 7 Jun 2018 15:28:12 +0200 Subject: [PATCH] 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 --- bgpd/bgp_labelpool.c | 6 +++++ bgpd/bgp_main.c | 15 ++++++++--- bgpd/bgp_zebra.c | 3 ++- bgpd/bgp_zebra.h | 3 ++- bgpd/bgpd.c | 4 +-- bgpd/bgpd.h | 2 +- doc/manpages/bgpd.rst | 7 +++++ ldpd/lde.c | 2 +- lib/log.c | 1 + lib/zclient.c | 16 +++++++++--- lib/zclient.h | 3 ++- tests/test_lblmgr.c | 2 +- zebra/label_manager.c | 5 ++-- zebra/zapi_msg.c | 61 +++++++++---------------------------------- 14 files changed, 65 insertions(+), 65 deletions(-) diff --git a/bgpd/bgp_labelpool.c b/bgpd/bgp_labelpool.c index 7a7a40027..73d3dc67e 100644 --- a/bgpd/bgp_labelpool.c +++ b/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; diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index d888090e6..6643795f5 100644 --- a/bgpd/bgp_main.c +++ b/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 : ""), bm->port); diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index deed8788e..3b762a362 100644 --- a/bgpd/bgp_zebra.c +++ b/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) diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index c00d9925e..0223c423d 100644 --- a/bgpd/bgp_zebra.h +++ b/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); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7df8de55f..fd3a6c944 100644 --- a/bgpd/bgpd.c +++ b/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); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 0a8962b4c..ebcb9b5ea 100644 --- a/bgpd/bgpd.h +++ b/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); diff --git a/doc/manpages/bgpd.rst b/doc/manpages/bgpd.rst index 94213db4d..f1736ffd0 100644 --- a/doc/manpages/bgpd.rst +++ b/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 ===== diff --git a/ldpd/lde.c b/ldpd/lde.c index 810439888..4f74d9304 100644 --- a/ldpd/lde.c +++ b/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); } diff --git a/lib/log.c b/lib/log.c index 52ea8d900..2d800baae 100644 --- a/lib/log.c +++ b/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), diff --git a/lib/zclient.c b/lib/zclient.c index 1cdf4ff22..e6626a178 100644 --- a/lib/zclient.c +++ b/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; diff --git a/lib/zclient.h b/lib/zclient.h index cab734ae5..54f363590 100644 --- a/lib/zclient.h +++ b/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); diff --git a/tests/test_lblmgr.c b/tests/test_lblmgr.c index 652d7618a..9d1c05436 100644 --- a/tests/test_lblmgr.c +++ b/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", diff --git a/zebra/label_manager.c b/zebra/label_manager.c index 378a8ade4..d2aeb2ed2 100644 --- a/zebra/label_manager.c +++ b/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; diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 55c5b934f..b9897bea0 100644 --- a/zebra/zapi_msg.c +++ b/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, -- 2.39.5