*: remove the configuration lock from all daemons

A while ago all FRR configuration commands were converted to use the
QOBJ infrastructure to keep track of configuration objects. This
means the configuration lock isn't necessary anymore because the
QOBJ code detects when someones tries to edit a configuration object
that was deleted and react accordingly (log an error and abort the
command).  The possibility of accessing dangling pointers doesn't
exist anymore since vty->index was removed.

Summary of the changes:
* remove the configuration lock and the vty_config_lockless() function.
* rename vty_config_unlock() to vty_config_exit() since we need to
  clean up a few things when exiting from the configuration mode.
* rename vty_config_lock() to vty_config_enter() to remove code
  duplication that existed between the three different "configuration"
  commands (terminal, private and exclusive).

Configuration commands converted to the new northbound model don't
need the configuration lock either since the northbound API also
detects when someone tries to edit a configuration object that
doesn't exist anymore.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c
index 77bd2eaefa79caa8020d5d305846016243779960..c5d38f30093e51de41a8e4ae16db7c890f2a28c5 100644 (file)
--- a/bgpd/bgp_rpki.c
+++ b/bgpd/bgp_rpki.c
@@ -1282,7 +1282,7 @@ DEFUN_NOSH (rpki_end,
 {
        int ret = reset(false);
 
-       vty_config_unlock(vty);
+       vty_config_exit(vty);
        vty->node = ENABLE_NODE;
        return ret == SUCCESS ? CMD_SUCCESS : CMD_WARNING;
 }

diff --git a/isisd/isis_main.c b/isisd/isis_main.c
index c325a3d6fe9c34c5fedeab77ff2da8d828826cce..2d540348e450fbfb689d569be48bf7060b04bb34 100644 (file)
--- a/isisd/isis_main.c
+++ b/isisd/isis_main.c
@@ -203,7 +203,6 @@ int main(int argc, char **argv, char **envp)
                }
        }
 
-       vty_config_lockless();
        /* thread master */
        master = frr_init();
 

diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c
index 1280567f830eca937d4c125246feab2566fdf577..771d3b7459a0ce97521e3151b7872a2f63ca8a1a 100644 (file)
--- a/ldpd/ldpd.c
+++ b/ldpd/ldpd.c
@@ -335,7 +335,6 @@ main(int argc, char *argv[])
 
        master = frr_init();
 
-       vty_config_lockless();
        vrf_init(NULL, NULL, NULL, NULL, NULL);
        access_list_init();
        ldp_vty_init();

diff --git a/lib/command.c b/lib/command.c
index bd000c37465d1f7859edf80449f41703a0875ba2..80ba86db373ed04744fc87596ee86e65d7ac8012 100644 (file)
--- a/lib/command.c
+++ b/lib/command.c
@@ -1386,19 +1386,7 @@ DEFUN (config_terminal,
        "Configuration from vty interface\n"
        "Configuration terminal\n")
 {
-       if (vty_config_lock(vty))
-               vty->node = CONFIG_NODE;
-       else {
-               vty_out(vty, "VTY configuration is locked by other VTY\n");
-               return CMD_WARNING_CONFIG_FAILED;
-       }
-
-       vty->private_config = false;
-       vty->candidate_config = vty_shared_candidate_config;
-       if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL)
-               vty->candidate_config_base = nb_config_dup(running_config);
-
-       return CMD_SUCCESS;
+       return vty_config_enter(vty, false, false);
 }
 
 /* Enable command */
@@ -1450,7 +1438,7 @@ void cmd_exit(struct vty *vty)
                break;
        case CONFIG_NODE:
                vty->node = ENABLE_NODE;
-               vty_config_unlock(vty);
+               vty_config_exit(vty);
                break;
        case INTERFACE_NODE:
        case PW_NODE:
@@ -1594,7 +1582,7 @@ DEFUN (config_end,
        case LINK_PARAMS_NODE:
        case BFD_NODE:
        case BFD_PEER_NODE:
-               vty_config_unlock(vty);
+               vty_config_exit(vty);
                vty->node = ENABLE_NODE;
                break;
        default:

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index 8ae44e72d52b0b1d800ffcdc0cab574d03d5b31e..01f577fd5bc7e55d8d4b121ccd6cf5d4665177a3 100644 (file)
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -492,20 +492,7 @@ DEFUN (config_exclusive,
        "Configuration from vty interface\n"
        "Configure exclusively from this terminal\n")
 {
-       if (vty_config_exclusive_lock(vty))
-               vty->node = CONFIG_NODE;
-       else {
-               vty_out(vty, "VTY configuration is locked by other VTY\n");
-               return CMD_WARNING_CONFIG_FAILED;
-       }
-
-       vty->private_config = true;
-       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");
-
-       return CMD_SUCCESS;
+       return vty_config_enter(vty, true, true);
 }
 
 /* Configure using a private candidate configuration. */
@@ -515,20 +502,7 @@ DEFUN (config_private,
        "Configuration from vty interface\n"
        "Configure using a private candidate configuration\n")
 {
-       if (vty_config_lock(vty))
-               vty->node = CONFIG_NODE;
-       else {
-               vty_out(vty, "VTY configuration is locked by other VTY\n");
-               return CMD_WARNING_CONFIG_FAILED;
-       }
-
-       vty->private_config = true;
-       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");
-
-       return CMD_SUCCESS;
+       return vty_config_enter(vty, true, false);
 }
 
 DEFPY (config_commit,

diff --git a/lib/vty.c b/lib/vty.c
index 811c23c2186a4dc818b6eee27cdee5808d0f5202..9908ada7f04607eab8cf576f64222fafcda976e1 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -86,10 +86,6 @@ static vector Vvty_serv_thread;
 /* Current directory. */
 char *vty_cwd = NULL;
 
-/* Configure lock. */
-static int vty_config;
-static int vty_config_is_lockless = 0;
-
 /* Exclusive configuration lock. */
 struct vty *vty_exclusive_lock;
 
@@ -824,7 +820,7 @@ static void vty_end_config(struct vty *vty)
        case BGP_EVPN_VNI_NODE:
        case BFD_NODE:
        case BFD_PEER_NODE:
-               vty_config_unlock(vty);
+               vty_config_exit(vty);
                vty->node = ENABLE_NODE;
                break;
        default:
@@ -1225,7 +1221,7 @@ static void vty_stop_input(struct vty *vty)
        case VTY_NODE:
        case BFD_NODE:
        case BFD_PEER_NODE:
-               vty_config_unlock(vty);
+               vty_config_exit(vty);
                vty->node = ENABLE_NODE;
                break;
        default:
@@ -2351,7 +2347,7 @@ void vty_close(struct vty *vty)
        }
 
        /* Check configure. */
-       vty_config_unlock(vty);
+       vty_config_exit(vty);
 
        /* OK free vty. */
        XFREE(MTYPE_VTY, vty);
@@ -2690,18 +2686,33 @@ void vty_log_fixed(char *buf, size_t len)
        }
 }
 
