--- openbsc/include/openbsc/mncc.h | 5 +- openbsc/src/libmsc/mncc.c | 103 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 100 insertions(+), 8 deletions(-)
diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index 46ba237..92389c2 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -191,8 +191,11 @@ struct gsm_mncc_rtp { uint32_t payload_msg_type; };
-char *get_mncc_name(int value); +const char *get_mncc_name(int value); void mncc_set_cause(struct gsm_mncc *data, int loc, int val); +int mncc_has_cause(struct gsm_mncc *data); +const char *get_mncc_location(int value); +const char *get_mncc_cause(int value); void cc_tx_to_mncc(struct gsm_network *net, struct msgb *msg);
/* input from CC code into mncc_builtin */ diff --git a/openbsc/src/libmsc/mncc.c b/openbsc/src/libmsc/mncc.c index 2767faa..e8bd9ba 100644 --- a/openbsc/src/libmsc/mncc.c +++ b/openbsc/src/libmsc/mncc.c @@ -34,10 +34,12 @@ #include <openbsc/transaction.h> #include <openbsc/rtp_proxy.h>
-static struct mncc_names { +struct name_value { char *name; int value; -} mncc_names[] = { +}; + +static struct name_value mncc_names[] = { {"MNCC_SETUP_REQ", 0x0101}, {"MNCC_SETUP_IND", 0x0102}, {"MNCC_SETUP_RSP", 0x0103}, @@ -95,16 +97,103 @@ static struct mncc_names {
{NULL, 0} };
-char *get_mncc_name(int value) +static struct name_value mncc_locations[] = { + {"GSM48_CAUSE_LOC_USER", 0x00}, + {"GSM48_CAUSE_LOC_PRN_S_LU", 0x01}, + {"GSM48_CAUSE_LOC_PUN_S_LU", 0x02}, + {"GSM48_CAUSE_LOC_TRANS_NET", 0x03}, + {"GSM48_CAUSE_LOC_PUN_S_RU", 0x04}, + {"GSM48_CAUSE_LOC_PRN_S_RU", 0x05}, + /* not defined */ + {"GSM48_CAUSE_LOC_INN_NET", 0x07}, + {"GSM48_CAUSE_LOC_NET_BEYOND", 0x0a}, + + {NULL, 0} }; + +static struct name_value mncc_causes[] = { + {"GSM48_CC_CAUSE_UNASSIGNED_NR", 1}, + {"GSM48_CC_CAUSE_NO_ROUTE", 3}, + {"GSM48_CC_CAUSE_CHAN_UNACCEPT", 6}, + {"GSM48_CC_CAUSE_OP_DET_BARRING", 8}, + {"GSM48_CC_CAUSE_NORM_CALL_CLEAR", 16}, + {"GSM48_CC_CAUSE_USER_BUSY", 17}, + {"GSM48_CC_CAUSE_USER_NOTRESPOND", 18}, + {"GSM48_CC_CAUSE_USER_ALERTING_NA", 19}, + {"GSM48_CC_CAUSE_CALL_REJECTED", 21}, + {"GSM48_CC_CAUSE_NUMBER_CHANGED", 22}, + {"GSM48_CC_CAUSE_PRE_EMPTION", 25}, + {"GSM48_CC_CAUSE_NONSE_USER_CLR", 26}, + {"GSM48_CC_CAUSE_DEST_OOO", 27}, + {"GSM48_CC_CAUSE_INV_NR_FORMAT", 28}, + {"GSM48_CC_CAUSE_FACILITY_REJ", 29}, + {"GSM48_CC_CAUSE_RESP_STATUS_INQ", 30}, + {"GSM48_CC_CAUSE_NORMAL_UNSPEC", 31}, + {"GSM48_CC_CAUSE_NO_CIRCUIT_CHAN", 34}, + {"GSM48_CC_CAUSE_NETWORK_OOO", 38}, + {"GSM48_CC_CAUSE_TEMP_FAILURE", 41}, + {"GSM48_CC_CAUSE_SWITCH_CONG", 42}, + {"GSM48_CC_CAUSE_ACC_INF_DISCARD", 43}, + {"GSM48_CC_CAUSE_REQ_CHAN_UNAVAIL", 44}, + {"GSM48_CC_CAUSE_RESOURCE_UNAVAIL", 47}, + {"GSM48_CC_CAUSE_QOS_UNAVAIL", 49}, + {"GSM48_CC_CAUSE_REQ_FAC_NOT_SUBSC", 50}, + {"GSM48_CC_CAUSE_INC_BARRED_CUG", 55}, + {"GSM48_CC_CAUSE_BEARER_CAP_UNAUTH", 57}, + {"GSM48_CC_CAUSE_BEARER_CA_UNAVAIL", 58}, + {"GSM48_CC_CAUSE_SERV_OPT_UNAVAIL", 63}, + {"GSM48_CC_CAUSE_BEARERSERV_UNIMPL", 65}, + {"GSM48_CC_CAUSE_ACM_GE_ACM_MAX", 68}, + {"GSM48_CC_CAUSE_REQ_FAC_NOTIMPL", 69}, + {"GSM48_CC_CAUSE_RESTR_BCAP_AVAIL", 70}, + {"GSM48_CC_CAUSE_SERV_OPT_UNIMPL", 79}, + {"GSM48_CC_CAUSE_INVAL_TRANS_ID", 81}, + {"GSM48_CC_CAUSE_USER_NOT_IN_CUG", 87}, + {"GSM48_CC_CAUSE_INCOMPAT_DEST", 88}, + {"GSM48_CC_CAUSE_INVAL_TRANS_NET", 91}, + {"GSM48_CC_CAUSE_SEMANTIC_INCORR", 95}, + {"GSM48_CC_CAUSE_INVAL_MAND_INF", 96}, + {"GSM48_CC_CAUSE_MSGTYPE_NOTEXIST", 97}, + {"GSM48_CC_CAUSE_MSGTYPE_INCOMPAT", 98}, + {"GSM48_CC_CAUSE_IE_NOTEXIST", 99}, + {"GSM48_CC_CAUSE_COND_IE_ERR", 100}, + {"GSM48_CC_CAUSE_MSG_INCOMP_STATE", 101}, + {"GSM48_CC_CAUSE_RECOVERY_TIMER", 102}, + {"GSM48_CC_CAUSE_PROTO_ERR", 111}, + {"GSM48_CC_CAUSE_INTERWORKING", 127}, + + {NULL, 0} }; + +const char *get_name_by_value(struct name_value *pairs, int value, const char *default_val) { int i;
- for (i = 0; mncc_names[i].name; i++) { - if (mncc_names[i].value == value) - return mncc_names[i].name; + for (i = 0; pairs[i].name; i++) { + if (pairs[i].value == value) + return pairs[i].name; }
- return "MNCC_Unknown"; + return default_val; +} + + +const char *get_mncc_name(int value) +{ + return get_name_by_value(mncc_names, value, "MNCC_Unknown"); +} + +const char *get_mncc_location(int value) +{ + return get_name_by_value(mncc_locations, value, "GSM48_CAUSE_LOC_Unknown"); +} + +const char *get_mncc_cause(int value) +{ + return get_name_by_value(mncc_causes, value, "GSM48_CC_CAUSE_Unknown"); +} + +int mncc_has_cause(struct gsm_mncc *data) +{ + return data->fields & MNCC_F_CAUSE; }
void mncc_set_cause(struct gsm_mncc *data, int loc, int val)
--- openbsc/src/libmsc/gsm_04_08.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 7db7586..5dd9247 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1349,20 +1349,26 @@ static int mncc_recvmsg(struct gsm_network *net, struct gsm_trans *trans, if (trans) if (trans->conn && trans->conn->lchan) DEBUGP(DCC, "(bts %d trx %d ts %d ti %x sub %s) " - "Sending '%s' to MNCC.\n", + "Sending '%s' cause %s/%s to MNCC.\n", trans->conn->lchan->ts->trx->bts->nr, trans->conn->lchan->ts->trx->nr, trans->conn->lchan->ts->nr, trans->transaction_id, (trans->subscr)?(trans->subscr->extension):"-", - get_mncc_name(msg_type)); + get_mncc_name(msg_type), + (mncc_has_cause(mncc))?get_mncc_location(mncc->cause.location):"-", + (mncc_has_cause(mncc))?get_mncc_cause(mncc->cause.value):"-"); else DEBUGP(DCC, "(bts - trx - ts - ti -- sub %s) " - "Sending '%s' to MNCC.\n", + "Sending '%s' cause %s/%s to MNCC.\n", (trans->subscr)?(trans->subscr->extension):"-", - get_mncc_name(msg_type)); + get_mncc_name(msg_type), + (mncc_has_cause(mncc))?get_mncc_location(mncc->cause.location):"-", + (mncc_has_cause(mncc))?get_mncc_cause(mncc->cause.value):"-"); else DEBUGP(DCC, "(bts - trx - ts - ti -- sub -) " - "Sending '%s' to MNCC.\n", get_mncc_name(msg_type)); + "Sending '%s' cause %s/%s to MNCC.\n", get_mncc_name(msg_type), + (mncc_has_cause(mncc))?get_mncc_location(mncc->cause.location):"-", + (mncc_has_cause(mncc))?get_mncc_cause(mncc->cause.value):"-");
mncc->msg_type = msg_type;
Previously we were sending a generic "Resource unavailable" cause code making it impossible to distinguish real error cases from a regular radio link failure. This was the reason for many "unknown" call errors we've seen at Rhizomatica installations. Now they are properly classified as non-erroneous call failures. --- openbsc/include/openbsc/transaction.h | 1 + openbsc/src/libbsc/bsc_api.c | 2 +- openbsc/src/libmsc/gsm_04_08.c | 14 ++++++++++---- openbsc/src/libmsc/transaction.c | 9 +++++++-- 4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/openbsc/include/openbsc/transaction.h b/openbsc/include/openbsc/transaction.h index 6ef1612..303ef6e 100644 --- a/openbsc/include/openbsc/transaction.h +++ b/openbsc/include/openbsc/transaction.h @@ -70,6 +70,7 @@ struct gsm_trans *trans_alloc(struct gsm_network *net, struct gsm_subscriber *subscr, uint8_t protocol, uint8_t trans_id, uint32_t callref); +void trans_free_cause(struct gsm_trans *trans, int gsm0808_cause); void trans_free(struct gsm_trans *trans);
int trans_assign_trans_id(struct gsm_network *net, struct gsm_subscriber *subscr, diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c index 504f044..54083f3 100644 --- a/openbsc/src/libbsc/bsc_api.c +++ b/openbsc/src/libbsc/bsc_api.c @@ -830,7 +830,7 @@ static void handle_release(struct gsm_subscriber_connection *conn,
/* clear the connection now */ if (bsc->clear_request) - destruct = bsc->clear_request(conn, 0); + destruct = bsc->clear_request(conn, GSM0808_CAUSE_RADIO_INTERFACE_FAILURE);
/* now give up all channels */ if (conn->lchan == lchan) diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 5dd9247..94d186d 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -58,6 +58,7 @@
#include <osmocom/gsm/gsm48.h> #include <osmocom/gsm/gsm0480.h> +#include <osmocom/gsm/protocol/gsm_08_08.h> #include <osmocom/gsm/gsm_utils.h> #include <osmocom/core/msgb.h> #include <osmocom/core/talloc.h> @@ -410,7 +411,7 @@ void gsm0408_clear_request(struct gsm_subscriber_connection *conn, uint32_t caus restart: llist_for_each_entry_safe(trans, temp, &conn->bts->network->trans_list, entry) { if (trans->conn == conn) { - trans_free(trans); + trans_free_cause(trans, cause); goto restart; } } @@ -1399,16 +1400,21 @@ int mncc_release_ind(struct gsm_network *net, struct gsm_trans *trans,
/* Call Control Specific transaction release. * gets called by trans_free, DO NOT CALL YOURSELF! */ -void _gsm48_cc_trans_free(struct gsm_trans *trans) +void _gsm48_cc_trans_free(struct gsm_trans *trans, int gsm0808_cause) { gsm48_stop_cc_timer(trans);
/* send release to L4, if callref still exists */ if (trans->callref) { - /* Ressource unavailable */ + /* Release CC connection */ + int cc_cause = GSM48_CC_CAUSE_RESOURCE_UNAVAIL; + if (gsm0808_cause == GSM0808_CAUSE_RADIO_INTERFACE_FAILURE) + { + cc_cause = GSM48_CC_CAUSE_DEST_OOO; + } mncc_release_ind(trans->net, trans, trans->callref, GSM48_CAUSE_LOC_PRN_S_LU, - GSM48_CC_CAUSE_RESOURCE_UNAVAIL); + cc_cause); } if (trans->cc.state != GSM_CSTATE_NULL) new_cc_state(trans, GSM_CSTATE_NULL); diff --git a/openbsc/src/libmsc/transaction.c b/openbsc/src/libmsc/transaction.c index a750362..95939d3 100644 --- a/openbsc/src/libmsc/transaction.c +++ b/openbsc/src/libmsc/transaction.c @@ -31,7 +31,7 @@
void *tall_trans_ctx;
-void _gsm48_cc_trans_free(struct gsm_trans *trans); +void _gsm48_cc_trans_free(struct gsm_trans *trans, int gsm0808_cause);
struct gsm_trans *trans_find_by_id(struct gsm_subscriber_connection *conn, uint8_t proto, uint8_t trans_id) @@ -89,9 +89,14 @@ struct gsm_trans *trans_alloc(struct gsm_network *net,
void trans_free(struct gsm_trans *trans) { + trans_free_cause(trans, -1); +} + +void trans_free_cause(struct gsm_trans *trans, int gsm0808_cause) +{ switch (trans->protocol) { case GSM48_PDISC_CC: - _gsm48_cc_trans_free(trans); + _gsm48_cc_trans_free(trans, gsm0808_cause); break; case GSM48_PDISC_SMS: _gsm411_sms_trans_free(trans);
bump. Still hope to get this merged.
On Wed, Nov 25, 2015 at 3:31 PM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
openbsc/include/openbsc/mncc.h | 5 +- openbsc/src/libmsc/mncc.c | 103 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 100 insertions(+), 8 deletions(-)
diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index 46ba237..92389c2 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -191,8 +191,11 @@ struct gsm_mncc_rtp { uint32_t payload_msg_type; };
-char *get_mncc_name(int value); +const char *get_mncc_name(int value); void mncc_set_cause(struct gsm_mncc *data, int loc, int val); +int mncc_has_cause(struct gsm_mncc *data); +const char *get_mncc_location(int value); +const char *get_mncc_cause(int value); void cc_tx_to_mncc(struct gsm_network *net, struct msgb *msg);
/* input from CC code into mncc_builtin */ diff --git a/openbsc/src/libmsc/mncc.c b/openbsc/src/libmsc/mncc.c index 2767faa..e8bd9ba 100644 --- a/openbsc/src/libmsc/mncc.c +++ b/openbsc/src/libmsc/mncc.c @@ -34,10 +34,12 @@ #include <openbsc/transaction.h> #include <openbsc/rtp_proxy.h>
-static struct mncc_names { +struct name_value { char *name; int value; -} mncc_names[] = { +};
+static struct name_value mncc_names[] = { {"MNCC_SETUP_REQ", 0x0101}, {"MNCC_SETUP_IND", 0x0102}, {"MNCC_SETUP_RSP", 0x0103}, @@ -95,16 +97,103 @@ static struct mncc_names {
{NULL, 0} };-char *get_mncc_name(int value) +static struct name_value mncc_locations[] = {
{"GSM48_CAUSE_LOC_USER", 0x00},{"GSM48_CAUSE_LOC_PRN_S_LU", 0x01},{"GSM48_CAUSE_LOC_PUN_S_LU", 0x02},{"GSM48_CAUSE_LOC_TRANS_NET", 0x03},{"GSM48_CAUSE_LOC_PUN_S_RU", 0x04},{"GSM48_CAUSE_LOC_PRN_S_RU", 0x05},/* not defined */{"GSM48_CAUSE_LOC_INN_NET", 0x07},{"GSM48_CAUSE_LOC_NET_BEYOND", 0x0a},{NULL, 0} };+static struct name_value mncc_causes[] = {
{"GSM48_CC_CAUSE_UNASSIGNED_NR", 1},{"GSM48_CC_CAUSE_NO_ROUTE", 3},{"GSM48_CC_CAUSE_CHAN_UNACCEPT", 6},{"GSM48_CC_CAUSE_OP_DET_BARRING", 8},{"GSM48_CC_CAUSE_NORM_CALL_CLEAR", 16},{"GSM48_CC_CAUSE_USER_BUSY", 17},{"GSM48_CC_CAUSE_USER_NOTRESPOND", 18},{"GSM48_CC_CAUSE_USER_ALERTING_NA", 19},{"GSM48_CC_CAUSE_CALL_REJECTED", 21},{"GSM48_CC_CAUSE_NUMBER_CHANGED", 22},{"GSM48_CC_CAUSE_PRE_EMPTION", 25},{"GSM48_CC_CAUSE_NONSE_USER_CLR", 26},{"GSM48_CC_CAUSE_DEST_OOO", 27},{"GSM48_CC_CAUSE_INV_NR_FORMAT", 28},{"GSM48_CC_CAUSE_FACILITY_REJ", 29},{"GSM48_CC_CAUSE_RESP_STATUS_INQ", 30},{"GSM48_CC_CAUSE_NORMAL_UNSPEC", 31},{"GSM48_CC_CAUSE_NO_CIRCUIT_CHAN", 34},{"GSM48_CC_CAUSE_NETWORK_OOO", 38},{"GSM48_CC_CAUSE_TEMP_FAILURE", 41},{"GSM48_CC_CAUSE_SWITCH_CONG", 42},{"GSM48_CC_CAUSE_ACC_INF_DISCARD", 43},{"GSM48_CC_CAUSE_REQ_CHAN_UNAVAIL", 44},{"GSM48_CC_CAUSE_RESOURCE_UNAVAIL", 47},{"GSM48_CC_CAUSE_QOS_UNAVAIL", 49},{"GSM48_CC_CAUSE_REQ_FAC_NOT_SUBSC", 50},{"GSM48_CC_CAUSE_INC_BARRED_CUG", 55},{"GSM48_CC_CAUSE_BEARER_CAP_UNAUTH", 57},{"GSM48_CC_CAUSE_BEARER_CA_UNAVAIL", 58},{"GSM48_CC_CAUSE_SERV_OPT_UNAVAIL", 63},{"GSM48_CC_CAUSE_BEARERSERV_UNIMPL", 65},{"GSM48_CC_CAUSE_ACM_GE_ACM_MAX", 68},{"GSM48_CC_CAUSE_REQ_FAC_NOTIMPL", 69},{"GSM48_CC_CAUSE_RESTR_BCAP_AVAIL", 70},{"GSM48_CC_CAUSE_SERV_OPT_UNIMPL", 79},{"GSM48_CC_CAUSE_INVAL_TRANS_ID", 81},{"GSM48_CC_CAUSE_USER_NOT_IN_CUG", 87},{"GSM48_CC_CAUSE_INCOMPAT_DEST", 88},{"GSM48_CC_CAUSE_INVAL_TRANS_NET", 91},{"GSM48_CC_CAUSE_SEMANTIC_INCORR", 95},{"GSM48_CC_CAUSE_INVAL_MAND_INF", 96},{"GSM48_CC_CAUSE_MSGTYPE_NOTEXIST", 97},{"GSM48_CC_CAUSE_MSGTYPE_INCOMPAT", 98},{"GSM48_CC_CAUSE_IE_NOTEXIST", 99},{"GSM48_CC_CAUSE_COND_IE_ERR", 100},{"GSM48_CC_CAUSE_MSG_INCOMP_STATE", 101},{"GSM48_CC_CAUSE_RECOVERY_TIMER", 102},{"GSM48_CC_CAUSE_PROTO_ERR", 111},{"GSM48_CC_CAUSE_INTERWORKING", 127},{NULL, 0} };+const char *get_name_by_value(struct name_value *pairs, int value, const char *default_val) { int i;
for (i = 0; mncc_names[i].name; i++) {if (mncc_names[i].value == value)return mncc_names[i].name;
for (i = 0; pairs[i].name; i++) {if (pairs[i].value == value)return pairs[i].name; }
return "MNCC_Unknown";
return default_val;+}
+const char *get_mncc_name(int value) +{
return get_name_by_value(mncc_names, value, "MNCC_Unknown");+}
+const char *get_mncc_location(int value) +{
return get_name_by_value(mncc_locations, value, "GSM48_CAUSE_LOC_Unknown");+}
+const char *get_mncc_cause(int value) +{
return get_name_by_value(mncc_causes, value, "GSM48_CC_CAUSE_Unknown");+}
+int mncc_has_cause(struct gsm_mncc *data) +{
return data->fields & MNCC_F_CAUSE;}
void mncc_set_cause(struct gsm_mncc *data, int loc, int val)
1.9.1
On 12 Dec 2015, at 15:49, Alexander Chemeris alexander.chemeris@gmail.com wrote:
bump. Still hope to get this merged.
on my list. but will probably be after christmas. Do you have an idea why patchwork is not picking up your patches?
holger
On Sat, Dec 12, 2015 at 11:16 AM, Holger Freyther holger@freyther.de wrote:
On 12 Dec 2015, at 15:49, Alexander Chemeris alexander.chemeris@gmail.com wrote:
bump. Still hope to get this merged.
on my list. but will probably be after christmas.
Thanks! No pressure, I just wanted to make sure it's not lost again.
Do you have an idea why patchwork is not picking up your patches?
Not really :\ It seems like we're not the only ones affected, but there is no solution offered: https://lists.ozlabs.org/pipermail/patchwork/2015-October/001888.html
I'm happy to do more testing if needed, as it seems my patches get lost routinely.
Hi Alexander,
I may just have broken your patch-set accidentially by converting mncc_names to value_string, sorry for that :/
Regarding your patch, I have to disagree about several topics:
On Wed, Nov 25, 2015 at 03:31:04PM -0500, Alexander Chemeris wrote:
-static struct mncc_names { +struct name_value { char *name; int value; -} mncc_names[] = { +};
Is there a specific reason to introduce a new structure rather than use struct value_string from libosmocore, which is used all over the Osmo* code for this kind of name-value mapping? All code should use that infrastructure, and the occasional separate implementation should bec converted to value_string.
+static struct name_value mncc_locations[] = {
- {"GSM48_CAUSE_LOC_USER", 0x00},
1) The cause values and cause locations are GSM 04.08, not MNCC specific. As such, they belong into libosmogsm, not OsmoNITB. Also, they shouldn't be called 'mncc_locations' if in fact they are GSM 04.08 locations.
2) Also, rather than introducing magic numbers, the defnitions from <osmocmo/gsm/protocol/gsm_04_08.h> should be used, making the above an
struct value_string gsm48_cause_loc_names[] = { { GSM48_CAUSE_LOC_USER, "GSM48_CAUSE_LOC_USER" },
3) And finally, the string should be more user-readable than the cryptic #define/enum value, i.e.
struct value_string gsm48_cause_loc_names[] = { { GSM48_CAUSE_LOC_USER, "User" },
Regards, Harald
-
Alexander Chemeris -
Harald Welte -
Holger Freyther