diff mbox

[6/6] smpp: refactor initialization, add bind address

Message ID 1456343319-572-7-git-send-email-nhofmeyr@sysmocom.de
State Accepted
Headers show

Commit Message

Neels Hofmeyr Feb. 24, 2016, 7:48 p.m. UTC
Make the SMPP bind address configurable (used to be harcoded as "0.0.0.0").

Add VTY command

    smpp
     local-tcp A.B.C.D <1-65535>

while keeping the old command 'local-tcp-port <1-65535>'. Both the old and the
new command immediately change the SMPP listening address and port.

Add a LOGL_NOTICE log when the SMPP listening address and/or port change.

However, to be useful, this patch has to go somewhat further: refactor the
initialization procedure, because it was impossible to run the VTY commands
without an already established connection.

The SMPP initialization procedure was weird. It would first open a connection
on the default port, and a subsequent VTY port reconfiguration while reading
the config file would try to re-establish a connection on a different port. If
that failed, smpp would switch back to the default port instead of failing the
program launch as the user would expect. If anything else ran on port 2775,
SMPP would thus refuse to launch despite the config file having a different
port: the first bind would always happen on 0.0.0.0:2775. Change that.

In the VTY commands, merely store address and port if no fd is established yet.

Introduce several SMPP initialization stages:

* allocate struct and initialize pointers,
* then read config file without immediately starting to listen,
* and once the main program is ready, start listening.

After that, the VTY command behaves as before: try to re-establish the old
connection if the newly supplied address and port don't work out. I'm not
actually sure why this switch-back behavior is needed, but fair enough.

In detail, replace the function
  smpp_smsc_init()
with the various steps
  smpp_smsc_alloc_init() -- prepare struct for VTY commands
  smpp_smsc_conf() -- set addr an port only, for reading the config file
  smpp_smsc_start() -- establish a first connection, for main()
  smpp_smsc_restart() -- switch running connection, for telnet VTY
  smpp_smsc_stop() -- tear down connection, used by _start() twice

And replace
  smpp_openbsc_init()
  smpp_openbsc_set_net()
with
  smpp_openbsc_alloc_init()
  smpp_openbsc_start()

I'd have picked function names like "_bind"/"_unbind", but in the SMPP protocol
there is also a bind/unbind process, so instead I chose the names "_start",
"_restart" and "_stop".

The smsc struct used to be talloc'd outside of smpp_smsc_init(). Since the smsc
code internally uses talloc anyway and employs the smsc struct as talloc
context, I decided to enforce talloc allocation within smpp_smsc_alloc_init().

Be stricter about osmo_signal_register_handler() return codes.
---
 openbsc/include/openbsc/smpp.h    |  4 +-
 openbsc/src/libmsc/smpp_openbsc.c | 43 +++++++++++-------
 openbsc/src/libmsc/smpp_smsc.c    | 93 +++++++++++++++++++++++++++++----------
 openbsc/src/libmsc/smpp_smsc.h    |  7 ++-
 openbsc/src/libmsc/smpp_vty.c     | 75 ++++++++++++++++++++++++++-----
 openbsc/src/osmo-nitb/bsc_hack.c  |  4 +-
 6 files changed, 169 insertions(+), 57 deletions(-)
diff mbox

Patch

diff --git a/openbsc/include/openbsc/smpp.h b/openbsc/include/openbsc/smpp.h
index 9941cee..bcdac8f 100644
--- a/openbsc/include/openbsc/smpp.h
+++ b/openbsc/include/openbsc/smpp.h
@@ -1,4 +1,4 @@ 
 #pragma once
 
-int smpp_openbsc_init(void *ctx, uint16_t port);
-void smpp_openbsc_set_net(struct gsm_network *net);
+int smpp_openbsc_alloc_init(void *ctx);
+int smpp_openbsc_start(struct gsm_network *net);
diff --git a/openbsc/src/libmsc/smpp_openbsc.c b/openbsc/src/libmsc/smpp_openbsc.c
index a2fa0f4..0269f4b 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -569,27 +569,38 @@  struct smsc *smsc_from_vty(struct vty *v)
 	return g_smsc;
 }
 
