[2/2] Add vty command to explicitly reset given BVCI
diff mbox

Message ID 1458225747-19222-2-git-send-email-msuraev@sysmocom.de
State New
Headers show

Commit Message

Max March 17, 2016, 2:42 p.m. UTC
From: Max <msuraev@sysmocom.de>

It's useful for debugging and is similar to existing nsvc reset vty
command.
---
 src/gb/gprs_bssgp_vty.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Harald Welte March 17, 2016, 3:54 p.m. UTC | #1
Hi Max,

On Thu, Mar 17, 2016 at 03:42:27PM +0100, msuraev@sysmocom.de wrote:
> +DEFUN(bvc_reset, bvc_reset_cmd,
> +	"bvc reset nsei <0-65535> bvci <0-65535>",

[...]

> +	if (!strcmp(argv[0], "reset"))
> +		return CMD_WARNING;

no need to do this.  the generic vty parser code will make sure that you
don't end up here if the command doesn't match the synfax above.

> +	if (argc != 2)
> +		return CMD_WARNING;

same here.  If the number of arguments is not the number of mandatory
arguments, you will never get called.

So while defensive programming is generally a good idea, in the VTY case
it doesn't add any benefit and just makes the functions longer.  We
never do this kind of checking in other functions, as we rely on
libosmovty to handle this.

The only places where you need to check for argc is when you have
optional arguments, and !strcmp() is only needed if you have something
like "bvc (start|stop)" in order to determien if start or stop was
stated.

Also, it might make sense to prefix the entire command with bssgp so
that it becomes "bssgp bvc reset ...."
Neels Hofmeyr March 18, 2016, 10:17 a.m. UTC | #2
On Thu, Mar 17, 2016 at 04:54:58PM +0100, Harald Welte wrote:
> Hi Max,
> 
> On Thu, Mar 17, 2016 at 03:42:27PM +0100, msuraev@sysmocom.de wrote:
> > +DEFUN(bvc_reset, bvc_reset_cmd,
> > +	"bvc reset nsei <0-65535> bvci <0-65535>",
> 
> [...]
> 
> > +	if (!strcmp(argv[0], "reset"))
> > +		return CMD_WARNING;
> 
> no need to do this.  the generic vty parser code will make sure that you
> don't end up here if the command doesn't match the synfax above.
> 
> > +	if (argc != 2)
> > +		return CMD_WARNING;
> 
> same here.  If the number of arguments is not the number of mandatory
> arguments, you will never get called.

I used to do the same kind of checking for my first VTY commands, until I
figured out that all of the formatting is already checked ;)
Quite handy.

~Neels

Patch
diff mbox

diff --git a/src/gb/gprs_bssgp_vty.c b/src/gb/gprs_bssgp_vty.c
index 080867b..2725d1f 100644
--- a/src/gb/gprs_bssgp_vty.c
+++ b/src/gb/gprs_bssgp_vty.c
@@ -33,6 +33,7 @@ 
 #include <osmocom/core/rate_ctr.h>
 #include <osmocom/gprs/gprs_ns.h>
 #include <osmocom/gprs/gprs_bssgp.h>
+#include <osmocom/gprs/gprs_bssgp_bss.h>
 
 #include <osmocom/vty/vty.h>
 #include <osmocom/vty/command.h>
@@ -113,6 +114,31 @@  static void dump_bssgp(struct vty *vty, int stats)
 	}
 }
 
+DEFUN(bvc_reset, bvc_reset_cmd,
+	"bvc reset nsei <0-65535> bvci <0-65535>",
+	"Initiate BVC RESET procedure for a given NSEI and BVCI\n")
+{
+	uint16_t nsei = atoi(argv[0]), bvci = atoi(argv[1]);
+	struct bssgp_bvc_ctx *bvc;
+
+	if (!strcmp(argv[0], "reset"))
+		return CMD_WARNING;
+
+	if (argc != 2)
+		return CMD_WARNING;
+
+	bvc = btsctx_by_bvci_nsei(bvci, nsei);
+	if (!bvc) {
+		vty_out(vty, "No BVC for NSEI %d BVCI %d%s", nsei, bvci,
+			VTY_NEWLINE);
+		return CMD_WARNING;
+	}
+	int r = bssgp_tx_bvc_reset(bvc, bvci, BSSGP_CAUSE_OML_INTERV);
+	vty_out(vty, "Sent BVC RESET for NSEI %d BVCI %d: %d%s", nsei, bvci, r,
+		VTY_NEWLINE);
+	return CMD_SUCCESS;
+}
+
 #define BSSGP_STR "Show information about the BSSGP protocol\n"
 
 DEFUN(show_bssgp, show_bssgp_cmd, "show bssgp",
@@ -185,6 +211,7 @@  int bssgp_vty_init(void)
 	install_element_ve(&show_bssgp_stats_cmd);
 	install_element_ve(&show_bvc_cmd);
 	install_element_ve(&logging_fltr_bvc_cmd);
+	install_element_ve(&bvc_reset_cmd);
 
 	install_element(CFG_LOG_NODE, &logging_fltr_bvc_cmd);