From 6681eb99d3e1425aef91ac9c170d57ea4fbfd397 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 2 Feb 2023 16:42:00 +0100 Subject: [PATCH] Netconf monitoring statistics frm RFC 6022 --- CHANGELOG.md | 9 +- apps/backend/backend_client.c | 39 ++++--- apps/backend/backend_client.h | 1 + apps/backend/backend_get.c | 16 +-- apps/backend/backend_main.c | 2 + apps/backend/clixon_backend_handle.c | 1 + lib/clixon/clixon_netconf_monitoring.h | 2 + lib/src/clixon_data.c | 2 +- lib/src/clixon_netconf_monitoring.c | 134 +++++++++++++++++++++++-- lib/src/clixon_proto.c | 2 +- lib/src/clixon_xml.c | 2 +- test/test_netconf_monitoring.sh | 11 +- test/test_netconf_notifications.sh | 4 + 13 files changed, 186 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41dec11b..89162cf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,14 +51,15 @@ Expected: beginning of 2023 * New plugin callback: `ca_yang_mount` * Standards: RFC 8528 * To enable configure with `--enable-yang-schema-mount` -* Netconf monitoring RFC 6022 , part 2 - * Datastores and sessions +* Netconf monitoring RFC 6022 + * This is part 2, first part was in 6.0 + * Datastores, sessions and statistics * Added clixon-specific transport identities: cli, snmp, netconf, restconf - * Added source-host fro native restonf, but no other transports + * Added source-host from native restonf, but no other transports + * Hello statistics is based on backend statistics, hellos from RESTCONF, SNMP and CLI clients are included and dropped external NETCONF sessions are not * Standards * RFC 6022 "YANG Module for NETCONF Monitoring" * See [Feature Request: Support RFC 6022 (NETCONF Monitoring)](https://github.com/clicon/clixon/issues/370) - * Remaining: statistics state ### API changes on existing protocol/config features diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 871c0427..b2867873 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -32,7 +32,7 @@ the terms of any one of the Apache License version 2 or the GPL. ***** END LICENSE BLOCK ***** - + * note: there is also client code in clixon_backend_handle.c */ #ifdef HAVE_CONFIG_H @@ -117,6 +117,9 @@ ce_event_cb(clicon_handle h, } break; } + /* note there may be other notifications than RFC5277 streams */ + ce->ce_out_notifications++; + netconf_monitoring_counter_inc(h, "out-notifications"); } return 0; } @@ -218,7 +221,7 @@ backend_monitoring_state_get(clicon_handle h, cprintf(cb, "%u", ce->ce_in_rpcs); cprintf(cb, "%u", ce->ce_in_bad_rpcs); cprintf(cb, "%u", ce->ce_out_rpc_errors); - cprintf(cb, "%u", 0); + cprintf(cb, "%u", ce->ce_out_notifications); cprintf(cb, ""); } cprintf(cb, ""); @@ -263,7 +266,6 @@ backend_client_rm(clicon_handle h, clicon_err(OE_YANG, ENOENT, "No yang spec"); goto done; } - if (if_feature(yspec, "ietf-netconf", "confirmed-commit")) { if (confirmed_commit_state_get(h) == EPHEMERAL) { /* See if this client is the origin */ @@ -301,9 +303,8 @@ backend_client_rm(clicon_handle h, ce_prev = &c->ce_next; } retval = backend_client_delete(h, ce); /* actually purge it */ - - done: - return retval; + done: + return retval; } /*! Get clixon per datastore stats @@ -325,7 +326,6 @@ clixon_stats_datastore_get(clicon_handle h, cxobj *xn = NULL; /* This is the db cache */ - if ((xt = xmldb_cache_get(h, dbname)) == NULL){ /* Trigger cache if no exist */ if (xmldb_get(h, dbname, NULL, "/", &xn) < 0) @@ -1604,7 +1604,6 @@ from_client_msg(clicon_handle h, } if (strcmp(rpcname, "rpc") == 0){ - ce->ce_in_rpcs++; /* Track all RPCs */ } else if (strcmp(rpcname, "hello") == 0){ if ((ret = from_client_hello(h, x, ce, cbret)) <0) @@ -1614,23 +1613,31 @@ from_client_msg(clicon_handle h, else{ if (netconf_unknown_element(cbret, "protocol", rpcname, "Unrecognized netconf operation")< 0) goto done; - ce->ce_in_bad_rpcs++; + ce->ce_in_bad_rpcs++; ce->ce_out_rpc_errors++; /* Number of messages sent that contained an */ + netconf_monitoring_counter_inc(h, "in-bad-rpcs"); + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } /* As a side-effect, this expands xt with default values according to "report-all" * This may not be correct, the RFC does not mention expanding default values for * input RPC */ - if ((ret = xml_yang_validate_rpc(h, x, 1, &xret)) < 0) + if ((ret = xml_yang_validate_rpc(h, x, 1, &xret)) < 0){ + ce->ce_in_bad_rpcs++; + netconf_monitoring_counter_inc(h, "in-bad-rpcs"); goto done; + } if (ret == 0){ if (clixon_xml2cbuf(cbret, xret, 0, 0, -1, 0) < 0) goto done; ce->ce_in_bad_rpcs++; - ce->ce_in_rpcs--; /* Track all RPCs */ + netconf_monitoring_counter_inc(h, "in-bad-rpcs"); goto reply; } + ce->ce_in_rpcs++; /* Track all RPCs */ + netconf_monitoring_counter_inc(h, "in-rpcs"); + xe = NULL; username = xml_find_value(x, "username"); /* May be used by callbacks, etc */ @@ -1641,6 +1648,7 @@ from_client_msg(clicon_handle h, if (netconf_operation_not_supported(cbret, "protocol", rpc) < 0) goto done; ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } if ((ymod = ys_module(ye)) == NULL){ @@ -1669,6 +1677,7 @@ from_client_msg(clicon_handle h, goto done; if (ret == 0){ /* credentials fail */ ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } /* NACM rpc operation exec validation */ @@ -1676,6 +1685,7 @@ from_client_msg(clicon_handle h, goto done; if (ret == 0){ /* Not permitted and cbret set */ ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } } @@ -1685,16 +1695,19 @@ from_client_msg(clicon_handle h, goto done; clicon_log(LOG_NOTICE, "%s Error in rpc_callback_call:%s", __FUNCTION__, xml_name(xe)); ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; /* Dont quit here on user callbacks */ } if (ret == 0){ ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } if (nr == 0){ /* not handled by callback */ if (netconf_operation_not_supported(cbret, "application", "RPC operation not supported")< 0) goto done; ce->ce_out_rpc_errors++; + netconf_monitoring_counter_inc(h, "out-rpc-errors"); goto reply; } if (xnacm){ @@ -1775,8 +1788,10 @@ from_client(int s, } if (clicon_msg_rcv(ce->ce_s, &msg, &eof) < 0) goto done; - if (eof) + if (eof){ backend_client_rm(h, ce); + netconf_monitoring_counter_inc(h, "dropped-sessions"); + } else if (from_client_msg(h, ce, msg) < 0) goto done; diff --git a/apps/backend/backend_client.h b/apps/backend/backend_client.h index c9f2be9d..62a8ce75 100644 --- a/apps/backend/backend_client.h +++ b/apps/backend/backend_client.h @@ -62,6 +62,7 @@ struct client_entry{ uint32_t ce_in_rpcs ; /* Number of correct messages received. */ uint32_t ce_in_bad_rpcs; /* Not correct messages */ uint32_t ce_out_rpc_errors; /* messages*/ + uint32_t ce_out_notifications; /* Outgoing notifications */ }; /* diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c index 4938bd59..ef7e1270 100644 --- a/apps/backend/backend_get.c +++ b/apps/backend/backend_get.c @@ -121,9 +121,9 @@ restconf_client_get_capabilities(clicon_handle h, * @param[in] module Name of yang module * @param[in] top Top symbol, ie netconf or restconf-state * @param[in,out] xret Existing XML tree, merge x into this - * @retval -1 Error (fatal) - * @retval 0 Statedata callback failed * @retval 1 OK + * @retval 0 Statedata callback failed + * @retval -1 Error (fatal) */ static int client_get_streams(clicon_handle h, @@ -182,9 +182,9 @@ client_get_streams(clicon_handle h, * @param[in] nsc XML Namespace context for xpath * @param[in] wdef With-defaults parameter, see RFC 6243 * @param[in,out] xret Existing XML tree, merge x into this, or rpc-error - * @retval -1 Error (fatal) - * @retval 0 Statedata callback failed (error in xret) * @retval 1 OK + * @retval 0 Statedata callback failed (error in xret) + * @retval -1 Error (fatal) * @note This code in general does not look at xpath, needs to be filtered in retrospect * @note Awkward error handling. Even if most of this is during development phase, except for plugin * state callbacks. @@ -497,9 +497,9 @@ get_nacm_and_reply(clicon_handle h, * @param[in] defaultstr Default string which is accepted and sets value to 0 * @param[in,out] cbret Output buffer for internal bad-element RPC message if invalid * @param[out] value Value - * @retval -1 Error - * @retval 0 Invalid, netconf bad-element error cbret set * @retval 1 OK (or not found) + * @retval 0 Invalid, netconf bad-element error cbret set + * @retval -1 Error */ static int element2value(clicon_handle h, @@ -527,9 +527,9 @@ element2value(clicon_handle h, * @param[out] offset Number of entries in the working result-set that should be skipped * @param[out] limit Limits the number of entries returned from the working result-set * @param[out] cbret Return xml tree, eg ..., ce_id + 1); gettimeofday(&ce->ce_time, NULL); + netconf_monitoring_counter_inc(h, "in-sessions"); bh->bh_ce_list = ce; return ce; } diff --git a/lib/clixon/clixon_netconf_monitoring.h b/lib/clixon/clixon_netconf_monitoring.h index a32b232d..be1d36db 100644 --- a/lib/clixon/clixon_netconf_monitoring.h +++ b/lib/clixon/clixon_netconf_monitoring.h @@ -42,5 +42,7 @@ * Prototypes */ int netconf_monitoring_state_get(clicon_handle h, yang_stmt *yspec, char *xpath, cvec *nsc, cxobj **xret, cxobj **xerr); +int netconf_monitoring_statistics_init(clicon_handle h); +int netconf_monitoring_counter_inc(clicon_handle h, char *name); #endif /* _CLIXON_NETCONF_MONITORING_H_ */ diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c index 8e3c0916..37e3e55e 100644 --- a/lib/src/clixon_data.c +++ b/lib/src/clixon_data.c @@ -102,7 +102,7 @@ clicon_data_get(clicon_handle h, /*! Set generic clixon data on the form = where is string * @param[in] h Clicon handle * @param[in] name Data name - * @param[in] val Data value as null-terminated string + * @param[in] val Data value as null-terminated string (copied) * @retval 0 OK * @retval -1 Error * @see clicon_option_str_set diff --git a/lib/src/clixon_netconf_monitoring.c b/lib/src/clixon_netconf_monitoring.c index 87d8a841..a3a165a7 100644 --- a/lib/src/clixon_netconf_monitoring.c +++ b/lib/src/clixon_netconf_monitoring.c @@ -41,6 +41,7 @@ #include #include +#include #include #include #include @@ -62,6 +63,7 @@ #include "clixon_netconf_lib.h" #include "clixon_options.h" #include "clixon_err.h" +#include "clixon_data.h" #include "clixon_datastore.h" #include "clixon_netconf_monitoring.h" @@ -100,8 +102,8 @@ per_datastore(clicon_handle h, * @param[in] h Clicon handle * @param[in] yspec Yang spec * @param[in,out] cb CLIgen buffer - * @retval -1 Error (fatal) * @retval 0 OK + * @retval -1 Error (fatal) * @see RFC 6022 Section 2.1.2 */ static int @@ -131,8 +133,8 @@ netconf_monitoring_datastores(clicon_handle h, * @param[in] h Clicon handle * @param[in] yspec Yang spec * @param[in,out] cb CLIgen buffer - * @retval -1 Error (fatal) * @retval 0 OK + * @retval -1 Error (fatal) * @see RFC 6022 Section 2.1.3 */ static int @@ -182,20 +184,45 @@ netconf_monitoring_schemas(clicon_handle h, * @param[in] h Clicon handle * @param[in] yspec Yang spec * @param[in,out] cb CLIgen buffer - * @retval -1 Error (fatal) * @retval 0 OK + * @retval -1 Error (fatal) * @see RFC 6022 Section 2.1.5 - * XXX: NYI */ static int netconf_monitoring_statistics(clicon_handle h, yang_stmt *yspec, cbuf *cb) { - int retval = -1; - + int retval = -1; + char *str; + cvec *cvv = NULL; + cg_var *cv; + + cprintf(cb, ""); + if (clicon_data_get(h, "netconf-start-time", &str) == 0 && + str != NULL){ + cprintf(cb, "%s", str); + } + if ((cvv = clicon_data_cvec_get(h, "netconf-statistics")) == NULL) + goto ok; + if ((cv = cvec_find(cvv, "in-bad-hellos")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "in-sessions")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "dropped-sessions")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "in-rpcs")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "in-bad-rpcs")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "out-rpc-errors")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + if ((cv = cvec_find(cvv, "out-notifications")) != NULL) + cprintf(cb, "%u", cv_uint32_get(cv)); + cprintf(cb, ""); + ok: retval = 0; - //done: + // done: return retval; } @@ -209,9 +236,9 @@ netconf_monitoring_statistics(clicon_handle h, * @param[in] nsc XML Namespace context for xpath * @param[in,out] xret Existing XML tree, merge x into this * @param[out] xerr XML error tree, if retval = 0 - * @retval -1 Error (fatal) - * @retval 0 Statedata callback failed, error in xret * @retval 1 OK + * @retval 0 Statedata callback failed, error in xret + * @retval -1 Error (fatal) * @see backend_monitoring_state_get * @see RFC 6022 */ @@ -257,3 +284,92 @@ netconf_monitoring_state_get(clicon_handle h, retval = 0; goto done; } + +/*! Add RFC6022 empty counter32 with zero + * + * @param[in] cvv Cligen vector + * @param[in] name Name of new counter + */ +static int +stat_counter_add(cvec *cvv, + char *name) +{ + int retval = -1; + cg_var *cv; + + if ((cv = cvec_add(cvv, CGV_UINT32)) == NULL){ + clicon_err(OE_UNIX, errno, "cvec_add"); + goto done; + } + cv_name_set(cv, name); + cv_uint32_set(cv, 0); + retval = 0; + done: + return retval; +} + +/*! Init RFC6022 stats + * + * @param[in] h Clicon handle + */ +int +netconf_monitoring_statistics_init(clicon_handle h) +{ + int retval = -1; + struct timeval tv; + char timestr[28]; + cvec *cvv = NULL; + + gettimeofday(&tv, NULL); + if (time2str(tv, timestr, sizeof(timestr)) < 0) + goto done; + clicon_data_set(h, "netconf-start-time", timestr); /* RFC 6022 */ + if ((cvv = cvec_new(0)) == NULL){ + clicon_err(OE_UNIX, errno, "cvec_new"); + goto done; + } + if (clicon_data_cvec_set(h, "netconf-statistics", cvv) < 0) + goto done; + if (stat_counter_add(cvv, "in-bad-hellos") < 0) + goto done; + if (stat_counter_add(cvv, "in-sessions") < 0) + goto done; + if (stat_counter_add(cvv, "dropped-sessions") < 0) + goto done; + if (stat_counter_add(cvv, "in-rpcs") < 0) + goto done; + if (stat_counter_add(cvv, "in-bad-rpcs") < 0) + goto done; + if (stat_counter_add(cvv, "out-rpc-errors") < 0) + goto done; + if (stat_counter_add(cvv, "out-notifications") < 0) + goto done; + retval = 0; + done: + return retval; +} + +/*! Increment RFC6022 statistics counter + * + * @param[in] h Clixon handle + * @param[in] name Name of counter + */ +int +netconf_monitoring_counter_inc(clicon_handle h, + char *name) +{ + int retval = -1; + cvec *cvv = NULL; + cg_var *cv; + uint32_t u32; + + if ((cvv = clicon_data_cvec_get(h, "netconf-statistics")) != NULL){ + if ((cv = cvec_find(cvv, name)) != NULL){ + u32 = cv_uint32_get(cv); + u32++; + cv_uint32_set(cv, u32); + } + } + retval = 0; + return retval; +} diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index a02f9b1f..09e14898 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -768,7 +768,7 @@ send_msg_notify(int s, * @param[in] xev Event as XML * @retval 0 OK * @retval -1 Error - * @see send_msg_notify XXX beauty contest + * @see send_msg_notify */ int send_msg_notify_xml(clicon_handle h, diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index b22eca55..c72fd759 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -231,7 +231,7 @@ xml_type2str(enum cxobj_type type) return (char*)clicon_int2str(xsmap, type); } -/* Stats */ +/* Stats (too low-level to hang it on handle) */ static uint64_t _stats_xml_nr = 0; /*! Get global statistics about XML objects diff --git a/test/test_netconf_monitoring.sh b/test/test_netconf_monitoring.sh index 81d394de..4bdb84b6 100755 --- a/test/test_netconf_monitoring.sh +++ b/test/test_netconf_monitoring.sh @@ -68,9 +68,9 @@ new "wait backend" wait_backend new "Retrieving all state via operation" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:base:1.1.*urn:ietf:params:xml:ns:yang:ietf-netconf-monitoring.*candidaterunning.*.*" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:base:1.1.*urn:ietf:params:xml:ns:yang:ietf-netconf-monitoring.*candidaterunning.*.*.*" -# send multiple frames +# send multiple frames using locks for more interesting datastore stat rpc=$(chunked_framing "") rpc="${rpc} $(chunked_framing "candidate")" @@ -94,13 +94,18 @@ done # 4.1. Retrieving Schema List via Operation # match bith module and sub-module +# 2.1.3 new "Retrieving Schema List via Operation" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "clixon-example2022-01-01yangurn:example:clixonNETCONF.*clixon-sub2022-01-01yangurn:example:clixonNETCONF.*" -# Session +# Session 2.1.4 new "Retrieve Session" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "[1-9][0-9]*cl:netconf.*.*[0-9][0-9]*[0-9][0-9]*[0-9][0-9]*[0-9][0-9]*.*" +# Statistics 2.1.5 +new "Retrieve Statistics" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "20[0-9][0-9]\-[0-9][0-9]\-[0-9][0-9]T[0-9][0-9]:[0-9][0-9]:[0-9][0-9]\.[0-9]*Z[0-9]\+[1-9][0-9]*[0-9]\+[1-9][0-9]*[0-9]\+[0-9]\+[0-9]\+" + # 4.2. Retrieving Schema Instances # From 2b. bar, version 2008-06-1 in YANG format, via get-schema new "Retrieving clixon-example schema instance using id, version, format" diff --git a/test/test_netconf_notifications.sh b/test/test_netconf_notifications.sh index 5a8fafd2..5cea4864 100755 --- a/test/test_netconf_notifications.sh +++ b/test/test_netconf_notifications.sh @@ -42,6 +42,7 @@ cat < $cfg true streams 60 + true EOF @@ -117,6 +118,9 @@ expectwait "$clixon_netconf -D $DBG -qef $cfg" 0 "$DEFAULTHELLO" "EXAMPLE" "applicationbad-elementstartTimeerrorregexp match fail:" "" +new "out-notification statistics expect 1-3" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "[1-3]" + new "netconf EXAMPLE subscription with simple filter" expectwait "$clixon_netconf -D $DBG -qef $cfg" 0 "$DEFAULTHELLO" "EXAMPLE" $NCWAIT "" "20"