Hi Harald,
We're trying out SMPP interconnection and found that for Submit_SM (aka MT-SMS) sender is always set to an extension of subscriber ID 1.
Looking into the code, this happens because we set "sms->sender" to point to subscriber ID 1 in submit_to_sms() function in SMPP code:
/* 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);
And then src.addr/ton/npi are ignored when we store to the SMS DB and only subscriber ID is stored:
/* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " "(created, sender_id, 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, 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);
So when we read SMS back from the DB, sender address is taken from that subscriber.
It seems that a solution is to store actual ton/npi/addr in the SMS DB instead of storing ID. Then we could set sms->sender to NULL and just ignore it.
Is there anything wrong with this approach? Otherwise we'll work on a patch to implement that.
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
On Wed, Aug 28, 2013 at 11:17:37PM +0400, Alexander Chemeris wrote:
Hi Harald,
Hi,
http://git.osmocom.org/openbsc/commit/?h=zecke/features/sms-smpp&id=f3f0...
has already some work in progress for this.
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
that is one of the reasons I didn't merge it. We should have schema migration. E.g. if you could create a patch that migrates an old SMS table to the new one that would be great.
Hi Holger,
On Thu, Aug 29, 2013 at 9:43 AM, Holger Hans Peter Freyther holger@freyther.de wrote:
On Wed, Aug 28, 2013 at 11:17:37PM +0400, Alexander Chemeris wrote: http://git.osmocom.org/openbsc/commit/?h=zecke/features/sms-smpp&id=f3f0...
has already some work in progress for this.
Thanks!
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
that is one of the reasons I didn't merge it. We should have schema migration. E.g. if you could create a patch that migrates an old SMS table to the new one that would be great.
Ok, I hope I'll find some time to create the patch.
One more comment in addition to Holgers response:
The problem you are describing only exists in store-and-forward mode, it doesn' exist when you select SMPP transaction mode. In the latter case, the SMS is not queued but immediately delivered to the MS, and an error returned if the MS is not reachable.
And yes, the DB schema has to change, definitely. It was wrong to put the subscriber IDs in the SMS table in the first place.
Regards, Harald
On Wed, Aug 28, 2013 at 11:17:37PM +0400, Alexander Chemeris wrote:
Hi Harald,
We're trying out SMPP interconnection and found that for Submit_SM (aka MT-SMS) sender is always set to an extension of subscriber ID 1.
Looking into the code, this happens because we set "sms->sender" to point to subscriber ID 1 in submit_to_sms() function in SMPP code:
/* 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);
And then src.addr/ton/npi are ignored when we store to the SMS DB and only subscriber ID is stored:
/* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " "(created, sender_id, 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, 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);
So when we read SMS back from the DB, sender address is taken from that subscriber.
It seems that a solution is to store actual ton/npi/addr in the SMS DB instead of storing ID. Then we could set sms->sender to NULL and just ignore it.
Is there anything wrong with this approach? Otherwise we'll work on a patch to implement that.
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
-- Regards, Alexander Chemeris. CEO, Fairwaves LLC / ООО УмРадио http://fairwaves.ru
Yes, I figured that transaction mode should work, but we have to use async mode here, unfortunately.
Regarding DB schema - Holger's patch looks sane to me. I would do it the same way if I implemented it. I hope it'll lend to master when we amend it with the DB update procedure.
On Thu, Aug 29, 2013 at 11:52 AM, Harald Welte laforge@gnumonks.org wrote:
One more comment in addition to Holgers response:
The problem you are describing only exists in store-and-forward mode, it doesn' exist when you select SMPP transaction mode. In the latter case, the SMS is not queued but immediately delivered to the MS, and an error returned if the MS is not reachable.
And yes, the DB schema has to change, definitely. It was wrong to put the subscriber IDs in the SMS table in the first place.
Regards, Harald
On Wed, Aug 28, 2013 at 11:17:37PM +0400, Alexander Chemeris wrote:
Hi Harald,
We're trying out SMPP interconnection and found that for Submit_SM (aka MT-SMS) sender is always set to an extension of subscriber ID 1.
Looking into the code, this happens because we set "sms->sender" to point to subscriber ID 1 in submit_to_sms() function in SMPP code:
/* 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);
And then src.addr/ton/npi are ignored when we store to the SMS DB and only subscriber ID is stored:
/* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " "(created, sender_id, 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, 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);
So when we read SMS back from the DB, sender address is taken from that subscriber.
It seems that a solution is to store actual ton/npi/addr in the SMS DB instead of storing ID. Then we could set sms->sender to NULL and just ignore it.
Is there anything wrong with this approach? Otherwise we'll work on a patch to implement that.
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
-- Regards, Alexander Chemeris. CEO, Fairwaves LLC / ООО УмРадио http://fairwaves.ru
--
- Harald Welte laforge@gnumonks.org http://laforge.gnumonks.org/
============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
Attached is an attempt to implement the conversion. I haven't tested it yet. If the general approach is correct, I'll test it tomorrow.
PS For some reason this patch breaks build of channel_test, probably because it introduces gsm_network to db.c. I would appreciate if someone with better knowledge of libs interdependencies help me fix that.
On Thu, Aug 29, 2013 at 9:00 PM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
Yes, I figured that transaction mode should work, but we have to use async mode here, unfortunately.
Regarding DB schema - Holger's patch looks sane to me. I would do it the same way if I implemented it. I hope it'll lend to master when we amend it with the DB update procedure.
On Thu, Aug 29, 2013 at 11:52 AM, Harald Welte laforge@gnumonks.org wrote:
One more comment in addition to Holgers response:
The problem you are describing only exists in store-and-forward mode, it doesn' exist when you select SMPP transaction mode. In the latter case, the SMS is not queued but immediately delivered to the MS, and an error returned if the MS is not reachable.
And yes, the DB schema has to change, definitely. It was wrong to put the subscriber IDs in the SMS table in the first place.
Regards, Harald
On Wed, Aug 28, 2013 at 11:17:37PM +0400, Alexander Chemeris wrote:
Hi Harald,
We're trying out SMPP interconnection and found that for Submit_SM (aka MT-SMS) sender is always set to an extension of subscriber ID 1.
Looking into the code, this happens because we set "sms->sender" to point to subscriber ID 1 in submit_to_sms() function in SMPP code:
/* 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);
And then src.addr/ton/npi are ignored when we store to the SMS DB and only subscriber ID is stored:
/* FIXME: correct validity period */ result = dbi_conn_queryf(conn, "INSERT INTO SMS " "(created, sender_id, 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, 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);
So when we read SMS back from the DB, sender address is taken from that subscriber.
It seems that a solution is to store actual ton/npi/addr in the SMS DB instead of storing ID. Then we could set sms->sender to NULL and just ignore it.
Is there anything wrong with this approach? Otherwise we'll work on a patch to implement that.
PS What is the current policy about changing DB layouts - just increase SCHEMA_REVISION?
-- Regards, Alexander Chemeris. CEO, Fairwaves LLC / ООО УмРадио http://fairwaves.ru
--
- Harald Welte laforge@gnumonks.org http://laforge.gnumonks.org/
============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
-- Regards, Alexander Chemeris. CEO, Fairwaves LLC / ООО УмРадио http://fairwaves.ru