]> git.proxmox.com Git - mirror_frr.git/commitdiff
Revert "lib: introduce a read-write lock for northbound configurations"
authorRenato Westphal <renato@opensourcerouting.org>
Tue, 17 Sep 2019 00:51:11 +0000 (21:51 -0300)
committerRenato Westphal <renato@opensourcerouting.org>
Wed, 18 Sep 2019 17:35:10 +0000 (14:35 -0300)
Adding a lock to protect the global running configuration doesn't
help much since the FRR daemons are not prepared to process
configuration changes in a pthread that is not the main one (a
whole lot of new protections would be necessary to prevent race
conditions).

This means the lock added by commit 83981138 only adds more
complexity for no benefit. Remove it now to simplify the code.

All northbound clients, including the gRPC one, should either run
in the main pthread or use synchronization primitives to process
configuration transactions in the main pthread.

This reverts commit 83981138fe8c1e0a40b8dede74eca65449dda5de.

14 files changed:
bfdd/bfd.c
isisd/isis_cli.c
lib/command.c
lib/if.c
lib/libfrr.c
lib/northbound.c
lib/northbound.h
lib/northbound_cli.c
lib/northbound_confd.c
lib/northbound_grpc.cpp
lib/northbound_sysrepo.c
lib/vty.c
ripd/ripd.c
ripngd/ripngd.c

index feb9827ce1ff30093f166ecea7cb1e0a65ea15cc..245f2dc5ee2e5e057214479d6698be384ef806c7 100644 (file)
@@ -1791,16 +1791,11 @@ void bfd_session_update_vrf_name(struct bfd_session *bs, struct vrf *vrf)
                snprintf(xpath + slen, sizeof(xpath) - slen, "[vrf='%s']/vrf",
                         bs->key.vrfname);
 
-               pthread_rwlock_wrlock(&running_config->lock);
-               {
-                       bfd_dnode = yang_dnode_get(
-                                                  running_config->dnode,
-                                                  xpath, bs->key.vrfname);
-                       if (bfd_dnode) {
-                               yang_dnode_change_leaf(bfd_dnode, vrf->name);
-                               running_config->version++;
-                       }
-                       pthread_rwlock_unlock(&running_config->lock);
+               bfd_dnode = yang_dnode_get(running_config->dnode, xpath,
+                                          bs->key.vrfname);
+               if (bfd_dnode) {
+                       yang_dnode_change_leaf(bfd_dnode, vrf->name);
+                       running_config->version++;
                }
        }
        memset(bs->key.vrfname, 0, sizeof(bs->key.vrfname));
index bd06286755910d3563515c84c3d6a8e4bb8fdacc..37f4dcfabdec496d5b4b57d3307d0e037f21da02 100644 (file)
@@ -188,14 +188,10 @@ DEFPY(ip_router_isis, ip_router_isis_cmd, "ip router isis WORD$tag",
        }
 
        /* check if the interface is a loopback and if so set it as passive */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-               if (ifp && if_is_loopback(ifp))
-                       nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
-                                             NB_OP_MODIFY, "true");
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+       if (ifp && if_is_loopback(ifp))
+               nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
+                                     NB_OP_MODIFY, "true");
 
        return nb_cli_apply_changes(vty, NULL);
 }
@@ -262,14 +258,10 @@ DEFPY(ip6_router_isis, ip6_router_isis_cmd, "ipv6 router isis WORD$tag",
        }
 
        /* check if the interface is a loopback and if so set it as passive */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-               if (ifp && if_is_loopback(ifp))
-                       nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
-                                             NB_OP_MODIFY, "true");
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+       if (ifp && if_is_loopback(ifp))
+               nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
+                                     NB_OP_MODIFY, "true");
 
        return nb_cli_apply_changes(vty, NULL);
 }
