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