Harald Welte has submitted this change and it was merged. (
https://gerrit.osmocom.org/10334 )
Change subject: GSCON: call api of a_reset.c with msc object directly
......................................................................
GSCON: call api of a_reset.c with msc object directly
The API of a_reset.c is currently called with a pointer to struct
reset_ctx. This puts the responsibility of checking the presence of
msc->a.reset_fsm to the caller. It would be much more effective if the
caller would check if msc->a.reset_fsm before dereferencing it.
This also fixes at least one segfault that ocurrs when gscon_timer_cb()
is called but no sccp connection is present yet. Therefore the pointer
to bsc_msc_data would not be populated. This is now detected by
a_reset.c itsself.
- minor code cleanups
- call a_reset.c functions with msc (struct bsc_msc_data)
Change-Id: I0802aaadf0af4e58e41c98999e8c6823838adb61
Related: OS#3447
---
M include/osmocom/bsc/a_reset.h
M src/osmo-bsc/a_reset.c
M src/osmo-bsc/bsc_subscr_conn_fsm.c
M src/osmo-bsc/gsm_08_08.c
M src/osmo-bsc/osmo_bsc_bssap.c
M src/osmo-bsc/osmo_bsc_sigtran.c
6 files changed, 54 insertions(+), 39 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/a_reset.h b/include/osmocom/bsc/a_reset.h
index 6b6ce81..a09972e 100644
--- a/include/osmocom/bsc/a_reset.h
+++ b/include/osmocom/bsc/a_reset.h
@@ -20,17 +20,19 @@
#pragma once
+struct bsc_msc_data;
+
/* Create and start state machine which handles the reset/reset-ack procedure
*/
-struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb,
void *priv);
+void a_reset_alloc(struct bsc_msc_data *msc, const char *name, void *cb);
/* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm);
+void a_reset_ack_confirm(struct bsc_msc_data *msc);
/* Report a failed connection */
-void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm);
+void a_reset_conn_fail(struct bsc_msc_data *msc);
/* Report a successful connection */
-void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm);
+void a_reset_conn_success(struct bsc_msc_data *msc);
/* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm);
+bool a_reset_conn_ready(struct bsc_msc_data *msc);
diff --git a/src/osmo-bsc/a_reset.c b/src/osmo-bsc/a_reset.c
index b8f8c8c..3c21142 100644
--- a/src/osmo-bsc/a_reset.c
+++ b/src/osmo-bsc/a_reset.c
@@ -137,68 +137,83 @@
};
/* Create and start state machine which handles the reset/reset-ack procedure
*/
-struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb,
void *priv)
+void a_reset_alloc(struct bsc_msc_data *msc, const char *name, void *cb)
{
- OSMO_ASSERT(name);
-
struct reset_ctx *reset_ctx;
struct osmo_fsm_inst *reset_fsm;
+ OSMO_ASSERT(msc);
+ OSMO_ASSERT(name);
+ OSMO_ASSERT(cb);
+
+ /* There must not be any double allocation! */
+ OSMO_ASSERT(msc->a.reset_fsm == NULL);
+
/* 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_ctx = talloc_zero(ctx, struct reset_ctx);
+ reset_ctx = talloc_zero(msc, struct reset_ctx);
OSMO_ASSERT(reset_ctx);
- reset_ctx->priv = priv;
+ reset_ctx->priv = msc;
reset_ctx->cb = cb;
reset_ctx->conn_loss_counter = 0;
- reset_fsm = osmo_fsm_inst_alloc(&fsm, ctx, reset_ctx, LOGL_DEBUG, name);
+ reset_fsm = osmo_fsm_inst_alloc(&fsm, msc, reset_ctx, LOGL_DEBUG, name);
OSMO_ASSERT(reset_fsm);
/* kick off reset-ack sending mechanism */
osmo_fsm_inst_state_chg(reset_fsm, ST_DISC, RESET_RESEND_INTERVAL,
RESET_RESEND_TIMER_NO);
- return reset_fsm;
+ msc->a.reset_fsm = reset_fsm;
}
/* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm)
+void a_reset_ack_confirm(struct bsc_msc_data *msc)
{
- OSMO_ASSERT(reset_fsm);
- osmo_fsm_inst_dispatch(reset_fsm, EV_RESET_ACK, NULL);
+ if (!msc)
+ return;
+
+ if (!msc->a.reset_fsm)
+ return;
+
+ osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_RESET_ACK, NULL);
}
/* Report a failed connection */
-void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm)
+void a_reset_conn_fail(struct bsc_msc_data *msc)
{
- /* If no reset context is supplied, just drop the info */
- if (!reset_fsm)
+ if (!msc)
return;
- osmo_fsm_inst_dispatch(reset_fsm, EV_N_DISCONNECT, NULL);
+ if (!msc->a.reset_fsm)
+ return;
+
+ osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_N_DISCONNECT, NULL);
}
/* Report a successful connection */
-void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm)
+void a_reset_conn_success(struct bsc_msc_data *msc)
{
- /* If no reset context is supplied, just drop the info */
- if (!reset_fsm)
+ if (!msc)
return;
- osmo_fsm_inst_dispatch(reset_fsm, EV_N_CONNECT, NULL);
+ if (!msc->a.reset_fsm)
+ return;
+
+ osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_N_CONNECT, NULL);
}
/* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm)
+bool a_reset_conn_ready(struct bsc_msc_data *msc)
{
- /* If no reset context is supplied, we assume that
- * the connection can't be ready! */
- if (!reset_fsm)
+ if (!msc)
return false;
- if (reset_fsm->state == ST_CONN)
+ if (!msc->a.reset_fsm)
+ return false;
+
+ if (msc->a.reset_fsm->state == ST_CONN)
return true;
return false;
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 81fe9f6..32375d4 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -771,7 +771,7 @@
* could indicate a bad SCCP connection. We now inform the the
* FSM that controls the BSSMAP reset about the event. Maybe
* a BSSMAP reset is necessary. */
- a_reset_conn_fail(conn->sccp.msc->a.reset_fsm);
+ a_reset_conn_fail(conn->sccp.msc);
/* Since we could not reach the MSC, we give up and terminate
* the FSM instance now (N-DISCONNET.req is sent in
@@ -783,7 +783,7 @@
* disconnected. */
LOGPFSML(fi, LOGL_ERROR, "Long after a BSSMAP Clear Command,
the conn is still not"
" released. For sanity, discarding this conn now.\n");
- a_reset_conn_fail(conn->sccp.msc->a.reset_fsm);
+ a_reset_conn_fail(conn->sccp.msc);
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
break;
default:
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index d1a6a96..0d7cdf0 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -48,7 +48,7 @@
return false;
/* Reset procedure not (yet) executed */
- if (a_reset_conn_ready(conn->sccp.msc->a.reset_fsm) == false)
+ if (a_reset_conn_ready(conn->sccp.msc) == false)
return false;
return true;
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 95bad7b..c067dd3 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -59,7 +59,7 @@
/* Inform the FSM that controls the RESET/RESET-ACK procedure
* that we have successfully received the reset-ack message */
- a_reset_ack_confirm(msc->a.reset_fsm);
+ a_reset_ack_confirm(msc);
return 0;
}
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index b97d51b..7e5f5f6 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -240,7 +240,7 @@
/* Incoming data is a sign of a vital connection */
conn = get_bsc_conn_by_conn_id(scu_prim->u.data.conn_id);
if (conn) {
- a_reset_conn_success(conn->sccp.msc->a.reset_fsm);
+ a_reset_conn_success(conn->sccp.msc);
handle_data_from_msc(conn, oph->msg);
}
break;
@@ -284,7 +284,7 @@
LOGP(DMSC, LOGL_NOTICE, "Initializing resources for new SIGTRAN
connection to MSC: %s...\n",
osmo_sccp_addr_name(ss7, &msc->a.msc_addr));
- if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+ if (a_reset_conn_ready(msc) == false) {
LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
return BSC_CON_REJECT_NO_LINK;
}
@@ -314,7 +314,7 @@
msc = conn->sccp.msc;
- if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+ if (a_reset_conn_ready(msc) == false) {
LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
return -EINVAL;
}
@@ -373,7 +373,7 @@
} else
LOGP(DMSC, LOGL_ERROR, "Tx MSC (message too short)\n");
- if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+ if (a_reset_conn_ready(msc) == false) {
LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
msgb_free(msg);
return -EINVAL;
@@ -532,9 +532,7 @@
return -EINVAL;
/* Start MSC-Reset procedure */
- msc->a.reset_fsm = a_reset_alloc(msc, msc_name,
osmo_bsc_sigtran_reset_cb, msc);
- if (!msc->a.reset_fsm)
- return -EINVAL;
+ a_reset_alloc(msc, msc_name, osmo_bsc_sigtran_reset_cb);
/* If we have detected that the SS7 configuration of the MSC we
have just initalized
* was incomplete or completely missing, we can not tolerate
another incomplete
--
To view, visit https://gerrit.osmocom.org/10334
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0802aaadf0af4e58e41c98999e8c6823838adb61
Gerrit-Change-Number: 10334
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
