cellular-infrastructure
/
osmo-msc
mirror of https://gerrit.osmocom.org/osmo-msc
9
0
Fork 0
Code Issues Releases Wiki Activity

fix regression: fix internal MNCC operation 

While developing the inter-MSC handover refactoring, I was annoyed by the fact
that mncc_tx_to_cc() receives an MNCC message struct containing a msg_type, as
well as a separate msg_type argument, which may deviate from each other. So, as
a first step I wanted to make sure that all callers send identical values for
both by inserting an OSMO_ASSERT(msg_type == msg->msg_type). Later I was going
to remove the separate msg_type argument.

I then forgot to
- carry on to remove the argument and
- to actually test with internal MNCC (it so happens that all of our ttcn3
  tests also use external MNCC).

As a result, the "large refactoring" patch for inter-MSC Handover breaks
internal MNCC operation.

Fix that: remove the separate msg_type argument and make sure that all callers
of mncc_tx_to_cc() indeed pass the desired msg_type in msg->msg_type, and hence
also remove the odd duality of arguments.

Various functions in mncc_builtin.c also exhibit this separate msg_type
argument, which are all unused and make absolutely no sense. Remove those as
well.

Related: OS#3989
Change-Id: I966ce764796982709ea3312e76988a95257acb8d
This commit is contained in:
Neels Hofmeyr 2019-05-09 01:23:09 +02:00
parent e39f6cd752
commit 5255874529
6 changed files with 57 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/osmocom/msc/gsm_04_08.h
View File

