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 2c98cd9ef9fb97d30a3d90405c351e921a8131c5..db9b1cdf1324ac92d63983cc6e734c2ef4484662 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 5158717b5d97ec2d6bec927b81175248fc25c7e3..b190d86787b3659cbc57743711b9850a3fb38531 100644 (file)
--- a/bgpd/bgp_main.c
+++ b/bgpd/bgp_main.c
@@ -72,6 +72,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 */
@@ -333,16 +334,18 @@ 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:rSne:", longopts,
+               "p:l:rSne:I:", longopts,
                "  -p, --bgp_port     Set BGP listen port number (0 means do not listen).\n"
                "  -l, --listenon     Listen on specified address (implies -n)\n"
                "  -r, --retain       When program terminates, retain added route by bgpd.\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) {
@@ -384,6 +387,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;
@@ -405,7 +414,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 375af350fb5d35d8a554a1b080ca4d01e96cbc9a..48aa24ddad15295ec1708dd65deb8102797b1696 100644 (file)
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -2474,7 +2474,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;
 
@@ -2511,6 +2511,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 e3c88b9db664712e4eab521a4faef8da42214cef..44921f18eab60f5d2678f20f2d2e6071247ecc79 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 41f85f4a6fb74fccab253384a76c8b81d825b69e..d5a4c7a69c2a6d1b4f9bdcdf4a453ce6adba18a3 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -7629,7 +7629,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
@@ -7639,7 +7639,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 b2602d9499233b42ad9de4d6f88a52f1b190def0..0a8da1b3df2cc9ef4d08d69427a8f9e937c8d871 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -1411,7 +1411,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 03b62b482b26076fd10c0162d217dec626c2a6f7..eb78f2bcc0b2e9fd17d0bc386ad2394ada572c72 100644 (file)
--- a/ldpd/lde.c
+++ b/ldpd/lde.c
@@ -1643,7 +1643,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 dbfc95da86251fd6cbf2925637d2cae4f9b93a5a..1b7d61f0c95f1e51a17e75818a4713d9b202b1bc 100644 (file)
--- a/lib/log.c
+++ b/lib/log.c
@@ -940,6 +940,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 1b46ed671ebb2a27cd3c3954e5c6ff989b69aeaa..4c29b9ec24b69a1ad2423841f6bfd257ea5c8b2c 100644 (file)
--- a/lib/zclient.c
+++ b/lib/zclient.c
@@ -2046,24 +2046,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);
@@ -2089,8 +2094,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 581acf4652f84aebbd5b98bf587532730b8e21bc..f8052035a3eb052299f07d6835e0eb018b2203e9 100644 (file)
--- a/lib/zclient.h
+++ b/lib/zclient.h
@@ -116,6 +116,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,
@@ -608,7 +609,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 c751c0b12d03bf04d4260c77352279dc084c0338..255e9a537bd404129f5043c2cc4177a34435cb61 100644 (file)
--- a/tests/test_lblmgr.c
+++ b/tests/test_lblmgr.c
@@ -55,7 +55,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 f3fa3ba94efd618e9730c46527f8c2f64f14390f..9591843bbe236df74153c52fe2314915b945f7ec 100644 (file)
--- a/zebra/label_manager.c
+++ b/zebra/label_manager.c
@@ -221,8 +221,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 ffc0a4f6f9dbbcb257a7f3a80e5342b3ec928304..4703f406f98ac55e5667364d72c36776eeff3831 100644 (file)
--- a/zebra/zapi_msg.c
+++ b/zebra/zapi_msg.c
@@ -2385,7 +2385,8 @@ static void zread_label_manager_connect(struct zserv *client,
        if ((proto >= ZEBRA_ROUTE_MAX) || (proto <= ZEBRA_ROUTE_STATIC)) {
                zlog_err("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",
@@ -2403,31 +2404,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) {
-               zlog_err("%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) {
-               zlog_err(
-                       "%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)
@@ -2448,20 +2430,15 @@ 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)
                zlog_err(
                        "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);
 
@@ -2485,12 +2462,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;
@@ -2498,8 +2470,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)
@@ -2507,15 +2479,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) {
-                               zlog_err(
-                                       "Got label request from an unidentified client");
-                               return;
-                       }
                        if (hdr->command == ZEBRA_GET_LABEL_CHUNK)
                                zread_get_label_chunk(client, msg,
                                                      zvrf_id(zvrf));
@@ -2989,6 +2956,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,