From 78fc746b10755a645d8d1b52bdcca88fa492f32c Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Fri, 10 Mar 2017 02:17:14 +0100 Subject: Reinstate msc subscriber conn ref counting Indeed the easiest way of ensuring all code paths can cause conn discarding while still all code paths can check on whether a conn needs to be discarded. Change-Id: I641fe36d9fa2077e3caf63cc583aaa380603bff0 --- openbsc/src/libmsc/gsm_04_08.c | 5 ++-- openbsc/src/libmsc/gsm_04_11.c | 4 +-- openbsc/src/libmsc/iucs.c | 3 +- openbsc/src/libmsc/osmo_msc.c | 65 +++++++++++++++++++++------------------- openbsc/src/libmsc/silent_call.c | 4 +-- openbsc/src/libmsc/subscr_conn.c | 16 +++------- openbsc/src/libmsc/transaction.c | 2 +- 7 files changed, 47 insertions(+), 52 deletions(-) (limited to 'openbsc/src') diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index ade072097..01cae149e 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -199,7 +199,6 @@ void gsm0408_clear_request(struct gsm_subscriber_connection *conn, uint32_t caus LOGP(DMM, LOGL_ERROR, "%s: Conn clear request on uninitialized conn\n", vlr_subscr_name(conn->vsub)); - msc_subscr_con_free(conn); return; } @@ -3362,7 +3361,7 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) } /* Assign conn */ - trans->conn = subscr_con_get(conn); + trans->conn = msc_conn_get(conn); vlr_subscr_put(vsub); } else { /* update the subscriber we deal with */ @@ -3512,7 +3511,7 @@ static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *m return -ENOMEM; } /* Assign transaction */ - trans->conn = subscr_con_get(conn); + trans->conn = msc_conn_get(conn); } /* find function for current state and message */ diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index d2a9723bf..cb02be2f7 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -832,7 +832,7 @@ int gsm0411_rcv_sms(struct gsm_subscriber_connection *conn, gsm411_smr_init(&trans->sms.smr_inst, 0, 1, gsm411_rl_recv, gsm411_mn_send); - trans->conn = subscr_con_get(conn); + trans->conn = msc_conn_get(conn); new_trans = 1; } @@ -913,7 +913,7 @@ int gsm411_send_sms(struct gsm_subscriber_connection *conn, struct gsm_sms *sms) gsm411_rl_recv, gsm411_mn_send); trans->sms.sms = sms; - trans->conn = subscr_con_get(conn); + trans->conn = msc_conn_get(conn); /* Hardcode SMSC Originating Address for now */ data = (uint8_t *)msgb_put(msg, 8); diff --git a/openbsc/src/libmsc/iucs.c b/openbsc/src/libmsc/iucs.c index 8d763d223..0ef09a820 100644 --- a/openbsc/src/libmsc/iucs.c +++ b/openbsc/src/libmsc/iucs.c @@ -169,7 +169,8 @@ int gsm0408_rcvmsg_iucs(struct gsm_network *network, struct msgb *msg, uint8_t pdisc = gh->proto_discr & 0x0f; OSMO_ASSERT(pdisc != GSM48_PDISC_RR); - rc = gsm0408_dispatch(conn, msg); + msc_dtap(conn, ue_ctx->conn_id, msg); + rc = 0; } else { /* allocate a new connection */ diff --git a/openbsc/src/libmsc/osmo_msc.c b/openbsc/src/libmsc/osmo_msc.c index e7976f5ad..644c55c1c 100644 --- a/openbsc/src/libmsc/osmo_msc.c +++ b/openbsc/src/libmsc/osmo_msc.c @@ -48,38 +48,45 @@ static int msc_clear_request(struct gsm_subscriber_connection *conn, uint32_t ca return 1; } +static void subscr_conn_bump(struct gsm_subscriber_connection *conn) +{ + if (!conn) + return; + if (!conn->conn_fsm) + return; + if (!(conn->conn_fsm->state == SUBSCR_CONN_S_ACCEPTED + || conn->conn_fsm->state == SUBSCR_CONN_S_COMMUNICATING)) + return; + osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_BUMP, NULL); +} + /* receive a Level 3 Complete message and return MSC_CONN_ACCEPT or * MSC_CONN_REJECT */ int msc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg, uint16_t chosen_channel) { + msc_conn_get(conn); gsm0408_dispatch(conn, msg); + /* Bump whether the conn wants to be closed */ + subscr_conn_bump(conn); + msc_conn_put(conn); + /* Always return acceptance, because even if the conn was not accepted, * we assumed ownership of it and the caller shall not interfere with * that. We may even already have discarded the conn. */ return MSC_CONN_ACCEPT; } -static void subscr_conn_bump(struct gsm_subscriber_connection *conn) -{ - if (!conn) - return; - if (!conn->conn_fsm) - return; - if (!(conn->conn_fsm->state == SUBSCR_CONN_S_ACCEPTED - || conn->conn_fsm->state == SUBSCR_CONN_S_COMMUNICATING)) - return; - osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_BUMP, NULL); -} - /* Receive a DTAP message from BSC */ void msc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id, struct msgb *msg) { + msc_conn_get(conn); gsm0408_dispatch(conn, msg); /* Bump whether the conn wants to be closed */ subscr_conn_bump(conn); + msc_conn_put(conn); } /* Receive an ASSIGNMENT COMPLETE from BSC */ @@ -203,7 +210,6 @@ void msc_subscr_con_cleanup(struct gsm_subscriber_connection *conn) ? OSMO_FSM_TERM_REGULAR : OSMO_FSM_TERM_ERROR, NULL); - conn->conn_fsm = NULL; } void msc_subscr_con_free(struct gsm_subscriber_connection *conn) @@ -241,20 +247,17 @@ void msc_close_connection(struct gsm_subscriber_connection *conn) return; if (conn->in_release) return; - if (!conn->conn_fsm) { - /* No FSM means no valid process is ongoing. Discard right - * away. */ - msc_subscr_con_free(conn); + if (!conn->conn_fsm) return; - } if (conn->conn_fsm->state == SUBSCR_CONN_S_RELEASED) return; osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_CN_CLOSE, NULL); } /* increment the ref-count. Needs to be called by every user */ -struct gsm_subscriber_connection *_subscr_con_get(struct gsm_subscriber_connection *conn, - const char *file, int line) +struct gsm_subscriber_connection * +_msc_conn_get(struct gsm_subscriber_connection *conn, + const char *file, int line) { OSMO_ASSERT(conn); @@ -262,33 +265,33 @@ struct gsm_subscriber_connection *_subscr_con_get(struct gsm_subscriber_connecti return NULL; conn->use_count++; - LOGPSRC(DMSC, LOGL_DEBUG, file, line, - "subscr %s: increased subscr_con use_count to %u\n", + LOGPSRC(DREF, LOGL_DEBUG, file, line, + "%s: MSC conn use + 1 == %u\n", vlr_subscr_name(conn->vsub), conn->use_count); return conn; } /* decrement the ref-count. Once it reaches zero, we release */ -void _subscr_con_put(struct gsm_subscriber_connection *conn, - const char *file, int line) +void _msc_conn_put(struct gsm_subscriber_connection *conn, + const char *file, int line) { OSMO_ASSERT(conn); if (conn->use_count == 0) { - LOGP(DMSC, LOGL_ERROR, "trying to decrement conn use count, but is alrady 0\n"); + LOGPSRC(DREF, LOGL_ERROR, file, line, + "%s: MSC conn use - 1 failed: is already 0\n", + vlr_subscr_name(conn->vsub)); return; } conn->use_count--; - LOGPSRC(DMSC, LOGL_DEBUG, file, line, - "subscr %s: decreased subscr_conn use_count to %u\n", + LOGPSRC(DREF, LOGL_DEBUG, file, line, + "%s: MSC conn use - 1 == %u\n", vlr_subscr_name(conn->vsub), conn->use_count); -#if 0 - if (conn->use_count == 0 && conn->conn_fsm) - osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_MO_CLOSE, NULL); -#endif + if (conn->use_count == 0) + msc_subscr_con_free(conn); } void msc_stop_paging(struct vlr_subscr *vsub) diff --git a/openbsc/src/libmsc/silent_call.c b/openbsc/src/libmsc/silent_call.c index 7027dce11..f519fbb8c 100644 --- a/openbsc/src/libmsc/silent_call.c +++ b/openbsc/src/libmsc/silent_call.c @@ -57,7 +57,7 @@ static int paging_cb_silent(unsigned int hooknum, unsigned int event, conn->lchan->ts->nr, conn->lchan->ts->trx->arfcn); #endif conn->silent_call = 1; - subscr_con_get(conn); + msc_conn_get(conn); /* increment lchan reference count */ osmo_signal_dispatch(SS_SCALL, S_SCALL_SUCCESS, &sigdata); break; @@ -154,7 +154,7 @@ int gsm_silent_call_stop(struct vlr_subscr *vsub) #endif conn->silent_call = 0; - subscr_con_put(conn); + msc_conn_put(conn); return 0; } diff --git a/openbsc/src/libmsc/subscr_conn.c b/openbsc/src/libmsc/subscr_conn.c index 6eeb2dba3..41414f28f 100644 --- a/openbsc/src/libmsc/subscr_conn.c +++ b/openbsc/src/libmsc/subscr_conn.c @@ -231,15 +231,7 @@ static void subscr_conn_fsm_cleanup(struct osmo_fsm_inst *fi, conn->in_release = true; conn->conn_fsm = NULL; - /* If we're closing in a middle of a trans, we need to clean up */ - if (conn->use_count) { - DEBUGP(DMM, "%s: still in use (%u), cleaning up transactions\n", - vlr_subscr_name(conn->vsub), conn->use_count); - trans_conn_closed(conn); - } - if (conn->use_count) - LOGP(DMM, LOGL_ERROR, "%s: closing conn but still in use (%u)\n", - vlr_subscr_name(conn->vsub), conn->use_count); + trans_conn_closed(conn); if (conn->via_ran == RAN_UTRAN_IU) iu_tx_release(conn->iu.ue_ctx, NULL); @@ -247,7 +239,7 @@ static void subscr_conn_fsm_cleanup(struct osmo_fsm_inst *fi, * received from the UE, or a timeout expires. For now, the log * says "unknown UE" for each release outcome. */ - msc_subscr_con_free(conn); + msc_conn_put(conn); } int subscr_conn_fsm_timeout(struct osmo_fsm_inst *fi) @@ -340,8 +332,8 @@ int msc_create_conn_fsm(struct gsm_subscriber_connection *conn, const char *id) * subscriber connection. If the FSM is freed along with the subscriber * connection, then in _osmo_fsm_inst_term() the osmo_fsm_inst_free() * that follows the cleanup() call would run into a double free. */ - fi = osmo_fsm_inst_alloc(&subscr_conn_fsm, conn->network, conn, - LOGL_DEBUG, id); + fi = osmo_fsm_inst_alloc(&subscr_conn_fsm, conn->network, + msc_conn_get(conn), LOGL_DEBUG, id); if (!fi) { LOGP(DMM, LOGL_ERROR, diff --git a/openbsc/src/libmsc/transaction.c b/openbsc/src/libmsc/transaction.c index af9b30003..2c205af95 100644 --- a/openbsc/src/libmsc/transaction.c +++ b/openbsc/src/libmsc/transaction.c @@ -132,7 +132,7 @@ void trans_free(struct gsm_trans *trans) llist_del(&trans->entry); if (trans->conn) - subscr_con_put(trans->conn); + msc_conn_put(trans->conn); trans->conn = NULL; talloc_free(trans); -- cgit v1.2.3