mgmt, lib: differentiate DELETE and REMOVE operations

Currently, there's a single operation type which doesn't return error
if the object doesn't exists. To be compatible with NETCONF/RESTCONF,
we should support differentiate between DELETE (fails when object
doesn't exist) and REMOVE (doesn't fail if the object doesn't exist).

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

diff --git a/doc/developer/northbound/retrofitting-configuration-commands.rst b/doc/developer/northbound/retrofitting-configuration-commands.rst
index b407246049d2f9e9b07654d65915c22e7e65617f..41f9902b6ac7e5d49de34528a59711ee3d69feaa 100644 (file)
--- a/doc/developer/northbound/retrofitting-configuration-commands.rst
+++ b/doc/developer/northbound/retrofitting-configuration-commands.rst
@@ -1005,7 +1005,7 @@ configuration. Here’s the declaration of this structure (taken from the
 
            /*
             * Operation to apply (either NB_OP_CREATE, NB_OP_MODIFY or
-            * NB_OP_DELETE).
+            * NB_OP_DESTROY).
             */
            enum nb_operation operation;
 
@@ -1205,7 +1205,7 @@ This example shows how to create a list entry:
                    },
                    {
                            .xpath = "./access-list",
-                           .operation = acl ? NB_OP_MODIFY : NB_OP_DELETE,
+                           .operation = acl ? NB_OP_MODIFY : NB_OP_DESTROY,
                            .value = acl,
                    },
            };
@@ -1242,7 +1242,7 @@ When deleting a list entry, all non-key leaves can be ignored:
            struct cli_config_change changes[] = {
                    {
                            .xpath = ".",
-                           .operation = NB_OP_DELETE,
+                           .operation = NB_OP_DESTROY,
                    },
            };
 

