diff options
-rw-r--r-- | bgpd/bgp_vty.c | 36 | ||||
-rw-r--r-- | eigrpd/eigrp_cli.c | 2 | ||||
-rw-r--r-- | isisd/isis_cli.c | 2 | ||||
-rw-r--r-- | lib/command.c | 2 | ||||
-rw-r--r-- | lib/if.c | 7 | ||||
-rw-r--r-- | lib/lib_vty.c | 9 | ||||
-rw-r--r-- | lib/northbound.c | 35 | ||||
-rw-r--r-- | lib/northbound.h | 1 | ||||
-rw-r--r-- | lib/northbound_cli.c | 106 | ||||
-rw-r--r-- | lib/northbound_cli.h | 27 | ||||
-rw-r--r-- | lib/vrf.c | 3 | ||||
-rw-r--r-- | lib/vty.c | 4 | ||||
-rw-r--r-- | lib/vty.h | 5 | ||||
-rw-r--r-- | ripd/rip_cli.c | 2 | ||||
-rw-r--r-- | ripngd/ripng_cli.c | 2 | ||||
-rw-r--r-- | tests/bgpd/test_peer_attr.c | 6 |
16 files changed, 160 insertions, 89 deletions
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index b2769e21d..9748df9f9 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1386,7 +1386,7 @@ DEFUN_YANG_NOSH(router_bgp, NB_OP_MODIFY, "false"); } - ret = nb_cli_apply_changes(vty, base_xpath); + ret = nb_cli_apply_changes_clear_pending(vty, base_xpath); if (ret == CMD_SUCCESS) { VTY_PUSH_XPATH(BGP_NODE, base_xpath); @@ -1394,7 +1394,6 @@ DEFUN_YANG_NOSH(router_bgp, * For backward compatibility with old commands we still * need to use the qobj infrastructure. */ - nb_cli_pending_commit_check(vty); bgp = bgp_lookup(as, name); if (bgp) VTY_PUSH_CONTEXT(BGP_NODE, bgp); @@ -1444,7 +1443,8 @@ DEFUN_YANG(no_router_bgp, nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, base_xpath); + /* We want to finish any classic config after a no router */ + return nb_cli_apply_changes_clear_pending(vty, base_xpath); } void cli_show_router_bgp(struct vty *vty, struct lyd_node *dnode, @@ -4675,7 +4675,11 @@ DEFUN_YANG(no_neighbor, nb_cli_enqueue_change(vty, base_xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); + /* + * Need to commit any pending so this command doesn't merge with a + * create into a modify, which BGP can't handle + */ + return nb_cli_apply_changes_clear_pending(vty, NULL); } DEFUN_YANG(no_neighbor_interface_config, @@ -4699,7 +4703,11 @@ DEFUN_YANG(no_neighbor_interface_config, nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, base_xpath); + /* + * Need to commit any pending so this command doesn't merge with a + * create into a modify, which BGP can't handle + */ + return nb_cli_apply_changes_clear_pending(vty, base_xpath); } DEFUN_YANG(no_neighbor_peer_group, @@ -4717,7 +4725,11 @@ DEFUN_YANG(no_neighbor_peer_group, nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, base_xpath); + /* + * Need to commit any pending so this command doesn't merge with a + * create into a modify, which BGP can't handle + */ + return nb_cli_apply_changes_clear_pending(vty, base_xpath); } DEFUN_YANG(no_neighbor_interface_peer_group_remote_as, @@ -4756,7 +4768,11 @@ DEFUN_YANG(no_neighbor_interface_peer_group_remote_as, nb_cli_enqueue_change(vty, base_xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); + /* + * Need to commit any pending so this command doesn't merge with a + * create into a modify, which BGP can't handle + */ + return nb_cli_apply_changes_clear_pending(vty, NULL); } DEFUN_YANG(neighbor_local_as, @@ -5087,7 +5103,11 @@ DEFUN_YANG (no_neighbor_set_peer_group, nb_cli_enqueue_change(vty, "./peer-group", NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, base_xpath); + /* + * Need to commit any pending so this command doesn't merge with a + * create into a modify, which BGP can't handle + */ + return nb_cli_apply_changes_clear_pending(vty, base_xpath); } ALIAS_HIDDEN(no_neighbor_set_peer_group, no_neighbor_set_peer_group_hidden_cmd, diff --git a/eigrpd/eigrp_cli.c b/eigrpd/eigrp_cli.c index cf3999b45..3a978cae3 100644 --- a/eigrpd/eigrp_cli.c +++ b/eigrpd/eigrp_cli.c @@ -79,7 +79,7 @@ DEFPY_YANG( as_str, vrf ? vrf : VRF_DEFAULT_NAME); nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); + return nb_cli_apply_changes_clear_pending(vty, NULL); } void eigrp_cli_show_header(struct vty *vty, struct lyd_node *dnode, diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index 5aea9f25d..c3b250e7c 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -126,7 +126,7 @@ DEFPY_YANG(no_router_isis, no_router_isis_cmd, nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL); - return nb_cli_apply_changes( + return nb_cli_apply_changes_clear_pending( vty, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); } diff --git a/lib/command.c b/lib/command.c index 7fb405bdf..5cf1a4f57 100644 --- a/lib/command.c +++ b/lib/command.c @@ -980,7 +980,7 @@ static int cmd_execute_command_real(vector vline, enum cmd_filter_type filter, * non-YANG command. */ if (matched_element->attr != CMD_ATTR_YANG) - nb_cli_pending_commit_check(vty); + (void)nb_cli_pending_commit_check(vty); } ret = matched_element->func(matched_element, vty, argc, argv); @@ -905,8 +905,8 @@ connected_log(struct connected *connected, char *str) p = connected->destination; if (p) { - strncat(logbuf, inet_ntop(p->family, &p->u.prefix, buf, BUFSIZ), - BUFSIZ - strlen(logbuf)); + strlcat(logbuf, inet_ntop(p->family, &p->u.prefix, buf, BUFSIZ), + BUFSIZ); } zlog_info("%s", logbuf); } @@ -1238,7 +1238,7 @@ DEFPY_YANG_NOSH (interface, vrf_name); nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL); - ret = nb_cli_apply_changes(vty, xpath_list); + ret = nb_cli_apply_changes_clear_pending(vty, xpath_list); if (ret == CMD_SUCCESS) { VTY_PUSH_XPATH(INTERFACE_NODE, xpath_list); @@ -1248,7 +1248,6 @@ DEFPY_YANG_NOSH (interface, * all interface-level commands are converted to the new * northbound model. */ - nb_cli_pending_commit_check(vty); ifp = if_lookup_by_name(ifname, vrf_id); if (ifp) VTY_PUSH_CONTEXT(INTERFACE_NODE, ifp); diff --git a/lib/lib_vty.c b/lib/lib_vty.c index 128261a39..85816c512 100644 --- a/lib/lib_vty.c +++ b/lib/lib_vty.c @@ -37,6 +37,7 @@ #include "module.h" #include "defaults.h" #include "lib_vty.h" +#include "northbound_cli.h" /* Looking up memory status from vty interface. */ #include "vector.h" @@ -231,6 +232,8 @@ DEFUN_HIDDEN (start_config, { callback.readin_time = monotime(NULL); + vty->pending_allowed = 1; + if (callback.start_config) (*callback.start_config)(); @@ -244,6 +247,7 @@ DEFUN_HIDDEN (end_config, { time_t readin_time; char readin_time_str[MONOTIME_STRLEN]; + int ret; readin_time = monotime(NULL); readin_time -= callback.readin_time; @@ -251,12 +255,15 @@ DEFUN_HIDDEN (end_config, frrtime_to_interval(readin_time, readin_time_str, sizeof(readin_time_str)); + vty->pending_allowed = 0; + ret = nb_cli_pending_commit_check(vty); + zlog_info("Configuration Read in Took: %s", readin_time_str); if (callback.end_config) (*callback.end_config)(); - return CMD_SUCCESS; + return ret; } void cmd_init_config_callbacks(void (*start_config_cb)(void), diff --git a/lib/northbound.c b/lib/northbound.c index 1ce815ea1..47af77018 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -512,6 +512,7 @@ static int nb_lyd_diff_get_op(const struct lyd_node *dnode) return 'n'; } +#if 0 /* Used below in nb_config_diff inside normally disabled code */ static inline void nb_config_diff_dnode_log_path(const char *context, const char *path, const struct lyd_node *dnode) @@ -535,6 +536,7 @@ static inline void nb_config_diff_dnode_log(const char *context, nb_config_diff_dnode_log_path(context, path, dnode); free(path); } +#endif /* Calculate the delta between two different configurations. */ static void nb_config_diff(const struct nb_config *config1, @@ -548,8 +550,7 @@ static void nb_config_diff(const struct nb_config *config1, LY_ERR err; char *path; -#if 0 /* Useful (but noisy) when debugging diff code, and for improving later \ - */ +#if 0 /* Useful (noisy) when debugging diff code, and for improving later */ if (DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) { LY_LIST_FOR(config1->dnode, root) { LYD_TREE_DFS_BEGIN(root, dnode) { @@ -570,18 +571,25 @@ static void nb_config_diff(const struct nb_config *config1, LYD_DIFF_DEFAULTS, &diff); assert(!err); - if (diff && DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) - nb_config_diff_dnode_log("iterating diff", diff); + if (diff && DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) { + char *s; + + if (!lyd_print_mem(&s, diff, LYD_JSON, + LYD_PRINT_WITHSIBLINGS | LYD_PRINT_WD_ALL)) { + zlog_debug("%s: %s", __func__, s); + free(s); + } + } uint32_t seq = 0; + LY_LIST_FOR (diff, root) { LYD_TREE_DFS_BEGIN (root, dnode) { op = nb_lyd_diff_get_op(dnode); path = lyd_path(dnode, LYD_PATH_STD, NULL, 0); -#if 0 /* Useful (but noisy) when debugging diff code, and for improving later \ - */ +#if 0 /* Useful (noisy) when debugging diff code, and for improving later */ if (DEBUG_MODE_CHECK(&nb_dbg_cbs_config, DEBUG_MODE_ALL)) { char context[80]; snprintf(context, sizeof(context), @@ -631,7 +639,7 @@ static void nb_config_diff(const struct nb_config *config1, } } - lyd_free_tree(diff); + lyd_free_all(diff); } int nb_candidate_edit(struct nb_config *candidate, @@ -664,6 +672,14 @@ int nb_candidate_edit(struct nb_config *candidate, xpath_edit, err); return NB_ERR; } else if (dnode) { + /* Create default nodes */ + LY_ERR err = lyd_new_implicit_tree( + dnode, LYD_IMPLICIT_NO_STATE, NULL); + if (err) { + flog_warn(EC_LIB_LIBYANG, + "%s: lyd_new_implicit_all failed: %d", + __func__, err); + } /* * create dependency * @@ -679,6 +695,11 @@ int nb_candidate_edit(struct nb_config *candidate, ly_native_ctx, dep_xpath, NULL, LYD_NEW_PATH_UPDATE, &dep_dnode); + /* Create default nodes */ + if (!err) + err = lyd_new_implicit_tree( + dep_dnode, + LYD_IMPLICIT_NO_STATE, NULL); if (err) { flog_warn( EC_LIB_LIBYANG, diff --git a/lib/northbound.h b/lib/northbound.h index 7ccab5cad..bf04dbda1 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -704,6 +704,7 @@ extern struct debug nb_dbg_cbs_state; extern struct debug nb_dbg_cbs_rpc; extern struct debug nb_dbg_notif; extern struct debug nb_dbg_events; +extern struct debug nb_dbg_libyang; /* Global running configuration. */ extern struct nb_config *running_config; diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index d291a1f24..b74a0e6c2 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -74,7 +74,7 @@ static int nb_cli_classic_commit(struct vty *vty) default: vty_out(vty, "%% Configuration failed.\n\n"); vty_show_nb_errors(vty, ret, errmsg); - if (vty->t_pending_commit) + if (vty->pending_commit) vty_out(vty, "The following commands were dynamically grouped into the same transaction and rejected:\n%s", vty->pending_cmds_buf); @@ -89,49 +89,22 @@ static int nb_cli_classic_commit(struct vty *vty) static void nb_cli_pending_commit_clear(struct vty *vty) { - THREAD_OFF(vty->t_pending_commit); - vty->backoff_cmd_count = 0; + vty->pending_commit = 0; XFREE(MTYPE_TMP, vty->pending_cmds_buf); vty->pending_cmds_buflen = 0; vty->pending_cmds_bufpos = 0; } -static int nb_cli_pending_commit_cb(struct thread *thread) +int nb_cli_pending_commit_check(struct vty *vty) { - struct vty *vty = THREAD_ARG(thread); - - (void)nb_cli_classic_commit(vty); - nb_cli_pending_commit_clear(vty); + int ret = CMD_SUCCESS; - return 0; -} - -void nb_cli_pending_commit_check(struct vty *vty) -{ - if (vty->t_pending_commit) { - (void)nb_cli_classic_commit(vty); + if (vty->pending_commit) { + ret = nb_cli_classic_commit(vty); nb_cli_pending_commit_clear(vty); } -} - -static bool nb_cli_backoff_start(struct vty *vty) -{ - struct timeval now, delta; - - /* - * Start the configuration backoff timer only if 100 YANG-modeled - * commands or more were entered within the last second. - */ - monotime(&now); - if (monotime_since(&vty->backoff_start, &delta) >= 1000000) { - vty->backoff_start = now; - vty->backoff_cmd_count = 1; - return false; - } - if (++vty->backoff_cmd_count < 100) - return false; - return true; + return ret; } static int nb_cli_schedule_command(struct vty *vty) @@ -154,9 +127,7 @@ static int nb_cli_schedule_command(struct vty *vty) vty->pending_cmds_buflen); /* Schedule the commit operation. */ - THREAD_OFF(vty->t_pending_commit); - thread_add_timer_msec(master, nb_cli_pending_commit_cb, vty, 100, - &vty->t_pending_commit); + vty->pending_commit = 1; return CMD_SUCCESS; } @@ -180,21 +151,16 @@ void nb_cli_enqueue_change(struct vty *vty, const char *xpath, change->value = value; } -int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) +static int nb_cli_apply_changes_internal(struct vty *vty, + const char *xpath_base, + bool clear_pending) { - char xpath_base[XPATH_MAXLEN] = {}; bool error = false; - VTY_CHECK_XPATH; - - /* Parse the base XPath format string. */ - if (xpath_base_fmt) { - va_list ap; + if (xpath_base == NULL) + xpath_base = ""; - va_start(ap, xpath_base_fmt); - vsnprintf(xpath_base, sizeof(xpath_base), xpath_base_fmt, ap); - va_end(ap); - } + VTY_CHECK_XPATH; /* Edit candidate configuration. */ for (size_t i = 0; i < vty->num_cfg_changes; i++) { @@ -207,10 +173,9 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) /* Handle relative XPaths. */ memset(xpath, 0, sizeof(xpath)); if (vty->xpath_index > 0 - && ((xpath_base_fmt && xpath_base[0] == '.') - || change->xpath[0] == '.')) + && (xpath_base[0] == '.' || change->xpath[0] == '.')) strlcpy(xpath, VTY_CURR_XPATH, sizeof(xpath)); - if (xpath_base_fmt) { + if (xpath_base[0]) { if (xpath_base[0] == '.') strlcat(xpath, xpath_base + 1, sizeof(xpath)); else @@ -267,7 +232,7 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) } /* - * Do an implicit commit when using the classic CLI mode. + * Maybe do an implicit commit when using the classic CLI mode. * * NOTE: the implicit commit might be scheduled to run later when * too many commands are being sent at the same time. This is a @@ -276,14 +241,49 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) * faster. */ if (frr_get_cli_mode() == FRR_CLI_CLASSIC) { - if (vty->t_pending_commit || nb_cli_backoff_start(vty)) + if (clear_pending) { + if (vty->pending_commit) + return nb_cli_pending_commit_check(vty); + } else if (vty->pending_allowed) return nb_cli_schedule_command(vty); + assert(!vty->pending_commit); return nb_cli_classic_commit(vty); } return CMD_SUCCESS; } +int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) +{ + char xpath_base[XPATH_MAXLEN] = {}; + + /* Parse the base XPath format string. */ + if (xpath_base_fmt) { + va_list ap; + + va_start(ap, xpath_base_fmt); + vsnprintf(xpath_base, sizeof(xpath_base), xpath_base_fmt, ap); + va_end(ap); + } + return nb_cli_apply_changes_internal(vty, xpath_base, false); +} + +int nb_cli_apply_changes_clear_pending(struct vty *vty, + const char *xpath_base_fmt, ...) +{ + char xpath_base[XPATH_MAXLEN] = {}; + + /* Parse the base XPath format string. */ + if (xpath_base_fmt) { + va_list ap; + + va_start(ap, xpath_base_fmt); + vsnprintf(xpath_base, sizeof(xpath_base), xpath_base_fmt, ap); + va_end(ap); + } + return nb_cli_apply_changes_internal(vty, xpath_base, true); +} + int nb_cli_rpc(struct vty *vty, const char *xpath, struct list *input, struct list *output) { diff --git a/lib/northbound_cli.h b/lib/northbound_cli.h index 2290a76b8..28f81f8b3 100644 --- a/lib/northbound_cli.h +++ b/lib/northbound_cli.h @@ -57,7 +57,26 @@ extern void nb_cli_enqueue_change(struct vty *vty, const char *xpath, const char *value); /* - * Apply enqueued changes to the candidate configuration. + * Apply enqueued changes to the candidate configuration, do not batch, + * and apply any pending commits along with the currently enqueued. + * + * vty + * The vty context. + * + * xpath_base_fmt + * Prepend the given XPath (absolute or relative) to all enqueued + * configuration changes. This is an optional parameter. + * + * Returns: + * CMD_SUCCESS on success, CMD_WARNING_CONFIG_FAILED otherwise. + */ +extern int nb_cli_apply_changes_clear_pending(struct vty *vty, + const char *xpath_base_fmt, ...); + +/* + * Apply enqueued changes to the candidate configuration, this function + * may not immediately apply the changes, instead adding them to a pending + * queue. * * vty * The vty context. @@ -116,8 +135,12 @@ extern void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *dnode, * * vty * The vty context. + * + * Returns + * CMD_SUCCESS on success (or no pending), CMD_WARNING_CONFIG_FAILED + * otherwise. */ -extern void nb_cli_pending_commit_check(struct vty *vty); +extern int nb_cli_pending_commit_check(struct vty *vty); /* Prototypes of internal functions. */ extern void nb_cli_show_config_prepare(struct nb_config *config, @@ -690,10 +690,9 @@ int vrf_handler_create(struct vty *vty, const char *vrfname, vrfname); nb_cli_enqueue_change(vty, xpath_list, NB_OP_CREATE, NULL); - ret = nb_cli_apply_changes(vty, xpath_list); + ret = nb_cli_apply_changes_clear_pending(vty, xpath_list); if (ret == CMD_SUCCESS) { VTY_PUSH_XPATH(VRF_NODE, xpath_list); - nb_cli_pending_commit_check(vty); vrfp = vrf_lookup_by_name(vrfname); if (vrfp) VTY_PUSH_CONTEXT(VRF_NODE, vrfp); @@ -2622,8 +2622,8 @@ int vty_config_node_exit(struct vty *vty) { vty->xpath_index = 0; - /* Perform pending commit if any. */ - nb_cli_pending_commit_check(vty); + /* Perform any pending commits. */ + (void)nb_cli_pending_commit_check(vty); /* Check if there's a pending confirmed commit. */ if (vty->t_confirmed_commit_timeout) { @@ -135,9 +135,8 @@ struct vty { struct nb_config *candidate_config_base; /* Dynamic transaction information. */ - struct timeval backoff_start; - size_t backoff_cmd_count; - struct thread *t_pending_commit; + bool pending_allowed; + bool pending_commit; char *pending_cmds_buf; size_t pending_cmds_buflen; size_t pending_cmds_bufpos; diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c index 0c5730b4d..8a3ce24f5 100644 --- a/ripd/rip_cli.c +++ b/ripd/rip_cli.c @@ -80,7 +80,7 @@ DEFPY_YANG (no_router_rip, nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); + return nb_cli_apply_changes_clear_pending(vty, NULL); } void cli_show_router_rip(struct vty *vty, struct lyd_node *dnode, diff --git a/ripngd/ripng_cli.c b/ripngd/ripng_cli.c index 7e0d01408..722c0c7dd 100644 --- a/ripngd/ripng_cli.c +++ b/ripngd/ripng_cli.c @@ -80,7 +80,7 @@ DEFPY_YANG (no_router_ripng, nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); + return nb_cli_apply_changes_clear_pending(vty, NULL); } void cli_show_router_ripng(struct vty *vty, struct lyd_node *dnode, diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index cd5186f2a..44b55a238 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -776,6 +776,10 @@ static void test_execute(struct test *test, const char *fmt, ...) /* Execute command (non-strict). */ ret = cmd_execute_command(vline, test->vty, NULL, 0); + if (ret == CMD_SUCCESS) { + /* Commit any pending changes, irnore error */ + ret = nb_cli_pending_commit_check(test->vty); + } if (ret != CMD_SUCCESS) { test->state = TEST_COMMAND_ERROR; test->error = str_printf( @@ -783,8 +787,6 @@ static void test_execute(struct test *test, const char *fmt, ...) cmd, ret); } - nb_cli_pending_commit_check(test->vty); - /* Free memory. */ cmd_free_strvec(vline); XFREE(MTYPE_TMP, cmd); |