diff --git a/lib/libfrr.c b/lib/libfrr.c index 6ebe24eef7..9119b04992 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -653,7 +653,7 @@ struct thread_master *frr_init(void) lib_error_init(); yang_init(); - nb_init(di->yang_modules, di->n_yang_modules); + nb_init(master, di->yang_modules, di->n_yang_modules); return master; } diff --git a/lib/northbound.c b/lib/northbound.c index 490b3abe57..8503f87d60 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -1539,7 +1539,8 @@ static void nb_load_callbacks(const struct frr_yang_module_info *module) } } -void nb_init(const struct frr_yang_module_info *modules[], size_t nmodules) +void nb_init(struct thread_master *tm, + const struct frr_yang_module_info *modules[], size_t nmodules) { unsigned int errors = 0; @@ -1574,7 +1575,7 @@ void nb_init(const struct frr_yang_module_info *modules[], size_t nmodules) running_config = nb_config_new(NULL); /* Initialize the northbound CLI. */ - nb_cli_init(); + nb_cli_init(tm); } void nb_terminate(void) diff --git a/lib/northbound.h b/lib/northbound.h index e26a2f8617..c8e8d75701 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -20,6 +20,7 @@ #ifndef _FRR_NORTHBOUND_H_ #define _FRR_NORTHBOUND_H_ +#include "thread.h" #include "hook.h" #include "linklist.h" #include "openbsd-tree.h" @@ -825,7 +826,7 @@ extern const char *nb_client_name(enum nb_client client); * nmodules * Size of the modules array. */ -extern void nb_init(const struct frr_yang_module_info *modules[], +extern void nb_init(struct thread_master *tm, const struct frr_yang_module_info *modules[], size_t nmodules); /* diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index 0d8345d39f..d685a4e7c2 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -36,6 +36,7 @@ int debug_northbound; struct nb_config *vty_shared_candidate_config; +static struct thread_master *master; static void vty_show_libyang_errors(struct vty *vty, struct ly_ctx *ly_ctx) { @@ -213,16 +214,80 @@ int nb_cli_rpc(const char *xpath, struct list *input, struct list *output) } } -static int nb_cli_commit(struct vty *vty, bool force, char *comment) +void nb_cli_confirmed_commit_clean(struct vty *vty) +{ + THREAD_TIMER_OFF(vty->t_confirmed_commit_timeout); + nb_config_free(vty->confirmed_commit_rollback); + vty->confirmed_commit_rollback = NULL; +} + +int nb_cli_confirmed_commit_rollback(struct vty *vty) { uint32_t transaction_id; int ret; + /* Perform the rollback. */ + ret = nb_candidate_commit( + vty->confirmed_commit_rollback, NB_CLIENT_CLI, true, + "Rollback to previous configuration - confirmed commit has timed out", + &transaction_id); + if (ret == NB_OK) + vty_out(vty, + "Rollback performed successfully (Transaction ID #%u).\n", + transaction_id); + else + vty_out(vty, "Failed to rollback to previous configuration.\n"); + + return ret; +} + +static int nb_cli_confirmed_commit_timeout(struct thread *thread) +{ + struct vty *vty = THREAD_ARG(thread); + + /* XXX: broadcast this message to all logged-in users? */ + vty_out(vty, + "\nConfirmed commit has timed out, rolling back to previous configuration.\n\n"); + + nb_cli_confirmed_commit_rollback(vty); + nb_cli_confirmed_commit_clean(vty); + + return 0; +} + +static int nb_cli_commit(struct vty *vty, bool force, + unsigned int confirmed_timeout, char *comment) +{ + uint32_t transaction_id; + int ret; + + /* Check if there's a pending confirmed commit. */ + if (vty->t_confirmed_commit_timeout) { + if (confirmed_timeout) { + /* Reset timeout if "commit confirmed" is used again. */ + vty_out(vty, + "%% Resetting confirmed-commit timeout to %u minute(s)\n\n", + confirmed_timeout); + + THREAD_TIMER_OFF(vty->t_confirmed_commit_timeout); + thread_add_timer(master, + nb_cli_confirmed_commit_timeout, vty, + confirmed_timeout * 60, + &vty->t_confirmed_commit_timeout); + } else { + /* Accept commit confirmation. */ + vty_out(vty, "%% Commit complete.\n\n"); + nb_cli_confirmed_commit_clean(vty); + } + 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, "%% Candidate configuration needs to be updated before commit.\n\n"); @@ -231,6 +296,16 @@ static int nb_cli_commit(struct vty *vty, bool force, char *comment) return CMD_WARNING; } + /* "confirm" parameter. */ + if (confirmed_timeout) { + vty->confirmed_commit_rollback = nb_config_dup(running_config); + + vty->t_confirmed_commit_timeout = NULL; + thread_add_timer(master, nb_cli_confirmed_commit_timeout, vty, + confirmed_timeout * 60, + &vty->t_confirmed_commit_timeout); + } + ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, true, comment, &transaction_id); @@ -534,18 +609,22 @@ DEFUN (config_private, DEFPY (config_commit, config_commit_cmd, - "commit [force$force]", + "commit [{force$force|confirmed (1-60)}]", "Commit changes into the running configuration\n" - "Force commit even if the candidate is outdated\n") + "Force commit even if the candidate is outdated\n" + "Rollback this commit unless there is a confirming commit\n" + "Timeout in minutes for the commit to be confirmed\n") { - return nb_cli_commit(vty, !!force, NULL); + return nb_cli_commit(vty, !!force, confirmed, NULL); } DEFPY (config_commit_comment, config_commit_comment_cmd, - "commit [force$force] comment LINE...", + "commit [{force$force|confirmed (1-60)}] comment LINE...", "Commit changes into the running configuration\n" "Force commit even if the candidate is outdated\n" + "Rollback this commit unless there is a confirming commit\n" + "Timeout in minutes for the commit to be confirmed\n" "Assign a comment to this commit\n" "Comment for this commit (Max 80 characters)\n") { @@ -555,7 +634,7 @@ DEFPY (config_commit_comment, argv_find(argv, argc, "LINE", &idx); comment = argv_concat(argv, argc, idx); - ret = nb_cli_commit(vty, !!force, comment); + ret = nb_cli_commit(vty, !!force, confirmed, comment); XFREE(MTYPE_TMP, comment); return ret; @@ -1543,8 +1622,10 @@ static const struct cmd_variable_handler yang_var_handlers[] = { .completions = yang_translator_autocomplete}, {.completions = NULL}}; -void nb_cli_init(void) +void nb_cli_init(struct thread_master *tm) { + master = tm; + /* Initialize the shared candidate configuration. */ vty_shared_candidate_config = nb_config_new(NULL); diff --git a/lib/northbound_cli.h b/lib/northbound_cli.h index febcbd86f1..362a4bc325 100644 --- a/lib/northbound_cli.h +++ b/lib/northbound_cli.h @@ -105,8 +105,10 @@ extern void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *dnode, bool show_defaults); /* Prototypes of internal functions. */ +extern void nb_cli_confirmed_commit_clean(struct vty *vty); +extern int nb_cli_confirmed_commit_rollback(struct vty *vty); extern void nb_cli_install_default(int node); -extern void nb_cli_init(void); +extern void nb_cli_init(struct thread_master *tm); extern void nb_cli_terminate(void); #endif /* _FRR_NORTHBOUND_CLI_H_ */ diff --git a/lib/vty.c b/lib/vty.c index 9908ada7f0..085cbac742 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2714,6 +2714,14 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive) void vty_config_exit(struct vty *vty) { + /* Check if there's a pending confirmed commit. */ + if (vty->t_confirmed_commit_timeout) { + vty_out(vty, + "WARNING: exiting with a pending confirmed commit. Rolling back to previous configuration.\n\n"); + nb_cli_confirmed_commit_rollback(vty); + nb_cli_confirmed_commit_clean(vty); + } + vty_config_exclusive_unlock(vty); if (vty->candidate_config) { diff --git a/lib/vty.h b/lib/vty.h index ae6c4bae96..ad4dc273b9 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -126,6 +126,10 @@ struct vty { /* Base candidate configuration. */ struct nb_config *candidate_config_base; + /* Confirmed-commit timeout and rollback configuration. */ + struct thread *t_confirmed_commit_timeout; + struct nb_config *confirmed_commit_rollback; + /* qobj object ID (replacement for "index") */ uint64_t qobj_index; diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 2fbc686e1e..78016dc9ce 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -1384,10 +1384,10 @@ static void bgp_startup(void) LOG_DAEMON); zprivs_preinit(&bgpd_privs); zprivs_init(&bgpd_privs); - yang_init(); - nb_init(NULL, 0); master = thread_master_create(NULL); + yang_init(); + nb_init(master, NULL, 0); bgp_master_init(master); bgp_option_set(BGP_OPT_NO_LISTEN); vrf_init(NULL, NULL, NULL, NULL, NULL); diff --git a/tests/helpers/c/main.c b/tests/helpers/c/main.c index 9e34a7c255..768cf296ad 100644 --- a/tests/helpers/c/main.c +++ b/tests/helpers/c/main.c @@ -156,7 +156,7 @@ int main(int argc, char **argv) vty_init(master); memory_init(); yang_init(); - nb_init(NULL, 0); + nb_init(master, NULL, 0); /* OSPF vty inits. */ test_vty_init(); diff --git a/tests/lib/cli/common_cli.c b/tests/lib/cli/common_cli.c index 04f1e3253d..3cf30f00c3 100644 --- a/tests/lib/cli/common_cli.c +++ b/tests/lib/cli/common_cli.c @@ -84,7 +84,7 @@ int main(int argc, char **argv) vty_init(master); memory_init(); yang_init(); - nb_init(NULL, 0); + nb_init(master, NULL, 0); test_init(argc, argv); diff --git a/tests/lib/cli/test_commands.c b/tests/lib/cli/test_commands.c index 74816ece8c..ba46bdcea9 100644 --- a/tests/lib/cli/test_commands.c +++ b/tests/lib/cli/test_commands.c @@ -143,7 +143,7 @@ static void test_init(void) cmd_init(1); yang_init(); - nb_init(NULL, 0); + nb_init(master, NULL, 0); install_node(&bgp_node, NULL); install_node(&rip_node, NULL); diff --git a/tests/lib/northbound/test_oper_data.c b/tests/lib/northbound/test_oper_data.c index a9a89ee491..7c5713d8f9 100644 --- a/tests/lib/northbound/test_oper_data.c +++ b/tests/lib/northbound/test_oper_data.c @@ -449,7 +449,7 @@ int main(int argc, char **argv) vty_init(master); memory_init(); yang_init(); - nb_init(modules, array_size(modules)); + nb_init(master, modules, array_size(modules)); /* Create artificial data. */ create_data(num_vrfs, num_interfaces, num_routes);