diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c index 56b0cb52e1..83f60ea58b 100644 --- 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 aa9a74e27f..7ce6c5eef5 100644 --- 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 c5582fc21c..e9c89d2029 100644 --- 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 0a0b043195..e763379efe 100644 --- 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"); + MGMTD_FE_CLIENT_DBG("Got %sconnected %s MGMTD Frontend Server", + !connected ? "dis: " : "", + !connected ? "from" : "to"); + + /* + * 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; - /* - * TODO: Setup or teardown front-end sessions for existing - * VTY connections. - */ + /* 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 7a591ad172..d26531f781 100644 --- 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);