diff mbox

move to hex TMSI representation

Message ID 1459081030-23070-1-git-send-email-axilirator@gmail.com
State New
Headers show

Commit Message

Vadim Yanitskiy March 27, 2016, 12:17 p.m. UTC 
In OpenBSC, we traditionally displayed a TMSI in its integer
representation, which is quite unusual in the telecom world. A TMSI is
normally printed as a series of 8 hex digits.

This patch aligns OpenBSC with the telecom industry standard.

- Use hex representation in VTY
- Increased DB SCHEMA_REVISION
- Implemented DB migration code

Signed-off-by: Vadim Yanitskiy <axilirator@gmail.com>
---
 openbsc/include/openbsc/gsm_subscriber.h    |  6 +-
 openbsc/src/libcommon/gsm_subscriber_base.c |  5 +-
 openbsc/src/libmsc/db.c                     | 96 ++++++++++++++++++++++++++---
 openbsc/src/libmsc/gsm_04_08.c              | 30 +++------
 openbsc/src/libmsc/gsm_subscriber.c         | 10 ++-
 openbsc/src/libmsc/vty_interface_layer3.c   |  2 +-
 openbsc/src/osmo-bsc/osmo_bsc_filter.c      |  2 +-
 openbsc/src/osmo-bsc/osmo_bsc_vty.c         |  4 +-
 openbsc/tests/db/db_test.c                  |  4 +-
 openbsc/tests/gsm0408/gsm0408_test.c        |  2 +-
 10 files changed, 115 insertions(+), 46 deletions(-)

Comments

Holger Freyther March 27, 2016, 4:44 p.m. UTC | #1 
> On 27 Mar 2016, at 14:17, Vadim Yanitskiy <axilirator@gmail.com> wrote:

Dear Vadim,


> 
> In OpenBSC, we traditionally displayed a TMSI in its integer
> representation, which is quite unusual in the telecom world. A TMSI is
> normally printed as a series of 8 hex digits.
> 
> This patch aligns OpenBSC with the telecom industry standard.

thanks a lot, I am afraid we need one more round.


> -#define tmsi_from_string(str) strtoul(str, NULL, 10)
> +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)

this macro is used for parsing strings from the network. We should not modify it.



> struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_subscriber_group *sgrp,
> -					     uint32_t tmsi)
> +					     const char *tmsi)


why? the number of bytes needed fit in uint32_t so we should remain with this internal storage. We should just make sure to always print it as hex.


> -#define SCHEMA_REVISION "4"
> +#define SCHEMA_REVISION "5"

good, but I think we should change the schema to use uint32_t/INTEGER for the TMSI instead of text.



> +		/* Update old TMSI */
> +		update = dbi_conn_queryf(conn,
> +				"UPDATE Subscriber "
> +				"SET tmsi = '0x%08x' "
> +				"WHERE id = %llu",

but to int :)


kind regards
	holger
Vadim Yanitskiy March 29, 2016, 4:10 p.m. UTC | #2 
Hello!

> thanks a lot, I am afraid we need one more round.

No problem :)

> -#define tmsi_from_string(str) strtoul(str, NULL, 10)
> +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
>
> this macro is used for parsing strings from the network. We should not
modify it.

I cannot find where...

Searching 420 files for "tmsi_from_string" (regex)

/home/wmn/openbsc/openbsc/include/openbsc/gsm_subscriber.h:
   15  #define GSM_SUBSCRIBER_FIRST_CONTACT    0x00000001
   16  /* gprs_sgsn.h defines additional flags including and above bit 16
(0x10000) */
   17: #define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
   18
   19  #define GSM_SUBSCRIBER_NO_EXPIRATION    0x0

