This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Harald Welte gerrit-no-reply at lists.osmocom.orgHarald 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 <pmaier at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180807/7df26ce9/attachment.htm>