vtysh: remove resync workaround when exiting to config node

When exiting from a level below the config node, like `router rip`, vtysh executes a resync by sending "end" and "conf term [file-lock]" commands to all the daemons. As statet in the description comment, it's done "in case one of the daemons is somewhere else". I don't think this actually ever happens, but even if it is, it is a bug in a daemon that needs to be fixed. This resync was okay before the introduction of mgmtd, but now it unlocks and locks back the datastores during the configuration reading process, which can lead to a failure which is explained in the previous commit. Signed-off-by: Igor Ryzhov <[email protected]>
FRRouting · Feb 7, 2024 · 2574f03 · 2574f03
1 parent 0db4d55
commit 2574f03
Show file tree

Hide file tree

Showing 3 changed files with 1 addition and 35 deletions.
diff --git a/lib/vty.h b/lib/vty.h
@@ -232,10 +232,6 @@ struct vty {
 	uintptr_t mgmt_req_pending_data;
 	bool mgmt_locked_candidate_ds;
 	bool mgmt_locked_running_ds;
-	/* Need to track when we file-lock in vtysh to re-lock on end/conf t
-	 * workaround
-	 */
-	bool vtysh_file_locked;
 };
 
 static inline void vty_push_context(struct vty *vty, int node, uint64_t id)

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
@@ -1669,7 +1669,6 @@ static int vtysh_end(void)
 		/* Nothing to do. */
 		break;
 	default:
-		vty->vtysh_file_locked = false;
 		vty->node = ENABLE_NODE;
 		break;
 	}
@@ -2393,23 +2392,12 @@ DEFUNSH(VTYSH_REALLYALL, vtysh_disable, vtysh_disable_cmd, "disable",
 }
 
 DEFUNSH(VTYSH_REALLYALL, vtysh_config_terminal, vtysh_config_terminal_cmd,
-	"configure [terminal]",
-	"Configuration from vty interface\n"
-	"Configuration terminal\n")
-{
-	vty->node = CONFIG_NODE;
-	return CMD_SUCCESS;
-}
-
-DEFUNSH(VTYSH_REALLYALL, vtysh_config_terminal_file_lock,
-	vtysh_config_terminal_file_lock_cmd,
-	"configure terminal file-lock",
+	"configure [terminal [file-lock]]",
 	"Configuration from vty interface\n"
 	"Configuration terminal\n"
 	"Configuration with locked datastores\n")
 {
 	vty->node = CONFIG_NODE;
-	vty->vtysh_file_locked = true;
 	return CMD_SUCCESS;
 }
 
@@ -2424,21 +2412,6 @@ static int vtysh_exit(struct vty *vty)
 	if (cnode->parent_node)
 		vty->node = cnode->parent_node;
 
-	if (vty->node == CONFIG_NODE) {
-		bool locked = vty->vtysh_file_locked;
-
-		/* resync in case one of the daemons is somewhere else */
-		vtysh_execute("end");
-		/* NOTE: a rather expensive thing to do, can we avoid it? */
-
-		if (locked)
-			vtysh_execute("configure terminal file-lock");
-		else
-			vtysh_execute("configure terminal");
-	} else if (vty->node == ENABLE_NODE) {
-		vty->vtysh_file_locked = false;
-	}
-
 	return CMD_SUCCESS;
 }
 
@@ -5125,7 +5098,6 @@ void vtysh_init_vty(void)
 	if (!user_mode)
 		install_element(VIEW_NODE, &vtysh_enable_cmd);
 	install_element(ENABLE_NODE, &vtysh_config_terminal_cmd);
-	install_element(ENABLE_NODE, &vtysh_config_terminal_file_lock_cmd);
 	install_element(ENABLE_NODE, &vtysh_disable_cmd);
 
 	/* "exit" command. */

diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
@@ -623,7 +623,6 @@ static int vtysh_read_file(FILE *confp, bool dry_run)
 	 */
 	while (vtysh_execute_no_pager("conf term file-lock") == CMD_WARNING_CONFIG_FAILED)
 		usleep(100000);
-	vty->vtysh_file_locked = true;
 
 	if (!dry_run)
 		vtysh_execute_no_pager("XFRR_start_configuration");
@@ -635,7 +634,6 @@ static int vtysh_read_file(FILE *confp, bool dry_run)
 		vtysh_execute_no_pager("XFRR_end_configuration");
 
 	vtysh_execute_no_pager("end");
-	vty->vtysh_file_locked = false;
 	vtysh_execute_no_pager("disable");
 
 	vty_close(vty);