/home/wmn/openbsc/openbsc/src/libcommon/gsm_subscriber_base.c:
  122  {
  123      struct gsm_subscriber *subscr;
  124:     uint8_t tmsi_val = tmsi_from_string(tmsi);
  125
  126      llist_for_each_entry(subscr, subscr_bsc_active_subscribers(),
entry) {

/home/wmn/openbsc/openbsc/src/libmsc/db.c:
  890      string = dbi_result_get_string(result, "tmsi");
  891      if (string)
  892:         subscr->tmsi = tmsi_from_string(string);
  893
  894      string = dbi_result_get_string(result, "name");

/home/wmn/openbsc/openbsc/src/libmsc/gsm_subscriber.c:
  215  {
  216      struct gsm_subscriber *subscr;
  217:     uint32_t tmsi_val = tmsi_from_string(tmsi);
  218
  219      /* we might have a record in memory already */

/home/wmn/openbsc/openbsc/tests/gsm0408/gsm0408_test.c:
   94      mi_len = gsm48_generate_mid_from_tmsi(mi, tmsi);
   95      gsm48_mi_to_string(mi_parsed, sizeof(mi_parsed), mi + 2, mi_len
- 2);
   96:     COMPARE((uint32_t)tmsi_from_string(mi_parsed), ==, tmsi);
   97
   98      /* imsi code */

5 matches across 5 files

> struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_subscriber_group
*sgrp,
> -                                          uint32_t tmsi)
> +                                          const char *tmsi)
>
> why? the number of bytes needed fit in uint32_t so we should remain with
this internal storage. We should just make sure to always print it as hex.

Ok, I'll revert this changes.

> -#define SCHEMA_REVISION "4"
> +#define SCHEMA_REVISION "5"
>
> good, but I think we should change the schema to use uint32_t/INTEGER for
the TMSI instead of text.

I'll do it in db.c.


С наилучшими пожеланиями,
Яницкий Вадим.

2016-03-27 22:44 GMT+06:00 Holger Freyther <holger@freyther.de>:

>
> > On 27 Mar 2016, at 14:17, Vadim Yanitskiy <axilirator@gmail.com> wrote:
>
> Dear Vadim,
>
>
> >
> > In OpenBSC, we traditionally displayed a TMSI in its integer
> > representation, which is quite unusual in the telecom world. A TMSI is
> > normally printed as a series of 8 hex digits.
> >
> > This patch aligns OpenBSC with the telecom industry standard.
>
> thanks a lot, I am afraid we need one more round.
>
>
> > -#define tmsi_from_string(str) strtoul(str, NULL, 10)
> > +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
>
> this macro is used for parsing strings from the network. We should not
> modify it.
>
>
>
> > struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_subscriber_group
> *sgrp,
> > -                                          uint32_t tmsi)
> > +                                          const char *tmsi)
>
>
> why? the number of bytes needed fit in uint32_t so we should remain with
> this internal storage. We should just make sure to always print it as hex.
>
>
> > -#define SCHEMA_REVISION "4"
> > +#define SCHEMA_REVISION "5"
>
> good, but I think we should change the schema to use uint32_t/INTEGER for
> the TMSI instead of text.
>
>
>
> > +             /* Update old TMSI */
> > +             update = dbi_conn_queryf(conn,
> > +                             "UPDATE Subscriber "
> > +                             "SET tmsi = '0x%08x' "
> > +                             "WHERE id = %llu",
>
> but to int :)
>
>
> kind regards
>         holger
Holger Freyther March 29, 2016, 4:39 p.m. UTC | #3 
> On 29 Mar 2016, at 18:10, Вадим Яницкий <axilirator@gmail.com> wrote:
> 
> Hello!
> 
> > thanks a lot, I am afraid we need one more round.
> 
> No problem :)
> 
> > -#define tmsi_from_string(str) strtoul(str, NULL, 10)
> > +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
> >
> > this macro is used for parsing strings from the network. We should not modify it.
> 
> I cannot find where...


ah I see. You change the invocations in gsm_04_08.c. I would prefer if the name of this method is changed but once we work with uint32_t it will mostly go away anyway?

holger
Vadim Yanitskiy March 29, 2016, 6:42 p.m. UTC | #4 
Yes, we needn't this macro now. The only place it can be used is
vty_interface_layer3.c:

