From f526d5b7a0835f38d53922b8cbf065d2f9075c48 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 23 Jul 2023 15:11:34 +0200 Subject: [PATCH] Autocli optimization feature for generating smaller CLISPECs for large YANGs using treerefs New `grouping-treeref` option added to clixon-autocli.yang Default is disabled, set to true to generate smaller memory footprint of clixon_cl Add prefix "mtpoint:" to cli api-paths to identify change of yang schemas --- CHANGELOG.md | 4 + apps/cli/cli_auto.c | 4 +- apps/cli/cli_autocli.c | 43 ++++ apps/cli/cli_autocli.h | 1 + apps/cli/cli_common.c | 49 +++- apps/cli/cli_generate.c | 281 +++++++++++++++++++-- apps/cli/cli_generate.h | 5 + apps/cli/cli_show.c | 55 +++- include/clixon_custom.h | 14 + lib/clixon/clixon_yang_parse_lib.h | 1 + lib/src/clixon_path.c | 33 ++- lib/src/clixon_yang_parse_lib.c | 15 +- test/test_autocli_grouping.sh | 211 ++++++++++++++++ yang/clixon/clixon-autocli@2023-05-01.yang | 16 ++ 14 files changed, 668 insertions(+), 64 deletions(-) create mode 100755 test/test_autocli_grouping.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index a7c335e5..34367048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ Users may have to change how they access the system * rpc `` is not backward compatible * New `clixon-autocli@2023-05-01.yang` revision * New `alias` and `skip` extensions + * New `grouping-treeref` option ### C/CLI-API changes on existing features Developers may need to change their code @@ -72,6 +73,9 @@ Developers may need to change their code ### Minor features +* Autocli optimization feature for generating smaller CLISPECs for large YANGs using treerefs + * New `grouping-treeref` option added to clixon-autocli.yang + * Default is disabled, set to true to generate smaller memory footprint of clixon_cli * Changed YANG uses/grouping to keep uses statement and flag it with YANG_FLAG_USES_EXP * Removed extras/ and build-root/ build code since they are not properly maintained * Refactored cli-syntax code to use cligen pt_head instead (long overdue) diff --git a/apps/cli/cli_auto.c b/apps/cli/cli_auto.c index f75e4398..5af4d9d5 100644 --- a/apps/cli/cli_auto.c +++ b/apps/cli/cli_auto.c @@ -135,8 +135,8 @@ cli_auto_edit(clicon_handle h, } api_path_fmt = cv_string_get(cvec_i(argv, argc++)); str = cv_string_get(cvec_i(argv, argc++)); - if (str && str[0] == '/'){ /* ad-hoc to see if 2nd arg is mountpoint */ - mtpoint = str; + if (str && strncmp(str, "mtpoint:", strlen("mtpoint:")) == 0){ + mtpoint = str + strlen("mtpoint:"); clicon_debug(1, "%s mtpoint:%s", __FUNCTION__, mtpoint); treename = cv_string_get(cvec_i(argv, argc++)); } diff --git a/apps/cli/cli_autocli.c b/apps/cli/cli_autocli.c index 64e3ecaa..d83cc73a 100644 --- a/apps/cli/cli_autocli.c +++ b/apps/cli/cli_autocli.c @@ -384,6 +384,49 @@ autocli_completion(clicon_handle h, return retval; } +/*! Return autocli grouping treeref option + * + * When false replaces uses with grouping, when true use tree reference + * @param[in] h Clixon handle + * @param[out] treeref grouping using treerefs enabled + * @retval -1 Error + * @retval 0 OK + */ +int +autocli_grouping_treeref(clicon_handle h, + int *treeref) +{ + int retval = -1; + char *str; + uint8_t val; + char *reason = NULL; + int ret; + cxobj *xautocli; + + if (treeref == NULL){ + clicon_err(OE_YANG, EINVAL, "Argument is NULL"); + goto done; + } + if ((xautocli = clicon_conf_autocli(h)) == NULL){ + clicon_err(OE_YANG, 0, "No clixon-autocli"); + goto done; + } + if ((str = xml_find_body(xautocli, "grouping-treeref")) == NULL){ + clicon_err(OE_XML, EINVAL, "No grouping-treeref rule"); + goto done; + } + if ((ret = parse_bool(str, &val, &reason)) < 0){ + clicon_err(OE_CFG, errno, "parse_bool"); + goto done; + } + *treeref = val; + retval = 0; + done: + if (reason) + free(reason); + return retval; +} + /*! Return default autocli list keyword setting * * Currently only returns list-keyword-default, could be extended to rules diff --git a/apps/cli/cli_autocli.h b/apps/cli/cli_autocli.h index 2abec68f..bbd197c9 100644 --- a/apps/cli/cli_autocli.h +++ b/apps/cli/cli_autocli.h @@ -52,6 +52,7 @@ enum autocli_op{ */ int autocli_module(clicon_handle h, char *modname, int *enable); int autocli_completion(clicon_handle h, int *completion); +int autocli_grouping_treeref(clicon_handle h, int *grouping_treeref); int autocli_list_keyword(clicon_handle h, autocli_listkw_t *listkw); int autocli_compress(clicon_handle h, yang_stmt *ys, int *compress); int autocli_treeref_state(clicon_handle h, int *treeref_state); diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index 77809909..7cf8bafa 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -348,7 +348,10 @@ mtpoint_paths(yang_stmt *yspec0, * * @param[in] h Clicon handle * @param[in] cvv Vector of cli string and instantiated variables - * @param[in] argv Vector: [], eg "/aaa/%s" + * @param[in] argv Arguments given at the callback: + * Generated API PATH (this is added implicitly, not actually given in the cvv) + * * Added by merge in treeref_merge_co + * [] Optional YANG path-arg/xpath from mount-point * @param[in] op Operation to perform on database * @param[in] nsctx Namespace context for last value added * cvv first contains the complete cli string, and then a set of optional @@ -373,10 +376,10 @@ cli_dbxml(clicon_handle h, cvec *nsctx) { int retval = -1; + cbuf *api_path_fmt_cb = NULL; /* xml key format */ char *api_path_fmt; /* xml key format */ char *api_path_fmt01 = NULL; char *api_path = NULL; - cg_var *arg; cbuf *cb = NULL; cxobj *xbot = NULL; /* xpath, NULL if datastore */ yang_stmt *y = NULL; /* yang spec of xpath */ @@ -384,24 +387,46 @@ cli_dbxml(clicon_handle h, cxobj *xerr = NULL; int ret; cg_var *cv; + char *str; int cvvi = 0; char *mtpoint = NULL; yang_stmt *yspec0 = NULL; + int argc = 0; + int i; - if (cvec_len(argv) != 1 && cvec_len(argv) != 2){ - clicon_err(OE_PLUGIN, EINVAL, "Requires one element to be xml key format string"); - goto done; - } /* Top-level yspec */ if ((yspec0 = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_FATAL, 0, "No DB_SPEC"); goto done; } - arg = cvec_i(argv, 0); - api_path_fmt = cv_string_get(arg); - if (cvec_len(argv) > 1){ - arg = cvec_i(argv, 1); - mtpoint = cv_string_get(arg); + if ((api_path_fmt_cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + /* Iterate through all api_path_fmt:s, assume they start with / */ + for (argc=0; argc in argv"); + goto done; + } + /* Append a api_path_fmt from sub-parts */ + for (i=argc-1; i>=0; i--){ + cprintf(api_path_fmt_cb, "%s", cv_string_get(cvec_i(argv, i))); + } + api_path_fmt = cbuf_get(api_path_fmt_cb); + if (cvec_len(argv) > argc){ + cv = cvec_i(argv, argc++); + str = cv_string_get(cv); + if (strncmp(str, "mtpoint:", strlen("mtpoint:")) != 0){ + clicon_err(OE_PLUGIN, 0, "mtpoint does not begin with 'mtpoint:'"); + goto done; + } + mtpoint = str + strlen("mtpoint:"); } /* Remove all keywords */ if (cvec_exclude_keys(cvv) < 0) @@ -483,6 +508,8 @@ cli_dbxml(clicon_handle h, goto done; retval = 0; done: + if (api_path_fmt_cb) + cbuf_free(api_path_fmt_cb); if (api_path_fmt01) free(api_path_fmt01); if (xerr) diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index 46b66f1b..0691d03a 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -103,12 +103,6 @@ You can see which CLISPEC it generates via clixon_cli -D 2: #include "cli_autocli.h" #include "cli_generate.h" -/* - * Constants - */ -/* variable expand function */ -#define GENERATE_EXPAND_XMLDB "expand_dbvar" - /*! Create cligen variable expand entry with api-pathfmt format string as argument * * Add api-fmt as first argument to callback, eg: @@ -160,8 +154,9 @@ cli_expand_var_generate(clicon_handle h, cprintf(cb, " %s(\"candidate\",\"%s\"", GENERATE_EXPAND_XMLDB, api_path_fmt); - if (cv) /* Add optional mountpoint */ - cprintf(cb, ",\"%s\"", cv_string_get(cv)); + if (cv){ /* Add optional mountpoint */ + cprintf(cb, ",\"%s%s\"", MTPOINT_PREFIX, cv_string_get(cv)); + } cprintf(cb, ")>"); retval = 0; done: @@ -192,8 +187,9 @@ cli_callback_generate(clicon_handle h, if (yang2api_path_fmt(ys, 0, &api_path_fmt) < 0) goto done; cprintf(cb, ",%s(\"%s\"", GENERATE_CALLBACK, api_path_fmt); - if (cv) /* Add optional mountpoint */ - cprintf(cb, ",\"%s\"", cv_string_get(cv)); + if (cv){ /* Add optional mountpoint */ + cprintf(cb, ",\"%s%s\"", MTPOINT_PREFIX, cv_string_get(cv)); + } cprintf(cb, ")"); retval = 0; done: @@ -265,7 +261,7 @@ yang2cli_var_identityref(yang_stmt *ys, if ((ymod = yang_find_module_by_name(yspec, prefix)) != NULL && (yprefix = yang_find(ymod, Y_PREFIX, NULL)) != NULL){ if (i++) - cprintf(cb, "|"); + cprintf(cb, "|"); cprintf(cb, "%s:%s", yang_argument_get(yprefix), id); } if (prefix){ @@ -461,21 +457,21 @@ yang2cli_var_sub(clicon_handle h, /* enumeration special case completion */ if (type){ if (strcmp(type, "enumeration") == 0 || strcmp(type, "bits") == 0){ - cprintf(cb, " choice:"); + cprintf(cb, " choice:"); i = 0; yi = NULL; while ((yi = yn_each(ytype, yi)) != NULL){ if (yang_keyword_get(yi) != Y_ENUM && yang_keyword_get(yi) != Y_BIT) continue; if (i) - cprintf(cb, "|"); + cprintf(cb, "|"); /* Encode by escaping delimiters */ arg = yang_argument_get(yi); len = strlen(arg); for (j=0; jco_command); + co_prefix_set(co, prefix); + } + } + /* Post-processing, iterate over the generated cligen parse-tree with corresponding yang + * Note cannot do it inline in yang2cli above since: + * 1. labels cannot be set on "empty" + * 2. a; , fn() cannot be set properly + */ + config = 1; + if (yang2cli_post(h, NULL, pt, 0, ys, NULL, &config) < 0){ + goto done; + } + if (clicon_data_int_get(h, "autocli-print-debug") == 1) + clicon_log(LOG_NOTICE, "%s: Top-level cli-spec %s:\n%s", + __FUNCTION__, treename, cbuf_get(cb)); + else + clicon_debug(CLIXON_DBG_DETAIL, "%s: Top-level cli-spec %s:\n%s", + __FUNCTION__, treename, cbuf_get(cb)); + if (cligen_parsetree_merge(pt0, NULL, pt) < 0){ + clicon_err(OE_YANG, errno, "cligen_parsetree_merge"); + goto done; + } + pt_free(pt, 1); + pt = NULL; + + /* Resolve the expand callback functions in the generated syntax. + * This "should" only be GENERATE_EXPAND_XMLDB + * handle=NULL for global namespace, this means expand callbacks must be in + * CLICON namespace, not in a cli frontend plugin. + */ + if (cligen_expandv_str2fn(pt0, (expandv_str2fn_t*)clixon_str2fn, NULL) < 0) + goto done; + /* Append cligen tree and name it */ + if ((ph = cligen_ph_add(cli_cligen(h), treename)) == NULL){ + clicon_err(OE_UNIX, 0, "cligen_ph_add"); + goto done; + } + if (cligen_ph_parsetree_set(ph, pt0) < 0){ + clicon_err(OE_UNIX, 0, "cligen_ph_parsetree_set"); + goto done; + } + pt0 = NULL; + retval = 1; + done: + if (pt) + pt_free(pt, 1); + if (pt0) + pt_free(pt0, 1); + if (cb) + cbuf_free(cb); + return retval; + empty: + retval = 0; + goto done; +} + /*! Generate clispec for all modules in yspec (except excluded) * * Called in cli main function for top-level yangs. But may also be called dynamically for @@ -1402,7 +1638,6 @@ yang2cli_yspec(clicon_handle h, clicon_err(OE_UNIX, errno, "pt_new"); goto done; } - /* Parse the buffer using cligen parser. load cli syntax */ if (clispec_parse_str(cli_cligen(h), cbuf_get(cb), "yang2cli", NULL, pt, NULL) < 0){ fprintf(stderr, "%s\n", cbuf_get(cb)); @@ -1446,7 +1681,7 @@ yang2cli_yspec(clicon_handle h, * handle=NULL for global namespace, this means expand callbacks must be in * CLICON namespace, not in a cli frontend plugin. */ - if (cligen_expandv_str2fn(pt0, (expandv_str2fn_t*)clixon_str2fn, NULL) < 0) + if (cligen_expandv_str2fn(pt0, (expandv_str2fn_t*)clixon_str2fn, NULL) < 0) goto done; /* Append cligen tree and name it */ if ((ph = cligen_ph_add(cli_cligen(h), treename)) == NULL){ diff --git a/apps/cli/cli_generate.h b/apps/cli/cli_generate.h index b1a3a168..69014df2 100644 --- a/apps/cli/cli_generate.h +++ b/apps/cli/cli_generate.h @@ -48,6 +48,11 @@ * where the virtual callback (overwrite_me) is overwritten by cli_set. */ #define GENERATE_CALLBACK "overwrite_me" +#define GROUPING_CALLBACK "prepend_me" +#define MTPOINT_PREFIX "mtpoint:" + +/* variable expand function */ +#define GENERATE_EXPAND_XMLDB "expand_dbvar" /* Name of autocli CLIgen treename */ diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index 733af0b3..00d25051 100644 --- a/apps/cli/cli_show.c +++ b/apps/cli/cli_show.c @@ -173,13 +173,17 @@ xpath_append(cbuf *cb0, * functionality. * * Assume callback given in a cligen spec: a ") - * @param[in] h clicon handle + * @param[in] h clicon handle * @param[in] name Name of this function (eg "expand_dbvar") * @param[in] cvv The command so far. Eg: cvec [0]:"a 5 b"; [1]: x=5; - * @param[in] argv Arguments given at the callback: [] + * @param[in] argv Arguments given at the callback: + * Name of datastore, such as "running" + * Generated API PATH (this is added implicitly, not actually given in the cvv) + * [] Optional YANG path-arg/xpath from mount-point * @param[out] commands vector of function pointers to callback functions * @param[out] helptxt vector of pointers to helptexts * @see cli_expand_var_generate This is where arg is generated + * @see cli_expand_var_generate where api_path_fmt + mt-point are generated */ int expand_dbvar(void *h, @@ -190,10 +194,10 @@ expand_dbvar(void *h, cvec *helptexts) { int retval = -1; - char *api_path_fmt; + char *api_path_fmt = NULL; char *api_path = NULL; char *api_path_fmt01 = NULL; - char *dbstr; + char *dbstr; cxobj *xt = NULL; char *xpath = NULL; cxobj **xvec = NULL; @@ -220,7 +224,10 @@ expand_dbvar(void *h, yang_stmt *yspec; yang_stmt *yu = NULL; cvec *nsc0 = NULL; - + char *str; + int grouping_treeref; + cbuf *cbprepend; + if (argv == NULL || (cvec_len(argv) != 2 && cvec_len(argv) != 3)){ clicon_err(OE_PLUGIN, EINVAL, "requires arguments: []"); goto done; @@ -244,10 +251,33 @@ expand_dbvar(void *h, clicon_err(OE_PLUGIN, 0, "Error when accessing argument "); goto done; } - api_path_fmt = cv_string_get(cv); + if (autocli_grouping_treeref(h, &grouping_treeref) < 0) + goto done; + if (grouping_treeref && + (cbprepend = cligen_expand_prepend_get(cli_cligen(h))) != NULL){ + cbuf *cb; + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cb, "%s%s", cbuf_get(cbprepend), cv_string_get(cv)); + if ((api_path_fmt = strdup(cbuf_get(cb))) == NULL){ + clicon_err(OE_UNIX, errno, "strdup"); + goto done; + } + } + else if ((api_path_fmt = strdup(cv_string_get(cv))) == NULL){ + clicon_err(OE_UNIX, errno, "strdup"); + goto done; + } if (cvec_len(argv) > 2){ cv = cvec_i(argv, 2); - mtpoint = cv_string_get(cv); + str = cv_string_get(cv); + if (strncmp(str, "mtpoint:", strlen("mtpoint:")) != 0){ + clicon_err(OE_PLUGIN, 0, "mtpoint does not begin with 'mtpoint:'"); + goto done; + } + mtpoint = str + strlen("mtpoint:"); } /* api_path_fmt = /interface/%s/address/%s * api_path: --> /interface/eth0/address/.* @@ -384,6 +414,8 @@ expand_dbvar(void *h, done: if (nsc0) cvec_free(nsc0); + if (api_path_fmt) + free(api_path_fmt); if (api_path_fmt01) free(api_path_fmt01); if (cbxpath) @@ -847,8 +879,8 @@ int cli_show_version(clicon_handle h, * @param[in] h Clixon handle * @param[in] cvv Vector of variables from CLIgen command-line * @param[in] argv String vector of show options, format: - * Generated API PATH (this is added implicitly, not actually given in the cvv) - * [] Extra api-path from mount-point + * Generated API PATH (this is added implicitly, not actually given in argv) + * [] Optional YANG path-arg/xpath from mount-point * Name of datastore, such as "running" * -- from here optional: * "text"|"xml"|"json"|"cli"|"netconf" (see format_enum), default: xml @@ -872,6 +904,7 @@ int cli_show_version(clicon_handle h, * @see cli_show_auto_mode autocli with edit menu support * * XXX merge cli_show_auto and cli_show_auto_mode + * @see cli_callback_generate where api_path_fmt + mt-point are generated */ int cli_show_auto(clicon_handle h, @@ -903,8 +936,8 @@ cli_show_auto(clicon_handle h, } api_path_fmt = cv_string_get(cvec_i(argv, argc++)); str = cv_string_get(cvec_i(argv, argc++)); - if (str && str[0] == '/'){ /* ad-hoc to see if 2nd arg is mountpoint */ - mtpoint = str; + if (str && strncmp(str, "mtpoint:", strlen("mtpoint:")) == 0){ + mtpoint = str + strlen("mtpoint:"); dbname = cv_string_get(cvec_i(argv, argc++)); } else diff --git a/include/clixon_custom.h b/include/clixon_custom.h index f063d4cc..e8ca9996 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -156,3 +156,17 @@ * Consider moving to configure.ac(compile-time) or to clixon-config.yang(run-time) */ #define PRETTYPRINT_INDENT 3 + +/*! Autocli uses/grouping references for top-level + * Exception of expand-grouping in clixon-autocli.yang + * If enabled do not expand-grouping if a yang uses is directly under module or submodule + * Disabled does not work today and is temporary and for documentation + */ +#define AUTOCLI_GROUPING_TOPLEVEL_SKIP + +/*! Autocli uses/grouping references for augment/uses + * Exception of expand-grouping in clixon-autocli.yang + * If enabled do not expand-grouping if a yang uses is directly under augment + * Disabled does not work today and is temporary and for documentation + */ +#define AUTOCLI_GROUPING_AUGMENT_SKIP diff --git a/lib/clixon/clixon_yang_parse_lib.h b/lib/clixon/clixon_yang_parse_lib.h index 724f71c0..335f839a 100644 --- a/lib/clixon/clixon_yang_parse_lib.h +++ b/lib/clixon/clixon_yang_parse_lib.h @@ -51,6 +51,7 @@ /* * Prototypes */ +int ys_grouping_resolve(yang_stmt *yuses, char *prefix, char *name, yang_stmt **ygrouping0); yang_stmt *yang_parse_file(FILE *fp, const char *name, yang_stmt *ysp); int yang_file_find_match(clicon_handle h, const char *module, const char *revision, cbuf *fbuf); yang_stmt *yang_parse_filename(clicon_handle h, const char *filename, yang_stmt *ysp); diff --git a/lib/src/clixon_path.c b/lib/src/clixon_path.c index e9949cee..a595e5aa 100644 --- a/lib/src/clixon_path.c +++ b/lib/src/clixon_path.c @@ -290,8 +290,8 @@ xml_yang_root(cxobj *x, * @param[in] ys Yang statement * @param[in] inclkey If set include key leaf (eg last leaf d in ex) * @param[out] cb api_path_fmt, - * @retval 0 OK - * @retval -1 Error + * @retval 0 OK + * @retval -1 Error * @see RFC8040 3.5.3 where "api-path" is defined as "URI-encoded path expression" */ static int @@ -305,18 +305,26 @@ yang2api_path_fmt_1(yang_stmt *ys, int i; cvec *cvk = NULL; /* vector of index keys */ int retval = -1; + enum rfc_6020 ypkeyw = -1; if ((yp = yang_parent_get(ys)) == NULL){ clicon_err(OE_YANG, EINVAL, "yang expected parent %s", yang_argument_get(ys)); goto done; } - if (yp != NULL && /* XXX rm */ - yang_keyword_get(yp) != Y_MODULE && - yang_keyword_get(yp) != Y_SUBMODULE){ - + if (yp) + ypkeyw = yang_keyword_get(yp); + switch (ypkeyw){ + case Y_MODULE: + case Y_SUBMODULE: + cprintf(cb, "/%s:", yang_argument_get(yp)); + break; + case Y_GROUPING: /* mainly for expand-grouping in clixon-autocli.yang */ + cprintf(cb, "/"); + break; + default: if (yang2api_path_fmt_1((yang_stmt *)yp, 1, cb) < 0) /* recursive call */ goto done; - if (yang_keyword_get(yp) != Y_CHOICE && yang_keyword_get(yp) != Y_CASE){ + if (ypkeyw != Y_CHOICE && ypkeyw != Y_CASE){ #if 0 /* In some cases, such as cli_show_auto, a trailing '/' should * NOT be present if ys is a key in a list. @@ -324,7 +332,7 @@ yang2api_path_fmt_1(yang_stmt *ys, * so a patch is added in cli_show_auto instead. */ if (yang_keyword_get(ys) == Y_LEAF && yp && - yang_keyword_get(yp) == Y_LIST && + ypkeyw == Y_LIST && yang_key_match(yp, ys->ys_argument, NULL) == 1) ; else @@ -338,17 +346,15 @@ yang2api_path_fmt_1(yang_stmt *ys, goto done; if (ypmod != ymod) cprintf(cb, "%s:", yang_argument_get(ymod)); + break; } - else /* top symbol - mark with name prefix */ - cprintf(cb, "/%s:", yang_argument_get(yp)); - if (inclkey){ if (yang_keyword_get(ys) != Y_CHOICE && yang_keyword_get(ys) != Y_CASE) cprintf(cb, "%s", yang_argument_get(ys)); } else{ if (yang_keyword_get(ys) == Y_LEAF && yp && - yang_keyword_get(yp) == Y_LIST){ + ypkeyw == Y_LIST){ if (yang_key_match(yp, yang_argument_get(ys), NULL) == 0) cprintf(cb, "%s", yang_argument_get(ys)); /* Not if leaf and key */ } @@ -381,6 +387,7 @@ yang2api_path_fmt_1(yang_stmt *ys, } /*! Construct an api_path_format from yang statement using wildcards for keys + * * Recursively construct it to the top. * Example: * yang: container a -> list b -> key c -> leaf d @@ -389,7 +396,7 @@ yang2api_path_fmt_1(yang_stmt *ys, * @param[in] inclkey If set include key leaf (eg last leaf d in ex) * @param[out] api_path_fmt XML api path. Needs to be freed after use. * @retval 0 OK - * @retval -1 Error + * @retval -1 Error * "api-path" is "URI-encoded path expression" definition in RFC8040 3.5.3 */ int diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index 5ff4bb45..1cb94f89 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -156,7 +156,7 @@ ys_grouping_module_resolve(yang_stmt *ymod, * @retval 0 OK, ygrouping may be NULL * @retval -1 Error, with clicon_err called */ -static int +int ys_grouping_resolve(yang_stmt *yuses, char *prefix, char *name, @@ -235,7 +235,6 @@ yang_augment_node(clicon_handle h, clicon_err(OE_YANG, 0, "My yang module not found"); goto done; } - /* */ schema_nodeid = yang_argument_get(ys); clicon_debug(CLIXON_DBG_DETAIL, "%s %s", __FUNCTION__, schema_nodeid); /* Find the target */ @@ -279,7 +278,11 @@ yang_augment_node(clicon_handle h, while ((yc0 = yn_each(ys, yc0)) != NULL) { childkey = yang_keyword_get(yc0); /* Only shemanodes and extensions */ - if (!yang_schemanode(yc0) && childkey != Y_UNKNOWN) + if (!yang_schemanode(yc0) && childkey != Y_UNKNOWN +#ifndef AUTOCLI_GROUPING_AUGMENT_SKIP + && childkey != Y_USES +#endif + ) continue; switch (targetkey){ case Y_CONTAINER: @@ -344,9 +347,12 @@ yang_augment_node(clicon_handle h, default: break; } - if ((yc = ys_dup(yc0)) == NULL) goto done; +#ifdef AUTOCLI_GROUPING_AUGMENT_SKIP + /* cornercase: always expand uses under augment */ + yang_flag_reset(yc, YANG_FLAG_GROUPING); +#endif yc->ys_mymodule = ymod; if (yn_insert(ytarget, yc) < 0) @@ -679,6 +685,7 @@ yang_expand_uses_node(yang_stmt *yn, yang_flag_set(yg, YANG_FLAG_NOKEY); yn->ys_stmt[i+k+1] = yg; yg->ys_parent = yn; + yang_flag_set(yg, YANG_FLAG_GROUPING); k++; } /* Remove the grouping copy */ diff --git a/test/test_autocli_grouping.sh b/test/test_autocli_grouping.sh new file mode 100755 index 00000000..293ce141 --- /dev/null +++ b/test/test_autocli_grouping.sh @@ -0,0 +1,211 @@ +#!/usr/bin/env bash +# Tests for saving memory by not expanding grouping/uses in the autocli + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + +APPNAME=example + +cfg=$dir/conf_yang.xml +fyang=$dir/example.yang +fyang2=$dir/example-external.yang +fyang3=$dir/example-external3.yang + +# Whether grouping treeref is enabled +grouping_treeref=true + +cat < $cfg + + $cfg + ietf-netconf:startup + ${YANG_INSTALLDIR} + $dir + $dir + /usr/local/lib/$APPNAME/backend + $dir + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + false + + false + kw-nokey + ${grouping_treeref} + + include ${APPNAME} + enable + ${APPNAME}* + + + +EOF + +cat < $dir/example.cli +CLICON_MODE="example"; +CLICON_PROMPT="%U@%H %W> "; + +# Autocli syntax tree operations +edit @datamodel, cli_auto_edit("datamodel"); +up, cli_auto_up("datamodel"); +top, cli_auto_top("datamodel"); +set @datamodel, cli_auto_set(); +merge @datamodel, cli_auto_merge(); +create @datamodel, cli_auto_create(); +commit("Commit the changes"), cli_commit(); +delete("Delete a configuration item") { + @datamodel, cli_auto_del(); + all("Delete whole candidate configuration"), delete_all("candidate"); +} +show("Show a particular state of the system"){ + configuration("Show configuration"), cli_show_auto_mode("candidate", "xml", false, false); +} +EOF + +# Yang specs must be here first for backend. But then the specs are changed but just for CLI +# Annotate original Yang spec example directly +# First annotate /table/parameter +# Had a problem with unknown in grouping -> test uses uses/grouping +cat < $fyang +module example { + namespace "urn:example:clixon"; + prefix ex; + import clixon-autocli{ + prefix autocli; + } + import example-external{ + prefix ext; + } + grouping pg1 { + leaf value1{ + description "a value"; + type string; + } + list index1{ + key i; + leaf i{ + type string; + } + leaf iv{ + description "a value"; + type string; + } + } + } + container table{ + list parameter{ + key name; + leaf name{ + type string; + } + leaf value0{ + description "a value"; + type string; + } + uses pg1; + uses ext:pg2; + } + } + uses pg1; +} +EOF + +# Original no annotations for backend +cat < $fyang2 +module example-external { + namespace "urn:example:external"; + prefix ext; + import example-external3{ + prefix ext3; + } + grouping pg2 { + leaf value2{ + description "a value"; + type string; + } + container c2{ + uses ext3:pg3; + } + } +} +EOF + +# Original no annotations for backend +cat < $fyang3 +module example-external3 { + namespace "urn:example:external"; + prefix ext3; + grouping pg3 { + leaf value3{ + description "a value"; + type string; + } + } +} +EOF + +new "test params: -f $cfg" +if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -z -f $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg" + start_backend -s init -f $cfg +fi + +new "wait backend" +wait_backend + +# Test of generated clispecs. With autocli grouping_treeref, there should be treerefs for +# groupings. Without they should not be present +# The testcase assumes enabled +if ${grouping_treeref}; then + new "verify grouping is enabled" + expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" +else + new "verify grouping is disabled" + expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 --not-- "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" +fi + +new "set top-level grouping" +expectpart "$($clixon_cli -f $cfg -1 set value1 39)" 0 "" + +new "set inline grouping value" +expectpart "$($clixon_cli -f $cfg -1 set table parameter x value0 40)" 0 "" + +new "set grouping in same module" +expectpart "$($clixon_cli -f $cfg -1 set table parameter x value1 41)" 0 "" + +new "set list grouping in same module" +expectpart "$($clixon_cli -f $cfg -1 set table parameter x index1 a iv foo)" 0 "" + +new "set grouping in other module" +expectpart "$($clixon_cli -f $cfg -1 set table parameter x value2 42)" 0 "" + +new "set grouping in other+other module" +expectpart "$($clixon_cli -f $cfg -1 set table parameter x c2 value3 43)" 0 "" + +new "commit" +expectpart "$($clixon_cli -f $cfg -1 commit)" 0 "" + +new "show config" +expectpart "$($clixon_cli -f $cfg -1 show config)" 0 "x4041afoo4243
" "39" + +if [ $BE -ne 0 ]; then + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg +fi + +rm -rf $dir + +new "endtest" +endtest diff --git a/yang/clixon/clixon-autocli@2023-05-01.yang b/yang/clixon/clixon-autocli@2023-05-01.yang index ee2e9736..9c05806c 100644 --- a/yang/clixon/clixon-autocli@2023-05-01.yang +++ b/yang/clixon/clixon-autocli@2023-05-01.yang @@ -44,6 +44,7 @@ module clixon-autocli{ revision 2023-05-01 { description "Added extensions skip and alias + Added grouping-treeref Released in Clixon 6.3"; } revision 2022-02-11 { @@ -199,6 +200,21 @@ module clixon-autocli{ type boolean; default true; } + leaf grouping-treeref { + description + "Controls the behaviour when generating CLISPEC of YANG 'uses' statements into the + corresponding 'grouping' definition: macro expansion. + For optimization of memory footprint. + If 'false', replace the uses definition with the grouping definition. + If 'true' use indirect tree reference '@treeref' to reference the grouping definition. This + saves memory for large YANGs. + + See also AUTOCLI_GROUPING_TOPLEVEL_SKIP and AUTOCLI_GROUPING_AUGMENT_SKIP for + temporary disabled cornercases. + This option was introduced in Clixon 6.3"; + type boolean; + default false; + } /* rules */ list rule { description