[PATCH 1/2] Refactor internal mncc code

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

msuraev at sysmocom.de msuraev at sysmocom.de
Wed Mar 23 18:14:50 UTC 2016


From: Max <msuraev at sysmocom.de>

Consistently use enums instead of (u)int, char etc.
Use bool for boolean types.
Add extra debug output with channel mode and type to simplify
troubleshooting.
---
 openbsc/include/openbsc/bsc_api.h    |  4 +++-
 openbsc/include/openbsc/gsm_04_08.h  |  2 +-
 openbsc/include/openbsc/mncc.h       |  6 ++++--
 openbsc/include/openbsc/mncc_int.h   |  5 ++++-
 openbsc/src/libbsc/bsc_api.c         | 21 ++++++++++++---------
 openbsc/src/libbsc/gsm_04_08_utils.c | 12 +++---------
 openbsc/src/libmsc/gsm_04_08.c       | 22 +++++++++++++++++-----
 openbsc/src/libmsc/mncc_builtin.c    | 15 +++++++++++----
 openbsc/src/osmo-nitb/bsc_hack.c     |  4 +++-
 9 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/openbsc/include/openbsc/bsc_api.h b/openbsc/include/openbsc/bsc_api.h
index a3d12f2..d0a0164 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 <stdbool.h>
+
 #include "gsm_data.h"
 
 #define BSC_API_CONN_POL_ACCEPT	0
@@ -45,7 +47,7 @@ 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, bool full_rate);
 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..ce2f87d 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 {
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 e6d820d..dadb082 100644
--- a/openbsc/src/libbsc/bsc_api.c
+++ b/openbsc/src/libbsc/bsc_api.c
@@ -21,6 +21,8 @@
  *
  */
 
+#include <stdbool.h>
+
 #include <openbsc/bsc_api.h>
 #include <openbsc/bsc_rll.h>
 #include <openbsc/gsm_data.h>
@@ -34,7 +36,7 @@
 #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>
 
 #define GSM0808_T10_VALUE    6, 0
@@ -155,7 +157,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, bool full_rate)
 {
 	struct bsc_api *api;
 	api = conn->bts->network->bsc_api;
@@ -197,12 +199,10 @@ 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, bool full_rate)
 {
 	struct gsm_lchan *new_lchan;
-	int chan_type;
-
-	chan_type = full_rate ? GSM_LCHAN_TCH_F : GSM_LCHAN_TCH_H;
+	enum gsm_chan_t chan_type = full_rate ? GSM_LCHAN_TCH_F : GSM_LCHAN_TCH_H;
 
 	new_lchan = lchan_alloc(conn->bts, chan_type, 0);
 
@@ -337,7 +337,7 @@ 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, bool full_rate)
 {
 	switch (chan_mode) {
 	case GSM48_CMODE_SIGN:
@@ -384,7 +384,8 @@ static int chan_compat_with_mode(struct gsm_lchan *lchan, int chan_mode, int ful
  *
  * 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, bool full_rate)
 {
 	struct bsc_api *api;
 	api = conn->bts->network->bsc_api;
@@ -394,7 +395,9 @@ int gsm0808_assign_req(struct gsm_subscriber_connection *conn, int chan_mode, in
 			goto error;
 	} else {
 		LOGP(DMSC, LOGL_NOTICE,
-			"Sending ChanModify for speech %d %d\n", chan_mode, full_rate);
+		     "Sending ChanModify for speech: %s on channel %s, "
+		     "full_rate %d\n", osmo_gsm48_chan_mode2str(chan_mode),
+		     osmo_gsm48_chan_type2str(conn->lchan->type), full_rate);
 		if (chan_mode == GSM48_CMODE_SPEECH_AMR)
 			handle_mr_config(conn, conn->lchan, full_rate);
 
diff --git a/openbsc/src/libbsc/gsm_04_08_utils.c b/openbsc/src/libbsc/gsm_04_08_utils.c
index 8c6dbef..79a8547 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_tx_chan_mode_modify(struct gsm_lchan *lchan, uint8_t mode)
+int gsm48_tx_chan_mode_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));
@@ -513,15 +513,9 @@ int gsm48_tx_chan_mode_modify(struct gsm_lchan *lchan, uint8_t mode)
 	return gsm48_sendmsg(msg);
 }
 
-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 rc;
-
-	rc = gsm48_tx_chan_mode_modify(lchan, lchan_mode);
-	if (rc < 0)
-		return rc;
-
-	return rc;
+	return gsm48_tx_chan_mode_modify(lchan, lchan_mode);
 }
 
 int gsm48_rx_rr_modif_ack(struct msgb *msg)
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index 1524ec4..95dd647 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -59,6 +59,7 @@
 #include <osmocom/gsm/gsm48.h>
 #include <osmocom/gsm/gsm0480.h>
 #include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
 #include <osmocom/core/msgb.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/gsm/tlv.h>
