Change '%s(bsc)#' to '%s(config-bsc)# '. The missing trailing blank brakes osmopy's VTYInteract.command() because the blank is contained in the end patterns which are checked to decide whether to leave the select loop. Thus trying to execute the 'bsc' command there blocks the test script forever. --- openbsc/src/osmo-bsc/osmo_bsc_vty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_vty.c b/openbsc/src/osmo-bsc/osmo_bsc_vty.c index 8eaa55b..f6cf1a0 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_vty.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_vty.c @@ -43,7 +43,7 @@ static struct osmo_msc_data *osmo_msc_data(struct vty *vty)
static struct cmd_node bsc_node = { BSC_NODE, - "%s(bsc)#", + "%s(config-bsc)# ", 1, };
These tests check for the availability of 'exit' and 'end' in each configuration node and for the node specific commands to traverse the tree. In addition, using these commands from within inner contexts is checked. This will detect problems, when an outer command word is a prefix of an inner command, like with 'mgcp' and 'mgcp-through-msc-ipa'.
Several assertions are disabled due to inconsistencies and missing commands (see above). --- openbsc/tests/vty_test_runner.py | 156 +++++++++++++++++++++++++++++++++++++- 1 file changed, 153 insertions(+), 3 deletions(-)
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 87a45bb..7b8498b 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -56,7 +56,53 @@ class TestVTYBase(unittest.TestCase): self.vty = None osmoutil.end_proc(self.proc)
-class TestVTYNITB(TestVTYBase): + +class TestVTYGenericBTS(TestVTYBase): + + def checkForEndAndExit(self): + res = self.vty.command("list") + #print ('looking for "exit"\n') + self.assert_(res.find(' exit\r') > 0) + #print 'found "exit"\nlooking for "end"\n' + self.assert_(res.find(' end\r') > 0) + #print 'found "end"\n' + + def ignoredCheckForEndAndExit(self): + sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n') + try: + self.checkForEndAndExit() + except BaseException as e: + sys.stderr.write('Expected and ignored failure: %s\n' % (str(e))) + + def notIgnoredTest(self): + sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n') + return False + + + def _testConfigNetworkTree(self): + self.vty.enable() + self.vty.verify("configure terminal",['']) + self.assertEquals(self.vty.node(), 'config') + # self.checkForEndAndExit() + self.vty.verify("network",['']) + self.assertEquals(self.vty.node(), 'config-net') + self.checkForEndAndExit() + self.vty.verify("bts 0",['']) + self.assertEquals(self.vty.node(), 'config-net-bts') + self.checkForEndAndExit() + self.vty.verify("trx 0",['']) + self.assertEquals(self.vty.node(), 'config-net-bts-trx') + self.checkForEndAndExit() + self.vty.verify("exit",['']) + self.assertEquals(self.vty.node(), 'config-net-bts') + self.vty.verify("exit",['']) + self.assertEquals(self.vty.node(), 'config-net') + self.vty.verify("exit",['']) + self.assertEquals(self.vty.node(), 'config') + self.vty.verify("exit",['']) + self.assertTrue(self.vty.node() is None) + +class TestVTYNITB(TestVTYGenericBTS):
def vty_command(self): return ["./src/osmo-nitb/osmo-nitb", "-c", @@ -65,6 +111,39 @@ class TestVTYNITB(TestVTYBase): def vty_app(self): return (4242, "./src/osmo-nitb/osmo-nitb", "OpenBSC", "nitb")
+ def testConfigNetworkTree(self): + self._testConfigNetworkTree() + + def testVtyTree(self): + self.vty.enable() + self.vty.verify("configure terminal", ['']) + self.assertEquals(self.vty.node(), 'config') + self.ignoredCheckForEndAndExit() + self.vty.verify('mncc-int', ['']) + self.assertEquals(self.vty.node(), 'config-mncc-int') + self.checkForEndAndExit() + if self.notIgnoredTest(): + self.vty.verify('exit', ['']) + self.assertEquals(self.vty.node(), 'config') + else: + self.vty.verify("configure terminal", ['']) + self.vty.verify('smpp', ['']) + self.assertEquals(self.vty.node(), 'config-smpp') + self.ignoredCheckForEndAndExit() + self.vty.verify("exit", ['']) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + self.vty.verify("exit", ['']) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command("configure terminal") + self.vty.command('mncc-int') + self.vty.command('smpp') + self.assertEquals(self.vty.node(), 'config-smpp') + self.vty.command('mncc-int') + self.assertEquals(self.vty.node(), 'config-mncc-int') + def testEnableDisablePeriodicLU(self): self.vty.enable() self.vty.command("configure terminal") @@ -88,7 +167,7 @@ class TestVTYNITB(TestVTYBase): self.assertEquals(res.find('periodic location update 60'), -1) self.assert_(res.find('no periodic location update') > 0)
-class TestVTYBSC(TestVTYBase): +class TestVTYBSC(TestVTYGenericBTS):
def vty_command(self): return ["./src/osmo-bsc/osmo-bsc", "-c", @@ -97,6 +176,41 @@ class TestVTYBSC(TestVTYBase): def vty_app(self): return (4242, "./src/osmo-bsc/osmo-bsc", "OsmoBSC", "bsc")
+ def testConfigNetworkTree(self): + self._testConfigNetworkTree() + + def testVtyTree(self): + self.vty.enable() + self.vty.verify("configure terminal", ['']) + self.assertEquals(self.vty.node(), 'config') + # self.ignoredCheckForEndAndExit() + self.vty.verify("msc 0", ['']) + self.assertEquals(self.vty.node(), 'config-msc') + # self.ignoredCheckForEndAndExit() + self.vty.verify("exit", ['']) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + else: + self.vty.verify("configure terminal", ['']) + self.vty.command("bsc") + self.assertEquals(self.vty.node(), 'config-bsc') + self.ignoredCheckForEndAndExit() + self.vty.verify("exit", ['']) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + else: + self.vty.verify("configure terminal", ['']) + self.vty.verify("exit", ['']) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command("configure terminal") + self.vty.command('msc 0') + self.vty.command("bsc") + self.assertEquals(self.vty.node(), 'config-bsc') + self.vty.command("msc 0") + self.assertEquals(self.vty.node(), 'config-msc') + def testUssdNotifications(self): self.vty.enable() self.vty.command("configure terminal") @@ -128,7 +242,7 @@ class TestVTYBSC(TestVTYBase): self.assert_(res.find('no bsc-welcome-text') > 0) self.assertEquals(res.find('bsc-welcome-text Hello MS'), -1)
-class TestVTYNAT(TestVTYBase): +class TestVTYNAT(TestVTYGenericBTS):
def vty_command(self): return ["./src/osmo-bsc_nat/osmo-bsc_nat", "-c", @@ -137,6 +251,42 @@ class TestVTYNAT(TestVTYBase): def vty_app(self): return (4244, "src/osmo-bsc_nat/osmo-bsc_nat", "OsmoBSCNAT", "nat")
+ def testVtyTree(self): + self.vty.enable() + self.vty.verify('configure terminal', ['']) + self.assertEquals(self.vty.node(), 'config') + # self.checkForEndAndExit() + self.vty.verify('mgcp', ['']) + self.assertEquals(self.vty.node(), 'config-mgcp') + self.checkForEndAndExit() + self.vty.verify('exit', ['']) + self.assertEquals(self.vty.node(), 'config') + self.vty.verify('nat', ['']) + self.assertEquals(self.vty.node(), 'config-nat') + self.checkForEndAndExit() + self.vty.verify('bsc 0', ['']) + self.assertEquals(self.vty.node(), 'config-nat-bsc') + self.checkForEndAndExit() + self.vty.verify('exit', ['']) + self.assertEquals(self.vty.node(), 'config-nat') + self.vty.verify('exit', ['']) + self.assertEquals(self.vty.node(), 'config') + self.vty.verify('exit', ['']) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command('configure terminal') + self.vty.command('mgcp') + self.vty.command('nat') + self.assertEquals(self.vty.node(), 'config-nat') + self.vty.command('line vty') + self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('nat') + self.assertEquals(self.vty.node(), 'config-nat') + self.vty.command('bsc 0') + self.vty.command('line vty') + self.assertEquals(self.vty.node(), 'config-line') + def testRewriteNoRewrite(self): self.vty.enable() res = self.vty.command("configure terminal")
On Fri, Aug 30, 2013 at 06:33:59PM +0200, Jacob Erlbeck wrote:
-class TestVTYNITB(TestVTYBase):
+class TestVTYGenericBTS(TestVTYBase):
Why BTS? NITB/BSC mostly define the BSC functionality. I would call it TestVTYGenericBSC. I can change it locally if you do not object.
- def notIgnoredTest(self):
sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n')return False
I am still a bit puzzled here. Is this statement still accurate? Specially because this is used with a if/else statement.
On 08/31/2013 03:41 PM, Holger Hans Peter Freyther wrote:
On Fri, Aug 30, 2013 at 06:33:59PM +0200, Jacob Erlbeck wrote:
-class TestVTYNITB(TestVTYBase):
+class TestVTYGenericBTS(TestVTYBase):
Why BTS? NITB/BSC mostly define the BSC functionality. I would call it TestVTYGenericBSC. I can change it locally if you do not object.
Oops, that was a typo. Thanks for finding and fixing.
- def notIgnoredTest(self):
sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n')return FalseI am still a bit puzzled here. Is this statement still accurate? Specially because this is used with a if/else statement.
That's just an intermediate to keep the tests running until refactoring is done. So I didn't put much effort into this. We could even remove it with PATCH 5/6 of this set.
ournode_exit() duplicates most of bsc_vty_go_parent(). This patch fixes the inconsistencies of both functions within bsc_vty_go_parent() and replaces the implementation of ournode_exit() by a call to it. This makes 'exit' behave exactly like ^D in all openbsc nodes.
ournode_end() has been changed to walk through the intermediate nodes until one of the top nodes is reached. This allows for cleanups to be done on the way.
Note that in config mode if the tree is searched along the nodes toward the config node and a command is not found this way, a rollback is done by just replacing the vty's node and index member variable by the saved old values which might break the whole thing, when there has been a free() on the way. --- openbsc/src/libcommon/common_vty.c | 140 ++++++++++-------------------------- openbsc/tests/vty_test_runner.py | 65 +++++++++-------- 2 files changed, 68 insertions(+), 137 deletions(-)
diff --git a/openbsc/src/libcommon/common_vty.c b/openbsc/src/libcommon/common_vty.c index 7342f14..948bd49 100644 --- a/openbsc/src/libcommon/common_vty.c +++ b/openbsc/src/libcommon/common_vty.c @@ -48,6 +48,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts *bts = vty->index; vty->index = bts->network; + vty->index_sub = NULL; } break; case TRX_NODE: @@ -56,6 +57,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts_trx *trx = vty->index; vty->index = trx->bts; + vty->index_sub = &trx->bts->description; } break; case TS_NODE: @@ -64,18 +66,19 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts_trx_ts *ts = vty->index; vty->index = ts->trx; + vty->index_sub = &ts->trx->description; } break; case OML_NODE: case OM2K_NODE: vty->node = ENABLE_NODE; + /* NOTE: this only works because it's not part of the config + * tree, where outer commands are searched via vty_go_parent() + * and only (!) executed when a matching one is found. + */ talloc_free(vty->index); vty->index = NULL; break; - case NAT_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; case NAT_BSC_NODE: vty->node = NAT_NODE; { @@ -85,19 +88,31 @@ enum node_type bsc_vty_go_parent(struct vty *vty) break; case PGROUP_NODE: vty->node = NAT_NODE; + vty->index = NULL; break; case TRUNK_NODE: vty->node = MGCP_NODE; + vty->index = NULL; break; case SMPP_ESME_NODE: vty->node = SMPP_NODE; vty->index = NULL; break; case SMPP_NODE: + case MGCP_NODE: + case GBPROXY_NODE: + case SGSN_NODE: + case NAT_NODE: + case BSC_NODE: case MSC_NODE: case MNCC_INT_NODE: default: - vty->node = CONFIG_NODE; + if (bsc_vty_is_config_node(vty, vty->node)) + vty->node = CONFIG_NODE; + else + vty->node = ENABLE_NODE; + + vty->index = NULL; }
return vty->node; @@ -107,78 +122,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) gDEFUN(ournode_exit, ournode_exit_cmd, "exit", "Exit current mode and down to previous mode\n") { - switch (vty->node) { - case GSMNET_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; - case BTS_NODE: - vty->node = GSMNET_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts *bts = vty->index; - vty->index = bts->network; - vty->index_sub = NULL; - } - break; - case TRX_NODE: - vty->node = BTS_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts_trx *trx = vty->index; - vty->index = trx->bts; - vty->index_sub = &trx->bts->description; - } - break; - case TS_NODE: - vty->node = TRX_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts_trx_ts *ts = vty->index; - vty->index = ts->trx; - vty->index_sub = &ts->trx->description; - } - break; - case NAT_BSC_NODE: - vty->node = NAT_NODE; - { - struct bsc_config *bsc_config = vty->index; - vty->index = bsc_config->nat; - } - break; - case PGROUP_NODE: - vty->node = NAT_NODE; - break; - case SMPP_ESME_NODE: - vty->node = SMPP_NODE; - vty->index = NULL; - break; - case SMPP_NODE: - case MGCP_NODE: - case GBPROXY_NODE: - case SGSN_NODE: - case NAT_NODE: - case BSC_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; - case OML_NODE: - case OM2K_NODE: - vty->node = ENABLE_NODE; - talloc_free(vty->index); - vty->index = NULL; - break; - case MSC_NODE: - case MNCC_INT_NODE: - vty->node = CONFIG_NODE; - break; - case TRUNK_NODE: - vty->node = MGCP_NODE; - vty->index = NULL; - break; - default: - break; - } + bsc_vty_go_parent (vty); return CMD_SUCCESS; }
@@ -186,36 +130,24 @@ gDEFUN(ournode_exit, gDEFUN(ournode_end, ournode_end_cmd, "end", "End current mode and change to enable mode.") { - switch (vty->node) { - case VIEW_NODE: - case ENABLE_NODE: - /* Nothing to do. */ - break; - case CONFIG_NODE: - case GSMNET_NODE: - case BTS_NODE: - case TRX_NODE: - case TS_NODE: - case MGCP_NODE: - case TRUNK_NODE: - case GBPROXY_NODE: - case SGSN_NODE: - case VTY_NODE: - case NAT_NODE: - case NAT_BSC_NODE: - case PGROUP_NODE: - case MSC_NODE: - case MNCC_INT_NODE: - case SMPP_NODE: - case SMPP_ESME_NODE: - case BSC_NODE: + enum node_type last_node = CONFIG_NODE; + + if (vty->node >= CONFIG_NODE) { + /* Repeatedly call go_parent until a top node is reached. */ + while (vty->node > CONFIG_NODE) { + if (vty->node == last_node) { + /* Ensure termination, this shouldn't happen. */ + break; + } + last_node = vty->node; + bsc_vty_go_parent(vty); + } + vty_config_unlock(vty); - vty->node = ENABLE_NODE; + if (vty->node > ENABLE_NODE) + vty->node = ENABLE_NODE; vty->index = NULL; vty->index_sub = NULL; - break; - default: - break; } return CMD_SUCCESS; } diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 7b8498b..4426860 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -81,25 +81,25 @@ class TestVTYGenericBTS(TestVTYBase):
def _testConfigNetworkTree(self): self.vty.enable() - self.vty.verify("configure terminal",['']) + self.assertTrue(self.vty.verify("configure terminal",[''])) self.assertEquals(self.vty.node(), 'config') # self.checkForEndAndExit() - self.vty.verify("network",['']) + self.assertTrue(self.vty.verify("network",[''])) self.assertEquals(self.vty.node(), 'config-net') self.checkForEndAndExit() - self.vty.verify("bts 0",['']) + self.assertTrue(self.vty.verify("bts 0",[''])) self.assertEquals(self.vty.node(), 'config-net-bts') self.checkForEndAndExit() - self.vty.verify("trx 0",['']) + self.assertTrue(self.vty.verify("trx 0",[''])) self.assertEquals(self.vty.node(), 'config-net-bts-trx') self.checkForEndAndExit() - self.vty.verify("exit",['']) + self.assertTrue(self.vty.verify("exit",[''])) self.assertEquals(self.vty.node(), 'config-net-bts') - self.vty.verify("exit",['']) + self.assertTrue(self.vty.verify("exit",[''])) self.assertEquals(self.vty.node(), 'config-net') - self.vty.verify("exit",['']) + self.assertTrue(self.vty.verify("exit",[''])) self.assertEquals(self.vty.node(), 'config') - self.vty.verify("exit",['']) + self.assertTrue(self.vty.verify("exit",[''])) self.assertTrue(self.vty.node() is None)
class TestVTYNITB(TestVTYGenericBTS): @@ -116,24 +116,23 @@ class TestVTYNITB(TestVTYGenericBTS):
def testVtyTree(self): self.vty.enable() - self.vty.verify("configure terminal", ['']) + self.assertTrue(self.vty.verify("configure terminal", [''])) self.assertEquals(self.vty.node(), 'config') self.ignoredCheckForEndAndExit() - self.vty.verify('mncc-int', ['']) + self.assertTrue(self.vty.verify('mncc-int', [''])) self.assertEquals(self.vty.node(), 'config-mncc-int') self.checkForEndAndExit() - if self.notIgnoredTest(): - self.vty.verify('exit', ['']) - self.assertEquals(self.vty.node(), 'config') - else: - self.vty.verify("configure terminal", ['']) - self.vty.verify('smpp', ['']) + self.assertTrue(self.vty.verify('exit', [''])) + self.assertEquals(self.vty.node(), 'config') + self.assertTrue(self.vty.verify('smpp', [''])) self.assertEquals(self.vty.node(), 'config-smpp') self.ignoredCheckForEndAndExit() - self.vty.verify("exit", ['']) + self.assertTrue(self.vty.verify("exit", [''])) if self.notIgnoredTest(): self.assertEquals(self.vty.node(), 'config') - self.vty.verify("exit", ['']) + else: + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
# Check searching for outer node's commands @@ -181,26 +180,26 @@ class TestVTYBSC(TestVTYGenericBTS):
def testVtyTree(self): self.vty.enable() - self.vty.verify("configure terminal", ['']) + self.assertTrue(self.vty.verify("configure terminal", [''])) self.assertEquals(self.vty.node(), 'config') # self.ignoredCheckForEndAndExit() - self.vty.verify("msc 0", ['']) + self.assertTrue(self.vty.verify("msc 0", [''])) self.assertEquals(self.vty.node(), 'config-msc') # self.ignoredCheckForEndAndExit() - self.vty.verify("exit", ['']) + self.assertTrue(self.vty.verify("exit", [''])) if self.notIgnoredTest(): self.assertEquals(self.vty.node(), 'config') else: - self.vty.verify("configure terminal", ['']) + self.assertTrue(self.vty.verify("configure terminal", [''])) self.vty.command("bsc") self.assertEquals(self.vty.node(), 'config-bsc') self.ignoredCheckForEndAndExit() - self.vty.verify("exit", ['']) + self.assertTrue(self.vty.verify("exit", [''])) if self.notIgnoredTest(): self.assertEquals(self.vty.node(), 'config') else: - self.vty.verify("configure terminal", ['']) - self.vty.verify("exit", ['']) + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
# Check searching for outer node's commands @@ -253,25 +252,25 @@ class TestVTYNAT(TestVTYGenericBTS):
def testVtyTree(self): self.vty.enable() - self.vty.verify('configure terminal', ['']) + self.assertTrue(self.vty.verify('configure terminal', [''])) self.assertEquals(self.vty.node(), 'config') # self.checkForEndAndExit() - self.vty.verify('mgcp', ['']) + self.assertTrue(self.vty.verify('mgcp', [''])) self.assertEquals(self.vty.node(), 'config-mgcp') self.checkForEndAndExit() - self.vty.verify('exit', ['']) + self.assertTrue(self.vty.verify('exit', [''])) self.assertEquals(self.vty.node(), 'config') - self.vty.verify('nat', ['']) + self.assertTrue(self.vty.verify('nat', [''])) self.assertEquals(self.vty.node(), 'config-nat') self.checkForEndAndExit() - self.vty.verify('bsc 0', ['']) + self.assertTrue(self.vty.verify('bsc 0', [''])) self.assertEquals(self.vty.node(), 'config-nat-bsc') self.checkForEndAndExit() - self.vty.verify('exit', ['']) + self.assertTrue(self.vty.verify('exit', [''])) self.assertEquals(self.vty.node(), 'config-nat') - self.vty.verify('exit', ['']) + self.assertTrue(self.vty.verify('exit', [''])) self.assertEquals(self.vty.node(), 'config') - self.vty.verify('exit', ['']) + self.assertTrue(self.vty.verify('exit', [''])) self.assertTrue(self.vty.node() is None)
# Check searching for outer node's commands
On Fri, Aug 30, 2013 at 06:34:00PM +0200, Jacob Erlbeck wrote:
Note that in config mode if the tree is searched along the nodes toward the config node and a command is not found this way, a rollback is done by just replacing the vty's node and index member variable by the saved old values which might break the whole thing, when there has been a free() on the way.
okay, but in this case the config parsing will fail too. So right now we should be safe. Thanks for looking into this! o
@@ -186,36 +130,24 @@ gDEFUN(ournode_exit, gDEFUN(ournode_end, ournode_end_cmd, "end", "End current mode and change to enable mode.") {
- if (vty->node >= CONFIG_NODE) {
this would be true for the OM2K node, which is not a configure node. Can you elaborate why not using the 'is config node' predicate is not necessary?
def _testConfigNetworkTree(self): self.vty.enable()
self.vty.verify("configure terminal",[''])
self.assertTrue(self.vty.verify("configure terminal",['']))
where is the change from self.vty.verify to self assertTrue coming from? is this related to your change?
On 08/31/2013 03:47 PM, Holger Hans Peter Freyther wrote:
On Fri, Aug 30, 2013 at 06:34:00PM +0200, Jacob Erlbeck wrote:
Note that in config mode if the tree is searched along the nodes toward the config node and a command is not found this way, a rollback is done by just replacing the vty's node and index member variable by the saved old values which might break the whole thing, when there has been a free() on the way.
okay, but in this case the config parsing will fail too. So right now we should be safe. Thanks for looking into this! o
The problem would rather arise in interactive mode when somebody mistypes a command. It don't know, why outer context searching is enabled in config mode only and if somebody someday would like to provide this in non-config sub-nodes, too. Even if not, the scheme used for UM2K (alloc() in the node command and free() in vty_go_parent()) must not be used for config nodes. Having the same function (vty_go_parent() ) for just looking up the outer nodes and unwinding the node contexts effectively forbids side-effects. Perhaps one could add an additional boolean parameter to select between both modes of operation and separate the search phase from the unwinding phase.
@@ -186,36 +130,24 @@ gDEFUN(ournode_exit, gDEFUN(ournode_end, ournode_end_cmd, "end", "End current mode and change to enable mode.") {
- if (vty->node >= CONFIG_NODE) {
this would be true for the OM2K node, which is not a configure node. Can you elaborate why not using the 'is config node' predicate is not necessary?
The 'end' command only makes sense for node >= ENABLE_NODE. If must not be applied before the vty is in enabled mode, so it's explicitly a no-op in these cases.
What I'm not sure about is whether the 'end' command should be available in the non-config nodes, too (or what's the pure cisco way of it). It doesn't 'end' the configuration then but it returns to the top node in a defined way, which might be helpful in automated scripting.
def _testConfigNetworkTree(self): self.vty.enable()
self.vty.verify("configure terminal",[''])
self.assertTrue(self.vty.verify("configure terminal",['']))where is the change from self.vty.verify to self assertTrue coming from? is this related to your change?
Not at all, it accidently went into that patch. I'd rather put it into the PATCH 2/6 'test' patch.
This patch only adds bsc_install_default() and does the renaming so that in can be reverted easily if the generic handling of exit and end is moved to libosmocore.
Since it breaks the vty test suites due to duplicated commands, those suites have been disabled completely. --- openbsc/include/openbsc/vty.h | 2 ++ openbsc/src/gprs/gb_proxy_vty.c | 2 +- openbsc/src/gprs/sgsn_vty.c | 2 +- openbsc/src/libbsc/abis_nm_vty.c | 2 +- openbsc/src/libbsc/abis_om2000_vty.c | 2 +- openbsc/src/libbsc/bsc_vty.c | 8 ++++---- openbsc/src/libcommon/common_vty.c | 10 ++++++++++ openbsc/src/libmgcp/mgcp_vty.c | 4 ++-- openbsc/src/libmsc/smpp_vty.c | 4 ++-- openbsc/src/libmsc/vty_interface_layer3.c | 2 +- openbsc/src/osmo-bsc/osmo_bsc_vty.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_nat_vty.c | 6 +++--- openbsc/tests/vty_test_runner.py | 22 ++++++++-------------- 13 files changed, 38 insertions(+), 32 deletions(-)
diff --git a/openbsc/include/openbsc/vty.h b/openbsc/include/openbsc/vty.h index 183fc25..f74516a 100644 --- a/openbsc/include/openbsc/vty.h +++ b/openbsc/include/openbsc/vty.h @@ -42,6 +42,8 @@ enum bsc_vty_node { extern int bsc_vty_is_config_node(struct vty *vty, int node); extern void bsc_replace_string(void *ctx, char **dst, const char *newstr);
+void bsc_install_default(enum node_type node); + struct log_info; int bsc_vty_init(const struct log_info *cat); int bsc_vty_init_extra(void); diff --git a/openbsc/src/gprs/gb_proxy_vty.c b/openbsc/src/gprs/gb_proxy_vty.c index bfa1f3b..ccbeead 100644 --- a/openbsc/src/gprs/gb_proxy_vty.c +++ b/openbsc/src/gprs/gb_proxy_vty.c @@ -82,7 +82,7 @@ int gbproxy_vty_init(void)
install_element(CONFIG_NODE, &cfg_gbproxy_cmd); install_node(&gbproxy_node, config_write_gbproxy); - install_default(GBPROXY_NODE); + bsc_install_default(GBPROXY_NODE); install_element(GBPROXY_NODE, &ournode_exit_cmd); install_element(GBPROXY_NODE, &ournode_end_cmd); install_element(GBPROXY_NODE, &cfg_nsip_sgsn_nsei_cmd); diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c index a4ba280..1acaae7 100644 --- a/openbsc/src/gprs/sgsn_vty.c +++ b/openbsc/src/gprs/sgsn_vty.c @@ -418,7 +418,7 @@ int sgsn_vty_init(void)
install_element(CONFIG_NODE, &cfg_sgsn_cmd); install_node(&sgsn_node, config_write_sgsn); - install_default(SGSN_NODE); + bsc_install_default(SGSN_NODE); install_element(SGSN_NODE, &ournode_exit_cmd); install_element(SGSN_NODE, &ournode_end_cmd); install_element(SGSN_NODE, &cfg_sgsn_bind_addr_cmd); diff --git a/openbsc/src/libbsc/abis_nm_vty.c b/openbsc/src/libbsc/abis_nm_vty.c index fd60210..6ac931a 100644 --- a/openbsc/src/libbsc/abis_nm_vty.c +++ b/openbsc/src/libbsc/abis_nm_vty.c @@ -183,7 +183,7 @@ int abis_nm_vty_init(void) install_element(ENABLE_NODE, &oml_classnum_inst_cmd); install_node(&oml_node, dummy_config_write);
- install_default(OML_NODE); + bsc_install_default(OML_NODE); install_element(OML_NODE, &ournode_exit_cmd); install_element(OML_NODE, &oml_chg_adm_state_cmd); install_element(OML_NODE, &oml_opstart_cmd); diff --git a/openbsc/src/libbsc/abis_om2000_vty.c b/openbsc/src/libbsc/abis_om2000_vty.c index 3df005b..57bed8a 100644 --- a/openbsc/src/libbsc/abis_om2000_vty.c +++ b/openbsc/src/libbsc/abis_om2000_vty.c @@ -445,7 +445,7 @@ int abis_om2k_vty_init(void) install_element(ENABLE_NODE, &om2k_classnum_inst_cmd); install_node(&om2k_node, dummy_config_write);
- install_default(OM2K_NODE); + bsc_install_default(OM2K_NODE); install_element(OM2K_NODE, &ournode_exit_cmd); install_element(OM2K_NODE, &om2k_reset_cmd); install_element(OM2K_NODE, &om2k_start_cmd); diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index 45df90f..ca6ff4c 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -3002,7 +3002,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(CONFIG_NODE, &cfg_net_cmd); install_node(&net_node, config_write_net); - install_default(GSMNET_NODE); + bsc_install_default(GSMNET_NODE); install_element(GSMNET_NODE, &ournode_exit_cmd); install_element(GSMNET_NODE, &ournode_end_cmd); install_element(GSMNET_NODE, &cfg_net_ncc_cmd); @@ -3040,7 +3040,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(GSMNET_NODE, &cfg_bts_cmd); install_node(&bts_node, config_write_bts); - install_default(BTS_NODE); + bsc_install_default(BTS_NODE); install_element(BTS_NODE, &ournode_exit_cmd); install_element(BTS_NODE, &ournode_end_cmd); install_element(BTS_NODE, &cfg_bts_type_cmd); @@ -3102,7 +3102,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(BTS_NODE, &cfg_trx_cmd); install_node(&trx_node, dummy_config_write); - install_default(TRX_NODE); + bsc_install_default(TRX_NODE); install_element(TRX_NODE, &ournode_exit_cmd); install_element(TRX_NODE, &ournode_end_cmd); install_element(TRX_NODE, &cfg_trx_arfcn_cmd); @@ -3116,7 +3116,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(TRX_NODE, &cfg_ts_cmd); install_node(&ts_node, dummy_config_write); - install_default(TS_NODE); + bsc_install_default(TS_NODE); install_element(TS_NODE, &ournode_exit_cmd); install_element(TS_NODE, &ournode_end_cmd); install_element(TS_NODE, &cfg_ts_pchan_cmd); diff --git a/openbsc/src/libcommon/common_vty.c b/openbsc/src/libcommon/common_vty.c index 948bd49..b5b77c4 100644 --- a/openbsc/src/libcommon/common_vty.c +++ b/openbsc/src/libcommon/common_vty.c @@ -174,3 +174,13 @@ void bsc_replace_string(void *ctx, char **dst, const char *newstr) talloc_free(*dst); *dst = talloc_strdup(ctx, newstr); } + +void bsc_install_default(enum node_type node) +{ + install_default (node); + + if (node > CONFIG_NODE) { + install_element(node, &ournode_exit_cmd); + install_element(node, &ournode_end_cmd); + } +} diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c index 833908a..ccb9c26 100644 --- a/openbsc/src/libmgcp/mgcp_vty.c +++ b/openbsc/src/libmgcp/mgcp_vty.c @@ -786,7 +786,7 @@ int mgcp_vty_init(void) install_element(CONFIG_NODE, &cfg_mgcp_cmd); install_node(&mgcp_node, config_write_mgcp);
- install_default(MGCP_NODE); + bsc_install_default(MGCP_NODE); install_element(MGCP_NODE, &ournode_exit_cmd); install_element(MGCP_NODE, &ournode_end_cmd); install_element(MGCP_NODE, &cfg_mgcp_local_ip_cmd); @@ -820,7 +820,7 @@ int mgcp_vty_init(void)
install_element(MGCP_NODE, &cfg_mgcp_trunk_cmd); install_node(&trunk_node, config_write_trunk); - install_default(TRUNK_NODE); + bsc_install_default(TRUNK_NODE); install_element(TRUNK_NODE, &ournode_exit_cmd); install_element(TRUNK_NODE, &ournode_end_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_number_cmd); diff --git a/openbsc/src/libmsc/smpp_vty.c b/openbsc/src/libmsc/smpp_vty.c index 4829eb9..abf9733 100644 --- a/openbsc/src/libmsc/smpp_vty.c +++ b/openbsc/src/libmsc/smpp_vty.c @@ -487,7 +487,7 @@ static int config_write_esme(struct vty *v) int smpp_vty_init(void) { install_node(&smpp_node, config_write_smpp); - install_default(SMPP_NODE); + bsc_install_default(SMPP_NODE); install_element(CONFIG_NODE, &cfg_smpp_cmd);
install_element(SMPP_NODE, &cfg_smpp_port_cmd); @@ -497,7 +497,7 @@ int smpp_vty_init(void) install_element(SMPP_NODE, &cfg_no_esme_cmd);
install_node(&esme_node, config_write_esme); - install_default(SMPP_ESME_NODE); + bsc_install_default(SMPP_ESME_NODE); install_element(SMPP_ESME_NODE, &cfg_esme_passwd_cmd); install_element(SMPP_ESME_NODE, &cfg_esme_no_passwd_cmd); install_element(SMPP_ESME_NODE, &cfg_esme_route_pfx_cmd); diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index 79c3457..ce06837 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -975,7 +975,7 @@ int bsc_vty_init_extra(void)
install_element(CONFIG_NODE, &cfg_mncc_int_cmd); install_node(&mncc_int_node, config_write_mncc_int); - install_default(MNCC_INT_NODE); + bsc_install_default(MNCC_INT_NODE); install_element(MNCC_INT_NODE, &ournode_exit_cmd); install_element(MNCC_INT_NODE, &ournode_end_cmd); install_element(MNCC_INT_NODE, &mnccint_def_codec_f_cmd); diff --git a/openbsc/src/osmo-bsc/osmo_bsc_vty.c b/openbsc/src/osmo-bsc/osmo_bsc_vty.c index f6cf1a0..90b0a0c 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_vty.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_vty.c @@ -609,7 +609,7 @@ int bsc_vty_init_extra(void) install_element(CONFIG_NODE, &cfg_net_bsc_cmd);
install_node(&bsc_node, config_write_bsc); - install_default(BSC_NODE); + bsc_install_default(BSC_NODE); install_element(BSC_NODE, &cfg_net_bsc_mid_call_text_cmd); install_element(BSC_NODE, &cfg_net_bsc_mid_call_timeout_cmd); install_element(BSC_NODE, &cfg_net_rf_socket_cmd); @@ -617,7 +617,7 @@ int bsc_vty_init_extra(void) install_element(BSC_NODE, &cfg_net_no_rf_off_time_cmd);
install_node(&msc_node, config_write_msc); - install_default(MSC_NODE); + bsc_install_default(MSC_NODE); install_element(MSC_NODE, &cfg_net_bsc_token_cmd); install_element(MSC_NODE, &cfg_net_bsc_ncc_cmd); install_element(MSC_NODE, &cfg_net_bsc_mcc_cmd); diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c index 36a46f2..5699223 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c @@ -1192,7 +1192,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) /* nat group */ install_element(CONFIG_NODE, &cfg_nat_cmd); install_node(&nat_node, config_write_nat); - install_default(NAT_NODE); + bsc_install_default(NAT_NODE); install_element(NAT_NODE, &ournode_exit_cmd); install_element(NAT_NODE, &ournode_end_cmd); install_element(NAT_NODE, &cfg_nat_msc_ip_cmd); @@ -1235,7 +1235,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_nat_pgroup_cmd); install_element(NAT_NODE, &cfg_nat_no_pgroup_cmd); install_node(&pgroup_node, config_write_pgroup); - install_default(PGROUP_NODE); + bsc_install_default(PGROUP_NODE); install_element(PGROUP_NODE, &ournode_exit_cmd); install_element(PGROUP_NODE, &ournode_end_cmd); install_element(PGROUP_NODE, &cfg_pgroup_lac_cmd); @@ -1244,7 +1244,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) /* BSC subgroups */ install_element(NAT_NODE, &cfg_bsc_cmd); install_node(&bsc_node, config_write_bsc); - install_default(NAT_BSC_NODE); + bsc_install_default(NAT_BSC_NODE); install_element(NAT_BSC_NODE, &ournode_exit_cmd); install_element(NAT_BSC_NODE, &ournode_end_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_token_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 4426860..725eb10 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -182,23 +182,17 @@ class TestVTYBSC(TestVTYGenericBTS): self.vty.enable() self.assertTrue(self.vty.verify("configure terminal", [''])) self.assertEquals(self.vty.node(), 'config') - # self.ignoredCheckForEndAndExit() + self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify("msc 0", [''])) self.assertEquals(self.vty.node(), 'config-msc') - # self.ignoredCheckForEndAndExit() + self.checkForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.vty.command("bsc") self.assertEquals(self.vty.node(), 'config-bsc') - self.ignoredCheckForEndAndExit() + self.checkForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
@@ -414,8 +408,8 @@ if __name__ == '__main__': os.chdir(workdir) print "Running tests for specific VTY commands" suite = unittest.TestSuite() - suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestVTYNITB)) - add_bsc_test(suite, workdir) - add_nat_test(suite, workdir) + #suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestVTYNITB)) + #add_bsc_test(suite, workdir) + #add_nat_test(suite, workdir) res = unittest.TextTestRunner(verbosity=verbose_level).run(suite) sys.exit(len(res.errors) + len(res.failures))
Since 'exit' and 'end' are added automatically to each node, the explicit registrations of these commands are removed by this patch.
The related test succeed now without work-arounds (except for the 'config' node itself which is part of libosmocore). --- openbsc/src/gprs/gb_proxy_vty.c | 2 -- openbsc/src/gprs/sgsn_vty.c | 2 -- openbsc/src/libbsc/abis_nm_vty.c | 1 - openbsc/src/libbsc/abis_om2000_vty.c | 1 - openbsc/src/libbsc/bsc_vty.c | 8 -------- openbsc/src/libmgcp/mgcp_vty.c | 4 ---- openbsc/src/libmsc/vty_interface_layer3.c | 2 -- openbsc/src/osmo-bsc_nat/bsc_nat_vty.c | 6 ------ openbsc/tests/vty_test_runner.py | 15 ++++++--------- 9 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/openbsc/src/gprs/gb_proxy_vty.c b/openbsc/src/gprs/gb_proxy_vty.c index ccbeead..63546d3 100644 --- a/openbsc/src/gprs/gb_proxy_vty.c +++ b/openbsc/src/gprs/gb_proxy_vty.c @@ -83,8 +83,6 @@ int gbproxy_vty_init(void) install_element(CONFIG_NODE, &cfg_gbproxy_cmd); install_node(&gbproxy_node, config_write_gbproxy); bsc_install_default(GBPROXY_NODE); - install_element(GBPROXY_NODE, &ournode_exit_cmd); - install_element(GBPROXY_NODE, &ournode_end_cmd); install_element(GBPROXY_NODE, &cfg_nsip_sgsn_nsei_cmd);
return 0; diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c index 1acaae7..8106b7d 100644 --- a/openbsc/src/gprs/sgsn_vty.c +++ b/openbsc/src/gprs/sgsn_vty.c @@ -419,8 +419,6 @@ int sgsn_vty_init(void) install_element(CONFIG_NODE, &cfg_sgsn_cmd); install_node(&sgsn_node, config_write_sgsn); bsc_install_default(SGSN_NODE); - install_element(SGSN_NODE, &ournode_exit_cmd); - install_element(SGSN_NODE, &ournode_end_cmd); install_element(SGSN_NODE, &cfg_sgsn_bind_addr_cmd); install_element(SGSN_NODE, &cfg_ggsn_remote_ip_cmd); //install_element(SGSN_NODE, &cfg_ggsn_remote_port_cmd); diff --git a/openbsc/src/libbsc/abis_nm_vty.c b/openbsc/src/libbsc/abis_nm_vty.c index 6ac931a..5db667d 100644 --- a/openbsc/src/libbsc/abis_nm_vty.c +++ b/openbsc/src/libbsc/abis_nm_vty.c @@ -184,7 +184,6 @@ int abis_nm_vty_init(void) install_node(&oml_node, dummy_config_write);
bsc_install_default(OML_NODE); - install_element(OML_NODE, &ournode_exit_cmd); install_element(OML_NODE, &oml_chg_adm_state_cmd); install_element(OML_NODE, &oml_opstart_cmd);
diff --git a/openbsc/src/libbsc/abis_om2000_vty.c b/openbsc/src/libbsc/abis_om2000_vty.c index 57bed8a..eb8f4d5 100644 --- a/openbsc/src/libbsc/abis_om2000_vty.c +++ b/openbsc/src/libbsc/abis_om2000_vty.c @@ -446,7 +446,6 @@ int abis_om2k_vty_init(void) install_node(&om2k_node, dummy_config_write);
bsc_install_default(OM2K_NODE); - install_element(OM2K_NODE, &ournode_exit_cmd); install_element(OM2K_NODE, &om2k_reset_cmd); install_element(OM2K_NODE, &om2k_start_cmd); install_element(OM2K_NODE, &om2k_status_cmd); diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index ca6ff4c..5748945 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -3003,8 +3003,6 @@ int bsc_vty_init(const struct log_info *cat) install_element(CONFIG_NODE, &cfg_net_cmd); install_node(&net_node, config_write_net); bsc_install_default(GSMNET_NODE); - install_element(GSMNET_NODE, &ournode_exit_cmd); - install_element(GSMNET_NODE, &ournode_end_cmd); install_element(GSMNET_NODE, &cfg_net_ncc_cmd); install_element(GSMNET_NODE, &cfg_net_mnc_cmd); install_element(GSMNET_NODE, &cfg_net_name_short_cmd); @@ -3041,8 +3039,6 @@ int bsc_vty_init(const struct log_info *cat) install_element(GSMNET_NODE, &cfg_bts_cmd); install_node(&bts_node, config_write_bts); bsc_install_default(BTS_NODE); - install_element(BTS_NODE, &ournode_exit_cmd); - install_element(BTS_NODE, &ournode_end_cmd); install_element(BTS_NODE, &cfg_bts_type_cmd); install_element(BTS_NODE, &cfg_description_cmd); install_element(BTS_NODE, &cfg_no_description_cmd); @@ -3103,8 +3099,6 @@ int bsc_vty_init(const struct log_info *cat) install_element(BTS_NODE, &cfg_trx_cmd); install_node(&trx_node, dummy_config_write); bsc_install_default(TRX_NODE); - install_element(TRX_NODE, &ournode_exit_cmd); - install_element(TRX_NODE, &ournode_end_cmd); install_element(TRX_NODE, &cfg_trx_arfcn_cmd); install_element(TRX_NODE, &cfg_description_cmd); install_element(TRX_NODE, &cfg_no_description_cmd); @@ -3117,8 +3111,6 @@ int bsc_vty_init(const struct log_info *cat) install_element(TRX_NODE, &cfg_ts_cmd); install_node(&ts_node, dummy_config_write); bsc_install_default(TS_NODE); - install_element(TS_NODE, &ournode_exit_cmd); - install_element(TS_NODE, &ournode_end_cmd); install_element(TS_NODE, &cfg_ts_pchan_cmd); install_element(TS_NODE, &cfg_ts_pchan_compat_cmd); install_element(TS_NODE, &cfg_ts_tsc_cmd); diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c index ccb9c26..9421f4f 100644 --- a/openbsc/src/libmgcp/mgcp_vty.c +++ b/openbsc/src/libmgcp/mgcp_vty.c @@ -787,8 +787,6 @@ int mgcp_vty_init(void) install_node(&mgcp_node, config_write_mgcp);
bsc_install_default(MGCP_NODE); - install_element(MGCP_NODE, &ournode_exit_cmd); - install_element(MGCP_NODE, &ournode_end_cmd); install_element(MGCP_NODE, &cfg_mgcp_local_ip_cmd); install_element(MGCP_NODE, &cfg_mgcp_bts_ip_cmd); install_element(MGCP_NODE, &cfg_mgcp_bind_ip_cmd); @@ -821,8 +819,6 @@ int mgcp_vty_init(void) install_element(MGCP_NODE, &cfg_mgcp_trunk_cmd); install_node(&trunk_node, config_write_trunk); bsc_install_default(TRUNK_NODE); - install_element(TRUNK_NODE, &ournode_exit_cmd); - install_element(TRUNK_NODE, &ournode_end_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_number_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_name_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_number_cmd_old); diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index ce06837..19c78ea 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -976,8 +976,6 @@ int bsc_vty_init_extra(void) install_element(CONFIG_NODE, &cfg_mncc_int_cmd); install_node(&mncc_int_node, config_write_mncc_int); bsc_install_default(MNCC_INT_NODE); - install_element(MNCC_INT_NODE, &ournode_exit_cmd); - install_element(MNCC_INT_NODE, &ournode_end_cmd); install_element(MNCC_INT_NODE, &mnccint_def_codec_f_cmd); install_element(MNCC_INT_NODE, &mnccint_def_codec_h_cmd);
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c index 5699223..511d62a 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c @@ -1193,8 +1193,6 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(CONFIG_NODE, &cfg_nat_cmd); install_node(&nat_node, config_write_nat); bsc_install_default(NAT_NODE); - install_element(NAT_NODE, &ournode_exit_cmd); - install_element(NAT_NODE, &ournode_end_cmd); install_element(NAT_NODE, &cfg_nat_msc_ip_cmd); install_element(NAT_NODE, &cfg_nat_msc_port_cmd); install_element(NAT_NODE, &cfg_nat_auth_time_cmd); @@ -1236,8 +1234,6 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_nat_no_pgroup_cmd); install_node(&pgroup_node, config_write_pgroup); bsc_install_default(PGROUP_NODE); - install_element(PGROUP_NODE, &ournode_exit_cmd); - install_element(PGROUP_NODE, &ournode_end_cmd); install_element(PGROUP_NODE, &cfg_pgroup_lac_cmd); install_element(PGROUP_NODE, &cfg_pgroup_no_lac_cmd);
@@ -1245,8 +1241,6 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_bsc_cmd); install_node(&bsc_node, config_write_bsc); bsc_install_default(NAT_BSC_NODE); - install_element(NAT_BSC_NODE, &ournode_exit_cmd); - install_element(NAT_BSC_NODE, &ournode_end_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_token_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_lac_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_no_lac_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 725eb10..7783189 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -83,7 +83,7 @@ class TestVTYGenericBTS(TestVTYBase): self.vty.enable() self.assertTrue(self.vty.verify("configure terminal",[''])) self.assertEquals(self.vty.node(), 'config') - # self.checkForEndAndExit() + self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify("network",[''])) self.assertEquals(self.vty.node(), 'config-net') self.checkForEndAndExit() @@ -128,10 +128,7 @@ class TestVTYNITB(TestVTYGenericBTS): self.assertEquals(self.vty.node(), 'config-smpp') self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
@@ -248,7 +245,7 @@ class TestVTYNAT(TestVTYGenericBTS): self.vty.enable() self.assertTrue(self.vty.verify('configure terminal', [''])) self.assertEquals(self.vty.node(), 'config') - # self.checkForEndAndExit() + self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify('mgcp', [''])) self.assertEquals(self.vty.node(), 'config-mgcp') self.checkForEndAndExit() @@ -408,8 +405,8 @@ if __name__ == '__main__': os.chdir(workdir) print "Running tests for specific VTY commands" suite = unittest.TestSuite() - #suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestVTYNITB)) - #add_bsc_test(suite, workdir) - #add_nat_test(suite, workdir) + suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestVTYNITB)) + add_bsc_test(suite, workdir) + add_nat_test(suite, workdir) res = unittest.TextTestRunner(verbosity=verbose_level).run(suite) sys.exit(len(res.errors) + len(res.failures))
On Fri, Aug 30, 2013 at 06:34:02PM +0200, Jacob Erlbeck wrote:
Dear Jacob,
Since 'exit' and 'end' are added automatically to each node, the explicit registrations of these commands are removed by this patch.
I know I asked you to make two separate patches but after seing what I asked for, I think I would like to squash the previous and this commit together. Any objection?
thanks for the patch set!
Currently the 'mgcp' command fails in the 'config-nat' node, because it get confused with 'mgcp-through-msc-ipa' which is executed instead because of the prefix based command selection. Thus the latter command is renamed by this patch to avoid the common prefix.
The workaround in the test suite is removed. --- openbsc/src/osmo-bsc_nat/bsc_nat_vty.c | 10 +++++----- openbsc/tests/vty_test_runner.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c index 511d62a..125fbf1 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c @@ -152,7 +152,7 @@ static int config_write_nat(struct vty *vty) llist_for_each_entry(pgroup, &_nat->paging_groups, entry) write_pgroup_lst(vty, pgroup); if (_nat->mgcp_ipa) - vty_out(vty, " mgcp-through-msc-ipa%s", VTY_NEWLINE); + vty_out(vty, " use-msc-ipa-for-mgcp%s", VTY_NEWLINE);
return CMD_SUCCESS; } @@ -754,9 +754,9 @@ DEFUN(cfg_nat_ussd_local, return CMD_SUCCESS; }
-DEFUN(cfg_nat_mgcp_ipa, - cfg_nat_mgcp_ipa_cmd, - "mgcp-through-msc-ipa", +DEFUN(cfg_nat_use_ipa_for_mgcp, + cfg_nat_use_ipa_for_mgcp_cmd, + "use-msc-ipa-for-mgcp", "This needs to be set at start. Handle MGCP messages through " "the IPA protocol and not through the UDP socket.\n") { @@ -1209,7 +1209,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_nat_ussd_query_cmd); install_element(NAT_NODE, &cfg_nat_ussd_token_cmd); install_element(NAT_NODE, &cfg_nat_ussd_local_cmd); - install_element(NAT_NODE, &cfg_nat_mgcp_ipa_cmd); + install_element(NAT_NODE, &cfg_nat_use_ipa_for_mgcp_cmd);
/* access-list */ install_element(NAT_NODE, &cfg_lst_imsi_allow_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 7783189..06202e1 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -269,13 +269,13 @@ class TestVTYNAT(TestVTYGenericBTS): self.vty.command('mgcp') self.vty.command('nat') self.assertEquals(self.vty.node(), 'config-nat') - self.vty.command('line vty') - self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('mgcp') + self.assertEquals(self.vty.node(), 'config-mgcp') self.vty.command('nat') self.assertEquals(self.vty.node(), 'config-nat') self.vty.command('bsc 0') - self.vty.command('line vty') - self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('mgcp') + self.assertEquals(self.vty.node(), 'config-mgcp')
def testRewriteNoRewrite(self): self.vty.enable()
On Fri, Aug 30, 2013 at 06:34:03PM +0200, Jacob Erlbeck wrote:
Currently the 'mgcp' command fails in the 'config-nat' node, because it get confused with 'mgcp-through-msc-ipa' which is executed instead because of the prefix based command selection. Thus the latter command is renamed by this patch to avoid the common prefix.
Looks good!
These tests check for the availability of 'exit' and 'end' in each configuration node and for the node specific commands to traverse the tree. In addition, using these commands from within inner contexts is checked. This will detect problems, when an outer command word is a prefix of an inner command, like with 'mgcp' and 'mgcp-through-msc-ipa'.
Several assertions are disabled due to inconsistencies and missing commands (see above). --- openbsc/tests/vty_test_runner.py | 155 +++++++++++++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 3 deletions(-)
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 87a45bb..e91d018 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -56,7 +56,53 @@ class TestVTYBase(unittest.TestCase): self.vty = None osmoutil.end_proc(self.proc)
-class TestVTYNITB(TestVTYBase): + +class TestVTYGenericBSC(TestVTYBase): + + def checkForEndAndExit(self): + res = self.vty.command("list") + #print ('looking for "exit"\n') + self.assert_(res.find(' exit\r') > 0) + #print 'found "exit"\nlooking for "end"\n' + self.assert_(res.find(' end\r') > 0) + #print 'found "end"\n' + + def ignoredCheckForEndAndExit(self): + sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n') + try: + self.checkForEndAndExit() + except BaseException as e: + sys.stderr.write('Expected and ignored failure: %s\n' % (str(e))) + + def notIgnoredTest(self): + sys.stderr.write('Going to ignore the next assertion(s) due to known bugs\n') + return False + + + def _testConfigNetworkTree(self): + self.vty.enable() + self.assertTrue(self.vty.verify("configure terminal",[''])) + self.assertEquals(self.vty.node(), 'config') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify("network",[''])) + self.assertEquals(self.vty.node(), 'config-net') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify("bts 0",[''])) + self.assertEquals(self.vty.node(), 'config-net-bts') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify("trx 0",[''])) + self.assertEquals(self.vty.node(), 'config-net-bts-trx') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify("exit",[''])) + self.assertEquals(self.vty.node(), 'config-net-bts') + self.assertTrue(self.vty.verify("exit",[''])) + self.assertEquals(self.vty.node(), 'config-net') + self.assertTrue(self.vty.verify("exit",[''])) + self.assertEquals(self.vty.node(), 'config') + self.assertTrue(self.vty.verify("exit",[''])) + self.assertTrue(self.vty.node() is None) + +class TestVTYNITB(TestVTYGenericBSC):
def vty_command(self): return ["./src/osmo-nitb/osmo-nitb", "-c", @@ -65,6 +111,38 @@ class TestVTYNITB(TestVTYBase): def vty_app(self): return (4242, "./src/osmo-nitb/osmo-nitb", "OpenBSC", "nitb")
+ def testConfigNetworkTree(self): + self._testConfigNetworkTree() + + def testVtyTree(self): + self.vty.enable() + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify('mncc-int', [''])) + self.assertEquals(self.vty.node(), 'config-mncc-int') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify('exit', [''])) + self.assertEquals(self.vty.node(), 'config') + self.assertTrue(self.vty.verify('smpp', [''])) + self.assertEquals(self.vty.node(), 'config-smpp') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify("exit", [''])) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + else: + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertTrue(self.vty.verify("exit", [''])) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command("configure terminal") + self.vty.command('mncc-int') + self.vty.command('smpp') + self.assertEquals(self.vty.node(), 'config-smpp') + self.vty.command('mncc-int') + self.assertEquals(self.vty.node(), 'config-mncc-int') + def testEnableDisablePeriodicLU(self): self.vty.enable() self.vty.command("configure terminal") @@ -88,7 +166,7 @@ class TestVTYNITB(TestVTYBase): self.assertEquals(res.find('periodic location update 60'), -1) self.assert_(res.find('no periodic location update') > 0)
-class TestVTYBSC(TestVTYBase): +class TestVTYBSC(TestVTYGenericBSC):
def vty_command(self): return ["./src/osmo-bsc/osmo-bsc", "-c", @@ -97,6 +175,41 @@ class TestVTYBSC(TestVTYBase): def vty_app(self): return (4242, "./src/osmo-bsc/osmo-bsc", "OsmoBSC", "bsc")
+ def testConfigNetworkTree(self): + self._testConfigNetworkTree() + + def testVtyTree(self): + self.vty.enable() + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify("msc 0", [''])) + self.assertEquals(self.vty.node(), 'config-msc') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify("exit", [''])) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + else: + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertTrue(self.vty.verify("bsc", [''])) + self.assertEquals(self.vty.node(), 'config-bsc') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify("exit", [''])) + if self.notIgnoredTest(): + self.assertEquals(self.vty.node(), 'config') + else: + self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertTrue(self.vty.verify("exit", [''])) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command("configure terminal") + self.vty.command('msc 0') + self.vty.command("bsc") + self.assertEquals(self.vty.node(), 'config-bsc') + self.vty.command("msc 0") + self.assertEquals(self.vty.node(), 'config-msc') + def testUssdNotifications(self): self.vty.enable() self.vty.command("configure terminal") @@ -128,7 +241,7 @@ class TestVTYBSC(TestVTYBase): self.assert_(res.find('no bsc-welcome-text') > 0) self.assertEquals(res.find('bsc-welcome-text Hello MS'), -1)
-class TestVTYNAT(TestVTYBase): +class TestVTYNAT(TestVTYGenericBSC):
def vty_command(self): return ["./src/osmo-bsc_nat/osmo-bsc_nat", "-c", @@ -137,6 +250,42 @@ class TestVTYNAT(TestVTYBase): def vty_app(self): return (4244, "src/osmo-bsc_nat/osmo-bsc_nat", "OsmoBSCNAT", "nat")
+ def testVtyTree(self): + self.vty.enable() + self.assertTrue(self.vty.verify('configure terminal', [''])) + self.assertEquals(self.vty.node(), 'config') + self.ignoredCheckForEndAndExit() + self.assertTrue(self.vty.verify('mgcp', [''])) + self.assertEquals(self.vty.node(), 'config-mgcp') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify('exit', [''])) + self.assertEquals(self.vty.node(), 'config') + self.assertTrue(self.vty.verify('nat', [''])) + self.assertEquals(self.vty.node(), 'config-nat') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify('bsc 0', [''])) + self.assertEquals(self.vty.node(), 'config-nat-bsc') + self.checkForEndAndExit() + self.assertTrue(self.vty.verify('exit', [''])) + self.assertEquals(self.vty.node(), 'config-nat') + self.assertTrue(self.vty.verify('exit', [''])) + self.assertEquals(self.vty.node(), 'config') + self.assertTrue(self.vty.verify('exit', [''])) + self.assertTrue(self.vty.node() is None) + + # Check searching for outer node's commands + self.vty.command('configure terminal') + self.vty.command('mgcp') + self.vty.command('nat') + self.assertEquals(self.vty.node(), 'config-nat') + self.vty.command('line vty') + self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('nat') + self.assertEquals(self.vty.node(), 'config-nat') + self.vty.command('bsc 0') + self.vty.command('line vty') + self.assertEquals(self.vty.node(), 'config-line') + def testRewriteNoRewrite(self): self.vty.enable() res = self.vty.command("configure terminal")
ournode_exit() duplicates most of bsc_vty_go_parent(). This patch fixes the inconsistencies of both functions within bsc_vty_go_parent() and replaces the implementation of ournode_exit() by a call to it. This makes 'exit' behave exactly like ^D in all openbsc nodes.
ournode_end() has been changed to walk through the intermediate nodes until one of the top nodes is reached. This allows for cleanups to be done on the way.
Note that in config mode if the tree is searched along the nodes toward the config node and a command is not found this way, a rollback is done by just replacing the vty's node and index member variable by the saved old values which might break the whole thing, when there has been a free() on the way. --- openbsc/src/libcommon/common_vty.c | 140 ++++++++++-------------------------- 1 file changed, 36 insertions(+), 104 deletions(-)
diff --git a/openbsc/src/libcommon/common_vty.c b/openbsc/src/libcommon/common_vty.c index 7342f14..948bd49 100644 --- a/openbsc/src/libcommon/common_vty.c +++ b/openbsc/src/libcommon/common_vty.c @@ -48,6 +48,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts *bts = vty->index; vty->index = bts->network; + vty->index_sub = NULL; } break; case TRX_NODE: @@ -56,6 +57,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts_trx *trx = vty->index; vty->index = trx->bts; + vty->index_sub = &trx->bts->description; } break; case TS_NODE: @@ -64,18 +66,19 @@ enum node_type bsc_vty_go_parent(struct vty *vty) /* set vty->index correctly ! */ struct gsm_bts_trx_ts *ts = vty->index; vty->index = ts->trx; + vty->index_sub = &ts->trx->description; } break; case OML_NODE: case OM2K_NODE: vty->node = ENABLE_NODE; + /* NOTE: this only works because it's not part of the config + * tree, where outer commands are searched via vty_go_parent() + * and only (!) executed when a matching one is found. + */ talloc_free(vty->index); vty->index = NULL; break; - case NAT_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; case NAT_BSC_NODE: vty->node = NAT_NODE; { @@ -85,19 +88,31 @@ enum node_type bsc_vty_go_parent(struct vty *vty) break; case PGROUP_NODE: vty->node = NAT_NODE; + vty->index = NULL; break; case TRUNK_NODE: vty->node = MGCP_NODE; + vty->index = NULL; break; case SMPP_ESME_NODE: vty->node = SMPP_NODE; vty->index = NULL; break; case SMPP_NODE: + case MGCP_NODE: + case GBPROXY_NODE: + case SGSN_NODE: + case NAT_NODE: + case BSC_NODE: case MSC_NODE: case MNCC_INT_NODE: default: - vty->node = CONFIG_NODE; + if (bsc_vty_is_config_node(vty, vty->node)) + vty->node = CONFIG_NODE; + else + vty->node = ENABLE_NODE; + + vty->index = NULL; }
return vty->node; @@ -107,78 +122,7 @@ enum node_type bsc_vty_go_parent(struct vty *vty) gDEFUN(ournode_exit, ournode_exit_cmd, "exit", "Exit current mode and down to previous mode\n") { - switch (vty->node) { - case GSMNET_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; - case BTS_NODE: - vty->node = GSMNET_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts *bts = vty->index; - vty->index = bts->network; - vty->index_sub = NULL; - } - break; - case TRX_NODE: - vty->node = BTS_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts_trx *trx = vty->index; - vty->index = trx->bts; - vty->index_sub = &trx->bts->description; - } - break; - case TS_NODE: - vty->node = TRX_NODE; - { - /* set vty->index correctly ! */ - struct gsm_bts_trx_ts *ts = vty->index; - vty->index = ts->trx; - vty->index_sub = &ts->trx->description; - } - break; - case NAT_BSC_NODE: - vty->node = NAT_NODE; - { - struct bsc_config *bsc_config = vty->index; - vty->index = bsc_config->nat; - } - break; - case PGROUP_NODE: - vty->node = NAT_NODE; - break; - case SMPP_ESME_NODE: - vty->node = SMPP_NODE; - vty->index = NULL; - break; - case SMPP_NODE: - case MGCP_NODE: - case GBPROXY_NODE: - case SGSN_NODE: - case NAT_NODE: - case BSC_NODE: - vty->node = CONFIG_NODE; - vty->index = NULL; - break; - case OML_NODE: - case OM2K_NODE: - vty->node = ENABLE_NODE; - talloc_free(vty->index); - vty->index = NULL; - break; - case MSC_NODE: - case MNCC_INT_NODE: - vty->node = CONFIG_NODE; - break; - case TRUNK_NODE: - vty->node = MGCP_NODE; - vty->index = NULL; - break; - default: - break; - } + bsc_vty_go_parent (vty); return CMD_SUCCESS; }
@@ -186,36 +130,24 @@ gDEFUN(ournode_exit, gDEFUN(ournode_end, ournode_end_cmd, "end", "End current mode and change to enable mode.") { - switch (vty->node) { - case VIEW_NODE: - case ENABLE_NODE: - /* Nothing to do. */ - break; - case CONFIG_NODE: - case GSMNET_NODE: - case BTS_NODE: - case TRX_NODE: - case TS_NODE: - case MGCP_NODE: - case TRUNK_NODE: - case GBPROXY_NODE: - case SGSN_NODE: - case VTY_NODE: - case NAT_NODE: - case NAT_BSC_NODE: - case PGROUP_NODE: - case MSC_NODE: - case MNCC_INT_NODE: - case SMPP_NODE: - case SMPP_ESME_NODE: - case BSC_NODE: + enum node_type last_node = CONFIG_NODE; + + if (vty->node >= CONFIG_NODE) { + /* Repeatedly call go_parent until a top node is reached. */ + while (vty->node > CONFIG_NODE) { + if (vty->node == last_node) { + /* Ensure termination, this shouldn't happen. */ + break; + } + last_node = vty->node; + bsc_vty_go_parent(vty); + } + vty_config_unlock(vty); - vty->node = ENABLE_NODE; + if (vty->node > ENABLE_NODE) + vty->node = ENABLE_NODE; vty->index = NULL; vty->index_sub = NULL; - break; - default: - break; } return CMD_SUCCESS; }
On Mon, Sep 02, 2013 at 01:17:15PM +0200, Jacob Erlbeck wrote:
Dear Jacob,
thanks a lot for the updated patch! I have applied this patch locally (and updated the osmo-python-tests installation).
- if (vty->node >= CONFIG_NODE) {
as discussed in the office, I have replaced this with "> ENABLE_NODE". Can you please double check tomorrow if I have done that correctly?
/* Repeatedly call go_parent until a top node is reached. */while (vty->node > CONFIG_NODE) {if (vty->node == last_node) {
Shall we attempt to print a LOGL_ERROR if that happens? E.g. we would jump from the loop to the enable node.
Add bsc_install_default() and replace all install_default()
This patch adds bsc_install_default() which calls install_default() and add 'exit' and 'end'. All other calls to install_default() are replaced by calls to bsc_install_default().
Since 'exit' and 'end' are now added automatically to each node, the explicit registrations of these commands are removed by this patch, too.
The related tests succeed now without work-arounds (except for the 'config' node itself which is part of libosmocore). --- openbsc/include/openbsc/vty.h | 2 ++ openbsc/src/gprs/gb_proxy_vty.c | 4 +--- openbsc/src/gprs/sgsn_vty.c | 4 +--- openbsc/src/libbsc/abis_nm_vty.c | 3 +-- openbsc/src/libbsc/abis_om2000_vty.c | 3 +-- openbsc/src/libbsc/bsc_vty.c | 16 ++++------------ openbsc/src/libcommon/common_vty.c | 10 ++++++++++ openbsc/src/libmgcp/mgcp_vty.c | 8 ++------ openbsc/src/libmsc/smpp_vty.c | 4 ++-- openbsc/src/libmsc/vty_interface_layer3.c | 4 +--- openbsc/src/osmo-bsc/osmo_bsc_vty.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_nat_vty.c | 12 +++--------- openbsc/tests/vty_test_runner.py | 19 +++++-------------- 13 files changed, 35 insertions(+), 58 deletions(-)
diff --git a/openbsc/include/openbsc/vty.h b/openbsc/include/openbsc/vty.h index 183fc25..f74516a 100644 --- a/openbsc/include/openbsc/vty.h +++ b/openbsc/include/openbsc/vty.h @@ -42,6 +42,8 @@ enum bsc_vty_node { extern int bsc_vty_is_config_node(struct vty *vty, int node); extern void bsc_replace_string(void *ctx, char **dst, const char *newstr);
+void bsc_install_default(enum node_type node); + struct log_info; int bsc_vty_init(const struct log_info *cat); int bsc_vty_init_extra(void); diff --git a/openbsc/src/gprs/gb_proxy_vty.c b/openbsc/src/gprs/gb_proxy_vty.c index bfa1f3b..63546d3 100644 --- a/openbsc/src/gprs/gb_proxy_vty.c +++ b/openbsc/src/gprs/gb_proxy_vty.c @@ -82,9 +82,7 @@ int gbproxy_vty_init(void)
install_element(CONFIG_NODE, &cfg_gbproxy_cmd); install_node(&gbproxy_node, config_write_gbproxy); - install_default(GBPROXY_NODE); - install_element(GBPROXY_NODE, &ournode_exit_cmd); - install_element(GBPROXY_NODE, &ournode_end_cmd); + bsc_install_default(GBPROXY_NODE); install_element(GBPROXY_NODE, &cfg_nsip_sgsn_nsei_cmd);
return 0; diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c index a4ba280..8106b7d 100644 --- a/openbsc/src/gprs/sgsn_vty.c +++ b/openbsc/src/gprs/sgsn_vty.c @@ -418,9 +418,7 @@ int sgsn_vty_init(void)
install_element(CONFIG_NODE, &cfg_sgsn_cmd); install_node(&sgsn_node, config_write_sgsn); - install_default(SGSN_NODE); - install_element(SGSN_NODE, &ournode_exit_cmd); - install_element(SGSN_NODE, &ournode_end_cmd); + bsc_install_default(SGSN_NODE); install_element(SGSN_NODE, &cfg_sgsn_bind_addr_cmd); install_element(SGSN_NODE, &cfg_ggsn_remote_ip_cmd); //install_element(SGSN_NODE, &cfg_ggsn_remote_port_cmd); diff --git a/openbsc/src/libbsc/abis_nm_vty.c b/openbsc/src/libbsc/abis_nm_vty.c index fd60210..5db667d 100644 --- a/openbsc/src/libbsc/abis_nm_vty.c +++ b/openbsc/src/libbsc/abis_nm_vty.c @@ -183,8 +183,7 @@ int abis_nm_vty_init(void) install_element(ENABLE_NODE, &oml_classnum_inst_cmd); install_node(&oml_node, dummy_config_write);
- install_default(OML_NODE); - install_element(OML_NODE, &ournode_exit_cmd); + bsc_install_default(OML_NODE); install_element(OML_NODE, &oml_chg_adm_state_cmd); install_element(OML_NODE, &oml_opstart_cmd);
diff --git a/openbsc/src/libbsc/abis_om2000_vty.c b/openbsc/src/libbsc/abis_om2000_vty.c index 3df005b..eb8f4d5 100644 --- a/openbsc/src/libbsc/abis_om2000_vty.c +++ b/openbsc/src/libbsc/abis_om2000_vty.c @@ -445,8 +445,7 @@ int abis_om2k_vty_init(void) install_element(ENABLE_NODE, &om2k_classnum_inst_cmd); install_node(&om2k_node, dummy_config_write);
- install_default(OM2K_NODE); - install_element(OM2K_NODE, &ournode_exit_cmd); + bsc_install_default(OM2K_NODE); install_element(OM2K_NODE, &om2k_reset_cmd); install_element(OM2K_NODE, &om2k_start_cmd); install_element(OM2K_NODE, &om2k_status_cmd); diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index 45df90f..5748945 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -3002,9 +3002,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(CONFIG_NODE, &cfg_net_cmd); install_node(&net_node, config_write_net); - install_default(GSMNET_NODE); - install_element(GSMNET_NODE, &ournode_exit_cmd); - install_element(GSMNET_NODE, &ournode_end_cmd); + bsc_install_default(GSMNET_NODE); install_element(GSMNET_NODE, &cfg_net_ncc_cmd); install_element(GSMNET_NODE, &cfg_net_mnc_cmd); install_element(GSMNET_NODE, &cfg_net_name_short_cmd); @@ -3040,9 +3038,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(GSMNET_NODE, &cfg_bts_cmd); install_node(&bts_node, config_write_bts); - install_default(BTS_NODE); - install_element(BTS_NODE, &ournode_exit_cmd); - install_element(BTS_NODE, &ournode_end_cmd); + bsc_install_default(BTS_NODE); install_element(BTS_NODE, &cfg_bts_type_cmd); install_element(BTS_NODE, &cfg_description_cmd); install_element(BTS_NODE, &cfg_no_description_cmd); @@ -3102,9 +3098,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(BTS_NODE, &cfg_trx_cmd); install_node(&trx_node, dummy_config_write); - install_default(TRX_NODE); - install_element(TRX_NODE, &ournode_exit_cmd); - install_element(TRX_NODE, &ournode_end_cmd); + bsc_install_default(TRX_NODE); install_element(TRX_NODE, &cfg_trx_arfcn_cmd); install_element(TRX_NODE, &cfg_description_cmd); install_element(TRX_NODE, &cfg_no_description_cmd); @@ -3116,9 +3110,7 @@ int bsc_vty_init(const struct log_info *cat)
install_element(TRX_NODE, &cfg_ts_cmd); install_node(&ts_node, dummy_config_write); - install_default(TS_NODE); - install_element(TS_NODE, &ournode_exit_cmd); - install_element(TS_NODE, &ournode_end_cmd); + bsc_install_default(TS_NODE); install_element(TS_NODE, &cfg_ts_pchan_cmd); install_element(TS_NODE, &cfg_ts_pchan_compat_cmd); install_element(TS_NODE, &cfg_ts_tsc_cmd); diff --git a/openbsc/src/libcommon/common_vty.c b/openbsc/src/libcommon/common_vty.c index 948bd49..b5b77c4 100644 --- a/openbsc/src/libcommon/common_vty.c +++ b/openbsc/src/libcommon/common_vty.c @@ -174,3 +174,13 @@ void bsc_replace_string(void *ctx, char **dst, const char *newstr) talloc_free(*dst); *dst = talloc_strdup(ctx, newstr); } + +void bsc_install_default(enum node_type node) +{ + install_default (node); + + if (node > CONFIG_NODE) { + install_element(node, &ournode_exit_cmd); + install_element(node, &ournode_end_cmd); + } +} diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c index 833908a..9421f4f 100644 --- a/openbsc/src/libmgcp/mgcp_vty.c +++ b/openbsc/src/libmgcp/mgcp_vty.c @@ -786,9 +786,7 @@ int mgcp_vty_init(void) install_element(CONFIG_NODE, &cfg_mgcp_cmd); install_node(&mgcp_node, config_write_mgcp);
- install_default(MGCP_NODE); - install_element(MGCP_NODE, &ournode_exit_cmd); - install_element(MGCP_NODE, &ournode_end_cmd); + bsc_install_default(MGCP_NODE); install_element(MGCP_NODE, &cfg_mgcp_local_ip_cmd); install_element(MGCP_NODE, &cfg_mgcp_bts_ip_cmd); install_element(MGCP_NODE, &cfg_mgcp_bind_ip_cmd); @@ -820,9 +818,7 @@ int mgcp_vty_init(void)
install_element(MGCP_NODE, &cfg_mgcp_trunk_cmd); install_node(&trunk_node, config_write_trunk); - install_default(TRUNK_NODE); - install_element(TRUNK_NODE, &ournode_exit_cmd); - install_element(TRUNK_NODE, &ournode_end_cmd); + bsc_install_default(TRUNK_NODE); install_element(TRUNK_NODE, &cfg_trunk_payload_number_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_name_cmd); install_element(TRUNK_NODE, &cfg_trunk_payload_number_cmd_old); diff --git a/openbsc/src/libmsc/smpp_vty.c b/openbsc/src/libmsc/smpp_vty.c index 4829eb9..abf9733 100644 --- a/openbsc/src/libmsc/smpp_vty.c +++ b/openbsc/src/libmsc/smpp_vty.c @@ -487,7 +487,7 @@ static int config_write_esme(struct vty *v) int smpp_vty_init(void) { install_node(&smpp_node, config_write_smpp); - install_default(SMPP_NODE); + bsc_install_default(SMPP_NODE); install_element(CONFIG_NODE, &cfg_smpp_cmd);
install_element(SMPP_NODE, &cfg_smpp_port_cmd); @@ -497,7 +497,7 @@ int smpp_vty_init(void) install_element(SMPP_NODE, &cfg_no_esme_cmd);
install_node(&esme_node, config_write_esme); - install_default(SMPP_ESME_NODE); + bsc_install_default(SMPP_ESME_NODE); install_element(SMPP_ESME_NODE, &cfg_esme_passwd_cmd); install_element(SMPP_ESME_NODE, &cfg_esme_no_passwd_cmd); install_element(SMPP_ESME_NODE, &cfg_esme_route_pfx_cmd); diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index 79c3457..19c78ea 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -975,9 +975,7 @@ int bsc_vty_init_extra(void)
install_element(CONFIG_NODE, &cfg_mncc_int_cmd); install_node(&mncc_int_node, config_write_mncc_int); - install_default(MNCC_INT_NODE); - install_element(MNCC_INT_NODE, &ournode_exit_cmd); - install_element(MNCC_INT_NODE, &ournode_end_cmd); + bsc_install_default(MNCC_INT_NODE); install_element(MNCC_INT_NODE, &mnccint_def_codec_f_cmd); install_element(MNCC_INT_NODE, &mnccint_def_codec_h_cmd);
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_vty.c b/openbsc/src/osmo-bsc/osmo_bsc_vty.c index f6cf1a0..90b0a0c 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_vty.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_vty.c @@ -609,7 +609,7 @@ int bsc_vty_init_extra(void) install_element(CONFIG_NODE, &cfg_net_bsc_cmd);
install_node(&bsc_node, config_write_bsc); - install_default(BSC_NODE); + bsc_install_default(BSC_NODE); install_element(BSC_NODE, &cfg_net_bsc_mid_call_text_cmd); install_element(BSC_NODE, &cfg_net_bsc_mid_call_timeout_cmd); install_element(BSC_NODE, &cfg_net_rf_socket_cmd); @@ -617,7 +617,7 @@ int bsc_vty_init_extra(void) install_element(BSC_NODE, &cfg_net_no_rf_off_time_cmd);
install_node(&msc_node, config_write_msc); - install_default(MSC_NODE); + bsc_install_default(MSC_NODE); install_element(MSC_NODE, &cfg_net_bsc_token_cmd); install_element(MSC_NODE, &cfg_net_bsc_ncc_cmd); install_element(MSC_NODE, &cfg_net_bsc_mcc_cmd); diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c index 36a46f2..511d62a 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c @@ -1192,9 +1192,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) /* nat group */ install_element(CONFIG_NODE, &cfg_nat_cmd); install_node(&nat_node, config_write_nat); - install_default(NAT_NODE); - install_element(NAT_NODE, &ournode_exit_cmd); - install_element(NAT_NODE, &ournode_end_cmd); + bsc_install_default(NAT_NODE); install_element(NAT_NODE, &cfg_nat_msc_ip_cmd); install_element(NAT_NODE, &cfg_nat_msc_port_cmd); install_element(NAT_NODE, &cfg_nat_auth_time_cmd); @@ -1235,18 +1233,14 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_nat_pgroup_cmd); install_element(NAT_NODE, &cfg_nat_no_pgroup_cmd); install_node(&pgroup_node, config_write_pgroup); - install_default(PGROUP_NODE); - install_element(PGROUP_NODE, &ournode_exit_cmd); - install_element(PGROUP_NODE, &ournode_end_cmd); + bsc_install_default(PGROUP_NODE); install_element(PGROUP_NODE, &cfg_pgroup_lac_cmd); install_element(PGROUP_NODE, &cfg_pgroup_no_lac_cmd);
/* BSC subgroups */ install_element(NAT_NODE, &cfg_bsc_cmd); install_node(&bsc_node, config_write_bsc); - install_default(NAT_BSC_NODE); - install_element(NAT_BSC_NODE, &ournode_exit_cmd); - install_element(NAT_BSC_NODE, &ournode_end_cmd); + bsc_install_default(NAT_BSC_NODE); install_element(NAT_BSC_NODE, &cfg_bsc_token_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_lac_cmd); install_element(NAT_BSC_NODE, &cfg_bsc_no_lac_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index e91d018..912f9f2 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -128,10 +128,7 @@ class TestVTYNITB(TestVTYGenericBSC): self.assertEquals(self.vty.node(), 'config-smpp') self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
@@ -185,20 +182,14 @@ class TestVTYBSC(TestVTYGenericBSC): self.ignoredCheckForEndAndExit() self.assertTrue(self.vty.verify("msc 0", [''])) self.assertEquals(self.vty.node(), 'config-msc') - self.ignoredCheckForEndAndExit() + self.checkForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.assertTrue(self.vty.verify("bsc", [''])) self.assertEquals(self.vty.node(), 'config-bsc') - self.ignoredCheckForEndAndExit() + self.checkForEndAndExit() self.assertTrue(self.vty.verify("exit", [''])) - if self.notIgnoredTest(): - self.assertEquals(self.vty.node(), 'config') - else: - self.assertTrue(self.vty.verify("configure terminal", [''])) + self.assertEquals(self.vty.node(), 'config') self.assertTrue(self.vty.verify("exit", [''])) self.assertTrue(self.vty.node() is None)
Currently the 'mgcp' command fails in the 'config-nat' node, because it get confused with 'mgcp-through-msc-ipa' which is executed instead because of the prefix based command selection. Thus the latter command is renamed by this patch to avoid the common prefix.
The workaround in the test suite is removed. --- openbsc/src/osmo-bsc_nat/bsc_nat_vty.c | 10 +++++----- openbsc/tests/vty_test_runner.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c index 511d62a..125fbf1 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c @@ -152,7 +152,7 @@ static int config_write_nat(struct vty *vty) llist_for_each_entry(pgroup, &_nat->paging_groups, entry) write_pgroup_lst(vty, pgroup); if (_nat->mgcp_ipa) - vty_out(vty, " mgcp-through-msc-ipa%s", VTY_NEWLINE); + vty_out(vty, " use-msc-ipa-for-mgcp%s", VTY_NEWLINE);
return CMD_SUCCESS; } @@ -754,9 +754,9 @@ DEFUN(cfg_nat_ussd_local, return CMD_SUCCESS; }
-DEFUN(cfg_nat_mgcp_ipa, - cfg_nat_mgcp_ipa_cmd, - "mgcp-through-msc-ipa", +DEFUN(cfg_nat_use_ipa_for_mgcp, + cfg_nat_use_ipa_for_mgcp_cmd, + "use-msc-ipa-for-mgcp", "This needs to be set at start. Handle MGCP messages through " "the IPA protocol and not through the UDP socket.\n") { @@ -1209,7 +1209,7 @@ int bsc_nat_vty_init(struct bsc_nat *nat) install_element(NAT_NODE, &cfg_nat_ussd_query_cmd); install_element(NAT_NODE, &cfg_nat_ussd_token_cmd); install_element(NAT_NODE, &cfg_nat_ussd_local_cmd); - install_element(NAT_NODE, &cfg_nat_mgcp_ipa_cmd); + install_element(NAT_NODE, &cfg_nat_use_ipa_for_mgcp_cmd);
/* access-list */ install_element(NAT_NODE, &cfg_lst_imsi_allow_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index 912f9f2..e85dbe2 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -269,13 +269,13 @@ class TestVTYNAT(TestVTYGenericBSC): self.vty.command('mgcp') self.vty.command('nat') self.assertEquals(self.vty.node(), 'config-nat') - self.vty.command('line vty') - self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('mgcp') + self.assertEquals(self.vty.node(), 'config-mgcp') self.vty.command('nat') self.assertEquals(self.vty.node(), 'config-nat') self.vty.command('bsc 0') - self.vty.command('line vty') - self.assertEquals(self.vty.node(), 'config-line') + self.vty.command('mgcp') + self.assertEquals(self.vty.node(), 'config-mgcp')
def testRewriteNoRewrite(self): self.vty.enable()
-
Holger Hans Peter Freyther -
Jacob Erlbeck