lib, mgmtd: rework processing of yang notifications

Currently, YANG notification processing is done using a special type of
callbacks registered in backend clients. In this commit, we start using
regular northbound infrastructure instead, because it already has a
convenient way of registering xpath-specific callbacks without the need
for creating additional structures for each necessary notification. We
also now pass a notification data to the callback, instead of a plain
JSON. This allows to use regular YANG library functions for inspecting
notification fields, instead of manually parsing the JSON.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
index e2a720acb7d8681003189ce76c2e4e6854fc295b..286555c56484798b7bb0dc592fb7201ed20e8ea0 100644 (file)
--- a/lib/mgmt_be_client.c
+++ b/lib/mgmt_be_client.c
@@ -963,11 +963,10 @@ static void be_client_handle_notify(struct mgmt_be_client *client, void *msgbuf,
                                    size_t msg_len)
 {
        struct mgmt_msg_notify_data *notif_msg = msgbuf;
-       struct mgmt_be_client_notification_cb *cb;
+       struct nb_node *nb_node;
        char notif[XPATH_MAXLEN];
        struct lyd_node *dnode;
        LY_ERR err;
-       uint i;
 
        debug_be_client("Received notification for client %s", client->name);
 
@@ -978,15 +977,15 @@ static void be_client_handle_notify(struct mgmt_be_client *client, void *msgbuf,
 
        lysc_path(dnode->schema, LYSC_PATH_DATA, notif, sizeof(notif));
 
-       lyd_free_all(dnode);
-
-       for (i = 0; i < client->cbs.nnotify_cbs; i++) {
-               cb = &client->cbs.notify_cbs[i];
-               if (strncmp(cb->xpath, notif, strlen(cb->xpath)))
-                       continue;
-               cb->callback(client, client->user_data, cb,
-                            (const char *)notif_msg->result);
+       nb_node = nb_node_find(notif);
+       if (!nb_node || !nb_node->cbs.notify) {
+               debug_be_client("No notification callback for %s", notif);
+               goto cleanup;
        }
+
+       nb_callback_notify(nb_node, notif, dnode);
+cleanup:
+       lyd_free_all(dnode);
 }
 
 /*
@@ -1057,8 +1056,6 @@ int mgmt_be_send_subscr_req(struct mgmt_be_client *client_ctx,
 {
        Mgmtd__BeMessage be_msg;
        Mgmtd__BeSubscribeReq subscr_req;
-       const char **notif_xpaths = NULL;
-       int ret;
 
        mgmtd__be_subscribe_req__init(&subscr_req);
        subscr_req.client_name = client_ctx->name;
@@ -1068,16 +1065,8 @@ int mgmt_be_send_subscr_req(struct mgmt_be_client *client_ctx,
        subscr_req.oper_xpaths = oper_xpaths;
 
        /* See if we should register for notifications */
-       subscr_req.n_notif_xpaths = client_ctx->cbs.nnotify_cbs;
-       if (client_ctx->cbs.nnotify_cbs) {
-               struct mgmt_be_client_notification_cb *cb, *ecb;
-
-               cb = client_ctx->cbs.notify_cbs;
-               ecb = cb + client_ctx->cbs.nnotify_cbs;
-               for (; cb < ecb; cb++)
-                       *darr_append(notif_xpaths) = cb->xpath;
-       }
-       subscr_req.notif_xpaths = (char **)notif_xpaths;
+       subscr_req.n_notif_xpaths = client_ctx->cbs.nnotif_xpaths;
+       subscr_req.notif_xpaths = (char **)client_ctx->cbs.notif_xpaths;
 
        mgmtd__be_message__init(&be_msg);
        be_msg.message_case = MGMTD__BE_MESSAGE__MESSAGE_SUBSCR_REQ;
@@ -1087,9 +1076,7 @@ int mgmt_be_send_subscr_req(struct mgmt_be_client *client_ctx,
                        subscr_req.client_name, subscr_req.n_config_xpaths,
                        subscr_req.n_oper_xpaths, subscr_req.n_notif_xpaths);
 
-       ret = mgmt_be_client_send_msg(client_ctx, &be_msg);
-       darr_free(notif_xpaths);
-       return ret;
+       return mgmt_be_client_send_msg(client_ctx, &be_msg);
 }
 
 static int _notify_conenct_disconnect(struct msg_client *msg_client,

diff --git a/lib/mgmt_be_client.h b/lib/mgmt_be_client.h
index d144ebc7280adf55c86deefc244d19f3bebe9567..361899fc1da01184aa77ec5d28bdc6a17267f41a 100644 (file)
--- a/lib/mgmt_be_client.h
+++ b/lib/mgmt_be_client.h
@@ -73,16 +73,8 @@ struct mgmt_be_client_cbs {
                           struct mgmt_be_client_txn_ctx *txn_ctx,
                           bool destroyed);
 
-       struct mgmt_be_client_notification_cb *notify_cbs;
-       uint nnotify_cbs;
-};
-
-struct mgmt_be_client_notification_cb {
-       const char *xpath; /* the notification */
-       uint8_t format;    /* currently only LYD_JSON supported */
-       void (*callback)(struct mgmt_be_client *client, uintptr_t usr_data,
-                        struct mgmt_be_client_notification_cb *this,
-                        const char *notif_data);
+       const char **notif_xpaths;
+       uint nnotif_xpaths;
 };
 
 /***************************************************************

diff --git a/lib/northbound.c b/lib/northbound.c
index a0b1bd18c54f273bee50d2680ac57f52a0a6dcb3..d74773194c717f18c9a5ab9235f00d1deadb4a96 100644 (file)
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -284,6 +284,8 @@ static unsigned int nb_node_validate_cbs(const struct nb_node *nb_node)
                                     !!nb_node->cbs.lookup_entry, false);
        error += nb_node_validate_cb(nb_node, NB_CB_RPC, !!nb_node->cbs.rpc,
                                     false);
+       error += nb_node_validate_cb(nb_node, NB_CB_NOTIFY,
+                                    !!nb_node->cbs.notify, true);
 
        return error;
 }
@@ -1605,6 +1607,18 @@ int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath,
        return nb_node->cbs.rpc(&args);
 }
 
+void nb_callback_notify(const struct nb_node *nb_node, const char *xpath,
+                       struct lyd_node *dnode)
+{
+       struct nb_cb_notify_args args = {};
+
+       DEBUGD(&nb_dbg_cbs_notify, "northbound notify: %s", xpath);
+
+       args.xpath = xpath;
+       args.dnode = dnode;
+       nb_node->cbs.notify(&args);
+}
+
 /*
  * Call the northbound configuration callback associated to a given
  * configuration change.
@@ -1653,6 +1667,7 @@ static int nb_callback_configuration(struct nb_context *context,
        case NB_CB_GET_KEYS:
        case NB_CB_LOOKUP_ENTRY:
        case NB_CB_RPC:
+       case NB_CB_NOTIFY:
                yang_dnode_get_path(dnode, xpath, sizeof(xpath));
                flog_err(EC_LIB_DEVELOPMENT,
                         "%s: unknown operation (%u) [xpath %s]", __func__,
@@ -2047,6 +2062,10 @@ bool nb_cb_operation_is_valid(enum nb_cb_operation operation,
                        return false;
                }
                return true;
+       case NB_CB_NOTIFY:
+               if (snode->nodetype != LYS_NOTIF)
+                       return false;
+               return true;
        default:
                return false;
        }
@@ -2279,6 +2298,8 @@ const char *nb_cb_operation_name(enum nb_cb_operation operation)
                return "lookup_entry";
        case NB_CB_RPC:
                return "rpc";
+       case NB_CB_NOTIFY:
+               return "notify";
        }
 
        assert(!"Reached end of function we should never hit");

diff --git a/lib/northbound.h b/lib/northbound.h
index 9279122deb8a5a635d633ff3b3d71d6b2d589b09..e9f2db9b6ef368db0d17a17fae0e89ebf771c72a 100644 (file)
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -99,6 +99,7 @@ enum nb_cb_operation {
        NB_CB_GET_KEYS,
        NB_CB_LOOKUP_ENTRY,
        NB_CB_RPC,
+       NB_CB_NOTIFY,
 };
 
 union nb_resource {
@@ -286,6 +287,18 @@ struct nb_cb_rpc_args {
        size_t errmsg_len;
 };
 
+struct nb_cb_notify_args {
+       /* XPath of the notification. */
+       const char *xpath;
+
+       /*
+        * libyang data node representing the notification. If the notification
+        * is not top-level, it still points to the notification node, but it's
+        * part of the full data tree with all its parents.
+        */
+       struct lyd_node *dnode;
+};
+
 /*
  * Set of configuration callbacks that can be associated to a northbound node.
  */