@@ -1598,11 +1599,15 @@ static int tch_map(struct gsm_lchan *lchan, struct gsm_lchan *remote_lchan)
 {
 	struct gsm_bts *bts = lchan->ts->trx->bts;
 	struct gsm_bts *remote_bts = remote_lchan->ts->trx->bts;
+	enum gsm_chan_t lt = lchan->type, rt = remote_lchan->type;
 	int rc;
 
-	DEBUGP(DCC, "Setting up TCH map between (bts=%u,trx=%u,ts=%u) and (bts=%u,trx=%u,ts=%u)\n",
-		bts->nr, lchan->ts->trx->nr, lchan->ts->nr,
-		remote_bts->nr, remote_lchan->ts->trx->nr, remote_lchan->ts->nr);
+	DEBUGP(DCC, "Setting up TCH map between (bts=%u,trx=%u,ts=%u,%s) and "
+	       "(bts=%u,trx=%u,ts=%u,%s)\n",
+	       bts->nr, lchan->ts->trx->nr, lchan->ts->nr,
+	       osmo_gsm48_chan_type2str(lt),
+	       remote_bts->nr, remote_lchan->ts->trx->nr, remote_lchan->ts->nr,
+	       osmo_gsm48_chan_type2str(rt));
 
 	if (bts->type != remote_bts->type) {
 		LOGP(DCC, LOGL_ERROR, "Cannot switch calls between different BTS types yet\n");
@@ -2999,6 +3004,7 @@ static int tch_rtp_create(struct gsm_network *net, uint32_t callref)
 	struct gsm_bts *bts;
 	struct gsm_lchan *lchan;
 	struct gsm_trans *trans;
+	enum gsm48_chan_mode m;
 
 	/* Find callref */
 	trans = trans_find_by_callref(net, callref);
@@ -3038,8 +3044,11 @@ static int tch_rtp_create(struct gsm_network *net, uint32_t callref)
 	 */
 	if (lchan->tch_mode == GSM48_CMODE_SIGN) {
 		trans->conn->mncc_rtp_create_pending = 1;
-		return gsm0808_assign_req(trans->conn,
-				mncc_codec_for_mode(lchan->type),
+		m = mncc_codec_for_mode(lchan->type);
+		LOGP(DMNCC, LOGL_DEBUG, "RTP create: codec=%s, chan_type=%s\n",
+		     osmo_gsm48_chan_mode2str(m),
+		     osmo_gsm48_chan_type2str(lchan->type));
+		return gsm0808_assign_req(trans->conn, m,
 				lchan->type != GSM_LCHAN_TCH_H);
 	}
 
@@ -3068,6 +3077,9 @@ static int tch_rtp_connect(struct gsm_network *net, void *arg)
 	}
 
 	lchan = trans->conn->lchan;
+	LOGP(DMNCC, LOGL_DEBUG, "RTP connect: codec=%s, chan_type=%s\n",
+		     osmo_gsm48_chan_mode2str(mncc_codec_for_mode(lchan->type)),
+		     osmo_gsm48_chan_type2str(lchan->type));
 
 	/* TODO: Check if payload_msg_type is compatible with what we have */
 	if (rtp->payload_type != lchan->abis_ip.rtp_payload) {
diff --git a/openbsc/src/libmsc/mncc_builtin.c b/openbsc/src/libmsc/mncc_builtin.c
index 77df6fb..b1993ca 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);
 }
@@ -138,7 +141,8 @@ static int mncc_setup_ind(struct gsm_call *call, int msg_type,
 	memset(&mncc, 0, sizeof(struct gsm_mncc));
 	mncc.callref = call->callref;
 	mncc.lchan_mode = determine_lchan_mode(setup);
-	DEBUGP(DMNCC, "(call %x) Modify channel mode.\n", call->callref);
+	DEBUGP(DMNCC, "(call %x) Modify channel mode: %s\n", call->callref,
+	       osmo_gsm48_chan_mode2str(mncc.lchan_mode));
 	mncc_tx_to_cc(call->net, MNCC_LCHAN_MODIFY, &mncc);
 
 	/* send setup to remote */
@@ -207,13 +211,16 @@ static int mncc_setup_cnf(struct gsm_call *call, int msg_type,
 	DEBUGP(DMNCC, "(call %x) Bridging with remote.\n", call->callref);
 
 	/* in direct mode, we always have to bridge the channels */
-	if (ipacc_rtp_direct)
+	if (ipacc_rtp_direct) {
+		DEBUGP(DMNCC, "Bridging: direct RTP.\n");
 		return mncc_tx_to_cc(call->net, MNCC_BRIDGE, &bridge);
+	}
 
 	/* proxy mode */
 	if (!net->handover.active) {
 		/* in the no-handover case, we can bridge, i.e. use
 		 * the old RTP proxy code */
+		DEBUGP(DMNCC, "Bridging: no handover is active.\n");
 		return mncc_tx_to_cc(call->net, MNCC_BRIDGE, &bridge);
 	} else {
 		/* in case of handover, we need to re-write the RTP
diff --git a/openbsc/src/osmo-nitb/bsc_hack.c b/openbsc/src/osmo-nitb/bsc_hack.c
index dffe642..0b360dc 100644
--- a/openbsc/src/osmo-nitb/bsc_hack.c
+++ b/openbsc/src/osmo-nitb/bsc_hack.c
@@ -288,8 +288,10 @@ int main(int argc, char **argv)
 		rc = bsc_bootstrap_network(mncc_sock_from_cc, config_file);
 		if (rc >= 0)
 			mncc_sock_init(bsc_gsmnet, mncc_sock_path);
-	} else
+	} else {
+		DEBUGP(DMNCC, "Using internal MNCC handler.\n");
 		rc = bsc_bootstrap_network(int_mncc_recv, config_file);
+	}
 	if (rc < 0)
 		exit(1);
 #ifdef BUILD_SMPP
-- 
2.7.4




More information about the OpenBSC mailing list