lib: mgmtd: fixes for startup config file processing

Signed-off-by: Christian Hopps <chopps@labn.net>

diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c
index 56b0cb52e1eb518f9f3f6a0a88a381e6defdb597..83f60ea58b0bf83486824636ee6df423707113a5 100644 (file)
--- a/lib/mgmt_fe_client.c
+++ b/lib/mgmt_fe_client.c
@@ -361,6 +361,7 @@ static int mgmt_fe_client_handle_msg(struct mgmt_fe_client_ctx *client_ctx,
                                client_ctx, fe_msg->session_req->session_id);
                }
 
+               /* The session state may be deleted by the callback */
                if (session && session->client_ctx &&
                    session->client_ctx->client_params.client_session_notify)
                        (*session->client_ctx->client_params
@@ -549,6 +550,7 @@ static int _notify_connect_disconnect(struct msg_client *client, bool connected)
 {
        struct mgmt_fe_client_ctx *client_ctx =
                container_of(client, struct mgmt_fe_client_ctx, client);
+       struct mgmt_fe_client_session *session;
        int ret;
 
        /* Send REGISTER_REQ message */
@@ -557,6 +559,32 @@ static int _notify_connect_disconnect(struct msg_client *client, bool connected)
                        return ret;
        }
 
+       /* Walk list of sessions for this FE client deleting them */
+       if (!connected && mgmt_sessions_count(&client_ctx->client_sessions)) {
+               MGMTD_FE_CLIENT_DBG("Cleaning up existing sessions");
+
+               FOREACH_SESSION_IN_LIST (client_ctx, session) {
+                       assert(session->client_ctx);
+
+                       /* unlink from list first this avoids double free */
+                       mgmt_sessions_del(&client_ctx->client_sessions,
+                                         session);
+
+                       /* notify FE client the session is being deleted */
+                       if (session->client_ctx->client_params
+                                   .client_session_notify) {
+                               (*session->client_ctx->client_params
+                                         .client_session_notify)(
+                                       (uintptr_t)client_ctx,
+                                       client_ctx->client_params.user_data,
+                                       session->client_id, false, true,
+                                       session->session_id, session->user_ctx);
+                       }
+
+                       XFREE(MTYPE_MGMTD_FE_SESSION, session);
+               }
+       }
+
        /* Notify FE client through registered callback (if any). */
        if (client_ctx->client_params.client_connect_notify)
                (void)(*client_ctx->client_params.client_connect_notify)(
@@ -653,6 +681,14 @@ void mgmt_fe_client_lib_vty_init(void)
        install_element(CONFIG_NODE, &debug_mgmt_client_fe_cmd);
 }
 
+uint mgmt_fe_client_session_count(uintptr_t lib_hndl)
+{
+       struct mgmt_fe_client_ctx *client_ctx =
+               (struct mgmt_fe_client_ctx *)lib_hndl;
+
+       return mgmt_sessions_count(&client_ctx->client_sessions);
+}
+
 /*
  * Create a new Session for a Frontend Client connection.
  */
@@ -705,8 +741,8 @@ enum mgmt_result mgmt_fe_destroy_client_session(uintptr_t lib_hndl,
        if (session->session_id &&
            mgmt_fe_send_session_req(client_ctx, session, false) != 0)
                MGMTD_FE_CLIENT_ERR(
-                       "Failed to send session destroy request for the session-id %lu",
-                       (unsigned long)session->session_id);
+                       "Failed to send session destroy request for the session-id %" PRIu64,
+                       session->session_id);
 
        mgmt_sessions_del(&client_ctx->client_sessions, session);
        XFREE(MTYPE_MGMTD_FE_SESSION, session);

diff --git a/lib/mgmt_fe_client.h b/lib/mgmt_fe_client.h
index aa9a74e27f07ee7acdbefae26bdd87fd67cc9228..7ce6c5eef528f8e81af8e240a91818482f004a76 100644 (file)
--- a/lib/mgmt_fe_client.h
+++ b/lib/mgmt_fe_client.h
@@ -365,6 +365,11 @@ mgmt_fe_register_yang_notify(uintptr_t lib_hndl, uint64_t session_id,
  */
 extern void mgmt_fe_client_lib_destroy(void);
 
+/*
+ * Get count of open sessions.
+ */
+extern uint mgmt_fe_client_session_count(uintptr_t lib_hndl);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index c5582fc21c2338edb3036e28a00cf75ed32d14cf..e9c89d202906dfac18c38d70b086cb7c6adee212 100644 (file)
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -195,9 +195,12 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
                va_end(ap);
        }
 
-       if (vty_mgmt_fe_enabled()) {
+       if (vty_mgmt_should_process_cli_apply_changes(vty)) {
                VTY_CHECK_XPATH;
 
+               if (vty->type == VTY_FILE)
+                       return CMD_SUCCESS;
+
                implicit_commit = vty_needs_implicit_commit(vty);
                ret = vty_mgmt_send_config_data(vty);
                if (ret >= 0 && !implicit_commit)
@@ -224,7 +227,7 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty,
                va_end(ap);
        }
 
-       if (vty_mgmt_fe_enabled()) {
+       if (vty_mgmt_should_process_cli_apply_changes(vty)) {
                VTY_CHECK_XPATH;
 
                implicit_commit = vty_needs_implicit_commit(vty);

diff --git a/lib/vty.c b/lib/vty.c
index 0a0b04319564deb8c4f6c36142bc1a54508944e1..e763379efe08b1b93fbea50b4a3c85b7a16b0d7e 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -74,13 +74,6 @@ static bool mgmt_candidate_ds_wr_locked;
 static uint64_t mgmt_client_id_next;
 static uint64_t mgmt_last_req_id = UINT64_MAX;
 
-static bool vty_debug;
-#define VTY_DBG(fmt, ...)                                                      \
-       do {                                                                   \
-               if (vty_debug)                                                 \
-                       zlog_debug(fmt, ##__VA_ARGS__);                        \
-       } while (0)
-
 PREDECL_DLIST(vtyservs);
 
 struct vty_serv {
@@ -2203,6 +2196,7 @@ bool mgmt_vty_read_configs(void)
        vty->type = VTY_FILE;
        vty->node = CONFIG_NODE;
        vty->config = true;
+       vty->pending_allowed = true;
        vty->candidate_config = vty_shared_candidate_config;
        vty->mgmt_locked_candidate_ds = true;
        mgmt_candidate_ds_wr_locked = true;
@@ -2215,7 +2209,7 @@ bool mgmt_vty_read_configs(void)
                if (!confp)
                        continue;
 
-               MGMTD_FE_CLIENT_DBG("mgmtd: reading config file %s", path);
+               zlog_info("mgmtd: reading config file: %s", path);
 
                /* Execute configuration file */
                line_num = 0;
@@ -2223,16 +2217,11 @@ bool mgmt_vty_read_configs(void)
                count++;
        }
 
-       MGMTD_FE_CLIENT_DBG(
-               "mgmtd: done with daemon configs, checking mgmtd config");
-
        snprintf(path, sizeof(path), "%s/mgmtd.conf", frr_sysconfdir);
        confp = vty_open_config(path, config_default);
        if (!confp) {
                char *orig;
 
-               MGMTD_FE_CLIENT_DBG("mgmtd: no mgmtd config file %s", path);
-
                snprintf(path, sizeof(path), "%s/zebra.conf", frr_sysconfdir);
                orig = XSTRDUP(MTYPE_TMP, host_config_get());
 
@@ -2244,22 +2233,25 @@ bool mgmt_vty_read_configs(void)
        }
 
        if (confp) {
+               zlog_info("mgmtd: reading config file: %s", path);
+
                line_num = 0;
                (void)config_from_file(vty, confp, &line_num);
                count++;
        }
 
-       if (!count) {
-               MGMTD_FE_CLIENT_DBG("mgmtd: read no config files");
-               vty_close(vty);
-       } else {
-               MGMTD_FE_CLIENT_DBG("mgmtd: done reading all config files");
-               vty_read_file_finish(vty, vty->candidate_config);
-       }
+       vty->pending_allowed = false;
 
        vty->mgmt_locked_candidate_ds = false;
        mgmt_candidate_ds_wr_locked = false;
 
+       if (!count)
+               vty_close(vty);
+       else
+               vty_read_file_finish(vty, NULL);
+
+       zlog_info("mgmtd: finished reading config files");
+
        return true;
 }
 
@@ -2411,7 +2403,7 @@ void vty_close(struct vty *vty)
 
        vty->status = VTY_CLOSE;
 
-       if (mgmt_lib_hndl) {
+       if (mgmt_lib_hndl && vty->mgmt_session_id) {
                mgmt_fe_destroy_client_session(mgmt_lib_hndl,
                                               vty->mgmt_client_id);
                vty->mgmt_session_id = 0;
@@ -3377,25 +3369,46 @@ void vty_init_vtysh(void)
        /* currently nothing to do, but likely to have future use */
 }
 
+
+/*
+ * These functions allow for CLI handling to be placed inside daemons; however,
+ * currently they are only used by mgmtd, with mgmtd having each daemons CLI
+ * functionality linked into it. This design choice was taken for efficiency.
+ */
+
 static void vty_mgmt_server_connected(uintptr_t lib_hndl, uintptr_t usr_data,
                                      bool connected)
 {
-       VTY_DBG("%sGot %sconnected %s MGMTD Frontend Server",
-               !connected ? "ERROR: " : "", !connected ? "dis: " : "",
-               !connected ? "from" : "to");
-
-       mgmt_fe_connected = connected;
+       MGMTD_FE_CLIENT_DBG("Got %sconnected %s MGMTD Frontend Server",
+                           !connected ? "dis: " : "",
+                           !connected ? "from" : "to");
 
        /*
-        * TODO: Setup or teardown front-end sessions for existing
-        * VTY connections.
+        * We should not have any sessions for connecting or disconnecting case.
+        * The  fe client library will delete all session on disconnect before
+        * calling us.
         */
+       assert(mgmt_fe_client_session_count(lib_hndl) == 0);
+
+       mgmt_fe_connected = connected;
+
+       /* Start or stop listening for vty connections */
+       if (connected) {
+               zlog_info("mgmtd: starting vty servers");
+               frr_vty_serv_start();
+       } else {
+               zlog_info("mgmtd: stopping vty servers");
+               frr_vty_serv_stop();
+       }
 }
 
-static void vty_mgmt_session_created(uintptr_t lib_hndl, uintptr_t usr_data,
-                                    uint64_t client_id, bool create,
-                                    bool success, uintptr_t session_id,
-                                    uintptr_t session_ctx)
+/*
+ * A session has successfully been created for a vty.
+ */
+static void vty_mgmt_session_notify(uintptr_t lib_hndl, uintptr_t usr_data,
+                                   uint64_t client_id, bool create,
+                                   bool success, uintptr_t session_id,
+                                   uintptr_t session_ctx)
 {
        struct vty *vty;
 
@@ -3407,10 +3420,16 @@ static void vty_mgmt_session_created(uintptr_t lib_hndl, uintptr_t usr_data,
                return;
        }
 
-       VTY_DBG("%s session for client %" PRIu64 " successfully",
-               create ? "Created" : "Destroyed", client_id);
-       if (create)
+       MGMTD_FE_CLIENT_DBG("%s session for client %" PRIu64 " successfully",
+                           create ? "Created" : "Destroyed", client_id);
+
+       if (create) {
+               assert(session_id != 0);
                vty->mgmt_session_id = session_id;
+       } else {
+               vty->mgmt_session_id = 0;
+               vty_close(vty);
+       }
 }
 
 static void vty_mgmt_ds_lock_notified(uintptr_t lib_hndl, uintptr_t usr_data,
@@ -3430,8 +3449,8 @@ static void vty_mgmt_ds_lock_notified(uintptr_t lib_hndl, uintptr_t usr_data,
                vty_out(vty, "ERROR: %socking for DS %u failed, Err: '%s'\n",
                        lock_ds ? "L" : "Unl", ds_id, errmsg_if_any);
        } else {
-               VTY_DBG("%socked DS %u successfully", lock_ds ? "L" : "Unl",
-                       ds_id);
+               MGMTD_FE_CLIENT_DBG("%socked DS %u successfully",
+                                   lock_ds ? "L" : "Unl", ds_id);
        }
 
        vty_mgmt_resume_response(vty, success);
@@ -3453,9 +3472,9 @@ static void vty_mgmt_set_config_result_notified(
                vty_out(vty, "ERROR: SET_CONFIG request failed, Error: %s\n",
                        errmsg_if_any ? errmsg_if_any : "Unknown");
        } else {
-               VTY_DBG("SET_CONFIG request for client 0x%" PRIx64
-                       " req-id %" PRIu64 " was successfull",
-                       client_id, req_id);
+               MGMTD_FE_CLIENT_DBG("SET_CONFIG request for client 0x%" PRIx64
+                                   " req-id %" PRIu64 " was successfull",
+                                   client_id, req_id);
        }
 
        vty_mgmt_resume_response(vty, success);
@@ -3478,7 +3497,8 @@ static void vty_mgmt_commit_config_result_notified(
                vty_out(vty, "ERROR: COMMIT_CONFIG request failed, Error: %s\n",
                        errmsg_if_any ? errmsg_if_any : "Unknown");
        } else {
-               VTY_DBG("COMMIT_CONFIG request for client 0x%" PRIx64
+               MGMTD_FE_CLIENT_DBG(
+                       "COMMIT_CONFIG request for client 0x%" PRIx64
                        " req-id %" PRIu64 " was successfull",
                        client_id, req_id);
                if (errmsg_if_any)
@@ -3509,9 +3529,9 @@ static enum mgmt_result vty_mgmt_get_data_result_notified(
                return MGMTD_INTERNAL_ERROR;
        }
 
-       VTY_DBG("GET_DATA request succeeded, client 0x%" PRIx64
-               " req-id %" PRIu64,
-               client_id, req_id);
+       MGMTD_FE_CLIENT_DBG("GET_DATA request succeeded, client 0x%" PRIx64
+                           " req-id %" PRIu64,
+                           client_id, req_id);
 
        if (req_id != mgmt_last_req_id) {
                mgmt_last_req_id = req_id;
@@ -3532,7 +3552,7 @@ static enum mgmt_result vty_mgmt_get_data_result_notified(
 
 static struct mgmt_fe_client_params client_params = {
        .client_connect_notify = vty_mgmt_server_connected,
-       .client_session_notify = vty_mgmt_session_created,
+       .client_session_notify = vty_mgmt_session_notify,
        .lock_ds_notify = vty_mgmt_ds_lock_notified,
        .set_config_notify = vty_mgmt_set_config_result_notified,
        .commit_config_notify = vty_mgmt_commit_config_result_notified,
@@ -3555,7 +3575,12 @@ void vty_init_mgmt_fe(void)
 
 bool vty_mgmt_fe_enabled(void)
 {
-       return mgmt_lib_hndl && mgmt_fe_connected ? true : false;
+       return mgmt_lib_hndl && mgmt_fe_connected;
+}
+
+bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty)
+{
+       return vty->type != VTY_FILE && vty_mgmt_fe_enabled();
 }
 
 int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id,
@@ -3591,6 +3616,19 @@ int vty_mgmt_send_config_data(struct vty *vty)
        int cnt;
        bool implicit_commit = false;
 
+       if (vty->type == VTY_FILE) {
+               /*
+                * if this is a config file read we will not send any of the
+                * changes until we are done reading the file and have modified
+                * the local candidate DS.
+                */
+               assert(vty->mgmt_locked_candidate_ds);
+               /* no-one else should be sending data right now */
+               assert(!vty->mgmt_num_pending_setcfg);
+               return 0;
+       }
+
+
        if (mgmt_lib_hndl && vty->mgmt_client_id && !vty->mgmt_session_id) {
                /*
                 * We are connected to mgmtd but we do not yet have an

diff --git a/lib/vty.h b/lib/vty.h
index 7a591ad1724e373337ed3336fbabc5fe5b230177..d26531f781ca3f536a9b161dcdc468c03dca59ac 100644 (file)
--- a/lib/vty.h
+++ b/lib/vty.h
@@ -226,6 +226,9 @@ struct vty {
        uint64_t mgmt_session_id; /* FE adapter identifies session w/ this */
        uint64_t mgmt_client_id;  /* FE vty client identifies w/ this ID */
        uint64_t mgmt_req_id;
+       /* set when we have sent mgmtd a *REQ command in response to some vty
+        * CLI command and we are waiting on the reply so we can respond to the
+        * vty user. */
        bool mgmt_req_pending;
        bool mgmt_locked_candidate_ds;
 };
@@ -401,7 +404,7 @@ extern void vty_stdio_close(void);
 
 extern void vty_init_mgmt_fe(void);
 extern bool vty_mgmt_fe_enabled(void);
-extern bool vty_mgmt_should_process_cli_changes(struct vty *vty);
+extern bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty);
 
 extern bool mgmt_vty_read_configs(void);
 extern int vty_mgmt_send_config_data(struct vty *vty);