@@ -410,26 +402,20 @@ DEFPY(no_is_type, no_is_type_cmd,
       "Act as both a station router and an area router\n"
       "Act as an area router only\n")
 {
-       const char *value;
-
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               struct isis_area *area;
+       const char *value = NULL;
+       struct isis_area *area;
 
-               area = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-
-               /*
-                * Put the is-type back to defaults:
-                * - level-1-2 on first area
-                * - level-1 for the rest
-                */
-               if (area && listgetdata(listhead(isis->area_list)) == area)
-                       value = "level-1-2";
-               else
-                       value = NULL;
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       area = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
 
+       /*
+        * Put the is-type back to defaults:
+        * - level-1-2 on first area
+        * - level-1 for the rest
+        */
+       if (area && listgetdata(listhead(isis->area_list)) == area)
+               value = "level-1-2";
+       else
+               value = NULL;
        nb_cli_enqueue_change(vty, "./is-type", NB_OP_MODIFY, value);
 
        return nb_cli_apply_changes(vty, NULL);
@@ -1817,45 +1803,52 @@ DEFPY(no_isis_circuit_type, no_isis_circuit_type_cmd,
       "Level-1-2 adjacencies are formed\n"
       "Level-2 only adjacencies are formed\n")
 {
-       const char *circ_type = NULL;
+       struct interface *ifp;
+       struct isis_circuit *circuit;
+       int is_type;
+       const char *circ_type;
 
        /*
         * Default value depends on whether the circuit is part of an area,
         * and the is-type of the area if there is one. So we need to do this
         * here.
         */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               struct interface *ifp;
-               struct isis_circuit *circuit;
+       ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+       if (!ifp)
+               goto def_val;
 
-               ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-               if (!ifp)
-                       goto unlock;
+       circuit = circuit_scan_by_ifp(ifp);
+       if (!circuit)
+               goto def_val;
 
-               circuit = circuit_scan_by_ifp(ifp);
-               if (!circuit || circuit->state != C_STATE_UP)
-                       goto unlock;
+       if (circuit->state == C_STATE_UP)
+               is_type = circuit->area->is_type;
+       else
+               goto def_val;
 
-               switch (circuit->area->is_type) {
-               case IS_LEVEL_1:
-                       circ_type = "level-1";
-                       break;
-               case IS_LEVEL_2:
-                       circ_type = "level-2";
-                       break;
-               case IS_LEVEL_1_AND_2:
-                       circ_type = "level-1-2";
-                       break;
-               }
+       switch (is_type) {
+       case IS_LEVEL_1:
+               circ_type = "level-1";
+               break;
+       case IS_LEVEL_2:
+               circ_type = "level-2";
+               break;
+       case IS_LEVEL_1_AND_2:
+               circ_type = "level-1-2";
+               break;
+       default:
+               return CMD_ERR_NO_MATCH;
        }
-unlock:
-       pthread_rwlock_unlock(&running_config->lock);
-
        nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
                              NB_OP_MODIFY, circ_type);
 
        return nb_cli_apply_changes(vty, NULL);
+
+def_val:
+       nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
+                             NB_OP_MODIFY, NULL);
+
+       return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_ip_isis_circ_type(struct vty *vty, struct lyd_node *dnode,
index eecca2a5f54dd0b7b30e28e1ffe732b0c507e778..04f2bd95a08177e4789f0a32869181f35e1fb8ae 100644 (file)
@@ -1718,16 +1718,12 @@ static int vty_write_config(struct vty *vty)
        vty_out(vty, "frr defaults %s\n", DFLT_NAME);
        vty_out(vty, "!\n");
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               for (i = 0; i < vector_active(cmdvec); i++)
-                       if ((node = vector_slot(cmdvec, i)) && node->func
-                           && (node->vtysh || vty->type != VTY_SHELL)) {
-                               if ((*node->func)(vty))
-                                       vty_out(vty, "!\n");
-                       }
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       for (i = 0; i < vector_active(cmdvec); i++)
+               if ((node = vector_slot(cmdvec, i)) && node->func
+                   && (node->vtysh || vty->type != VTY_SHELL)) {
+                       if ((*node->func)(vty))
+                               vty_out(vty, "!\n");
+               }
 
        if (vty->type == VTY_TERM) {
                vty_out(vty, "end\n");
index 5f92327562e490a18af3807a675b57acd4986f1b..a3e63136677d6d82c0fc8b6119cb334032c1c297 100644 (file)
--- a/lib/if.c
+++ b/lib/if.c
@@ -207,21 +207,18 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
        if (yang_module_find("frr-interface")) {
                struct lyd_node *if_dnode;
 
-               pthread_rwlock_wrlock(&running_config->lock);
-               {
-                       if_dnode = yang_dnode_get(
-                               running_config->dnode,
-                               "/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf",
-                               ifp->name, old_vrf->name);
-                       if (if_dnode) {
-                               yang_dnode_change_leaf(if_dnode, vrf->name);
-                               running_config->version++;
-                       }
+               if_dnode = yang_dnode_get(
+                       running_config->dnode,
+                       "/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf",
+                       ifp->name, old_vrf->name);
+               if (if_dnode) {
+                       yang_dnode_change_leaf(if_dnode, vrf->name);
+                       running_config->version++;
                }
-               pthread_rwlock_unlock(&running_config->lock);
        }
 }
 
+
 /* Delete interface structure. */
 void if_delete_retain(struct interface *ifp)
 {
index 478fe4e20523398f29c77f99b7a8a373b2716452..d4aa1f899a11bf6450999d85f7a13907b5ee478c 100644 (file)
@@ -868,12 +868,7 @@ static int frr_config_read_in(struct thread *t)
        /*
         * Update the shared candidate after reading the startup configuration.
         */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               nb_config_replace(vty_shared_candidate_config, running_config,
-                                 true);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       nb_config_replace(vty_shared_candidate_config, running_config, true);
 
        return 0;
 }
index fe7c8bfbc6de56295e0d59358fa5acd318f60195..bffe62f60e29d9162998a7f9157439ce0428ff0b 100644 (file)
@@ -270,7 +270,6 @@ struct nb_config *nb_config_new(struct lyd_node *dnode)
        else
                config->dnode = yang_dnode_new(ly_native_ctx, true);
        config->version = 0;
-       pthread_rwlock_init(&config->lock, NULL);
 
        return config;
 }
@@ -279,7 +278,6 @@ void nb_config_free(struct nb_config *config)
 {
        if (config->dnode)
                yang_dnode_free(config->dnode);
-       pthread_rwlock_destroy(&config->lock);
        XFREE(MTYPE_NB_CONFIG, config);
 }
 
@@ -290,7 +288,6 @@ struct nb_config *nb_config_dup(const struct nb_config *config)
        dup = XCALLOC(MTYPE_NB_CONFIG, sizeof(*dup));
        dup->dnode = yang_dnode_dup(config->dnode);
        dup->version = config->version;
-       pthread_rwlock_init(&dup->lock, NULL);
 
        return dup;
 }