-/*! \brief Initialize the OpenBSC SMPP interface */
-int smpp_openbsc_init(void *ctx, uint16_t port)
+/*! \brief Allocate the OpenBSC SMPP interface struct and init VTY. */
+int smpp_openbsc_alloc_init(void *ctx)
+{
+	g_smsc = smpp_smsc_alloc_init(ctx);
+	if (!g_smsc) {
+		LOGP(DSMPP, LOGL_FATAL, "Cannot allocate smsc struct\n");
+		return -1;
+	}
+	return smpp_vty_init();
+}
+
+/*! \brief Launch the OpenBSC SMPP interface with the parameters set from VTY.
+ */
+int smpp_openbsc_start(struct gsm_network *net)
 {
-	struct smsc *smsc = talloc_zero(ctx, struct smsc);
 	int rc;
+	g_smsc->priv = net;
 
-	rc = smpp_smsc_init(smsc, port);
+	/* If a VTY configuration has taken place, the values have been stored
+	 * in the smsc struct. Otherwise, use the defaults (NULL -> any, 0 ->
+	 * default SMPP port, see smpp_smsc_bind()). */
+	rc = smpp_smsc_start(g_smsc, g_smsc->bind_addr, g_smsc->listen_port);
 	if (rc < 0)
-		talloc_free(smsc);
-
-	osmo_signal_register_handler(SS_SMS, smpp_sms_cb, smsc);
-	osmo_signal_register_handler(SS_SUBSCR, smpp_subscr_cb, smsc);
+		return rc;
 
-	g_smsc = smsc;
-
-	smpp_vty_init();
+	rc = osmo_signal_register_handler(SS_SMS, smpp_sms_cb, g_smsc);
+	if (rc < 0)
+		return rc;
+	rc = osmo_signal_register_handler(SS_SUBSCR, smpp_subscr_cb, g_smsc);
+	if (rc < 0)
+		return rc;
 
-	return rc;
+	return 0;
 }
 
-void smpp_openbsc_set_net(struct gsm_network *net)
-{
-	g_smsc->priv = net;
-}
diff --git a/openbsc/src/libmsc/smpp_smsc.c b/openbsc/src/libmsc/smpp_smsc.c
index c1ec22f..ef4277a 100644
--- a/openbsc/src/libmsc/smpp_smsc.c
+++ b/openbsc/src/libmsc/smpp_smsc.c
@@ -931,39 +931,84 @@  static int smsc_fd_cb(struct osmo_fd *ofd, unsigned int what)
 	return link_accept_cb(ofd->data, rc, &sa, sa_len);
 }
 
-/*! \brief Initialize the SMSC-side SMPP implementation */
-int smpp_smsc_init(struct smsc *smsc, uint16_t port)
+/*! \brief allocate and initialize an smsc struct from talloc context ctx. */
+struct smsc *smpp_smsc_alloc_init(void *ctx)
+{
+	struct smsc *smsc = talloc_zero(ctx, struct smsc);
+
+	INIT_LLIST_HEAD(&smsc->esme_list);
+	INIT_LLIST_HEAD(&smsc->acl_list);
+	INIT_LLIST_HEAD(&smsc->route_list);
+
+	smsc->listen_ofd.data = smsc;
+	smsc->listen_ofd.cb = smsc_fd_cb;
+
+	return smsc;
+}
+
+/*! \brief Set the SMPP address and port without binding. */
+int smpp_smsc_conf(struct smsc *smsc, const char *bind_addr, uint16_t port)
+{
+	talloc_free((void*)smsc->bind_addr);
+	smsc->bind_addr = NULL;
+	if (bind_addr) {
+		smsc->bind_addr = talloc_strdup(smsc, bind_addr);
+		if (!smsc->bind_addr)
+			return -ENOMEM;
+	}
+	smsc->listen_port = port;
+	return 0;
+}
+
+/*! \brief Bind to given address and port and accept connections.
+ * \param[in] bind_addr Local IP address, may be NULL for any.
+ * \param[in] port TCP port number, may be 0 for default SMPP (2775).
+ */
+int smpp_smsc_start(struct smsc *smsc, const char *bind_addr, uint16_t port)
 {
 	int rc;
 
 	/* default port for SMPP */
-	if (port == 0)
+	if (!port)
 		port = 2775;
 
-	/* This will not work if we were to actually ever use FD 0
-	 * (stdin) for this ... */
-	if (smsc->listen_ofd.fd <= 0) {
-		INIT_LLIST_HEAD(&smsc->esme_list);
-		INIT_LLIST_HEAD(&smsc->acl_list);
-		INIT_LLIST_HEAD(&smsc->route_list);
-		smsc->listen_ofd.data = smsc;
-		smsc->listen_ofd.cb = smsc_fd_cb;
-	} else {
-		close(smsc->listen_ofd.fd);
-		osmo_fd_unregister(&smsc->listen_ofd);
-	}
+	smpp_smsc_stop(smsc);
+
+	LOGP(DSMPP, LOGL_NOTICE, "SMPP at %s %d\n",
+	     bind_addr? bind_addr : "0.0.0.0", port);
 
 	rc = osmo_sock_init_ofd(&smsc->listen_ofd, AF_UNSPEC, SOCK_STREAM,
-				IPPROTO_TCP, NULL, port,
+				IPPROTO_TCP, bind_addr, port,
 				OSMO_SOCK_F_BIND);
+	if (rc < 0)
+		return rc;
 
-	/* if there is an error, try to re-bind to the old port */
-	if (rc < 0) {
-		rc = osmo_sock_init_ofd(&smsc->listen_ofd, AF_UNSPEC,
-					SOCK_STREAM, IPPROTO_TCP, NULL,
-					smsc->listen_port, OSMO_SOCK_F_BIND);
-	} else
-		smsc->listen_port = port;
-
+	/* store new address and port */
+	rc = smpp_smsc_conf(smsc, bind_addr, port);
+	if (rc)
+		smpp_smsc_stop(smsc);
 	return rc;
 }