@@ -509,6 +522,17 @@ struct nb_callbacks {
         */
        int (*rpc)(struct nb_cb_rpc_args *args);
 
+       /*
+        * Notification callback.
+        *
+        * The callback is called when a YANG notification is received.
+        *
+        * args
+        *    Refer to the documentation comments of nb_cb_notify_args for
+        *    details.
+        */
+       void (*notify)(struct nb_cb_notify_args *args);
+
        /*
         * Optional callback to compare the data nodes when printing
         * the CLI commands associated with them.
@@ -786,6 +810,7 @@ DECLARE_HOOK(nb_client_debug_set_all, (uint32_t flags, bool set), (flags, set));
 extern struct debug nb_dbg_cbs_config;
 extern struct debug nb_dbg_cbs_state;
 extern struct debug nb_dbg_cbs_rpc;
+extern struct debug nb_dbg_cbs_notify;
 extern struct debug nb_dbg_notif;
 extern struct debug nb_dbg_events;
 extern struct debug nb_dbg_libyang;
@@ -814,6 +839,8 @@ extern const void *nb_callback_lookup_next(const struct nb_node *nb_node,
 extern int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath,
                           const struct list *input, struct list *output,
                           char *errmsg, size_t errmsg_len);
+extern void nb_callback_notify(const struct nb_node *nb_node, const char *xpath,
+                              struct lyd_node *dnode);
 
 /*
  * Create a northbound node for all YANG schema nodes.

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index 0358a0f377c7bbaf74e8002d714049b204f05094..8809ec2ad87da009cc2021bc0e874973b95b3283 100644 (file)
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -25,6 +25,7 @@
 struct debug nb_dbg_cbs_config = {0, "Northbound callbacks: configuration"};
 struct debug nb_dbg_cbs_state = {0, "Northbound callbacks: state"};
 struct debug nb_dbg_cbs_rpc = {0, "Northbound callbacks: RPCs"};
+struct debug nb_dbg_cbs_notify = {0, "Northbound callbacks: notifications"};
 struct debug nb_dbg_notif = {0, "Northbound notifications"};
 struct debug nb_dbg_events = {0, "Northbound events"};
 struct debug nb_dbg_libyang = {0, "libyang debugging"};
@@ -1772,13 +1773,15 @@ DEFPY (rollback_config,
 /* Debug CLI commands. */
 static struct debug *nb_debugs[] = {
        &nb_dbg_cbs_config, &nb_dbg_cbs_state, &nb_dbg_cbs_rpc,
-       &nb_dbg_notif,      &nb_dbg_events,    &nb_dbg_libyang,
+       &nb_dbg_cbs_notify, &nb_dbg_notif,     &nb_dbg_events,
+       &nb_dbg_libyang,
 };
 
 static const char *const nb_debugs_conflines[] = {
        "debug northbound callbacks configuration",
        "debug northbound callbacks state",
        "debug northbound callbacks rpc",
+       "debug northbound callbacks notify",
        "debug northbound notifications",
        "debug northbound events",
        "debug northbound libyang",
@@ -1803,7 +1806,7 @@ DEFPY (debug_nb,
        debug_nb_cmd,
        "[no] debug northbound\
           [<\
-           callbacks$cbs [{configuration$cbs_cfg|state$cbs_state|rpc$cbs_rpc}]\
+           callbacks$cbs [{configuration$cbs_cfg|state$cbs_state|rpc$cbs_rpc|notify$cbs_notify}]\
            |notifications$notifications\
            |events$events\
            |libyang$libyang\
@@ -1816,13 +1819,14 @@ DEFPY (debug_nb,
        "State\n"
        "RPC\n"
        "Notifications\n"
+       "Notifications\n"
        "Events\n"
        "libyang debugging\n")
 {
        uint32_t mode = DEBUG_NODE2MODE(vty->node);
 
        if (cbs) {
-               bool none = (!cbs_cfg && !cbs_state && !cbs_rpc);
+               bool none = (!cbs_cfg && !cbs_state && !cbs_rpc && !cbs_notify);
 
                if (none || cbs_cfg)
                        DEBUG_MODE_SET(&nb_dbg_cbs_config, mode, !no);
@@ -1830,6 +1834,8 @@ DEFPY (debug_nb,
                        DEBUG_MODE_SET(&nb_dbg_cbs_state, mode, !no);
                if (none || cbs_rpc)
                        DEBUG_MODE_SET(&nb_dbg_cbs_rpc, mode, !no);
+               if (none || cbs_notify)
+                       DEBUG_MODE_SET(&nb_dbg_cbs_notify, mode, !no);
        }
        if (notifications)
                DEBUG_MODE_SET(&nb_dbg_notif, mode, !no);

diff --git a/mgmtd/mgmt_testc.c b/mgmtd/mgmt_testc.c
index a664c9d7a5ac0b674659d25267cd22c6103bb072..7e3ded8209e60b2e30e8a8e3770310bec7c3f7be 100644 (file)
--- a/mgmtd/mgmt_testc.c
+++ b/mgmtd/mgmt_testc.c
@@ -11,14 +11,13 @@
 #include "darr.h"
 #include "libfrr.h"
 #include "mgmt_be_client.h"
+#include "northbound.h"
 
 /* ---------------- */
 /* Local Prototypes */
 /* ---------------- */
 
-static void async_notification(struct mgmt_be_client *client, uintptr_t usr_data,
-                              struct mgmt_be_client_notification_cb *this,
-                              const char *notif_data);
+static void async_notification(struct nb_cb_notify_args *args);
 
 static void sigusr1(void);
 static void sigint(void);
@@ -83,6 +82,10 @@ static const struct frr_yang_module_info frr_ripd_info = {
        .name = "frr-ripd",
        .ignore_cfg_cbs = true,
        .nodes = {
+               {
+                       .xpath = "/frr-ripd:authentication-failure",
+                       .cbs.notify = async_notification,
+               },
                {
                        .xpath = NULL,
                }
@@ -109,7 +112,7 @@ FRR_DAEMON_INFO(mgmtd_testc, MGMTD_TESTC,
        );
 /* clang-format on */
 
-struct mgmt_be_client_notification_cb *__notify_cbs;
+const char **__notif_xpaths;
 
 struct mgmt_be_client_cbs __client_cbs = {};
 struct event *event_timeout;
@@ -131,7 +134,7 @@ static void quit(int exit_code)
 {
        EVENT_OFF(event_timeout);
        frr_fini();
-       darr_free(__client_cbs.notify_cbs);
+       darr_free(__client_cbs.notif_xpaths);
        exit(exit_code);
 }
 
@@ -147,13 +150,12 @@ static void timeout(struct event *event)
        quit(1);
 }
 
-static void async_notification(struct mgmt_be_client *client, uintptr_t usr_data,
-                              struct mgmt_be_client_notification_cb *this,
-                              const char *notif_data)
+static void async_notification(struct nb_cb_notify_args *args)
 {
        zlog_notice("Received YANG notification");
 
-       printf("%s\n", notif_data);
+       printf("{\"frr-ripd:authentication-failure\": {\"interface-name\": \"%s\"}}\n",
+              yang_dnode_get_string(args->dnode, "interface-name"));
 
        if (o_notif_count && !--o_notif_count)
                quit(0);
@@ -205,17 +207,12 @@ int main(int argc, char **argv)
                exit(1);
        }
        if (argc && f_listen) {
-               struct mgmt_be_client_notification_cb *cb;
-
                for (i = 0; i < argc; i++) {
                        zlog_notice("Listen on xpath: %s", argv[i]);
-                       cb = darr_append(__notify_cbs);
-                       cb->xpath = argv[i];
-                       cb->format = LYD_JSON;
-                       cb->callback = async_notification;
+                       darr_push(__notif_xpaths, argv[i]);
                }
-               __client_cbs.notify_cbs = __notify_cbs;
-               __client_cbs.nnotify_cbs = darr_len(__notify_cbs);
+               __client_cbs.notif_xpaths = __notif_xpaths;
+               __client_cbs.nnotif_xpaths = darr_len(__notif_xpaths);
        }
 
        mgmt_be_client = mgmt_be_client_create("mgmtd-testc", &__client_cbs, 0,

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index 3290c8d54afa2168246565291b68abf33acd4db7..f90f8983d448e8bb40d2c30845433fc653c58916 100644 (file)
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -3169,7 +3169,7 @@ DEFUNSH(VTYSH_ALL, debug_nb,
        debug_nb_cmd,
        "[no] debug northbound\
           [<\
-           callbacks [{configuration|state|rpc}]\
+           callbacks [{configuration|state|rpc|notify}]\
            |notifications\
            |events\
            |libyang\
@@ -3182,6 +3182,7 @@ DEFUNSH(VTYSH_ALL, debug_nb,
        "State\n"
        "RPC\n"
        "Notifications\n"
+       "Notifications\n"
        "Events\n"
        "libyang debugging\n")
 {