From 16c42b5fbacb93867f41f529893b6af8a1a54c5b Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 2 Apr 2018 12:26:16 +0200 Subject: 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 --- src/libmsc/gsm_04_08.c | 22 ++++++++-------------- src/libmsc/subscr_conn.c | 36 +++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 31 deletions(-) (limited to 'src') 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)); +} -- cgit v1.2.3