From bc7fabb8b92a4c05ffaaa3e6c7a34e3d8a985ac6 Mon Sep 17 00:00:00 2001 From: Philipp Maier Date: Fri, 30 Jun 2017 11:12:18 +0200 Subject: reset: fix allocation and add deallocator for reset procedure When the reset is started, state machines are allocated. The user must provide a correctly filled structure where the reset start function adds the state machine into. This is prone to errors, besides of that, a proper deallocator function which tears down the osmo fsm is missing. make a more cofmortable allocator function and add deallocator function. Change-Id: I0054fb31589fb9f4c63c45df436e83448c59bd97 --- include/openbsc/a_reset.h | 5 +++- include/openbsc/bsc_msc.h | 2 +- src/libcommon-cs/a_reset.c | 54 +++++++++++++++++++++++++++++++++++------ src/osmo-bsc/osmo_bsc_bssap.c | 2 +- src/osmo-bsc/osmo_bsc_sigtran.c | 17 ++++++------- 5 files changed, 59 insertions(+), 21 deletions(-) diff --git a/include/openbsc/a_reset.h b/include/openbsc/a_reset.h index 124cf13f1..343454a28 100644 --- a/include/openbsc/a_reset.h +++ b/include/openbsc/a_reset.h @@ -45,7 +45,10 @@ struct a_reset_ctx { }; /* Create and start state machine which handles the reset/reset-ack procedure */ -void a_reset_start(struct a_reset_ctx *reset); +struct a_reset_ctx *a_reset_alloc(void *ctx, char *name, void *cb, void *priv); + +/* Tear down state machine */ +void a_reset_free(struct a_reset_ctx *reset); /* Confirm that we sucessfully received a reset acknowlege message */ void a_reset_ack_confirm(struct a_reset_ctx *reset); diff --git a/include/openbsc/bsc_msc.h b/include/openbsc/bsc_msc.h index a7c881b84..380eb17c1 100644 --- a/include/openbsc/bsc_msc.h +++ b/include/openbsc/bsc_msc.h @@ -61,7 +61,7 @@ struct bsc_msc_connection { struct osmo_sccp_user *sccp_user; struct osmo_sccp_addr g_calling_addr; struct osmo_sccp_addr g_called_addr; - struct a_reset_ctx reset; + struct a_reset_ctx *reset; int conn_id_counter; }; diff --git a/src/libcommon-cs/a_reset.c b/src/libcommon-cs/a_reset.c index 94d215562..a6110a22b 100644 --- a/src/libcommon-cs/a_reset.c +++ b/src/libcommon-cs/a_reset.c @@ -132,18 +132,47 @@ static struct osmo_fsm fsm = { }; /* Create and start state machine which handles the reset/reset-ack procedure */ -void a_reset_start(struct a_reset_ctx *reset) +struct a_reset_ctx *a_reset_alloc(void *ctx, char *name, void *cb, void *priv) { - OSMO_ASSERT(reset); + OSMO_ASSERT(name); + + struct a_reset_ctx *reset; + + /* Register the fsm description (if not already done) */ + if (osmo_fsm_find_by_name(fsm.name) != &fsm) + osmo_fsm_register(&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; + strncpy(reset->name, name, sizeof(reset->name)); + reset->conn_loss_counter = 0; reset->fsm = osmo_fsm_inst_alloc(&fsm, NULL, NULL, LOGL_DEBUG, "FSM RESET INST"); OSMO_ASSERT(reset->fsm); - reset->fsm->priv = reset; + LOGP(DMSC, LOGL_NOTICE, "(%s) reset handler fsm created.\n", reset->name); /* kick off reset-ack sending mechanism */ 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); + + LOGP(DMSC, LOGL_NOTICE, "(%s) reset handler fsm destroyed.\n", reset->name); } /* Confirm that we sucessfully received a reset acknowlege message */ @@ -158,7 +187,10 @@ void a_reset_ack_confirm(struct a_reset_ctx *reset) /* Report a failed connection */ void a_reset_conn_fail(struct a_reset_ctx *reset) { - OSMO_ASSERT(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); @@ -167,7 +199,10 @@ void a_reset_conn_fail(struct a_reset_ctx *reset) /* Report a successful connection */ void a_reset_conn_success(struct a_reset_ctx *reset) { - OSMO_ASSERT(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); @@ -176,9 +211,12 @@ void a_reset_conn_success(struct a_reset_ctx *reset) /* Check if we have a connection to a specified msc */ bool a_reset_conn_ready(struct a_reset_ctx *reset) { - OSMO_ASSERT(reset); - OSMO_ASSERT(reset->fsm); + /* If no reset context is supplied, we assume that + * the connection can't be ready! */ + if (!reset) + return false; + OSMO_ASSERT(reset->fsm); if (reset->fsm->state == ST_CONN) return true; diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c index d6ae7dc0c..ee7557c85 100644 --- a/src/osmo-bsc/osmo_bsc_bssap.c +++ b/src/osmo-bsc/osmo_bsc_bssap.c @@ -198,7 +198,7 @@ static int bssmap_handle_reset_ack(struct bsc_msc_data *msc, /* Inform the FSM that controls the RESET/RESET-ACK procedure * that we have successfully received the reset-ack message */ - a_reset_ack_confirm(&msc->msc_con->reset); + a_reset_ack_confirm(msc->msc_con->reset); return 0; } diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c index 80dac828d..3b1a1c0f9 100644 --- a/src/osmo-bsc/osmo_bsc_sigtran.c +++ b/src/osmo-bsc/osmo_bsc_sigtran.c @@ -180,7 +180,7 @@ static int sccp_sap_up(struct osmo_prim_hdr *oph, void *_scu) /* Incoming data is a sign of a vital connection */ bsc_con = get_bsc_conn_by_conn_id(scu_prim->u.disconnect.conn_id); if (bsc_con) - a_reset_conn_success(&bsc_con->msc->msc_con->reset); + a_reset_conn_success(bsc_con->msc->msc_con->reset); rc = handle_data_from_msc(scu_prim->u.data.conn_id, oph->msg); break; @@ -200,7 +200,7 @@ static int sccp_sap_up(struct osmo_prim_hdr *oph, void *_scu) /* We might have a connectivity problem. Maybe we need to go * through the reset procedure again? */ if (scu_prim->u.disconnect.cause == 0) - a_reset_conn_fail(&bsc_con->msc->msc_con->reset); + a_reset_conn_fail(bsc_con->msc->msc_con->reset); rc = osmo_bsc_sigtran_del_conn(bsc_con); } @@ -227,7 +227,7 @@ enum bsc_con osmo_bsc_sigtran_new_conn(struct gsm_subscriber_connection *conn, s LOGP(DMSC, LOGL_NOTICE, "Initalizing resources for new SIGTRAN connection to MSC No.: %i...\n", msc->nr); - if (a_reset_conn_ready(&msc->msc_con->reset) == false) { + if (a_reset_conn_ready(msc->msc_con->reset) == false) { LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n"); return BSC_CON_REJECT_NO_LINK; } @@ -272,7 +272,7 @@ int osmo_bsc_sigtran_open_conn(struct osmo_bsc_sccp_con *conn, struct msgb *msg) msc = conn->msc; - if (a_reset_conn_ready(&msc->msc_con->reset) == false) { + if (a_reset_conn_ready(msc->msc_con->reset) == false) { LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n"); return -EINVAL; } @@ -299,7 +299,7 @@ int osmo_bsc_sigtran_send(struct osmo_bsc_sccp_con *conn, struct msgb *msg) msc = conn->msc; - if (a_reset_conn_ready(&msc->msc_con->reset) == false) { + if (a_reset_conn_ready(msc->msc_con->reset) == false) { LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n"); return -EINVAL; } @@ -330,7 +330,7 @@ int osmo_bsc_sigtran_del_conn(struct osmo_bsc_sccp_con *conn) /* This bahaviour might be caused by a bad connection. Maybe we * will have to go through the reset procedure again */ - a_reset_conn_fail(&conn->msc->msc_con->reset); + a_reset_conn_fail(conn->msc->msc_con->reset); } llist_del(&conn->entry); @@ -412,10 +412,7 @@ int osmo_bsc_sigtran_init(struct llist_head *mscs) osmo_sccp_user_bind(msc->msc_con->sccp, msc_name, sccp_sap_up, SCCP_SSN_BSSAP); /* Start MSC reset procedure */ - msc->msc_con->reset.priv = msc; - msc->msc_con->reset.cb = osmo_bsc_sigtran_reset_cb; - snprintf(msc->msc_con->reset.name,sizeof(msc->msc_con->reset.name),"MSC No. %u",msc->nr); - a_reset_start(&msc->msc_con->reset); + msc->msc_con->reset = a_reset_alloc(NULL, msc_name, osmo_bsc_sigtran_reset_cb, msc); } return 0; -- cgit v1.2.3