subscr_conn: store complete_layer3_type in conn, not FSM event arg

Instead of jumping through hoops to pass the Complete Layer 3 operation that
created this conn via FSM event dispatch parameters, put it right in the
gsm_subscriber_connection struct, where it always belonged.

Move definition of the enum complete_layer3_type to gsm_data.h, where
gsm_subscriber_connection is defined.

Introduce msc_subscr_conn_update_id() to set the complete_layer3_type of the
conn as soon as a Complete Layer 3 message is received.

In msc_subscr_conn_update_id(), already include an mi_string argument to
prepare for an upcoming patch where the FSM will be allocated much earlier when
the Mobile Identity is not known yet, and we'll also update the fi->id here.

The odd logging change in the msc_vlr_tests output uncovers a wrong use of the
osmo_fsm_inst_dispatch() data argument for SUBSCR_CONN_E_CN_CLOSE events: if a
child FSM signals unsuccessful result, instead of the failure cause, it passed
the complete_layer3_type, as requested upon FSM allocation, which was then
misinterpreted as a failure cause. Now a child FSM failure will pass NULL
instead, while other SUBSCR_CONN_E_CN_CLOSE events may still pass a valid cause
value.

Related: OS#3122
Change-Id: Iae30dd57a8861c4eaaf56999f872d4e635ba97fb

2 files changed, 27 insertions, 31 deletions

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 4624e9039..a90ff172f 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -317,7 +317,6 @@ static const struct value_string lupd_names[] = {
  * Keep this function non-static for direct invocation by unit tests. */
 int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb *msg)
 {
-	static const enum complete_layer3_type conn_from_lu = SUBSCR_CONN_FROM_LU;
 	struct gsm_network *net = conn->network;
 	struct gsm48_hdr *gh = msgb_l3(msg);
 	struct gsm48_loc_upd_req *lu;
@@ -342,6 +341,8 @@ int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb *msg)
 		/* logging already happened in msc_create_conn_fsm() */
 		return rc;
 
+	msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_LU, mi_string);
+
 	conn->classmark.classmark1 = lu->classmark1;
 	conn->classmark.classmark1_set = true;
 
@@ -394,9 +395,7 @@ int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb *msg)
 
 	is_utran = (conn->via_ran == RAN_UTRAN_IU);
 	lu_fsm = vlr_loc_update(conn->fi,
-				SUBSCR_CONN_E_ACCEPTED,
-				SUBSCR_CONN_E_CN_CLOSE,
-				(void*)&conn_from_lu,
+				SUBSCR_CONN_E_ACCEPTED, SUBSCR_CONN_E_CN_CLOSE, NULL,
 				net->vlr, conn, vlr_lu_type, tmsi, imsi,
 				&old_lai, &new_lai,
 				is_utran || conn->network->authentication_required,
@@ -671,6 +670,7 @@ accept_reuse:
 	DEBUGP(DMM, "%s: re-using already accepted connection\n",
 	       vlr_subscr_name(conn->vsub));
 	conn->received_cm_service_request = true;
+	msc_subscr_conn_update_id(conn, conn->complete_layer3_type, mi_string);
 	return conn->network->vlr->ops.tx_cm_serv_acc(conn);
 }
 
