diff --git a/lib/northbound.c b/lib/northbound.c index 5e031ac2ce..58dcc02ac3 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -40,6 +40,21 @@ struct nb_config *running_config; /* Hash table of user pointers associated with configuration entries. */ static struct hash *running_config_entries; +/* Management lock for the running configuration. */ +static struct { + /* Mutex protecting this structure. */ + pthread_mutex_t mtx; + + /* Actual lock. */ + bool locked; + + /* Northbound client who owns this lock. */ + enum nb_client owner_client; + + /* Northbound user who owns this lock. */ + const void *owner_user; +} running_config_mgmt_lock; + /* * Global lock used to prevent multiple configuration transactions from * happening concurrently. @@ -51,6 +66,7 @@ static int nb_callback_configuration(const enum nb_event event, static struct nb_transaction *nb_transaction_new(struct nb_config *config, struct nb_config_cbs *changes, enum nb_client client, + const void *user, const char *comment); static void nb_transaction_free(struct nb_transaction *transaction); static int nb_transaction_process(enum nb_event event, @@ -583,7 +599,8 @@ int nb_candidate_validate(struct nb_config *candidate) } int nb_candidate_commit_prepare(struct nb_config *candidate, - enum nb_client client, const char *comment, + enum nb_client client, const void *user, + const char *comment, struct nb_transaction **transaction) { struct nb_config_cbs changes; @@ -608,7 +625,8 @@ int nb_candidate_commit_prepare(struct nb_config *candidate, return NB_ERR_VALIDATION; } - *transaction = nb_transaction_new(candidate, &changes, client, comment); + *transaction = + nb_transaction_new(candidate, &changes, client, user, comment); if (*transaction == NULL) { flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED, "%s: failed to create transaction", __func__); @@ -645,13 +663,13 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction, } int nb_candidate_commit(struct nb_config *candidate, enum nb_client client, - bool save_transaction, const char *comment, - uint32_t *transaction_id) + const void *user, bool save_transaction, + const char *comment, uint32_t *transaction_id) { struct nb_transaction *transaction = NULL; int ret; - ret = nb_candidate_commit_prepare(candidate, client, comment, + ret = nb_candidate_commit_prepare(candidate, client, user, comment, &transaction); /* * Apply the changes if the preparation phase succeeded. Otherwise abort @@ -666,6 +684,60 @@ int nb_candidate_commit(struct nb_config *candidate, enum nb_client client, return ret; } +int nb_running_lock(enum nb_client client, const void *user) +{ + int ret = -1; + + pthread_mutex_lock(&running_config_mgmt_lock.mtx); + { + if (!running_config_mgmt_lock.locked) { + running_config_mgmt_lock.locked = true; + running_config_mgmt_lock.owner_client = client; + running_config_mgmt_lock.owner_user = user; + ret = 0; + } + } + pthread_mutex_unlock(&running_config_mgmt_lock.mtx); + + return ret; +} + +int nb_running_unlock(enum nb_client client, const void *user) +{ + int ret = -1; + + pthread_mutex_lock(&running_config_mgmt_lock.mtx); + { + if (running_config_mgmt_lock.locked + && running_config_mgmt_lock.owner_client == client + && running_config_mgmt_lock.owner_user == user) { + running_config_mgmt_lock.locked = false; + running_config_mgmt_lock.owner_client = NB_CLIENT_NONE; + running_config_mgmt_lock.owner_user = NULL; + ret = 0; + } + } + pthread_mutex_unlock(&running_config_mgmt_lock.mtx); + + return ret; +} + +int nb_running_lock_check(enum nb_client client, const void *user) +{ + int ret = -1; + + pthread_mutex_lock(&running_config_mgmt_lock.mtx); + { + if (!running_config_mgmt_lock.locked + || (running_config_mgmt_lock.owner_client == client + && running_config_mgmt_lock.owner_user == user)) + ret = 0; + } + pthread_mutex_unlock(&running_config_mgmt_lock.mtx); + + return ret; +} + static void nb_log_callback(const enum nb_event event, enum nb_operation operation, const char *xpath, const char *value) @@ -812,13 +884,20 @@ int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath, return nb_node->cbs.rpc(xpath, input, output); } -static struct nb_transaction *nb_transaction_new(struct nb_config *config, - struct nb_config_cbs *changes, - enum nb_client client, - const char *comment) +static struct nb_transaction * +nb_transaction_new(struct nb_config *config, struct nb_config_cbs *changes, + enum nb_client client, const void *user, const char *comment) { struct nb_transaction *transaction; + if (nb_running_lock_check(client, user)) { + flog_warn( + EC_LIB_NB_TRANSACTION_CREATION_FAILED, + "%s: running configuration is locked by another client", + __func__); + return NULL; + } + if (transaction_in_progress) { flog_warn( EC_LIB_NB_TRANSACTION_CREATION_FAILED, @@ -1761,6 +1840,7 @@ void nb_init(struct thread_master *tm, running_config_entries = hash_create(running_config_entry_key_make, running_config_entry_cmp, "Running Configuration Entries"); + pthread_mutex_init(&running_config_mgmt_lock.mtx, NULL); /* Initialize the northbound CLI. */ nb_cli_init(tm); @@ -1778,4 +1858,5 @@ void nb_terminate(void) hash_clean(running_config_entries, running_config_entry_free); hash_free(running_config_entries); nb_config_free(running_config); + pthread_mutex_destroy(&running_config_mgmt_lock.mtx); } diff --git a/lib/northbound.h b/lib/northbound.h index 14f27c1d41..dca33b905f 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -414,7 +414,8 @@ enum nb_error { /* Northbound clients. */ enum nb_client { - NB_CLIENT_CLI = 0, + NB_CLIENT_NONE = 0, + NB_CLIENT_CLI, NB_CLIENT_CONFD, NB_CLIENT_SYSREPO, }; @@ -662,6 +663,9 @@ extern int nb_candidate_validate(struct nb_config *candidate); * client * Northbound client performing the commit. * + * user + * Northbound user performing the commit (can be NULL). + * * comment * Optional comment describing the commit. * @@ -682,7 +686,7 @@ extern int nb_candidate_validate(struct nb_config *candidate); * - NB_ERR for other errors. */ extern int nb_candidate_commit_prepare(struct nb_config *candidate, - enum nb_client client, + enum nb_client client, const void *user, const char *comment, struct nb_transaction **transaction); @@ -727,6 +731,9 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction, * client * Northbound client performing the commit. * + * user + * Northbound user performing the commit (can be NULL). + * * save_transaction * Specify whether the transaction should be recorded in the transactions log * or not. @@ -748,11 +755,57 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction, * - NB_ERR for other errors. */ extern int nb_candidate_commit(struct nb_config *candidate, - enum nb_client client, bool save_transaction, - const char *comment, uint32_t *transaction_id); + enum nb_client client, const void *user, + bool save_transaction, const char *comment, + uint32_t *transaction_id); /* - * Iterate over operetional data. + * Lock the running configuration. + * + * client + * Northbound client. + * + * user + * Northbound user (can be NULL). + * + * Returns: + * 0 on success, -1 when the running configuration is already locked. + */ +extern int nb_running_lock(enum nb_client client, const void *user); + +/* + * Unlock the running configuration. + * + * client + * Northbound client. + * + * user + * Northbound user (can be NULL). + * + * Returns: + * 0 on success, -1 when the running configuration is already unlocked or + * locked by another client/user. + */ +extern int nb_running_unlock(enum nb_client client, const void *user); + +/* + * Check if the running configuration is locked or not for the given + * client/user. + * + * client + * Northbound client. + * + * user + * Northbound user (can be NULL). + * + * Returns: + * 0 if the running configuration is unlocked or if the client/user owns the + * lock, -1 otherwise. + */ +extern int nb_running_lock_check(enum nb_client client, const void *user); + +/* + * Iterate over operational data. * * xpath * Data path of the YANG data we want to iterate over. diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index 48faa7595a..cd79c0608e 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -185,7 +185,7 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) /* Do an implicit "commit" when using the classic CLI mode. */ if (frr_get_cli_mode() == FRR_CLI_CLASSIC) { ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, - false, NULL, NULL); + vty, false, NULL, NULL); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) { vty_out(vty, "%% Configuration failed: %s.\n\n", nb_err_name(ret)); @@ -237,7 +237,7 @@ int nb_cli_confirmed_commit_rollback(struct vty *vty) /* Perform the rollback. */ ret = nb_candidate_commit( - vty->confirmed_commit_rollback, NB_CLIENT_CLI, true, + vty->confirmed_commit_rollback, NB_CLIENT_CLI, vty, true, "Rollback to previous configuration - confirmed commit has timed out", &transaction_id); if (ret == NB_OK) @@ -291,11 +291,6 @@ static int nb_cli_commit(struct vty *vty, bool force, return CMD_SUCCESS; } - if (vty_exclusive_lock != NULL && vty_exclusive_lock != vty) { - vty_out(vty, "%% Configuration is locked by another VTY.\n\n"); - return CMD_WARNING; - } - /* "force" parameter. */ if (!force && nb_candidate_needs_update(vty->candidate_config)) { vty_out(vty, @@ -315,8 +310,8 @@ static int nb_cli_commit(struct vty *vty, bool force, &vty->t_confirmed_commit_timeout); } - ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, true, - comment, &transaction_id); + ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, vty, + true, comment, &transaction_id); /* Map northbound return code to CLI return code. */ switch (ret) { @@ -1509,7 +1504,7 @@ static int nb_cli_rollback_configuration(struct vty *vty, snprintf(comment, sizeof(comment), "Rollback to transaction %u", transaction_id); - ret = nb_candidate_commit(candidate, NB_CLIENT_CLI, true, comment, + ret = nb_candidate_commit(candidate, NB_CLIENT_CLI, vty, true, comment, NULL); nb_config_free(candidate); switch (ret) { diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c index 0df286511e..2fc3c81cf2 100644 --- a/lib/northbound_confd.c +++ b/lib/northbound_confd.c @@ -322,7 +322,7 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen) */ transaction = NULL; ret = nb_candidate_commit_prepare(candidate, NB_CLIENT_CONFD, NULL, - &transaction); + NULL, &transaction); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) { enum confd_errcode errcode; const char *errmsg; diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index 33b6c24782..e21f57d37e 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -282,15 +282,15 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session, * single event (SR_EV_ENABLED). This means we need to perform * the full two-phase commit protocol in one go here. */ - ret = nb_candidate_commit(candidate, NB_CLIENT_SYSREPO, true, - NULL, NULL); + ret = nb_candidate_commit(candidate, NB_CLIENT_SYSREPO, NULL, + true, NULL, NULL); } else { /* * Validate the configuration changes and allocate all resources * required to apply them. */ ret = nb_candidate_commit_prepare(candidate, NB_CLIENT_SYSREPO, - NULL, &transaction); + NULL, NULL, &transaction); } /* Map northbound return code to sysrepo return code. */ diff --git a/lib/vty.c b/lib/vty.c index dae8e82599..da9bee6e10 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -86,9 +86,6 @@ static vector Vvty_serv_thread; /* Current directory. */ char *vty_cwd = NULL; -/* Exclusive configuration lock. */ -struct vty *vty_exclusive_lock; - /* Login password check. */ static int no_password_check = 0; @@ -2369,7 +2366,7 @@ static void vty_read_file(struct nb_config *config, FILE *confp) if (config == NULL && vty->candidate_config && frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) { ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, - true, "Read configuration file", + vty, true, "Read configuration file", NULL); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) zlog_err("%s: failed to read configuration file.", @@ -2601,8 +2598,8 @@ void vty_log_fixed(char *buf, size_t len) int vty_config_enter(struct vty *vty, bool private_config, bool exclusive) { - if (exclusive && !vty_config_exclusive_lock(vty)) { - vty_out(vty, "VTY configuration is locked by other VTY\n"); + if (exclusive && nb_running_lock(NB_CLIENT_CLI, vty)) { + vty_out(vty, "%% Configuration is locked by other client\n"); return CMD_WARNING; } @@ -2636,7 +2633,7 @@ void vty_config_exit(struct vty *vty) nb_cli_confirmed_commit_clean(vty); } - vty_config_exclusive_unlock(vty); + (void)nb_running_unlock(NB_CLIENT_CLI, vty); if (vty->candidate_config) { if (vty->private_config) @@ -2651,21 +2648,6 @@ void vty_config_exit(struct vty *vty) vty->config = false; } -int vty_config_exclusive_lock(struct vty *vty) -{ - if (vty_exclusive_lock == NULL) { - vty_exclusive_lock = vty; - return 1; - } - return 0; -} - -void vty_config_exclusive_unlock(struct vty *vty) -{ - if (vty_exclusive_lock == vty) - vty_exclusive_lock = NULL; -} - /* Master of the threads. */ static struct thread_master *vty_master; diff --git a/lib/vty.h b/lib/vty.h index 9c2653e1ac..98d75542fd 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -289,9 +289,6 @@ struct vty_arg { #define IS_DIRECTORY_SEP(c) ((c) == DIRECTORY_SEP) #endif -/* Exported variables */ -extern struct vty *vty_exclusive_lock; - /* Prototypes. */ extern void vty_init(struct thread_master *); extern void vty_init_vtysh(void); @@ -321,8 +318,6 @@ extern void vty_log(const char *level, const char *proto, const char *fmt, extern int vty_config_enter(struct vty *vty, bool private_config, bool exclusive); extern void vty_config_exit(struct vty *); -extern int vty_config_exclusive_lock(struct vty *vty); -extern void vty_config_exclusive_unlock(struct vty *vty); extern int vty_shell(struct vty *); extern int vty_shell_serv(struct vty *); extern void vty_hello(struct vty *);