<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/16162">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">vty: track parent nodes also for telnet sessions<br><br>Keep track of parent nodes and go back hierarchically, not only for .cfg file<br>reading, but also for telnet VTY sessions.<br><br>A long time ago cfg file parsing was made strictly hierarchical: node exits go<br>back to parent nodes exactly as they were entered. However, live telnet VTY<br>sessions still lacked this and depended on the go_parent_cb().<br><br>From this commit on, implementing a go_parent_cb() is completely optional. The<br>go_parent_cb() no longer has the task to determine the correct parent node,<br>neither for cfg files (as already the case before this patch) nor for telnet<br>VTY sessions (added by this patch). Instead, a go_parent_cb() implementation<br>can merely take actions it requires on node exits, for example applying some<br>config when leaving a specific node.<br><br>The node value that is returned by the go_parent_cb() and the vty->node and<br>vty->index values that might be set are completely ignored; instead the<br>implicit parent node tracking determines the parent and node object.<br><br>As a side effect, the is_config_node() callback is no longer needed, since the<br>VTY now always implicitly knows when to exit back to the CONFIG_NODE.<br><br>For example, osmo_ss7_is_config_node() could now be dropped, and the<br>osmo_ss7_vty_go_parent() could be shortened by five switch cases, does no<br>longer need to set vty->node nor vty->index and could thus be shortened to:<br><br>int osmo_ss7_vty_go_parent(struct vty *vty)<br>{<br> struct osmo_ss7_asp *asp;<br> struct osmo_xua_server *oxs;<br><br> switch (vty->node) {<br> case L_CS7_ASP_NODE:<br> asp = vty->index;<br> /* If no local addr was set */<br> if (!asp->cfg.local.host_cnt) {<br> asp->cfg.local.host[0] = NULL;<br> asp->cfg.local.host_cnt = 1;<br> }<br> osmo_ss7_asp_restart(asp);<br> break;<br> case L_CS7_XUA_NODE:<br> oxs = vty->index;<br> /* If no local addr was set, or erased after _create(): */<br> if (!oxs->cfg.local.host_cnt)<br> osmo_ss7_xua_server_set_local_host(oxs, NULL);<br> if (osmo_ss7_xua_server_bind(oxs) < 0)<br> vty_out(vty, "%% Unable to bind xUA server to IP(s)%s", VTY_NEWLINE);<br> break;<br> }<br> return 0;<br>}<br><br>Before parent tracking, every program was required to write a go_parent_cb()<br>which has to return every node's parent node, basically a switch() statement<br>that manually traces the way back out of child nodes. If the go_parent_cb() has<br>errors, we may wildly jump around the node tree: a common error is to jump<br>right out to the top config node with one exit, even though we were N levels<br>deep. This kind of error has been eliminated for cfg files long ago, but still<br>exists for telnet VTY sessions, which this patch fixes.<br><br>This came up when I was adding multi-level config nodes to osmo-hlr to support<br>Distributed GSM / remote MS lookup: the config file worked fine, while vty node<br>tests failed to exit to the correct nodes.<br><br>Change-Id: I2b32b4fe20732728db6e9cdac7e484d96ab86dc5<br>---<br>M include/osmocom/vty/vty.h<br>M src/vty/command.c<br>M tests/vty/vty_test.c<br>3 files changed, 31 insertions(+), 48 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/62/16162/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/vty/vty.h b/include/osmocom/vty/vty.h</span><br><span>index 03a2924..9acaa7d 100644</span><br><span>--- a/include/osmocom/vty/vty.h</span><br><span>+++ b/include/osmocom/vty/vty.h</span><br><span>@@ -178,9 +178,14 @@</span><br><span> const char *copyright;</span><br><span> /*! \ref talloc context */</span><br><span> void *tall_ctx;</span><br><span style="color: hsl(0, 100%, 40%);">- /*! call-back for returning to parent n ode */</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Call-back for taking actions upon exiting a node.</span><br><span style="color: hsl(120, 100%, 40%);">+ * The return value is ignored, and changes to vty->node and vty->index made in this callback are ignored.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Implicit parent node tracking always sets the correct parent node and vty->index after this callback exits,</span><br><span style="color: hsl(120, 100%, 40%);">+ * so this callback can handle only those nodes that should take specific actions upon node exit, or can be left</span><br><span style="color: hsl(120, 100%, 40%);">+ * NULL entirely. */</span><br><span> int (*go_parent_cb)(struct vty *vty);</span><br><span style="color: hsl(0, 100%, 40%);">- /*! call-back to determine if node is config node */</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! OBSOLETED: Implicit parent node tracking has replaced the use of this callback. This callback is no longer</span><br><span style="color: hsl(120, 100%, 40%);">+ * called, ever, and can be left NULL. */</span><br><span> int (*is_config_node)(struct vty *vty, int node);</span><br><span> /*! Check if the config is consistent before write */</span><br><span> int (*config_is_consistent)(struct vty *vty);</span><br><span>diff --git a/src/vty/command.c b/src/vty/command.c</span><br><span>index 6a9d18a..daee5c5 100644</span><br><span>--- a/src/vty/command.c</span><br><span>+++ b/src/vty/command.c</span><br><span>@@ -200,18 +200,6 @@</span><br><span> return strcmp(a->cmd, b->cmd);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int is_config_child(struct vty *vty)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- if (vty->node <= CONFIG_NODE)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- else if (vty->node > CONFIG_NODE && vty->node < _LAST_OSMOVTY_NODE)</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">- else if (host.app_info->is_config_node)</span><br><span style="color: hsl(0, 100%, 40%);">- return host.app_info->is_config_node(vty, vty->node);</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- return vty->node > CONFIG_NODE;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /*! Sort each node's command element according to command string. */</span><br><span> void sort_node(void)</span><br><span> {</span><br><span>@@ -2187,25 +2175,10 @@</span><br><span> vty_clear_parents(vty);</span><br><span> break;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- case CFG_LOG_NODE:</span><br><span style="color: hsl(0, 100%, 40%);">- case VTY_NODE:</span><br><span style="color: hsl(0, 100%, 40%);">- vty->node = CONFIG_NODE;</span><br><span style="color: hsl(0, 100%, 40%);">- vty_clear_parents(vty);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> default:</span><br><span style="color: hsl(0, 100%, 40%);">- if (host.app_info->go_parent_cb) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (host.app_info->go_parent_cb)</span><br><span> host.app_info->go_parent_cb(vty);</span><br><span style="color: hsl(0, 100%, 40%);">- vty_pop_parent(vty);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- else if (is_config_child(vty)) {</span><br><span style="color: hsl(0, 100%, 40%);">- vty->node = CONFIG_NODE;</span><br><span style="color: hsl(0, 100%, 40%);">- vty_clear_parents(vty);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- else {</span><br><span style="color: hsl(0, 100%, 40%);">- vty->node = VIEW_NODE;</span><br><span style="color: hsl(0, 100%, 40%);">- vty_clear_parents(vty);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_pop_parent(vty);</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span>@@ -2365,9 +2338,30 @@</span><br><span> </span><br><span> if (matched_element->daemon)</span><br><span> rc = CMD_SUCCESS_DAEMON;</span><br><span style="color: hsl(0, 100%, 40%);">- else /* Execute matched command. */</span><br><span style="color: hsl(120, 100%, 40%);">+ else {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Execute matched command. */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct vty_parent_node this_node = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .node = vty->node,</span><br><span style="color: hsl(120, 100%, 40%);">+ .priv = vty->priv,</span><br><span style="color: hsl(120, 100%, 40%);">+ .indent = vty->indent,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+ struct vty_parent_node *parent = vty_parent(vty);</span><br><span> rc = (*matched_element->func) (matched_element, vty, argc, argv);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* If we have stepped down into a child node, push a parent frame.</span><br><span style="color: hsl(120, 100%, 40%);">+ * The causality is such: we don't expect every single node entry implementation to push</span><br><span style="color: hsl(120, 100%, 40%);">+ * a parent node entry onto vty->parent_nodes. Instead we expect vty_go_parent() to *pop*</span><br><span style="color: hsl(120, 100%, 40%);">+ * a parent node. Hence if the node changed without the parent node changing, we must</span><br><span style="color: hsl(120, 100%, 40%);">+ * have stepped into a child node. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (vty->node != this_node.node && parent == vty_parent(vty)</span><br><span style="color: hsl(120, 100%, 40%);">+ && vty->node > CONFIG_NODE) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Push the parent node. */</span><br><span style="color: hsl(120, 100%, 40%);">+ parent = talloc_zero(vty, struct vty_parent_node);</span><br><span style="color: hsl(120, 100%, 40%);">+ *parent = this_node;</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_add(&parent->entry, &vty->parent_nodes);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> rc_free_deopt_ctx:</span><br><span> /* Now after we called the command func, we can free temporary strings */</span><br><span> talloc_free(cmd_deopt_ctx);</span><br><span>diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c</span><br><span>index 1139638..9627b6d 100644</span><br><span>--- a/tests/vty/vty_test.c</span><br><span>+++ b/tests/vty/vty_test.c</span><br><span>@@ -482,27 +482,11 @@</span><br><span> destroy_test_vty(&test, vty);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int go_parent_cb(struct vty *vty)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * - For the interactive VTY tests above, it is expected to bounce back to</span><br><span style="color: hsl(0, 100%, 40%);">- * the CONFIG_NODE. Hence do so in go_parent_cb().</span><br><span style="color: hsl(0, 100%, 40%);">- * - In the config file parsing tests, setting vty->node in go_parent_cb() has no</span><br><span style="color: hsl(0, 100%, 40%);">- * effect, because we will subsequently pop a parent node from the parent stack</span><br><span style="color: hsl(0, 100%, 40%);">- * and override to go to the node that was recorded as the actual parent.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- vty->node = CONFIG_NODE;</span><br><span style="color: hsl(0, 100%, 40%);">- vty->index = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> int main(int argc, char **argv)</span><br><span> {</span><br><span> struct vty_app_info vty_info = {</span><br><span> .name = "VtyTest",</span><br><span> .version = 0,</span><br><span style="color: hsl(0, 100%, 40%);">- .go_parent_cb = go_parent_cb,</span><br><span style="color: hsl(0, 100%, 40%);">- .is_config_node = NULL,</span><br><span> };</span><br><span> </span><br><span> const struct log_info_cat default_categories[] = {};</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/16162">change 16162</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/16162"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2b32b4fe20732728db6e9cdac7e484d96ab86dc5 </div>
<div style="display:none"> Gerrit-Change-Number: 16162 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>