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

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 <osmocom/bsc/bsc_subscr_conn_fsm.h>
 #include <osmocom/bsc/osmo_bsc.h>
 #include <osmocom/bsc/bsc_msc_data.h>
+#include <osmocom/bsc/bsc_subscriber.h>
 #include <osmocom/bsc/debug.h>
 
 #include <osmocom/bsc/gsm_04_80.h>
@@ -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