]> git.proxmox.com Git - mirror_frr.git/commitdiff
lib, tests: add support for keyless YANG lists
authorRenato Westphal <renato@opensourcerouting.org>
Sat, 8 Dec 2018 19:31:16 +0000 (17:31 -0200)
committerRenato Westphal <renato@opensourcerouting.org>
Sun, 9 Dec 2018 15:58:53 +0000 (13:58 -0200)
YANG allows lists without keys for operational data, in which case
the list elements are uniquely identified using a positional index
(starting from one).

This commit does the following:
* Remove the need to implement the 'get_keys' and 'lookup_entry'
  callbacks for keyless lists.
* Extend nb_oper_data_iter_list() so that it special-cases keyless
  lists appropriately. Since both the CLI and the sysrepo plugin
  use nb_oper_data_iterate() to fetch operational data, both these
  northbound clients automatically gain the ability to understand
  keyless lists without additional changes.
* Extend the confd plugin to special-case keyless lists as well. This
  was a bit painful to implement given ConfD's clumsy API, but
  keyless lists should work ok now.
* Update the "test_oper_data" unit test to test keyless YANG lists in
  addition to regular lists.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
lib/northbound.c
lib/northbound.h
lib/northbound_confd.c
tests/lib/northbound/test_oper_data.c
yang/frr-test-module.yang

index 8503f87d608c5a32783c419e8d9fe8f0aa9a0f42..8b96dc4a6cb15d793624d72b5570ecaabeca36ae 100644 (file)
@@ -95,6 +95,13 @@ static int nb_node_new_cb(const struct lys_node *snode, void *arg)
                if (config_only)
                        SET_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY);
        }
+       if (CHECK_FLAG(snode->nodetype, LYS_LIST)) {
+               struct lys_node_list *slist;
+
+               slist = (struct lys_node_list *)snode;
+               if (slist->keys_size == 0)
+                       SET_FLAG(nb_node->flags, F_NB_NODE_KEYLESS_LIST);
+       }
 
        /*
         * Link the northbound node and the libyang schema node with one
@@ -1056,6 +1063,7 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
 {
        struct lys_node_list *slist = (struct lys_node_list *)nb_node->snode;
        const void *list_entry = NULL;
+       uint32_t position = 1;
 
        if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY))
                return NB_OK;
@@ -1073,19 +1081,31 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
                        /* End of the list. */
                        break;
 
-               /* Obtain the list entry keys. */
-               if (nb_node->cbs.get_keys(list_entry, &list_keys) != NB_OK) {
-                       flog_warn(EC_LIB_NB_CB_STATE,
-                                 "%s: failed to get list keys", __func__);
-                       return NB_ERR;
-               }
-
-               /* Build XPath of the list entry. */
-               strlcpy(xpath, xpath_list, sizeof(xpath));
-               for (unsigned int i = 0; i < list_keys.num; i++) {
-                       snprintf(xpath + strlen(xpath),
-                                sizeof(xpath) - strlen(xpath), "[%s='%s']",
-                                slist->keys[i]->name, list_keys.key[i]);
+               if (!CHECK_FLAG(nb_node->flags, F_NB_NODE_KEYLESS_LIST)) {
+                       /* Obtain the list entry keys. */
+                       if (nb_node->cbs.get_keys(list_entry, &list_keys)
+                           != NB_OK) {
+                               flog_warn(EC_LIB_NB_CB_STATE,
+                                         "%s: failed to get list keys",
+                                         __func__);
+                               return NB_ERR;
+                       }
+
+                       /* Build XPath of the list entry. */
+                       strlcpy(xpath, xpath_list, sizeof(xpath));
+                       for (unsigned int i = 0; i < list_keys.num; i++) {
+                               snprintf(xpath + strlen(xpath),
+                                        sizeof(xpath) - strlen(xpath),
+                                        "[%s='%s']", slist->keys[i]->name,
+                                        list_keys.key[i]);
+                       }
+               } else {
+                       /*
+                        * Keyless list - build XPath using a positional index.
+                        */
+                       snprintf(xpath, sizeof(xpath), "%s[%u]", xpath_list,
+                                position);
+                       position++;
                }
 
                /* Iterate over the child nodes. */
@@ -1400,6 +1420,8 @@ bool nb_operation_is_valid(enum nb_operation operation,
                case LYS_LIST:
                        if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY))
                                return false;
+                       if (CHECK_FLAG(nb_node->flags, F_NB_NODE_KEYLESS_LIST))
+                               return false;
                        break;
                default:
                        return false;