diff --git a/lib/mgmt.proto b/lib/mgmt.proto
index 9a39789c82e552689984d07d64ca53c9948f24a4..e0b6019d2b6ccc795ca7c7e93e4e08cce6fc0fb1 100644 (file)
--- a/lib/mgmt.proto
+++ b/lib/mgmt.proto
@@ -55,8 +55,9 @@ message YangData {
 enum CfgDataReqType {
   REQ_TYPE_NONE = 0;
   SET_DATA = 1;
-  DELETE_DATA = 2;
+  REMOVE_DATA = 2;
   CREATE_DATA = 3;
+  DELETE_DATA = 4;
 }
 
 message YangCfgDataReq {

diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
index dd2613f132b45230b11e309faa2802cdfc6412a8..53df191b2c73d747b60e590069f823314418a3fa 100644 (file)
--- a/lib/mgmt_be_client.c
+++ b/lib/mgmt_be_client.c
@@ -565,11 +565,25 @@ static int mgmt_be_update_setcfg_in_batch(struct mgmt_be_client *client_ctx,
        for (index = 0; index < num_req; index++) {
                cfg_chg = &txn_req->req.set_cfg.cfg_changes[index];
 
-               if (cfg_req[index]->req_type
-                   == MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA)
+               /*
+                * Treat all operations as destroy or modify, because we don't
+                * need additional existence checks on the backend. Everything
+                * is already checked by mgmtd.
+                */
+               switch (cfg_req[index]->req_type) {
+               case MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA:
+               case MGMTD__CFG_DATA_REQ_TYPE__REMOVE_DATA:
                        cfg_chg->operation = NB_OP_DESTROY;
-               else
+                       break;
+               case MGMTD__CFG_DATA_REQ_TYPE__SET_DATA:
+               case MGMTD__CFG_DATA_REQ_TYPE__CREATE_DATA:
                        cfg_chg->operation = NB_OP_MODIFY;
+                       break;
+               case MGMTD__CFG_DATA_REQ_TYPE__REQ_TYPE_NONE:
+               case _MGMTD__CFG_DATA_REQ_TYPE_IS_INT_SIZE:
+               default:
+                       continue;
+               }
 
                strlcpy(cfg_chg->xpath, cfg_req[index]->data->xpath,
                        sizeof(cfg_chg->xpath));

diff --git a/lib/northbound.c b/lib/northbound.c
index f7e0d698b3d7cdbdec06fde22c396b3f44a262d1..01bb091859f131cb8d0679b0e5c640c0015e0af1 100644 (file)
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -731,13 +731,14 @@ int nb_candidate_edit(struct nb_config *candidate,
                }
                break;
        case NB_OP_DESTROY:
+       case NB_OP_DELETE:
                dnode = yang_dnode_get(candidate->dnode, xpath_edit);
-               if (!dnode)
-                       /*
-                        * Return a special error code so the caller can choose
-                        * whether to ignore it or not.
-                        */
-                       return NB_ERR_NOT_FOUND;
+               if (!dnode) {
+                       if (operation == NB_OP_DELETE)
+                               return NB_ERR;
+                       else
+                               return NB_OK;
+               }
                /* destroy dependant */
                if (nb_node->dep_cbs.get_dependant_xpath) {
                        nb_node->dep_cbs.get_dependant_xpath(dnode, dep_xpath);
@@ -767,6 +768,8 @@ const char *nb_operation_name(enum nb_operation operation)
                return "modify";
        case NB_OP_DESTROY:
                return "destroy";
+       case NB_OP_DELETE:
+               return "delete";
        case NB_OP_MOVE:
                return "move";
        }
@@ -777,7 +780,8 @@ const char *nb_operation_name(enum nb_operation operation)
 bool nb_is_operation_allowed(struct nb_node *nb_node, enum nb_operation oper)
 {
        if (lysc_is_key(nb_node->snode)) {
-               if (oper == NB_OP_MODIFY || oper == NB_OP_DESTROY)
+               if (oper == NB_OP_MODIFY || oper == NB_OP_DESTROY
+                   || oper == NB_OP_DELETE)
                        return false;
        }
        return true;
@@ -842,7 +846,7 @@ void nb_candidate_edit_config_changes(
                ret = nb_candidate_edit(candidate_config, nb_node,
                                        change->operation, xpath, NULL, data);
                yang_data_free(data);
-               if (ret != NB_OK && ret != NB_ERR_NOT_FOUND) {
+               if (ret != NB_OK) {
                        flog_warn(
                                EC_LIB_NB_CANDIDATE_EDIT_ERROR,
                                "%s: failed to edit candidate configuration: operation [%s] xpath [%s]",

diff --git a/lib/northbound.h b/lib/northbound.h
index e11958c77d01601a8abc8f509d7f092666448a2d..ef774fb476cb756ab71baaf30fba7c4205ba8aa9 100644 (file)
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -722,6 +722,7 @@ enum nb_operation {
        NB_OP_CREATE,
        NB_OP_MODIFY,
        NB_OP_DESTROY,
+       NB_OP_DELETE,
        NB_OP_MOVE,
 };
 
@@ -953,7 +954,6 @@ extern bool nb_is_operation_allowed(struct nb_node *nb_node,
  *
  * Returns:
  *    - NB_OK on success.
- *    - NB_ERR_NOT_FOUND when the element to be deleted was not found.
  *    - NB_ERR for other errors.
  */
 extern int nb_candidate_edit(struct nb_config *candidate,

diff --git a/lib/northbound_cli.h b/lib/northbound_cli.h
index c8f8a8481ab7c52235dfd00680169ffcfc023a0a..1a45794d452642e5ebab70c3c8e025a8ad859539 100644 (file)
--- a/lib/northbound_cli.h
+++ b/lib/northbound_cli.h
@@ -32,7 +32,7 @@ extern struct nb_config *vty_shared_candidate_config;
  *    XPath (absolute or relative) of the configuration option being edited.
  *
  * operation
- *    Operation to apply (either NB_OP_CREATE, NB_OP_MODIFY or NB_OP_DELETE).
+ *    Operation to apply (either NB_OP_CREATE, NB_OP_MODIFY or NB_OP_DESTROY).
  *
  * value
  *    New value of the configuration option. Should be NULL for typeless YANG

diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index 1a194935ac2eee9a7bdc740e6c2d059494655d35..198d96e3811cd98e3ad9cc8425bda11b8c66d01f 100644 (file)
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -221,7 +221,7 @@ static int frr_sr_process_change(struct nb_config *candidate,
 
        ret = nb_candidate_edit(candidate, nb_node, nb_op, xpath, NULL, data);
        yang_data_free(data);
-       if (ret != NB_OK && ret != NB_ERR_NOT_FOUND) {
+       if (ret != NB_OK) {
                flog_warn(
                        EC_LIB_NB_CANDIDATE_EDIT_ERROR,
                        "%s: failed to edit candidate configuration: operation [%s] xpath [%s]",

diff --git a/lib/vty.c b/lib/vty.c
index 69df0a024505a9c2cc315027b3f99c9049a58529..7e856e3741e22bc826ebfe096e7855fadc5d6077 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -3996,11 +3996,16 @@ int vty_mgmt_send_config_data(struct vty *vty, const char *xpath_base,
                mgmt_yang_cfg_data_req_init(&cfg_req[indx]);
                cfg_req[indx].data = &cfg_data[indx];
                switch (vty->cfg_changes[indx].operation) {
-               case NB_OP_DESTROY:
+               case NB_OP_DELETE:
                        cfg_req[indx].req_type =
                                MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA;
                        break;
 
+               case NB_OP_DESTROY:
+                       cfg_req[indx].req_type =
+                               MGMTD__CFG_DATA_REQ_TYPE__REMOVE_DATA;
+                       break;
+
                case NB_OP_CREATE_EXCL:
                        cfg_req[indx].req_type =
                                MGMTD__CFG_DATA_REQ_TYPE__CREATE_DATA;

diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index abae2a6a53b13981ed8ad06437b933fb0f75d7eb..dee75e6586598dae9678f9be4087a5fad64d4ebb 100644 (file)
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -918,7 +918,7 @@ static int mgmt_txn_create_config_batches(struct mgmt_txn_req *txn_req,
                         */
                        if (chg->cb.operation == NB_CB_DESTROY)
                                batch->cfg_data[batch->num_cfg_data].req_type =
-                                       MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA;
+                                       MGMTD__CFG_DATA_REQ_TYPE__REMOVE_DATA;
                        else
                                batch->cfg_data[batch->num_cfg_data].req_type =
                                        MGMTD__CFG_DATA_REQ_TYPE__SET_DATA;
@@ -2052,6 +2052,9 @@ int mgmt_txn_send_set_config_req(uint64_t txn_id, uint64_t req_id,
 
                switch (cfg_req[indx]->req_type) {
                case MGMTD__CFG_DATA_REQ_TYPE__DELETE_DATA:
+                       cfg_chg->operation = NB_OP_DELETE;
+                       break;
+               case MGMTD__CFG_DATA_REQ_TYPE__REMOVE_DATA:
                        cfg_chg->operation = NB_OP_DESTROY;
                        break;
                case MGMTD__CFG_DATA_REQ_TYPE__SET_DATA:

diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c
index aa46d5c783633bbf16b6fd3aa81627d7dc830fc4..b12f54ccbe332678c3168233b2222bfbab4f4071 100644 (file)
--- a/mgmtd/mgmt_vty.c
+++ b/mgmtd/mgmt_vty.c
@@ -185,6 +185,23 @@ DEFPY(mgmt_delete_config_data, mgmt_delete_config_data_cmd,
       "XPath expression specifying the YANG data path\n")
 {
 
+       strlcpy(vty->cfg_changes[0].xpath, path,
+               sizeof(vty->cfg_changes[0].xpath));
+       vty->cfg_changes[0].value = NULL;
+       vty->cfg_changes[0].operation = NB_OP_DELETE;
+       vty->num_cfg_changes = 1;
+
+       vty_mgmt_send_config_data(vty, NULL, false);
+       return CMD_SUCCESS;
+}
+
+DEFPY(mgmt_remove_config_data, mgmt_remove_config_data_cmd,
+      "mgmt remove-config WORD$path",
+      MGMTD_STR
+      "Remove configuration data\n"
+      "XPath expression specifying the YANG data path\n")
+{
+
        strlcpy(vty->cfg_changes[0].xpath, path,
                sizeof(vty->cfg_changes[0].xpath));
        vty->cfg_changes[0].value = NULL;
@@ -547,6 +564,7 @@ void mgmt_vty_init(void)
        install_element(CONFIG_NODE, &mgmt_create_config_data_cmd);
        install_element(CONFIG_NODE, &mgmt_set_config_data_cmd);
        install_element(CONFIG_NODE, &mgmt_delete_config_data_cmd);
+       install_element(CONFIG_NODE, &mgmt_remove_config_data_cmd);
        install_element(CONFIG_NODE, &mgmt_load_config_cmd);
        install_element(CONFIG_NODE, &mgmt_save_config_cmd);
        install_element(CONFIG_NODE, &mgmt_rollback_cmd);