diff --git a/CHANGELOG.md b/CHANGELOG.md index ae3f3961..d7456b4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ ## 7.3.0 Expected: January 2025 +### Corrected Bugs + +* Fixed: [SNMP: snmpwalk is slow and can timeout](https://github.com/clicon/clixon/issues/404) + ## 7.2.0 28 October 2024 @@ -303,7 +307,7 @@ Users may have to change how they access the system ### C/CLI-API changes on existing features Developers may need to change their code - +p * Changed return value of `xml_add_attr` from 0/-1 to xa/NULL * You need to change eg `if (xml_add_attr < 0)` to if (xml_add_attr == NULL)` * Changed signature of `clicon_netconf_error()` and `netconf_err2cb()` diff --git a/apps/snmp/snmp_handler.c b/apps/snmp/snmp_handler.c index 34fce207..218f7997 100644 --- a/apps/snmp/snmp_handler.c +++ b/apps/snmp/snmp_handler.c @@ -60,6 +60,12 @@ #include "snmp_register.h" #include "snmp_handler.h" +struct snmp_getnext_cache { + cxobj *sg_xml; + char *sg_xpath; + struct timeval sg_timer; +}; + /*! Common code for handling incoming SNMP request * * Get clixon handle from snmp request, print debug data @@ -1143,6 +1149,86 @@ snmp_table_set(clixon_handle h, goto done; } + + +/*! Use a cache for getnext tables instead of an RPC to the backend every time + * + * The cache works as follows: + * It has three parts: + * - xpath Of the requested YANG + * - xml tree Saved from previous rpc to this xpath + * - timestamp Time of last rpc call for this xpath + * On a new call, the cache is used if: + * - the cache exist, and + * - the xpath is the same as the requested xpath, and + * - the age of the cache entry is less than a threshold + * @param[in] h Clixon handle + * @param[in] xpath XPath of requetsed YANG + * @param[in] nsc Namespace context + * @param[out] xt XML, either cached or new, dont free + * @retval 0 OK + * @retval -1 Error + */ +#define GETNEXT_THRESHOLD_US 1000000 +static int +table_getnext_cache(clixon_handle h, + char *xpath, + cvec *nsc, + cxobj **xtp) +{ + int retval = -1; + cxobj *xerr; + cxobj *xt = NULL; + int64_t tdiff_us = 0; + struct timeval now; + struct timeval td; + struct snmp_getnext_cache *sg = NULL; + + clicon_ptr_get(h, "snmp-getnext-cache", (void**)&sg); + if (sg == NULL){ + if ((sg = calloc(1, sizeof(*sg))) == NULL){ + clixon_err(OE_UNIX, errno, "calloc"); + goto done; + } + clicon_ptr_set(h, "snmp-getnext-cache", sg); + } + if (timerisset(&sg->sg_timer)){ + gettimeofday(&now, NULL); + timersub(&now, &sg->sg_timer, &td); + tdiff_us = 1000000*td.tv_sec + td.tv_usec; + } + if (sg->sg_xml == NULL || sg->sg_xpath == NULL || + strcmp(xpath, sg->sg_xpath) != 0 || tdiff_us > 1000000){ // gettime + if (sg->sg_xml){ + xml_free(sg->sg_xml); + sg->sg_xml = NULL; + } + if (sg->sg_xpath){ + free(sg->sg_xpath); + sg->sg_xpath = NULL; + } + if (clicon_rpc_get(h, xpath, nsc, CONTENT_ALL, -1, NULL, &xt) < 0) + goto done; + if ((xerr = xpath_first(xt, NULL, "/rpc-error")) != NULL){ + clixon_err_netconf(h, OE_NETCONF, 0, xerr, "Get configuration"); + goto done; + } + gettimeofday(&sg->sg_timer, NULL); + if ((sg->sg_xpath = strdup(xpath)) == NULL){ + clixon_err(OE_UNIX, errno, "strdup"); + goto done; + } + sg->sg_xml = xt; + xt = NULL; + } + *xtp = sg->sg_xml; + retval = 0; + done: + if (xt) + xml_free(xt); + return retval; +} + /*! Find "next" object from oids minus key and return that. * * @param[in] h Clixon handle @@ -1168,7 +1254,6 @@ snmp_table_getnext(clixon_handle h, cvec *nsc = NULL; char *xpath = NULL; cxobj *xt = NULL; - cxobj *xerr; cxobj *xtable; cxobj *xrow; cxobj *xcol; @@ -1197,12 +1282,9 @@ snmp_table_getnext(clixon_handle h, goto done; if (snmp_yang2xpath(ys, NULL, &xpath) < 0) goto done; - if (clicon_rpc_get(h, xpath, nsc, CONTENT_ALL, -1, NULL, &xt) < 0) - goto done; - if ((xerr = xpath_first(xt, NULL, "/rpc-error")) != NULL){ - clixon_err_netconf(h, OE_NETCONF, 0, xerr, "Get configuration"); + /* Get next via cache */ + if (table_getnext_cache(h, xpath, nsc, &xt) < 0) goto done; - } if ((xtable = xpath_first(xt, nsc, "%s", xpath)) != NULL) { /* Make a clone of key-list, but replace names with values */ if ((cvk_name = yang_cvec_get(ylist)) == NULL){ @@ -1253,12 +1335,10 @@ snmp_table_getnext(clixon_handle h, } retval = found; done: - if (cb) - cbuf_free(cb); if (xpath) free(xpath); - if (xt) - xml_free(xt); + if (cb) + cbuf_free(cb); if (nsc) xml_nsctx_free(nsc); return retval; @@ -1423,3 +1503,20 @@ clixon_snmp_table_handler(netsnmp_mib_handler *handler, done: return retval; } + +/*! Clear cache + */ +int +clixon_snmp_table_exit(clixon_handle h) +{ + struct snmp_getnext_cache *sg = NULL; + + if (clicon_ptr_get(h, "snmp-getnext-cache", (void**)&sg) == 0 && sg){ + if (sg->sg_xml) + xml_free(sg->sg_xml); + if (sg->sg_xpath) + free(sg->sg_xpath); + free(sg); + } + return 0; +} diff --git a/apps/snmp/snmp_handler.h b/apps/snmp/snmp_handler.h index d5924bcd..49f08e48 100644 --- a/apps/snmp/snmp_handler.h +++ b/apps/snmp/snmp_handler.h @@ -52,6 +52,7 @@ int clixon_snmp_scalar_handler(netsnmp_mib_handler *handler, netsnmp_handler_registration *nhreg, netsnmp_agent_request_info *reqinfo, netsnmp_request_info *requests); +int clixon_snmp_table_exit(clixon_handle h); #endif /* _SNMP_HANDLER_H_ */ diff --git a/apps/snmp/snmp_main.c b/apps/snmp/snmp_main.c index efb8fd73..f21e6017 100644 --- a/apps/snmp/snmp_main.c +++ b/apps/snmp/snmp_main.c @@ -69,6 +69,7 @@ #include "snmp_lib.h" #include "snmp_register.h" #include "snmp_stream.h" +#include "snmp_handler.h" /* Command line options to be passed to getopt(3) */ #define SNMP_OPTS "hVD:f:l:C:o:z" @@ -120,6 +121,7 @@ snmp_terminate(clixon_handle h) xml_free(x); x = NULL; } + clixon_snmp_table_exit(h); clicon_rpc_close_session(h); yang_exit(h); if ((nsctx = clicon_nsctx_global_get(h)) != NULL) diff --git a/docker/test/Makefile.in b/docker/test/Makefile.in index 4a4bbce2..1a70ccb7 100644 --- a/docker/test/Makefile.in +++ b/docker/test/Makefile.in @@ -93,12 +93,12 @@ test: docker mem: docker ./cleanup.sh ; ./start.sh # kill (ignore error) and the start it - sudo docker exec -t clixon-test bash -c 'sudo apk add --update valgrind; cd /usr/local/bin/test && ./mem.sh' + sudo docker exec -t clixon-test bash -c 'sudo apk add --update valgrind valgrind-scripts; cd /usr/local/bin/test && ./mem.sh' # Special-purpose memory test for snmp memsnmp: docker ./cleanup.sh ; ./start.sh # kill (ignore error) and the start it - sudo docker exec -t clixon-test bash -c 'sudo apk add --update valgrind; cd /usr/local/bin/test && pattern=test_snmp*.sh ./mem.sh snmp' + sudo docker exec -t clixon-test bash -c 'sudo apk add --update valgrind valgrind-scripts; cd /usr/local/bin/test && pattern=test_snmp*.sh ./mem.sh snmp' depend: