From 5ffa10aac1cb9e58d8113f43e2a105b4494a0c31 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Mon, 6 Jan 2025 08:07:23 -0500 Subject: [PATCH 1/3] mgmtd: improve debug statement Signed-off-by: Christian Hopps --- mgmtd/mgmt_fe_adapter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index 7f7a5d9a8e91..22656f189466 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -1237,8 +1237,8 @@ static void fe_adapter_handle_session_req(struct mgmt_fe_client_adapter *adapter struct mgmt_fe_session_ctx *session; uint64_t client_id; - __dbg("Got session-req creating: %u for refer-id %" PRIu64 " from '%s'", - msg->refer_id == 0, msg->refer_id, adapter->name); + __dbg("Got session-req is create %u req-id %Lu for refer-id %Lu from '%s'", + msg->refer_id == 0, msg->req_id, msg->refer_id, adapter->name); if (msg->refer_id) { uint64_t session_id = msg->refer_id; From 9aa2b0487445c7127cac215b9b2dcb32a5c7692e Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 5 Jan 2025 00:29:05 -0500 Subject: [PATCH 2/3] lib: change and improve walk finish callback function API Signed-off-by: Christian Hopps --- lib/mgmt_be_client.c | 22 ++++++++++++---------- lib/northbound.h | 15 +++++++++------ lib/northbound_oper.c | 33 ++++++++++++++++----------------- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c index f03006ad0e44..031e0772b284 100644 --- a/lib/mgmt_be_client.c +++ b/lib/mgmt_be_client.c @@ -854,8 +854,15 @@ static enum nb_error be_client_send_tree_data_batch(const struct lyd_node *tree, more = true; ret = NB_OK; } - if (ret != NB_OK) + if (ret != NB_OK) { + if (be_client_send_error(client, args->txn_id, args->req_id, false, -EINVAL, + "BE client %s txn-id %Lu error fetching oper state %d", + client->name, args->txn_id, ret)) + ret = NB_ERR; + else + ret = NB_OK; goto done; + } tree_msg = mgmt_msg_native_alloc_msg(struct mgmt_msg_tree_data, 0, MTYPE_MSG_NATIVE_TREE_DATA); @@ -870,20 +877,15 @@ static enum nb_error be_client_send_tree_data_batch(const struct lyd_node *tree, (LYD_PRINT_SHRINK | LYD_PRINT_WD_EXPLICIT | LYD_PRINT_WITHSIBLINGS)); if (err) { - ret = NB_ERR; - goto done; + mgmt_msg_native_free_msg(tree_msg); + /* We will be called again to send the error */ + return NB_ERR; } (void)be_client_send_native_msg(client, tree_msg, mgmt_msg_native_get_msg_len(tree_msg), false); -done: mgmt_msg_native_free_msg(tree_msg); - if (ret) - be_client_send_error(client, args->txn_id, args->req_id, false, - -EINVAL, - "BE client %s txn-id %" PRIu64 - " error fetching oper state %d", - client->name, args->txn_id, ret); +done: if (ret != NB_OK || !more) XFREE(MTYPE_MGMTD_BE_GT_CB_ARGS, args); return ret; diff --git a/lib/northbound.h b/lib/northbound.h index 97a1d31e5792..268bf64ec10b 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -777,16 +777,19 @@ typedef int (*nb_oper_data_cb)(const struct lysc_node *snode, * error. * * If nb_op_iterate_yielding() was passed with @should_batch set then this - * callback will be invoked during each portion (batch) of the walk. + * callback will be invoked during each portion (batch) of the walk with @ret + * set to NB_YIELD. * * The @tree is read-only and should not be modified or freed. * - * If this function returns anything but NB_OK then the walk will be terminated. - * and this function will not be called again regardless of if @ret was - * `NB_YIELD` or not. + * When @ret is NB_YIELD and this function returns anything but NB_OK then the + * walk will be terminated, and this function *will* be called again with @ret + * set the non-NB_OK return value it just returned. This allows the callback + * have a single bit of code to send an error message and do any cleanup for any + * type of failure, whether that failure was from itself or from the infra code. * - * Return: NB_OK to continue or complete the walk normally, otherwise an error - * to immediately terminate the walk. + * Return: NB_OK or an error during handling of @ret == NB_YIELD otherwise the + * value is ignored. */ /* Callback function used by nb_oper_data_iter_yielding(). */ typedef enum nb_error (*nb_oper_data_finish_cb)(const struct lyd_node *tree, diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c index a3ff3607800e..01af38965df8 100644 --- a/lib/northbound_oper.c +++ b/lib/northbound_oper.c @@ -1483,17 +1483,13 @@ static void nb_op_walk_continue(struct event *thread) ret = __walk(ys, true); if (ret == NB_YIELD) { - if (nb_op_yield(ys) != NB_OK) { - if (ys->should_batch) - goto stopped; - else - goto finish; - } - return; + ret = nb_op_yield(ys); + if (ret == NB_OK) + return; } finish: + assert(ret != NB_YIELD); (*ys->finish)(ys_root_node(ys), ys->finish_arg, ret); -stopped: nb_op_free_yield_state(ys, false); } @@ -1552,6 +1548,13 @@ static void nb_op_trim_yield_state(struct nb_op_yield_state *ys) (int)darr_lasti(ys->node_infos)); } +/** + * nb_op_yield() - Yield during the walk. + * @ys: the yield state tracking the walk. + * + * Return: Any error from the `ys->finish` callback which should terminate the + * walk. Otherwise if `ys->should_batch` == false always returns NB_OK. + */ static enum nb_error nb_op_yield(struct nb_op_yield_state *ys) { enum nb_error ret; @@ -1764,17 +1767,13 @@ void *nb_oper_walk(const char *xpath, struct yang_translator *translator, ret = nb_op_walk_start(ys); if (ret == NB_YIELD) { - if (nb_op_yield(ys) != NB_OK) { - if (ys->should_batch) - goto stopped; - else - goto finish; - } - return ys; + ret = nb_op_yield(ys); + if (ret == NB_OK) + return ys; } -finish: + + assert(ret != NB_YIELD); (void)(*ys->finish)(ys_root_node(ys), ys->finish_arg, ret); -stopped: nb_op_free_yield_state(ys, false); return NULL; } From bdfb6a3db7f00859a7cfe3433dc01a497c5d58b2 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Tue, 7 Jan 2025 01:34:13 -0500 Subject: [PATCH 3/3] tools: we specifically added %Lu to our sprintfrr so allow it Signed-off-by: Christian Hopps --- tools/checkpatch.pl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/checkpatch.pl b/tools/checkpatch.pl index 2c773f7fbcb3..9c5eb323be40 100755 --- a/tools/checkpatch.pl +++ b/tools/checkpatch.pl @@ -6285,13 +6285,14 @@ sub process { while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { my $string = substr($rawline, $-[1], $+[1] - $-[1]); $string =~ s/%%/__/g; - # check for %L - if ($show_L && $string =~ /%[\*\d\.\$]*L([diouxX])/) { - WARN("PRINTF_L", - "\%L$1 is non-standard C, use %ll$1\n" . $herecurr); - $show_L = 0; - } - # check for %Z + # check for %L + # OK in FRR + # if ($show_L && $string =~ /%[\*\d\.\$]*L([diouxX])/) { + # WARN("PRINTF_L", + # "\%L$1 is non-standard C, use %ll$1\n" . $herecurr); + # $show_L = 0; + # } + # check for %Z if ($show_Z && $string =~ /%[\*\d\.\$]*Z([diouxX])/) { WARN("PRINTF_Z", "%Z$1 is non-standard C, use %z$1\n" . $herecurr);