diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index a649e896fa..9633e46415 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -116,8 +116,8 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) addr.addr_len = dotformat2buff(buff, net_title); memcpy(addr.area_addr, buff, addr.addr_len); if (addr.area_addr[addr.addr_len - 1] != 0) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "nsel byte (last byte) in area address must be 0"); return NB_ERR_VALIDATION; } @@ -125,8 +125,8 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) /* Check that the SystemID portions match */ if (memcmp(isis->sysid, GETSYSID((&addr)), ISIS_SYS_ID_LEN)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "System ID must not change when defining additional area addresses"); return NB_ERR_VALIDATION; } @@ -332,8 +332,8 @@ int isis_instance_lsp_mtu_modify(struct nb_cb_modify_args *args) && circuit->state != C_STATE_UP) continue; if (lsp_mtu > isis_circuit_pdu_size(circuit)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "ISIS area contains circuit %s, which has a maximum PDU size of %zu", circuit->interface->name, isis_circuit_pdu_size(circuit)); @@ -1047,6 +1047,7 @@ int isis_instance_redistribute_ipv6_metric_modify( */ static int isis_multi_topology_common(enum nb_event event, const struct lyd_node *dnode, + char *errmsg, size_t errmsg_len, const char *topology, bool create) { struct isis_area *area; @@ -1056,8 +1057,8 @@ static int isis_multi_topology_common(enum nb_event event, switch (event) { case NB_EV_VALIDATE: if (mtid == (uint16_t)-1) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Unknown topology %s", topology); + snprintf(errmsg, errmsg_len, "Unknown topology %s", + topology); return NB_ERR_VALIDATION; } break; @@ -1100,6 +1101,7 @@ int isis_instance_multi_topology_ipv4_multicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv4-multicast", true); } @@ -1107,6 +1109,7 @@ int isis_instance_multi_topology_ipv4_multicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv4-multicast", false); } @@ -1126,15 +1129,17 @@ int isis_instance_multi_topology_ipv4_multicast_overload_modify( int isis_instance_multi_topology_ipv4_management_create( struct nb_cb_create_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv4-mgmt", - true); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv4-mgmt", true); } int isis_instance_multi_topology_ipv4_management_destroy( struct nb_cb_destroy_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv4-mgmt", - false); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv4-mgmt", false); } /* @@ -1154,6 +1159,7 @@ int isis_instance_multi_topology_ipv6_unicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-unicast", true); } @@ -1161,6 +1167,7 @@ int isis_instance_multi_topology_ipv6_unicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-unicast", false); } @@ -1181,6 +1188,7 @@ int isis_instance_multi_topology_ipv6_multicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-multicast", true); } @@ -1188,6 +1196,7 @@ int isis_instance_multi_topology_ipv6_multicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-multicast", false); } @@ -1207,15 +1216,17 @@ int isis_instance_multi_topology_ipv6_multicast_overload_modify( int isis_instance_multi_topology_ipv6_management_create( struct nb_cb_create_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv6-mgmt", - true); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv6-mgmt", true); } int isis_instance_multi_topology_ipv6_management_destroy( struct nb_cb_destroy_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv6-mgmt", - false); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv6-mgmt", false); } /* @@ -1235,6 +1246,7 @@ int isis_instance_multi_topology_ipv6_dstsrc_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-dstsrc", true); } @@ -1242,6 +1254,7 @@ int isis_instance_multi_topology_ipv6_dstsrc_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-dstsrc", false); } @@ -1721,10 +1734,10 @@ int lib_interface_isis_create(struct nb_cb_create_args *args) min_mtu = DEFAULT_LSP_MTU; #endif /* ifndef FABRICD */ if (actual_mtu < min_mtu) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Interface %s has MTU %" PRIu32 - ", minimum MTU for the area is %" PRIu32 "", - ifp->name, actual_mtu, min_mtu); + snprintf(args->errmsg, args->errmsg_len, + "Interface %s has MTU %" PRIu32 + ", minimum MTU for the area is %" PRIu32 "", + ifp->name, actual_mtu, min_mtu); return NB_ERR_VALIDATION; } break; @@ -1801,9 +1814,9 @@ int lib_interface_isis_area_tag_modify(struct nb_cb_modify_args *args) area_tag = yang_dnode_get_string(args->dnode, NULL); if (circuit && circuit->area && circuit->area->area_tag && strcmp(circuit->area->area_tag, area_tag)) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "ISIS circuit is already defined on %s", - circuit->area->area_tag); + snprintf(args->errmsg, args->errmsg_len, + "ISIS circuit is already defined on %s", + circuit->area->area_tag); return NB_ERR_VALIDATION; } } @@ -1839,9 +1852,9 @@ int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args) if (circuit && circuit->state == C_STATE_UP && circuit->area->is_type != IS_LEVEL_1_AND_2 && circuit->area->is_type != circ_type) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Invalid circuit level for area %s", - circuit->area->area_tag); + snprintf(args->errmsg, args->errmsg_len, + "Invalid circuit level for area %s", + circuit->area->area_tag); return NB_ERR_VALIDATION; } break; @@ -2163,16 +2176,16 @@ int lib_interface_isis_network_type_modify(struct nb_cb_modify_args *args) if (!circuit) break; if (circuit->circ_type == CIRCUIT_T_LOOPBACK) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "Cannot change network type on loopback interface"); return NB_ERR_VALIDATION; } if (net_type == CIRCUIT_T_BROADCAST && circuit->state == C_STATE_UP && !if_is_broadcast(circuit->interface)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "Cannot configure non-broadcast interface for broadcast operation"); return NB_ERR_VALIDATION; } @@ -2208,8 +2221,8 @@ int lib_interface_isis_passive_modify(struct nb_cb_modify_args *args) if (!ifp) return NB_OK; if (if_is_loopback(ifp)) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Loopback is always passive"); + snprintf(args->errmsg, args->errmsg_len, + "Loopback is always passive"); return NB_ERR_VALIDATION; } } @@ -2312,7 +2325,8 @@ int lib_interface_isis_disable_three_way_handshake_modify( * /frr-interface:lib/interface/frr-isisd:isis/multi-topology/ipv4-unicast */ static int lib_interface_isis_multi_topology_common( - enum nb_event event, const struct lyd_node *dnode, uint16_t mtid) + enum nb_event event, const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len, uint16_t mtid) { struct isis_circuit *circuit; bool value; @@ -2321,8 +2335,8 @@ static int lib_interface_isis_multi_topology_common( case NB_EV_VALIDATE: circuit = nb_running_get_entry(dnode, NULL, false); if (circuit && circuit->area && circuit->area->oldmetric) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + errmsg, errmsg_len, "Multi topology IS-IS can only be used with wide metrics"); return NB_ERR_VALIDATION; } @@ -2344,7 +2358,8 @@ int lib_interface_isis_multi_topology_ipv4_unicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_UNICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_UNICAST); } /* @@ -2355,7 +2370,8 @@ int lib_interface_isis_multi_topology_ipv4_multicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_MULTICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_MULTICAST); } /* @@ -2366,7 +2382,8 @@ int lib_interface_isis_multi_topology_ipv4_management_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_MGMT); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_MGMT); } /* @@ -2377,7 +2394,8 @@ int lib_interface_isis_multi_topology_ipv6_unicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_UNICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_UNICAST); } /* @@ -2388,7 +2406,8 @@ int lib_interface_isis_multi_topology_ipv6_multicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_MULTICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_MULTICAST); } /* @@ -2399,7 +2418,8 @@ int lib_interface_isis_multi_topology_ipv6_management_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_MGMT); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_MGMT); } /* @@ -2409,5 +2429,6 @@ int lib_interface_isis_multi_topology_ipv6_dstsrc_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_DSTSRC); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_DSTSRC); } diff --git a/lib/if.c b/lib/if.c index b34744f80d..f03574e787 100644 --- a/lib/if.c +++ b/lib/if.c @@ -1567,8 +1567,8 @@ static int lib_interface_destroy(struct nb_cb_destroy_args *args) case NB_EV_VALIDATE: ifp = nb_running_get_entry(args->dnode, NULL, true); if (CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE)) { - zlog_warn("%s: only inactive interfaces can be deleted", - __func__); + snprintf(args->errmsg, args->errmsg_len, + "only inactive interfaces can be deleted"); return NB_ERR_VALIDATION; } break; diff --git a/lib/libfrr.c b/lib/libfrr.c index ac165f254e..cac9929577 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -902,14 +902,18 @@ static int frr_config_read_in(struct thread *t) * reading the configuration file. */ if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL) { + struct nb_context context = {}; + char errmsg[BUFSIZ] = {0}; int ret; - ret = nb_candidate_commit(vty_shared_candidate_config, - NB_CLIENT_CLI, NULL, true, - "Read configuration file", NULL); + context.client = NB_CLIENT_CLI; + ret = nb_candidate_commit(&context, vty_shared_candidate_config, + true, "Read configuration file", NULL, + errmsg, sizeof(errmsg)); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) - zlog_err("%s: failed to read configuration file.", - __func__); + zlog_err( + "%s: failed to read configuration file: %s (%s)", + __func__, nb_err_name(ret), errmsg); } return 0; diff --git a/lib/northbound.c b/lib/northbound.c index e393c33928..48b8499bfc 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -63,19 +63,24 @@ static struct { */ static bool transaction_in_progress; -static int nb_callback_pre_validate(const struct nb_node *nb_node, - const struct lyd_node *dnode); -static int nb_callback_configuration(const enum nb_event event, - struct nb_config_change *change); -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 int nb_callback_pre_validate(struct nb_context *context, + const struct nb_node *nb_node, + const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len); +static int nb_callback_configuration(struct nb_context *context, + const enum nb_event event, + struct nb_config_change *change, + char *errmsg, size_t errmsg_len); +static struct nb_transaction * +nb_transaction_new(struct nb_context *context, struct nb_config *config, + struct nb_config_cbs *changes, const char *comment, + char *errmsg, size_t errmsg_len); static void nb_transaction_free(struct nb_transaction *transaction); static int nb_transaction_process(enum nb_event event, - struct nb_transaction *transaction); -static void nb_transaction_apply_finish(struct nb_transaction *transaction); + struct nb_transaction *transaction, + char *errmsg, size_t errmsg_len); +static void nb_transaction_apply_finish(struct nb_transaction *transaction, + char *errmsg, size_t errmsg_len); static int nb_oper_data_iter_node(const struct lys_node *snode, const char *xpath, const void *list_entry, const struct yang_list_keys *list_keys, @@ -581,20 +586,25 @@ int nb_candidate_update(struct nb_config *candidate) * WARNING: lyd_validate() can change the configuration as part of the * validation process. */ -static int nb_candidate_validate_yang(struct nb_config *candidate) +static int nb_candidate_validate_yang(struct nb_config *candidate, char *errmsg, + size_t errmsg_len) { if (lyd_validate(&candidate->dnode, LYD_OPT_STRICT | LYD_OPT_CONFIG | LYD_OPT_WHENAUTODEL, ly_native_ctx) - != 0) + != 0) { + yang_print_errors(ly_native_ctx, errmsg, errmsg_len); return NB_ERR_VALIDATION; + } return NB_OK; } /* Perform code-level validation using the northbound callbacks. */ -static int nb_candidate_validate_code(struct nb_config *candidate, - struct nb_config_cbs *changes) +static int nb_candidate_validate_code(struct nb_context *context, + struct nb_config *candidate, + struct nb_config_cbs *changes, + char *errmsg, size_t errmsg_len) { struct nb_config_cb *cb; struct lyd_node *root, *next, *child; @@ -609,7 +619,8 @@ static int nb_candidate_validate_code(struct nb_config *candidate, if (!nb_node->cbs.pre_validate) goto next; - ret = nb_callback_pre_validate(nb_node, child); + ret = nb_callback_pre_validate(context, nb_node, child, + errmsg, errmsg_len); if (ret != NB_OK) return NB_ERR_VALIDATION; @@ -622,7 +633,8 @@ static int nb_candidate_validate_code(struct nb_config *candidate, RB_FOREACH (cb, nb_config_cbs, changes) { struct nb_config_change *change = (struct nb_config_change *)cb; - ret = nb_callback_configuration(NB_EV_VALIDATE, change); + ret = nb_callback_configuration(context, NB_EV_VALIDATE, change, + errmsg, errmsg_len); if (ret != NB_OK) return NB_ERR_VALIDATION; } @@ -630,30 +642,36 @@ static int nb_candidate_validate_code(struct nb_config *candidate, return NB_OK; } -int nb_candidate_validate(struct nb_config *candidate) +int nb_candidate_validate(struct nb_context *context, + struct nb_config *candidate, char *errmsg, + size_t errmsg_len) { struct nb_config_cbs changes; int ret; - if (nb_candidate_validate_yang(candidate) != NB_OK) + if (nb_candidate_validate_yang(candidate, errmsg, sizeof(errmsg_len)) + != NB_OK) return NB_ERR_VALIDATION; RB_INIT(nb_config_cbs, &changes); nb_config_diff(running_config, candidate, &changes); - ret = nb_candidate_validate_code(candidate, &changes); + ret = nb_candidate_validate_code(context, candidate, &changes, errmsg, + errmsg_len); nb_config_diff_del_changes(&changes); return ret; } -int nb_candidate_commit_prepare(struct nb_config *candidate, - enum nb_client client, const void *user, +int nb_candidate_commit_prepare(struct nb_context *context, + struct nb_config *candidate, const char *comment, - struct nb_transaction **transaction) + struct nb_transaction **transaction, + char *errmsg, size_t errmsg_len) { struct nb_config_cbs changes; - if (nb_candidate_validate_yang(candidate) != NB_OK) { + if (nb_candidate_validate_yang(candidate, errmsg, errmsg_len) + != NB_OK) { flog_warn(EC_LIB_NB_CANDIDATE_INVALID, "%s: failed to validate candidate configuration", __func__); @@ -665,7 +683,9 @@ int nb_candidate_commit_prepare(struct nb_config *candidate, if (RB_EMPTY(nb_config_cbs, &changes)) return NB_ERR_NO_CHANGES; - if (nb_candidate_validate_code(candidate, &changes) != NB_OK) { + if (nb_candidate_validate_code(context, candidate, &changes, errmsg, + errmsg_len) + != NB_OK) { flog_warn(EC_LIB_NB_CANDIDATE_INVALID, "%s: failed to validate candidate configuration", __func__); @@ -673,29 +693,37 @@ int nb_candidate_commit_prepare(struct nb_config *candidate, return NB_ERR_VALIDATION; } - *transaction = - nb_transaction_new(candidate, &changes, client, user, comment); + *transaction = nb_transaction_new(context, candidate, &changes, comment, + errmsg, errmsg_len); if (*transaction == NULL) { flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED, - "%s: failed to create transaction", __func__); + "%s: failed to create transaction: %s", __func__, + errmsg); nb_config_diff_del_changes(&changes); return NB_ERR_LOCKED; } - return nb_transaction_process(NB_EV_PREPARE, *transaction); + return nb_transaction_process(NB_EV_PREPARE, *transaction, errmsg, + errmsg_len); } void nb_candidate_commit_abort(struct nb_transaction *transaction) { - (void)nb_transaction_process(NB_EV_ABORT, transaction); + char errmsg[BUFSIZ] = {0}; + + (void)nb_transaction_process(NB_EV_ABORT, transaction, errmsg, + sizeof(errmsg)); nb_transaction_free(transaction); } void nb_candidate_commit_apply(struct nb_transaction *transaction, bool save_transaction, uint32_t *transaction_id) { - (void)nb_transaction_process(NB_EV_APPLY, transaction); - nb_transaction_apply_finish(transaction); + char errmsg[BUFSIZ] = {0}; + + (void)nb_transaction_process(NB_EV_APPLY, transaction, errmsg, + sizeof(errmsg)); + nb_transaction_apply_finish(transaction, errmsg, sizeof(errmsg)); /* Replace running by candidate. */ transaction->config->version++; @@ -710,15 +738,16 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction, nb_transaction_free(transaction); } -int nb_candidate_commit(struct nb_config *candidate, enum nb_client client, - const void *user, bool save_transaction, - const char *comment, uint32_t *transaction_id) +int nb_candidate_commit(struct nb_context *context, struct nb_config *candidate, + bool save_transaction, const char *comment, + uint32_t *transaction_id, char *errmsg, + size_t errmsg_len) { struct nb_transaction *transaction = NULL; int ret; - ret = nb_candidate_commit_prepare(candidate, client, user, comment, - &transaction); + ret = nb_candidate_commit_prepare(context, candidate, comment, + &transaction, errmsg, errmsg_len); /* * Apply the changes if the preparation phase succeeded. Otherwise abort * the transaction. @@ -736,7 +765,7 @@ int nb_running_lock(enum nb_client client, const void *user) { int ret = -1; - frr_with_mutex(&running_config_mgmt_lock.mtx) { + frr_with_mutex (&running_config_mgmt_lock.mtx) { if (!running_config_mgmt_lock.locked) { running_config_mgmt_lock.locked = true; running_config_mgmt_lock.owner_client = client; @@ -752,7 +781,7 @@ int nb_running_unlock(enum nb_client client, const void *user) { int ret = -1; - frr_with_mutex(&running_config_mgmt_lock.mtx) { + frr_with_mutex (&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) { @@ -770,7 +799,7 @@ int nb_running_lock_check(enum nb_client client, const void *user) { int ret = -1; - frr_with_mutex(&running_config_mgmt_lock.mtx) { + frr_with_mutex (&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)) @@ -802,78 +831,237 @@ static void nb_log_config_callback(const enum nb_event event, value); } -static int nb_callback_create(const struct nb_node *nb_node, +static int nb_callback_create(struct nb_context *context, + const struct nb_node *nb_node, enum nb_event event, const struct lyd_node *dnode, - union nb_resource *resource) + union nb_resource *resource, char *errmsg, + size_t errmsg_len) { struct nb_cb_create_args args = {}; + bool unexpected_error = false; + int ret; nb_log_config_callback(event, NB_OP_CREATE, dnode); + args.context = context; args.event = event; args.dnode = dnode; args.resource = resource; - return nb_node->cbs.create(&args); + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; + ret = nb_node->cbs.create(&args); + + /* Detect and log unexpected errors. */ + switch (ret) { + case NB_OK: + case NB_ERR: + break; + case NB_ERR_VALIDATION: + if (event != NB_EV_VALIDATE) + unexpected_error = true; + break; + case NB_ERR_RESOURCE: + if (event != NB_EV_PREPARE) + unexpected_error = true; + break; + case NB_ERR_INCONSISTENCY: + if (event == NB_EV_VALIDATE) + unexpected_error = true; + break; + default: + unexpected_error = true; + break; + } + if (unexpected_error) + DEBUGD(&nb_dbg_cbs_config, + "northbound callback: unexpected return value: %s", + nb_err_name(ret)); + + return ret; } -static int nb_callback_modify(const struct nb_node *nb_node, +static int nb_callback_modify(struct nb_context *context, + const struct nb_node *nb_node, enum nb_event event, const struct lyd_node *dnode, - union nb_resource *resource) + union nb_resource *resource, char *errmsg, + size_t errmsg_len) { struct nb_cb_modify_args args = {}; + bool unexpected_error = false; + int ret; nb_log_config_callback(event, NB_OP_MODIFY, dnode); + args.context = context; args.event = event; args.dnode = dnode; args.resource = resource; - return nb_node->cbs.modify(&args); + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; + ret = nb_node->cbs.modify(&args); + + /* Detect and log unexpected errors. */ + switch (ret) { + case NB_OK: + case NB_ERR: + break; + case NB_ERR_VALIDATION: + if (event != NB_EV_VALIDATE) + unexpected_error = true; + break; + case NB_ERR_RESOURCE: + if (event != NB_EV_PREPARE) + unexpected_error = true; + break; + case NB_ERR_INCONSISTENCY: + if (event == NB_EV_VALIDATE) + unexpected_error = true; + break; + default: + unexpected_error = true; + break; + } + if (unexpected_error) + DEBUGD(&nb_dbg_cbs_config, + "northbound callback: unexpected return value: %s", + nb_err_name(ret)); + + return ret; } -static int nb_callback_destroy(const struct nb_node *nb_node, +static int nb_callback_destroy(struct nb_context *context, + const struct nb_node *nb_node, enum nb_event event, - const struct lyd_node *dnode) + const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len) { struct nb_cb_destroy_args args = {}; + bool unexpected_error = false; + int ret; nb_log_config_callback(event, NB_OP_DESTROY, dnode); + args.context = context; args.event = event; args.dnode = dnode; - return nb_node->cbs.destroy(&args); + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; + ret = nb_node->cbs.destroy(&args); + + /* Detect and log unexpected errors. */ + switch (ret) { + case NB_OK: + case NB_ERR: + break; + case NB_ERR_VALIDATION: + if (event != NB_EV_VALIDATE) + unexpected_error = true; + break; + case NB_ERR_INCONSISTENCY: + if (event == NB_EV_VALIDATE) + unexpected_error = true; + break; + default: + unexpected_error = true; + break; + } + if (unexpected_error) + DEBUGD(&nb_dbg_cbs_config, + "northbound callback: unexpected return value: %s", + nb_err_name(ret)); + + return ret; } -static int nb_callback_move(const struct nb_node *nb_node, enum nb_event event, - const struct lyd_node *dnode) +static int nb_callback_move(struct nb_context *context, + const struct nb_node *nb_node, enum nb_event event, + const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len) { struct nb_cb_move_args args = {}; + bool unexpected_error = false; + int ret; nb_log_config_callback(event, NB_OP_MOVE, dnode); + args.context = context; args.event = event; args.dnode = dnode; - return nb_node->cbs.move(&args); + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; + ret = nb_node->cbs.move(&args); + + /* Detect and log unexpected errors. */ + switch (ret) { + case NB_OK: + case NB_ERR: + break; + case NB_ERR_VALIDATION: + if (event != NB_EV_VALIDATE) + unexpected_error = true; + break; + case NB_ERR_INCONSISTENCY: + if (event == NB_EV_VALIDATE) + unexpected_error = true; + break; + default: + unexpected_error = true; + break; + } + if (unexpected_error) + DEBUGD(&nb_dbg_cbs_config, + "northbound callback: unexpected return value: %s", + nb_err_name(ret)); + + return ret; } -static int nb_callback_pre_validate(const struct nb_node *nb_node, - const struct lyd_node *dnode) +static int nb_callback_pre_validate(struct nb_context *context, + const struct nb_node *nb_node, + const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len) { struct nb_cb_pre_validate_args args = {}; + bool unexpected_error = false; + int ret; nb_log_config_callback(NB_EV_VALIDATE, NB_OP_PRE_VALIDATE, dnode); args.dnode = dnode; - return nb_node->cbs.pre_validate(&args); + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; + ret = nb_node->cbs.pre_validate(&args); + + /* Detect and log unexpected errors. */ + switch (ret) { + case NB_OK: + case NB_ERR_VALIDATION: + break; + default: + unexpected_error = true; + break; + } + if (unexpected_error) + DEBUGD(&nb_dbg_cbs_config, + "northbound callback: unexpected return value: %s", + nb_err_name(ret)); + + return ret; } -static void nb_callback_apply_finish(const struct nb_node *nb_node, - const struct lyd_node *dnode) +static void nb_callback_apply_finish(struct nb_context *context, + const struct nb_node *nb_node, + const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len) { struct nb_cb_apply_finish_args args = {}; nb_log_config_callback(NB_EV_APPLY, NB_OP_APPLY_FINISH, dnode); + args.context = context; args.dnode = dnode; + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; nb_node->cbs.apply_finish(&args); } @@ -953,8 +1141,10 @@ int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath, * Call the northbound configuration callback associated to a given * configuration change. */ -static int nb_callback_configuration(const enum nb_event event, - struct nb_config_change *change) +static int nb_callback_configuration(struct nb_context *context, + const enum nb_event event, + struct nb_config_change *change, + char *errmsg, size_t errmsg_len) { enum nb_operation operation = change->cb.operation; char xpath[XPATH_MAXLEN]; @@ -963,7 +1153,6 @@ static int nb_callback_configuration(const enum nb_event event, union nb_resource *resource; int ret = NB_ERR; - if (event == NB_EV_VALIDATE) resource = NULL; else @@ -971,16 +1160,20 @@ static int nb_callback_configuration(const enum nb_event event, switch (operation) { case NB_OP_CREATE: - ret = nb_callback_create(nb_node, event, dnode, resource); + ret = nb_callback_create(context, nb_node, event, dnode, + resource, errmsg, errmsg_len); break; case NB_OP_MODIFY: - ret = nb_callback_modify(nb_node, event, dnode, resource); + ret = nb_callback_modify(context, nb_node, event, dnode, + resource, errmsg, errmsg_len); break; case NB_OP_DESTROY: - ret = nb_callback_destroy(nb_node, event, dnode); + ret = nb_callback_destroy(context, nb_node, event, dnode, + errmsg, errmsg_len); break; case NB_OP_MOVE: - ret = nb_callback_move(nb_node, event, dnode); + ret = nb_callback_move(context, nb_node, event, dnode, errmsg, + errmsg_len); break; default: yang_dnode_get_path(dnode, xpath, sizeof(xpath)); @@ -1015,45 +1208,48 @@ static int nb_callback_configuration(const enum nb_event event, break; default: flog_err(EC_LIB_DEVELOPMENT, - "%s: unknown event (%u) [xpath %s]", - __func__, event, xpath); + "%s: unknown event (%u) [xpath %s]", __func__, + event, xpath); exit(1); } flog(priority, ref, - "%s: error processing configuration change: error [%s] event [%s] operation [%s] xpath [%s]", - __func__, nb_err_name(ret), nb_event_name(event), + "error processing configuration change: error [%s] event [%s] operation [%s] xpath [%s]", + nb_err_name(ret), nb_event_name(event), nb_operation_name(operation), xpath); + if (strlen(errmsg) > 0) + flog(priority, ref, + "error processing configuration change: %s", + errmsg); } return ret; } 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) +nb_transaction_new(struct nb_context *context, struct nb_config *config, + struct nb_config_cbs *changes, const char *comment, + char *errmsg, size_t errmsg_len) { 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__); + if (nb_running_lock_check(context->client, context->user)) { + strlcpy(errmsg, + "running configuration is locked by another client", + errmsg_len); return NULL; } if (transaction_in_progress) { - flog_warn( - EC_LIB_NB_TRANSACTION_CREATION_FAILED, - "%s: error - there's already another transaction in progress", - __func__); + strlcpy(errmsg, + "there's already another transaction in progress", + errmsg_len); return NULL; } transaction_in_progress = true; transaction = XCALLOC(MTYPE_TMP, sizeof(*transaction)); - transaction->client = client; + transaction->context = context; if (comment) strlcpy(transaction->comment, comment, sizeof(transaction->comment)); @@ -1072,7 +1268,8 @@ static void nb_transaction_free(struct nb_transaction *transaction) /* Process all configuration changes associated to a transaction. */ static int nb_transaction_process(enum nb_event event, - struct nb_transaction *transaction) + struct nb_transaction *transaction, + char *errmsg, size_t errmsg_len) { struct nb_config_cb *cb; @@ -1088,7 +1285,8 @@ static int nb_transaction_process(enum nb_event event, break; /* Call the appropriate callback. */ - ret = nb_callback_configuration(event, change); + ret = nb_callback_configuration(transaction->context, event, + change, errmsg, errmsg_len); switch (event) { case NB_EV_PREPARE: if (ret != NB_OK) @@ -1142,7 +1340,8 @@ nb_apply_finish_cb_find(struct nb_config_cbs *cbs, } /* Call the 'apply_finish' callbacks. */ -static void nb_transaction_apply_finish(struct nb_transaction *transaction) +static void nb_transaction_apply_finish(struct nb_transaction *transaction, + char *errmsg, size_t errmsg_len) { struct nb_config_cbs cbs; struct nb_config_cb *cb; @@ -1201,7 +1400,8 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction) /* Call the 'apply_finish' callbacks, sorted by their priorities. */ RB_FOREACH (cb, nb_config_cbs, &cbs) - nb_callback_apply_finish(cb->nb_node, cb->dnode); + nb_callback_apply_finish(transaction->context, cb->nb_node, + cb->dnode, errmsg, errmsg_len); /* Release memory. */ while (!RB_EMPTY(nb_config_cbs, &cbs)) { @@ -1939,7 +2139,7 @@ const char *nb_err_name(enum nb_error error) case NB_ERR_LOCKED: return "resource is locked"; case NB_ERR_VALIDATION: - return "validation error"; + return "validation"; case NB_ERR_RESOURCE: return "failed to allocate resource"; case NB_ERR_INCONSISTENCY: diff --git a/lib/northbound.h b/lib/northbound.h index 2a1beeabaa..bd57013f59 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -91,6 +91,9 @@ union nb_resource { */ struct nb_cb_create_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* * The transaction phase. Refer to the documentation comments of * nb_event for more details. @@ -107,9 +110,18 @@ struct nb_cb_create_args { * resource(s). It's set to NULL when the event is NB_EV_VALIDATE. */ union nb_resource *resource; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_modify_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* * The transaction phase. Refer to the documentation comments of * nb_event for more details. @@ -126,9 +138,18 @@ struct nb_cb_modify_args { * resource(s). It's set to NULL when the event is NB_EV_VALIDATE. */ union nb_resource *resource; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_destroy_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* * The transaction phase. Refer to the documentation comments of * nb_event for more details. @@ -137,9 +158,18 @@ struct nb_cb_destroy_args { /* libyang data node that is being deleted. */ const struct lyd_node *dnode; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_move_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* * The transaction phase. Refer to the documentation comments of * nb_event for more details. @@ -148,16 +178,40 @@ struct nb_cb_move_args { /* libyang data node that is being moved. */ const struct lyd_node *dnode; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_pre_validate_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* libyang data node associated with the 'pre_validate' callback. */ const struct lyd_node *dnode; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_apply_finish_args { + /* Context of the configuration transaction. */ + struct nb_context *context; + /* libyang data node associated with the 'apply_finish' callback. */ const struct lyd_node *dnode; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; struct nb_cb_get_elem_args { @@ -305,6 +359,10 @@ struct nb_callbacks { * args * Refer to the documentation comments of nb_cb_pre_validate_args for * details. + * + * Returns: + * - NB_OK on success. + * - NB_ERR_VALIDATION when a validation error occurred. */ int (*pre_validate)(struct nb_cb_pre_validate_args *args); @@ -538,6 +596,29 @@ enum nb_client { NB_CLIENT_GRPC, }; +/* Northbound context. */ +struct nb_context { + /* Northbound client. */ + enum nb_client client; + + /* Northbound user (can be NULL). */ + const void *user; + + /* Client-specific data. */ +#if 0 + union { + struct { + } cli; + struct { + } confd; + struct { + } sysrepo; + struct { + } grpc; + } client_data; +#endif +}; + /* Northbound configuration. */ struct nb_config { struct lyd_node *dnode; @@ -564,7 +645,7 @@ struct nb_config_change { /* Northbound configuration transaction. */ struct nb_transaction { - enum nb_client client; + struct nb_context *context; char comment[80]; struct nb_config *config; struct nb_config_cbs changes; @@ -762,28 +843,36 @@ extern int nb_candidate_update(struct nb_config *candidate); * WARNING: the candidate can be modified as part of the validation process * (e.g. add default nodes). * + * context + * Context of the northbound transaction. + * * candidate * Candidate configuration to validate. * + * errmsg + * Buffer to store human-readable error message in case of error. + * + * errmsg_len + * Size of errmsg. + * * Returns: * NB_OK on success, NB_ERR_VALIDATION otherwise. */ -extern int nb_candidate_validate(struct nb_config *candidate); +extern int nb_candidate_validate(struct nb_context *context, + struct nb_config *candidate, char *errmsg, + size_t errmsg_len); /* * Create a new configuration transaction but do not commit it yet. Only * validate the candidate and prepare all resources required to apply the * configuration changes. * + * context + * Context of the northbound transaction. + * * candidate * Candidate configuration to commit. * - * client - * Northbound client performing the commit. - * - * user - * Northbound user performing the commit (can be NULL). - * * comment * Optional comment describing the commit. * @@ -793,6 +882,12 @@ extern int nb_candidate_validate(struct nb_config *candidate); * nb_candidate_commit_abort() or committed using * nb_candidate_commit_apply(). * + * errmsg + * Buffer to store human-readable error message in case of error. + * + * errmsg_len + * Size of errmsg. + * * Returns: * - NB_OK on success. * - NB_ERR_NO_CHANGES when the candidate is identical to the running @@ -803,10 +898,11 @@ extern int nb_candidate_validate(struct nb_config *candidate); * the candidate configuration. * - NB_ERR for other errors. */ -extern int nb_candidate_commit_prepare(struct nb_config *candidate, - enum nb_client client, const void *user, +extern int nb_candidate_commit_prepare(struct nb_context *context, + struct nb_config *candidate, const char *comment, - struct nb_transaction **transaction); + struct nb_transaction **transaction, + char *errmsg, size_t errmsg_len); /* * Abort a previously created configuration transaction, releasing all resources @@ -842,16 +938,13 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction, * take into account the results of the preparation phase of multiple managed * entities. * + * context + * Context of the northbound transaction. + * * candidate * Candidate configuration to commit. It's preserved regardless if the commit * operation fails or not. * - * 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. @@ -862,6 +955,12 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction, * transaction_id * Optional output parameter providing the ID of the committed transaction. * + * errmsg + * Buffer to store human-readable error message in case of error. + * + * errmsg_len + * Size of errmsg. + * * Returns: * - NB_OK on success. * - NB_ERR_NO_CHANGES when the candidate is identical to the running @@ -872,10 +971,11 @@ extern void nb_candidate_commit_apply(struct nb_transaction *transaction, * the candidate configuration. * - NB_ERR for other errors. */ -extern int nb_candidate_commit(struct nb_config *candidate, - enum nb_client client, const void *user, +extern int nb_candidate_commit(struct nb_context *context, + struct nb_config *candidate, bool save_transaction, const char *comment, - uint32_t *transaction_id); + uint32_t *transaction_id, char *errmsg, + size_t errmsg_len); /* * Lock the running configuration. @@ -1061,8 +1161,8 @@ extern void *nb_running_unset_entry(const struct lyd_node *dnode); * Returns: * User pointer if found, NULL otherwise. */ -extern void *nb_running_get_entry(const struct lyd_node *dnode, const char *xpath, - bool abort_if_not_found); +extern void *nb_running_get_entry(const struct lyd_node *dnode, + const char *xpath, bool abort_if_not_found); /* * Return a human-readable string representing a northbound event. diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index d4467facaf..105fc83cef 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -46,23 +46,11 @@ struct debug nb_dbg_libyang = {0, "libyang debugging"}; 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) +static void vty_show_nb_errors(struct vty *vty, int error, const char *errmsg) { - struct ly_err_item *ei; - const char *path; - - ei = ly_err_first(ly_ctx); - if (!ei) - return; - - for (; ei; ei = ei->next) - vty_out(vty, "%s\n", ei->msg); - - path = ly_errpath(ly_ctx); - if (path) - vty_out(vty, "YANG path: %s\n", path); - - ly_err_clean(ly_ctx, NULL); + vty_out(vty, "Error type: %s\n", nb_err_name(error)); + if (strlen(errmsg) > 0) + vty_out(vty, "Error description: %s\n", errmsg); } void nb_cli_enqueue_change(struct vty *vty, const char *xpath, @@ -158,24 +146,31 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) } if (error) { + char buf[BUFSIZ]; + /* * Failure to edit the candidate configuration should never * happen in practice, unless there's a bug in the code. When * that happens, log the error but otherwise ignore it. */ vty_out(vty, "%% Failed to edit configuration.\n\n"); - vty_show_libyang_errors(vty, ly_native_ctx); + vty_out(vty, "%s", + yang_print_errors(ly_native_ctx, buf, sizeof(buf))); } /* 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, - vty, false, NULL, NULL); + struct nb_context context = {}; + char errmsg[BUFSIZ] = {0}; + + context.client = NB_CLIENT_CLI; + context.user = vty; + ret = nb_candidate_commit(&context, vty->candidate_config, + false, NULL, NULL, errmsg, + sizeof(errmsg)); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) { - vty_out(vty, "%% Configuration failed: %s.\n\n", - nb_err_name(ret)); - vty_out(vty, - "Please check the logs for more details.\n"); + vty_out(vty, "%% Configuration failed.\n\n"); + vty_show_nb_errors(vty, ret, errmsg); /* Regenerate candidate for consistency. */ nb_config_replace(vty->candidate_config, running_config, @@ -217,20 +212,27 @@ void nb_cli_confirmed_commit_clean(struct vty *vty) int nb_cli_confirmed_commit_rollback(struct vty *vty) { + struct nb_context context = {}; uint32_t transaction_id; + char errmsg[BUFSIZ] = {0}; int ret; /* Perform the rollback. */ + context.client = NB_CLIENT_CLI; + context.user = vty; ret = nb_candidate_commit( - vty->confirmed_commit_rollback, NB_CLIENT_CLI, vty, true, + &context, vty->confirmed_commit_rollback, true, "Rollback to previous configuration - confirmed commit has timed out", - &transaction_id); + &transaction_id, errmsg, sizeof(errmsg)); 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"); + else { + vty_out(vty, + "Failed to rollback to previous configuration.\n\n"); + vty_show_nb_errors(vty, ret, errmsg); + } return ret; } @@ -252,7 +254,9 @@ static int nb_cli_confirmed_commit_timeout(struct thread *thread) static int nb_cli_commit(struct vty *vty, bool force, unsigned int confirmed_timeout, char *comment) { + struct nb_context context = {}; uint32_t transaction_id = 0; + char errmsg[BUFSIZ] = {0}; int ret; /* Check if there's a pending confirmed commit. */ @@ -295,8 +299,11 @@ 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, vty, - true, comment, &transaction_id); + context.client = NB_CLIENT_CLI; + context.user = vty; + ret = nb_candidate_commit(&context, vty->candidate_config, true, + comment, &transaction_id, errmsg, + sizeof(errmsg)); /* Map northbound return code to CLI return code. */ switch (ret) { @@ -312,9 +319,8 @@ static int nb_cli_commit(struct vty *vty, bool force, return CMD_SUCCESS; default: vty_out(vty, - "%% Failed to commit candidate configuration: %s.\n\n", - nb_err_name(ret)); - vty_out(vty, "Please check the logs for more details.\n"); + "%% Failed to commit candidate configuration.\n\n"); + vty_show_nb_errors(vty, ret, errmsg); return CMD_WARNING; } } @@ -328,6 +334,7 @@ static int nb_cli_candidate_load_file(struct vty *vty, struct lyd_node *dnode; struct ly_ctx *ly_ctx; int ly_format; + char buf[BUFSIZ]; switch (format) { case NB_CFG_FMT_CMDS: @@ -350,7 +357,9 @@ static int nb_cli_candidate_load_file(struct vty *vty, flog_warn(EC_LIB_LIBYANG, "%s: lyd_parse_path() failed", __func__); vty_out(vty, "%% Failed to load configuration:\n\n"); - vty_show_libyang_errors(vty, ly_ctx); + vty_out(vty, "%s", + yang_print_errors(ly_native_ctx, buf, + sizeof(buf))); return CMD_WARNING; } if (translator @@ -371,7 +380,8 @@ static int nb_cli_candidate_load_file(struct vty *vty, != NB_OK) { vty_out(vty, "%% Failed to merge the loaded configuration:\n\n"); - vty_show_libyang_errors(vty, ly_native_ctx); + vty_out(vty, "%s", + yang_print_errors(ly_native_ctx, buf, sizeof(buf))); return CMD_WARNING; } @@ -383,6 +393,7 @@ static int nb_cli_candidate_load_transaction(struct vty *vty, bool replace) { struct nb_config *loaded_config; + char buf[BUFSIZ]; loaded_config = nb_db_transaction_load(transaction_id); if (!loaded_config) { @@ -397,7 +408,8 @@ static int nb_cli_candidate_load_transaction(struct vty *vty, != NB_OK) { vty_out(vty, "%% Failed to merge the loaded configuration:\n\n"); - vty_show_libyang_errors(vty, ly_native_ctx); + vty_out(vty, "%s", + yang_print_errors(ly_native_ctx, buf, sizeof(buf))); return CMD_WARNING; } @@ -690,13 +702,18 @@ DEFPY (config_commit_check, "Commit changes into the running configuration\n" "Check if the configuration changes are valid\n") { + struct nb_context context = {}; + char errmsg[BUFSIZ] = {0}; int ret; - ret = nb_candidate_validate(vty->candidate_config); + context.client = NB_CLIENT_CLI; + context.user = vty; + ret = nb_candidate_validate(&context, vty->candidate_config, errmsg, + sizeof(errmsg)); if (ret != NB_OK) { vty_out(vty, "%% Failed to validate candidate configuration.\n\n"); - vty_show_libyang_errors(vty, ly_native_ctx); + vty_show_nb_errors(vty, ret, errmsg); return CMD_WARNING; } @@ -1530,8 +1547,10 @@ DEFPY (show_yang_module_translator, static int nb_cli_rollback_configuration(struct vty *vty, uint32_t transaction_id) { + struct nb_context context = {}; struct nb_config *candidate; char comment[80]; + char errmsg[BUFSIZ] = {0}; int ret; candidate = nb_db_transaction_load(transaction_id); @@ -1544,8 +1563,10 @@ 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, vty, true, comment, - NULL); + context.client = NB_CLIENT_CLI; + context.user = vty; + ret = nb_candidate_commit(&context, candidate, true, comment, NULL, + errmsg, sizeof(errmsg)); nb_config_free(candidate); switch (ret) { case NB_OK: @@ -1558,7 +1579,7 @@ static int nb_cli_rollback_configuration(struct vty *vty, return CMD_WARNING; default: vty_out(vty, "%% Rollback failed.\n\n"); - vty_out(vty, "Please check the logs for more details.\n"); + vty_show_nb_errors(vty, ret, errmsg); return CMD_WARNING; } } diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c index 2fc3c81cf2..a3aaf02f08 100644 --- a/lib/northbound_confd.c +++ b/lib/northbound_confd.c @@ -285,8 +285,10 @@ frr_confd_cdb_diff_iter(confd_hkeypath_t *kp, enum cdb_iter_op cdb_op, static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen) { + struct nb_context context = {}; struct nb_config *candidate; struct cdb_iter_args iter_args; + char errmsg[BUFSIZ] = {0}; int ret; candidate = nb_config_dup(running_config); @@ -321,24 +323,21 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen) * required to apply them. */ transaction = NULL; - ret = nb_candidate_commit_prepare(candidate, NB_CLIENT_CONFD, NULL, - NULL, &transaction); + context.client = NB_CLIENT_CONFD; + ret = nb_candidate_commit_prepare(&context, candidate, NULL, + &transaction, errmsg, sizeof(errmsg)); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) { enum confd_errcode errcode; - const char *errmsg; switch (ret) { case NB_ERR_LOCKED: errcode = CONFD_ERRCODE_IN_USE; - errmsg = "Configuration is locked by another process"; break; case NB_ERR_RESOURCE: errcode = CONFD_ERRCODE_RESOURCE_DENIED; - errmsg = "Failed do allocate resources"; break; default: - errcode = CONFD_ERRCODE_INTERNAL; - errmsg = "Internal error"; + errcode = CONFD_ERRCODE_APPLICATION; break; } @@ -612,7 +611,7 @@ static int frr_confd_data_get_elem(struct confd_trans_ctx *tctx, confd_hkeypath_t *kp) { struct nb_node *nb_node; - char xpath[BUFSIZ]; + char xpath[XPATH_MAXLEN]; struct yang_data *data; confd_value_t v; const void *list_entry = NULL; @@ -650,7 +649,7 @@ static int frr_confd_data_get_next(struct confd_trans_ctx *tctx, confd_hkeypath_t *kp, long next) { struct nb_node *nb_node; - char xpath[BUFSIZ]; + char xpath[XPATH_MAXLEN]; struct yang_data *data; const void *parent_list_entry, *nb_next; confd_value_t v[LIST_MAXKEYS]; @@ -758,8 +757,8 @@ static int frr_confd_data_get_object(struct confd_trans_ctx *tctx, { struct nb_node *nb_node; const struct lys_node *child; - char xpath[BUFSIZ]; - char xpath_child[XPATH_MAXLEN]; + char xpath[XPATH_MAXLEN]; + char xpath_child[XPATH_MAXLEN * 2]; struct list *elements; struct yang_data *data; const void *list_entry; @@ -832,7 +831,7 @@ static int frr_confd_data_get_object(struct confd_trans_ctx *tctx, static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx, confd_hkeypath_t *kp, long next) { - char xpath[BUFSIZ]; + char xpath[XPATH_MAXLEN]; struct nb_node *nb_node; struct list *elements; const void *parent_list_entry; @@ -916,7 +915,7 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx, /* Loop through list child nodes. */ LY_TREE_FOR (nb_node->snode->child, child) { struct nb_node *nb_node_child = child->priv; - char xpath_child[XPATH_MAXLEN]; + char xpath_child[XPATH_MAXLEN * 2]; confd_value_t *v; if (nvalues > CONFD_MAX_CHILD_NODES) @@ -1059,7 +1058,7 @@ static int frr_confd_action_execute(struct confd_user_info *uinfo, struct xml_tag *name, confd_hkeypath_t *kp, confd_tag_value_t *params, int nparams) { - char xpath[BUFSIZ]; + char xpath[XPATH_MAXLEN]; struct nb_node *nb_node; struct list *input; struct list *output; @@ -1091,7 +1090,7 @@ static int frr_confd_action_execute(struct confd_user_info *uinfo, /* Process input nodes. */ for (int i = 0; i < nparams; i++) { - char xpath_input[BUFSIZ]; + char xpath_input[XPATH_MAXLEN * 2]; char value_str[YANG_VALUE_MAXLEN]; snprintf(xpath_input, sizeof(xpath_input), "%s/%s", xpath, diff --git a/lib/northbound_db.c b/lib/northbound_db.c index 598805b961..244e760b2b 100644 --- a/lib/northbound_db.c +++ b/lib/northbound_db.c @@ -86,7 +86,7 @@ int nb_db_transaction_save(const struct nb_transaction *transaction, if (!ss) goto exit; - client_name = nb_client_name(transaction->client); + client_name = nb_client_name(transaction->context->client); /* Always record configurations in the XML format. */ if (lyd_print_mem(&config_str, transaction->config->dnode, LYD_XML, LYP_FORMAT | LYP_WITHSIBLINGS) diff --git a/lib/northbound_grpc.cpp b/lib/northbound_grpc.cpp index 2962a977eb..83d7e0ce95 100644 --- a/lib/northbound_grpc.cpp +++ b/lib/northbound_grpc.cpp @@ -175,14 +175,13 @@ class NorthboundImpl void HandleGetCapabilities(RpcState *tag) { - if (nb_dbg_client_grpc) - zlog_debug("received RPC GetCapabilities()"); - switch (tag->state) { case CREATE: REQUEST_RPC(GetCapabilities); tag->state = PROCESS; case PROCESS: { + if (nb_dbg_client_grpc) + zlog_debug("received RPC GetCapabilities()"); // Response: string frr_version = 1; tag->response.set_frr_version(FRR_VERSION); @@ -298,14 +297,14 @@ class NorthboundImpl void HandleCreateCandidate(RpcState *tag) { - if (nb_dbg_client_grpc) - zlog_debug("received RPC CreateCandidate()"); - switch (tag->state) { case CREATE: REQUEST_RPC(CreateCandidate); tag->state = PROCESS; case PROCESS: { + if (nb_dbg_client_grpc) + zlog_debug("received RPC CreateCandidate()"); + struct candidate *candidate = create_candidate(); if (!candidate) { tag->responder.Finish( @@ -672,15 +671,22 @@ class NorthboundImpl // Execute the user request. + struct nb_context context = {}; + context.client = NB_CLIENT_GRPC; + char errmsg[BUFSIZ] = {0}; + switch (phase) { case frr::CommitRequest::VALIDATE: - ret = nb_candidate_validate(candidate->config); + ret = nb_candidate_validate( + &context, candidate->config, errmsg, + sizeof(errmsg)); break; case frr::CommitRequest::PREPARE: ret = nb_candidate_commit_prepare( - candidate->config, NB_CLIENT_GRPC, NULL, + &context, candidate->config, comment.c_str(), - &candidate->transaction); + &candidate->transaction, errmsg, + sizeof(errmsg)); break; case frr::CommitRequest::ABORT: nb_candidate_commit_abort( @@ -693,71 +699,51 @@ class NorthboundImpl break; case frr::CommitRequest::ALL: ret = nb_candidate_commit( - candidate->config, NB_CLIENT_GRPC, NULL, - true, comment.c_str(), &transaction_id); + &context, candidate->config, true, + comment.c_str(), &transaction_id, + errmsg, sizeof(errmsg)); break; } - // Map northbound error codes to gRPC error codes. + // Map northbound error codes to gRPC status codes. + grpc::Status status; switch (ret) { + case NB_OK: + status = grpc::Status::OK; + break; case NB_ERR_NO_CHANGES: - tag->responder.Finish( - tag->response, - grpc::Status( - grpc::StatusCode::ABORTED, - "No configuration changes detected"), - tag); - tag->state = FINISH; - return; + status = grpc::Status(grpc::StatusCode::ABORTED, + errmsg); + break; case NB_ERR_LOCKED: - tag->responder.Finish( - tag->response, - grpc::Status( - grpc::StatusCode::UNAVAILABLE, - "There's already a transaction in progress"), - tag); - tag->state = FINISH; - return; + status = grpc::Status( + grpc::StatusCode::UNAVAILABLE, errmsg); + break; case NB_ERR_VALIDATION: - tag->responder.Finish( - tag->response, - grpc::Status(grpc::StatusCode:: - INVALID_ARGUMENT, - "Validation error"), - tag); - tag->state = FINISH; - return; + status = grpc::Status( + grpc::StatusCode::INVALID_ARGUMENT, + errmsg); + break; case NB_ERR_RESOURCE: - tag->responder.Finish( - tag->response, - grpc::Status( - grpc::StatusCode:: - RESOURCE_EXHAUSTED, - "Failed do allocate resources"), - tag); - tag->state = FINISH; - return; + status = grpc::Status( + grpc::StatusCode::RESOURCE_EXHAUSTED, + errmsg); + break; case NB_ERR: - tag->responder.Finish( - tag->response, - grpc::Status(grpc::StatusCode::INTERNAL, - "Internal error"), - tag); - tag->state = FINISH; - return; default: + status = grpc::Status( + grpc::StatusCode::INTERNAL, errmsg); break; } + if (ret == NB_OK) { + // Response: uint32 transaction_id = 1; + if (transaction_id) + tag->response.set_transaction_id( + transaction_id); + } - // Response: uint32 transaction_id = 1; - if (transaction_id) - tag->response.set_transaction_id( - transaction_id); - - tag->responder.Finish(tag->response, grpc::Status::OK, - tag); + tag->responder.Finish(tag->response, status, tag); tag->state = FINISH; - break; } case FINISH: @@ -769,9 +755,6 @@ class NorthboundImpl HandleListTransactions(RpcState *tag) { - if (nb_dbg_client_grpc) - zlog_debug("received RPC ListTransactions()"); - switch (tag->state) { case CREATE: REQUEST_RPC_STREAMING(ListTransactions); @@ -781,6 +764,9 @@ class NorthboundImpl tag->context); tag->state = PROCESS; case PROCESS: { + if (nb_dbg_client_grpc) + zlog_debug("received RPC ListTransactions()"); + auto list = static_cast> *>( tag->context); @@ -889,14 +875,14 @@ class NorthboundImpl void HandleLockConfig( RpcState *tag) { - if (nb_dbg_client_grpc) - zlog_debug("received RPC LockConfig()"); - switch (tag->state) { case CREATE: REQUEST_RPC(LockConfig); tag->state = PROCESS; case PROCESS: { + if (nb_dbg_client_grpc) + zlog_debug("received RPC LockConfig()"); + if (nb_running_lock(NB_CLIENT_GRPC, NULL)) { tag->responder.Finish( tag->response, @@ -922,14 +908,14 @@ class NorthboundImpl void HandleUnlockConfig(RpcState *tag) { - if (nb_dbg_client_grpc) - zlog_debug("received RPC UnlockConfig()"); - switch (tag->state) { case CREATE: REQUEST_RPC(UnlockConfig); tag->state = PROCESS; case PROCESS: { + if (nb_dbg_client_grpc) + zlog_debug("received RPC UnlockConfig()"); + if (nb_running_unlock(NB_CLIENT_GRPC, NULL)) { tag->responder.Finish( tag->response, diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index b94c939763..500203173c 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -245,7 +245,9 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session, sr_change_oper_t sr_op; sr_val_t *sr_old_val, *sr_new_val; char xpath[XPATH_MAXLEN]; + struct nb_context context = {}; struct nb_config *candidate; + char errmsg[BUFSIZ] = {0}; snprintf(xpath, sizeof(xpath), "/%s:*", module_name); ret = sr_get_changes_iter(session, xpath, &it); @@ -276,21 +278,33 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session, } transaction = NULL; + context.client = NB_CLIENT_SYSREPO; if (startup_config) { /* * sysrepod sends the entire startup configuration using a * 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, NULL, - true, NULL, NULL); + ret = nb_candidate_commit(&context, candidate, true, NULL, NULL, + errmsg, sizeof(errmsg)); + if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) + flog_warn( + EC_LIB_LIBSYSREPO, + "%s: failed to apply startup configuration: %s (%s)", + __func__, nb_err_name(ret), errmsg); } else { /* * Validate the configuration changes and allocate all resources * required to apply them. */ - ret = nb_candidate_commit_prepare(candidate, NB_CLIENT_SYSREPO, - NULL, NULL, &transaction); + ret = nb_candidate_commit_prepare(&context, candidate, NULL, + &transaction, errmsg, + sizeof(errmsg)); + if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) + flog_warn( + EC_LIB_LIBSYSREPO, + "%s: failed to prepare configuration transaction: %s (%s)", + __func__, nb_err_name(ret), errmsg); } /* Map northbound return code to sysrepo return code. */ diff --git a/lib/vrf.c b/lib/vrf.c index 9df5d19516..fb64589287 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -1082,8 +1082,8 @@ static int lib_vrf_destroy(struct nb_cb_destroy_args *args) case NB_EV_VALIDATE: vrfp = nb_running_get_entry(args->dnode, NULL, true); if (CHECK_FLAG(vrfp->status, VRF_ACTIVE)) { - zlog_debug("%s Only inactive VRFs can be deleted", - __func__); + snprintf(args->errmsg, args->errmsg_len, + "Only inactive VRFs can be deleted"); return NB_ERR_VALIDATION; } break; diff --git a/lib/vty.c b/lib/vty.c index 784f9cf2ac..ffef05e4dc 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2349,12 +2349,18 @@ static void vty_read_file(struct nb_config *config, FILE *confp) * reading the configuration file. */ if (config == NULL) { - ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, - vty, true, "Read configuration file", - NULL); + struct nb_context context = {}; + char errmsg[BUFSIZ] = {0}; + + context.client = NB_CLIENT_CLI; + context.user = vty; + ret = nb_candidate_commit(&context, vty->candidate_config, true, + "Read configuration file", NULL, + errmsg, sizeof(errmsg)); if (ret != NB_OK && ret != NB_ERR_NO_CHANGES) - zlog_err("%s: failed to read configuration file.", - __func__); + zlog_err( + "%s: failed to read configuration file: %s (%s)", + __func__, nb_err_name(ret), errmsg); } vty_close(vty); diff --git a/lib/vty.h b/lib/vty.h index 694aac3944..5d5676199b 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -43,7 +43,7 @@ extern "C" { #define VTY_MAXHIST 20 #define VTY_MAXDEPTH 8 -#define VTY_MAXCFGCHANGES 8 +#define VTY_MAXCFGCHANGES 16 struct vty_error { char error_buf[VTY_BUFSIZ]; diff --git a/lib/yang.c b/lib/yang.c index 7ac39f4182..0714ddf7bb 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -647,6 +647,34 @@ static void ly_log_cb(LY_LOG_LEVEL level, const char *msg, const char *path) zlog(priority, "libyang: %s", msg); } +const char *yang_print_errors(struct ly_ctx *ly_ctx, char *buf, size_t buf_len) +{ + struct ly_err_item *ei; + const char *path; + + ei = ly_err_first(ly_ctx); + if (!ei) + return ""; + + strlcpy(buf, "YANG error(s):\n", buf_len); + for (; ei; ei = ei->next) { + strlcat(buf, " ", buf_len); + strlcat(buf, ei->msg, buf_len); + strlcat(buf, "\n", buf_len); + } + + path = ly_errpath(ly_ctx); + if (path) { + strlcat(buf, " YANG path: ", buf_len); + strlcat(buf, path, buf_len); + strlcat(buf, "\n", buf_len); + } + + ly_err_clean(ly_ctx, NULL); + + return buf; +} + void yang_debugging_set(bool enable) { if (enable) { diff --git a/lib/yang.h b/lib/yang.h index 3be0fe5383..85ef0d758c 100644 --- a/lib/yang.h +++ b/lib/yang.h @@ -518,6 +518,24 @@ extern struct ly_ctx *yang_ctx_new_setup(bool embedded_modules); */ extern void yang_debugging_set(bool enable); +/* + * Print libyang error messages into the provided buffer. + * + * ly_ctx + * libyang context to operate on. + * + * buf + * Buffer to store the libyang error messages. + * + * buf_len + * Size of buf. + * + * Returns: + * The provided buffer. + */ +extern const char *yang_print_errors(struct ly_ctx *ly_ctx, char *buf, + size_t buf_len); + /* * Initialize the YANG subsystem. Should be called only once during the * daemon initialization process. diff --git a/zebra/zebra_nb_config.c b/zebra/zebra_nb_config.c index 5b87a18a06..948ef51320 100644 --- a/zebra/zebra_nb_config.c +++ b/zebra/zebra_nb_config.c @@ -941,13 +941,15 @@ int lib_interface_zebra_ip_addrs_create(struct nb_cb_create_args *args) case NB_EV_VALIDATE: if (prefix.family == AF_INET && ipv4_martian(&prefix.u.prefix4)) { - zlog_debug("invalid address %s", - prefix2str(&prefix, buf, sizeof(buf))); + snprintf(args->errmsg, args->errmsg_len, + "invalid address %s", + prefix2str(&prefix, buf, sizeof(buf))); return NB_ERR_VALIDATION; } else if (prefix.family == AF_INET6 && ipv6_martian(&prefix.u.prefix6)) { - zlog_debug("invalid address %s", - prefix2str(&prefix, buf, sizeof(buf))); + snprintf(args->errmsg, args->errmsg_len, + "invalid address %s", + prefix2str(&prefix, buf, sizeof(buf))); return NB_ERR_VALIDATION; } break; @@ -982,16 +984,18 @@ int lib_interface_zebra_ip_addrs_destroy(struct nb_cb_destroy_args *args) /* Check current interface address. */ ifc = connected_check_ptp(ifp, &prefix, NULL); if (!ifc) { - zlog_debug("interface %s Can't find address\n", - ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface %s Can't find address\n", + ifp->name); return NB_ERR_VALIDATION; } } else if (prefix.family == AF_INET6) { /* Check current interface address. */ ifc = connected_check(ifp, &prefix); if (!ifc) { - zlog_debug("interface can't find address %s", - ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface can't find address %s", + ifp->name); return NB_ERR_VALIDATION; } } else @@ -999,7 +1003,8 @@ int lib_interface_zebra_ip_addrs_destroy(struct nb_cb_destroy_args *args) /* This is not configured address. */ if (!CHECK_FLAG(ifc->conf, ZEBRA_IFC_CONFIGURED)) { - zlog_debug("interface %s not configured", ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface %s not configured", ifp->name); return NB_ERR_VALIDATION; } @@ -1244,8 +1249,8 @@ int lib_vrf_zebra_ribs_rib_create(struct nb_cb_create_args *args) switch (args->event) { case NB_EV_VALIDATE: if (!zrt) { - zlog_debug("%s: vrf %s table is not found.", __func__, - vrf->name); + snprintf(args->errmsg, args->errmsg_len, + "vrf %s table is not found.", vrf->name); return NB_ERR_VALIDATION; } break; @@ -1376,7 +1381,8 @@ int lib_route_map_entry_match_condition_source_protocol_modify( case NB_EV_VALIDATE: type = yang_dnode_get_string(args->dnode, NULL); if (proto_name2num(type) == -1) { - zlog_warn("%s: invalid protocol: %s", __func__, type); + snprintf(args->errmsg, args->errmsg_len, + "invalid protocol: %s", type); return NB_ERR_VALIDATION; } return NB_OK; @@ -1470,8 +1476,9 @@ int lib_route_map_entry_set_action_source_v4_modify( memset(&p, 0, sizeof(p)); yang_dnode_get_ipv4p(&p, args->dnode, NULL); if (zebra_check_addr(&p) == 0) { - zlog_warn("%s: invalid IPv4 address: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "invalid IPv4 address: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } @@ -1482,8 +1489,9 @@ int lib_route_map_entry_set_action_source_v4_modify( break; } if (pif == NULL) { - zlog_warn("%s: is not a local adddress: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "is not a local adddress: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } return NB_OK; @@ -1536,8 +1544,9 @@ int lib_route_map_entry_set_action_source_v6_modify( memset(&p, 0, sizeof(p)); yang_dnode_get_ipv6p(&p, args->dnode, NULL); if (zebra_check_addr(&p) == 0) { - zlog_warn("%s: invalid IPv6 address: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "invalid IPv6 address: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } @@ -1548,8 +1557,9 @@ int lib_route_map_entry_set_action_source_v6_modify( break; } if (pif == NULL) { - zlog_warn("%s: is not a local adddress: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "is not a local adddress: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } return NB_OK;