+
+/*! \brief Change a running connection to a different address/port, and upon
+ * error switch back to the running configuration. */
+int smpp_smsc_restart(struct smsc *smsc, const char *bind_addr, uint16_t port)
+{
+	int rc;
+
+	rc = smpp_smsc_start(smsc, bind_addr, port);
+	if (rc)
+		/* if there is an error, try to re-bind to the old port */
+		return smpp_smsc_start(smsc, smsc->bind_addr, smsc->listen_port);
+	return 0;
+}
+
+/*! /brief Close SMPP connection. */
+void smpp_smsc_stop(struct smsc *smsc)
+{
+	if (smsc->listen_ofd.fd > 0) {
+		close(smsc->listen_ofd.fd);
+		smsc->listen_ofd.fd = 0;
+		osmo_fd_unregister(&smsc->listen_ofd);
+	}
+}
diff --git a/openbsc/src/libmsc/smpp_smsc.h b/openbsc/src/libmsc/smpp_smsc.h
index 3dd6562..bd20137 100644
--- a/openbsc/src/libmsc/smpp_smsc.h
+++ b/openbsc/src/libmsc/smpp_smsc.h
@@ -89,6 +89,7 @@  struct smsc {
 	struct llist_head esme_list;
 	struct llist_head acl_list;
 	struct llist_head route_list;
+	const char *bind_addr;
 	uint16_t listen_port;
 	char system_id[SMPP_SYS_ID_LEN+1];
 	int accept_all;
@@ -100,7 +101,11 @@  struct smsc {
 int smpp_addr_eq(const struct osmo_smpp_addr *a,
 		 const struct osmo_smpp_addr *b);
 
-int smpp_smsc_init(struct smsc *smsc, uint16_t port);
+struct smsc *smpp_smsc_alloc_init(void *ctx);
+int smpp_smsc_conf(struct smsc *smsc, const char *bind_addr, uint16_t port);
+int smpp_smsc_start(struct smsc *smsc, const char *bind_addr, uint16_t port);
+int smpp_smsc_restart(struct smsc *smsc, const char *bind_addr, uint16_t port);
+void smpp_smsc_stop(struct smsc *smsc);
 
 void smpp_esme_get(struct osmo_esme *esme);
 void smpp_esme_put(struct osmo_esme *esme);
diff --git a/openbsc/src/libmsc/smpp_vty.c b/openbsc/src/libmsc/smpp_vty.c
index c0695fe..5ab632f 100644
--- a/openbsc/src/libmsc/smpp_vty.c
+++ b/openbsc/src/libmsc/smpp_vty.c
@@ -76,31 +76,76 @@  DEFUN(cfg_no_smpp_first, cfg_no_smpp_first_cmd,
 	return CMD_SUCCESS;
 }
 
-DEFUN(cfg_smpp_port, cfg_smpp_port_cmd,
-	"local-tcp-port <1-65535>",
-	"Set the local TCP port on which we listen for SMPP\n"
-	"TCP port number")
+static int smpp_local_tcp(struct vty *vty,
+			  const char *bind_addr, uint16_t port)
 {
 	struct smsc *smsc = smsc_from_vty(vty);
-	uint16_t port = atoi(argv[0]);
+	int is_running = smsc->listen_ofd.fd > 0;
+	int same_bind_addr;
 	int rc;
 
-	rc = smpp_smsc_init(smsc, port);
+	/* If it is not up yet, don't rebind, just set values. */
+	if (!is_running) {
+		rc = smpp_smsc_conf(smsc, bind_addr, port);
+		if (rc < 0) {
+			vty_out(vty, "%% Cannot configure new address:port%s",
+				VTY_NEWLINE);
+			return CMD_WARNING;
+		}
+		return CMD_SUCCESS;
+	}
+
+	rc = smpp_smsc_restart(smsc, bind_addr, port);
 	if (rc < 0) {
-		vty_out(vty, "%% Cannot bind to new port %u nor to "
-			"old port %u%s", port, smsc->listen_port, VTY_NEWLINE);
+		vty_out(vty, "%% Cannot bind to new port %s:%u nor to"
+			" old port %s:%u%s",
+			bind_addr? bind_addr : "0.0.0.0",
+			port,
+			smsc->bind_addr? smsc->bind_addr : "0.0.0.0",
+			smsc->listen_port,
+			VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
-	if (port != smsc->listen_port) {
-		vty_out(vty, "%% Cannot bind to new port %u, staying on old"
-			"port %u%s", port, smsc->listen_port, VTY_NEWLINE);
+	same_bind_addr = (bind_addr == smsc->bind_addr)
+		|| (bind_addr && smsc->bind_addr
+		    && (strcmp(bind_addr, smsc->bind_addr) == 0));
+
+	if (!same_bind_addr || port != smsc->listen_port) {
+		vty_out(vty, "%% Cannot bind to new port %s:%u, staying on"
+			" old port %s:%u%s",
+			bind_addr? bind_addr : "0.0.0.0",
+			port,
+			smsc->bind_addr? smsc->bind_addr : "0.0.0.0",
+			smsc->listen_port,
+			VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
 	return CMD_SUCCESS;
 }
 
+DEFUN(cfg_smpp_port, cfg_smpp_port_cmd,
+	"local-tcp-port <1-65535>",
+	"Set the local TCP port on which we listen for SMPP\n"
+	"TCP port number")
+{
+	struct smsc *smsc = smsc_from_vty(vty);
+	uint16_t port = atoi(argv[0]);
+	return smpp_local_tcp(vty, smsc->bind_addr, port);
+}
+
+DEFUN(cfg_smpp_addr_port, cfg_smpp_addr_port_cmd,
+	"local-tcp A.B.C.D <1-65535>",
+	"Set the local IP address and TCP port on which we listen for SMPP\n"
+	"Local IP address\n"
+	"TCP port number")
+{
+	const char *bind_addr = argv[0];
+	uint16_t port = atoi(argv[1]);
+	return smpp_local_tcp(vty, bind_addr, port);
+}
+
 DEFUN(cfg_smpp_sys_id, cfg_smpp_sys_id_cmd,
 	"system-id ID",
 	"Set the System ID of this SMSC\n"
@@ -138,7 +183,12 @@  static int config_write_smpp(struct vty *vty)
 	struct smsc *smsc = smsc_from_vty(vty);
 
 	vty_out(vty, "smpp%s", VTY_NEWLINE);
-	vty_out(vty, " local-tcp-port %u%s", smsc->listen_port, VTY_NEWLINE);
+	if (smsc->bind_addr)
+		vty_out(vty, " local-tcp %s %u%s", smsc->bind_addr,
+			smsc->listen_port, VTY_NEWLINE);
+	else
+		vty_out(vty, " local-tcp-port %u%s", smsc->listen_port,
+			VTY_NEWLINE);
 	if (strlen(smsc->system_id) > 0)
 		vty_out(vty, " system-id %s%s", smsc->system_id, VTY_NEWLINE);
 	vty_out(vty, " policy %s%s",
@@ -535,6 +585,7 @@  int smpp_vty_init(void)
 	install_element(SMPP_NODE, &cfg_smpp_first_cmd);
 	install_element(SMPP_NODE, &cfg_no_smpp_first_cmd);
 	install_element(SMPP_NODE, &cfg_smpp_port_cmd);
+	install_element(SMPP_NODE, &cfg_smpp_addr_port_cmd);
 	install_element(SMPP_NODE, &cfg_smpp_sys_id_cmd);
 	install_element(SMPP_NODE, &cfg_smpp_policy_cmd);
 	install_element(SMPP_NODE, &cfg_esme_cmd);
diff --git a/openbsc/src/osmo-nitb/bsc_hack.c b/openbsc/src/osmo-nitb/bsc_hack.c
index 4bd03fc..3bd73f4 100644
--- a/openbsc/src/osmo-nitb/bsc_hack.c
+++ b/openbsc/src/osmo-nitb/bsc_hack.c
@@ -276,7 +276,7 @@  int main(int argc, char **argv)
 	ctrl_vty_init(tall_bsc_ctx);
 
 #ifdef BUILD_SMPP
-	if (smpp_openbsc_init(tall_bsc_ctx, 0) < 0)
+	if (smpp_openbsc_alloc_init(tall_bsc_ctx) < 0)
 		return -1;
 #endif
 
@@ -293,7 +293,7 @@  int main(int argc, char **argv)
 	if (rc < 0)
 		exit(1);
 #ifdef BUILD_SMPP
-	smpp_openbsc_set_net(bsc_gsmnet);
+	smpp_openbsc_start(bsc_gsmnet);
 #endif
 	bsc_api_init(bsc_gsmnet, msc_bsc_api());