static struct gsm_subscriber *get_subscr_by_argv(struct gsm_network *gsmnet,
                         const char *type,
                         const char *id)
{
    if (!strcmp(type, "extension"))
        return subscr_get_by_extension(gsmnet->subscr_group, id);
    else if (!strcmp(type, "imsi"))
        return subscr_get_by_imsi(gsmnet->subscr_group, id);
    else if (!strcmp(type, "tmsi"))
        return subscr_get_by_tmsi(gsmnet->subscr_group, id);
    else if (!strcmp(type, "id"))
        return subscr_get_by_id(gsmnet->subscr_group, atoi(id));

    return NULL;
}

In this place we have to convert a string (written from VTY) to uint32_t
and then call the subscr_get_by_tmsi() with converted value.
User input can be unexpected, so we should check/limit the length
and check if there is '0x' sequence or not.


С наилучшими пожеланиями,
Яницкий Вадим.

2016-03-29 22:39 GMT+06:00 Holger Freyther <holger@freyther.de>:

>
> > On 29 Mar 2016, at 18:10, Вадим Яницкий <axilirator@gmail.com> wrote:
> >
> > Hello!
> >
> > > thanks a lot, I am afraid we need one more round.
> >
> > No problem :)
> >
> > > -#define tmsi_from_string(str) strtoul(str, NULL, 10)
> > > +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
> > >
> > > this macro is used for parsing strings from the network. We should not
> modify it.
> >
> > I cannot find where...
>
>
> ah I see. You change the invocations in gsm_04_08.c. I would prefer if the
> name of this method is changed but once we work with uint32_t it will
> mostly go away anyway?
>
> holger
diff mbox

Patch

diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h
index 7d6c776..de80530 100644
--- a/openbsc/include/openbsc/gsm_subscriber.h
+++ b/openbsc/include/openbsc/gsm_subscriber.h
@@ -14,7 +14,7 @@ 
 
 #define GSM_SUBSCRIBER_FIRST_CONTACT	0x00000001
 /* gprs_sgsn.h defines additional flags including and above bit 16 (0x10000) */
-#define tmsi_from_string(str) strtoul(str, NULL, 10)
+#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
 
 #define GSM_SUBSCRIBER_NO_EXPIRATION	0x0
 
@@ -93,7 +93,7 @@  struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr);
 struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgrp,
 						const char *imsi);
 struct gsm_subscriber *subscr_get_by_tmsi(struct gsm_subscriber_group *sgrp,
-					  uint32_t tmsi);
+					  const char *tmsi);
 struct gsm_subscriber *subscr_get_by_imsi(struct gsm_subscriber_group *sgrp,
 					  const char *imsi);
 struct gsm_subscriber *subscr_get_by_extension(struct gsm_subscriber_group *sgrp,
@@ -104,7 +104,7 @@  struct gsm_subscriber *subscr_get_or_create(struct gsm_subscriber_group *sgrp,
 					const char *imsi);
 int subscr_update(struct gsm_subscriber *s, struct gsm_bts *bts, int reason);
 struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_subscriber_group *sgrp,
-					     uint32_t tmsi);
+					     const char *tmsi);
 struct gsm_subscriber *subscr_active_by_imsi(struct gsm_subscriber_group *sgrp,
 					     const char *imsi);
 
diff --git a/openbsc/src/libcommon/gsm_subscriber_base.c b/openbsc/src/libcommon/gsm_subscriber_base.c
index a455824..c7fb831 100644
--- a/openbsc/src/libcommon/gsm_subscriber_base.c
+++ b/openbsc/src/libcommon/gsm_subscriber_base.c
@@ -118,12 +118,13 @@  struct gsm_subscriber *subscr_get_or_create(struct gsm_subscriber_group *sgrp,
 }
 
 struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_subscriber_group *sgrp,
