[PATCH] Refactor MNCC code

msuraev at sysmocom.de msuraev at sysmocom.de
Mon Apr 25 10:22:17 UTC 2016


From: Max <msuraev at sysmocom.de>

* Explicitly use gsm_chan_t enum when checking for full rate channels.
* Consistently use enums instead of (u)int, char etc.
Note: because actual enum representation is up to compiler this might
change the size of gsm_mncc struct so the MNCC protocol version is
bumped.
---
 openbsc/include/openbsc/bsc_api.h     |  6 +++-
 openbsc/include/openbsc/gsm_04_08.h   |  2 +-
 openbsc/include/openbsc/mncc.h        |  8 ++++--
 openbsc/include/openbsc/mncc_int.h    |  5 +++-
 openbsc/src/libbsc/bsc_api.c          | 53 ++++++++++++++++++-----------------
 openbsc/src/libbsc/gsm_04_08_utils.c  |  2 +-
 openbsc/src/libmsc/gsm_04_08.c        |  5 ++--
 openbsc/src/libmsc/mncc_builtin.c     |  7 +++--
 openbsc/src/osmo-bsc/osmo_bsc_bssap.c | 10 ++++---
 9 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/openbsc/include/openbsc/bsc_api.h b/openbsc/include/openbsc/bsc_api.h
index a3d12f2..b2fc1ed 100644
--- a/openbsc/include/openbsc/bsc_api.h
+++ b/openbsc/include/openbsc/bsc_api.h
@@ -3,6 +3,8 @@
 #ifndef OPENBSC_BSC_API_H
 #define OPENBSC_BSC_API_H
 
+#include <osmocom/gsm/gsm_utils.h>
+
 #include "gsm_data.h"
 
 #define BSC_API_CONN_POL_ACCEPT	0
