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.
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.
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.
Sadly there is no proper foreign key relationship on the tables
that related to the Subscriber. This means we can't use a DELETE
with Cascade and need to delete everything by hand. To make things
worse maybe the SMS/Paging code is still using the subscriber
making the operation more dangerous. I had added NULL checks for
sender_id/receiver_id at 30C3 so we should not crash in this
situation.
Fixes: SYS#274
Avoid a crash when reading a SMS and a Subscriber could not be resolved.
It is not clear why the read was failing. The sender_id and the receiver_id
was valid for the given sms. I assume that the database has been locked
due external access to it.
The side-effect is that in case of such a failure the sms_queue will start
to deliver starting from subscriber id = 0 again.
#1 0x0000000000428bec in sms_from_result (net=0x156a270, result=0x15eda30) at db.c:1146
#2 0x000000000042a8e0 in db_sms_get_unsent_by_subscr (net=0x156a270,
min_subscr_id=<optimized out>, failed=<optimized out>) at db.c:1255
#3 0x000000000042e900 in take_next_sms (smsq=<optimized out>) at sms_queue.c:193
#4 sms_submit_pending (_data=0x158e300) at sms_queue.c:227
#5 0x00007f3fd30de3bc in osmo_timers_update () at timer.c:243
#6 0x00007f3fd30de69b in osmo_select_main (polling=0) at select.c:133
#7 0x0000000000406fbc in main (argc=9, argv=<optimized out>) at bsc_hack.c:346
(gdb) frame 1
#1 0x0000000000428bec in sms_from_result (net=0x156a270, result=0x15eda30) at db.c:1146
1146 strncpy(sms->src.addr, sms->sender->extension, sizeof(sms->src.addr)-1);
(gdb) p *sms
(gdb) p sms->sender
$1 = (struct gsm_subscriber *) 0x0
(gdb) p sender_id
$2 = <optimized out>
Location Update Requests time out and get rejected because the
subscriber is not authorized. Authorizing the subscriber through
openBSC or sqlite3 doesn't help the subscriber is still seen and
shown as not authorized
The value is read as uint, this is the wrong type it's a (u)longlong
in libdbi 0.9.0 and later.
Disable the periodic LU using "no periodic location update" VTY
command. In that case set the expire_lu to 0 which will then be
translated to a NULL in the database layer. This leads to a bit of
copy and paste in the db_sync_subscriber method but I don't see
how we could easily use 'datetime(%i, 'unixepoch')' and 'NULL'
at the same time.
Change the query to find expired queries to check for NOT NULL
and the time being in the past. This means if there are still
old subscribers in the database they might not be expired. One
would need to execute a query like "UPATE Subscriber SET expire_lu
= 0 WHERE expire_lu is null". The same applies when disabling the
periodic LU. One would need to update the database by hand.
Manual tests executed/passed:
1.) periodic LU enabled:
* use gst LUTest.st to do a LU
* UPDATE Subscriber SET expire_lu=datetime('now');
* observe the subscriber being expired (it was)
2.) periodic LU disabled:
* use gst LUTest.st to do a LU
* verify that the expire_lu is NULL in the database
bsc_api.c:417:3: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘unsigned int’ [-Wformat]
bsc_api.c: In function ‘handle_ass_fail’:
bsc_api.c:458:3: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘unsigned int’ [-Wformat]
db.c: In function ‘db_sync_subscriber’:
db.c:785:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 8 has type ‘time_t’ [-Wformat]
osmo_msc.c: In function ‘msc_release_connection’:
osmo_msc.c:145:20: warning: unused variable ‘trans’ [-Wunused-variable]
smpp_smsc.c: In function ‘link_accept_cb’:
smpp_smsc.c:891:24: warning: assignment from incompatible pointer type [enabled by default]
smpp_smsc.c:271:1: warning: ‘esme_by_system_id’ defined but not used [-Wunused-function]
smpp_openbsc.c: In function ‘smpp_openbsc_init’:
smpp_openbsc.c:545:2: warning: implicit declaration of function ‘smpp_vty_init’ [-Wimplicit-function-declaration]
osmo_bsc_ctrl.c: In function ‘verify_bts_loc’:
osmo_bsc_ctrl.c:340:19: warning: variable ‘height’ set but not used [-Wunused-but-set-variable
smpp_mirror.c: In function ‘main’:
smpp_mirror.c:297:2: warning: implicit declaration of function ‘osmo_init_logging’ [-Wimplicit-function-declaration]
Set the subscriber expiry timeout to twice the duration of the location
update period and provide functions subscr_expire() and
db_subscriber_expire() to mark subscribers offline that have missed two
location update periods.
This patch increases the DB revision to 3, so the hlr will be
incompatible with prior versions.
We should allow 0 for T3212 as well to disable the location update
period. In that case we will need a way to indicate that in the
database.
libosmogsm is a new library that is distributed in the libosmocore.
Now, openbsc depends on it. This patch gets openbsc with this
change.
This patch also rewrites all include path to the new
osmocom/[gsm|core]
Signed-off-by: Pablo Neira Ayuso <pablo@gnumonks.org>
Holger Hans Peter Freyther
Harald Welte
Sylvain Munaut