@@ -540,28 +537,17 @@ int nb_candidate_edit(struct nb_config *candidate,
 
 bool nb_candidate_needs_update(const struct nb_config *candidate)
 {
-       bool ret = false;
-
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               if (candidate->version < running_config->version)
-                       ret = true;
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       if (candidate->version < running_config->version)
+               return true;
 
-       return ret;
+       return false;
 }
 
 int nb_candidate_update(struct nb_config *candidate)
 {
        struct nb_config *updated_config;
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               updated_config = nb_config_dup(running_config);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
-
+       updated_config = nb_config_dup(running_config);
        if (nb_config_merge(updated_config, candidate, true) != NB_OK)
                return NB_ERR;
 
@@ -645,13 +631,9 @@ int nb_candidate_validate(struct nb_config *candidate)
                return NB_ERR_VALIDATION;
 
        RB_INIT(nb_config_cbs, &changes);
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               nb_config_diff(running_config, candidate, &changes);
-               ret = nb_candidate_validate_code(candidate, &changes);
-               nb_config_diff_del_changes(&changes);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       nb_config_diff(running_config, candidate, &changes);
+       ret = nb_candidate_validate_code(candidate, &changes);
+       nb_config_diff_del_changes(&changes);
 
        return ret;
 }
@@ -671,35 +653,26 @@ int nb_candidate_commit_prepare(struct nb_config *candidate,
        }
 
        RB_INIT(nb_config_cbs, &changes);
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               nb_config_diff(running_config, candidate, &changes);
-               if (RB_EMPTY(nb_config_cbs, &changes)) {
-                       pthread_rwlock_unlock(&running_config->lock);
-                       return NB_ERR_NO_CHANGES;
-               }
+       nb_config_diff(running_config, candidate, &changes);
+       if (RB_EMPTY(nb_config_cbs, &changes))
+               return NB_ERR_NO_CHANGES;
 
-               if (nb_candidate_validate_code(candidate, &changes) != NB_OK) {
-                       flog_warn(
-                               EC_LIB_NB_CANDIDATE_INVALID,
-                               "%s: failed to validate candidate configuration",
-                               __func__);
-                       nb_config_diff_del_changes(&changes);
-                       pthread_rwlock_unlock(&running_config->lock);
-                       return NB_ERR_VALIDATION;
-               }
+       if (nb_candidate_validate_code(candidate, &changes) != NB_OK) {
+               flog_warn(EC_LIB_NB_CANDIDATE_INVALID,
+                         "%s: failed to validate candidate configuration",
+                         __func__);
+               nb_config_diff_del_changes(&changes);
+               return NB_ERR_VALIDATION;
+       }
 
-               *transaction = nb_transaction_new(candidate, &changes, client,
-                                                 user, comment);
-               if (*transaction == NULL) {
-                       flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
-                                 "%s: failed to create transaction", __func__);
-                       nb_config_diff_del_changes(&changes);
-                       pthread_rwlock_unlock(&running_config->lock);
-                       return NB_ERR_LOCKED;
-               }
+       *transaction =
+               nb_transaction_new(candidate, &changes, client, user, comment);
+       if (*transaction == NULL) {
+               flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
+                         "%s: failed to create transaction", __func__);
+               nb_config_diff_del_changes(&changes);
+               return NB_ERR_LOCKED;
        }
