From e5b625e722c520635219495a19475b5dd94923c7 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 23 Jul 2017 12:59:02 +0200 Subject: [PATCH] Added support for yang presence and no-presence containers. --- CHANGELOG.md | 24 +++++-------- apps/backend/backend_client.c | 11 ++---- apps/cli/cli_show.c | 2 +- apps/netconf/netconf_plugin.c | 14 ++++++++ datastore/keyvalue/clixon_keyvalue.c | 2 +- datastore/text/clixon_xmldb_text.c | 50 ++++++++++++++++++++++++++++ doc/FAQ.md | 44 ++++++++++++++++++++++++ example/README.md | 7 ++++ example/routing_backend.c | 7 ++-- lib/clixon/clixon_yang.h | 2 +- lib/src/clixon_yang.c | 42 +++++++++++------------ test/README.md | 12 ++++--- test/test6.sh | 38 --------------------- test/{test1.sh => test_cli.sh} | 4 +-- test/{test5.sh => test_datastore.sh} | 0 test/{test7.sh => test_leafref.sh} | 0 test/{test2.sh => test_netconf.sh} | 15 +++++---- test/{test3.sh => test_restconf.sh} | 0 test/{test4.sh => test_yang.sh} | 18 ++++++++-- 19 files changed, 188 insertions(+), 104 deletions(-) delete mode 100755 test/test6.sh rename test/{test1.sh => test_cli.sh} (96%) rename test/{test5.sh => test_datastore.sh} (100%) rename test/{test7.sh => test_leafref.sh} (100%) rename test/{test2.sh => test_netconf.sh} (91%) rename test/{test3.sh => test_restconf.sh} (100%) rename test/{test4.sh => test_yang.sh} (80%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac27dac3..4127a252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,29 +1,23 @@ # Clixon CHANGELOG -- Extended ietf-ip example with ietf-routing. +Major changes: +- Added support for yang presence and no-presence containers. Previous default was "presence". -- Added YANG RPC support, with example rpc documentation and testcase (test7.sh). +- Added YANG RPC support for nentconf and CLI. With example rpc documentation and testcase. This replaces the previous "downcall" mechanism. -- Added completion for generated cli leafrefs for both absolute and relatve paths. - -- Added validation for leafref forward and backward references. +- Enhanced leafref functionality: (1) validation for leafref forward and backward references; (2)CLI completion for generated cli leafrefs for both absolute and relatve paths. -- Added new backend plugin callback: "plugin_statedata()" for retreiving state data +- Added state data: Netconf operation, new backend plugin callback: "plugin_statedata()" for retreiving state data. +Minor changes: +- Removed 'margin' parameter of yang_print(). +- Extended example with ietf-routing (not only ietf-ip) for rpc operations. - Added yang dir with ietf-netconf and clixon-config yang specs for internal usage. - -- Added state data: Netconf operation introduced; Error when - adding state data in . - - Fixed bug where cli set of leaf-list were doubled, eg cli set foo -> foofoo - - Restricted yang (sub)module file match to match RFC6020 exactly - -- Generalized yang type resolution to all included (sub)modules not just the topmost - - Generic map_str2int generic mapping tables - - Removed vector return values from xmldb_get() +- Generalized yang type resolution to all included (sub)modules not just the topmost ## 3.3.1 June 7 2017 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index b9548b49..9b7728bf 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -242,14 +242,9 @@ from_client_get_config(clicon_handle h, goto ok; } cprintf(cbret, ""); - if (xret!=NULL){ - if (xml_child_nr(xret)){ - if (xml_name_set(xret, "config") < 0) - goto done; - if (clicon_xml2cbuf(cbret, xret, 0, 0) < 0) - goto done; - } - } + if (xret!=NULL && + clicon_xml2cbuf(cbret, xret, 0, 0) < 0) + goto done; cprintf(cbret, ""); ok: retval = 0; diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index 134ee731..15e0ce0a 100644 --- a/apps/cli/cli_show.c +++ b/apps/cli/cli_show.c @@ -387,7 +387,7 @@ show_yang(clicon_handle h, } else yn = (yang_node*)yspec; - yang_print(stdout, yn, 0); + yang_print(stdout, yn); return 0; } int show_yangv(clicon_handle h, cvec *vars, cvec *argv) diff --git a/apps/netconf/netconf_plugin.c b/apps/netconf/netconf_plugin.c index c1ff19e1..a07606eb 100644 --- a/apps/netconf/netconf_plugin.c +++ b/apps/netconf/netconf_plugin.c @@ -238,6 +238,8 @@ netconf_plugin_callbacks(clicon_handle h, yang_spec *yspec; yang_stmt *yrpc; yang_stmt *yinput; + yang_stmt *youtput; + cxobj *xoutput; find_rpc_arg fra = {0,0}; int ret; @@ -279,6 +281,18 @@ netconf_plugin_callbacks(clicon_handle h, */ if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0) goto done; + /* Sanity check of outgoing XML */ + if ((youtput = yang_find((yang_node*)yrpc, Y_OUTPUT, NULL)) != NULL){ + xoutput=xpath_first(*xret, "/"); + xml_spec_set(xoutput, youtput); /* needed for xml_spec_populate */ + if (xml_apply(xoutput, CX_ELMNT, xml_spec_populate, yinput) < 0) + goto done; + if (xml_apply(xoutput, CX_ELMNT, + (xml_applyfn_t*)xml_yang_validate_all, NULL) < 0) + goto done; + if (xml_yang_validate_add(xoutput, NULL) < 0) + goto done; + } retval = 1; /* handled by callback */ goto done; } diff --git a/datastore/keyvalue/clixon_keyvalue.c b/datastore/keyvalue/clixon_keyvalue.c index 56b7196b..b15fe7ba 100644 --- a/datastore/keyvalue/clixon_keyvalue.c +++ b/datastore/keyvalue/clixon_keyvalue.c @@ -673,7 +673,7 @@ put(char *dbfile, clicon_debug(1, "%s xk0:%s ys:%s", __FUNCTION__, xk0, ys->ys_argument); if (debug){ xml_print(stderr, xt); - // yang_print(stderr, (yang_node*)ys, 0); + // yang_print(stderr, (yang_node*)ys); } if ((opstr = xml_find_value(xt, "operation")) != NULL) if (xml_operation(opstr, &op) < 0) diff --git a/datastore/text/clixon_xmldb_text.c b/datastore/text/clixon_xmldb_text.c index a6357508..787a32fb 100644 --- a/datastore/text/clixon_xmldb_text.c +++ b/datastore/text/clixon_xmldb_text.c @@ -665,6 +665,50 @@ text_modify_top(cxobj *x0, return retval; } +/*! For containers without presence and no children, remove + * @param[in] x XML tree node + * @note This should really be unnecessary since yspec should be set on creation + * @code + * xml_apply(xc, CX_ELMNT, xml_spec_populate, yspec) + * @endcode + * See section 7.5.1 in rfc6020bis-02.txt: + * No presence: + * those that exist only for organizing the hierarchy of data nodes: + * the container has no meaning of its own, existing + * only to contain child nodes. This is the default style. + * (Remove these if no children) + * Presence: + * the presence of the container itself is + * configuration data, representing a single bit of configuration data. + * The container acts as both a configuration knob and a means of + * organizing related configuration. These containers are explicitly + * created and deleted. + * (Dont touch these) + */ +int +xml_container_presence(cxobj *x, + void *arg) +{ + int retval = -1; + char *name; + yang_stmt *y; /* yang node */ + + name = xml_name(x); + if ((y = (yang_stmt*)xml_spec(x)) == NULL){ + clicon_log(LOG_WARNING, "%s: no xml_spec(%s)", __FUNCTION__, name); + retval = 0; + goto done; + } + /* Mark node that is: container, have no children, dont have presence */ + if (y->ys_keyword == Y_CONTAINER && + xml_child_nr(x)==0 && + yang_find((yang_node*)y, Y_PRESENCE, NULL) == NULL) + xml_flag_set(x, XML_FLAG_MARK); /* Mark, remove later */ + retval = 0; + done: + return retval; +} + /*! Modify database provided an xml tree and an operation * This is a clixon datastore plugin of the the xmldb api * @see xmldb_put @@ -745,6 +789,12 @@ text_put(xmldb_handle xh, if (xml_apply(x0, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_NONE) < 0) goto done; + /* Mark non-presence containers that do not have children */ + if (xml_apply(x0, CX_ELMNT, (xml_applyfn_t*)xml_container_presence, NULL) < 0) + goto done; + /* Remove (prune) nodes that are marked (non-presence containers w/o children) */ + if (xml_tree_prune_flagged(x0, XML_FLAG_MARK, 1) < 0) + goto done; // output: /* Print out top-level xml tree after modification to file */ if ((cb = cbuf_new()) == NULL){ diff --git a/doc/FAQ.md b/doc/FAQ.md index a0cd18a9..92269257 100644 --- a/doc/FAQ.md +++ b/doc/FAQ.md @@ -197,8 +197,10 @@ They are documented in [CLIgen tutorial](https://github.com/olofhagsand/cligen/b ## How do I write a validation function? Similar to a commit function, but instead write the transaction_validate() function. Check for inconsistencies in the XML trees and if they fail, make an clicon_err() call. +``` clicon_err(OE_PLUGIN, 0, "Route %s lacks ipv4 addr", name); return -1; +``` The validation or commit will then be aborted. ## How do I write a state data callback function? @@ -210,3 +212,45 @@ To return state data, you need to write a backend state data callback with the name "plugin_statedata()" where you return an XML tree. Please look at the example for an example on how to write a state data callback. + +## How do I write an RPC function? + +A YANG RPC is an application specific operation. Example: +``` + rpc fib-route { + input { + leaf inarg { type string; } + } + output { + leaf outarg { type string; } + } + } +``` +which defines the fib-route operation present in the example (the arguments have been changed). + +Clixon automatically relays the RPC to the clixon backend. To +implement the RFC, you need to register an RPC callback in the backend plugin: +Example: +``` +int +plugin_init(clicon_handle h) +{ +... + backend_rpc_cb_register(h, fib_route, NULL, "fib-route"); +... +} +``` +And then define the callback itself: +``` +static int +fib_route(clicon_handle h, /* Clicon handle */ + cxobj *xe, /* Request: */ + struct client_entry *ce, /* Client session */ + cbuf *cbret, /* Reply eg ... */ + void *arg) /* Argument given at register */ +{ + cprintf(cbret, ""); + return 0; +} +``` +Here, the callback is over-simplified. \ No newline at end of file diff --git a/example/README.md b/example/README.md index 86806df6..31fa83a7 100644 --- a/example/README.md +++ b/example/README.md @@ -103,6 +103,13 @@ fib_route(clicon_handle h, cprintf(cbret, ""); return 0; } +int +plugin_init(clicon_handle h) +{ +... + backend_rpc_cb_register(h, fib_route, NULL, "fib-route"); +... +} ``` ## State data diff --git a/example/routing_backend.c b/example/routing_backend.c index e65bbd0d..d5bbc1f3 100644 --- a/example/routing_backend.c +++ b/example/routing_backend.c @@ -121,13 +121,16 @@ notification_timer_setup(clicon_handle h) /*! IETF Routing fib-route rpc */ static int -fib_route(clicon_handle h, +fib_route(clicon_handle h, /* Clicon handle */ cxobj *xe, /* Request: */ struct client_entry *ce, /* Client session */ cbuf *cbret, /* Reply eg ... */ void *arg) /* Argument given at register */ { - cprintf(cbret, ""); + cprintf(cbret, "" + "ipv4" + "2.3.4.5" + ""); return 0; } diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index 408567b9..fc22bb00 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -204,8 +204,8 @@ yang_stmt *yang_find(yang_node *yn, int keyword, char *argument); yang_stmt *yang_find_syntax(yang_node *yn, char *argument); yang_stmt *yang_find_topnode(yang_spec *ysp, char *name); +int yang_print(FILE *f, yang_node *yn); int yang_print_cbuf(cbuf *cb, yang_node *yn, int marginal); -int yang_print(FILE *f, yang_node *yn, int marginal); int yang_parse(clicon_handle h, const char *yang_dir, const char *module, const char *revision, yang_spec *ysp); int yang_apply(yang_node *yn, enum rfc_6020 key, yang_applyfn_t fn, diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 16576bb6..d8a0e940 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -682,43 +682,41 @@ quotedstring(char *s) } /*! Print yang specification to file + * @param[in] f File to print to. + * @param[in] yn Yang node to print * @see yang_print_cbuf */ int yang_print(FILE *f, - yang_node *yn, - int marginal) + yang_node *yn) { - yang_stmt *ys = NULL; + int retval = -1; + cbuf *cb = NULL; - while ((ys = yn_each(yn, ys)) != NULL) { - fprintf(f, "%*s%s", marginal, "", yang_key2str(ys->ys_keyword)); - fflush(f); - if (ys->ys_argument){ - if (quotedstring(ys->ys_argument)) - fprintf(f, " \"%s\"", ys->ys_argument); - else - fprintf(f, " %s", ys->ys_argument); - } - if (ys->ys_len){ - fprintf(f, " {\n"); - yang_print(f, (yang_node*)ys, marginal+3); - fprintf(f, "%*s%s\n", marginal, "", "}"); - } - else - fprintf(f, ";\n"); + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_YANG, errno, "%s: cbuf_new", __FUNCTION__); + goto done; } - return 0; + if (yang_print_cbuf(cb, yn, 0) < 0) + goto done; + fprintf(f, "%s", cbuf_get(cb)); + if (cb) + cbuf_free(cb); + retval = 0; + done: + return retval; } /*! Print yang specification to cligen buf + * @param[in] cb Cligen buffer. This is where the pretty print is. + * @param[in] yn Yang node to print + * @param[in] marginal Tab indentation, mainly for recursion. * @code * cbuf *cb = cbuf_new(); * yang_print_cbuf(cb, yn, 0); * // output is in cbuf_buf(cb); * cbuf_free(cb); * @endcode - * @see yang_print */ int yang_print_cbuf(cbuf *cb, @@ -1944,7 +1942,7 @@ yang_spec_main(clicon_handle h, goto done; clicon_dbspec_yang_set(h, yspec); if (printspec) - yang_print(f, (yang_node*)yspec, 0); + yang_print(f, (yang_node*)yspec); retval = 0; done: return retval; diff --git a/test/README.md b/test/README.md index d58863c2..7d078fde 100644 --- a/test/README.md +++ b/test/README.md @@ -4,8 +4,10 @@ This directory contains testing code for clixon and the example routing application: - clixon A top-level script clones clixon in /tmp and starts all.sh. You can copy this file (review it first) and place as cron script - all.sh Run through all tests named 'test*.sh' in this directory. Therefore, if you place a test in this directory matching 'test*.sh' it will be run automatically. -- test1.sh CLI tests -- test2.sh Netconf tests -- test3.sh Restconf tests -- test4.sh Yang tests -- test5.sh Datastore tests +- test_cli.sh CLI tests +- test_netconf.sh Netconf tests +- test_restconf.sh Restconf tests +- test_yang.sh Yang tests for constructs not in the example. +- test_leafref.sh Yang leafref tests +- test_datastore.sh Datastore tests + diff --git a/test/test6.sh b/test/test6.sh deleted file mode 100755 index e3cf7b07..00000000 --- a/test/test6.sh +++ /dev/null @@ -1,38 +0,0 @@ -#!/bin/bash -# Test6: Yang specifics: rpc and state info - -# include err() and new() functions -. ./lib.sh - -# For memcheck -# clixon_netconf="valgrind --leak-check=full --show-leak-kinds=all clixon_netconf" -clixon_netconf=clixon_netconf -clixon_cli=clixon_cli - -# kill old backend (if any) -new "kill old backend" -sudo clixon_backend -zf $clixon_cf -if [ $? -ne 0 ]; then - err -fi - -new "start backend" -# start new backend -sudo clixon_backend -If $clixon_cf -if [ $? -ne 0 ]; then - err -fi -new "netconf rpc" -expecteof "$clixon_netconf -qf $clixon_cf" "ipv4]]>]]>" "^]]>]]>$" - -new "Kill backend" -# Check if still alive -pid=`pgrep clixon_backend` -if [ -z "$pid" ]; then - err "backend already dead" -fi -# kill backend -sudo clixon_backend -zf $clixon_cf -if [ $? -ne 0 ]; then - err "kill backend" -fi diff --git a/test/test1.sh b/test/test_cli.sh similarity index 96% rename from test/test1.sh rename to test/test_cli.sh index d60031ed..8931b045 100755 --- a/test/test1.sh +++ b/test/test_cli.sh @@ -31,8 +31,8 @@ new "cli tests" new "cli configure top" expectfn "$clixon_cli -1f $clixon_cf set interfaces" "" -new "cli show configuration top" -expectfn "$clixon_cli -1f $clixon_cf show conf cli" "^interfaces$" +new "cli show configuration top (no presence)" +expectfn "$clixon_cli -1f $clixon_cf show conf cli" "" new "cli configure delete top" expectfn "$clixon_cli -1f $clixon_cf delete interfaces" "" diff --git a/test/test5.sh b/test/test_datastore.sh similarity index 100% rename from test/test5.sh rename to test/test_datastore.sh diff --git a/test/test7.sh b/test/test_leafref.sh similarity index 100% rename from test/test7.sh rename to test/test_leafref.sh diff --git a/test/test2.sh b/test/test_netconf.sh similarity index 91% rename from test/test2.sh rename to test/test_netconf.sh index 9af012db..a3f9c0ef 100755 --- a/test/test2.sh +++ b/test/test_netconf.sh @@ -24,13 +24,13 @@ fi new "netconf tests" new "netconf get empty config" -expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' +expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' new "Add subtree eth/0/0 using none which should not change anything" expecteof "$clixon_netconf -qf $clixon_cf" "noneeth/0/0]]>]]>" "^]]>]]>$" new "Check nothing added" -expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' +expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' new "Add subtree eth/0/0 using none and create which should add eth/0/0" expecteof "$clixon_netconf -qf $clixon_cf" 'eth/0/0ethnone ]]>]]>' "^]]>]]>$" @@ -44,8 +44,8 @@ expecteof "$clixon_netconf -qf $clixon_cf" 'eth/0/0ethnone ]]>]]>' "^]]>]]>$" -#new "Check deleted eth/0/0" -#expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' +new "Check deleted eth/0/0 (non-presence container)" +expecteof "$clixon_netconf -qf $clixon_cf" ']]>]]>' '^]]>]]>$' new "Re-Delete eth/0/0 using none should generate error" expecteof "$clixon_netconf -qf $clixon_cf" 'eth/0/0ethnone ]]>]]>' "^" @@ -66,7 +66,7 @@ new "netconf discard-changes" expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" new "netconf get empty config2" -expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" +expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" new "netconf edit config eth1" expecteof "$clixon_netconf -qf $clixon_cf" "eth1eth]]>]]>" "^]]>]]>$" @@ -117,7 +117,10 @@ new "netconf delete startup" expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" new "netconf check empty startup" -expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" +expecteof "$clixon_netconf -qf $clixon_cf" "]]>]]>" "^]]>]]>$" + +new "netconf rpc" +expecteof "$clixon_netconf -qf $clixon_cf" "ipv4ipv4]]>]]>" "^ipv4" new "netconf subscription" expectwait "$clixon_netconf -qf $clixon_cf" "ROUTING]]>]]>" "^]]>]]>Routing notification]]>]]>$" 30 diff --git a/test/test3.sh b/test/test_restconf.sh similarity index 100% rename from test/test3.sh rename to test/test_restconf.sh diff --git a/test/test4.sh b/test/test_yang.sh similarity index 80% rename from test/test4.sh rename to test/test_yang.sh index 78121d3f..e01c09b2 100755 --- a/test/test4.sh +++ b/test/test_yang.sh @@ -35,7 +35,15 @@ module example{ leaf g { type string; } - container h { + container nopresence { + description "No presence should be removed if no children"; + leaf j { + type string; + } + } + container presence { + description "Presence should not be removed even if no children"; + presence "even if empty should remain"; leaf j { type string; } @@ -81,7 +89,7 @@ expecteof "$clixon_netconf -qf $clixon_cf -y /tmp/test" "]]>]]>" "^hejhopp]]>]]>$" -new "netconf get (state data XXX should be some)" +new "netconf get (should be some)" expecteof "$clixon_netconf -qf $clixon_cf -y /tmp/test" "]]>]]>" "^125]]>]]>$" new "cli set leaf-list" @@ -89,10 +97,14 @@ expectfn "$clixon_cli -1f $clixon_cf -y /tmp/test set x f e foo" "" new "cli show leaf-list" expectfn "$clixon_cli -1f $clixon_cf -y /tmp/test show xpath /x/f/e" "foo" - new "netconf set state data (not allowed)" expecteof "$clixon_netconf -qf $clixon_cf -y /tmp/test" "42]]>]]>" "^invalid-value" +new "netconf set presence and not present" +expecteof "$clixon_netconf -qf $clixon_cf -y /tmp/test" "]]>]]>" "^]]>]]>$" + +new "netconf get" +expecteof "$clixon_netconf -qf $clixon_cf -y /tmp/test" "]]>]]>" "^]]>]]>$" new "Kill backend" # Check if still alive