@@ -687,8 +687,6 @@ accept_reuse:
  */
 int gsm48_rx_mm_serv_req(struct gsm_subscriber_connection *conn, struct msgb *msg)
 {
-	static const enum complete_layer3_type conn_from_cm_service_req =
-		SUBSCR_CONN_FROM_CM_SERVICE_REQ;
 	struct gsm_network *net = conn->network;
 	uint8_t mi_type;
 	char mi_string[GSM48_MI_SIZE];
@@ -774,12 +772,11 @@ int gsm48_rx_mm_serv_req(struct gsm_subscriber_connection *conn, struct msgb *ms
 		/* logging already happened in msc_create_conn_fsm() */
 		return rc;
 	}
+	msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_CM_SERVICE_REQ, mi_string);
 
 	is_utran = (conn->via_ran == RAN_UTRAN_IU);
 	vlr_proc_acc_req(conn->fi,
-			 SUBSCR_CONN_E_ACCEPTED,
-			 SUBSCR_CONN_E_CN_CLOSE,
-			 (void*)&conn_from_cm_service_req,
+			 SUBSCR_CONN_E_ACCEPTED, SUBSCR_CONN_E_CN_CLOSE, NULL,
 			 net->vlr, conn,
 			 VLR_PR_ARQ_T_CM_SERV_REQ, mi-1, &lai,
 			 is_utran || conn->network->authentication_required,
@@ -1144,8 +1141,6 @@ static uint8_t *gsm48_cm2_get_mi(uint8_t *classmark2_lv, unsigned int tot_len)
 /* Receive a PAGING RESPONSE message from the MS */
 static int gsm48_rx_rr_pag_resp(struct gsm_subscriber_connection *conn, struct msgb *msg)
 {
-	static const enum complete_layer3_type conn_from_paging_resp =
-		SUBSCR_CONN_FROM_PAGING_RESP;
 	struct gsm_network *net = conn->network;
 	struct gsm48_hdr *gh = msgb_l3(msg);
 	struct gsm48_pag_resp *resp;
@@ -1176,15 +1171,14 @@ static int gsm48_rx_rr_pag_resp(struct gsm_subscriber_connection *conn, struct m
 	if (rc)
 		/* logging already happened in msc_create_conn_fsm() */
 		return rc;
+	msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_PAGING_RESP, mi_string);
 
 	memcpy(conn->classmark.classmark2, classmark2_lv+1, *classmark2_lv);
 	conn->classmark.classmark2_len = *classmark2_lv;
 
 	is_utran = (conn->via_ran == RAN_UTRAN_IU);
 	vlr_proc_acc_req(conn->fi,
-			 SUBSCR_CONN_E_ACCEPTED,
-			 SUBSCR_CONN_E_CN_CLOSE,
-			 (void*)&conn_from_paging_resp,
+			 SUBSCR_CONN_E_ACCEPTED, SUBSCR_CONN_E_CN_CLOSE, NULL,
 			 net->vlr, conn,
 			 VLR_PR_ARQ_T_PAGING_RESP, mi_lv, &lai,
 			 is_utran || conn->network->authentication_required,
diff --git a/src/libmsc/subscr_conn.c b/src/libmsc/subscr_conn.c
index 526ad9235..fc89a663f 100644
--- a/src/libmsc/subscr_conn.c
+++ b/src/libmsc/subscr_conn.c
@@ -46,14 +46,6 @@ static const struct value_string subscr_conn_fsm_event_names[] = {
 	{ 0, NULL }
 };
 
-const struct value_string complete_layer3_type_names[] = {
-	OSMO_VALUE_STRING(SUBSCR_CONN_FROM_INVALID),
-	OSMO_VALUE_STRING(SUBSCR_CONN_FROM_LU),
-	OSMO_VALUE_STRING(SUBSCR_CONN_FROM_CM_SERVICE_REQ),
-	OSMO_VALUE_STRING(SUBSCR_CONN_FROM_PAGING_RESP),
-	{ 0, NULL }
-};
-
 static void paging_event(struct gsm_subscriber_connection *conn,
 			 enum gsm_paging_event pe)
 {
@@ -70,14 +62,8 @@ void subscr_conn_fsm_init(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 void subscr_conn_fsm_new(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	struct gsm_subscriber_connection *conn = fi->priv;
-	enum complete_layer3_type from = SUBSCR_CONN_FROM_INVALID;
 	bool success;
 
-	if (data) {
-		from = *(enum complete_layer3_type*)data;
-		LOGPFSM(fi, "%s\n", complete_layer3_type_name(from));
-	}
-
 	/* If accepted, transition the state, all other cases mean failure. */
 	switch (event) {
 	case SUBSCR_CONN_E_ACCEPTED:
@@ -103,13 +89,13 @@ void subscr_conn_fsm_new(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 
 	success = (fi->state == SUBSCR_CONN_S_ACCEPTED);
 
-	if (from == SUBSCR_CONN_FROM_LU)
+	if (conn->complete_layer3_type == COMPLETE_LAYER3_LU)
 		rate_ctr_inc(&conn->network->msc_ctrs->ctr[
 		             	success ? MSC_CTR_LOC_UPDATE_COMPLETED
 					: MSC_CTR_LOC_UPDATE_FAILED]);
 
 	/* signal paging success or failure in case this was a paging */
-	if (from == SUBSCR_CONN_FROM_PAGING_RESP)
+	if (conn->complete_layer3_type == COMPLETE_LAYER3_PAGING_RESP)
 		paging_event(conn,
 			     success ? GSM_PAGING_SUCCEEDED
 			     	     : GSM_PAGING_EXPIRED);
@@ -125,7 +111,7 @@ void subscr_conn_fsm_new(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 		return;
 	}
 
-	if (from == SUBSCR_CONN_FROM_CM_SERVICE_REQ) {
+	if (conn->complete_layer3_type == COMPLETE_LAYER3_CM_SERVICE_REQ) {
 		conn->received_cm_service_request = true;
 		LOGPFSML(fi, LOGL_DEBUG, "received_cm_service_request = true\n");
 	}
@@ -382,3 +368,19 @@ void msc_subscr_conn_init(void)
 {
 	osmo_fsm_register(&subscr_conn_fsm);
 }
+
+const struct value_string complete_layer3_type_names[] = {
+	{ COMPLETE_LAYER3_NONE, "NONE" },
+	{ COMPLETE_LAYER3_LU, "LU" },
+	{ COMPLETE_LAYER3_CM_SERVICE_REQ, "CM_SERVICE_REQ" },
+	{ COMPLETE_LAYER3_PAGING_RESP, "PAGING_RESP" },
+	{ 0, NULL }
+};
+
+void msc_subscr_conn_update_id(struct gsm_subscriber_connection *conn,
+			       enum complete_layer3_type from, const char *id)
+{
+       conn->complete_layer3_type = from;
+       osmo_fsm_inst_update_id(conn->fi, id);
+       LOGPFSML(conn->fi, LOGL_DEBUG, "Updated ID from %s\n", complete_layer3_type_name(from));
+}