a_reset: cleanup + remove dead code

a_reset.c/h was originally developed to be used in both, bsc and
msc without changes. Unfortunately no suitable library has been
found for a_reset.c/h so the file ended up as duplicated code in
both split brances. Eventually we decided to specialize the
generalized code again, which means some of the functions needed
only by osmo-bsc are removed.

- Remove dead code
- Fix timer identification number (T16)
- use fi->priv to hold context info
- Minor cosmetic fixes

Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Depends: libosmocore I36d221c973d3890721ef1d376fb9be82c4311378
Related: OS#3103

5 files changed, 59 insertions, 159 deletions

diff --git a/include/osmocom/msc/a_iface.h b/include/osmocom/msc/a_iface.h
index 3098b514d..217011d2e 100644
--- a/include/osmocom/msc/a_iface.h
+++ b/include/osmocom/msc/a_iface.h
@@ -51,7 +51,7 @@ struct bsc_context {
 	/* A pointer to the reset handler FSM, the
 	 * state machine is allocated when the BSC
 	 * is registerd. */
-	struct a_reset_ctx *reset;
+	struct osmo_fsm_inst *reset_fsm;
 
 	/* A pointer to the sccp_user that is associated
 	 * with the A interface. We need this information
diff --git a/include/osmocom/msc/a_reset.h b/include/osmocom/msc/a_reset.h
index cdb17c288..8eb3bbfda 100644
--- a/include/osmocom/msc/a_reset.h
+++ b/include/osmocom/msc/a_reset.h
@@ -20,45 +20,12 @@
 
 #pragma once
 
-
-
-/* Reset context data (callbacks, state machine etc...) */
-struct a_reset_ctx {
-
-	/* FSM instance, which handles the reset procedure */
-	struct osmo_fsm_inst *fsm;
-
-	/* Connection failure counter. When this counter
-	 * reaches a certain threshold, the reset procedure
-	 * will be triggered */
-	int conn_loss_counter;
-
-	/* A human readable name to display in the logs */
-	char name[256];
-
-	/* Callback function to be called when a connection
-	 * failure is detected and a rest must occur */
-	void (*cb)(void *priv);
-
-	/* Privated data for the callback function */
-	void *priv;
-};
-
 /* Create and start state machine which handles the reset/reset-ack procedure */
-struct a_reset_ctx *a_reset_alloc(const void *ctx, const char *name, void *cb, void *priv,
-				  bool already_connected);
-
-/* Tear down state machine */
-void a_reset_free(struct a_reset_ctx *reset);
+struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb,
+				    void *priv, bool already_connected);
 
 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct a_reset_ctx *reset);
-
-/* Report a failed connection */
-void a_reset_conn_fail(struct a_reset_ctx *reset);
-
-/* Report a successful connection */
-void a_reset_conn_success(struct a_reset_ctx *reset);
+void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm);
 
 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct a_reset_ctx *reset);
+bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm);
diff --git a/src/libmsc/a_iface.c b/src/libmsc/a_iface.c
index 6f2000e00..75fa438e6 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -209,7 +209,7 @@ int a_iface_tx_paging(const char *imsi, uint32_t tmsi, uint16_t lac)
 
 	/* Deliver paging request to all known BSCs */
 	llist_for_each_entry(bsc_ctx, &gsm_network->a.bscs, list) {
-		if (a_reset_conn_ready(bsc_ctx->reset)) {
+		if (a_reset_conn_ready(bsc_ctx->reset_fsm)) {
 			LOGP(DBSSAP, LOGL_DEBUG,
 			     "Tx BSSMAP paging message from MSC %s to BSC %s (imsi=%s, tmsi=0x%08x, lac=%u)\n",
 			     osmo_sccp_addr_name(ss7, &bsc_ctx->msc_addr),
@@ -471,10 +471,10 @@ static struct bsc_context *add_bsc(const struct osmo_sccp_addr *msc_addr,
 void a_start_reset(struct bsc_context *bsc_ctx, bool already_connected)
 {
 	char bsc_name[32];
-	OSMO_ASSERT(bsc_ctx->reset == NULL);
+	OSMO_ASSERT(bsc_ctx->reset_fsm == NULL);
 	/* Start reset procedure to make the new connection active */
 	snprintf(bsc_name, sizeof(bsc_name), "bsc-%i", bsc_ctx->bsc_addr.pc);
-	bsc_ctx->reset = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, bsc_ctx, already_connected);
+	bsc_ctx->reset_fsm = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, bsc_ctx, already_connected);
 }
 
 /* determine if given msg is BSSMAP RESET related (true) or not (false) */
@@ -521,7 +521,7 @@ static int sccp_sap_up(struct osmo_prim_hdr *oph, void *_scu)
 			a_start_reset(a_conn_info.bsc, false);
 		} else {
 			/* This BSC is already known to us, check if we have been through reset yet */
-			if (a_reset_conn_ready(a_conn_info.bsc->reset) == false) {
+			if (a_reset_conn_ready(a_conn_info.bsc->reset_fsm) == false) {
 				LOGP(DBSSAP, LOGL_NOTICE, "Refusing N-CONNECT.ind(%u, %s), BSC not reset yet\n",
 				     scu_prim->u.connect.conn_id, msgb_hexdump_l2(oph->msg));
 				rc = osmo_sccp_tx_disconn(scu, a_conn_info.conn_id, &a_conn_info.bsc->msc_addr,
@@ -580,7 +580,7 @@ static int sccp_sap_up(struct osmo_prim_hdr *oph, void *_scu)
 
 		/* As long as we are in the reset phase, only reset related BSSMAP messages may pass
 		 * beond here. */
-		if (!bssmap_is_reset(oph->msg) && a_reset_conn_ready(a_conn_info.bsc->reset) == false) {
+		if (!bssmap_is_reset(oph->msg) && a_reset_conn_ready(a_conn_info.bsc->reset_fsm) == false) {
 			LOGP(DBSSAP, LOGL_NOTICE, "Ignoring N-UNITDATA.ind(%s), BSC not reset yet\n",
 			     msgb_hexdump_l2(oph->msg));
 			break;
diff --git a/src/libmsc/a_iface_bssap.c b/src/libmsc/a_iface_bssap.c
index 6d5848aa1..1ace43dae 100644
--- a/src/libmsc/a_iface_bssap.c
+++ b/src/libmsc/a_iface_bssap.c
@@ -114,12 +114,12 @@ static void bssmap_rx_reset(struct osmo_sccp_user *scu, const struct a_conn_info
 	/* Make sure all orphand subscriber connections will be cleard */
 	a_clear_all(scu, &a_conn_info->bsc->bsc_addr);
 
-	if (!a_conn_info->bsc->reset)
+	if (!a_conn_info->bsc->reset_fsm)
 		a_start_reset(a_conn_info->bsc, true);
 
 	/* Treat an incoming RESET like an ACK to any RESET request we may have just sent.
 	 * After all, what we wanted is the A interface to be reset, which we now know has happened. */
-	a_reset_ack_confirm(a_conn_info->bsc->reset);
+	a_reset_ack_confirm(a_conn_info->bsc->reset_fsm);
 }
 
 /* Endpoint to handle BSSMAP reset acknowlegement */
@@ -133,7 +133,7 @@ static void bssmap_rx_reset_ack(const struct osmo_sccp_user *scu, const struct a
 	ss7 = osmo_ss7_instance_find(network->a.cs7_instance);
 	OSMO_ASSERT(ss7);
 
-	if (a_conn_info->bsc->reset == NULL) {
+	if (a_conn_info->bsc->reset_fsm == NULL) {
 		LOGP(DBSSAP, LOGL_ERROR, "Received RESET ACK from an unknown BSC %s, ignoring...\n",
 		     osmo_sccp_addr_name(ss7, &a_conn_info->bsc->bsc_addr));
 		return;
@@ -144,7 +144,7 @@ static void bssmap_rx_reset_ack(const struct osmo_sccp_user *scu, const struct a
 
 	/* Confirm that we managed to get the reset ack message
 	 * towards the connection reset logic */
-	a_reset_ack_confirm(a_conn_info->bsc->reset);
+	a_reset_ack_confirm(a_conn_info->bsc->reset_fsm);
 }
 
 /* Handle UNITDATA BSSMAP messages */
diff --git a/src/libmsc/a_reset.c b/src/libmsc/a_reset.c
index 701066f34..1e35a108c 100644
--- a/src/libmsc/a_reset.c
+++ b/src/libmsc/a_reset.c
@@ -28,188 +28,121 @@
 #include <osmocom/msc/debug.h>
 #include <osmocom/msc/a_reset.h>
 
-#define RESET_RESEND_INTERVAL 2	/* sec */
-#define RESET_RESEND_TIMER_NO 1234	/* FIXME: dig out the real timer number */
-#define BAD_CONNECTION_THRESOLD 3	/* connection failures */
+#define RESET_RESEND_INTERVAL 2		/* sec */
+#define RESET_RESEND_TIMER_NO 16	/* See also 3GPP TS 48.008 Chapter 3.1.4.1.3.2 */
 
-enum fsm_states {
+enum reset_fsm_states {
 	ST_DISC,		/* Disconnected from remote end */
 	ST_CONN,		/* We have a confirmed connection */
 };
 
-enum fsm_evt {
-	EV_RESET_ACK,		/* got reset acknowlegement from remote end */
-	EV_N_DISCONNECT,	/* lost a connection */
-	EV_N_CONNECT,		/* made a successful connection */
+enum reset_fsm_evt {
+	EV_CONN_ACK,		/* Received either BSSMAP RESET or BSSMAP RESET
+				 * ACK from the remote end */
+};
+
+/* Reset context data (callbacks, state machine etc...) */
+struct reset_ctx {
+	/* Callback function to be called when a connection
+	 * failure is detected and a rest must occur */
+	void (*cb)(void *priv);
+
+	/* Privated data for the callback function */
+	void *priv;
 };
 
 static const struct value_string fsm_event_names[] = {
-	OSMO_VALUE_STRING(EV_RESET_ACK),
-	OSMO_VALUE_STRING(EV_N_DISCONNECT),
-	OSMO_VALUE_STRING(EV_N_CONNECT),
+	OSMO_VALUE_STRING(EV_CONN_ACK),
 	{0, NULL}
 };
 
 /* Disconnected state */
 static void fsm_disc_cb(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
-	struct a_reset_ctx *reset = (struct a_reset_ctx *)data;
-	OSMO_ASSERT(reset);
-	OSMO_ASSERT(reset->fsm);
-	LOGPFSML(reset->fsm, LOGL_NOTICE, "SIGTRAN connection succeded.\n");
-
-	reset->conn_loss_counter = 0;
 	osmo_fsm_inst_state_chg(fi, ST_CONN, 0, 0);
 }
 
-/* Connected state */
-static void fsm_conn_cb(struct osmo_fsm_inst *fi, uint32_t event, void *data)
-{
-	struct a_reset_ctx *reset = (struct a_reset_ctx *)data;
-	OSMO_ASSERT(reset);
-
-	switch (event) {
-	case EV_N_DISCONNECT:
-		if (reset->conn_loss_counter >= BAD_CONNECTION_THRESOLD) {
-			LOGPFSML(reset->fsm, LOGL_NOTICE, "SIGTRAN connection down, reconnecting...\n");
-			osmo_fsm_inst_state_chg(fi, ST_DISC, RESET_RESEND_INTERVAL, RESET_RESEND_TIMER_NO);
-		} else
-			reset->conn_loss_counter++;
-		break;
-	case EV_N_CONNECT:
-		reset->conn_loss_counter = 0;
-		break;
-	}
-}
-
 /* Timer callback to retransmit the reset signal */
 static int fsm_reset_ack_timeout_cb(struct osmo_fsm_inst *fi)
 {
-	struct a_reset_ctx *reset = (struct a_reset_ctx *)fi->priv;
-	OSMO_ASSERT(reset->fsm);
-
-	LOGPFSML(reset->fsm, LOGL_NOTICE, "(re)sending BSSMAP RESET message...\n");
-
-	reset->cb(reset->priv);
-
+	struct reset_ctx *reset_ctx = (struct reset_ctx *)fi->priv;
+	LOGPFSML(fi, LOGL_NOTICE, "(re)sending BSSMAP RESET message...\n");
+	reset_ctx->cb(reset_ctx->priv);
 	osmo_fsm_inst_state_chg(fi, ST_DISC, RESET_RESEND_INTERVAL, RESET_RESEND_TIMER_NO);
 	return 0;
 }
 
-static struct osmo_fsm_state fsm_states[] = {
+static struct osmo_fsm_state reset_fsm_states[] = {
 	[ST_DISC] = {
-		     .in_event_mask = (1 << EV_RESET_ACK),
-		     .out_state_mask = (1 << ST_DISC) | (1 << ST_CONN),
+		     .in_event_mask = (1 << EV_CONN_ACK),
+		     .out_state_mask = (1 << ST_CONN) | (1 << ST_DISC),
 		     .name = "DISC",
 		     .action = fsm_disc_cb,
 		     },
 	[ST_CONN] = {
-		     .in_event_mask = (1 << EV_N_DISCONNECT) | (1 << EV_N_CONNECT),
-		     .out_state_mask = (1 << ST_DISC) | (1 << ST_CONN),
+		     .in_event_mask = (1 << EV_CONN_ACK),
 		     .name = "CONN",
-		     .action = fsm_conn_cb,
 		     },
 };
 
 /* State machine definition */
 static struct osmo_fsm fsm = {
 	.name = "A-RESET",
-	.states = fsm_states,
-	.num_states = ARRAY_SIZE(fsm_states),
+	.states = reset_fsm_states,
+	.num_states = ARRAY_SIZE(reset_fsm_states),
 	.log_subsys = DMSC,
 	.timer_cb = fsm_reset_ack_timeout_cb,
 	.event_names = fsm_event_names,
 };
 
 /* Create and start state machine which handles the reset/reset-ack procedure */
-struct a_reset_ctx *a_reset_alloc(const void *ctx, const char *name, void *cb, void *priv,
-				  bool already_connected)
+struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb,
+				    void *priv, bool already_connected)
 {
 	OSMO_ASSERT(name);
 
-	struct a_reset_ctx *reset;
+	struct reset_ctx *reset_ctx;
+	struct osmo_fsm_inst *reset_fsm;
 
 	/* Register the fsm description (if not already done) */
 	if (osmo_fsm_find_by_name(fsm.name) != &fsm)
 		osmo_fsm_register(&fsm);
 
 	/* Allocate and configure a new fsm instance */
-	reset = talloc_zero(ctx, struct a_reset_ctx);
-	OSMO_ASSERT(reset);
-	reset->priv = priv;
-	reset->cb = cb;
-	reset->conn_loss_counter = 0;
-	reset->fsm = osmo_fsm_inst_alloc(&fsm, NULL, NULL, LOGL_DEBUG, name);
-	OSMO_ASSERT(reset->fsm);
-	reset->fsm->priv = reset;
+	reset_ctx = talloc_zero(ctx, struct reset_ctx);
+	OSMO_ASSERT(reset_ctx);
+	reset_ctx->priv = priv;
+	reset_ctx->cb = cb;
+        reset_fsm = osmo_fsm_inst_alloc(&fsm, ctx, reset_ctx, LOGL_DEBUG, name);
+	OSMO_ASSERT(reset_fsm);
 
 	if (already_connected)
-		osmo_fsm_inst_state_chg(reset->fsm, ST_CONN, 0, 0);
+		osmo_fsm_inst_state_chg(reset_fsm, ST_CONN, 0, 0);
 	else {
 		/* kick off reset-ack sending mechanism */
-		osmo_fsm_inst_state_chg(reset->fsm, ST_DISC, RESET_RESEND_INTERVAL,
+		osmo_fsm_inst_state_chg(reset_fsm, ST_DISC, RESET_RESEND_INTERVAL,
 					RESET_RESEND_TIMER_NO);
 	}
 
-	return reset;
-}
-
-/* Tear down state machine */
-void a_reset_free(struct a_reset_ctx *reset)
-{
-	OSMO_ASSERT(reset);
-	OSMO_ASSERT(reset->fsm);
-
-	osmo_fsm_inst_free(reset->fsm);
-	reset->fsm = NULL;
-
-	memset(reset, 0, sizeof(*reset));
-	talloc_free(reset);
+	return reset_fsm;
 }
 
 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct a_reset_ctx *reset)
+void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm)
 {
-	OSMO_ASSERT(reset);
-	OSMO_ASSERT(reset->fsm);
-
-	osmo_fsm_inst_dispatch(reset->fsm, EV_RESET_ACK, reset);
-}
-
-/* Report a failed connection */
-void a_reset_conn_fail(struct a_reset_ctx *reset)
-{
-	/* If no reset context is supplied, just drop the info */
-	if (!reset)
-		return;
-
-	OSMO_ASSERT(reset->fsm);
-
-	osmo_fsm_inst_dispatch(reset->fsm, EV_N_DISCONNECT, reset);
-}
-
-/* Report a successful connection */
-void a_reset_conn_success(struct a_reset_ctx *reset)
-{
-	/* If no reset context is supplied, just drop the info */
-	if (!reset)
-		return;
-
-	OSMO_ASSERT(reset->fsm);
-
-	osmo_fsm_inst_dispatch(reset->fsm, EV_N_CONNECT, reset);
+	OSMO_ASSERT(reset_fsm);
+	osmo_fsm_inst_dispatch(reset_fsm, EV_CONN_ACK, NULL);
 }
 
 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct a_reset_ctx *reset)
+bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm)
 {
 	/* If no reset context is supplied, we assume that
 	 * the connection can't be ready! */
-	if (!reset)
+	if (!reset_fsm)
 		return false;
 
-	OSMO_ASSERT(reset->fsm);
-	if (reset->fsm->state == ST_CONN)
+	if (reset_fsm->state == ST_CONN)
 		return true;
 
 	return false;