In libosmocore, MM and CC message type bitmask makros were introduced. Replace hardcoded bitmasks with these makros. Also replace the protocol discriminator mask 0x0f with GSM48_PDISC_MASK.
Note: in some places, use of the bitmask is doubtful. This commit so far applies the makros without changing anything.
Also note: the MM bitmask is hardcoded as 0xbf, but libosmocore also adjusts the mask to the apparent de-facto standard of 0x3f, while the CC bitmask is so far left unchanged at 0xbf. So where in doubt, use the CC bitmask. --- openbsc/src/libfilter/bsc_msg_filter.c | 8 ++++---- openbsc/src/libmsc/gsm_04_08.c | 8 ++++---- openbsc/src/osmo-bsc/osmo_bsc_api.c | 8 ++++---- openbsc/src/osmo-bsc/osmo_bsc_filter.c | 8 ++++---- openbsc/src/osmo-bsc_nat/bsc_nat.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_ussd.c | 4 ++-- 7 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/openbsc/src/libfilter/bsc_msg_filter.c b/openbsc/src/libfilter/bsc_msg_filter.c index eafeff4..25674e1 100644 --- a/openbsc/src/libfilter/bsc_msg_filter.c +++ b/openbsc/src/libfilter/bsc_msg_filter.c @@ -339,8 +339,8 @@ int bsc_msg_filter_initial(struct gsm48_hdr *hdr48, size_t hdr48_len, cause->lu_reject_cause = GSM48_REJECT_PLMN_NOT_ALLOWED; *imsi = NULL;
- proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = hdr48->proto_discr & GSM48_PDISC_MASK; + msg_type = hdr48->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; if (proto == GSM48_PDISC_MM && msg_type == GSM48_MT_MM_LOC_UPD_REQUEST) { *con_type = FLT_CON_TYPE_LU; @@ -388,8 +388,8 @@ int bsc_msg_filter_data(struct gsm48_hdr *hdr48, size_t len, if (state->imsi_checked) return 0;
- proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = hdr48->proto_discr & GSM48_PDISC_MASK; + msg_type = hdr48->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; if (proto != GSM48_PDISC_MM || msg_type != GSM48_MT_MM_ID_RESP) return 0;
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index d9d7390..3279835 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1131,7 +1131,7 @@ static int gsm0408_rcv_mm(struct gsm_subscriber_connection *conn, struct msgb *m struct gsm48_hdr *gh = msgb_l3(msg); int rc = 0;
- switch (gh->msg_type & 0xbf) { + switch (gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK) { case GSM48_MT_MM_LOC_UPD_REQUEST: DEBUGP(DMM, "LOCATION UPDATING REQUEST: "); rc = mm_rx_loc_upd_req(conn, msg); @@ -1860,7 +1860,7 @@ static void gsm48_start_cc_timer(struct gsm_trans *trans, int current, static int gsm48_cc_rx_setup(struct gsm_trans *trans, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t msg_type = gh->msg_type & 0xbf; + uint8_t msg_type = gh->msg_type & GSM48_MT_CC_MSG_TYPE_MASK; unsigned int payload_len = msgb_l3len(msg) - sizeof(*gh); struct tlv_parsed tp; struct gsm_mncc setup; @@ -3487,7 +3487,7 @@ static struct datastate { static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t msg_type = gh->msg_type & 0xbf; + uint8_t msg_type = gh->msg_type & GSM48_MT_CC_MSG_TYPE_MASK; uint8_t transaction_id = ((gh->proto_discr & 0xf0) ^ 0x80) >> 4; /* flip */ struct gsm_trans *trans = NULL; int i, rc = 0; @@ -3578,7 +3578,7 @@ int gsm0408_new_conn(struct gsm_subscriber_connection *conn) int gsm0408_dispatch(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; + uint8_t pdisc = gh->proto_discr & GSM48_PDISC_MASK; int rc = 0;
LOGP(DRLL, LOGL_DEBUG, "Dispatching 04.08 message, pdisc=%d\n", pdisc); diff --git a/openbsc/src/osmo-bsc/osmo_bsc_api.c b/openbsc/src/osmo-bsc/osmo_bsc_api.c index fbeed77..fde3566 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_api.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_api.c @@ -180,8 +180,8 @@ static void bsc_send_ussd_no_srv(struct gsm_subscriber_connection *conn, return;
gh = msgb_l3(msg); - pdisc = gh->proto_discr & 0x0f; - mtype = gh->msg_type & 0xbf; + pdisc = gh->proto_discr & GSM48_PDISC_MASK; + mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK;
/* Is CM service request? */ if (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) { @@ -341,8 +341,8 @@ static int handle_cc_setup(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; - uint8_t mtype = gh->msg_type & 0xbf; + uint8_t pdisc = gh->proto_discr & GSM48_PDISC_MASK; + uint8_t mtype = gh->msg_type & GSM48_MT_CC_MSG_TYPE_MASK;
struct osmo_msc_data *msc; struct gsm_mncc_number called; diff --git a/openbsc/src/osmo-bsc/osmo_bsc_filter.c b/openbsc/src/osmo-bsc/osmo_bsc_filter.c index 389a124..e1e948b 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_filter.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_filter.c @@ -141,8 +141,8 @@ struct osmo_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn, }
gh = msgb_l3(msg); - pdisc = gh->proto_discr & 0x0f; - mtype = gh->msg_type & 0xbf; + pdisc = gh->proto_discr & GSM48_PDISC_MASK; + mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK;
/* * We are asked to select a MSC here but they are not equal. We @@ -213,7 +213,7 @@ int bsc_scan_bts_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); uint8_t pdisc = gh->proto_discr & 0x0f; - uint8_t mtype = gh->msg_type & 0xbf; + uint8_t mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK;
if (pdisc == GSM48_PDISC_MM) { if (mtype == GSM48_MT_MM_LOC_UPD_REQUEST) @@ -347,7 +347,7 @@ int bsc_scan_msc_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) gh = (struct gsm48_hdr *) msgb_l3(msg); length -= (const char *)&gh->data[0] - (const char *)gh;
- mtype = gh->msg_type & 0xbf; + mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; net = conn->bts->network; msc = conn->sccp_con->msc;
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index cdab406..cbfb091 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -670,8 +670,8 @@ static void update_con_authorize(struct nat_sccp_connection *con, if (!hdr48) return;
- proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = hdr48->proto_discr & GSM48_PDISC_MASK; + msg_type = hdr48->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; if (proto == GSM48_PDISC_MM && msg_type == GSM48_MT_MM_CM_SERV_ACC) con->authorized = 1; diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c index ca5670c..ce7dfb8 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c @@ -594,8 +594,8 @@ struct msgb *bsc_nat_rewrite_msg(struct bsc_nat *nat, struct msgb *msg, struct b return msg;
link_id = msg->l3h[1]; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = hdr48->proto_discr & GSM48_PDISC_MASK; + msg_type = hdr48->msg_type & GSM48_MT_CC_MSG_TYPE_MASK;
if (proto == GSM48_PDISC_CC && msg_type == GSM48_MT_CC_SETUP) new_msg = rewrite_setup(nat, msg, parsed, imsi, hdr48, len); diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c index 1082414..8954ac1 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c +++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c @@ -407,8 +407,8 @@ int bsc_ussd_check(struct nat_sccp_connection *con, struct bsc_nat_parsed *parse if (!hdr48) return 0;
- proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = hdr48->proto_discr & GSM48_PDISC_MASK; + msg_type = hdr48->msg_type & GSM48_MT_CC_MSG_TYPE_MASK; ti = (hdr48->proto_discr & 0x70) >> 4; if (proto != GSM48_PDISC_NC_SS) return 0;
According to spec, the bitmasks for message types apply only for MM and CC messages. However, various code used the bitmask even though no MM or CC messages are involved:
bsc_msg_filter.c: bsc_msg_filter_initial(): use bitmasked msg_type_mm for MM messages, use unmasked msg_type for RR.
osmo_bsc_filter.c: bsc_find_msc() and bsc_scan_bts_msg(): use msg_type_mm for MM, unmasked for RR.
bsc_nat_rewrite.c: bsc_nat_rewrite_msg(): use msg_type_cc for CC, unmasked for SMS.
bsc_ussd.c: don't mask, no MM nor CC involved. --- openbsc/src/libfilter/bsc_msg_filter.c | 9 +++++---- openbsc/src/osmo-bsc/osmo_bsc_filter.c | 11 +++++++---- openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c | 7 ++++--- openbsc/src/osmo-bsc_nat/bsc_ussd.c | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/openbsc/src/libfilter/bsc_msg_filter.c b/openbsc/src/libfilter/bsc_msg_filter.c index 25674e1..581bba1 100644 --- a/openbsc/src/libfilter/bsc_msg_filter.c +++ b/openbsc/src/libfilter/bsc_msg_filter.c @@ -332,7 +332,7 @@ int bsc_msg_filter_initial(struct gsm48_hdr *hdr48, size_t hdr48_len, char **imsi, struct bsc_filter_reject_cause *cause) { int ret = 0; - uint8_t msg_type, proto; + uint8_t msg_type, msg_type_mm, proto;
*con_type = FLT_CON_TYPE_NONE; cause->cm_reject_cause = GSM48_REJECT_PLMN_NOT_ALLOWED; @@ -340,14 +340,15 @@ int bsc_msg_filter_initial(struct gsm48_hdr *hdr48, size_t hdr48_len, *imsi = NULL;
proto = hdr48->proto_discr & GSM48_PDISC_MASK; - msg_type = hdr48->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; + msg_type = hdr48->msg_type; + msg_type_mm = msg_type & GSM48_MT_MM_MSG_TYPE_MASK; if (proto == GSM48_PDISC_MM && - msg_type == GSM48_MT_MM_LOC_UPD_REQUEST) { + msg_type_mm == GSM48_MT_MM_LOC_UPD_REQUEST) { *con_type = FLT_CON_TYPE_LU; ret = _cr_check_loc_upd(req->ctx, &hdr48->data[0], hdr48_len - sizeof(*hdr48), imsi); } else if (proto == GSM48_PDISC_MM && - msg_type == GSM48_MT_MM_CM_SERV_REQ) { + msg_type_mm == GSM48_MT_MM_CM_SERV_REQ) { *con_type = FLT_CON_TYPE_CM_SERV_REQ; ret = _cr_check_cm_serv_req(req->ctx, &hdr48->data[0], hdr48_len - sizeof(*hdr48), diff --git a/openbsc/src/osmo-bsc/osmo_bsc_filter.c b/openbsc/src/osmo-bsc/osmo_bsc_filter.c index e1e948b..017db57 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_filter.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_filter.c @@ -128,6 +128,7 @@ struct osmo_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn, struct gsm48_hdr *gh; int8_t pdisc; uint8_t mtype; + uint8_t mtype_mm; struct osmo_bsc_data *bsc; struct osmo_msc_data *msc, *pag_msc; struct gsm_subscriber *subscr; @@ -142,7 +143,8 @@ struct osmo_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn,
gh = msgb_l3(msg); pdisc = gh->proto_discr & GSM48_PDISC_MASK; - mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; + mtype = gh->msg_type; + mtype_mm = mtype & GSM48_MT_MM_MSG_TYPE_MASK;
/* * We are asked to select a MSC here but they are not equal. We @@ -152,7 +154,7 @@ struct osmo_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn, */ if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) goto paging; - else if (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) { + else if (pdisc == GSM48_PDISC_MM && mtype_mm == GSM48_MT_MM_CM_SERV_REQ) { is_emerg = is_cm_service_for_emerg(msg); goto round_robin; } else @@ -213,10 +215,11 @@ int bsc_scan_bts_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); uint8_t pdisc = gh->proto_discr & 0x0f; - uint8_t mtype = gh->msg_type & GSM48_MT_MM_MSG_TYPE_MASK; + uint8_t mtype = gh->msg_type; + uint8_t mtype_mm = mtype & GSM48_MT_MM_MSG_TYPE_MASK;
if (pdisc == GSM48_PDISC_MM) { - if (mtype == GSM48_MT_MM_LOC_UPD_REQUEST) + if (mtype_mm == GSM48_MT_MM_LOC_UPD_REQUEST) handle_lu_request(conn, msg); } else if (pdisc == GSM48_PDISC_RR) { if (mtype == GSM48_MT_RR_PAG_RESP) diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c index ce7dfb8..97d5e3d 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c @@ -576,7 +576,7 @@ struct msgb *bsc_nat_rewrite_msg(struct bsc_nat *nat, struct msgb *msg, struct b { struct gsm48_hdr *hdr48; uint32_t len; - uint8_t msg_type, proto; + uint8_t msg_type, msg_type_cc, proto; struct msgb *new_msg = NULL, *sccp; uint8_t link_id;
@@ -595,9 +595,10 @@ struct msgb *bsc_nat_rewrite_msg(struct bsc_nat *nat, struct msgb *msg, struct b
link_id = msg->l3h[1]; proto = hdr48->proto_discr & GSM48_PDISC_MASK; - msg_type = hdr48->msg_type & GSM48_MT_CC_MSG_TYPE_MASK; + msg_type = hdr48->msg_type; + msg_type_cc = msg_type & GSM48_MT_CC_MSG_TYPE_MASK;
- if (proto == GSM48_PDISC_CC && msg_type == GSM48_MT_CC_SETUP) + if (proto == GSM48_PDISC_CC && msg_type_cc == GSM48_MT_CC_SETUP) new_msg = rewrite_setup(nat, msg, parsed, imsi, hdr48, len); else if (proto == GSM48_PDISC_SMS && msg_type == GSM411_MT_CP_DATA) new_msg = rewrite_sms(nat, msg, parsed, imsi, hdr48, len); diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c index 8954ac1..96d4a84 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c +++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c @@ -408,7 +408,7 @@ int bsc_ussd_check(struct nat_sccp_connection *con, struct bsc_nat_parsed *parse return 0;
proto = hdr48->proto_discr & GSM48_PDISC_MASK; - msg_type = hdr48->msg_type & GSM48_MT_CC_MSG_TYPE_MASK; + msg_type = hdr48->msg_type; ti = (hdr48->proto_discr & 0x70) >> 4; if (proto != GSM48_PDISC_NC_SS) return 0;
On Thu, Mar 10, 2016 at 11:22:19PM +0100, Neels Hofmeyr wrote:
- msg_type = hdr48->msg_type & 0xbf;
- proto = hdr48->proto_discr & GSM48_PDISC_MASK;
- msg_type = hdr48->msg_type & GSM48_MT_MM_MSG_TYPE_MASK;
maybe it is a good idea to provide an (inline?) function for this, which determines the mask based on the rules of TS 24.007 (see my other mail).
Something like
msg_type = gsm48hdr_get_msgtype(hdr48);
which would then use the proper mask based on hdr48->proto_discr and the rules from 24.007. This way the user (developer) cannot really do anything wrong and the correct mask is chosen. Similarly, gsm48hdr_get_proto(hdr48) could be used for the proto_discr.
Regards, Harald
-
Harald Welte -
Neels Hofmeyr