From 1bcbd1b5a2558a48abaf925fd8784e3ef99c53af Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Tue, 9 Jan 2018 23:48:25 +0100 Subject: associate conn with bsc subscriber where possible (TODO) Various logging in the code (e.g. handover) attempts to log the subscriber information from conn->bsub, but so far, the only time we set conn->bsub is during paging. We do though get IMSI and/or TMSI information from Layer 3 Complete messaging, not only from the Paging Request, which is already used for IMSI filtering. Also extract TMSI and create a conn->bsub subscriber. <-- TODO Make sure conn->bsub also gets cleaned up by bsc_subscr_put() when the conn is freed. During gscon_cleanup, keep the conn->bsub as long as possible, to still allow logging the subscriber as context during teardown of other components. Change-Id: Icc20c141ec339385181949b93548a73121bb7615 --- src/libbsc/bsc_subscr_conn_fsm.c | 12 ++++++------ src/libbsc/bsc_subscriber.c | 5 ++--- src/libbsc/paging.c | 1 + src/osmo-bsc/osmo_bsc_api.c | 7 +++++++ tests/handover/handover_test.c | 13 +++++++++++-- tests/subscr/bsc_subscr_test.err | 3 +++ 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/libbsc/bsc_subscr_conn_fsm.c b/src/libbsc/bsc_subscr_conn_fsm.c index a63483e4e..788d4fb08 100644 --- a/src/libbsc/bsc_subscr_conn_fsm.c +++ b/src/libbsc/bsc_subscr_conn_fsm.c @@ -1019,12 +1019,6 @@ static void gscon_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cau conn->lchan = NULL; } - if (conn->bsub) { - LOGPFSML(fi, LOGL_DEBUG, "Putting bsc_subscr\n"); - bsc_subscr_put(conn->bsub); - conn->bsub = NULL; - } - if (conn->sccp.state != SUBSCR_SCCP_ST_NONE) { LOGPFSML(fi, LOGL_DEBUG, "Disconnecting SCCP\n"); struct bsc_msc_data *msc = conn->sccp.msc; @@ -1038,6 +1032,12 @@ static void gscon_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cau penalty_timers_free(&conn->hodec2.penalty_timers); + if (conn->bsub) { + LOGPFSML(fi, LOGL_DEBUG, "Putting bsc_subscr\n"); + bsc_subscr_put(conn->bsub); + conn->bsub = NULL; + } + llist_del(&conn->entry); talloc_free(conn); fi->priv = NULL; diff --git a/src/libbsc/bsc_subscriber.c b/src/libbsc/bsc_subscriber.c index 48bae041c..00f8beb09 100644 --- a/src/libbsc/bsc_subscriber.c +++ b/src/libbsc/bsc_subscriber.c @@ -41,7 +41,6 @@ static struct bsc_subscr *bsc_subscr_alloc(struct llist_head *list) return NULL; llist_add_tail(&bsub->entry, list); - bsub->use_count = 1; return bsc_subscr_get(bsub); } @@ -92,7 +91,7 @@ struct bsc_subscr *bsc_subscr_find_or_create_by_imsi(struct llist_head *list, return bsub; bsub = bsc_subscr_alloc(list); bsc_subscr_set_imsi(bsub, imsi); - return bsub; + return bsc_subscr_get(bsub); } struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list, @@ -104,7 +103,7 @@ struct bsc_subscr *bsc_subscr_find_or_create_by_tmsi(struct llist_head *list, return bsub; bsub = bsc_subscr_alloc(list); bsub->tmsi = tmsi; - return bsub; + return bsc_subscr_get(bsub); } const char *bsc_subscr_name(struct bsc_subscr *bsub) diff --git a/src/libbsc/paging.c b/src/libbsc/paging.c index 910a9b607..3f7c0e9c9 100644 --- a/src/libbsc/paging.c +++ b/src/libbsc/paging.c @@ -388,6 +388,7 @@ void paging_request_stop(struct llist_head *bts_list, struct gsm_bts *bts; log_set_context(LOG_CTX_BSC_SUBSCR, bsub); + conn->bsub = bsc_subscr_get(bsub); /* Stop this first and dispatch the request */ if (_bts) { diff --git a/src/osmo-bsc/osmo_bsc_api.c b/src/osmo-bsc/osmo_bsc_api.c index f3b120e70..ba4ce88df 100644 --- a/src/osmo-bsc/osmo_bsc_api.c +++ b/src/osmo-bsc/osmo_bsc_api.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -249,6 +250,12 @@ static int complete_layer3(struct gsm_subscriber_connection *conn, return BSC_API_CONN_POL_REJECT; } + /* TODO: also extract TMSI. We get an IMSI is only because the filtering functions extract the + * IMSI to filter by IMSI. A TMSI identity is never returned here, see e.g. _cr_check_loc_upd() + * and other similar functions called from bsc_msg_filter_initial(). */ + if (imsi) + conn->bsub = bsc_subscr_find_or_create_by_imsi(msc->network->bsc_subscribers, imsi); + /* allocate resource for a new connection */ ret = osmo_bsc_sigtran_new_conn(conn, msc); diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c index ab32a29a1..b299808c4 100644 --- a/tests/handover/handover_test.c +++ b/tests/handover/handover_test.c @@ -240,8 +240,11 @@ static struct gsm_bts *create_bts(int arfcn) void create_conn(struct gsm_lchan *lchan) { - struct gsm_subscriber_connection *conn; - conn = bsc_subscr_con_allocate(lchan->ts->trx->bts->network); + static unsigned int next_imsi = 0; + char imsi[sizeof(lchan->conn->bsub->imsi)]; + struct gsm_network *net = lchan->ts->trx->bts->network; + + conn = bsc_subscr_con_allocate(net); /* CAUTION HACK: When __real_mgcp_conn_modify() is called by the GSCON * FSM, then we need to know the reference to caller FSM (GSCON FSM). @@ -255,6 +258,12 @@ void create_conn(struct gsm_lchan *lchan) lchan->conn = conn; conn->lchan = lchan; + + /* Make up a new IMSI for this test, for logging the subscriber */ + next_imsi ++; + snprintf(imsi, sizeof(imsi), "%06u", next_imsi); + lchan->conn->bsub = bsc_subscr_find_or_create_by_imsi(net->bsc_subscribers, imsi); + /* kick the FSM from INIT through to the ACTIVE state */ osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CONN_REQ, NULL); osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CONN_CFM, NULL); diff --git a/tests/subscr/bsc_subscr_test.err b/tests/subscr/bsc_subscr_test.err index a66317a36..afc8bf778 100644 --- a/tests/subscr/bsc_subscr_test.err +++ b/tests/subscr/bsc_subscr_test.err @@ -1,5 +1,8 @@ +DREF BSC subscr IMSI:1234567890 usage increases to: 1 DREF BSC subscr IMSI:1234567890 usage increases to: 2 DREF BSC subscr IMSI:1234567890 usage decreases to: 1 +DREF BSC subscr IMSI:9876543210 usage increases to: 1 +DREF BSC subscr IMSI:5656565656 usage increases to: 1 DREF BSC subscr IMSI:1234567890 usage increases to: 2 DREF BSC subscr IMSI:1234567890 usage decreases to: 1 DREF BSC subscr IMSI:9876543210 usage increases to: 2 -- cgit v1.2.3