-int vty_config_lock(struct vty *vty)
+int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
 {
-       if (vty_config_is_lockless)
-               return 1;
-       if (vty_config == 0) {
-               vty->config = 1;
-               vty_config = 1;
+       if (exclusive && !vty_config_exclusive_lock(vty)) {
+               vty_out(vty, "VTY configuration is locked by other VTY\n");
+               return CMD_WARNING;
+       }
+
+       vty->node = CONFIG_NODE;
+       vty->config = true;
+       vty->private_config = private_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);
        }
-       return vty->config;
+
+       return CMD_SUCCESS;
 }
 
-int vty_config_unlock(struct vty *vty)
+void vty_config_exit(struct vty *vty)
 {
        vty_config_exclusive_unlock(vty);
 
@@ -2714,19 +2725,6 @@ int vty_config_unlock(struct vty *vty)
                nb_config_free(vty->candidate_config_base);
                vty->candidate_config_base = NULL;
        }
-
-       if (vty_config_is_lockless)
-               return 0;
-       if (vty_config == 1 && vty->config == 1) {
-               vty->config = 0;
-               vty_config = 0;
-       }
-       return vty->config;
-}
-
-void vty_config_lockless(void)
-{
-       vty_config_is_lockless = 1;
 }
 
 int vty_config_exclusive_lock(struct vty *vty)

diff --git a/lib/vty.h b/lib/vty.h
index 4c434fb2f206eaa43c6113d622528e53119ec47a..39d63b4bac3c8bf480f085795ba82b83e0e951e2 100644 (file)
--- a/lib/vty.h
+++ b/lib/vty.h
@@ -102,6 +102,9 @@ struct vty {
        int xpath_index;
        char xpath[VTY_MAXDEPTH][XPATH_MAXLEN];
 
+       /* In configure mode. */
+       bool config;
+
        /* Private candidate configuration mode. */
        bool private_config;
 
@@ -149,9 +152,6 @@ struct vty {
        /* Terminal monitor. */
        int monitor;
 
-       /* In configure mode. */
-       int config;
-
        /* Read and write thread. */
        struct thread *t_read;
        struct thread *t_write;
@@ -299,9 +299,9 @@ extern void vty_close(struct vty *);
 extern char *vty_get_cwd(void);
 extern void vty_log(const char *level, const char *proto, const char *fmt,
                    struct timestamp_control *, va_list);
-extern int vty_config_lock(struct vty *);
-extern int vty_config_unlock(struct vty *);
-extern void vty_config_lockless(void);
+extern int vty_config_enter(struct vty *vty, bool private_config,
+                           bool exclusive);
+extern void vty_config_exit(struct vty *);
 extern int vty_config_exclusive_lock(struct vty *vty);
 extern void vty_config_exclusive_unlock(struct vty *vty);
 extern int vty_shell(struct vty *);

diff --git a/ripd/rip_main.c b/ripd/rip_main.c
index 4ee5994a9d1176438912b467d9f9541df526fd21..5db9c4b7e9e0250d9240a9404d5163de9db210e6 100644 (file)
--- a/ripd/rip_main.c
+++ b/ripd/rip_main.c
@@ -165,8 +165,6 @@ int main(int argc, char **argv)
                }
        }
 
-       vty_config_lockless();
-
        /* Prepare master thread. */
        master = frr_init();
 

diff --git a/zebra/main.c b/zebra/main.c
index 5628b5e022434bba5dcf654a1dbced3577852d3f..95fe6f9fef8f9ea13486096a859af2f6fb2ff7f5 100644 (file)
--- a/zebra/main.c
+++ b/zebra/main.c
@@ -387,7 +387,6 @@ int main(int argc, char **argv)
                }
        }
 
-       vty_config_lockless();
        zebrad.master = frr_init();
 
        /* Zebra related initialize. */