-       pthread_rwlock_unlock(&running_config->lock);
 
        return nb_transaction_process(NB_EV_PREPARE, *transaction);
 }
@@ -718,11 +691,7 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction,
 
        /* Replace running by candidate. */
        transaction->config->version++;
-       pthread_rwlock_wrlock(&running_config->lock);
-       {
-               nb_config_replace(running_config, transaction->config, true);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       nb_config_replace(running_config, transaction->config, true);
 
        /* Record transaction. */
        if (save_transaction
@@ -996,52 +965,40 @@ static int nb_transaction_process(enum nb_event event,
 {
        struct nb_config_cb *cb;
 
-       /*
-        * Need to lock the running configuration since transaction->changes
-        * can contain pointers to data nodes from the running configuration.
-        */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               RB_FOREACH (cb, nb_config_cbs, &transaction->changes) {
-                       struct nb_config_change *change =
-                               (struct nb_config_change *)cb;
-                       int ret;
+       RB_FOREACH (cb, nb_config_cbs, &transaction->changes) {
+               struct nb_config_change *change = (struct nb_config_change *)cb;
+               int ret;
 
+               /*
+                * Only try to release resources that were allocated
+                * successfully.
+                */
+               if (event == NB_EV_ABORT && change->prepare_ok == false)
+                       break;
+
+               /* Call the appropriate callback. */
+               ret = nb_callback_configuration(event, change);
+               switch (event) {
+               case NB_EV_PREPARE:
+                       if (ret != NB_OK)
+                               return ret;
+                       change->prepare_ok = true;
+                       break;
+               case NB_EV_ABORT:
+               case NB_EV_APPLY:
                        /*
-                        * Only try to release resources that were allocated
-                        * successfully.
+                        * At this point it's not possible to reject the
+                        * transaction anymore, so any failure here can lead to
+                        * inconsistencies and should be treated as a bug.
+                        * Operations prone to errors, like validations and
+                        * resource allocations, should be performed during the
+                        * 'prepare' phase.
                         */
-                       if (event == NB_EV_ABORT && change->prepare_ok == false)
-                               break;
-
-                       /* Call the appropriate callback. */
-                       ret = nb_callback_configuration(event, change);
-                       switch (event) {
-                       case NB_EV_PREPARE:
-                               if (ret != NB_OK) {
-                                       pthread_rwlock_unlock(
-                                               &running_config->lock);
-                                       return ret;
-                               }
-                               change->prepare_ok = true;
-                               break;
-                       case NB_EV_ABORT:
-                       case NB_EV_APPLY:
-                               /*
-                                * At this point it's not possible to reject the
-                                * transaction anymore, so any failure here can
-                                * lead to inconsistencies and should be treated
-                                * as a bug. Operations prone to errors, like
-                                * validations and resource allocations, should
-                                * be performed during the 'prepare' phase.
-                                */
-                               break;
-                       default:
-                               break;
-                       }
+                       break;
+               default:
+                       break;
                }
        }
-       pthread_rwlock_unlock(&running_config->lock);
 
        return NB_OK;
 }
index 502fbb9d680d865e08b811cbb4395232b98adf02..50bddd157fc804591551d8f8d830eaddf9b0f282 100644 (file)
@@ -449,19 +449,8 @@ enum nb_client {
 
 /* Northbound configuration. */
 struct nb_config {
-       /* Configuration data. */
        struct lyd_node *dnode;
-
-       /* Configuration version. */
        uint32_t version;
-
-       /*
-        * Lock protecting this structure. The use of this lock is always
-        * necessary when reading or modifying the global running configuration.
-        * For candidate configurations, use of this lock is optional depending
-        * on the threading scheme of the northbound plugin.
-        */
-       pthread_rwlock_t lock;
 };
 
 /* Northbound configuration callback. */
index 884c01a457ec712de33cfe6bb45fbfd73e9e16cc..a15fe3d1c957283f9801d9b06093838c5c1ab7b6 100644 (file)
@@ -193,13 +193,8 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
                                "Please check the logs for more details.\n");
 
                        /* Regenerate candidate for consistency. */
-                       pthread_rwlock_rdlock(&running_config->lock);
-                       {
-                               nb_config_replace(vty->candidate_config,
-                                                 running_config, true);
-                       }
-                       pthread_rwlock_unlock(&running_config->lock);
-
+                       nb_config_replace(vty->candidate_config, running_config,
+                                         true);
                        return CMD_WARNING_CONFIG_FAILED;
                }
        }
@@ -307,12 +302,7 @@ static int nb_cli_commit(struct vty *vty, bool force,
 
        /* "confirm" parameter. */
        if (confirmed_timeout) {
-               pthread_rwlock_rdlock(&running_config->lock);
-               {
-                       vty->confirmed_commit_rollback =
-                               nb_config_dup(running_config);
-               }
-               pthread_rwlock_unlock(&running_config->lock);
+               vty->confirmed_commit_rollback = nb_config_dup(running_config);
 
                vty->t_confirmed_commit_timeout = NULL;
                thread_add_timer(master, nb_cli_confirmed_commit_timeout, vty,
@@ -326,13 +316,8 @@ static int nb_cli_commit(struct vty *vty, bool force,
        /* Map northbound return code to CLI return code. */
        switch (ret) {
        case NB_OK:
-               pthread_rwlock_rdlock(&running_config->lock);
-               {
-                       nb_config_replace(vty->candidate_config_base,
-                                         running_config, true);
-               }
-               pthread_rwlock_unlock(&running_config->lock);
-
+               nb_config_replace(vty->candidate_config_base, running_config,
+                                 true);
                vty_out(vty,
                        "%% Configuration committed successfully (Transaction ID #%u).\n\n",
                        transaction_id);
@@ -729,12 +714,7 @@ DEFPY (config_update,
                return CMD_WARNING;
        }
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               nb_config_replace(vty->candidate_config_base, running_config,
-                                 true);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       nb_config_replace(vty->candidate_config_base, running_config, true);
 
        vty_out(vty, "%% Candidate configuration updated successfully.\n\n");
 
@@ -834,12 +814,8 @@ DEFPY (show_config_running,
                }
        }
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               nb_cli_show_config(vty, running_config, format, translator,
-                                  !!with_defaults);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       nb_cli_show_config(vty, running_config, format, translator,
+                          !!with_defaults);
 
        return CMD_SUCCESS;
 }
@@ -953,68 +929,57 @@ DEFPY (show_config_compare,
        struct nb_config *config2, *config_transaction2 = NULL;
        int ret = CMD_WARNING;
 
-       /*
-        * For simplicity, lock the running configuration regardless if it's
-        * going to be used or not.
-        */
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               if (c1_candidate)
-                       config1 = vty->candidate_config;
-               else if (c1_running)
-                       config1 = running_config;
-               else {
-                       config_transaction1 = nb_db_transaction_load(c1_tid);
-                       if (!config_transaction1) {
-                               vty_out(vty,
-                                       "%% Transaction %u does not exist\n\n",
-                                       (unsigned int)c1_tid);
-                               goto exit;
-                       }
-                       config1 = config_transaction1;
+       if (c1_candidate)
+               config1 = vty->candidate_config;
+       else if (c1_running)
+               config1 = running_config;
+       else {
+               config_transaction1 = nb_db_transaction_load(c1_tid);
+               if (!config_transaction1) {
+                       vty_out(vty, "%% Transaction %u does not exist\n\n",
+                               (unsigned int)c1_tid);
+                       goto exit;
                }
+               config1 = config_transaction1;
+       }
 
-               if (c2_candidate)
-                       config2 = vty->candidate_config;
-               else if (c2_running)
-                       config2 = running_config;
-               else {
-                       config_transaction2 = nb_db_transaction_load(c2_tid);
-                       if (!config_transaction2) {
-                               vty_out(vty,
-                                       "%% Transaction %u does not exist\n\n",
-                                       (unsigned int)c2_tid);
-                               goto exit;
-                       }
-                       config2 = config_transaction2;
+       if (c2_candidate)
+               config2 = vty->candidate_config;
+       else if (c2_running)
+               config2 = running_config;
+       else {
+               config_transaction2 = nb_db_transaction_load(c2_tid);
+               if (!config_transaction2) {
+                       vty_out(vty, "%% Transaction %u does not exist\n\n",
+                               (unsigned int)c2_tid);
+                       goto exit;
                }
+               config2 = config_transaction2;
+       }
 
-               if (json)
-                       format = NB_CFG_FMT_JSON;
-               else if (xml)
-                       format = NB_CFG_FMT_XML;
-               else
-                       format = NB_CFG_FMT_CMDS;
+       if (json)
+               format = NB_CFG_FMT_JSON;
+       else if (xml)
+               format = NB_CFG_FMT_XML;
+       else
+               format = NB_CFG_FMT_CMDS;
 
-               if (translator_family) {
-                       translator = yang_translator_find(translator_family);
-                       if (!translator) {
-                               vty_out(vty,
-                                       "%% Module translator \"%s\" not found\n",
-                                       translator_family);
-                               goto exit;
-                       }
+       if (translator_family) {
+               translator = yang_translator_find(translator_family);
+               if (!translator) {
+                       vty_out(vty, "%% Module translator \"%s\" not found\n",
+                               translator_family);
+                       goto exit;
                }
-
-               ret = nb_cli_show_config_compare(vty, config1, config2, format,
-                                                translator);
-       exit:
-               if (config_transaction1)
-                       nb_config_free(config_transaction1);
-               if (config_transaction2)
-                       nb_config_free(config_transaction2);
        }
-       pthread_rwlock_unlock(&running_config->lock);
+
+       ret = nb_cli_show_config_compare(vty, config1, config2, format,
+                                        translator);
+exit:
+       if (config_transaction1)
+               nb_config_free(config_transaction1);
+       if (config_transaction2)
+               nb_config_free(config_transaction2);
 
        return ret;
 }
index e9669fc7e16c8d5cb6a313f022b0a5acb75c20e4..2fc3c81cf2a5987bfd0bb815bdc192a83abda8e7 100644 (file)
@@ -289,11 +289,7 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen)
        struct cdb_iter_args iter_args;
        int ret;
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               candidate = nb_config_dup(running_config);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       candidate = nb_config_dup(running_config);
 
        /* Iterate over all configuration changes. */
        iter_args.candidate = candidate;
index a55da23dd12de5cabce812aaef4a56180369f1c4..1d6317b005511419d3fe43048513a5217b421c62 100644 (file)
@@ -702,15 +702,10 @@ class NorthboundImpl final : public frr::Northbound::Service
        {
                struct lyd_node *dnode;
 
-               pthread_rwlock_rdlock(&running_config->lock);
-               {
-                       dnode = yang_dnode_get(running_config->dnode,
-                                              path.empty() ? NULL
-                                                           : path.c_str());
-                       if (dnode)
-                               dnode = yang_dnode_dup(dnode);
-               }
-               pthread_rwlock_unlock(&running_config->lock);
+               dnode = yang_dnode_get(running_config->dnode,
+                                      path.empty() ? NULL : path.c_str());
+               if (dnode)
+                       dnode = yang_dnode_dup(dnode);
 
                return dnode;
        }
@@ -817,11 +812,7 @@ class NorthboundImpl final : public frr::Northbound::Service
 
                struct candidate *candidate = &_candidates[candidate_id];
                candidate->id = candidate_id;
-               pthread_rwlock_rdlock(&running_config->lock);
-               {
-                       candidate->config = nb_config_dup(running_config);
-               }
-               pthread_rwlock_unlock(&running_config->lock);
+               candidate->config = nb_config_dup(running_config);
                candidate->transaction = NULL;
 
                return candidate;
index 77183282bab1ffff83a2f75e099a30817ea17d3e..b94c939763341645efcc129ca4dc387dd0dd8745 100644 (file)
@@ -256,11 +256,7 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
                return ret;
        }
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               candidate = nb_config_dup(running_config);
-       }
-       pthread_rwlock_unlock(&running_config->lock);
+       candidate = nb_config_dup(running_config);
 
        while ((ret = sr_get_change_next(session, it, &sr_op, &sr_old_val,
                                         &sr_new_val))
index deb9391bd506a0951bf6e8108e1944ff2dcf441e..502d2c9d04bdf35b72d640ede49c1bb9814d784a 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2582,22 +2582,17 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
        vty->private_config = private_config;
        vty->xpath_index = 0;
 
-       pthread_rwlock_rdlock(&running_config->lock);
-       {
-               if (private_config) {
-                       vty->candidate_config = nb_config_dup(running_config);
+       if (private_config) {
+               vty->candidate_config = nb_config_dup(running_config);
+               vty->candidate_config_base = nb_config_dup(running_config);
+               vty_out(vty,
+                       "Warning: uncommitted changes will be discarded on exit.\n\n");
+       } else {
+               vty->candidate_config = vty_shared_candidate_config;
+               if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL)
                        vty->candidate_config_base =
                                nb_config_dup(running_config);
-                       vty_out(vty,
-                               "Warning: uncommitted changes will be discarded on exit.\n\n");
-               } else {
-                       vty->candidate_config = vty_shared_candidate_config;
-                       if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL)
-                               vty->candidate_config_base =
-                                       nb_config_dup(running_config);
-               }
        }
-       pthread_rwlock_unlock(&running_config->lock);
 
        return CMD_SUCCESS;
 }
index cd2ddb1eba778aeb4eb89cbcdee7abc625502658..1b5a582cb1006014c47f03d3caa7b435d624f732 100644 (file)
@@ -3660,18 +3660,14 @@ static int rip_vrf_enable(struct vrf *vrf)
                if (yang_module_find("frr-ripd") && old_vrf_name) {
                        struct lyd_node *rip_dnode;
 
-                       pthread_rwlock_wrlock(&running_config->lock);
-                       {
-                               rip_dnode = yang_dnode_get(
-                                                  running_config->dnode,
-                                                  "/frr-ripd:ripd/instance[vrf='%s']/vrf",
-                                                  old_vrf_name);
-                               if (rip_dnode) {
-                                       yang_dnode_change_leaf(rip_dnode, vrf->name);
-                                       running_config->version++;
-                               }
+                       rip_dnode = yang_dnode_get(
+                               running_config->dnode,
+                               "/frr-ripd:ripd/instance[vrf='%s']/vrf",
+                               old_vrf_name);
+                       if (rip_dnode) {
+                               yang_dnode_change_leaf(rip_dnode, vrf->name);
+                               running_config->version++;
                        }
-                       pthread_rwlock_unlock(&running_config->lock);
                }
                if (old_vrf_name)
                        XFREE(MTYPE_RIP_VRF_NAME, old_vrf_name);
index 120f46f0da70a915409f7df8248bbd08f67f2967..ef4e474737d58614eecf5169d792cf75382210fa 100644 (file)
@@ -2795,18 +2795,14 @@ static int ripng_vrf_enable(struct vrf *vrf)
                if (yang_module_find("frr-ripngd") && old_vrf_name) {
                        struct lyd_node *ripng_dnode;
 
-                       pthread_rwlock_wrlock(&running_config->lock);
-                       {
-                               ripng_dnode = yang_dnode_get(
-                                                  running_config->dnode,
-                                                  "/frr-ripngd:ripngd/instance[vrf='%s']/vrf",
-                                                  old_vrf_name);
-                               if (ripng_dnode) {
-                                       yang_dnode_change_leaf(ripng_dnode, vrf->name);
-                                       running_config->version++;
-                               }
+                       ripng_dnode = yang_dnode_get(
+                               running_config->dnode,
+                               "/frr-ripngd:ripngd/instance[vrf='%s']/vrf",
+                               old_vrf_name);
+                       if (ripng_dnode) {
+                               yang_dnode_change_leaf(ripng_dnode, vrf->name);
+                               running_config->version++;
                        }
-                       pthread_rwlock_unlock(&running_config->lock);
                }
                if (old_vrf_name)
                        XFREE(MTYPE_RIPNG_VRF_NAME, old_vrf_name);