@@ -45,7 +47,9 @@ struct bsc_api {
 
 int bsc_api_init(struct gsm_network *network, struct bsc_api *api);
 int gsm0808_submit_dtap(struct gsm_subscriber_connection *conn, struct msgb *msg, int link_id, int allow_sacch);
-int gsm0808_assign_req(struct gsm_subscriber_connection *conn, int chan_mode, int full_rate);
+int gsm0808_assign_req(struct gsm_subscriber_connection *conn,
+		       enum gsm48_chan_mode chan_mode,
+		       enum gsm_chan_t chan);
 int gsm0808_cipher_mode(struct gsm_subscriber_connection *conn, int cipher,
 			const uint8_t *key, int len, int include_imeisv);
 int gsm0808_page(struct gsm_bts *bts, unsigned int page_group,
diff --git a/openbsc/include/openbsc/gsm_04_08.h b/openbsc/include/openbsc/gsm_04_08.h
index fd0b89d..d333e13 100644
--- a/openbsc/include/openbsc/gsm_04_08.h
+++ b/openbsc/include/openbsc/gsm_04_08.h
@@ -79,7 +79,7 @@ int gsm48_extract_mi(uint8_t *classmark2, int length, char *mi_string, uint8_t *
 int gsm48_paging_extract_mi(struct gsm48_pag_resp *pag, int length, char *mi_string, uint8_t *mi_type);
 int gsm48_handle_paging_resp(struct gsm_subscriber_connection *conn, struct msgb *msg, struct gsm_subscriber *subscr);
 
-int gsm48_lchan_modify(struct gsm_lchan *lchan, uint8_t lchan_mode);
+int gsm48_lchan_modify(struct gsm_lchan *lchan, enum gsm48_chan_mode lchan_mode);
 int gsm48_rx_rr_modif_ack(struct msgb *msg);
 int gsm48_parse_meas_rep(struct gsm_meas_rep *rep, struct msgb *msg);
 
diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h
index 49f0c8b..70866fe 100644
--- a/openbsc/include/openbsc/mncc.h
+++ b/openbsc/include/openbsc/mncc.h
@@ -26,6 +26,8 @@
 
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/gsm/mncc.h>
+#include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
 
 #include <stdint.h>
 
@@ -156,8 +158,8 @@ struct gsm_mncc {
 	int		emergency;
 	char		imsi[16];
 
-	unsigned char	lchan_type;
-	unsigned char	lchan_mode;
+	enum gsm_chan_t lchan_type;
+	enum gsm48_chan_mode lchan_mode;
 };
 
 struct gsm_data_frame {
@@ -166,7 +168,7 @@ struct gsm_data_frame {
 	unsigned char	data[0];
 };
 
-#define MNCC_SOCK_VERSION	5
+#define MNCC_SOCK_VERSION	6
 struct gsm_mncc_hello {
 	uint32_t	msg_type;
 	uint32_t	version;
diff --git a/openbsc/include/openbsc/mncc_int.h b/openbsc/include/openbsc/mncc_int.h
index 213ce14..dafef25 100644
--- a/openbsc/include/openbsc/mncc_int.h
+++ b/openbsc/include/openbsc/mncc_int.h
@@ -3,12 +3,15 @@
 
 #include <stdint.h>
 
+#include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
+
 struct mncc_int {
 	uint8_t def_codec[2];
 };
 
 extern struct mncc_int mncc_int;
 
-uint8_t mncc_codec_for_mode(int lchan_type);
+enum gsm48_chan_mode mncc_codec_for_mode(enum gsm_chan_t lchan_type);
 
 #endif
diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c
index b8b5967..2608504 100644
--- a/openbsc/src/libbsc/bsc_api.c
+++ b/openbsc/src/libbsc/bsc_api.c
@@ -34,8 +34,9 @@
 #include <openbsc/trau_mux.h>
 
 #include <osmocom/gsm/protocol/gsm_08_08.h>
-
+#include <osmocom/gsm/protocol/gsm_04_08.h>
 #include <osmocom/core/talloc.h>
+#include <osmocom/gsm/gsm_utils.h>
 
 #define GSM0808_T10_VALUE    6, 0
 
@@ -155,7 +156,7 @@ static void assignment_t10_timeout(void *_conn)
  * Handle the multirate config
  */
 static void handle_mr_config(struct gsm_subscriber_connection *conn,
-			     struct gsm_lchan *lchan, int full_rate)
+			     struct gsm_lchan *lchan, enum gsm_chan_t chan)
 {
 	struct bsc_api *api;
 	api = conn->bts->network->bsc_api;
@@ -163,12 +164,13 @@ static void handle_mr_config(struct gsm_subscriber_connection *conn,
 	struct gsm48_multi_rate_conf *mr_conf;
 
 	if (api->mr_config)
-		return api->mr_config(conn, lchan, full_rate);
+		return api->mr_config(conn, lchan,
+				      (GSM_LCHAN_TCH_H == chan) ? 0 : 1);
 
-	if (full_rate)
-		mr = &lchan->ts->trx->bts->mr_full;
-	else
+	if (GSM_LCHAN_TCH_H == chan)
 		mr = &lchan->ts->trx->bts->mr_half;
+	else
+		mr = &lchan->ts->trx->bts->mr_full;
 
 	mr_conf = (struct gsm48_multi_rate_conf *) mr->gsm48_ie;
 	mr_conf->ver = 1;
@@ -197,12 +199,11 @@ static void handle_mr_config(struct gsm_subscriber_connection *conn,
  * -> Assignment Complete/Assignment Failure
  *  5.) Release the SDCCH, continue signalling on the new link
  */
-static int handle_new_assignment(struct gsm_subscriber_connection *conn, int chan_mode, int full_rate)
+static int handle_new_assignment(struct gsm_subscriber_connection *conn,
+				 enum gsm48_chan_mode chan_mode,
+				 enum gsm_chan_t chan_type)
 {
 	struct gsm_lchan *new_lchan;
-	int chan_type;
-
-	chan_type = full_rate ? GSM_LCHAN_TCH_F : GSM_LCHAN_TCH_H;
 
 	new_lchan = lchan_alloc(conn->bts, chan_type, 0);
 
@@ -223,7 +224,7 @@ static int handle_new_assignment(struct gsm_subscriber_connection *conn, int cha
 
 	/* handle AMR correctly */
 	if (chan_mode == GSM48_CMODE_SPEECH_AMR)
-		handle_mr_config(conn, new_lchan, full_rate);
+		handle_mr_config(conn, new_lchan, chan_type);
 
 	if (rsl_chan_activate_lchan(new_lchan, 0x1, 0) < 0) {
 		LOGP(DHO, LOGL_ERROR, "could not activate channel\n");
@@ -337,7 +338,9 @@ int gsm0808_submit_dtap(struct gsm_subscriber_connection *conn,
 /*
  * \brief Check if the given channel is compatible with the mode/fullrate
  */
-static int chan_compat_with_mode(struct gsm_lchan *lchan, int chan_mode, int full_rate)
+static int chan_compat_with_mode(struct gsm_lchan *lchan,
+				 enum gsm48_chan_mode chan_mode,
+				 enum gsm_chan_t chan)
 {
 	switch (chan_mode) {
 	case GSM48_CMODE_SIGN:
@@ -348,16 +351,14 @@ static int chan_compat_with_mode(struct gsm_lchan *lchan, int chan_mode, int ful
 	case GSM48_CMODE_DATA_3k6:
 	case GSM48_CMODE_DATA_6k0:
 		/* these services can all run on TCH/H, but we may have
-		 * an explicit override by the 'full_rate' argument */
+		 * an explicit override by the explicit chan argument */
 		switch (lchan->type) {
 		case GSM_LCHAN_TCH_F:
 			return 1;
 		case GSM_LCHAN_TCH_H:
-			if (full_rate)
-				return 0;
-			else
+			if (GSM_LCHAN_TCH_H == chan)
 				return 1;
-			break;
+			return 0;
 		default:
 			return 0;
 		}
@@ -368,9 +369,7 @@ static int chan_compat_with_mode(struct gsm_lchan *lchan, int chan_mode, int ful
 		/* these services all explicitly require a TCH/F */
 		if (lchan->type == GSM_LCHAN_TCH_F)
 			return 1;
-		else
-			return 0;
-		break;
+		return 0;
 	}
 
 	return 0;
@@ -382,19 +381,21 @@ static int chan_compat_with_mode(struct gsm_lchan *lchan, int chan_mode, int ful
  * this is for audio handling only. In case the current channel does not allow
  * the selected mode a new one will be allocated.
  *
+ * If chan is not GSM_LCHAN_TCH_H than full rate is assumed.
+ *
  * TODO: Add multirate configuration, make it work for more than audio.
  */
-int gsm0808_assign_req(struct gsm_subscriber_connection *conn, int chan_mode, int full_rate)
+int gsm0808_assign_req(struct gsm_subscriber_connection *conn,
+		       enum gsm48_chan_mode chan_mode, enum gsm_chan_t chan)
 {
-	struct bsc_api *api;
-	api = conn->bts->network->bsc_api;
+	struct bsc_api *api = conn->bts->network->bsc_api;
 
-	if (!chan_compat_with_mode(conn->lchan, chan_mode, full_rate)) {
-		if (handle_new_assignment(conn, chan_mode, full_rate) != 0)
+	if (!chan_compat_with_mode(conn->lchan, chan_mode, chan)) {
+		if (handle_new_assignment(conn, chan_mode, chan) != 0)
 			goto error;
 	} else {
 		if (chan_mode == GSM48_CMODE_SPEECH_AMR)
-			handle_mr_config(conn, conn->lchan, full_rate);
+			handle_mr_config(conn, conn->lchan, chan);
 
 		LOGP(DMSC, LOGL_NOTICE,
 		     "Sending ChanModify for speech: %s on channel %s\n",
diff --git a/openbsc/src/libbsc/gsm_04_08_utils.c b/openbsc/src/libbsc/gsm_04_08_utils.c
index 635665a..327cff1 100644
--- a/openbsc/src/libbsc/gsm_04_08_utils.c
+++ b/openbsc/src/libbsc/gsm_04_08_utils.c
@@ -488,7 +488,7 @@ int gsm48_send_rr_ass_cmd(struct gsm_lchan *dest_lchan, struct gsm_lchan *lchan,
 }
 
 /* 9.1.5 Channel mode modify: Modify the mode on the MS side */
-int gsm48_lchan_modify(struct gsm_lchan *lchan, uint8_t mode)
+int gsm48_lchan_modify(struct gsm_lchan *lchan, enum gsm48_chan_mode mode)
 {
 	struct msgb *msg = gsm48_msgb_alloc_name("GSM 04.08 CHN MOD");
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index 7b78d48..4de3477 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -2939,7 +2939,7 @@ static int _gsm48_lchan_modify(struct gsm_trans *trans, void *arg)
 		return 0;
 
 	return gsm0808_assign_req(trans->conn, mode->lchan_mode,
-		trans->conn->lchan->type != GSM_LCHAN_TCH_H);
+				  trans->conn->lchan->type);
 }
 
 static void mncc_recv_rtp(struct gsm_network *net, uint32_t callref,
@@ -3048,8 +3048,7 @@ static int tch_rtp_create(struct gsm_network *net, uint32_t callref)
 		LOGP(DMNCC, LOGL_DEBUG, "RTP create: codec=%s, chan_type=%s\n",
 		     get_value_string(gsm48_chan_mode_names, m),
 		     get_value_string(gsm_chan_t_names, lchan->type));
-		return gsm0808_assign_req(trans->conn, m,
-				lchan->type != GSM_LCHAN_TCH_H);
+		return gsm0808_assign_req(trans->conn, m, lchan->type);
 	}
 
 	mncc_recv_rtp_sock(trans->net, trans, MNCC_RTP_CREATE);
diff --git a/openbsc/src/libmsc/mncc_builtin.c b/openbsc/src/libmsc/mncc_builtin.c
index ee98d2d..b879756 100644
--- a/openbsc/src/libmsc/mncc_builtin.c
+++ b/openbsc/src/libmsc/mncc_builtin.c
@@ -27,6 +27,9 @@
 #include <string.h>
 #include <errno.h>
 
+#include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
+
 #include <openbsc/gsm_04_08.h>
 #include <openbsc/debug.h>
 #include <openbsc/mncc.h>
@@ -65,7 +68,7 @@ static struct gsm_call *get_call_ref(uint32_t callref)
 	return NULL;
 }
 
-uint8_t mncc_codec_for_mode(int lchan_type)
+enum gsm48_chan_mode mncc_codec_for_mode(enum gsm_chan_t lchan_type)
 {
 	/* FIXME: check codec capabilities of the phone */
 
@@ -75,7 +78,7 @@ uint8_t mncc_codec_for_mode(int lchan_type)
 		return mncc_int.def_codec[1];
 }
 
-static uint8_t determine_lchan_mode(struct gsm_mncc *setup)
+static enum gsm48_chan_mode determine_lchan_mode(struct gsm_mncc *setup)
 {
 	return mncc_codec_for_mode(setup->lchan_type);
 }
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_bssap.c b/openbsc/src/osmo-bsc/osmo_bsc_bssap.c
index a60940d..01105a8 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_bssap.c
@@ -28,6 +28,7 @@
 
 #include <osmocom/gsm/protocol/gsm_08_08.h>
 #include <osmocom/gsm/gsm0808.h>
+#include <osmocom/gsm/gsm_utils.h>
 
 /*
  * helpers for the assignment command
@@ -292,7 +293,8 @@ static int bssmap_handle_assignm_req(struct osmo_bsc_sccp_con *conn,
 	uint8_t timeslot;
 	uint8_t multiplex;
 	enum gsm48_chan_mode chan_mode = GSM48_CMODE_SIGN;
-	int i, supported, port, full_rate = -1;
+	enum gsm_chan_t chan = GSM_LCHAN_TCH_H;
+	int i, supported, port;
 
 	if (!conn->conn) {
 		LOGP(DMSC, LOGL_ERROR, "No lchan/msc_data in cipher mode command.\n");
@@ -344,7 +346,6 @@ static int bssmap_handle_assignm_req(struct osmo_bsc_sccp_con *conn,
 	 * inner loop. The outer loop will exit due chan_mode having
 	 * the correct value.
 	 */
-	full_rate = 0;
 	msc = conn->msc;
 	for (supported = 0;
 		chan_mode == GSM48_CMODE_SIGN && supported < msc->audio_length;
@@ -354,7 +355,8 @@ static int bssmap_handle_assignm_req(struct osmo_bsc_sccp_con *conn,
 		for (i = 2; i < TLVP_LEN(&tp, GSM0808_IE_CHANNEL_TYPE); ++i) {
 			if ((data[i] & 0x7f) == perm_val) {
 				chan_mode = gsm88_to_chan_mode(perm_val);
-				full_rate = (data[i] & 0x4) == 0;
+				if (0 == (data[i] & 0x4))
+					chan = GSM_LCHAN_TCH_F;
 				break;
 			} else if ((data[i] & 0x80) == 0x00) {
 				break;
@@ -371,7 +373,7 @@ static int bssmap_handle_assignm_req(struct osmo_bsc_sccp_con *conn,
 	port = mgcp_timeslot_to_endpoint(multiplex, timeslot);
 	conn->rtp_port = rtp_calculate_port(port, msc->rtp_base);
 
-	return gsm0808_assign_req(conn->conn, chan_mode, full_rate);
+	return gsm0808_assign_req(conn->conn, chan_mode, chan);
 
 reject:
 	resp = gsm0808_create_assignment_failure(GSM0808_CAUSE_NO_RADIO_RESOURCE_AVAILABLE, NULL);
-- 
2.8.1



More information about the OpenBSC mailing list