diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/command.c | 16 | ||||
| -rw-r--r-- | lib/frr_pthread.c | 26 | ||||
| -rw-r--r-- | lib/if.c | 76 | ||||
| -rw-r--r-- | lib/libfrr.c | 7 | ||||
| -rw-r--r-- | lib/mpls.h | 1 | ||||
| -rw-r--r-- | lib/northbound.c | 265 | ||||
| -rw-r--r-- | lib/northbound.h | 26 | ||||
| -rw-r--r-- | lib/northbound_cli.c | 139 | ||||
| -rw-r--r-- | lib/northbound_confd.c | 6 | ||||
| -rw-r--r-- | lib/northbound_grpc.cpp | 19 | ||||
| -rw-r--r-- | lib/northbound_sysrepo.c | 6 | ||||
| -rw-r--r-- | lib/vty.c | 21 | ||||
| -rw-r--r-- | lib/yang_wrappers.c | 54 | ||||
| -rw-r--r-- | lib/yang_wrappers.h | 10 |
14 files changed, 387 insertions, 285 deletions
diff --git a/lib/command.c b/lib/command.c index eecca2a5f5..04f2bd95a0 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1718,16 +1718,12 @@ static int vty_write_config(struct vty *vty) vty_out(vty, "frr defaults %s\n", DFLT_NAME); vty_out(vty, "!\n"); - pthread_rwlock_rdlock(&running_config->lock); - { - for (i = 0; i < vector_active(cmdvec); i++) - if ((node = vector_slot(cmdvec, i)) && node->func - && (node->vtysh || vty->type != VTY_SHELL)) { - if ((*node->func)(vty)) - vty_out(vty, "!\n"); - } - } - pthread_rwlock_unlock(&running_config->lock); + for (i = 0; i < vector_active(cmdvec); i++) + if ((node = vector_slot(cmdvec, i)) && node->func + && (node->vtysh || vty->type != VTY_SHELL)) { + if ((*node->func)(vty)) + vty_out(vty, "!\n"); + } if (vty->type == VTY_TERM) { vty_out(vty, "end\n"); diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c index 97550eae53..5c71fac10a 100644 --- a/lib/frr_pthread.c +++ b/lib/frr_pthread.c @@ -35,6 +35,9 @@ DEFINE_MTYPE_STATIC(LIB, PTHREAD_PRIM, "POSIX sync primitives") static void *fpt_run(void *arg); static int fpt_halt(struct frr_pthread *fpt, void **res); +/* misc sigs */ +static void frr_pthread_destroy_nolock(struct frr_pthread *fpt); + /* default frr_pthread attributes */ struct frr_pthread_attr frr_pthread_attr_default = { .start = fpt_run, @@ -59,6 +62,14 @@ void frr_pthread_finish(void) frr_pthread_stop_all(); frr_with_mutex(&frr_pthread_list_mtx) { + struct listnode *n, *nn; + struct frr_pthread *fpt; + + for (ALL_LIST_ELEMENTS(frr_pthread_list, n, nn, fpt)) { + listnode_delete(frr_pthread_list, fpt); + frr_pthread_destroy_nolock(fpt); + } + list_delete(&frr_pthread_list); } } @@ -98,12 +109,8 @@ struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, return fpt; } -void frr_pthread_destroy(struct frr_pthread *fpt) +static void frr_pthread_destroy_nolock(struct frr_pthread *fpt) { - frr_with_mutex(&frr_pthread_list_mtx) { - listnode_delete(frr_pthread_list, fpt); - } - thread_master_free(fpt->master); pthread_mutex_destroy(&fpt->mtx); pthread_mutex_destroy(fpt->running_cond_mtx); @@ -114,6 +121,15 @@ void frr_pthread_destroy(struct frr_pthread *fpt) XFREE(MTYPE_FRR_PTHREAD, fpt); } +void frr_pthread_destroy(struct frr_pthread *fpt) +{ + frr_with_mutex(&frr_pthread_list_mtx) { + listnode_delete(frr_pthread_list, fpt); + } + + frr_pthread_destroy_nolock(fpt); +} + int frr_pthread_set_name(struct frr_pthread *fpt) { int ret = 0; @@ -207,21 +207,18 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id) if (yang_module_find("frr-interface")) { struct lyd_node *if_dnode; - pthread_rwlock_wrlock(&running_config->lock); - { - if_dnode = yang_dnode_get( - running_config->dnode, - "/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf", - ifp->name, old_vrf->name); - if (if_dnode) { - yang_dnode_change_leaf(if_dnode, vrf->name); - running_config->version++; - } + if_dnode = yang_dnode_get( + running_config->dnode, + "/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf", + ifp->name, old_vrf->name); + if (if_dnode) { + yang_dnode_change_leaf(if_dnode, vrf->name); + running_config->version++; } - pthread_rwlock_unlock(&running_config->lock); } } + /* Delete interface structure. */ void if_delete_retain(struct interface *ifp) { @@ -1461,6 +1458,60 @@ static int lib_interface_destroy(enum nb_event event, } /* + * XPath: /frr-interface:lib/interface + */ +static const void *lib_interface_get_next(const void *parent_list_entry, + const void *list_entry) +{ + struct vrf *vrf; + struct interface *pif = (struct interface *)list_entry; + + if (list_entry == NULL) { + vrf = RB_MIN(vrf_name_head, &vrfs_by_name); + assert(vrf); + pif = RB_MIN(if_name_head, &vrf->ifaces_by_name); + } else { + vrf = vrf_lookup_by_id(pif->vrf_id); + pif = RB_NEXT(if_name_head, pif); + /* if no more interfaces, switch to next vrf */ + while (pif == NULL) { + vrf = RB_NEXT(vrf_name_head, vrf); + if (!vrf) + return NULL; + pif = RB_MIN(if_name_head, &vrf->ifaces_by_name); + } + } + + return pif; +} + +static int lib_interface_get_keys(const void *list_entry, + struct yang_list_keys *keys) +{ + const struct interface *ifp = list_entry; + + struct vrf *vrf = vrf_lookup_by_id(ifp->vrf_id); + + assert(vrf); + + keys->num = 2; + strlcpy(keys->key[0], ifp->name, sizeof(keys->key[0])); + strlcpy(keys->key[1], vrf->name, sizeof(keys->key[1])); + + return NB_OK; +} + +static const void *lib_interface_lookup_entry(const void *parent_list_entry, + const struct yang_list_keys *keys) +{ + const char *ifname = keys->key[0]; + const char *vrfname = keys->key[1]; + struct vrf *vrf = vrf_lookup_by_name(vrfname); + + return if_lookup_by_name(ifname, vrf->vrf_id); +} + +/* * XPath: /frr-interface:lib/interface/description */ static int lib_interface_description_modify(enum nb_event event, @@ -1505,6 +1556,9 @@ const struct frr_yang_module_info frr_interface_info = { .create = lib_interface_create, .destroy = lib_interface_destroy, .cli_show = cli_show_interface, + .get_next = lib_interface_get_next, + .get_keys = lib_interface_get_keys, + .lookup_entry = lib_interface_lookup_entry, }, }, { diff --git a/lib/libfrr.c b/lib/libfrr.c index 478fe4e205..d4aa1f899a 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -868,12 +868,7 @@ static int frr_config_read_in(struct thread *t) /* * Update the shared candidate after reading the startup configuration. */ - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_replace(vty_shared_candidate_config, running_config, - true); - } - pthread_rwlock_unlock(&running_config->lock); + nb_config_replace(vty_shared_candidate_config, running_config, true); return 0; } diff --git a/lib/mpls.h b/lib/mpls.h index 472ee9bc46..635ecc77a1 100644 --- a/lib/mpls.h +++ b/lib/mpls.h @@ -47,6 +47,7 @@ extern "C" { #define MPLS_LABEL_OAM_ALERT 14 /* [RFC3429] */ #define MPLS_LABEL_EXTENSION 15 /* [RFC7274] */ #define MPLS_LABEL_MAX 1048575 +#define MPLS_LABEL_VALUE_MASK 0x000FFFFF #define MPLS_LABEL_NONE 0xFFFFFFFF /* for internal use only */ /* Minimum and maximum label values */ diff --git a/lib/northbound.c b/lib/northbound.c index a814f23e14..1b332fb1e8 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -64,6 +64,9 @@ static bool transaction_in_progress; static int nb_callback_configuration(const enum nb_event event, struct nb_config_change *change); +static void nb_log_callback(const enum nb_event event, + enum nb_operation operation, const char *xpath, + const char *value); static struct nb_transaction *nb_transaction_new(struct nb_config *config, struct nb_config_cbs *changes, enum nb_client client, @@ -180,7 +183,18 @@ static int nb_node_validate_cb(const struct nb_node *nb_node, valid = nb_operation_is_valid(operation, nb_node->snode); - if (!valid && callback_implemented) + /* + * Add an exception for operational data callbacks. A rw list usually + * doesn't need any associated operational data callbacks. But if this + * rw list is augmented by another module which adds state nodes under + * it, then this list will need to have the 'get_next()', 'get_keys()' + * and 'lookup_entry()' callbacks. As such, never log a warning when + * these callbacks are implemented when they are not needed, since this + * depends on context (e.g. some daemons might augment "frr-interface" + * while others don't). + */ + if (!valid && callback_implemented && operation != NB_OP_GET_NEXT + && operation != NB_OP_GET_KEYS && operation != NB_OP_LOOKUP_ENTRY) flog_warn(EC_LIB_NB_CB_UNNEEDED, "unneeded '%s' callback for '%s'", nb_operation_name(operation), nb_node->xpath); @@ -211,6 +225,8 @@ static unsigned int nb_node_validate_cbs(const struct nb_node *nb_node) !!nb_node->cbs.destroy, false); error += nb_node_validate_cb(nb_node, NB_OP_MOVE, !!nb_node->cbs.move, false); + error += nb_node_validate_cb(nb_node, NB_OP_PRE_VALIDATE, + !!nb_node->cbs.pre_validate, true); error += nb_node_validate_cb(nb_node, NB_OP_APPLY_FINISH, !!nb_node->cbs.apply_finish, true); error += nb_node_validate_cb(nb_node, NB_OP_GET_ELEM, @@ -265,7 +281,6 @@ struct nb_config *nb_config_new(struct lyd_node *dnode) else config->dnode = yang_dnode_new(ly_native_ctx, true); config->version = 0; - pthread_rwlock_init(&config->lock, NULL); return config; } @@ -274,7 +289,6 @@ void nb_config_free(struct nb_config *config) { if (config->dnode) yang_dnode_free(config->dnode); - pthread_rwlock_destroy(&config->lock); XFREE(MTYPE_NB_CONFIG, config); } @@ -285,7 +299,6 @@ struct nb_config *nb_config_dup(const struct nb_config *config) dup = XCALLOC(MTYPE_NB_CONFIG, sizeof(*dup)); dup->dnode = yang_dnode_dup(config->dnode); dup->version = config->version; - pthread_rwlock_init(&dup->lock, NULL); return dup; } @@ -335,21 +348,23 @@ static inline int nb_config_cb_compare(const struct nb_config_cb *a, return 1; /* - * Use XPath as a tie-breaker. This will naturally sort parent nodes - * before their children. + * Preserve the order of the configuration changes as told by libyang. */ - return strcmp(a->xpath, b->xpath); + return a->seq - b->seq; } RB_GENERATE(nb_config_cbs, nb_config_cb, entry, nb_config_cb_compare); static void nb_config_diff_add_change(struct nb_config_cbs *changes, enum nb_operation operation, + uint32_t *seq, const struct lyd_node *dnode) { struct nb_config_change *change; change = XCALLOC(MTYPE_TMP, sizeof(*change)); change->cb.operation = operation; + change->cb.seq = *seq; + *seq = *seq + 1; change->cb.nb_node = dnode->schema->priv; yang_dnode_get_path(dnode, change->cb.xpath, sizeof(change->cb.xpath)); change->cb.dnode = dnode; @@ -374,7 +389,7 @@ static void nb_config_diff_del_changes(struct nb_config_cbs *changes) * configurations. Given a new subtree, calculate all new YANG data nodes, * excluding default leafs and leaf-lists. This is a recursive function. */ -static void nb_config_diff_created(const struct lyd_node *dnode, +static void nb_config_diff_created(const struct lyd_node *dnode, uint32_t *seq, struct nb_config_cbs *changes) { enum nb_operation operation; @@ -393,16 +408,17 @@ static void nb_config_diff_created(const struct lyd_node *dnode, else return; - nb_config_diff_add_change(changes, operation, dnode); + nb_config_diff_add_change(changes, operation, seq, dnode); break; case LYS_CONTAINER: case LYS_LIST: if (nb_operation_is_valid(NB_OP_CREATE, dnode->schema)) - nb_config_diff_add_change(changes, NB_OP_CREATE, dnode); + nb_config_diff_add_change(changes, NB_OP_CREATE, seq, + dnode); /* Process child nodes recursively. */ LY_TREE_FOR (dnode->child, child) { - nb_config_diff_created(child, changes); + nb_config_diff_created(child, seq, changes); } break; default: @@ -410,11 +426,11 @@ static void nb_config_diff_created(const struct lyd_node *dnode, } } -static void nb_config_diff_deleted(const struct lyd_node *dnode, +static void nb_config_diff_deleted(const struct lyd_node *dnode, uint32_t *seq, struct nb_config_cbs *changes) { if (nb_operation_is_valid(NB_OP_DESTROY, dnode->schema)) - nb_config_diff_add_change(changes, NB_OP_DESTROY, dnode); + nb_config_diff_add_change(changes, NB_OP_DESTROY, seq, dnode); else if (CHECK_FLAG(dnode->schema->nodetype, LYS_CONTAINER)) { struct lyd_node *child; @@ -425,7 +441,7 @@ static void nb_config_diff_deleted(const struct lyd_node *dnode, * when applicable (i.e. optional nodes). */ LY_TREE_FOR (dnode->child, child) { - nb_config_diff_deleted(child, changes); + nb_config_diff_deleted(child, seq, changes); } } } @@ -436,6 +452,7 @@ static void nb_config_diff(const struct nb_config *config1, struct nb_config_cbs *changes) { struct lyd_difflist *diff; + uint32_t seq = 0; diff = lyd_diff(config1->dnode, config2->dnode, LYD_DIFFOPT_WITHDEFAULTS); @@ -450,15 +467,16 @@ static void nb_config_diff(const struct nb_config *config1, switch (type) { case LYD_DIFF_CREATED: dnode = diff->second[i]; - nb_config_diff_created(dnode, changes); + nb_config_diff_created(dnode, &seq, changes); break; case LYD_DIFF_DELETED: dnode = diff->first[i]; - nb_config_diff_deleted(dnode, changes); + nb_config_diff_deleted(dnode, &seq, changes); break; case LYD_DIFF_CHANGED: dnode = diff->second[i]; - nb_config_diff_add_change(changes, NB_OP_MODIFY, dnode); + nb_config_diff_add_change(changes, NB_OP_MODIFY, &seq, + dnode); break; case LYD_DIFF_MOVEDAFTER1: case LYD_DIFF_MOVEDAFTER2: @@ -535,28 +553,17 @@ int nb_candidate_edit(struct nb_config *candidate, bool nb_candidate_needs_update(const struct nb_config *candidate) { - bool ret = false; - - pthread_rwlock_rdlock(&running_config->lock); - { - if (candidate->version < running_config->version) - ret = true; - } - pthread_rwlock_unlock(&running_config->lock); + if (candidate->version < running_config->version) + return true; - return ret; + return false; } int nb_candidate_update(struct nb_config *candidate) { struct nb_config *updated_config; - pthread_rwlock_rdlock(&running_config->lock); - { - updated_config = nb_config_dup(running_config); - } - pthread_rwlock_unlock(&running_config->lock); - + updated_config = nb_config_dup(running_config); if (nb_config_merge(updated_config, candidate, true) != NB_OK) return NB_ERR; @@ -583,14 +590,45 @@ static int nb_candidate_validate_yang(struct nb_config *candidate) } /* Perform code-level validation using the northbound callbacks. */ -static int nb_candidate_validate_changes(struct nb_config *candidate, - struct nb_config_cbs *changes) +static int nb_candidate_validate_code(struct nb_config *candidate, + struct nb_config_cbs *changes) { struct nb_config_cb *cb; + struct lyd_node *root, *next, *child; + int ret; + + /* First validate the candidate as a whole. */ + LY_TREE_FOR (candidate->dnode, root) { + LY_TREE_DFS_BEGIN (root, next, child) { + struct nb_node *nb_node; + + nb_node = child->schema->priv; + if (!nb_node->cbs.pre_validate) + goto next; + + if (DEBUG_MODE_CHECK(&nb_dbg_cbs_config, + DEBUG_MODE_ALL)) { + char xpath[XPATH_MAXLEN]; + + yang_dnode_get_path(child, xpath, + sizeof(xpath)); + nb_log_callback(NB_EV_VALIDATE, + NB_OP_PRE_VALIDATE, xpath, + NULL); + } + + ret = (*nb_node->cbs.pre_validate)(child); + if (ret != NB_OK) + return NB_ERR_VALIDATION; + next: + LY_TREE_DFS_END(root, next, child); + } + } + + /* Now validate the configuration changes. */ RB_FOREACH (cb, nb_config_cbs, changes) { struct nb_config_change *change = (struct nb_config_change *)cb; - int ret; ret = nb_callback_configuration(NB_EV_VALIDATE, change); if (ret != NB_OK) @@ -609,13 +647,9 @@ int nb_candidate_validate(struct nb_config *candidate) return NB_ERR_VALIDATION; RB_INIT(nb_config_cbs, &changes); - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_diff(running_config, candidate, &changes); - ret = nb_candidate_validate_changes(candidate, &changes); - nb_config_diff_del_changes(&changes); - } - pthread_rwlock_unlock(&running_config->lock); + nb_config_diff(running_config, candidate, &changes); + ret = nb_candidate_validate_code(candidate, &changes); + nb_config_diff_del_changes(&changes); return ret; } @@ -635,36 +669,26 @@ int nb_candidate_commit_prepare(struct nb_config *candidate, } RB_INIT(nb_config_cbs, &changes); - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_diff(running_config, candidate, &changes); - if (RB_EMPTY(nb_config_cbs, &changes)) { - pthread_rwlock_unlock(&running_config->lock); - return NB_ERR_NO_CHANGES; - } + nb_config_diff(running_config, candidate, &changes); + if (RB_EMPTY(nb_config_cbs, &changes)) + return NB_ERR_NO_CHANGES; - if (nb_candidate_validate_changes(candidate, &changes) - != NB_OK) { - flog_warn( - EC_LIB_NB_CANDIDATE_INVALID, - "%s: failed to validate candidate configuration", - __func__); - nb_config_diff_del_changes(&changes); - pthread_rwlock_unlock(&running_config->lock); - return NB_ERR_VALIDATION; - } + if (nb_candidate_validate_code(candidate, &changes) != NB_OK) { + flog_warn(EC_LIB_NB_CANDIDATE_INVALID, + "%s: failed to validate candidate configuration", + __func__); + nb_config_diff_del_changes(&changes); + return NB_ERR_VALIDATION; + } - *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__); - nb_config_diff_del_changes(&changes); - pthread_rwlock_unlock(&running_config->lock); - return NB_ERR_LOCKED; - } + *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__); + nb_config_diff_del_changes(&changes); + return NB_ERR_LOCKED; } - pthread_rwlock_unlock(&running_config->lock); return nb_transaction_process(NB_EV_PREPARE, *transaction); } @@ -683,11 +707,7 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction, /* Replace running by candidate. */ transaction->config->version++; - pthread_rwlock_wrlock(&running_config->lock); - { - nb_config_replace(running_config, transaction->config, true); - } - pthread_rwlock_unlock(&running_config->lock); + nb_config_replace(running_config, transaction->config, true); /* Record transaction. */ if (save_transaction @@ -961,52 +981,40 @@ static int nb_transaction_process(enum nb_event event, { struct nb_config_cb *cb; - /* - * Need to lock the running configuration since transaction->changes - * can contain pointers to data nodes from the running configuration. - */ - pthread_rwlock_rdlock(&running_config->lock); - { - RB_FOREACH (cb, nb_config_cbs, &transaction->changes) { - struct nb_config_change *change = - (struct nb_config_change *)cb; - int ret; + RB_FOREACH (cb, nb_config_cbs, &transaction->changes) { + struct nb_config_change *change = (struct nb_config_change *)cb; + int ret; + /* + * Only try to release resources that were allocated + * successfully. + */ + if (event == NB_EV_ABORT && change->prepare_ok == false) + break; + + /* Call the appropriate callback. */ + ret = nb_callback_configuration(event, change); + switch (event) { + case NB_EV_PREPARE: + if (ret != NB_OK) + return ret; + change->prepare_ok = true; + break; + case NB_EV_ABORT: + case NB_EV_APPLY: /* - * Only try to release resources that were allocated - * successfully. + * At this point it's not possible to reject the + * transaction anymore, so any failure here can lead to + * inconsistencies and should be treated as a bug. + * Operations prone to errors, like validations and + * resource allocations, should be performed during the + * 'prepare' phase. */ - if (event == NB_EV_ABORT && change->prepare_ok == false) - break; - - /* Call the appropriate callback. */ - ret = nb_callback_configuration(event, change); - switch (event) { - case NB_EV_PREPARE: - if (ret != NB_OK) { - pthread_rwlock_unlock( - &running_config->lock); - return ret; - } - change->prepare_ok = true; - break; - case NB_EV_ABORT: - case NB_EV_APPLY: - /* - * At this point it's not possible to reject the - * transaction anymore, so any failure here can - * lead to inconsistencies and should be treated - * as a bug. Operations prone to errors, like - * validations and resource allocations, should - * be performed during the 'prepare' phase. - */ - break; - default: - break; - } + break; + default: + break; } } - pthread_rwlock_unlock(&running_config->lock); return NB_OK; } @@ -1310,9 +1318,27 @@ static int nb_oper_data_iter_node(const struct lys_node *snode, /* Update XPath. */ strlcpy(xpath, xpath_parent, sizeof(xpath)); - if (!first && snode->nodetype != LYS_USES) - snprintf(xpath + strlen(xpath), sizeof(xpath) - strlen(xpath), - "/%s", snode->name); + if (!first && snode->nodetype != LYS_USES) { + struct lys_node *parent; + + /* Get the real parent. */ + parent = snode->parent; + while (parent && parent->nodetype == LYS_USES) + parent = parent->parent; + + /* + * When necessary, include the namespace of the augmenting + * module. + */ + if (parent && parent->nodetype == LYS_AUGMENT) + snprintf(xpath + strlen(xpath), + sizeof(xpath) - strlen(xpath), "/%s:%s", + snode->module->name, snode->name); + else + snprintf(xpath + strlen(xpath), + sizeof(xpath) - strlen(xpath), "/%s", + snode->name); + } nb_node = snode->priv; switch (snode->nodetype) { @@ -1550,6 +1576,7 @@ bool nb_operation_is_valid(enum nb_operation operation, return false; } return true; + case NB_OP_PRE_VALIDATE: case NB_OP_APPLY_FINISH: if (!CHECK_FLAG(snode->flags, LYS_CONFIG_W)) return false; @@ -1768,6 +1795,8 @@ const char *nb_operation_name(enum nb_operation operation) return "destroy"; case NB_OP_MOVE: return "move"; + case NB_OP_PRE_VALIDATE: + return "pre_validate"; case NB_OP_APPLY_FINISH: return "apply_finish"; case NB_OP_GET_ELEM: diff --git a/lib/northbound.h b/lib/northbound.h index ce79d907f9..fbd7771db7 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -72,6 +72,7 @@ enum nb_operation { NB_OP_MODIFY, NB_OP_DESTROY, NB_OP_MOVE, + NB_OP_PRE_VALIDATE, NB_OP_APPLY_FINISH, NB_OP_GET_ELEM, NB_OP_GET_NEXT, @@ -199,6 +200,19 @@ struct nb_callbacks { /* * Optional configuration callback. * + * This callback can be used to validate subsections of the + * configuration being committed before validating the configuration + * changes themselves. It's useful to perform more complex validations + * that depend on the relationship between multiple nodes. + * + * dnode + * libyang data node associated with the 'pre_validate' callback. + */ + int (*pre_validate)(const struct lyd_node *dnode); + + /* + * Optional configuration callback. + * * The 'apply_finish' callbacks are called after all other callbacks * during the apply phase (NB_EV_APPLY). These callbacks are called only * under one of the following two cases: @@ -435,25 +449,15 @@ enum nb_client { /* Northbound configuration. */ struct nb_config { - /* Configuration data. */ struct lyd_node *dnode; - - /* Configuration version. */ uint32_t version; - - /* - * Lock protecting this structure. The use of this lock is always - * necessary when reading or modifying the global running configuration. - * For candidate configurations, use of this lock is optional depending - * on the threading scheme of the northbound plugin. - */ - pthread_rwlock_t lock; }; /* Northbound configuration callback. */ struct nb_config_cb { RB_ENTRY(nb_config_cb) entry; enum nb_operation operation; + uint32_t seq; char xpath[XPATH_MAXLEN]; const struct nb_node *nb_node; const struct lyd_node *dnode; diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index 884c01a457..a15fe3d1c9 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -193,13 +193,8 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) "Please check the logs for more details.\n"); /* Regenerate candidate for consistency. */ - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_replace(vty->candidate_config, - running_config, true); - } - pthread_rwlock_unlock(&running_config->lock); - + nb_config_replace(vty->candidate_config, running_config, + true); return CMD_WARNING_CONFIG_FAILED; } } @@ -307,12 +302,7 @@ static int nb_cli_commit(struct vty *vty, bool force, /* "confirm" parameter. */ if (confirmed_timeout) { - pthread_rwlock_rdlock(&running_config->lock); - { - vty->confirmed_commit_rollback = - nb_config_dup(running_config); - } - pthread_rwlock_unlock(&running_config->lock); + 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, @@ -326,13 +316,8 @@ static int nb_cli_commit(struct vty *vty, bool force, /* Map northbound return code to CLI return code. */ switch (ret) { case NB_OK: - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_replace(vty->candidate_config_base, - running_config, true); - } - pthread_rwlock_unlock(&running_config->lock); - + nb_config_replace(vty->candidate_config_base, running_config, + true); vty_out(vty, "%% Configuration committed successfully (Transaction ID #%u).\n\n", transaction_id); @@ -729,12 +714,7 @@ DEFPY (config_update, return CMD_WARNING; } - pthread_rwlock_rdlock(&running_config->lock); - { - nb_config_replace(vty->candidate_config_base, running_config, - true); - } - pthread_rwlock_unlock(&running_config->lock); + nb_config_replace(vty->candidate_config_base, running_config, true); vty_out(vty, "%% Candidate configuration updated successfully.\n\n"); @@ -834,12 +814,8 @@ DEFPY (show_config_running, } } - pthread_rwlock_rdlock(&running_config->lock); - { - nb_cli_show_config(vty, running_config, format, translator, - !!with_defaults); - } - pthread_rwlock_unlock(&running_config->lock); + nb_cli_show_config(vty, running_config, format, translator, + !!with_defaults); return CMD_SUCCESS; } @@ -953,68 +929,57 @@ DEFPY (show_config_compare, struct nb_config *config2, *config_transaction2 = NULL; int ret = CMD_WARNING; - /* - * For simplicity, lock the running configuration regardless if it's - * going to be used or not. - */ - pthread_rwlock_rdlock(&running_config->lock); - { - if (c1_candidate) - config1 = vty->candidate_config; - else if (c1_running) - config1 = running_config; - else { - config_transaction1 = nb_db_transaction_load(c1_tid); - if (!config_transaction1) { - vty_out(vty, - "%% Transaction %u does not exist\n\n", - (unsigned int)c1_tid); - goto exit; - } - config1 = config_transaction1; + if (c1_candidate) + config1 = vty->candidate_config; + else if (c1_running) + config1 = running_config; + else { + config_transaction1 = nb_db_transaction_load(c1_tid); + if (!config_transaction1) { + vty_out(vty, "%% Transaction %u does not exist\n\n", + (unsigned int)c1_tid); + goto exit; } + config1 = config_transaction1; + } - if (c2_candidate) - config2 = vty->candidate_config; - else if (c2_running) - config2 = running_config; - else { - config_transaction2 = nb_db_transaction_load(c2_tid); - if (!config_transaction2) { - vty_out(vty, - "%% Transaction %u does not exist\n\n", - (unsigned int)c2_tid); - goto exit; - } - config2 = config_transaction2; + if (c2_candidate) + config2 = vty->candidate_config; + else if (c2_running) + config2 = running_config; + else { + config_transaction2 = nb_db_transaction_load(c2_tid); + if (!config_transaction2) { + vty_out(vty, "%% Transaction %u does not exist\n\n", + (unsigned int)c2_tid); + goto exit; } + config2 = config_transaction2; + } - if (json) - format = NB_CFG_FMT_JSON; - else if (xml) - format = NB_CFG_FMT_XML; - else - format = NB_CFG_FMT_CMDS; + if (json) + format = NB_CFG_FMT_JSON; + else if (xml) + format = NB_CFG_FMT_XML; + else + format = NB_CFG_FMT_CMDS; - if (translator_family) { - translator = yang_translator_find(translator_family); - if (!translator) { - vty_out(vty, - "%% Module translator \"%s\" not found\n", - translator_family); - goto exit; - } + if (translator_family) { + translator = yang_translator_find(translator_family); + if (!translator) { + vty_out(vty, "%% Module translator \"%s\" not found\n", + translator_family); + goto exit; } - - ret = nb_cli_show_config_compare(vty, config1, config2, format, - translator); - exit: - if (config_transaction1) - nb_config_free(config_transaction1); - if (config_transaction2) - nb_config_free(config_transaction2); } - pthread_rwlock_unlock(&running_config->lock); + + ret = nb_cli_show_config_compare(vty, config1, config2, format, + translator); +exit: + if (config_transaction1) + nb_config_free(config_transaction1); + if (config_transaction2) + nb_config_free(config_transaction2); return ret; } diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c index e9669fc7e1..2fc3c81cf2 100644 --- a/lib/northbound_confd.c +++ b/lib/northbound_confd.c @@ -289,11 +289,7 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen) struct cdb_iter_args iter_args; int ret; - pthread_rwlock_rdlock(&running_config->lock); - { - candidate = nb_config_dup(running_config); - } - pthread_rwlock_unlock(&running_config->lock); + candidate = nb_config_dup(running_config); /* Iterate over all configuration changes. */ iter_args.candidate = candidate; diff --git a/lib/northbound_grpc.cpp b/lib/northbound_grpc.cpp index a55da23dd1..1d6317b005 100644 --- a/lib/northbound_grpc.cpp +++ b/lib/northbound_grpc.cpp @@ -702,15 +702,10 @@ class NorthboundImpl final : public frr::Northbound::Service { struct lyd_node *dnode; - pthread_rwlock_rdlock(&running_config->lock); - { - dnode = yang_dnode_get(running_config->dnode, - path.empty() ? NULL - : path.c_str()); - if (dnode) - dnode = yang_dnode_dup(dnode); - } - pthread_rwlock_unlock(&running_config->lock); + dnode = yang_dnode_get(running_config->dnode, + path.empty() ? NULL : path.c_str()); + if (dnode) + dnode = yang_dnode_dup(dnode); return dnode; } @@ -817,11 +812,7 @@ class NorthboundImpl final : public frr::Northbound::Service struct candidate *candidate = &_candidates[candidate_id]; candidate->id = candidate_id; - pthread_rwlock_rdlock(&running_config->lock); - { - candidate->config = nb_config_dup(running_config); - } - pthread_rwlock_unlock(&running_config->lock); + candidate->config = nb_config_dup(running_config); candidate->transaction = NULL; return candidate; diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index 77183282ba..b94c939763 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -256,11 +256,7 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session, return ret; } - pthread_rwlock_rdlock(&running_config->lock); - { - candidate = nb_config_dup(running_config); - } - pthread_rwlock_unlock(&running_config->lock); + candidate = nb_config_dup(running_config); while ((ret = sr_get_change_next(session, it, &sr_op, &sr_old_val, &sr_new_val)) @@ -2582,22 +2582,17 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive) vty->private_config = private_config; vty->xpath_index = 0; - pthread_rwlock_rdlock(&running_config->lock); - { - if (private_config) { - vty->candidate_config = nb_config_dup(running_config); + if (private_config) { + vty->candidate_config = nb_config_dup(running_config); + vty->candidate_config_base = nb_config_dup(running_config); + vty_out(vty, + "Warning: uncommitted changes will be discarded on exit.\n\n"); + } else { + vty->candidate_config = vty_shared_candidate_config; + if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) vty->candidate_config_base = nb_config_dup(running_config); - vty_out(vty, - "Warning: uncommitted changes will be discarded on exit.\n\n"); - } else { - vty->candidate_config = vty_shared_candidate_config; - if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) - vty->candidate_config_base = - nb_config_dup(running_config); - } } - pthread_rwlock_unlock(&running_config->lock); return CMD_SUCCESS; } diff --git a/lib/yang_wrappers.c b/lib/yang_wrappers.c index cd776f333d..50225f35a0 100644 --- a/lib/yang_wrappers.c +++ b/lib/yang_wrappers.c @@ -782,6 +782,60 @@ void yang_get_default_string_buf(char *buf, size_t size, const char *xpath_fmt, } /* + * Derived type: IP prefix. + */ +void yang_str2prefix(const char *value, union prefixptr prefix) +{ + (void)str2prefix(value, prefix.p); + apply_mask(prefix.p); +} + +struct yang_data *yang_data_new_prefix(const char *xpath, + union prefixconstptr prefix) +{ + char value_str[PREFIX2STR_BUFFER]; + + (void)prefix2str(prefix.p, value_str, sizeof(value_str)); + return yang_data_new(xpath, value_str); +} + +void yang_dnode_get_prefix(union prefixptr prefix, const struct lyd_node *dnode, + const char *xpath_fmt, ...) +{ + const struct lyd_node_leaf_list *dleaf; + + assert(dnode); + if (xpath_fmt) { + va_list ap; + char xpath[XPATH_MAXLEN]; + + va_start(ap, xpath_fmt); + vsnprintf(xpath, sizeof(xpath), xpath_fmt, ap); + va_end(ap); + dnode = yang_dnode_get(dnode, xpath); + YANG_DNODE_GET_ASSERT(dnode, xpath); + } + + dleaf = (const struct lyd_node_leaf_list *)dnode; + assert(dleaf->value_type == LY_TYPE_STRING); + (void)str2prefix(dleaf->value_str, prefix.p); +} + +void yang_get_default_prefix(union prefixptr var, const char *xpath_fmt, ...) +{ + char xpath[XPATH_MAXLEN]; + const char *value; + va_list ap; + + va_start(ap, xpath_fmt); + vsnprintf(xpath, sizeof(xpath), xpath_fmt, ap); + va_end(ap); + + value = yang_get_default_value(xpath); + yang_str2prefix(value, var); +} + +/* * Derived type: ipv4. */ void yang_str2ipv4(const char *value, struct in_addr *addr) diff --git a/lib/yang_wrappers.h b/lib/yang_wrappers.h index ab7abe173c..1a30ff3686 100644 --- a/lib/yang_wrappers.h +++ b/lib/yang_wrappers.h @@ -114,6 +114,16 @@ extern const char *yang_get_default_string(const char *xpath_fmt, ...); extern void yang_get_default_string_buf(char *buf, size_t size, const char *xpath_fmt, ...); +/* ip prefix */ +extern void yang_str2prefix(const char *value, union prefixptr prefix); +extern struct yang_data *yang_data_new_prefix(const char *xpath, + union prefixconstptr prefix); +extern void yang_dnode_get_prefix(union prefixptr prefix, + const struct lyd_node *dnode, + const char *xpath_fmt, ...); +extern void yang_get_default_prefix(union prefixptr var, const char *xpath_fmt, + ...); + /* ipv4 */ extern void yang_str2ipv4(const char *value, struct in_addr *addr); extern struct yang_data *yang_data_new_ipv4(const char *xpath, |