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>

diff --git a/bgpd/bgp_labelpool.c b/bgpd/bgp_labelpool.c
index 7a7a400278ada435753ef474785558a62c28b0c6..73d3dc67e8aae9a99d3c5a705d1b0ae4f649e5ed 100644 (file)
--- 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 d888090e60f7b95012c16f898bb67a4771df1dca..6643795f5535b0b9416898f333d7228309e72379 100644 (file)
--- 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 : "<all>"), bm->port);

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index deed8788e6930808b38760e36e1cb00763b68e39..3b762a362b0a431d8735fd15d1289cc51c77a1bb 100644 (file)
--- 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 c00d9925eeff24c3db05bc490a731a9ca7d385f3..0223c423df770153a85ae31c33104230eac525b9 100644 (file)
--- 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 7df8de55f03e5471981ae7694e58abc41591d285..fd3a6c94438004f23f2f0dce10e88ad1ad46eb90 100644 (file)
--- 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 0a8962b4c703dde608356e45adf19d02f8bae2c3..ebcb9b5ea243a2a061afb80710dcdb661af3572a 100644 (file)
--- 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 94213db4d75379800f66b256db329593bfc90467..f1736ffd0bd200782d381ac87a83dcf8620d4759 100644 (file)
--- 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 81043988863b1c5ab9a6caad4be78b427ff5e8e5..4f74d93044376cc3983f64887bb04d2d5d56b0f8 100644 (file)
--- 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 52ea8d9003259b55e0cb8bab469bd471cb9475c8..2d800baae142fb2371f76d211ca61b22914a8a8e 100644 (file)
--- 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 1cdf4ff22e81a5bc69c109bc2cb60f98768d900a..e6626a178b66e46959091fe317c4595a8bf719e9 100644 (file)
--- 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 cab734ae5e0d73f96ffc232d0523895fba803015..54f3635901417d4b3842c582d161a1f95e5576e0 100644 (file)
--- 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 652d7618a8665809411983ad4bbe438200550833..9d1c05436c2a42b26ea03c58761cb0ea2bc5a1bd 100644 (file)
--- 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 378a8ade4ba426138fa0f934c886807efd7d0bb7..d2aeb2ed28c1b1603ee8abfa17cd8610ebe9a212 100644 (file)
--- 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 55c5b934fc2f773a28f5a084d2742950f20c4d4c..b9897bea038c6052cb08877fa49e805cec8be84d 100644 (file)
--- 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,