-					     uint32_t tmsi)
+					     const char *tmsi)
 {
 	struct gsm_subscriber *subscr;
+	uint8_t tmsi_val = tmsi_from_string(tmsi);
 
 	llist_for_each_entry(subscr, subscr_bsc_active_subscribers(), entry) {
-		if (subscr->tmsi == tmsi && subscr->group == sgrp)
+		if (subscr->tmsi == tmsi_val && subscr->group == sgrp)
 			return subscr_get(subscr);
 	}
 
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 0935fc5..56b5a08 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -47,7 +47,7 @@  static char *db_basename = NULL;
 static char *db_dirname = NULL;
 static dbi_conn conn;
 
-#define SCHEMA_REVISION "4"
+#define SCHEMA_REVISION "5"
 
 enum {
 	SCHEMA_META,
@@ -212,6 +212,7 @@  static int update_db_revision_2(void)
 	}
 	dbi_result_free(result);
 
+	LOGP(DDB, LOGL_NOTICE, "Migration complete.\n");
 	return 0;
 }
 
@@ -357,6 +358,7 @@  static int update_db_revision_3(void)
 	else
 		dbi_result_free(result);
 
+	LOGP(DDB, LOGL_NOTICE, "Migration complete.\n");
 	return 0;
 
 rollback:
@@ -369,6 +371,77 @@  rollback:
 	return -EINVAL;
 }
 
+static int update_db_revision_4(void)
+{
+	dbi_result select;
+	dbi_result update;
+	long long unsigned int id;
+	const char *tmsi_old;
+	uint32_t tmsi_new;
+
+	LOGP(DDB, LOGL_NOTICE, "Going to migrate from revision 4\n");
+
+	/* Cycle through old TMSIs and convert them to the new format */
+	select = dbi_conn_query(conn, "SELECT * FROM Subscriber");
+	if (!select) {
+		LOGP(DDB, LOGL_ERROR,
+			 "Failed fetch subscriber data the old Subscriber table "
+			 "(upgrade from rev 4).\n");
+		return -EINVAL;
+	}
+
+	while (dbi_result_next_row(select)) {
+		/* Fetch the subscriber ID */
+		id = dbi_result_get_ulonglong(select, "id");
+
+		/* Fetch an old TMSI value */
+		tmsi_old = dbi_result_get_string(select, "tmsi");
+		tmsi_new = atoi(tmsi_old);
+
+		if (tmsi_new <= 0) {
+			LOGP(DDB, LOGL_ERROR,
+				 "Failed to convert an old TMSI '%s', ignoring "
+				 "(upgrade from rev 4).\n", tmsi_old);
+			continue;
+		}
+
+		/* Update old TMSI */
+		update = dbi_conn_queryf(conn,
+				"UPDATE Subscriber "
+				"SET tmsi = '0x%08x' "
+				"WHERE id = %llu",
+				tmsi_new, id);
+
+		if (!update) {
+			LOGP(DDB, LOGL_ERROR,
+				 "Failed update subscriber's TMSI "
+				 "(upgrade from rev 4).\n");
+
+			dbi_result_free(select);
+			return -EINVAL;
+		}
+
+		dbi_result_free(update);
+	}
+	dbi_result_free(select);
+
+	/* We're done. Bump DB Meta revision to 5 */
+	update = dbi_conn_query(conn,
+				"UPDATE Meta "
+				"SET value = '5' "
+				"WHERE key = 'revision'");
+	if (!update) {
+		LOGP(DDB, LOGL_ERROR,
+			 "Failed to update DB schema revision "
+			 "(upgrade from rev 4).\n");
+		return -EINVAL;
+	}
+
+	dbi_result_free(update);
+	LOGP(DDB, LOGL_NOTICE, "Migration complete.\n");
+	return 0;
+}
+
 static int check_db_revision(void)
 {
 	dbi_result result;
@@ -400,6 +473,12 @@  static int check_db_revision(void)
 			dbi_result_free(result);
 			return -EINVAL;
 		}
