From d31de237582f6fe3315d61bb9a488d4cda92654e Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Thu, 31 Oct 2019 16:09:23 +0100 Subject: vty: track parent nodes also for telnet sessions Keep track of parent nodes and go back hierarchically, not only for .cfg file reading, but also for telnet VTY sessions. A long time ago cfg file parsing was made strictly hierarchical: node exits go back to parent nodes exactly as they were entered. However, live telnet VTY sessions still lacked this and depended on the go_parent_cb(). From this commit on, implementing a go_parent_cb() is completely optional. The go_parent_cb() no longer has the task to determine the correct parent node, neither for cfg files (as already the case before this patch) nor for telnet VTY sessions (added by this patch). Instead, a go_parent_cb() implementation can merely take actions it requires on node exits, for example applying some config when leaving a specific node. The node value that is returned by the go_parent_cb() and the vty->node and vty->index values that might be set are completely ignored; instead the implicit parent node tracking determines the parent and node object. As a side effect, the is_config_node() callback is no longer needed, since the VTY now always implicitly knows when to exit back to the CONFIG_NODE. For example, osmo_ss7_is_config_node() could now be dropped, and the osmo_ss7_vty_go_parent() could be shortened by five switch cases, does no longer need to set vty->node nor vty->index and could thus be shortened to: int osmo_ss7_vty_go_parent(struct vty *vty) { struct osmo_ss7_asp *asp; struct osmo_xua_server *oxs; switch (vty->node) { case L_CS7_ASP_NODE: asp = vty->index; /* If no local addr was set */ if (!asp->cfg.local.host_cnt) { asp->cfg.local.host[0] = NULL; asp->cfg.local.host_cnt = 1; } osmo_ss7_asp_restart(asp); break; case L_CS7_XUA_NODE: oxs = vty->index; /* If no local addr was set, or erased after _create(): */ if (!oxs->cfg.local.host_cnt) osmo_ss7_xua_server_set_local_host(oxs, NULL); if (osmo_ss7_xua_server_bind(oxs) < 0) vty_out(vty, "%% Unable to bind xUA server to IP(s)%s", VTY_NEWLINE); break; } return 0; } Before parent tracking, every program was required to write a go_parent_cb() which has to return every node's parent node, basically a switch() statement that manually traces the way back out of child nodes. If the go_parent_cb() has errors, we may wildly jump around the node tree: a common error is to jump right out to the top config node with one exit, even though we were N levels deep. This kind of error has been eliminated for cfg files long ago, but still exists for telnet VTY sessions, which this patch fixes. This came up when I was adding multi-level config nodes to osmo-hlr to support Distributed GSM / remote MS lookup: the config file worked fine, while vty node tests failed to exit to the correct nodes. Change-Id: I2b32b4fe20732728db6e9cdac7e484d96ab86dc5 --- src/vty/command.c | 54 ++++++++++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 30 deletions(-) (limited to 'src/vty') diff --git a/src/vty/command.c b/src/vty/command.c index 6a9d18af..daee5c5a 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -200,18 +200,6 @@ static int cmp_desc(const void *p, const void *q) return strcmp(a->cmd, b->cmd); } -static int is_config_child(struct vty *vty) -{ - if (vty->node <= CONFIG_NODE) - return 0; - else if (vty->node > CONFIG_NODE && vty->node < _LAST_OSMOVTY_NODE) - return 1; - else if (host.app_info->is_config_node) - return host.app_info->is_config_node(vty, vty->node); - else - return vty->node > CONFIG_NODE; -} - /*! Sort each node's command element according to command string. */ void sort_node(void) { @@ -2187,25 +2175,10 @@ int vty_go_parent(struct vty *vty) vty_clear_parents(vty); break; - case CFG_LOG_NODE: - case VTY_NODE: - vty->node = CONFIG_NODE; - vty_clear_parents(vty); - break; - default: - if (host.app_info->go_parent_cb) { + if (host.app_info->go_parent_cb) host.app_info->go_parent_cb(vty); - vty_pop_parent(vty); - } - else if (is_config_child(vty)) { - vty->node = CONFIG_NODE; - vty_clear_parents(vty); - } - else { - vty->node = VIEW_NODE; - vty_clear_parents(vty); - } + vty_pop_parent(vty); break; } @@ -2365,9 +2338,30 @@ cmd_execute_command_real(vector vline, struct vty *vty, if (matched_element->daemon) rc = CMD_SUCCESS_DAEMON; - else /* Execute matched command. */ + else { + /* Execute matched command. */ + struct vty_parent_node this_node = { + .node = vty->node, + .priv = vty->priv, + .indent = vty->indent, + }; + struct vty_parent_node *parent = vty_parent(vty); rc = (*matched_element->func) (matched_element, vty, argc, argv); + /* If we have stepped down into a child node, push a parent frame. + * The causality is such: we don't expect every single node entry implementation to push + * a parent node entry onto vty->parent_nodes. Instead we expect vty_go_parent() to *pop* + * a parent node. Hence if the node changed without the parent node changing, we must + * have stepped into a child node. */ + if (vty->node != this_node.node && parent == vty_parent(vty) + && vty->node > CONFIG_NODE) { + /* Push the parent node. */ + parent = talloc_zero(vty, struct vty_parent_node); + *parent = this_node; + llist_add(&parent->entry, &vty->parent_nodes); + } + } + rc_free_deopt_ctx: /* Now after we called the command func, we can free temporary strings */ talloc_free(cmd_deopt_ctx); -- cgit v1.2.3