index c8e8d7570171e90babe151df258def7c568061a4..9d35a4e64a09daf0739ab1a034859353f086c60c 100644 (file)
@@ -254,7 +254,8 @@ struct nb_callbacks {
         * Operational data callback for YANG lists.
         *
         * The callback function should fill the 'keys' parameter based on the
-        * given list_entry.
+        * given list_entry. Keyless lists don't need to implement this
+        * callback.
         *
         * list_entry
         *    Pointer to list entry.
@@ -272,7 +273,8 @@ struct nb_callbacks {
         * Operational data callback for YANG lists.
         *
         * The callback function should return a list entry based on the list
-        * keys given as a parameter.
+        * keys given as a parameter. Keyless lists don't need to implement this
+        * callback.
         *
         * parent_list_entry
         *    Pointer to parent list entry.
@@ -367,6 +369,8 @@ struct nb_node {
 };
 /* The YANG container or list contains only config data. */
 #define F_NB_NODE_CONFIG_ONLY 0x01
+/* The YANG list doesn't contain key leafs. */
+#define F_NB_NODE_KEYLESS_LIST 0x02
 
 struct frr_yang_module_info {
        /* YANG module name. */
index e819384af719673f8ba361634365c572eace02d8..53149d0fd22b956d4341e78919e0d45bb7c86eb9 100644 (file)
@@ -138,10 +138,19 @@ static int frr_confd_hkeypath_get_list_entry(const confd_hkeypath_t *kp,
                        nb_node_list = nb_node_list->parent_list;
 
                /* Obtain list entry. */
-               *list_entry =
-                       nb_node_list->cbs.lookup_entry(*list_entry, &keys);
-               if (*list_entry == NULL)
-                       return -1;
+               if (!CHECK_FLAG(nb_node_list->flags, F_NB_NODE_KEYLESS_LIST)) {
+                       *list_entry = nb_node_list->cbs.lookup_entry(
+                               *list_entry, &keys);
+                       if (*list_entry == NULL)
+                               return -1;
+               } else {
+                       unsigned long ptr_ulong;
+
+                       /* Retrieve list entry from pseudo-key (string). */
+                       if (sscanf(keys.key[0], "%lu", &ptr_ulong) != 1)
+                               return -1;
+                       *list_entry = (const void *)ptr_ulong;
+               }
 
                curr_list++;
        }
@@ -640,7 +649,6 @@ static int frr_confd_data_get_next(struct confd_trans_ctx *tctx,
 {
        struct nb_node *nb_node;
        char xpath[BUFSIZ];
-       struct yang_list_keys keys;
        struct yang_data *data;
        const void *parent_list_entry, *nb_next;
        confd_value_t v[LIST_MAXKEYS];
@@ -672,18 +680,53 @@ static int frr_confd_data_get_next(struct confd_trans_ctx *tctx,
 
        switch (nb_node->snode->nodetype) {
        case LYS_LIST:
-               memset(&keys, 0, sizeof(keys));
-               if (nb_node->cbs.get_keys(nb_next, &keys) != NB_OK) {
-                       flog_warn(EC_LIB_NB_CB_STATE,
-                                 "%s: failed to get list keys", __func__);
-                       confd_data_reply_next_key(tctx, NULL, -1, -1);
-                       return CONFD_OK;
-               }
+               if (!CHECK_FLAG(nb_node->flags, F_NB_NODE_KEYLESS_LIST)) {
+                       struct yang_list_keys keys;
+
+                       memset(&keys, 0, sizeof(keys));
+                       if (nb_node->cbs.get_keys(nb_next, &keys) != NB_OK) {
+                               flog_warn(EC_LIB_NB_CB_STATE,
+                                         "%s: failed to get list keys",
+                                         __func__);
+                               confd_data_reply_next_key(tctx, NULL, -1, -1);
+                               return CONFD_OK;
+                       }
 
-               /* Feed keys to ConfD. */
-               for (size_t i = 0; i < keys.num; i++)
-                       CONFD_SET_STR(&v[i], keys.key[i]);
-               confd_data_reply_next_key(tctx, v, keys.num, (long)nb_next);
+                       /* Feed keys to ConfD. */
+                       for (size_t i = 0; i < keys.num; i++)
+                               CONFD_SET_STR(&v[i], keys.key[i]);
+                       confd_data_reply_next_key(tctx, v, keys.num,
+                                                 (long)nb_next);
+               } else {
+                       char pointer_str[16];
+
+                       /*
+                        * ConfD 6.6 user guide, chapter 6.11 (Operational data
+                        * lists without keys):
+                        * "To support this without having completely separate
+                        * APIs, we use a "pseudo" key in the ConfD APIs for
+                        * this type of list. This key is not part of the data
+                        * model, and completely hidden in the northbound agent
+                        * interfaces, but is used with e.g. the get_next() and
+                        * get_elem() callbacks as if it were a normal key. This
+                        * "pseudo" key is always a single signed 64-bit
+                        * integer, i.e. the confd_value_t type is C_INT64. The
+                        * values can be chosen arbitrarily by the application,
+                        * as long as a key value returned by get_next() can be
+                        * used to get the data for the corresponding list entry
+                        * with get_elem() or get_object() as usual. It could
+                        * e.g. be an index into an array that holds the data,
+                        * or even a memory address in integer form".
+                        *
+                        * Since we're using the CONFD_DAEMON_FLAG_STRINGSONLY
+                        * option, we must convert our pseudo-key (a void
+                        * pointer) to a string before sending it to confd.
+                        */
+                       snprintf(pointer_str, sizeof(pointer_str), "%lu",
+                                (unsigned long)nb_next);
+                       CONFD_SET_STR(&v[0], pointer_str);
+                       confd_data_reply_next_key(tctx, v, 1, (long)nb_next);
+               }
                break;
        case LYS_LEAFLIST:
                data = nb_node->cbs.get_elem(xpath, nb_next);
@@ -792,6 +835,7 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx,
        const void *nb_next;
 #define CONFD_OBJECTS_PER_TIME 100
        struct confd_next_object objects[CONFD_OBJECTS_PER_TIME + 1];
+       char pseudo_keys[CONFD_OBJECTS_PER_TIME][16];
        int nobjects = 0;
 
        frr_confd_get_xpath(kp, xpath, sizeof(xpath));
@@ -844,6 +888,26 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx,
                        XMALLOC(MTYPE_CONFD,
                                CONFD_MAX_CHILD_NODES * sizeof(confd_value_t));
 
+               /*
+                * ConfD 6.6 user guide, chapter 6.11 (Operational data lists
+                * without keys):
+                * "In the response to the get_next_object() callback, the data
+                * provider is expected to provide the key values along with the
+                * other leafs in an array that is populated according to the
+                * data model. This must be done also for this type of list,
+                * even though the key isn't actually in the data model. The
+                * "pseudo" key must always be the first element in the array".
+                */
+               if (CHECK_FLAG(nb_node->flags, F_NB_NODE_KEYLESS_LIST)) {
+                       confd_value_t *v;
+
+                       snprintf(pseudo_keys[j], sizeof(pseudo_keys[j]), "%lu",
+                                (unsigned long)nb_next);
+
+                       v = &object->v[nvalues++];
+                       CONFD_SET_STR(v, pseudo_keys[j]);
+               }
+
                /* Loop through list child nodes. */
                LY_TREE_FOR (nb_node->snode->child, child) {
                        struct nb_node *nb_node_child = child->priv;
index 7c5713d8f99ce5c50a0a8e0d9782026ca34c6879..7cd622854e886806746ec4eb2352bae020f93a33 100644 (file)
@@ -153,39 +153,6 @@ frr_test_module_vrfs_vrf_routes_route_get_next(const void *parent_list_entry,
        return node;
 }
 
-static int
-frr_test_module_vrfs_vrf_routes_route_get_keys(const void *list_entry,
-                                              struct yang_list_keys *keys)
-{
-       const struct troute *route;
-
-       route = listgetdata((struct listnode *)list_entry);
-
-       keys->num = 1;
-       (void)prefix2str(&route->prefix, keys->key[0], sizeof(keys->key[0]));
-
-       return NB_OK;
-}
-
-static const void *frr_test_module_vrfs_vrf_routes_route_lookup_entry(
-       const void *parent_list_entry, const struct yang_list_keys *keys)
-{
-       const struct tvrf *vrf;
-       const struct troute *route;
-       struct listnode *node;
-       struct prefix prefix;
-
-       yang_str2ipv4p(keys->key[0], &prefix);
-
-       vrf = listgetdata((struct listnode *)parent_list_entry);
-       for (ALL_LIST_ELEMENTS_RO(vrf->routes, node, route)) {
-               if (prefix_same((struct prefix *)&route->prefix, &prefix))
-                       return node;
-       }
-
-       return NULL;
-}
-
 /*
  * XPath: /frr-test-module:frr-test-module/vrfs/vrf/routes/route/prefix
  */
@@ -276,8 +243,6 @@ const struct frr_yang_module_info frr_test_module_info = {
                {
                        .xpath = "/frr-test-module:frr-test-module/vrfs/vrf/routes/route",
                        .cbs.get_next = frr_test_module_vrfs_vrf_routes_route_get_next,
-                       .cbs.get_keys = frr_test_module_vrfs_vrf_routes_route_get_keys,
-                       .cbs.lookup_entry = frr_test_module_vrfs_vrf_routes_route_lookup_entry,
                },
                {
                        .xpath = "/frr-test-module:frr-test-module/vrfs/vrf/routes/route/prefix",
index c02c0a11d7b38e492f760b8cd31981c3bb0553d8..d85b12ea06ee36adfeae3c56ae6b571407107c4d 100644 (file)
@@ -34,8 +34,6 @@ module frr-test-module {
         }
         container routes {
           list route {
-            key "prefix";
-
             leaf prefix {
               type inet:ipv4-prefix;
             }