From aeff7541107ef84dadf11540698fd328a2dbb7d7 Mon Sep 17 00:00:00 2001 From: Kristofer Hallin Date: Fri, 15 Oct 2021 13:46:38 +0200 Subject: [PATCH 01/13] Added tests for OpenConfig compress. --- test/test_cli_auto_genmodel.sh | 59 ++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/test/test_cli_auto_genmodel.sh b/test/test_cli_auto_genmodel.sh index 9f77fe47..d883b6b6 100755 --- a/test/test_cli_auto_genmodel.sh +++ b/test/test_cli_auto_genmodel.sh @@ -1,6 +1,7 @@ + #!/usr/bin/env bash # Tests for using the auto cli. -# In particular setting a config, displaying as cli commands and reconfigure it +# In particular setting a config, displaying as cli commands and reconfigure it # Tests: # Make a config in CLI. Show output as CLI, save it and ensure it is the same # Try the different GENMODEL settings @@ -25,11 +26,20 @@ fi # Use yang in example +if [ ! -d "$OPENCONFIG" ]; then +# err "Hmm Openconfig dir does not seem to exist, try git clone https://github.com/openconfig/public?" + echo "...skipped: OPENCONFIG not set" + if [ "$s" = $0 ]; then exit 0; else return 0; fi +fi + +OCDIR=$OPENCONFIG/release/models + cat < $cfg $cfg /usr/local/share/clixon $dir + $OCDIR/ $fyang /usr/local/lib/$APPNAME/backend $clidir @@ -49,6 +59,7 @@ cat < $fyang module $APPNAME { namespace "urn:example:clixon"; prefix ex; + import openconfig-extensions { prefix oc-ext; } container table{ list parameter{ key name; @@ -69,6 +80,42 @@ module $APPNAME { } } } + container interfaces { + oc-ext:operational; + list interface { + key name; + leaf name { + type string; + } + container config { + leaf enabled { + type boolean; + default false; + description "Whether the interface is enabled or not."; + } + } + container state { + config false; + leaf oper-status { + type enumeration { + enum UP { + value 1; + description "Ready to pass packets."; + } + enum DOWN { + value 2; + description "The interface does not pass any packets."; + } + } + } + } + leaf enabled { + type boolean; + default false; + description "Whether the interface is enabled or not."; + } + } + } } EOF @@ -127,6 +174,9 @@ function testrun() elif [ $mode = HIDE ]; then table= name= + elif [ $mode = OC_COMPRESS ]; then + table= + name= else table=" table" name= @@ -159,7 +209,7 @@ SAVED=$($clixon_cli -1 -o CLICON_CLI_GENMODEL_TYPE=$mode -f $cfg show config) new "delete a x" expectpart "$($clixon_cli -1 -o CLICON_CLI_GENMODEL_TYPE=$mode -f $cfg delete$table parameter$name a value x)" 0 "" - new "show match a & b xml" + new "show match a & b xml" expectpart "$($clixon_cli -1 -o CLICON_CLI_GENMODEL_TYPE=$mode -f $cfg show xml)" 0 "" "" "a" "" "" "b" "z" "" "
" --not-- "x" new "delete a" @@ -190,6 +240,9 @@ testrun HIDE new "keywords=ALL" testrun ALL +new "keywords=OC_COMPRESS" +testrun OC_COMPRESS + new "keywords=VARS" testrun VARS @@ -201,7 +254,7 @@ new "commit" expectpart "$($clixon_cli -1 -f $cfg commit)" 0 "" new "show state" -expectpart "$($clixon_cli -1 -f $cfg show state)" 0 "exstate sender x" "table parameter a" "table parameter a value x" +expectpart "$($clixon_cli -1 -f $cfg show state)" 0 "exstate sender x" "table parameter a" "table parameter a value x" new "show state exstate" expectpart "$($clixon_cli -1 -f $cfg show state exstate)" 0 "state sender x" --not-- "table parameter a" "table parameter a value x" From 2f56357adb95f086735ebac80886e59fbb26ce9e Mon Sep 17 00:00:00 2001 From: Kristofer Hallin Date: Fri, 15 Oct 2021 13:54:44 +0200 Subject: [PATCH 02/13] Added enum OC_COMPRESS to Clixon yang. --- yang/clixon/clixon-config@2021-07-11.yang | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/yang/clixon/clixon-config@2021-07-11.yang b/yang/clixon/clixon-config@2021-07-11.yang index 50bad2cc..1364455a 100644 --- a/yang/clixon/clixon-config@2021-07-11.yang +++ b/yang/clixon/clixon-config@2021-07-11.yang @@ -1,3 +1,4 @@ + module clixon-config { yang-version 1.1; namespace "http://clicon.org/config"; @@ -247,6 +248,9 @@ module clixon-config { enum HIDE{ description "Keywords on non-key variables and hide container around lists: a y "; } + enum OC_COMPRESS{ + description "See: https://github.com/openconfig/ygot/blob/master/docs/design.md#openconfig-path-compression"; + } } } typedef nacm_mode{ From 4ac1f0bad0c4725bf9fe07056150f79e853fe193 Mon Sep 17 00:00:00 2001 From: Kristofer Hallin Date: Fri, 15 Oct 2021 13:55:31 +0200 Subject: [PATCH 03/13] Implementation of OpenConfig path compression. --- apps/cli/cli_auto.c | 2 +- apps/cli/cli_generate.c | 19 +++++++++++++++---- lib/clixon/clixon_options.h | 1 + lib/src/clixon_options.c | 1 + lib/src/clixon_xml_map.c | 2 +- lib/src/clixon_yang.c | 2 +- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/cli/cli_auto.c b/apps/cli/cli_auto.c index 7b0cce6b..79fea435 100644 --- a/apps/cli/cli_auto.c +++ b/apps/cli/cli_auto.c @@ -353,7 +353,7 @@ cli_xml2cli(cxobj *xn, yang_keyword_get(ys) == Y_LEAF_LIST){ if (prepend) (*fn)(stdout, "%s", prepend); - if (gt == GT_ALL || gt == GT_VARS || gt == GT_HIDE) + if (gt == GT_ALL || gt == GT_VARS || gt == GT_HIDE || gt == GT_OC_COMPRESS) (*fn)(stdout, "%s ", xml_name(xn)); if ((body = xml_body(xn)) != NULL){ if (index(body, ' ')) diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index a2cd6cfb..bf0827a9 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -763,7 +763,7 @@ yang2cli_leaf(clicon_handle h, /* Look for autocli-op defined in clixon-lib.yang */ if (yang_extension_value(ys, "autocli-op", CLIXON_LIB_NS, &opext) < 0) goto done; - if (gt == GT_VARS|| gt == GT_ALL || gt == GT_HIDE){ + if (gt == GT_VARS || gt == GT_ALL || gt == GT_HIDE || gt == GT_OC_COMPRESS){ cprintf(cb, "%s", yang_argument_get(ys)); yang2cli_helptext(cb, helptext); cprintf(cb, " "); @@ -832,13 +832,24 @@ yang2cli_container(clicon_handle h, char *helptext = NULL; char *s; int hide = 0; + int hide_oc = 0; char *opext = NULL; + yang_stmt *ymod = NULL; + char **name; + + if (ys_real_module(ys, &ymod) < 0) + goto done; + if (yang_extension_value(ymod, "openconfig-extensions", "http://openconfig.net/yang/openconfig-ext", &opext) == 0) { + if (strcmp(yang_argument_get(ys), "config") == 0){ + hide_oc = 1; + } + } /* If non-presence container && HIDE mode && only child is * a list, then skip container keyword * See also xml2cli */ - if ((hide = yang_container_cli_hide(ys, gt)) == 0){ + if ((hide = yang_container_cli_hide(ys, gt)) == 0 && hide_oc == 0){ cprintf(cb, "%*s%s", level*3, "", yang_argument_get(ys)); if ((yd = yang_find(ys, Y_DESCRIPTION, NULL)) != NULL){ if ((helptext = strdup(yang_argument_get(yd))) == NULL){ @@ -868,7 +879,7 @@ yang2cli_container(clicon_handle h, while ((yc = yn_each(ys, yc)) != NULL) if (yang2cli_stmt(h, yc, gt, level+1, state, show_tree, cb) < 0) goto done; - if (hide == 0) + if (hide == 0 && hide_oc == 0) cprintf(cb, "%*s}\n", level*3, ""); retval = 0; done: @@ -955,7 +966,7 @@ yang2cli_list(clicon_handle h, cprintf(cb, "{\n"); } if (yang2cli_leaf(h, yleaf, - (gt==GT_VARS||gt==GT_HIDE)?GT_NONE:gt, level+1, + (gt==GT_VARS||gt==GT_HIDE||gt==GT_OC_COMPRESS)?GT_NONE:gt, level+1, last_key, show_tree, 1, cb) < 0) goto done; diff --git a/lib/clixon/clixon_options.h b/lib/clixon/clixon_options.h index 72073f09..ddec1eb8 100644 --- a/lib/clixon/clixon_options.h +++ b/lib/clixon/clixon_options.h @@ -77,6 +77,7 @@ enum genmodel_type{ GT_VARS, /* Keywords on non-key variables */ GT_ALL, /* Keywords on all variables */ GT_HIDE, /* Keywords on all variables and hide container around lists */ + GT_OC_COMPRESS, /* OpenConfig */ }; typedef enum genmodel_type genmodel_type; diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index aa2bcc3b..886781a8 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -89,6 +89,7 @@ static const map_str2int cli_genmodel_map[] = { {"VARS", GT_VARS}, {"ALL", GT_ALL}, {"HIDE", GT_HIDE}, + {"OC_COMPRESS", GT_OC_COMPRESS}, {NULL, -1} }; diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index aaf2c233..9abbf774 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -286,7 +286,7 @@ xml2cli_recurse(FILE *f, /* If presence container, then print as leaf (but continue to children) */ if (prepend) (*fn)(f, "%s", prepend); - if (gt == GT_ALL || gt == GT_VARS || gt == GT_HIDE) + if (gt == GT_ALL || gt == GT_VARS || gt == GT_HIDE || gt == GT_OC_COMPRESS) (*fn)(f, "%s ", xml_name(x)); if ((body = xml_body(x)) != NULL){ if (index(body, ' ')) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 7a588f26..72dfdc1b 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -3255,7 +3255,7 @@ yang_container_cli_hide(yang_stmt *ys, keyw = yang_keyword_get(ys); /* HIDE mode */ - if (gt != GT_HIDE) + if (gt != GT_HIDE && gt != GT_OC_COMPRESS) return 0; /* A container */ if (yang_keyword_get(ys) != Y_CONTAINER) From b29387ce1d9ef3b4a093c0ce013a083487fbeae2 Mon Sep 17 00:00:00 2001 From: Kristofer Hallin Date: Thu, 21 Oct 2021 22:01:04 +0200 Subject: [PATCH 04/13] Changes as per review. --- apps/cli/cli_generate.c | 7 +++++-- test/test_cli_auto_genmodel.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index bf0827a9..10de13e2 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -833,13 +833,16 @@ yang2cli_container(clicon_handle h, char *s; int hide = 0; int hide_oc = 0; + int exist = 0; char *opext = NULL; yang_stmt *ymod = NULL; - char **name; + if (ys_real_module(ys, &ymod) < 0) goto done; - if (yang_extension_value(ymod, "openconfig-extensions", "http://openconfig.net/yang/openconfig-ext", &opext) == 0) { + if (yang_extension_value(ymod, "openconfig-version", "http://openconfig.net/yang/openconfig-ext", &exist, NULL) < 0) + goto done; + if (exist) { if (strcmp(yang_argument_get(ys), "config") == 0){ hide_oc = 1; } diff --git a/test/test_cli_auto_genmodel.sh b/test/test_cli_auto_genmodel.sh index d883b6b6..7af005a1 100755 --- a/test/test_cli_auto_genmodel.sh +++ b/test/test_cli_auto_genmodel.sh @@ -81,7 +81,7 @@ module $APPNAME { } } container interfaces { - oc-ext:operational; + oc-ext:openconfig-version; list interface { key name; leaf name { From c1bcef37a72bbbeffcf56ad3ce68acc9842c484c Mon Sep 17 00:00:00 2001 From: Damian E <“d.ellwart@falconvsystems.com”> Date: Tue, 26 Oct 2021 22:13:04 +0200 Subject: [PATCH 05/13] Append prefix before cloning --- lib/src/clixon_json_parse.y | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/src/clixon_json_parse.y b/lib/src/clixon_json_parse.y index 019942c4..947cf538 100644 --- a/lib/src/clixon_json_parse.y +++ b/lib/src/clixon_json_parse.y @@ -223,12 +223,27 @@ json_current_clone(clixon_json_yacc *jy) clicon_debug(2, "%s", __FUNCTION__); if (jy->jy_current == NULL){ - return -1; + return -1; } xn = jy->jy_current; json_current_pop(jy); - if (jy->jy_current) - json_current_new(jy, xml_name(xn)); + + if (jy->jy_current) { + char* name = xml_name(xn); + char* prefix = xml_prefix(xn); + char* maybe_prefixed_name = NULL; + + if (prefix) { + char* name_parts[] = {prefix, name}; + maybe_prefixed_name = clicon_strjoin(2, name_parts, ":"); + } else { + maybe_prefixed_name = strdup(name); + } + json_current_new(jy, maybe_prefixed_name); + + if (maybe_prefixed_name) + free(maybe_prefixed_name); + } return 0; } From eea6d549f67f2e308f3b1b979dab405876f4143f Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 28 Oct 2021 14:49:33 +0200 Subject: [PATCH 06/13] List pagination locking for state-data changes Changed semantics of locking: instead of relying of locking running-db, an automatic lock bound to a session is maintained. When the session closes the lock is released. --- apps/backend/backend_get.c | 13 +++++++++---- apps/backend/clixon_backend_plugin.h | 5 +++++ apps/backend/clixon_backend_transaction.c | 17 +++++++++-------- apps/cli/cli_show.c | 6 ------ example/main/example_backend.c | 3 ++- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c index 1d814516..54042985 100644 --- a/apps/backend/backend_get.c +++ b/apps/backend/backend_get.c @@ -592,11 +592,16 @@ get_list_pagination(clicon_handle h, #endif } else {/* Check if running locked (by this session) */ - if ((iddb = xmldb_islocked(h, "running")) != 0 && - iddb == ce->ce_id) + if ((iddb = xmldb_islocked(h, PAGINATE_STATE_LOCK)) != 0){ + if (iddb == ce->ce_id) + locked = 1; + else + locked = 0; + } + else { + xmldb_lock(h, PAGINATE_STATE_LOCK, ce->ce_id); locked = 1; - else - locked = 0; + } if ((ret = clixon_pagination_cb_call(h, xpath, locked, offset, limit, xret)) < 0) diff --git a/apps/backend/clixon_backend_plugin.h b/apps/backend/clixon_backend_plugin.h index f88bb41f..c6ab9177 100644 --- a/apps/backend/clixon_backend_plugin.h +++ b/apps/backend/clixon_backend_plugin.h @@ -38,6 +38,11 @@ #ifndef _CLIXON_BACKEND_PLUGIN_H_ #define _CLIXON_BACKEND_PLUGIN_H_ +/* + * Constants + */ +#define PAGINATE_STATE_LOCK "paginate-state-lock" + /* * Types */ diff --git a/apps/backend/clixon_backend_transaction.c b/apps/backend/clixon_backend_transaction.c index 0a7889ce..7c718a63 100644 --- a/apps/backend/clixon_backend_transaction.c +++ b/apps/backend/clixon_backend_transaction.c @@ -309,15 +309,16 @@ pagination_limit(pagination_data pd) return ((pagination_data_t *)pd)->pd_limit; } -/*! Get pagination data: locked parameter +/*! Get pagination data: locked parameter for state data * - * Pagination can use a lock/transaction mechanism - * If locking is not used, the plugin cannot expect more pagination calls, and no state or - * caching should be used - * If locking is used, the pagination is part of a session transaction and the plugin may cache - * state (such as a cache) and can expect more pagination calls until the running db-lock is - * released, (see ca_lockdb) - * The transaction is the regular lock/unlock db of running-db of a specific session. + * A "paginate" lock is set for a specific session when list-pagination is in progress. + * If locking is set, the pagination is part of a session transaction and the plugin may + * cache state (such as a cache) and can expect more pagination calls until the "paginate" + * db-lock is released. + * + * If you want to do the same for config data, it is recommended to use lock-db on + * the appropriate database, but this risks of indefinite blocking, as long as the + * session endures. * @param[in] pd Pagination userdata * @retval locked 0: unlocked/stateless 1: locked by this caller */ diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index 07ba04ec..6f7f26a0 100644 --- a/apps/cli/cli_show.c +++ b/apps/cli/cli_show.c @@ -919,7 +919,6 @@ cli_pagination(clicon_handle h, uint32_t limit = 0; cxobj **xvec = NULL; size_t xlen; - int locked = 0; if (cvec_len(argv) != 5){ clicon_err(OE_PLUGIN, 0, "Expected usage: "); @@ -949,9 +948,6 @@ cli_pagination(clicon_handle h, } if ((nsc = xml_nsctx_init(prefix, namespace)) == NULL) goto done; - if (clicon_rpc_lock(h, "running") < 0) - goto done; - locked++; for (i = 0;; i++){ if (clicon_rpc_get_pageable_list(h, "running", xpath, nsc, CONTENT_ALL, @@ -1004,8 +1000,6 @@ cli_pagination(clicon_handle h, } /* for i */ retval = 0; done: - if (locked) - clicon_rpc_unlock(h, "running"); if (xvec) free(xvec); if (xret) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 947d9d70..6d5655d2 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -545,6 +545,7 @@ example_statefile(clicon_handle h, * @param[in] xpath Registered XPath using canonical prefixes * @param[in] userargs Per-call user arguments * @param[in] arg Per-path user argument + * @see example_lockdb */ int example_pagination(void *h0, @@ -667,7 +668,7 @@ example_lockdb(clicon_handle h, /* Part of cached pagination example */ - if (strcmp(db, "running") == 0 && lock == 0 && + if (strcmp(db, PAGINATE_STATE_LOCK) == 0 && lock == 0 && _state && _state_file && _state_file_cached && _state_file_transaction){ if (_state_xml_cache){ xml_free(_state_xml_cache); From 52744c930199e740a4d50bd5d54983db122159d4 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sat, 30 Oct 2021 15:49:23 +0200 Subject: [PATCH 07/13] Revert "List pagination locking for state-data changes" This reverts commit eea6d549f67f2e308f3b1b979dab405876f4143f. --- apps/backend/backend_get.c | 13 ++++--------- apps/backend/clixon_backend_plugin.h | 5 ----- apps/backend/clixon_backend_transaction.c | 17 ++++++++--------- apps/cli/cli_show.c | 6 ++++++ example/main/example_backend.c | 3 +-- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c index 54042985..1d814516 100644 --- a/apps/backend/backend_get.c +++ b/apps/backend/backend_get.c @@ -592,16 +592,11 @@ get_list_pagination(clicon_handle h, #endif } else {/* Check if running locked (by this session) */ - if ((iddb = xmldb_islocked(h, PAGINATE_STATE_LOCK)) != 0){ - if (iddb == ce->ce_id) - locked = 1; - else - locked = 0; - } - else { - xmldb_lock(h, PAGINATE_STATE_LOCK, ce->ce_id); + if ((iddb = xmldb_islocked(h, "running")) != 0 && + iddb == ce->ce_id) locked = 1; - } + else + locked = 0; if ((ret = clixon_pagination_cb_call(h, xpath, locked, offset, limit, xret)) < 0) diff --git a/apps/backend/clixon_backend_plugin.h b/apps/backend/clixon_backend_plugin.h index c6ab9177..f88bb41f 100644 --- a/apps/backend/clixon_backend_plugin.h +++ b/apps/backend/clixon_backend_plugin.h @@ -38,11 +38,6 @@ #ifndef _CLIXON_BACKEND_PLUGIN_H_ #define _CLIXON_BACKEND_PLUGIN_H_ -/* - * Constants - */ -#define PAGINATE_STATE_LOCK "paginate-state-lock" - /* * Types */ diff --git a/apps/backend/clixon_backend_transaction.c b/apps/backend/clixon_backend_transaction.c index 7c718a63..0a7889ce 100644 --- a/apps/backend/clixon_backend_transaction.c +++ b/apps/backend/clixon_backend_transaction.c @@ -309,16 +309,15 @@ pagination_limit(pagination_data pd) return ((pagination_data_t *)pd)->pd_limit; } -/*! Get pagination data: locked parameter for state data +/*! Get pagination data: locked parameter * - * A "paginate" lock is set for a specific session when list-pagination is in progress. - * If locking is set, the pagination is part of a session transaction and the plugin may - * cache state (such as a cache) and can expect more pagination calls until the "paginate" - * db-lock is released. - * - * If you want to do the same for config data, it is recommended to use lock-db on - * the appropriate database, but this risks of indefinite blocking, as long as the - * session endures. + * Pagination can use a lock/transaction mechanism + * If locking is not used, the plugin cannot expect more pagination calls, and no state or + * caching should be used + * If locking is used, the pagination is part of a session transaction and the plugin may cache + * state (such as a cache) and can expect more pagination calls until the running db-lock is + * released, (see ca_lockdb) + * The transaction is the regular lock/unlock db of running-db of a specific session. * @param[in] pd Pagination userdata * @retval locked 0: unlocked/stateless 1: locked by this caller */ diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index 6f7f26a0..07ba04ec 100644 --- a/apps/cli/cli_show.c +++ b/apps/cli/cli_show.c @@ -919,6 +919,7 @@ cli_pagination(clicon_handle h, uint32_t limit = 0; cxobj **xvec = NULL; size_t xlen; + int locked = 0; if (cvec_len(argv) != 5){ clicon_err(OE_PLUGIN, 0, "Expected usage: "); @@ -948,6 +949,9 @@ cli_pagination(clicon_handle h, } if ((nsc = xml_nsctx_init(prefix, namespace)) == NULL) goto done; + if (clicon_rpc_lock(h, "running") < 0) + goto done; + locked++; for (i = 0;; i++){ if (clicon_rpc_get_pageable_list(h, "running", xpath, nsc, CONTENT_ALL, @@ -1000,6 +1004,8 @@ cli_pagination(clicon_handle h, } /* for i */ retval = 0; done: + if (locked) + clicon_rpc_unlock(h, "running"); if (xvec) free(xvec); if (xret) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 6d5655d2..947d9d70 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -545,7 +545,6 @@ example_statefile(clicon_handle h, * @param[in] xpath Registered XPath using canonical prefixes * @param[in] userargs Per-call user arguments * @param[in] arg Per-path user argument - * @see example_lockdb */ int example_pagination(void *h0, @@ -668,7 +667,7 @@ example_lockdb(clicon_handle h, /* Part of cached pagination example */ - if (strcmp(db, PAGINATE_STATE_LOCK) == 0 && lock == 0 && + if (strcmp(db, "running") == 0 && lock == 0 && _state && _state_file && _state_file_cached && _state_file_transaction){ if (_state_xml_cache){ xml_free(_state_xml_cache); From 39960cc20594b5f67d471043a09969ef9445b7c8 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 22 Oct 2021 09:38:46 +0200 Subject: [PATCH 08/13] fix: restore signal mask must not use 0 --- lib/src/clixon_sig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/clixon_sig.c b/lib/src/clixon_sig.c index 611a27a8..80f305b7 100644 --- a/lib/src/clixon_sig.c +++ b/lib/src/clixon_sig.c @@ -156,7 +156,7 @@ clixon_signal_restore(sigset_t *sigset, int retval = -1; int i; - if (sigprocmask(0, sigset, NULL) < 0){ + if (sigprocmask(SIG_SETMASK, sigset, NULL) < 0){ clicon_err(OE_UNIX, errno, "sigprocmask"); goto done; } From a52b92bd43198b24e360ca01139a349b56b2ae65 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sat, 23 Oct 2021 17:28:29 +0200 Subject: [PATCH 09/13] Added negative openconfig path compress test --- test/test_cli_auto_genmodel.sh | 59 ++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/test/test_cli_auto_genmodel.sh b/test/test_cli_auto_genmodel.sh index 7af005a1..70593aa9 100755 --- a/test/test_cli_auto_genmodel.sh +++ b/test/test_cli_auto_genmodel.sh @@ -16,6 +16,7 @@ APPNAME=example cfg=$dir/conf_yang.xml fyang=$dir/$APPNAME.yang +fyang2=$dir/${APPNAME}2.yang fstate=$dir/state.xml clidir=$dir/cli if [ -d $clidir ]; then @@ -40,7 +41,7 @@ cat < $cfg /usr/local/share/clixon $dir $OCDIR/ - $fyang + $dir /usr/local/lib/$APPNAME/backend $clidir /usr/local/lib/$APPNAME/cli @@ -60,6 +61,8 @@ module $APPNAME { namespace "urn:example:clixon"; prefix ex; import openconfig-extensions { prefix oc-ext; } + /* Set openconfig version to "fake" an openconfig YANG */ + oc-ext:openconfig-version; container table{ list parameter{ key name; @@ -81,7 +84,50 @@ module $APPNAME { } } container interfaces { - oc-ext:openconfig-version; + list interface { + key name; + leaf name { + type string; + } + container config { + leaf enabled { + type boolean; + default false; + description "Whether the interface is enabled or not."; + } + } + container state { + config false; + leaf oper-status { + type enumeration { + enum UP { + value 1; + description "Ready to pass packets."; + } + enum DOWN { + value 2; + description "The interface does not pass any packets."; + } + } + } + } + leaf enabled { + type boolean; + default false; + description "Whether the interface is enabled or not."; + } + } + } +} +EOF + +# For openconfig but NO openconfig extension +cat < $fyang2 +module ${APPNAME}2 { + namespace "urn:example:clixon2"; + prefix ex2; + import openconfig-extensions { prefix oc-ext; } + container interfaces2 { list interface { key name; leaf name { @@ -259,6 +305,15 @@ expectpart "$($clixon_cli -1 -f $cfg show state)" 0 "exstate sender x" "table pa new "show state exstate" expectpart "$($clixon_cli -1 -f $cfg show state exstate)" 0 "state sender x" --not-- "table parameter a" "table parameter a value x" +#---- openconfig path compression + +new "Openconfig: check no config path" +expectpart "$($clixon_cli -1 -f $cfg set interfaces interface e config enabled true 2>&1)" 255 "Unknown command" + +# negative test +new "Openconfig: check exist config path" +expectpart "$($clixon_cli -1 -f $cfg set interfaces2 interface e config enabled true 2>&1)" 0 "^$" + new "Kill backend" # Check if premature kill pid=$(pgrep -u root -f clixon_backend) From 46647089adb95709080a5fdf11640c4b7ff1220d Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 31 Oct 2021 15:46:41 +0100 Subject: [PATCH 10/13] * Use new cligen cvv options. * NOTE cannot use old CLIgen API --- CHANGELOG.md | 2 +- apps/cli/cli_main.c | 2 +- apps/cli/cli_plugin.c | 44 +++++++++++++++++++++++++++++-------------- lib/src/clixon_xml.c | 5 ++--- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81dc1a13..83825d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,7 +105,7 @@ The 5.3 release has pagination support, Linkref changes in validation and auto-c * Updated state callback signature containing parameters for pagination * See API changes below * Work-in-progress - * Enable remaining attriute with LIST_PAGINATION_REMAINING compile-time option + * Enable remaining attribute with LIST_PAGINATION_REMAINING compile-time option * sort/direction/where etc not supported * For documentation: [User manual pagination](https://clixon-docs.readthedocs.io/en/latest/misc.html#pagination) * YANG Leafref feature update diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index 0264490e..f085d539 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -620,7 +620,7 @@ main(int argc, Should be 0 but default is 1 since all legacy apps use 1 Test legacy before shifting default to 0 */ - cv_exclude_keys(clicon_cli_varonly(h)); + cligen_exclude_keys_set(cli_cligen(h), clicon_cli_varonly(h)); /* Initialize plugin module by creating a handle holding plugin and callback lists */ if (clixon_plugin_module_init(h) < 0) diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c index 77b60cbe..57d6ea4d 100644 --- a/apps/cli/cli_plugin.c +++ b/apps/cli/cli_plugin.c @@ -525,34 +525,50 @@ cli_handler_err(FILE *f) /*! Variant of eval for context checking * @see cligen_eval */ -int -cligen_clixon_eval(cligen_handle h, +static int +clixon_cligen_eval(cligen_handle h, cg_obj *co, cvec *cvv) { struct cg_callback *cc; - int retval = 0; + int retval = -1; cvec *argv; - plugin_context_t *pc = NULL; + cvec *cvv1 = NULL; /* Modified */ + plugin_context_t *pc = NULL; /* Clixon-specific */ + + /* Save matched object for plugin use */ if (h) cligen_co_match_set(h, co); + /* Make a copy of var argument for modifications */ + if ((cvv1 = cvec_dup(cvv)) == NULL) + goto done; + /* Make modifications to cvv */ + if (cligen_expand_first_get(h) && + cvec_expand_first(cvv1) < 0) + goto done; + if (cligen_exclude_keys_get(h) && + cvec_exclude_keys(cvv1) < 0) + goto done; + /* Traverse callbacks */ for (cc = co->co_callbacks; cc; cc=cc->cc_next){ /* Vector cvec argument to callback */ if (cc->cc_fn_vec){ argv = cc->cc_cvec ? cvec_dup(cc->cc_cvec) : NULL; cligen_fn_str_set(h, cc->cc_fn_str); - if ((pc = plugin_context_get()) == NULL) + /* Clixon-specific */ + if ((pc = plugin_context_get()) == NULL) break; if ((retval = (*cc->cc_fn_vec)( cligen_userhandle(h)?cligen_userhandle(h):h, - cvv, + cvv1, argv)) < 0){ if (argv != NULL) cvec_free(argv); cligen_fn_str_set(h, NULL); - break; + goto done; } + /* Clixon-specific */ if (plugin_context_check(pc, "CLIgen", cc->cc_fn_str) < 0) break; if (pc){ @@ -564,8 +580,12 @@ cligen_clixon_eval(cligen_handle h, cligen_fn_str_set(h, NULL); } } - if (pc) + retval = 0; + done: + if (pc) /* Clixon-specific */ free(pc); + if (cvv1) + cvec_free(cvv1); return retval; } @@ -589,7 +609,7 @@ clicon_eval(clicon_handle h, if (!cligen_exiting(cli_cligen(h))) { clicon_err_reset(); - if ((retval = cligen_clixon_eval(cli_cligen(h), match_obj, cvv)) < 0) { + if ((retval = clixon_cligen_eval(cli_cligen(h), match_obj, cvv)) < 0) { #if 0 /* This is removed since we get two error messages on failure. But maybe only sometime? Both a real log when clicon_err is called, and the here again. @@ -663,11 +683,7 @@ clicon_parse(clicon_handle h, fprintf(stderr, "No such parse-tree registered: %s\n", modename); goto done; } - if ((cvv = cvec_new(0)) == NULL){ - clicon_err(OE_UNIX, errno, "cvec_new"); - goto done;; - } - if (cliread_parse(cli_cligen(h), cmd, pt, &match_obj, cvv, result, &reason) < 0) + if (cliread_parse(cli_cligen(h), cmd, pt, &match_obj, &cvv, result, &reason) < 0) goto done; /* Debug command and result code */ clicon_debug(1, "%s result:%d command: \"%s\"", __FUNCTION__, *result, cmd); diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 750bbab2..a3c32725 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -581,10 +581,11 @@ xml_parent(cxobj *xn) return xn->x_up; } -/*! Set parent of xnode, parent is copied. +/*! Set parent of xml node. * @param[in] xn xml node * @param[in] parent pointer to new parent xml node * @retval 0 OK + * @see xml_child_rm remove child from parent */ int xml_parent_set(cxobj *xn, @@ -1986,8 +1987,6 @@ xml_copy(cxobj *x0, return retval; } - - /*! Create and return a copy of xml tree. * * @code From d335a72659da5baf42b6faac01fb8c97eb43063b Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 31 Oct 2021 16:36:29 +0100 Subject: [PATCH 11/13] Use 5.3 cligen API --- apps/cli/cli_plugin.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c index 57d6ea4d..90b7ec5b 100644 --- a/apps/cli/cli_plugin.c +++ b/apps/cli/cli_plugin.c @@ -683,8 +683,15 @@ clicon_parse(clicon_handle h, fprintf(stderr, "No such parse-tree registered: %s\n", modename); goto done; } - if (cliread_parse(cli_cligen(h), cmd, pt, &match_obj, &cvv, result, &reason) < 0) +#if 0 // switch after 5.4 + if (cliread_parse2(cli_cligen(h), cmd, pt, &match_obj, &cvv, result, &reason) < 0) goto done; +#else + if ((cvv = cvec_new(0)) == NULL) + goto done;; + if (cliread_parse(cli_cligen(h), cmd, pt, &match_obj, cvv, result, &reason) < 0) + goto done; +#endif /* Debug command and result code */ clicon_debug(1, "%s result:%d command: \"%s\"", __FUNCTION__, *result, cmd); if (*result != CG_MATCH) From 5297ebe3cbde7acd300f2878a74917137bfce87d Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 4 Nov 2021 16:22:27 +0100 Subject: [PATCH 12/13] Fixed: [cl:autocli-op hide has no effect in yang submodule](https://github.com/clicon/clixon/issues/282) --- CHANGELOG.md | 1 + lib/src/clixon_yang.c | 11 +++++++ lib/src/clixon_yang_internal.h | 10 ++++-- test/test_cli_auto_extension.sh | 55 ++++++++++++++++++++++++++++++++- 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83825d83..6688382b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ Developers may need to change their code ### Corrected Bugs +* [cl:autocli-op hide has no effect in yang submodule](https://github.com/clicon/clixon/issues/282) * [Doxygen - Typo in Input #275](https://github.com/clicon/clixon/issues/275) ## 5.3.0 diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index b4b364fa..c9a8b0a0 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -2355,6 +2355,14 @@ ys_populate_unique(clicon_handle h, * RFC 7950 Sec 7.19: * If no "argument" statement is present, the keyword expects no argument when * it is used. + * Note there is some complexity in different yang modules with unknown-statements. + * y0) The location of the extension definition. E.g. extension autocli-op + * y1) The location of the unknown-statement (ys). This is for example: cl:autocli-op hide. + * Lookup of "cl" is lexically scoped in this context (to find (y0)). + * y2) The unknown statement may be used by uses/grouping of (y1). But "cl" is declared + * in y1 context. This is fixed by setting ys_mymodule to y1 which is then used in + * lookups such as in yang_extension_value(). + * @see yang_extension_value Called on expanded YANG, eg in context of (y2) */ static int ys_populate_unknown(clicon_handle h, @@ -2376,6 +2384,8 @@ ys_populate_unknown(clicon_handle h, clicon_err(OE_YANG, ENOENT, "Extension \"%s:%s\", module not found", prefix, id); goto done; } + /* To find right binding eg after grouping/uses */ + ys->ys_mymodule = ys_module(ys); if ((yext = yang_find(ymod, Y_EXTENSION, id)) == NULL){ clicon_err(OE_YANG, ENOENT, "Extension \"%s:%s\" not found", prefix, id); goto done; @@ -3572,6 +3582,7 @@ yang_anydata_add(yang_stmt *yp, * // use extension value * } * @endcode + * @see ys_populate_unknown Called when parsing YANGo */ int yang_extension_value(yang_stmt *ys, diff --git a/lib/src/clixon_yang_internal.h b/lib/src/clixon_yang_internal.h index 06a73d4c..4fb3a489 100644 --- a/lib/src/clixon_yang_internal.h +++ b/lib/src/clixon_yang_internal.h @@ -71,9 +71,13 @@ struct yang_stmt{ char *ys_argument; /* String / argument depending on keyword */ uint16_t ys_flags; /* Flags according to YANG_FLAG_MARK and others */ - yang_stmt *ys_mymodule; /* Shortcut to "my" module. Augmented - nodes can belong to other - modules than the ancestor module */ + yang_stmt *ys_mymodule; /* Shortcut to "my" module. Used by: + 1) Augmented nodes "belong" to the module where the + augment is declared, which may be differnt from + the direct ancestor module + 2) Unknown nodes "belong" to where the extension is + declared + */ cg_var *ys_cv; /* cligen variable. See ys_populate() Following stmts have cv:s: leaf: for default value diff --git a/test/test_cli_auto_extension.sh b/test/test_cli_auto_extension.sh index 412cb873..6a7c8f35 100755 --- a/test/test_cli_auto_extension.sh +++ b/test/test_cli_auto_extension.sh @@ -136,7 +136,6 @@ wait_backend function testparam() { - # Try hidden parameter list new "query table parameter hidden" expectpart "$(echo "set table ?" | $clixon_cli -f $cfg 2>&1)" 0 "set table" "" --not-- "parameter" @@ -282,6 +281,60 @@ EOF new "Test hidden parameter in table/param/value augment" testvalue +# Example using imported module where clixon-lib is NOT declared in main module +# see discussion in ys_populate_unknown (y1 vs y2) and +# https://github.com/clicon/clixon/issues/282 +cat < $fyang +module example { + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + import example-augment{ + prefix aug; + } + + container table{ + list parameter{ + key name; + leaf name{ + type string; + } + uses aug:pg; + } + } +} +EOF + +# Use this as grouping (not annotate) +cat < $fyang2 +module example-augment { + namespace "urn:example:augment"; + prefix aug; + import clixon-lib{ + prefix cl; + } + grouping pg { + cl:autocli-op hide; /* This is the extension */ + leaf value{ + description "a value"; + type string; + } + list index{ + key i; + leaf i{ + type string; + } + leaf iv{ + type string; + } + } + } +} +EOF + +new "Test hidden parameter in imported module" +testparam + new "Kill backend" # Check if premature kill pid=$(pgrep -u root -f clixon_backend) From a1fe08011138e01e515d479d77bd279de75d0cb8 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 5 Nov 2021 14:43:28 +0100 Subject: [PATCH 13/13] CLIgen mem leak, test augment --- apps/cli/cli_plugin.c | 4 +++- test/test_augment.sh | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c index 90b7ec5b..f83a99e6 100644 --- a/apps/cli/cli_plugin.c +++ b/apps/cli/cli_plugin.c @@ -651,7 +651,7 @@ clicon_parse(clicon_handle h, cli_syntax_t *stx = NULL; cli_syntaxmode_t *csm; parse_tree *pt; /* Orig */ - cg_obj *match_obj; + cg_obj *match_obj = NULL; cvec *cvv = NULL; FILE *f; char *reason = NULL; @@ -731,6 +731,8 @@ done: free(reason); if (cvv) cvec_free(cvv); + if (match_obj) + co_free(match_obj, 0); return retval; } diff --git a/test/test_augment.sh b/test/test_augment.sh index a25eaede..65dfaf0d 100755 --- a/test/test_augment.sh +++ b/test/test_augment.sh @@ -185,6 +185,18 @@ module example-augment { type string; } } + /* augment a list */ + augment "/if:interfaces" { + list ports{ + key id; + leaf id { + type int32; + } + leaf str { + type string; + } + } + } } EOF @@ -291,6 +303,30 @@ expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/ietf-inte new "restconf GET augment multi-namespace cross level 2" expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/ietf-interfaces:interfaces/interface=e1/example-augment:ospf/reference-bandwidth)" 0 "HTTP/$HVER 200" '{"example-augment:reference-bandwidth":23}' +new "delete interfaces" +expectpart "$(curl $CURLOPTS -X DELETE $RCPROTO://localhost/restconf/data/ietf-interfaces:interfaces)" 0 "HTTP/$HVER 204" + +# augmented lists +XML="22nisse44kalle" +new "netconf PUT augmented list" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOmerge$XML]]>]]>" "^]]>]]>$" + +new "netconf commit" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf get config" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "$XML]]>]]>" + +JSON='{"ietf-interfaces:interfaces":{"example-augment:ports":[{"id":22,"str":"foo"},{"id":44,"str":"bar"}]}}' + +new "restconf PUT augmented list" +expectpart "$(curl $CURLOPTS -X PUT -H 'Content-Type: application/yang-data+json' $RCPROTO://localhost/restconf/data/ietf-interfaces:interfaces -d "$JSON")" 0 "HTTP/$HVER 204" + +# Escaped [] why? +JSON1='{"ietf-interfaces:interfaces":{"example-augment:ports":\[{"id":22,"str":"foo"},{"id":44,"str":"bar"}\]}}' +new "restconf GET augmented list" +expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+json' $RCPROTO://localhost/restconf/data/ietf-interfaces:interfaces)" 0 "HTTP/$HVER 200" "$JSON1" + if [ $RC -ne 0 ]; then new "Kill restconf daemon" stop_restconf