Fixed: [SNMP smiv2 yang extension doesn't work on augmented nodes](https://github.com/clicon/clixon/issues/366)

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
 

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;

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? */
+    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 

apps/snmp/snmp_register.c

@@ -200,7 +200,7 @@ 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 
  *
@@ -212,21 +212,21 @@ mibyang_leaf_register(clicon_handle h,
  *      }
  *   }
  * @param[in]  h     Clixon handle
- * @param[in]  ys   Mib-Yang node (container)
+ * @param[in]  ylist Mib-Yang node (list)
- * @param[in]  yl   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  */
+    /* Note: This is wrong for augmented nodes where name is the original list, not the 
-    if ((ret = yangext_oid_get(ys, oid1, &oid1len, &oidstr)) < 0)
+     * augmented. For example, for IFMIB you get ifTable twice where you should get ifTable for
-	goto done;
+     * the original and ifXTable for the augmented.
-    if (ret == 0)
+     * But the name does not seem to have semantic significance, so I leave it as is.
-	goto ok;
+     */
     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, oid1len*sizeof(*oid1));
-    memcpy(sh->sh_oid, oid1, 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;

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 ifXTable ifName"
-   new "Test ifName"
+validate_oid $NAME23.1 $NAME23.1 "STRING" "test1"
-   validate_oid $NAME23.1 $NAME23.1 "STRING" "test1"
+validate_oid $NAME23.2 $NAME23.2 "STRING" "test2"
-   validate_oid $NAME23.2 $NAME23.2 "STRING" "test2"
-fi
 
 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"