22 Mar
2016
22 Mar
'16
2:54 p.m.
I just fixed some things (gsm_subscriber.c, VTY tmsi parsing)
and did some tests. It works!
Location Update Request with TMSI from previous network:
<0002> gsm_04_08.c:1127 LOCATION UPDATING REQUEST: MI(TMSI)=0x68044a6c
type=NORMAL
<0001> gsm_04_08.c:144 (bts 0 trx 0 ts 0 pd 05) Sending 0x18 to MS.
<0001> gsm_04_08.c:144 (bts 0 trx 0 ts 0 pd 05) Sending 0x18 to MS.
Outgoing SMS from OpenBSC VTY:
OpenBSC> subscriber tmsi 0xb7f861b6 sms sender tmsi 0xb7f861b6 send TEST
<0002> gsm_subscriber.c:175 Subscriber <IMSI> not paged yet.
<0004> abis_rsl.c:1465 (bts=0,trx=0,ts=0,ss=0) Activating ARFCN(***) SS(0)
lctype SDCCH r=OTHER ra=0x1a ta=1
<0004> abis_rsl.c:1199 (bts=0,trx=0,ts=0,ss=0) CHANNEL ACTIVATE ACK
<0000> abis_rsl.c:1653 (bts=0,trx=0,ts=0,ss=0) SAPI=0 ESTABLISH INDICATION
<0000> gsm_04_08.c:3573 Dispatching 04.08 message, pdisc=6
<0003> gsm_04_08.c:1180 PAGING RESPONSE: MI(TMSI)=0xb7f861b6
<0003> gsm_04_08.c:1198 <- Channel was requested by <IMSI>
<0003> gsm_04_08.c:1259 TX APPLICATION INFO id=0x00, len=4
<0001> gsm_04_08.c:144 (bts 0 trx 0 ts 0 pd 06) Sending 0x38 to MS.
<0001> transaction.c:71 subscr=0x2363f00, net=0x2333e90
USSD request also works:
<0002> gsm_04_08.c:956 <- CM SERVICE REQUEST serv_type=0x08
MI(TMSI)=0xb7f861b6
<0002> gsm_04_08_utils.c:692 -> CM SERVICE ACK
As you can see, now TMSI displays in hex.
I'll provide a new patch soon.
С наилучшими пожеланиями,
Яницкий Вадим.
2016-03-22 18:58 GMT+06:00 Вадим Яницкий <axilirator(a)gmail.com>om>:
Hello, Harald! Good day, Holger!
It looks like SQLite3 doesn't have support of unsigned 64-bit integers. :(
Of course, we can write some custom functions, which can emulate it,
but it isn't good solution, I think.
My suggestion is to keep the TMSI column type in string format: 0xffffffff.
I need to know your opinions before starting to write a new patch.
С наилучшими пожеланиями,
Яницкий Вадим.
2016-03-17 22:34 GMT+06:00 Holger Freyther <holger(a)freyther.de>de>:
someone
it happens. thanks for contributing
looking forward for the follow up.
holger
On 17 Mar 2016, at 17:21, Вадим Яницкий
<axilirator(a)gmail.com> wrote:
Hi Guys!
> If you have time, please check if there are other occurrences in
OpenBSC
or
OsmocomBB where the TMSI is printed as integer. Thanks!
No problem! :)
> I think it was a bit too quick. I foresee one problem. Let's assume > is using TMSIs and now upgrade the
sourcecode. All CM Service Requests
> will fail because the TMSI is not known. What is the
migration/mitigation
plan?
I absolutely agree with Holger. Maybe we can add some code that will
check
if database still stores TMSIs in old
representation style and convert
them to
uint32_t? We can change the
libmsc/db.c:db_prepare() for this purpose.
yes, we have a schema version and can just increase it. E.g. have a look
at how we migrate SMS.
tmsi_from_string will not work for this anymore.
Yes, I forgot to change the gsm_subscriber.c ... Sorry.
> there is no length check but that doesn't seem to be a big issue
right
now.
We can just write a function that will do this check instead of using
#define.
I think you will not need to touch this define at all. We might want to
change the name to _from_mi_string.
* a DB schema upgrade and store the TMSI as
uint32_t
* Use hex presentation in VTY
+1
27 Mar
27 Mar
2:17 p.m.
New subject: [PATCH] move to hex TMSI representation
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(a)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(-)
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);
--
2.7.4
6:44 p.m.
New subject: [PATCH] move to hex TMSI representation
On 27 Mar 2016, at 14:17, Vadim Yanitskiy
<axilirator(a)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
29 Mar
29 Mar
6:10 p.m.
New subject: [PATCH] move to hex TMSI representation
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(a)freyther.de>de>:
>
> > On 27 Mar 2016, at 14:17, Vadim Yanitskiy <axilirator(a)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
6:39 p.m.
New subject: [PATCH] move to hex TMSI representation
On 29 Mar 2016, at 18:10, Вадим Яницкий
<axilirator(a)gmail.com> wrote:
Hello!
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
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...
8:42 p.m.
New subject: [PATCH] move to hex TMSI representation
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(a)freyther.de>de>:
On 29 Mar 2016, at 18:10, Вадим Яницкий
<axilirator(a)gmail.com> wrote:
Hello!
modify
it.
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
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
31 Mar
31 Mar
9:23 a.m.
New subject: [PATCH] move to hex TMSI representation
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
and should be applied with corresponding patch for libosmocore.
- Use hex representation in VTY
- Increased DB SCHEMA_REVISION
- Implemented DB migration code
db_test is temporary broken because incremental migration
isn't implemented yet (WIP).
Signed-off-by: Vadim Yanitskiy <axilirator(a)gmail.com>
---
openbsc/include/openbsc/gsm_subscriber.h | 2 +-
openbsc/src/libmsc/db.c | 155 +++++++++++++++++++++++++-----
openbsc/src/libmsc/vty_interface_layer3.c | 3 +-
openbsc/tests/gsm0408/gsm0408_test.c | 2 +-
4 files changed, 133 insertions(+), 29 deletions(-)
diff --git a/openbsc/include/openbsc/gsm_subscriber.h
b/openbsc/include/openbsc/gsm_subscriber.h
index 7d6c776..785dc36 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
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 0935fc5..04aee79 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,
@@ -83,7 +83,7 @@ static const char *create_stmts[] = {
"name TEXT, "
"extension TEXT UNIQUE, "
"authorized INTEGER NOT NULL DEFAULT 0, "
- "tmsi TEXT UNIQUE, "
+ "tmsi INTEGER UNIQUE, "
"lac INTEGER NOT NULL DEFAULT 0, "
"expire_lu TIMESTAMP DEFAULT NULL"
")",
@@ -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,108 @@ rollback:
return -EINVAL;
}
+static int update_db_revision_4(void)
+{
+ dbi_result result;
+
+ LOGP(DDB, LOGL_NOTICE, "Going to migrate from revision 4\n");
+
+ result = dbi_conn_query(conn, "BEGIN EXCLUSIVE TRANSACTION");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to begin transaction "
+ "(upgrade from rev 4)\n");
+ return -EINVAL;
+ }
+ dbi_result_free(result);
+
+ /* Rename old Subscriber table to be able create a new one */
+ result = dbi_conn_query(conn,
+ "ALTER TABLE Subscriber RENAME TO Subscriber_4");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to rename the old Subscriber table "
+ "(upgrade from rev 4).\n");
+ goto rollback;
+ }
+ dbi_result_free(result);
+
+ /* Create new Subscriber table */
+ result = dbi_conn_query(conn, create_stmts[SCHEMA_SUBSCRIBER]);
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to create a new Subscriber table "
+ "(upgrade from rev 4).\n");
+ goto rollback;
+ }
+ dbi_result_free(result);
+
+ /* Copy subscriber data into the new table */
+ result = dbi_conn_query(conn,
+ "INSERT INTO Subscriber "
+ "SELECT * FROM Subscriber_4");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to copy subscriber data into the new table "
+ "(upgrade from rev 4).\n");
+ goto rollback;
+ }
+ dbi_result_free(result);
+
+ /* Remove the temporary table */
+ result = dbi_conn_query(conn, "DROP TABLE Subscriber_4");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to drop the old Subscriber table "
+ "(upgrade from rev 4).\n");
+ goto rollback;
+ }
+ dbi_result_free(result);
+
+ /* We're done. Bump DB Meta revision to 5 */
+ result = dbi_conn_query(conn,
+ "UPDATE Meta "
+ "SET value = '5' "
+ "WHERE key = 'revision'");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to update DB schema revision "
+ "(upgrade from rev 4).\n");
+ goto rollback;
+ }
+ dbi_result_free(result);
+
+ result = dbi_conn_query(conn, "COMMIT TRANSACTION");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to commit the transaction "
+ "(upgrade from rev 4)\n");
+ return -EINVAL;
+ }
+
+ /* Shrink DB file size by actually wiping out Subscriber_4 table data */
+ result = dbi_conn_query(conn, "VACUUM");
+ if (!result)
+ LOGP(DDB, LOGL_ERROR,
+ "VACUUM failed. Ignoring it "
+ "(upgrade from rev 4).\n");
+ else
+ dbi_result_free(result);
+
+ LOGP(DDB, LOGL_NOTICE, "Migration complete.\n");
+ return 0;
+
+rollback:
+ result = dbi_conn_query(conn, "ROLLBACK TRANSACTION");
+ if (!result)
+ LOGP(DDB, LOGL_ERROR,
+ "Rollback failed "
+ "(upgrade from rev 4).\n");
+ else
+ dbi_result_free(result);
+ return -EINVAL;
+}
+
static int check_db_revision(void)
{
dbi_result result;
@@ -388,20 +492,18 @@ static int check_db_revision(void)
dbi_result_free(result);
return -EINVAL;
}
- if (!strcmp(rev_s, "2")) {
- if (update_db_revision_2()) {
- 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, "3")) {
- if (update_db_revision_3()) {
- 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)) {
+
+ if (!strcmp(rev_s, SCHEMA_REVISION)) {
/* everything is fine */
+ } else if (!strcmp(rev_s, "2")) {
+ if (update_db_revision_2())
+ goto error;
+ } else if (!strcmp(rev_s, "3")) {
+ if (update_db_revision_3())
+ goto error;
+ } else if (!strcmp(rev_s, "4")) {
+ if (update_db_revision_4())
+ goto error;
} else {
LOGP(DDB, LOGL_FATAL, "Invalid database schema revision '%s'.\n",
rev_s);
dbi_result_free(result);
@@ -410,6 +512,11 @@ static int check_db_revision(void)
dbi_result_free(result);
return 0;
+
+error:
+ LOGP(DDB, LOGL_FATAL, "Failed to update database from schema revision
'%s'.\n", rev_s);
+ dbi_result_free(result);
+ return -EINVAL;
}
static int db_configure(void)
@@ -808,10 +915,6 @@ static void db_set_from_query(struct gsm_subscriber *subscr, dbi_conn
result)
if (string)
strncpy(subscr->imsi, string, GSM_IMSI_LENGTH-1);
- string = dbi_result_get_string(result, "tmsi");
- if (string)
- subscr->tmsi = tmsi_from_string(string);
-
string = dbi_result_get_string(result, "name");
if (string)
strncpy(subscr->name, string, GSM_NAME_LENGTH);
@@ -820,7 +923,8 @@ static void db_set_from_query(struct gsm_subscriber *subscr, dbi_conn
result)
if (string)
strncpy(subscr->extension, string, GSM_EXTENSION_LENGTH);
- subscr->lac = dbi_result_get_ulonglong(result, "lac");
+ subscr->tmsi = dbi_result_get_ulonglong(result, "tmsi");
+ subscr->lac = dbi_result_get_ulonglong(result, "lac");
if (!dbi_result_field_is_null(result, "expire_lu"))
subscr->expire_lu = dbi_result_get_datetime(result, "expire_lu");
@@ -828,7 +932,6 @@ static void db_set_from_query(struct gsm_subscriber *subscr, dbi_conn
result)
subscr->expire_lu = GSM_SUBSCRIBER_NO_EXPIRATION;
subscr->authorized = dbi_result_get_ulonglong(result, "authorized");
-
}
#define BASE_QUERY "SELECT * FROM Subscriber "
@@ -893,9 +996,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);
@@ -998,7 +1102,6 @@ int db_sync_subscriber(struct gsm_subscriber *subscriber)
}
dbi_result_free(result);
-
return 0;
}
@@ -1225,7 +1328,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
}
if (!dbi_result_next_row(result)) {
dbi_result_free(result);
- DEBUGP(DDB, "Allocated TMSI %u for IMSI %s.\n",
+ DEBUGP(DDB, "Allocated TMSI 0x%08x for IMSI %s.\n",
subscriber->tmsi, subscriber->imsi);
return db_sync_subscriber(subscriber);
}
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c
b/openbsc/src/libmsc/vty_interface_layer3.c
index f49c53a..9879a53 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -189,7 +189,8 @@ 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,
+ tmsi_from_string(id));
else if (!strcmp(type, "id"))
return subscr_get_by_id(gsmnet->subscr_group, atoi(id));
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);
--
2.8.0
2969
days inactive
2978
days old
6 comments
3 participants
participants (3)
-
Holger Freyther -
Vadim Yanitskiy -
Вадим Яницкий