From 6063d9a147b157938caaa7300048cb13ff6a90ce Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 27 Sep 2022 14:37:36 +0200 Subject: [PATCH] Fixed: [SNMP smiv2 yang extension doesn't work on augmented nodes](https://github.com/clicon/clixon/issues/366) --- CHANGELOG.md | 4 + apps/snmp/snmp_handler.c | 12 ++- apps/snmp/snmp_lib.h | 6 +- apps/snmp/snmp_register.c | 177 +++++++++++++++++++++++++++++++------- test/test_snmp_ifmib.sh | 8 +- 5 files changed, 164 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97bc23bb..14a3da29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ ## 6.0.0 Expected: November 2022 +### Corrected Bugs + +* Fixed: [SNMP "smiv2" yang extension doesn't work on augmented nodes](https://github.com/clicon/clixon/issues/366) + ## 5.9.0 24 September 2022 diff --git a/apps/snmp/snmp_handler.c b/apps/snmp/snmp_handler.c index f6e4a438..c113214d 100644 --- a/apps/snmp/snmp_handler.c +++ b/apps/snmp/snmp_handler.c @@ -805,6 +805,8 @@ clixon_snmp_scalar_handler(netsnmp_mib_handler *handler, * Query clixon if object exists, if so return value * @param[in] h Clixon handle * @param[in] yt Yang of table (of list type) + * @param[in] oidt OID of registered top container object (above list), may be different from yt + * @param[in] oidtlen OID length of list object OID * @param[in] oids OID of ultimate scalar value * @param[in] oidslen OID length of scalar * @param[in] reqinfo Agent transaction request structure @@ -816,14 +818,14 @@ clixon_snmp_scalar_handler(netsnmp_mib_handler *handler, static int snmp_table_get(clicon_handle h, yang_stmt *yt, + oid *oidt, + size_t oidtlen, oid *oids, size_t oidslen, netsnmp_agent_request_info *reqinfo, netsnmp_request_info *request) { int retval = -1; - oid oidt[MAX_OID_LEN] = {0,}; /* Table / list oid */ - size_t oidtlen = MAX_OID_LEN; oid oidleaf[MAX_OID_LEN] = {0,}; /* Leaf */ size_t oidleaflen = MAX_OID_LEN; oid *oidi; @@ -838,11 +840,6 @@ snmp_table_get(clicon_handle h, char *defaultval = NULL; int ret; - /* Get OID from table /list */ - if ((ret = yangext_oid_get(yt, oidt, &oidtlen, NULL)) < 0) - goto done; - if (ret == 0) - goto done; /* Get yang of leaf from first part of OID */ ys = NULL; while ((ys = yn_each(yt, ys)) != NULL) { @@ -1259,6 +1256,7 @@ clixon_snmp_table_handler1(netsnmp_mib_handler *handler, /* Create xpath from YANG table OID + 1 + n + cvk/key = requestvb->name */ if ((ret = snmp_table_get(sh->sh_h, sh->sh_ys, + sh->sh_oid2, sh->sh_oid2len, requestvb->name, requestvb->name_length, reqinfo, request)) < 0) goto done; diff --git a/apps/snmp/snmp_lib.h b/apps/snmp/snmp_lib.h index 5a96938c..ca5a286c 100644 --- a/apps/snmp/snmp_lib.h +++ b/apps/snmp/snmp_lib.h @@ -67,8 +67,10 @@ extern "C" { struct clixon_snmp_handle { clicon_handle sh_h; yang_stmt *sh_ys; /* Leaf for scalar, list for table */ - oid sh_oid[MAX_OID_LEN]; /* OID for debug, may be removed? */ - size_t sh_oidlen; + oid sh_oid[MAX_OID_LEN]; /* OID of registered table (list) */ + size_t sh_oidlen; + oid sh_oid2[MAX_OID_LEN]; /* OID of registered container */ + size_t sh_oid2len; char *sh_default; /* MIB default value leaf only */ cvec *sh_cvk_orig; /* Index/Key variable values (original) */ netsnmp_table_registration_info *sh_table_info; /* To mimic table-handler in libnetsnmp code diff --git a/apps/snmp/snmp_register.c b/apps/snmp/snmp_register.c index 2031c5c4..340cfa56 100644 --- a/apps/snmp/snmp_register.c +++ b/apps/snmp/snmp_register.c @@ -200,33 +200,33 @@ mibyang_leaf_register(clicon_handle h, return retval; } -/*! Register table entry handler itself (not column/row leafs) +/*! Register table entry handler itself (not column/row leafs) from list or augment * * Parse smiv2 extensions for YANG container/list * - * Typical table: - * container x { - * smiv2:oid "1.3.6.1.4.1.8072.2.2.1"; - * list y{ - * - * } - * } - * @param[in] h Clixon handle - * @param[in] ys Mib-Yang node (container) - * @param[in] yl Mib-Yang node (list) - * @retval 0 OK - * @retval -1 Error + * Typical table: + * container x { + * smiv2:oid "1.3.6.1.4.1.8072.2.2.1"; + * list y{ + * + * } + * } + * @param[in] h Clixon handle + * @param[in] ylist Mib-Yang node (list) + * @retval 0 OK + * @retval -1 Error */ static int mibyang_table_register(clicon_handle h, - yang_stmt *ylist) + yang_stmt *ylist, + oid *oid1, + size_t oid1len, + oid *oid2, + size_t oid2len, + char *oidstr) { int retval = -1; netsnmp_handler_registration *nhreg; - char *oidstr = NULL; - oid oid1[MAX_OID_LEN] = {0,}; - size_t oid1len = MAX_OID_LEN; - char *name; clixon_snmp_handle *sh; int ret; netsnmp_mib_handler *handler; @@ -237,19 +237,19 @@ mibyang_table_register(clicon_handle h, yang_stmt *yleaf; int asn1type; yang_stmt *ys; + char *name; if ((ys = yang_parent_get(ylist)) == NULL || yang_keyword_get(ys) != Y_CONTAINER){ clicon_err(OE_YANG, EINVAL, "ylist parent is not list"); goto done; } - /* Get OID from parent container */ - if ((ret = yangext_oid_get(ys, oid1, &oid1len, &oidstr)) < 0) - goto done; - if (ret == 0) - goto ok; - name = yang_argument_get(ys); - + /* Note: This is wrong for augmented nodes where name is the original list, not the + * augmented. For example, for IFMIB you get ifTable twice where you should get ifTable for + * the original and ifXTable for the augmented. + * But the name does not seem to have semantic significance, so I leave it as is. + */ + name = yang_argument_get(ys); /* Userdata to pass around in netsmp callbacks * XXX: not deallocated */ @@ -260,9 +260,10 @@ mibyang_table_register(clicon_handle h, memset(sh, 0, sizeof(*sh)); sh->sh_h = h; sh->sh_ys = ylist; - - memcpy(sh->sh_oid, oid1, sizeof(oid1)); + memcpy(sh->sh_oid, oid1, oid1len*sizeof(*oid1)); sh->sh_oidlen = oid1len; + memcpy(sh->sh_oid2, oid2, sizeof(*oid2)*oid2len); + sh->sh_oid2len = oid2len; if ((handler = netsnmp_create_handler(name, clixon_snmp_table_handler)) == NULL){ clicon_err(OE_XML, errno, "netsnmp_create_handler"); @@ -333,6 +334,117 @@ mibyang_table_register(clicon_handle h, return retval; } +/*! Register table entry handler from YANG list + * + * Parse smiv2 extensions for YANG container/list. For example: + * container x { + * smiv2:oid "1.3.6.1.4.1.8072.2.2.1"; + * list ylist{ + * + * } + * } + * @param[in] h Clixon handle + * @param[in] ylist Mib-Yang node (list) + * @retval 0 OK + * @retval -1 Error + * @see mibyang_augment_register + */ +static int +mibyang_list_register(clicon_handle h, + yang_stmt *ylist) +{ + int retval = -1; + yang_stmt *yc; + char *oidstr = NULL; + oid oid1[MAX_OID_LEN] = {0,}; + size_t oid1len = MAX_OID_LEN; + oid oid2[MAX_OID_LEN] = {0,}; + size_t oid2len = MAX_OID_LEN; + int ret; + + if ((yc = yang_parent_get(ylist)) == NULL || + yang_keyword_get(yc) != Y_CONTAINER){ + clicon_err(OE_YANG, EINVAL, "ylist parent is not container"); + goto done; + } + if ((ret = yangext_oid_get(ylist, oid2, &oid2len, NULL)) < 0) + goto done; + if (ret == 0) + goto ok; + if ((ret = yangext_oid_get(yc, oid1, &oid1len, &oidstr)) < 0) + goto done; + if (ret == 0) + goto ok; + if (mibyang_table_register(h, ylist, + oid1, oid1len, + oid2, oid2len, + oidstr) < 0) + goto done; + ok: + retval = 0; + done: + return retval; +} + +/*! Register table entry handler from YANG augment + * + * Difference from register a list is that the OIDs are taken from the augment statement + * Parse smiv2 extensions for YANG augment. For example (from IF-MIB): + * + * smiv2:alias "ifXTable" + * smiv2:oid "1.3.6.1.2.1.31.1.1"; + * smiv2:alias "ifXEntry" + * smiv2:oid "1.3.6.1.2.1.31.1.1.1"; + * augment "/if-mib:IF-MIB/if-mib:ifTable/if-mib:ifEntry" { + * smiv2:oid "1.3.6.1.2.1.31.1.1.1"; + * + * @param[in] h Clixon handle + * @param[in] yaug Mib-Yang node (augment) + * @retval 0 OK + * @retval -1 Error + * @see mibyang_list_register + */ +static int +mibyang_augment_register(clicon_handle h, + yang_stmt *yaug) +{ + int retval = -1; + char *schema_nodeid; + yang_stmt *ylist; + char *oidstr = NULL; + oid oid1[MAX_OID_LEN] = {0,}; + size_t oid1len = MAX_OID_LEN; + oid oid2[MAX_OID_LEN] = {0,}; + size_t oid2len = MAX_OID_LEN; + int ret; + char *ri; + + if ((ret = yangext_oid_get(yaug, oid2, &oid2len, &oidstr)) < 0) + goto done; + if (ret == 0) + goto ok; + /* Decrement oid of list object (oid2) to container object (oid1) */ + memcpy(oid1, oid2, sizeof(oid2)); + oid1len = oid2len - 1; + oid1[oid1len] = 0; + if ((ri = rindex(oidstr, '.')) != NULL) + *ri = '\0'; + schema_nodeid = yang_argument_get(yaug); + if (yang_abs_schema_nodeid(yaug, schema_nodeid, &ylist) < 0) + goto done; + if (yang_keyword_get(ylist) != Y_LIST) + goto ok; /* skip */ + if (mibyang_table_register(h, ylist, + oid1, oid1len, + oid2, oid2len, + oidstr) < 0) + goto done; + ok: + retval = 0; + done: + return retval; +} + /*! Register table sub-oid:s of existing entries in clixon * This assumes a table contains a set of keys and a list of leafs only * The function makes a query to the datastore and registers all table entries that @@ -447,6 +559,11 @@ mibyang_traverse(clicon_handle h, clicon_debug(1, "%s %s", __FUNCTION__, yang_argument_get(yn)); switch(yang_keyword_get(yn)){ + case Y_AUGMENT: + if (mibyang_augment_register(h, yn) < 0) + goto done; + goto ok; + break; case Y_LEAF: if (mibyang_leaf_register(h, yn, NULL, NULL, 0) < 0) goto done; @@ -456,8 +573,9 @@ mibyang_traverse(clicon_handle h, case Y_LIST: /* If parent is container -> identify as table */ yp = yang_parent_get(yn); if (yang_keyword_get(yp) == Y_CONTAINER){ + /* Register table entry handler itself (not column/row leafs) */ - if (mibyang_table_register(h, yn) < 0) + if (mibyang_list_register(h, yn) < 0) goto done; goto ok; } @@ -468,7 +586,8 @@ mibyang_traverse(clicon_handle h, /* Traverse data nodes in tree (module is special case */ ys = NULL; while ((ys = yn_each(yn, ys)) != NULL) { - if (!yang_schemanode(ys)) + /* augment special case of table */ + if (!yang_schemanode(ys) && yang_keyword_get(ys) != Y_AUGMENT) continue; if ((ret = mibyang_traverse(h, ys)) < 0) goto done; diff --git a/test/test_snmp_ifmib.sh b/test/test_snmp_ifmib.sh index 483da31c..98ba9890 100755 --- a/test/test_snmp_ifmib.sh +++ b/test/test_snmp_ifmib.sh @@ -360,11 +360,9 @@ validate_oid $OID22 $OID22 "OID" ".0.0" validate_oid $NAME22.1 $NAME22.1 "OID" "SNMPv2-SMI::zeroDotZero" validate_oid $NAME22.2 $NAME22.2 "OID" "iso.2.3" -if [ $AUGMENTTEST ]; then - new "Test ifName" - validate_oid $NAME23.1 $NAME23.1 "STRING" "test1" - validate_oid $NAME23.2 $NAME23.2 "STRING" "test2" -fi +new "Test ifXTable ifName" +validate_oid $NAME23.1 $NAME23.1 "STRING" "test1" +validate_oid $NAME23.2 $NAME23.2 "STRING" "test2" new "Test ifTable" expectpart "$($snmptable IF-MIB::ifTable)" 0 "Test 2" "1400" "1000" "11:22:33:44:55:66" "down" "111" "222" "333" "444" "555" "666" "777" "888" "999" "101010" "111111" "111"