@ -45,7 +45,7 @@ int gsm48_send_rr_app_info(struct msc_a *msc_a, uint8_t apdu_id, uint8_t apdu_le
int gsm48_send_rr_ass_cmd(struct gsm_lchan *dest_lchan, struct gsm_lchan *lchan, uint8_t power_class); int gsm48_send_rr_ass_cmd(struct gsm_lchan *dest_lchan, struct gsm_lchan *lchan, uint8_t power_class);
int gsm48_send_ho_cmd(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan, uint8_t power_command, uint8_t ho_ref); int gsm48_send_ho_cmd(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan, uint8_t power_command, uint8_t ho_ref);
int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg); int mncc_tx_to_cc(struct gsm_network *net, void *arg);
/* convert a ASCII phone number to call-control BCD */ /* convert a ASCII phone number to call-control BCD */
int encode_bcd_number(uint8_t *bcd_lv, uint8_t max_len, int encode_bcd_number(uint8_t *bcd_lv, uint8_t max_len,

3
src/libmsc/gsm_04_08_cc.c
View File

@ -2010,11 +2010,10 @@ struct mncc_call *mncc_find_by_callref_from_msg(const union mncc_msg *msg)
} }
/* Demux incoming genuine calls to GSM CC from MNCC forwarding for inter-MSC handover */ /* Demux incoming genuine calls to GSM CC from MNCC forwarding for inter-MSC handover */
int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) int mncc_tx_to_cc(struct gsm_network *net, void *arg)
{ {
const union mncc_msg *msg = arg; const union mncc_msg *msg = arg;
struct mncc_call *mncc_call = NULL; struct mncc_call *mncc_call = NULL;
OSMO_ASSERT(msg_type == msg->msg_type);
if (msg->msg_type == MNCC_SETUP_REQ) { if (msg->msg_type == MNCC_SETUP_REQ) {
/* Incoming call to forward for inter-MSC Handover? */ /* Incoming call to forward for inter-MSC Handover? */

85
src/libmsc/mncc_builtin.c
View File

@ -67,7 +67,7 @@ static struct gsm_call *get_call_ref(uint32_t callref)
} }
/* on incoming call, look up database and send setup to remote subscr. */ /* on incoming call, look up database and send setup to remote subscr. */
static int mncc_setup_ind(struct gsm_call *call, int msg_type, static int mncc_setup_ind(struct gsm_call *call,
struct gsm_mncc *setup) struct gsm_mncc *setup)
{ {
struct gsm_mncc mncc; struct gsm_mncc mncc;
@ -116,29 +116,33 @@ static int mncc_setup_ind(struct gsm_call *call, int msg_type,
/* send call proceeding */ /* send call proceeding */
memset(&mncc, 0, sizeof(struct gsm_mncc)); memset(&mncc, 0, sizeof(struct gsm_mncc));
mncc.callref = call->callref; mncc.callref = call->callref;
mncc.msg_type = MNCC_CALL_PROC_REQ;
DEBUGCC(call, remote, "Accepting call.\n"); DEBUGCC(call, remote, "Accepting call.\n");
mncc_tx_to_cc(call->net, MNCC_CALL_PROC_REQ, &mncc); mncc_tx_to_cc(call->net, &mncc);
/* modify mode */ /* modify mode */
memset(&mncc, 0, sizeof(struct gsm_mncc)); memset(&mncc, 0, sizeof(struct gsm_mncc));
mncc.callref = call->callref; mncc.callref = call->callref;
mncc.msg_type = MNCC_LCHAN_MODIFY;
DEBUGCC(call, remote, "Modify channel mode.\n"); DEBUGCC(call, remote, "Modify channel mode.\n");
mncc_tx_to_cc(call->net, MNCC_LCHAN_MODIFY, &mncc); mncc_tx_to_cc(call->net, &mncc);
/* send setup to remote */ /* send setup to remote */
// setup->fields |= MNCC_F_SIGNAL; // setup->fields |= MNCC_F_SIGNAL;
// setup->signal = GSM48_SIGNAL_DIALTONE; // setup->signal = GSM48_SIGNAL_DIALTONE;
setup->callref = remote->callref; setup->callref = remote->callref;
setup->msg_type = MNCC_SETUP_REQ;
DEBUGCC(call, remote, "Forwarding SETUP to remote.\n"); DEBUGCC(call, remote, "Forwarding SETUP to remote.\n");
return mncc_tx_to_cc(remote->net, MNCC_SETUP_REQ, setup); return mncc_tx_to_cc(remote->net, setup);
out_reject: out_reject:
mncc_tx_to_cc(call->net, MNCC_REJ_REQ, &mncc); mncc.msg_type = MNCC_REJ_REQ;
mncc_tx_to_cc(call->net, &mncc);
free_call(call); free_call(call);
return 0; return 0;
} }
static int mncc_alert_ind(struct gsm_call *call, int msg_type, static int mncc_alert_ind(struct gsm_call *call,
struct gsm_mncc *alert) struct gsm_mncc *alert)
{ {
struct gsm_call *remote; struct gsm_call *remote;
@ -147,11 +151,12 @@ static int mncc_alert_ind(struct gsm_call *call, int msg_type,
if (!(remote = get_call_ref(call->remote_ref))) if (!(remote = get_call_ref(call->remote_ref)))
return 0; return 0;
alert->callref = remote->callref; alert->callref = remote->callref;
alert->msg_type = MNCC_ALERT_REQ;
DEBUGCC(call, remote, "Forwarding ALERT to remote.\n"); DEBUGCC(call, remote, "Forwarding ALERT to remote.\n");
return mncc_tx_to_cc(remote->net, MNCC_ALERT_REQ, alert); return mncc_tx_to_cc(remote->net, alert);
} }
static int mncc_notify_ind(struct gsm_call *call, int msg_type, static int mncc_notify_ind(struct gsm_call *call,
struct gsm_mncc *notify) struct gsm_mncc *notify)
{ {
struct gsm_call *remote; struct gsm_call *remote;
@ -160,11 +165,12 @@ static int mncc_notify_ind(struct gsm_call *call, int msg_type,
if (!(remote = get_call_ref(call->remote_ref))) if (!(remote = get_call_ref(call->remote_ref)))
return 0; return 0;
notify->callref = remote->callref; notify->callref = remote->callref;
notify->msg_type = MNCC_NOTIFY_REQ;
DEBUGCC(call, remote, "Forwarding NOTIF to remote.\n"); DEBUGCC(call, remote, "Forwarding NOTIF to remote.\n");
return mncc_tx_to_cc(remote->net, MNCC_NOTIFY_REQ, notify); return mncc_tx_to_cc(remote->net, notify);
} }
static int mncc_setup_cnf(struct gsm_call *call, int msg_type, static int mncc_setup_cnf(struct gsm_call *call,
struct gsm_mncc *connect) struct gsm_mncc *connect)
{ {
struct gsm_mncc connect_ack; struct gsm_mncc connect_ack;
@ -173,26 +179,29 @@ static int mncc_setup_cnf(struct gsm_call *call, int msg_type,
/* acknowledge connect */ /* acknowledge connect */
memset(&connect_ack, 0, sizeof(struct gsm_mncc)); memset(&connect_ack, 0, sizeof(struct gsm_mncc));
connect_ack.msg_type = MNCC_SETUP_COMPL_REQ;
connect_ack.callref = call->callref; connect_ack.callref = call->callref;
DEBUGP(DMNCC, "(call %x) Acknowledge SETUP.\n", call->callref); DEBUGP(DMNCC, "(call %x) Acknowledge SETUP.\n", call->callref);
mncc_tx_to_cc(call->net, MNCC_SETUP_COMPL_REQ, &connect_ack); mncc_tx_to_cc(call->net, &connect_ack);
/* send connect message to remote */ /* send connect message to remote */
if (!(remote = get_call_ref(call->remote_ref))) if (!(remote = get_call_ref(call->remote_ref)))
return 0; return 0;
connect->msg_type = MNCC_SETUP_RSP;
connect->callref = remote->callref; connect->callref = remote->callref;
DEBUGCC(call, remote, "Sending CONNECT to remote.\n"); DEBUGCC(call, remote, "Sending CONNECT to remote.\n");
mncc_tx_to_cc(remote->net, MNCC_SETUP_RSP, connect); mncc_tx_to_cc(remote->net, connect);
/* bridge tch */ /* bridge tch */
bridge.msg_type = MNCC_BRIDGE;
bridge.callref[0] = call->callref; bridge.callref[0] = call->callref;
bridge.callref[1] = call->remote_ref; bridge.callref[1] = call->remote_ref;
DEBUGCC(call, remote, "Bridging with remote.\n"); DEBUGCC(call, remote, "Bridging with remote.\n");
return mncc_tx_to_cc(call->net, MNCC_BRIDGE, &bridge); return mncc_tx_to_cc(call->net, &bridge);
} }
static int mncc_disc_ind(struct gsm_call *call, int msg_type, static int mncc_disc_ind(struct gsm_call *call,
struct gsm_mncc *disc) struct gsm_mncc *disc)
{ {
struct gsm_call *remote; struct gsm_call *remote;
@ -200,18 +209,20 @@ static int mncc_disc_ind(struct gsm_call *call, int msg_type,
/* send release */ /* send release */
DEBUGP(DMNCC, "(call %x) Releasing call with cause %d\n", DEBUGP(DMNCC, "(call %x) Releasing call with cause %d\n",
call->callref, disc->cause.value); call->callref, disc->cause.value);
mncc_tx_to_cc(call->net, MNCC_REL_REQ, disc); disc->msg_type = MNCC_REL_REQ;
mncc_tx_to_cc(call->net, disc);
/* send disc to remote */ /* send disc to remote */
if (!(remote = get_call_ref(call->remote_ref))) { if (!(remote = get_call_ref(call->remote_ref))) {
return 0; return 0;
} }
disc->msg_type = MNCC_DISC_REQ;
disc->callref = remote->callref; disc->callref = remote->callref;
DEBUGCC(call, remote, "Disconnecting remote with cause %d\n", disc->cause.value); DEBUGCC(call, remote, "Disconnecting remote with cause %d\n", disc->cause.value);
return mncc_tx_to_cc(remote->net, MNCC_DISC_REQ, disc); return mncc_tx_to_cc(remote->net, disc);
} }
static int mncc_rel_ind(struct gsm_call *call, int msg_type, struct gsm_mncc *rel) static int mncc_rel_ind(struct gsm_call *call, struct gsm_mncc *rel)
{ {
struct gsm_call *remote; struct gsm_call *remote;
@ -221,6 +232,7 @@ static int mncc_rel_ind(struct gsm_call *call, int msg_type, struct gsm_mncc *re
return 0; return 0;
} }
rel->msg_type = MNCC_REL_REQ;
rel->callref = remote->callref; rel->callref = remote->callref;
DEBUGCC(call, remote, "Releasing remote with cause %d\n", rel->cause.value); DEBUGCC(call, remote, "Releasing remote with cause %d\n", rel->cause.value);
@ -230,12 +242,12 @@ static int mncc_rel_ind(struct gsm_call *call, int msg_type, struct gsm_mncc *re
* it and then we will end up with a double free and a crash * it and then we will end up with a double free and a crash
*/ */
free_call(call); free_call(call);
mncc_tx_to_cc(remote->net, MNCC_REL_REQ, rel); mncc_tx_to_cc(remote->net, rel);
return 0; return 0;
} }
static int mncc_rel_cnf(struct gsm_call *call, int msg_type, struct gsm_mncc *rel) static int mncc_rel_cnf(struct gsm_call *call, struct gsm_mncc *rel)
{ {
free_call(call); free_call(call);
return 0; return 0;
@ -273,10 +285,11 @@ int int_mncc_recv(struct gsm_network *net, struct msgb *msg)
struct gsm_mncc rel; struct gsm_mncc rel;
memset(&rel, 0, sizeof(struct gsm_mncc)); memset(&rel, 0, sizeof(struct gsm_mncc));
rel.msg_type = MNCC_REL_REQ;
rel.callref = callref; rel.callref = callref;
mncc_set_cause(&rel, GSM48_CAUSE_LOC_PRN_S_LU, mncc_set_cause(&rel, GSM48_CAUSE_LOC_PRN_S_LU,
GSM48_CC_CAUSE_RESOURCE_UNAVAIL); GSM48_CC_CAUSE_RESOURCE_UNAVAIL);
mncc_tx_to_cc(net, MNCC_REL_REQ, &rel); mncc_tx_to_cc(net, &rel);
goto out_free; goto out_free;
} }
llist_add_tail(&call->entry, &call_list); llist_add_tail(&call->entry, &call_list);
@ -296,47 +309,51 @@ int int_mncc_recv(struct gsm_network *net, struct msgb *msg)
switch(msg_type) { switch(msg_type) {
case MNCC_SETUP_IND: case MNCC_SETUP_IND:
rc = mncc_setup_ind(call, msg_type, arg); rc = mncc_setup_ind(call, arg);
break; break;
case MNCC_SETUP_CNF: case MNCC_SETUP_CNF:
rc = mncc_setup_cnf(call, msg_type, arg); rc = mncc_setup_cnf(call, arg);
break; break;
case MNCC_SETUP_COMPL_IND: case MNCC_SETUP_COMPL_IND:
break; break;
case MNCC_CALL_CONF_IND: case MNCC_CALL_CONF_IND:
/* we now need to MODIFY the channel */ /* we now need to MODIFY the channel */
mncc_tx_to_cc(call->net, MNCC_LCHAN_MODIFY, data); data->msg_type = MNCC_LCHAN_MODIFY;
mncc_tx_to_cc(call->net, data);
break; break;
case MNCC_ALERT_IND: case MNCC_ALERT_IND:
rc = mncc_alert_ind(call, msg_type, arg); rc = mncc_alert_ind(call, arg);
break; break;
case MNCC_NOTIFY_IND: case MNCC_NOTIFY_IND:
rc = mncc_notify_ind(call, msg_type, arg); rc = mncc_notify_ind(call, arg);
break; break;
case MNCC_DISC_IND: case MNCC_DISC_IND:
rc = mncc_disc_ind(call, msg_type, arg); rc = mncc_disc_ind(call, arg);
break; break;
case MNCC_REL_IND: case MNCC_REL_IND:
case MNCC_REJ_IND: case MNCC_REJ_IND:
rc = mncc_rel_ind(call, msg_type, arg); rc = mncc_rel_ind(call, arg);
break; break;
case MNCC_REL_CNF: case MNCC_REL_CNF:
rc = mncc_rel_cnf(call, msg_type, arg); rc = mncc_rel_cnf(call, arg);
break; break;
case MNCC_FACILITY_IND: case MNCC_FACILITY_IND:
break; break;
case MNCC_START_DTMF_IND: case MNCC_START_DTMF_IND:
rc = mncc_tx_to_cc(net, MNCC_START_DTMF_REJ, data); data->msg_type = MNCC_START_DTMF_REJ;
rc = mncc_tx_to_cc(net, data);
break; break;
case MNCC_STOP_DTMF_IND: case MNCC_STOP_DTMF_IND:
rc = mncc_tx_to_cc(net, MNCC_STOP_DTMF_RSP, data); data->msg_type = MNCC_STOP_DTMF_RSP;
rc = mncc_tx_to_cc(net, data);
break; break;
case MNCC_MODIFY_IND: case MNCC_MODIFY_IND:
mncc_set_cause(data, GSM48_CAUSE_LOC_PRN_S_LU, mncc_set_cause(data, GSM48_CAUSE_LOC_PRN_S_LU,
GSM48_CC_CAUSE_SERV_OPT_UNIMPL); GSM48_CC_CAUSE_SERV_OPT_UNIMPL);
DEBUGP(DMNCC, "(call %x) Rejecting MODIFY with cause %d\n", DEBUGP(DMNCC, "(call %x) Rejecting MODIFY with cause %d\n",
call->callref, data->cause.value); call->callref, data->cause.value);
rc = mncc_tx_to_cc(net, MNCC_MODIFY_REJ, data); data->msg_type = MNCC_MODIFY_REJ;
rc = mncc_tx_to_cc(net, data);
break; break;
case MNCC_MODIFY_CNF: case MNCC_MODIFY_CNF:
break; break;
@ -345,14 +362,16 @@ int int_mncc_recv(struct gsm_network *net, struct msgb *msg)
GSM48_CC_CAUSE_SERV_OPT_UNIMPL); GSM48_CC_CAUSE_SERV_OPT_UNIMPL);
DEBUGP(DMNCC, "(call %x) Rejecting HOLD with cause %d\n", DEBUGP(DMNCC, "(call %x) Rejecting HOLD with cause %d\n",
call->callref, data->cause.value); call->callref, data->cause.value);
rc = mncc_tx_to_cc(net, MNCC_HOLD_REJ, data); data->msg_type = MNCC_HOLD_REJ;
rc = mncc_tx_to_cc(net, data);
break; break;
case MNCC_RETRIEVE_IND: case MNCC_RETRIEVE_IND:
mncc_set_cause(data, GSM48_CAUSE_LOC_PRN_S_LU, mncc_set_cause(data, GSM48_CAUSE_LOC_PRN_S_LU,
GSM48_CC_CAUSE_SERV_OPT_UNIMPL); GSM48_CC_CAUSE_SERV_OPT_UNIMPL);
DEBUGP(DMNCC, "(call %x) Rejecting RETRIEVE with cause %d\n", DEBUGP(DMNCC, "(call %x) Rejecting RETRIEVE with cause %d\n",
call->callref, data->cause.value); call->callref, data->cause.value);
rc = mncc_tx_to_cc(net, MNCC_RETRIEVE_REJ, data); data->msg_type = MNCC_RETRIEVE_REJ;
rc = mncc_tx_to_cc(net, data);
break; break;
default: default:
LOGP(DMNCC, LOGL_NOTICE, "(call %x) Message unhandled\n", callref); LOGP(DMNCC, LOGL_NOTICE, "(call %x) Message unhandled\n", callref);

2
src/libmsc/mncc_sock.c
View File

@ -118,7 +118,7 @@ static int mncc_sock_read(struct osmo_fd *bfd)
rc = mncc_prim_check(mncc_prim, rc); rc = mncc_prim_check(mncc_prim, rc);
if (rc == 0) if (rc == 0)
rc = mncc_tx_to_cc(state->net, mncc_prim->msg_type, mncc_prim); rc = mncc_tx_to_cc(state->net, mncc_prim);
/* as we always synchronously process the message in mncc_send() and /* as we always synchronously process the message in mncc_send() and
* its callbacks, we can free the message here. */ * its callbacks, we can free the message here. */

2
tests/msc_vlr/msc_vlr_test_call.c
View File

@ -29,7 +29,7 @@
static void mncc_sends_to_cc(uint32_t msg_type, struct gsm_mncc *mncc) static void mncc_sends_to_cc(uint32_t msg_type, struct gsm_mncc *mncc)
{ {
mncc->msg_type = msg_type; mncc->msg_type = msg_type;
mncc_tx_to_cc(net, msg_type, mncc); mncc_tx_to_cc(net, mncc);
} }
/* /*

2
tests/msc_vlr/msc_vlr_tests.c
View File

@ -680,7 +680,7 @@ int mncc_recv(struct gsm_network *net, struct msgb *msg)
log("MNCC: callref 0x%x: Call Release triggering %s", mncc->callref, log("MNCC: callref 0x%x: Call Release triggering %s", mncc->callref,
get_mncc_name(on_call_release_mncc_sends_to_cc_data->msg_type)); get_mncc_name(on_call_release_mncc_sends_to_cc_data->msg_type));
mncc_tx_to_cc(net, on_call_release_mncc_sends_to_cc_data->msg_type, mncc_tx_to_cc(net,
on_call_release_mncc_sends_to_cc_data); on_call_release_mncc_sends_to_cc_data);
on_call_release_mncc_sends_to_cc_data = NULL; on_call_release_mncc_sends_to_cc_data = NULL;