From: Holger Hans Peter Freyther zecke@selfish.org
This is an incompatible database schema change. Store the type of the address in the database for both the sender and the receiver.
Currently it is possible to use SMPP to store a SMS and the NPI and TON will be lost on the delivery of the SMS. The schema is changed to make the delivery always use the right NPI/TON. This patch is not ready for the master branch as there is no upgrade path for the HLR yet. --- openbsc/include/openbsc/gsm_data.h | 1 - openbsc/src/libmsc/db.c | 62 +++++++++++++++++++++++--------------- openbsc/src/libmsc/gsm_04_11.c | 9 ++---- openbsc/src/libmsc/smpp_openbsc.c | 5 ++- 4 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/openbsc/include/openbsc/gsm_data.h b/openbsc/include/openbsc/gsm_data.h index 1b4720f..e6a94ad 100644 --- a/openbsc/include/openbsc/gsm_data.h +++ b/openbsc/include/openbsc/gsm_data.h @@ -304,7 +304,6 @@ struct gsm_sms_addr {
struct gsm_sms { unsigned long long id; - struct gsm_subscriber *sender; struct gsm_subscriber *receiver; struct gsm_sms_addr src, dst; enum gsm_sms_source_id source; diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index e26d3c5..39812c1 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -95,7 +95,6 @@ static char *create_stmts[] = { "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "sent TIMESTAMP, " - "sender_id INTEGER NOT NULL, " "receiver_id INTEGER NOT NULL, " "deliver_attempts INTEGER NOT NULL DEFAULT 0, " /* data directly copied/derived from SMS */ @@ -105,7 +104,12 @@ static char *create_stmts[] = { "protocol_id INTEGER NOT NULL, " "data_coding_scheme INTEGER NOT NULL, " "ud_hdr_ind INTEGER NOT NULL, " - "dest_addr TEXT, " + "src_addr TEXT NOT NULL, " + "src_ton INTEGER NOT NULL, " + "src_npi INTEGER NOT NULL, " + "dest_addr TEXT NOT NULL, " + "dest_ton INTEGER NOT NULL, " + "dest_npi INTEGER NOT NULL, " "user_data BLOB, " /* TP-UD */ /* additional data, interpreted from SMS */ "header BLOB, " /* UD Header */ @@ -1094,7 +1098,7 @@ int db_subscriber_assoc_imei(struct gsm_subscriber *subscriber, char imei[GSM_IM int db_sms_store(struct gsm_sms *sms) { dbi_result result; - char *q_text, *q_daddr; + char *q_text, *q_daddr, *q_saddr; unsigned char *q_udata; char *validity_timestamp = "2222-2-2";
@@ -1102,25 +1106,35 @@ int db_sms_store(struct gsm_sms *sms)
dbi_conn_quote_string_copy(conn, (char *)sms->text, &q_text); dbi_conn_quote_string_copy(conn, (char *)sms->dst.addr, &q_daddr); + dbi_conn_quote_string_copy(conn, (char *)sms->src.addr, &q_saddr); dbi_conn_quote_binary_copy(conn, sms->user_data, sms->user_data_len, &q_udata); + /* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " - "(created, sender_id, receiver_id, valid_until, " + "(created, receiver_id, valid_until, " "reply_path_req, status_rep_req, protocol_id, " - "data_coding_scheme, ud_hdr_ind, dest_addr, " - "user_data, text) VALUES " - "(datetime('now'), %llu, %llu, %u, " - "%u, %u, %u, %u, %u, %s, %s, %s)", - sms->sender->id, + "data_coding_scheme, ud_hdr_ind, " + "user_data, text, " + "dest_addr, dest_ton, dest_npi, " + "src_addr, src_ton, src_npi) VALUES " + "(datetime('now'), %llu, %u, " + "%u, %u, %u, " + "%u, %u, " + "%s, %s, " + "%s, %u, %u, " + "%s, %u, %u)", sms->receiver ? sms->receiver->id : 0, validity_timestamp, sms->reply_path_req, sms->status_rep_req, sms->protocol_id, sms->data_coding_scheme, sms->ud_hdr_ind, - q_daddr, q_udata, q_text); + q_udata, q_text, + q_daddr, sms->dst.ton, sms->dst.npi, + q_saddr, sms->src.ton, sms->src.npi); free(q_text); - free(q_daddr); free(q_udata); + free(q_daddr); + free(q_saddr);
if (!result) return -EIO; @@ -1132,8 +1146,8 @@ int db_sms_store(struct gsm_sms *sms) static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result result) { struct gsm_sms *sms = sms_alloc(); - long long unsigned int sender_id, receiver_id; - const char *text, *daddr; + long long unsigned int receiver_id; + const char *text, *daddr, *saddr; const unsigned char *user_data;
if (!sms) @@ -1141,18 +1155,6 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul
sms->id = dbi_result_get_ulonglong(result, "id");
- sender_id = dbi_result_get_ulonglong(result, "sender_id"); - sms->sender = subscr_get_by_id(net, sender_id); - if (!sms->sender) { - LOGP(DLSMS, LOGL_ERROR, - "Failed to find sender(%llu) for id(%llu)\n", - sender_id, sms->id); - sms_free(sms); - return NULL; - } - - strncpy(sms->src.addr, sms->sender->extension, sizeof(sms->src.addr)-1); - receiver_id = dbi_result_get_ulonglong(result, "receiver_id"); sms->receiver = subscr_get_by_id(net, receiver_id); if (!sms->receiver) { @@ -1173,12 +1175,22 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul "data_coding_scheme"); /* sms->msg_ref is temporary and not stored in DB */
+ sms->dst.npi = dbi_result_get_uint(result, "dest_npi"); + sms->dst.ton = dbi_result_get_uint(result, "dest_ton"); daddr = dbi_result_get_string(result, "dest_addr"); if (daddr) { strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr)); sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0'; }
+ sms->src.npi = dbi_result_get_uint(result, "src_npi"); + sms->src.ton = dbi_result_get_uint(result, "src_ton"); + saddr = dbi_result_get_string(result, "src_addr"); + if (saddr) { + strncpy(sms->src.addr, saddr, sizeof(sms->src.addr)); + sms->src.addr[sizeof(sms->src.addr)-1] = '\0'; + } + sms->user_data_len = dbi_result_get_field_length(result, "user_data"); user_data = dbi_result_get_binary(result, "user_data"); if (sms->user_data_len > sizeof(sms->user_data)) diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index 975a263..566feb9 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -73,8 +73,6 @@ struct gsm_sms *sms_alloc(void) void sms_free(struct gsm_sms *sms) { /* drop references to subscriber structure */ - if (sms->sender) - subscr_put(sms->sender); if (sms->receiver) subscr_put(sms->receiver); #ifdef BUILD_SMPP @@ -97,8 +95,7 @@ struct gsm_sms *sms_from_text(struct gsm_subscriber *receiver, sms->receiver = subscr_get(receiver); strncpy(sms->text, text, sizeof(sms->text)-1);
- sms->sender = subscr_get(sender); - strncpy(sms->src.addr, sms->sender->extension, sizeof(sms->src.addr)-1); + strncpy(sms->src.addr, sender->extension, sizeof(sms->src.addr)-1); sms->reply_path_req = 0; sms->status_rep_req = 0; sms->ud_hdr_ind = 0; @@ -378,12 +375,10 @@ static int gsm340_rx_tpdu(struct gsm_subscriber_connection *conn, struct msgb *m } }
- gsms->sender = subscr_get(conn->subscr); - LOGP(DLSMS, LOGL_INFO, "RX SMS: Sender: %s, MTI: 0x%02x, VPF: 0x%02x, " "MR: 0x%02x PID: 0x%02x, DCS: 0x%02x, DA: %s, " "UserDataLength: 0x%02x, UserData: "%s"\n", - subscr_name(gsms->sender), sms_mti, sms_vpf, gsms->msg_ref, + subscr_name(conn->subscr), sms_mti, sms_vpf, gsms->msg_ref, gsms->protocol_id, gsms->data_coding_scheme, gsms->dst.addr, gsms->user_data_len, sms_alphabet == DCS_7BIT_DEFAULT ? gsms->text : diff --git a/openbsc/src/libmsc/smpp_openbsc.c b/openbsc/src/libmsc/smpp_openbsc.c index ab558ce..ec541c2 100644 --- a/openbsc/src/libmsc/smpp_openbsc.c +++ b/openbsc/src/libmsc/smpp_openbsc.c @@ -133,7 +133,6 @@ static int submit_to_sms(struct gsm_sms **psms, struct gsm_network *net, strncpy(sms->dst.addr, dest->extension, sizeof(sms->dst.addr)-1);
/* fill in the source address */ - sms->sender = subscr_get_by_id(net, 1); sms->src.ton = submit->source_addr_ton; sms->src.npi = submit->source_addr_npi; strncpy(sms->src.addr, (char *)submit->source_addr, sizeof(sms->src.addr)-1); @@ -475,13 +474,13 @@ static int deliver_to_esme(struct osmo_esme *esme, struct gsm_sms *sms, deliver.source_addr_npi = NPI_Land_Mobile_E212; snprintf((char *)deliver.source_addr, sizeof(deliver.source_addr), "%s", - sms->sender->imsi); + conn->subscr->imsi); } else { deliver.source_addr_ton = TON_Network_Specific; deliver.source_addr_npi = NPI_ISDN_E163_E164; snprintf((char *)deliver.source_addr, sizeof(deliver.source_addr), "%s", - sms->sender->extension); + conn->subscr->extension); }
deliver.dest_addr_ton = sms->dst.ton;
From: Holger Hans Peter Freyther holger@moiji-mobile.com
Use the already created subscriber, create a sms and read it back from the subscriber. --- openbsc/tests/db/db_test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c index 3c5de90..ba3fdfb 100644 --- a/openbsc/tests/db/db_test.c +++ b/openbsc/tests/db/db_test.c @@ -20,6 +20,7 @@ #include <openbsc/debug.h> #include <openbsc/db.h> #include <openbsc/gsm_subscriber.h> +#include <openbsc/gsm_04_11.h>
#include <osmocom/core/application.h>
@@ -57,6 +58,64 @@ static struct gsm_network dummy_net; printf("Extensions do not match in %s:%d '%s' '%s'\n", \ __FUNCTION__, __LINE__, original->extension, copy->extension); \
+/* + * Create/Store a SMS and then try to load it. + */ +static void test_sms(void) +{ + int rc; + struct gsm_sms *sms; + struct gsm_subscriber *subscr; + subscr = db_get_subscriber(GSM_SUBSCRIBER_IMSI, "9993245423445"); + OSMO_ASSERT(subscr); + subscr->net = &dummy_net; + + sms = sms_alloc(); + sms->receiver = subscr_get(subscr); + + sms->src.ton = 0x23; + sms->src.npi = 0x24; + memcpy(sms->src.addr, "1234", strlen("1234") + 1); + + sms->dst.ton = 0x32; + sms->dst.npi = 0x42; + memcpy(sms->dst.addr, subscr->extension, sizeof(subscr->extension)); + + memcpy(sms->text, "Text123", strlen("Text123") + 1); + memcpy(sms->user_data, "UserData123", strlen("UserData123") + 1); + sms->user_data_len = strlen("UserData123"); + + /* random values */ + sms->reply_path_req = 1; + sms->status_rep_req = 2; + sms->ud_hdr_ind = 3; + sms->protocol_id = 4; + sms->data_coding_scheme = 5; + + rc = db_sms_store(sms); + sms_free(sms); + OSMO_ASSERT(rc == 0); + + /* now query */ + sms = db_sms_get_unsent_for_subscr(subscr); + OSMO_ASSERT(sms); + OSMO_ASSERT(sms->receiver == subscr); + OSMO_ASSERT(sms->reply_path_req == 1); + OSMO_ASSERT(sms->status_rep_req == 2); + OSMO_ASSERT(sms->ud_hdr_ind == 3); + OSMO_ASSERT(sms->protocol_id == 4); + OSMO_ASSERT(sms->data_coding_scheme == 5); + OSMO_ASSERT(sms->src.ton == 0x23); + OSMO_ASSERT(sms->src.npi == 0x24); + OSMO_ASSERT(sms->dst.ton == 0x32); + OSMO_ASSERT(sms->dst.npi == 0x42); + OSMO_ASSERT(strcmp((char *) sms->text, "Text123") == 0); + OSMO_ASSERT(sms->user_data_len == strlen("UserData123")); + OSMO_ASSERT(strcmp((char *) sms->user_data, "UserData123") == 0); + + subscr_put(subscr); +} + int main() { printf("Testing subscriber database code.\n"); @@ -108,6 +167,8 @@ int main() SUBSCR_PUT(alice); SUBSCR_PUT(alice_db);
+ test_sms(); + db_fini();
printf("Done\n");
From: Alexander Chemeris alexander.chemeris@gmail.com
That was a bad idea from the very beginning. A visible result of this is a wrong SMS routing when you change subscriber extensions, while having queued SMS. It's also a very wrong thing from the code layering perspective.
I think the next logical step should be to remove "receiver" pointer from the gsm_sms structure into a structure, special for the internal SMS queue. --- openbsc/src/libmsc/db.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 39812c1..2351485 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -95,7 +95,6 @@ static char *create_stmts[] = { "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "sent TIMESTAMP, " - "receiver_id INTEGER NOT NULL, " "deliver_attempts INTEGER NOT NULL DEFAULT 0, " /* data directly copied/derived from SMS */ "valid_until TIMESTAMP, " @@ -1113,19 +1112,19 @@ int db_sms_store(struct gsm_sms *sms) /* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " - "(created, receiver_id, valid_until, " + "(created, valid_until, " "reply_path_req, status_rep_req, protocol_id, " "data_coding_scheme, ud_hdr_ind, " "user_data, text, " "dest_addr, dest_ton, dest_npi, " "src_addr, src_ton, src_npi) VALUES " - "(datetime('now'), %llu, %u, " + "(datetime('now'), %u, " "%u, %u, %u, " "%u, %u, " "%s, %s, " "%s, %u, %u, " "%s, %u, %u)", - sms->receiver ? sms->receiver->id : 0, validity_timestamp, + validity_timestamp, sms->reply_path_req, sms->status_rep_req, sms->protocol_id, sms->data_coding_scheme, sms->ud_hdr_ind, q_udata, q_text, @@ -1146,7 +1145,6 @@ int db_sms_store(struct gsm_sms *sms) static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result result) { struct gsm_sms *sms = sms_alloc(); - long long unsigned int receiver_id; const char *text, *daddr, *saddr; const unsigned char *user_data;
@@ -1155,16 +1153,6 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul
sms->id = dbi_result_get_ulonglong(result, "id");
- receiver_id = dbi_result_get_ulonglong(result, "receiver_id"); - sms->receiver = subscr_get_by_id(net, receiver_id); - if (!sms->receiver) { - LOGP(DLSMS, LOGL_ERROR, - "Failed to find receiver(%llu) for id(%llu)\n", - receiver_id, sms->id); - sms_free(sms); - return NULL; - } - /* FIXME: validity */ /* FIXME: those should all be get_uchar, but sqlite3 is braindead */ sms->reply_path_req = dbi_result_get_uint(result, "reply_path_req"); @@ -1182,6 +1170,7 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr)); sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0'; } + sms->receiver = subscr_get_by_extension(net, sms->dst.addr);
sms->src.npi = dbi_result_get_uint(result, "src_npi"); sms->src.ton = dbi_result_get_uint(result, "src_ton"); @@ -1236,7 +1225,7 @@ struct gsm_sms *db_sms_get_unsent(struct gsm_network *net, unsigned long long mi result = dbi_conn_queryf(conn, "SELECT SMS.* " "FROM SMS JOIN Subscriber ON " - "SMS.receiver_id = Subscriber.id " + "SMS.dest_addr = Subscriber.extension " "WHERE SMS.id >= %llu AND SMS.sent IS NULL " "AND Subscriber.lac > 0 " "ORDER BY SMS.id LIMIT 1", @@ -1266,10 +1255,10 @@ struct gsm_sms *db_sms_get_unsent_by_subscr(struct gsm_network *net, result = dbi_conn_queryf(conn, "SELECT SMS.* " "FROM SMS JOIN Subscriber ON " - "SMS.receiver_id = Subscriber.id " - "WHERE SMS.receiver_id >= %llu AND SMS.sent IS NULL " + "SMS.dest_addr = Subscriber.extension " + "WHERE Subscriber.id >= %llu AND SMS.sent IS NULL " "AND Subscriber.lac > 0 AND SMS.deliver_attempts < %u " - "ORDER BY SMS.receiver_id, SMS.id LIMIT 1", + "ORDER BY Subscriber.id, SMS.id LIMIT 1", min_subscr_id, failed); if (!result) return NULL; @@ -1295,8 +1284,8 @@ struct gsm_sms *db_sms_get_unsent_for_subscr(struct gsm_subscriber *subscr) result = dbi_conn_queryf(conn, "SELECT SMS.* " "FROM SMS JOIN Subscriber ON " - "SMS.receiver_id = Subscriber.id " - "WHERE SMS.receiver_id = %llu AND SMS.sent IS NULL " + "SMS.dest_addr = Subscriber.extension " + "WHERE Subscriber.id = %llu AND SMS.sent IS NULL " "AND Subscriber.lac > 0 " "ORDER BY SMS.id LIMIT 1", subscr->id);
From: Holger Hans Peter Freyther holger@moiji-mobile.com
This is mostly based on Alexander's migration code. The code adds transaction handling and some sanity checks and cleanups to the code. We made the decision to fork the sms_from_result method and freeze it to that version. This way sms_from_result can move forward without having to deal with legacy. --- openbsc/src/libbsc/gsm_subscriber_base.c | 6 + openbsc/src/libmsc/db.c | 209 ++++++++++++++++++++++++++++--- 2 files changed, 199 insertions(+), 16 deletions(-)
diff --git a/openbsc/src/libbsc/gsm_subscriber_base.c b/openbsc/src/libbsc/gsm_subscriber_base.c index 5755687..5e00443 100644 --- a/openbsc/src/libbsc/gsm_subscriber_base.c +++ b/openbsc/src/libbsc/gsm_subscriber_base.c @@ -72,6 +72,12 @@ static void subscr_free(struct gsm_subscriber *subscr) talloc_free(subscr); }
+void subscr_direct_free(struct gsm_subscriber *subscr) +{ + OSMO_ASSERT(subscr->use_count == 1); + subscr_free(subscr); +} + struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr) { subscr->use_count++; diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 2351485..3b92691 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -38,23 +38,42 @@ #include <osmocom/core/statistics.h> #include <osmocom/core/rate_ctr.h>
+/* Semi-Private-Interface (SPI) for the subscriber code */ +void subscr_direct_free(struct gsm_subscriber *subscr); + static char *db_basename = NULL; static char *db_dirname = NULL; static dbi_conn conn;
-#define SCHEMA_REVISION "3" +#define SCHEMA_REVISION "4" + +enum { + SCHEMA_META, + INSERT_META, + SCHEMA_SUBSCRIBER, + SCHEMA_AUTH, + SCHEMA_EQUIPMENT, + SCHEMA_EQUIPMENT_WATCH, + SCHEMA_SMS, + SCHEMA_VLR, + SCHEMA_APDU, + SCHEMA_COUNTERS, + SCHEMA_RATE, + SCHEMA_AUTHKEY, + SCHEMA_AUTHLAST, +};
-static char *create_stmts[] = { - "CREATE TABLE IF NOT EXISTS Meta (" +static const char *create_stmts[] = { + [SCHEMA_META] = "CREATE TABLE IF NOT EXISTS Meta (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "key TEXT UNIQUE NOT NULL, " "value TEXT NOT NULL" ")", - "INSERT OR IGNORE INTO Meta " + [INSERT_META] = "INSERT OR IGNORE INTO Meta " "(key, value) " "VALUES " "('revision', " SCHEMA_REVISION ")", - "CREATE TABLE IF NOT EXISTS Subscriber (" + [SCHEMA_SUBSCRIBER] = "CREATE TABLE IF NOT EXISTS Subscriber (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " @@ -66,13 +85,13 @@ static char *create_stmts[] = { "lac INTEGER NOT NULL DEFAULT 0, " "expire_lu TIMESTAMP DEFAULT NULL" ")", - "CREATE TABLE IF NOT EXISTS AuthToken (" + [SCHEMA_AUTH] = "CREATE TABLE IF NOT EXISTS AuthToken (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "subscriber_id INTEGER UNIQUE NOT NULL, " "created TIMESTAMP NOT NULL, " "token TEXT UNIQUE NOT NULL" ")", - "CREATE TABLE IF NOT EXISTS Equipment (" + [SCHEMA_EQUIPMENT] = "CREATE TABLE IF NOT EXISTS Equipment (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " @@ -82,7 +101,7 @@ static char *create_stmts[] = { "classmark3 BLOB, " "imei NUMERIC UNIQUE NOT NULL" ")", - "CREATE TABLE IF NOT EXISTS EquipmentWatch (" + [SCHEMA_EQUIPMENT_WATCH] = "CREATE TABLE IF NOT EXISTS EquipmentWatch (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " @@ -90,7 +109,7 @@ static char *create_stmts[] = { "equipment_id NUMERIC NOT NULL, " "UNIQUE (subscriber_id, equipment_id) " ")", - "CREATE TABLE IF NOT EXISTS SMS (" + [SCHEMA_SMS] = "CREATE TABLE IF NOT EXISTS SMS (" /* metadata, not part of sms */ "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " @@ -114,39 +133,39 @@ static char *create_stmts[] = { "header BLOB, " /* UD Header */ "text TEXT " /* decoded UD after UDH */ ")", - "CREATE TABLE IF NOT EXISTS VLR (" + [SCHEMA_VLR] = "CREATE TABLE IF NOT EXISTS VLR (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " "subscriber_id NUMERIC UNIQUE NOT NULL, " "last_bts NUMERIC NOT NULL " ")", - "CREATE TABLE IF NOT EXISTS ApduBlobs (" + [SCHEMA_APDU] = "CREATE TABLE IF NOT EXISTS ApduBlobs (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "apdu_id_flags INTEGER NOT NULL, " "subscriber_id INTEGER NOT NULL, " "apdu BLOB " ")", - "CREATE TABLE IF NOT EXISTS Counters (" + [SCHEMA_COUNTERS] = "CREATE TABLE IF NOT EXISTS Counters (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "timestamp TIMESTAMP NOT NULL, " "value INTEGER NOT NULL, " "name TEXT NOT NULL " ")", - "CREATE TABLE IF NOT EXISTS RateCounters (" + [SCHEMA_RATE] = "CREATE TABLE IF NOT EXISTS RateCounters (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "timestamp TIMESTAMP NOT NULL, " "value INTEGER NOT NULL, " "name TEXT NOT NULL, " "idx INTEGER NOT NULL " ")", - "CREATE TABLE IF NOT EXISTS AuthKeys (" + [SCHEMA_AUTHKEY] = "CREATE TABLE IF NOT EXISTS AuthKeys (" "subscriber_id INTEGER PRIMARY KEY, " "algorithm_id INTEGER NOT NULL, " "a3a8_ki BLOB " ")", - "CREATE TABLE IF NOT EXISTS AuthLastTuples (" + [SCHEMA_AUTHLAST] = "CREATE TABLE IF NOT EXISTS AuthLastTuples (" "subscriber_id INTEGER PRIMARY KEY, " "issued TIMESTAMP NOT NULL, " "use_count INTEGER NOT NULL DEFAULT 0, " @@ -185,12 +204,164 @@ static int update_db_revision_2(void) "WHERE key = 'revision'"); if (!result) { LOGP(DDB, LOGL_ERROR, - "Failed set new revision (upgrade vom rev 2).\n"); + "Failed to update DB schema revision (upgrade from rev 2).\n"); + return -EINVAL; + } + dbi_result_free(result); + + return 0; +} + +/** + * Copied from the normal sms_from_result_v3 to avoid having + * to make sure that the real routine will remain backward + * compatible. + */ +static struct gsm_sms *sms_from_result_v3(dbi_result result) +{ + struct gsm_sms *sms = sms_alloc(); + long long unsigned int sender_id; + struct gsm_subscriber *sender; + const char *text, *daddr; + const unsigned char *user_data; + char buf[32]; + + if (!sms) + return NULL; + + sms->id = dbi_result_get_ulonglong(result, "id"); + + sender_id = dbi_result_get_ulonglong(result, "sender_id"); + snprintf(buf, sizeof(buf), "%llu", sender_id); + sender = db_get_subscriber(GSM_SUBSCRIBER_ID, buf); + OSMO_ASSERT(sender); + strncpy(sms->src.addr, sender->extension, sizeof(sms->src.addr)-1); + subscr_direct_free(sender); + sender = NULL; + + sms->reply_path_req = dbi_result_get_uint(result, "reply_path_req"); + sms->status_rep_req = dbi_result_get_uint(result, "status_rep_req"); + sms->ud_hdr_ind = dbi_result_get_uint(result, "ud_hdr_ind"); + sms->protocol_id = dbi_result_get_uint(result, "protocol_id"); + sms->data_coding_scheme = dbi_result_get_uint(result, + "data_coding_scheme"); + + daddr = dbi_result_get_string(result, "dest_addr"); + if (daddr) { + strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr)); + sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0'; + } + + sms->user_data_len = dbi_result_get_field_length(result, "user_data"); + user_data = dbi_result_get_binary(result, "user_data"); + if (sms->user_data_len > sizeof(sms->user_data)) + sms->user_data_len = (uint8_t) sizeof(sms->user_data); + memcpy(sms->user_data, user_data, sms->user_data_len); + + text = dbi_result_get_string(result, "text"); + if (text) { + strncpy(sms->text, text, sizeof(sms->text)); + sms->text[sizeof(sms->text)-1] = '\0'; + } + return sms; +} + +static int update_db_revision_3(void) +{ + dbi_result result; + struct gsm_sms *sms; + + result = dbi_conn_query(conn, "BEGIN EXCLUSIVE TRANSACTION"); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed to begin transaction (upgrade from rev 3)\n"); return -EINVAL; } dbi_result_free(result);
+ /* Rename old SMS table to be able create a new one */ + result = dbi_conn_query(conn, "ALTER TABLE SMS RENAME TO SMS_3"); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed to rename the old SMS table (upgrade from rev 3).\n"); + goto rollback; + } + dbi_result_free(result); + + /* Create new SMS table with all the bells and whistles! */ + result = dbi_conn_query(conn, create_stmts[SCHEMA_SMS]); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed to create a new SMS table (upgrade from rev 3).\n"); + goto rollback; + } + dbi_result_free(result); + + /* Cycle through old messages and convert them to the new format */ + result = dbi_conn_queryf(conn, "SELECT * FROM SMS_3"); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed fetch messages from the old SMS table (upgrade from rev 3).\n"); + goto rollback; + } + while (dbi_result_next_row(result)) { + sms = sms_from_result_v3(result); + if (db_sms_store(sms) != 0) { + LOGP(DDB, LOGL_ERROR, "Failed to store message to the new SMS table(upgrade from rev 3).\n"); + sms_free(sms); + dbi_result_free(result); + goto rollback; + } + sms_free(sms); + } + dbi_result_free(result); + + /* Remove the temporary table */ + result = dbi_conn_query(conn, "DROP TABLE SMS_3"); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed to drop the old SMS table (upgrade from rev 3).\n"); + goto rollback; + } + dbi_result_free(result); + + /* We're done. Bump DB Meta revision to 4 */ + result = dbi_conn_query(conn, + "UPDATE Meta " + "SET value = '4' " + "WHERE key = 'revision'"); + if (!result) { + LOGP(DDB, LOGL_ERROR, + "Failed to update DB schema revision (upgrade from rev 3).\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 3)\n"); + return -EINVAL; + } + + /* Shrink DB file size by actually wiping out SMS_3 table data */ + result = dbi_conn_query(conn, "VACUUM"); + if (!result) + LOGP(DDB, LOGL_ERROR, + "VACUUM failed. Ignoring it (upgrade from rev 3).\n"); + else + dbi_result_free(result); + return 0; + +rollback: + result = dbi_conn_query(conn, "ROLLBACK TRANSACTION"); + if (!result) + LOGP(DDB, LOGL_ERROR, + "Rollback failed (upgrade from rev 3).\n"); + else + dbi_result_free(result); + return -EINVAL; }
static int check_db_revision(void) @@ -218,6 +389,12 @@ static int check_db_revision(void) 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)) { /* everything is fine */ } else {
Hi Holger,
Thank you for improving robustness of the code. I looked through it and it looks good to me.
Please let me know when you merge to the master so I could rebase the SMS validity decoding code on top of it and submit it for review.
On Sat, Mar 8, 2014 at 1:54 PM, Holger Hans Peter Freyther < holger@freyther.de> wrote:
From: Holger Hans Peter Freyther holger@moiji-mobile.com
This is mostly based on Alexander's migration code. The code adds transaction handling and some sanity checks and cleanups to the code. We made the decision to fork the sms_from_result method and freeze it to that version. This way sms_from_result can move forward without having to deal with legacy.
openbsc/src/libbsc/gsm_subscriber_base.c | 6 + openbsc/src/libmsc/db.c | 209 ++++++++++++++++++++++++++++--- 2 files changed, 199 insertions(+), 16 deletions(-)
diff --git a/openbsc/src/libbsc/gsm_subscriber_base.c b/openbsc/src/libbsc/gsm_subscriber_base.c index 5755687..5e00443 100644 --- a/openbsc/src/libbsc/gsm_subscriber_base.c +++ b/openbsc/src/libbsc/gsm_subscriber_base.c @@ -72,6 +72,12 @@ static void subscr_free(struct gsm_subscriber *subscr) talloc_free(subscr); }
+void subscr_direct_free(struct gsm_subscriber *subscr) +{
OSMO_ASSERT(subscr->use_count == 1);subscr_free(subscr);+}
struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr) { subscr->use_count++; diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 2351485..3b92691 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -38,23 +38,42 @@ #include <osmocom/core/statistics.h> #include <osmocom/core/rate_ctr.h>
+/* Semi-Private-Interface (SPI) for the subscriber code */ +void subscr_direct_free(struct gsm_subscriber *subscr);
static char *db_basename = NULL; static char *db_dirname = NULL; static dbi_conn conn;
-#define SCHEMA_REVISION "3" +#define SCHEMA_REVISION "4"
+enum {
SCHEMA_META,INSERT_META,SCHEMA_SUBSCRIBER,SCHEMA_AUTH,SCHEMA_EQUIPMENT,SCHEMA_EQUIPMENT_WATCH,SCHEMA_SMS,SCHEMA_VLR,SCHEMA_APDU,SCHEMA_COUNTERS,SCHEMA_RATE,SCHEMA_AUTHKEY,SCHEMA_AUTHLAST,+};
-static char *create_stmts[] = {
"CREATE TABLE IF NOT EXISTS Meta ("+static const char *create_stmts[] = {
[SCHEMA_META] = "CREATE TABLE IF NOT EXISTS Meta (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "key TEXT UNIQUE NOT NULL, " "value TEXT NOT NULL" ")",
"INSERT OR IGNORE INTO Meta "
[INSERT_META] = "INSERT OR IGNORE INTO Meta " "(key, value) " "VALUES " "('revision', " SCHEMA_REVISION ")",
"CREATE TABLE IF NOT EXISTS Subscriber ("
[SCHEMA_SUBSCRIBER] = "CREATE TABLE IF NOT EXISTS Subscriber (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, "@@ -66,13 +85,13 @@ static char *create_stmts[] = { "lac INTEGER NOT NULL DEFAULT 0, " "expire_lu TIMESTAMP DEFAULT NULL" ")",
"CREATE TABLE IF NOT EXISTS AuthToken ("
[SCHEMA_AUTH] = "CREATE TABLE IF NOT EXISTS AuthToken (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "subscriber_id INTEGER UNIQUE NOT NULL, " "created TIMESTAMP NOT NULL, " "token TEXT UNIQUE NOT NULL" ")",
"CREATE TABLE IF NOT EXISTS Equipment ("
[SCHEMA_EQUIPMENT] = "CREATE TABLE IF NOT EXISTS Equipment (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, "@@ -82,7 +101,7 @@ static char *create_stmts[] = { "classmark3 BLOB, " "imei NUMERIC UNIQUE NOT NULL" ")",
"CREATE TABLE IF NOT EXISTS EquipmentWatch ("
[SCHEMA_EQUIPMENT_WATCH] = "CREATE TABLE IF NOT EXISTSEquipmentWatch (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " @@ -90,7 +109,7 @@ static char *create_stmts[] = { "equipment_id NUMERIC NOT NULL, " "UNIQUE (subscriber_id, equipment_id) " ")",
"CREATE TABLE IF NOT EXISTS SMS ("
[SCHEMA_SMS] = "CREATE TABLE IF NOT EXISTS SMS (" /* metadata, not part of sms */ "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, "@@ -114,39 +133,39 @@ static char *create_stmts[] = { "header BLOB, " /* UD Header */ "text TEXT " /* decoded UD after UDH */ ")",
"CREATE TABLE IF NOT EXISTS VLR ("
[SCHEMA_VLR] = "CREATE TABLE IF NOT EXISTS VLR (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "updated TIMESTAMP NOT NULL, " "subscriber_id NUMERIC UNIQUE NOT NULL, " "last_bts NUMERIC NOT NULL " ")",
"CREATE TABLE IF NOT EXISTS ApduBlobs ("
[SCHEMA_APDU] = "CREATE TABLE IF NOT EXISTS ApduBlobs (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "created TIMESTAMP NOT NULL, " "apdu_id_flags INTEGER NOT NULL, " "subscriber_id INTEGER NOT NULL, " "apdu BLOB " ")",
"CREATE TABLE IF NOT EXISTS Counters ("
[SCHEMA_COUNTERS] = "CREATE TABLE IF NOT EXISTS Counters (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "timestamp TIMESTAMP NOT NULL, " "value INTEGER NOT NULL, " "name TEXT NOT NULL " ")",
"CREATE TABLE IF NOT EXISTS RateCounters ("
[SCHEMA_RATE] = "CREATE TABLE IF NOT EXISTS RateCounters (" "id INTEGER PRIMARY KEY AUTOINCREMENT, " "timestamp TIMESTAMP NOT NULL, " "value INTEGER NOT NULL, " "name TEXT NOT NULL, " "idx INTEGER NOT NULL " ")",
"CREATE TABLE IF NOT EXISTS AuthKeys ("
[SCHEMA_AUTHKEY] = "CREATE TABLE IF NOT EXISTS AuthKeys (" "subscriber_id INTEGER PRIMARY KEY, " "algorithm_id INTEGER NOT NULL, " "a3a8_ki BLOB " ")",
"CREATE TABLE IF NOT EXISTS AuthLastTuples ("
[SCHEMA_AUTHLAST] = "CREATE TABLE IF NOT EXISTS AuthLastTuples (" "subscriber_id INTEGER PRIMARY KEY, " "issued TIMESTAMP NOT NULL, " "use_count INTEGER NOT NULL DEFAULT 0, "@@ -185,12 +204,164 @@ static int update_db_revision_2(void) "WHERE key = 'revision'"); if (!result) { LOGP(DDB, LOGL_ERROR,
"Failed set new revision (upgrade vom rev 2).\n");
"Failed to update DB schema revision (upgrade fromrev 2).\n");
return -EINVAL;}dbi_result_free(result);return 0;+}
+/**
- Copied from the normal sms_from_result_v3 to avoid having
- to make sure that the real routine will remain backward
- compatible.
- */
+static struct gsm_sms *sms_from_result_v3(dbi_result result) +{
struct gsm_sms *sms = sms_alloc();long long unsigned int sender_id;struct gsm_subscriber *sender;const char *text, *daddr;const unsigned char *user_data;char buf[32];if (!sms)return NULL;sms->id = dbi_result_get_ulonglong(result, "id");sender_id = dbi_result_get_ulonglong(result, "sender_id");snprintf(buf, sizeof(buf), "%llu", sender_id);sender = db_get_subscriber(GSM_SUBSCRIBER_ID, buf);OSMO_ASSERT(sender);strncpy(sms->src.addr, sender->extension, sizeof(sms->src.addr)-1);subscr_direct_free(sender);sender = NULL;sms->reply_path_req = dbi_result_get_uint(result,"reply_path_req");
sms->status_rep_req = dbi_result_get_uint(result,"status_rep_req");
sms->ud_hdr_ind = dbi_result_get_uint(result, "ud_hdr_ind");sms->protocol_id = dbi_result_get_uint(result, "protocol_id");sms->data_coding_scheme = dbi_result_get_uint(result,"data_coding_scheme");daddr = dbi_result_get_string(result, "dest_addr");if (daddr) {strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr));sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0';}sms->user_data_len = dbi_result_get_field_length(result,"user_data");
user_data = dbi_result_get_binary(result, "user_data");if (sms->user_data_len > sizeof(sms->user_data))sms->user_data_len = (uint8_t) sizeof(sms->user_data);memcpy(sms->user_data, user_data, sms->user_data_len);text = dbi_result_get_string(result, "text");if (text) {strncpy(sms->text, text, sizeof(sms->text));sms->text[sizeof(sms->text)-1] = '\0';}return sms;+}
+static int update_db_revision_3(void) +{
dbi_result result;struct gsm_sms *sms;result = dbi_conn_query(conn, "BEGIN EXCLUSIVE TRANSACTION");if (!result) {LOGP(DDB, LOGL_ERROR,"Failed to begin transaction (upgrade from rev3)\n"); return -EINVAL; } dbi_result_free(result);
/* Rename old SMS table to be able create a new one */result = dbi_conn_query(conn, "ALTER TABLE SMS RENAME TO SMS_3");if (!result) {LOGP(DDB, LOGL_ERROR,"Failed to rename the old SMS table (upgrade from rev3).\n");
goto rollback;}dbi_result_free(result);/* Create new SMS table with all the bells and whistles! */result = dbi_conn_query(conn, create_stmts[SCHEMA_SMS]);if (!result) {LOGP(DDB, LOGL_ERROR,"Failed to create a new SMS table (upgrade from rev3).\n");
goto rollback;}dbi_result_free(result);/* Cycle through old messages and convert them to the new format */result = dbi_conn_queryf(conn, "SELECT * FROM SMS_3");if (!result) {LOGP(DDB, LOGL_ERROR,"Failed fetch messages from the old SMS table(upgrade from rev 3).\n");
goto rollback;}while (dbi_result_next_row(result)) {sms = sms_from_result_v3(result);if (db_sms_store(sms) != 0) {LOGP(DDB, LOGL_ERROR, "Failed to store message tothe new SMS table(upgrade from rev 3).\n");
sms_free(sms);dbi_result_free(result);goto rollback;}sms_free(sms);}dbi_result_free(result);/* Remove the temporary table */result = dbi_conn_query(conn, "DROP TABLE SMS_3");if (!result) {LOGP(DDB, LOGL_ERROR,"Failed to drop the old SMS table (upgrade from rev3).\n");
goto rollback;}dbi_result_free(result);/* We're done. Bump DB Meta revision to 4 */result = dbi_conn_query(conn,"UPDATE Meta ""SET value = '4' ""WHERE key = 'revision'");if (!result) {LOGP(DDB, LOGL_ERROR,"Failed to update DB schema revision (upgrade fromrev 3).\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 fromrev 3)\n");
return -EINVAL;}/* Shrink DB file size by actually wiping out SMS_3 table data */result = dbi_conn_query(conn, "VACUUM");if (!result)LOGP(DDB, LOGL_ERROR,"VACUUM failed. Ignoring it (upgrade from rev3).\n");
elsedbi_result_free(result);return 0;+rollback:
result = dbi_conn_query(conn, "ROLLBACK TRANSACTION");if (!result)LOGP(DDB, LOGL_ERROR,"Rollback failed (upgrade from rev 3).\n");elsedbi_result_free(result);return -EINVAL;}
static int check_db_revision(void) @@ -218,6 +389,12 @@ static int check_db_revision(void) dbi_result_free(result); return -EINVAL; }
} else if (!strcmp(rev_s, "3")) {if (update_db_revision_3()) {LOGP(DDB, LOGL_FATAL, "Failed to update databasefrom schema revision '%s'.\n", rev_s);
dbi_result_free(result);return -EINVAL;} } else if (!strcmp(rev_s, SCHEMA_REVISION)) { /* everything is fine */ } else {-- 1.9.0