+	} else if (!strcmp(rev_s, "4")) {
+		if (update_db_revision_4()) {
+			LOGP(DDB, LOGL_FATAL, "Failed to update database from schema revision '%s'.\n", rev_s);
+			dbi_result_free(result);
+			return -EINVAL;
+		}
 	} else if (!strcmp(rev_s, SCHEMA_REVISION)) {
 		/* everything is fine */
 	} else {
@@ -893,9 +972,10 @@  struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field,
 	subscr->id = dbi_result_get_ulonglong(result, "id");
 
 	db_set_from_query(subscr, result);
-	DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', TMSI %u, EXTEN '%s', LAC %hu, AUTH %u\n",
-		subscr->id, subscr->imsi, subscr->name, subscr->tmsi, subscr->extension,
-		subscr->lac, subscr->authorized);
+	DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', "
+		"TMSI 0x%08x, EXTEN '%s', LAC %hu, AUTH %u\n",
+		subscr->id, subscr->imsi, subscr->name, subscr->tmsi,
+		subscr->extension, subscr->lac, subscr->authorized);
 	dbi_result_free(result);
 
 	get_equipment_by_subscr(subscr);
@@ -935,7 +1015,7 @@  int db_subscriber_update(struct gsm_subscriber *subscr)
 int db_sync_subscriber(struct gsm_subscriber *subscriber)
 {
 	dbi_result result;
-	char tmsi[14];
+	char tmsi[11];
 	char *q_tmsi, *q_name, *q_extension;
 
 	dbi_conn_quote_string_copy(conn, 
@@ -944,7 +1024,7 @@  int db_sync_subscriber(struct gsm_subscriber *subscriber)
 				   subscriber->extension, &q_extension);
 	
 	if (subscriber->tmsi != GSM_RESERVED_TMSI) {
-		sprintf(tmsi, "%u", subscriber->tmsi);
+		sprintf(tmsi, "0x%08x", subscriber->tmsi);
 		dbi_conn_quote_string_copy(conn,
 				   tmsi,
 				   &q_tmsi);
@@ -1194,7 +1274,7 @@  int db_subscriber_expire(void *priv, void (*callback)(void *priv, long long unsi
 int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
 {
 	dbi_result result = NULL;
-	char tmsi[14];
+	char tmsi[11];
 	char *tmsi_quoted;
 
 	for (;;) {
@@ -1205,7 +1285,7 @@  int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
 		if (subscriber->tmsi == GSM_RESERVED_TMSI)
 			continue;
 
-		sprintf(tmsi, "%u", subscriber->tmsi);
+		sprintf(tmsi, "0x%08x", subscriber->tmsi);
 		dbi_conn_quote_string_copy(conn, tmsi, &tmsi_quoted);
 		result = dbi_conn_queryf(conn,
 			"SELECT * FROM Subscriber "
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index 1524ec4..c9f1b66 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -653,8 +653,7 @@  static int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb
 	case GSM_MI_TYPE_TMSI:
 		DEBUGPC(DMM, "\n");
 		/* look up the subscriber based on TMSI, request IMSI if it fails */
-		subscr = subscr_get_by_tmsi(bts->network->subscr_group,
-					    tmsi_from_string(mi_string));
+		subscr = subscr_get_by_tmsi(bts->network->subscr_group, mi_string);
 		if (!subscr) {
 			/* send IDENTITY REQUEST message to get IMSI */
 			mm_tx_identity_req(conn, GSM_MI_TYPE_IMSI);
@@ -972,20 +971,15 @@  static int gsm48_rx_mm_serv_req(struct gsm_subscriber_connection *conn, struct m
 
 	if (mi_type == GSM_MI_TYPE_IMSI) {
 		DEBUGPC(DMM, "serv_type=0x%02x MI(%s)=%s\n",
-			req->cm_service_type, gsm48_mi_type_name(mi_type),
-			mi_string);
-		subscr = subscr_get_by_imsi(bts->network->subscr_group,
-					    mi_string);
+			req->cm_service_type, gsm48_mi_type_name(mi_type), mi_string);
+		subscr = subscr_get_by_imsi(bts->network->subscr_group, mi_string);
 	} else if (mi_type == GSM_MI_TYPE_TMSI) {
 		DEBUGPC(DMM, "serv_type=0x%02x MI(%s)=%s\n",
-			req->cm_service_type, gsm48_mi_type_name(mi_type),
-			mi_string);
-		subscr = subscr_get_by_tmsi(bts->network->subscr_group,
-				tmsi_from_string(mi_string));
+			req->cm_service_type, gsm48_mi_type_name(mi_type), mi_string);
+		subscr = subscr_get_by_tmsi(bts->network->subscr_group, mi_string);
 	} else {
 		DEBUGPC(DMM, "mi_type is not expected: %d\n", mi_type);
-		return gsm48_tx_mm_serv_rej(conn,
-					    GSM48_REJECT_INCORRECT_MESSAGE);
+		return gsm48_tx_mm_serv_rej(conn, GSM48_REJECT_INCORRECT_MESSAGE);
 	}
 
 	osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, (classmark2 + classmark2_len));
@@ -1038,13 +1032,11 @@  static int gsm48_rx_mm_imsi_detach_ind(struct gsm_subscriber_connection *conn, s
 	switch (mi_type) {
 	case GSM_MI_TYPE_TMSI:
 		DEBUGPC(DMM, "\n");
-		subscr = subscr_get_by_tmsi(bts->network->subscr_group,
-					    tmsi_from_string(mi_string));
+		subscr = subscr_get_by_tmsi(bts->network->subscr_group, mi_string);
 		break;
 	case GSM_MI_TYPE_IMSI:
 		DEBUGPC(DMM, "\n");
-		subscr = subscr_get_by_imsi(bts->network->subscr_group,
-					    mi_string);
+		subscr = subscr_get_by_imsi(bts->network->subscr_group, mi_string);
 		break;
 	case GSM_MI_TYPE_IMEI:
 	case GSM_MI_TYPE_IMEISV:
@@ -1189,12 +1181,10 @@  static int gsm48_rx_rr_pag_resp(struct gsm_subscriber_connection *conn, struct m
 
 	switch (mi_type) {
 	case GSM_MI_TYPE_TMSI:
-		subscr = subscr_get_by_tmsi(bts->network->subscr_group,
-					    tmsi_from_string(mi_string));
+		subscr = subscr_get_by_tmsi(bts->network->subscr_group, mi_string);
 		break;
 	case GSM_MI_TYPE_IMSI:
-		subscr = subscr_get_by_imsi(bts->network->subscr_group,
-					    mi_string);
+		subscr = subscr_get_by_imsi(bts->network->subscr_group, mi_string);
 		break;
 	}
 
diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c
index 57c10cf..d9761d9 100644
--- a/openbsc/src/libmsc/gsm_subscriber.c
+++ b/openbsc/src/libmsc/gsm_subscriber.c
@@ -46,7 +46,6 @@  extern struct llist_head *subscr_bsc_active_subscribers(void);
 int gsm48_secure_channel(struct gsm_subscriber_connection *conn, int key_seq,
                          gsm_cbfn *cb, void *cb_data);
 
-
 /*
  * Struct for pending channel requests. This is managed in the
  * llist_head requests of each subscriber. The reference counting
@@ -212,19 +211,18 @@  struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgr
 }
 
 struct gsm_subscriber *subscr_get_by_tmsi(struct gsm_subscriber_group *sgrp,
-					  uint32_t tmsi)
+					  const char *tmsi)
 {
-	char tmsi_string[14];
 	struct gsm_subscriber *subscr;
+	uint32_t tmsi_val = tmsi_from_string(tmsi);
 
 	/* we might have a record in memory already */
 	llist_for_each_entry(subscr, subscr_bsc_active_subscribers(), entry) {
-		if (tmsi == subscr->tmsi)
+		if (tmsi_val == subscr->tmsi)
 			return subscr_get(subscr);
 	}
 
-	sprintf(tmsi_string, "%u", tmsi);
-	return get_subscriber(sgrp, GSM_SUBSCRIBER_TMSI, tmsi_string);
+	return get_subscriber(sgrp, GSM_SUBSCRIBER_TMSI, tmsi);
 }
 
 struct gsm_subscriber *subscr_get_by_imsi(struct gsm_subscriber_group *sgrp,
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c
index f49c53a..19c15e1 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -189,7 +189,7 @@  static struct gsm_subscriber *get_subscr_by_argv(struct gsm_network *gsmnet,
 	else if (!strcmp(type, "imsi"))
 		return subscr_get_by_imsi(gsmnet->subscr_group, id);
 	else if (!strcmp(type, "tmsi"))
-		return subscr_get_by_tmsi(gsmnet->subscr_group, atoi(id));
+		return subscr_get_by_tmsi(gsmnet->subscr_group, id);
 	else if (!strcmp(type, "id"))
 		return subscr_get_by_id(gsmnet->subscr_group, atoi(id));
 
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_filter.c b/openbsc/src/osmo-bsc/osmo_bsc_filter.c
index 14e0b71..88db15e 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_filter.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_filter.c
@@ -79,7 +79,7 @@  static struct gsm_subscriber *extract_sub(struct gsm_subscriber_connection *conn
 	switch (mi_type) {
 	case GSM_MI_TYPE_TMSI:
 		subscr = subscr_active_by_tmsi(conn->bts->network->subscr_group,
-					       tmsi_from_string(mi_string));
+					       mi_string);
 		break;
 	case GSM_MI_TYPE_IMSI:
 		subscr = subscr_active_by_imsi(conn->bts->network->subscr_group,
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_vty.c b/openbsc/src/osmo-bsc/osmo_bsc_vty.c
index e623c9c..d871f01 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_vty.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_vty.c
@@ -43,7 +43,7 @@  static struct osmo_bsc_data *osmo_bsc_data(struct vty *vty)
 
 static struct osmo_msc_data *osmo_msc_data(struct vty *vty)
 {
-	return vty->index;
+	return osmo_msc_data_find(bsc_gsmnet, (int) vty->index);
 }
 
 static struct cmd_node bsc_node = {
@@ -70,7 +70,7 @@  DEFUN(cfg_net_msc, cfg_net_msc_cmd,
 		return CMD_WARNING;
 	}
 
-	vty->index = msc;
+	vty->index = (void *) index;
 	vty->node = MSC_NODE;
 	return CMD_SUCCESS;
 }
diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c
index a02d1f8..faea820 100644
--- a/openbsc/tests/db/db_test.c
+++ b/openbsc/tests/db/db_test.c
@@ -200,7 +200,7 @@  int main()
 	alice->lac=42;
 	db_sync_subscriber(alice);
 	/* Get by TMSI */
-	snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
+	snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi);
 	alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
 	COMPARE(alice, alice_db);
 	SUBSCR_PUT(alice_db);
@@ -227,7 +227,7 @@  int main()
 	db_subscriber_assoc_imei(alice, "1234567890");
 	db_subscriber_assoc_imei(alice, "6543560920");
 	/* Get by TMSI */
-	snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
+	snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi);
 	alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
 	COMPARE(alice, alice_db);
 	SUBSCR_PUT(alice_db);
diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c
index 781ef61..8ed57ca 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.c
+++ b/openbsc/tests/gsm0408/gsm0408_test.c
@@ -93,7 +93,7 @@  static void test_mi_functionality(void)
 	/* tmsi code */
 	mi_len = gsm48_generate_mid_from_tmsi(mi, tmsi);
 	gsm48_mi_to_string(mi_parsed, sizeof(mi_parsed), mi + 2, mi_len - 2);
-	COMPARE((uint32_t)strtoul(mi_parsed, NULL, 10), ==, tmsi);
+	COMPARE((uint32_t)tmsi_from_string(mi_parsed), ==, tmsi);
 
 	/* imsi code */
 	mi_len = gsm48_generate_mid_from_imsi(mi, imsi_odd);