Re: smppbox code questions

Hey, I didn't say it would work, just asked if you had tried it. You can't 
expect me to remember by heart where each #define is set. Unfortunately it 
needs patching.

BR,
Nikos

----- Original Message ----- 
From: "Rene Kluwen" <[email protected]>
To: "'Nikos Balkanas'" <[email protected]>; "'Victor Luchitz'" 
<[email protected]>; <[email protected]>
Sent: Monday, July 12, 2010 1:30 AM
Subject: RE: smppbox code questions


> Nikos, you disappoint me.
>
> report_versions() is linked in from the gwlib libraries with the values 
> that
> it was compiled with. Try it, and you will see.
>
> == Rene
>
> -----Original Message-----
> From: Nikos Balkanas [mailto:[email protected]]
> Sent: zondag 11 juli 2010 17:33
> To: Rene Kluwen; 'Victor Luchitz'; [email protected]
> Subject: Re: smppbox code questions
>
> Nope, that would defeat the whole puprose. Add it to your code, just 
> before
> calling report_versions()
>
> BR,
> Nikos
> ----- Original Message ----- 
> From: "Rene Kluwen" <[email protected]>
> To: "'Nikos Balkanas'" <[email protected]>; "'Victor Luchitz'"
> <[email protected]>; <[email protected]>
> Sent: Sunday, July 11, 2010 4:50 PM
> Subject: RE: smppbox code questions
>
>
>>I haven't tried.
>> But it will only work if I recompile gwlib after the new #define's.
>>
>> == Rene
>>
>> -----Original Message-----
>> From: Nikos Balkanas [mailto:[email protected]]
>> Sent: zondag 11 juli 2010 9:46
>> To: Rene Kluwen; 'Victor Luchitz'; [email protected]
>> Subject: Re: smppbox code questions
>>
>> Hi,
>>
>> Have you tried:
>>
>> #undef GW_NAME
>> #undef GW_VERSION
>>
>> #define GW_VERSION ...
>> #define GW_NAME ...
>>
>> and then call report_versions?
>>
>> BR,
>> Nikos
>> ----- Original Message ----- 
>> From: "Rene Kluwen" <[email protected]>
>> To: "'Victor Luchitz'" <[email protected]>; <[email protected]>
>> Sent: Saturday, July 10, 2010 11:48 PM
>> Subject: RE: smppbox code questions
>>
>>
>> That is a gwlib quirk. The function report_versions() has the Kannel
>> version
>>
>> hard coded in it.
>> Sqlbox has the same problem.
>>
>> Maybe we need to send in a patch for gwlib/utils.c.
>>
>> == Rene
>>
>> -----Original Message-----
>> From: Victor Luchitz [mailto:[email protected]]
>> Sent: zaterdag 10 juli 2010 22:17
>> To: [email protected]; Rene Kluwen
>> Subject: Re: smppbox code questions
>>
>> Yeah, I made similar changes locally with the same result: at startup,
>> smppbox prints the following message:
>> [91108] [0] DEBUG: Kannel smppbox version `svn-r4833M'.
>> which is kannel's svn revision number, not that of smppbox.
>>
>> 2010/7/10 Rene Kluwen <[email protected]>:
>>> I did an attempt to include the svn version numbers.
>>> But so far no luck. I did check things in, in case you want to have a
>>> look
>>
>>> at it.
>>>
>>> -----Original Message-----
>>> From: Victor Luchitz [mailto:[email protected]]
>>> Sent: zaterdag 10 juli 2010 18:27
>>> To: Rene Kluwen
>>> Subject: Re: smppbox code questions
>>>
>>> Ok, then. On a side note, the configure.in file needs to be updated to
>>> reflect the cvs -> svn change, currently the configure script still
>>> tries to fetch the version number from CVS/Entries file and fails at
>>> doing so. I am by no means an M4/autotools expert, guess some
>>> copy&paste job could be done using the main kannel configure.in..
>>>
>>> 2010/7/10 Rene Kluwen <[email protected]>:
>>>> I decided to change the sources with longer variables, so things are
>>>> consistent with smsbox.
>>>> cfg.diff has also been committed.
>>>>
>>>> == Rene
>>>>
>>>> -----Original Message-----
>>>> From: Victor Luchitz [mailto:[email protected]]
>>>> Sent: zaterdag 10 juli 2010 15:53
>>>> To: [email protected]; Rene Kluwen
>>>> Subject: Re: smppbox code questions
>>>>
>>>> Oh, my original patch was missing the smppbox-cfg.def part (in
>>>> attachment) so currently you can't specify any of the new vars,
>>>> otherwise
>>
>>>> smppbox doesn't start.
>>>> One thing I noticed is that you committed my patch with vars using
>>>> shorter names: src-addr-npi, etc, while the svn doc uses longer names
>>>> for
>>
>>>> them: source-addr-npi, dest-addr-npi and so on. Either the source code
>>>> needs correction to match the documentation or later is messed up
>>>> :)
>>>>
>>>> 2010/7/10 Rene Kluwen <[email protected]>:
>>>>> Right away I also checked in your ton/npi patch.
>>>>>
>>>>> == Rene
>>>>>
>>>>> -----Original Message-----
>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>> Sent: vrijdag 9 juli 2010 23:19
>>>>> To: Rene Kluwen
>>>>> Subject: Re: smppbox code questions
>>>>>
>>>>> Yeah, I was thinking about this "hack" as well, but it's going to
>>>>> create more problems than it solves. Btw, why does smppbox use
>>>>> system-type as boxc_id instead of ESME's login name? That forces
>>>>> EMSE's to have distinct system-type values, while almost all SMSC'es
>>>>> I've seen so far allow connections with empty system-type string, for
>>>>> example.
>>>>>
>>>>> 2010/7/10 Rene Kluwen <[email protected]>:
>>>>>> Heh... I think the way it works now is best for the average user.
>>>>>> But I am sure you are competent enough to change it to your own 
>>>>>> needs.
>>>>>> One "hack" that you can make is make the system-type of the client 
>>>>>> the
>>>>>> same as your smsc-id in your kannel.conf.
>>>>>> This is of course not recommended for most persons, but it might work
>>>>>> for you.
>>>>>>
>>>>>> == Rene
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>>> Sent: vrijdag 9 juli 2010 22:39
>>>>>> To: Rene Kluwen
>>>>>> Subject: Re: smppbox code questions
>>>>>>
>>>>>> I have a pretty good idea how it works, it's just that the way it
>>>>>> works doesn't suit my needs ;)
>>>>>>
>>>>>> 2010/7/9 Rene Kluwen <[email protected]>:
>>>>>>> Surely this is relevant.
>>>>>>>
>>>>>>> Smppbox is not interested in bearerbox generated dlr's. It just 
>>>>>>> needs
>>>>>>> to "dlr_find" the dlr's that it added itself via dlr_add.
>>>>>>> Bearerbox takes care of its own dlr's. Smppbox also takes care of 
>>>>>>> its
>>>>>>> own dlr's.
>>>>>>>
>>>>>>> I think you should re-read the code again to see how it works.
>>>>>>>
>>>>>>> == Rene
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>>>> Sent: vrijdag 9 juli 2010 15:35
>>>>>>> To: [email protected]; Rene Kluwen
>>>>>>> Subject: Re: smppbox code questions
>>>>>>>
>>>>>>> Yes, but how is this relevant? I mean, there are two possibilities
>>>>>>> to make smppbox be aware of bearerbox-generated DLRs:
>>>>>>>
>>>>>>> 1) use boxc_id as smsc-id in dlr_add in bearerbox and then pass the
>>>>>>> report_mo message to smppbox without issuing dlr_find in bearerbox
>>>>>>> 2) use "proper"/parent smsc-id in smppbox
>>>>>>>
>>>>>>> The whole issue arises from the need to pass SMSC-related DLR's to
>>>>>>> smppbox without the later issuing any DLR's itself. For example, a
>>>>>>> MT message may fail to be delivered due to insufficient funds on
>>>>>>> user's account.
>>>>>>>
>>>>>>> 2010/7/9 Rene Kluwen <[email protected]>:
>>>>>>>> If bearerbox sends a report_mo, then it should include a status 
>>>>>>>> (dlr
>>>>>>>> type) as well.
>>>>>>>> Or am I wrong?
>>>>>>>>
>>>>>>>> == Rene
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>>>>> Sent: vrijdag 9 juli 2010 14:24
>>>>>>>> To: [email protected]; Rene Kluwen
>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>
>>>>>>>> Unfortunately there's currently no way to add a SMSC_SUCCESS or
>>>>>>>> SMSC_FAIL DLR in smppbox so I have/need to do that in bearerbox.
>>>>>>>> But oh, well, I'll just go with boxc_id as smsc-id and live with
>>>>>>>> this fact.
>>>>>>>>
>>>>>>>> 2010/7/9 Rene Kluwen <[email protected]>:
>>>>>>>>> But bearerbox inserts it's own dlr's. As does smppbox.
>>>>>>>>> So bearerbox will find their dlr's. And smppbox will do also.
>>>>>>>>>
>>>>>>>>> == Rene
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>>>>>> Sent: vrijdag 9 juli 2010 13:55
>>>>>>>>> To: Rene Kluwen
>>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>>
>>>>>>>>> Well, this is going against the logic in bearerbox. For example,
>>>>>>>>> if you pass a DLR via standard Kannel HTTP protocol, bearerbox
>>>>>>>>> will try to find a matching DLR using its own smsc-id, upon
>>>>>>>>> failing to do, it won't pass the DLR to smppbox either.
>>>>>>>>>
>>>>>>>>> 2010/7/9 Rene Kluwen <[email protected]>:
>>>>>>>>>> The first parameter (smsc_id) is to determine "who's" dlr it is 
>>>>>>>>>> to
>>>>>>>>>> begin with. So in short: To which smsc it belongs.
>>>>>>>>>> Because smppbox does things the other way around, it passes the
>>>>>>>>>> boxc_id variable. So if two boxes happen to have the same "ts"
>>>>>>>>>> (which can in theory happen) the value is used to distinguish to
>>>>>>>>>> which box_id it belongs.
>>>>>>>>>>
>>>>>>>>>> == Rene
>>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Victor Luchitz [mailto:[email protected]]
>>>>>>>>>> Sent: vrijdag 9 juli 2010 11:12
>>>>>>>>>> To: [email protected]; Rene Kluwen
>>>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>>>
>>>>>>>>>> On a side note, why does smppbox use boxc_id as the first
>>>>>>>>>> parameter passed to dlr_add and dlr_find? Both functions take
>>>>>>>>>> smsc_id as the first argument and boxc_id value is obtained from
>>>>>>>>>> the sms struct.
>>>>>>>>>>
>>>>>>>>>> 2010/7/8 Rene Kluwen <[email protected]>:
>>>>>>>>>>> Done.
>>>>>>>>>>> Current revision is 17.
>>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: [email protected] [mailto:[email protected]]
>>>>>>>>>>> On Behalf Of Victor Luchitz
>>>>>>>>>>> Sent: donderdag 8 juli 2010 15:06
>>>>>>>>>>> To: [email protected]
>>>>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>>>>
>>>>>>>>>>> Yeah, you committed the proposed change to boxc->boxc_id in
>>>>>>>>>>> revision 15. What I'm asking about is the suggestion and patch I
>>>>>>>>>>> posted here:
>>>>>>>>>>> http://www.kannel.org/pipermail/devel/2010-July/003653.html
>>>>>>>>>>>
>>>>>>>>>>> 2010/7/8 Rene Kluwen <[email protected]>:
>>>>>>>>>>>> It's already in the code.
>>>>>>>>>>>> Current revision is 16.
>>>>>>>>>>>>
>>>>>>>>>>>> == Rene
>>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: [email protected]
>>>>>>>>>>>> [mailto:[email protected]] On Behalf Of Victor Luchitz
>>>>>>>>>>>> Sent: donderdag 8 juli 2010 7:52
>>>>>>>>>>>> To: [email protected]
>>>>>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>>>>>
>>>>>>>>>>>> Any hope this will be reviewed and committed?
>>>>>>>>>>>>
>>>>>>>>>>>> I'm also working on a patch that adds TLV support to smppbox
>>>>>>>>>>>> but I'd like to get this one included first.
>>>>>>>>>>>>
>>>>>>>>>>>> 2010/7/6 Victor Luchitz <[email protected]>:
>>>>>>>>>>>>> Yup, it's working fine now. Noticed there's another memleak
>>>>>>>>>>>>> though:
>>>>>>>>>>>>>
>>>>>>>>>>>>> another octstr_destroy(msgid); call is needed right after the:
>>>>>>>>>>>>> /* we could not find a corresponding dlr; nothing to send */
>>>>>>>>>>>>> line.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm also attaching another patch which allows transmission of
>>>>>>>>>>>>> custom error codes in DLR's in the same manner as the message
>>>>>>>>>>>>> text bit.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2010/7/6 Rene Kluwen <[email protected]>:
>>>>>>>>>>>>>> I have no way of testing this here. But since either way
>>>>>>>>>>>>>> cannot
>>
>>>>>>>>>>>>>> harm I changed it.
>>>>>>>>>>>>>> Current smppbox revision is now 15.
>>>>>>>>>>>>>> Could you please check out and see if this fixes your 
>>>>>>>>>>>>>> problem?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> == Rene
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>> From: [email protected]
>>>>>>>>>>>>>> [mailto:[email protected]] On Behalf Of Victor Luchitz
>>>>>>>>>>>>>> Sent: dinsdag 6 juli 2010 14:53
>>>>>>>>>>>>>> To: [email protected]
>>>>>>>>>>>>>> Subject: Re: smppbox code questions
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1) I think this assumption is incorrect. I have the routing
>>>>>>>>>>>>>> set up this way in bearerbox:
>>>>>>>>>>>>>> group = smsbox-route
>>>>>>>>>>>>>> smsbox-id = vma
>>>>>>>>>>>>>> smsc-id = HTTP
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So all messages on the 'HTTP' smsc get routed to smppbox.
>>>>>>>>>>>>>> However, the custom HTTP protocol in the above layer does not
>>>>>>>>>>>>>> use dlr_find to route messages to a specific box for two
>>>>>>>>>>>>>> reasons:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> a) wrong smsc-id is used in the query (bearerbox doesn't know
>>>>>>>>>>>>>> that smppbox overrides the smsc id with system-type) so
>>>>>>>>>>>>>> dlr_find always fails
>>>>>>>>>>>>>> b) dlr_find removes DLR from the table and then subsequently
>>>>>>>>>>>>>> readds it, which is rather stressful on the DB for no sane
>>>>>>>>>>>>>> reason
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> What it does instead is simply setting the sms_type to
>>>>>>>>>>>>>> report_mo, leaving box_id empty as in regular MO messages.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2010/7/6 Rene Kluwen <[email protected]>:
>>>>>>>>>>>>>>> To start with the last thing:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2) You are right. It should use the msgid's in the dlr_url
>>>>>>>>>>>>>>> from the dlr instance. I changed it.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> About 1): We assume msg->boxc_id and box->boxc_id are the
>>>>>>>>>>>>>>> same
>>
>>>>>>>>>>>>>>> in this case. Otherwise the message wouldn't have ended up
>>>>>>>>>>>>>>> there.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> == Rene
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>> From: [email protected]
>>>>>>>>>>>>>>> [mailto:[email protected]] On Behalf Of Victor
>>>>>>>>>>>>>>> Luchitz
>>>>>>>>>>>>>>> Sent: maandag 5 juli 2010 20:36
>>>>>>>>>>>>>>> To: [email protected]
>>>>>>>>>>>>>>> Subject: smppbox code questions
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I have a few questions for you regarding the handling of
>>>>>>>>>>>>>>> DLR's by smppbox, which might also turn out to be bugs.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 1)
>>>>>>>>>>>>>>> In msg_to_pdu function there's a line which reads:
>>>>>>>>>>>>>>> dlr = dlr_find(msg->sms.boxc_id, msgid, msg->sms.receiver,
>>>>>>>>>>>>>>> dlrtype);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I think it's incorrect because when a DLR is stored by
>>>>>>>>>>>>>>> smppbox in handle_pdu, the boxc_id it uses is that from
>>>>>>>>>>>>>>> smpp_logins file (system_type). That in turn may cause
>>>>>>>>>>>>>>> dlr_find to always fail. So in my opinion the correct
>>>>>>>>>>>>>>> dlr_find
>>
>>>>>>>>>>>>>>> call is this:
>>>>>>>>>>>>>>> dlr = dlr_find(box->boxc_id, msgid, msg->sms.receiver,
>>>>>>>>>>>>>>> dlrtype);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2) another thing I find not quite correct is the way smppbox
>>>>>>>>>>>>>>> splits message ids for concatenated DLR's. Basically, what
>>>>>>>>>>>>>>> smppbox does is
>>>>>>>>>>>>>>> this:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> parts = octstr_split(msg->sms.dlr_url, octstr_imm(";"));
>>>>>>>>>>>>>>> msgid = gwlist_extract_first(parts); ...
>>>>>>>>>>>>>>> Then it loops through all elements of the 'parts' list and
>>>>>>>>>>>>>>> here is where the potential problem lies. smppbox assumes
>>>>>>>>>>>>>>> that msgid for the concatenated DLR is always equal to
>>>>>>>>>>>>>>> dlr_url
>>
>>>>>>>>>>>>>>> which is not always true.
>>>>>>>>>>>>>>> In fact, I think it's never true for concatenated DLR's
>>>>>>>>>>>>>>> stored by the dlr_add call in handle_pdu. Also, for example,
>>>>>>>>>>>>>>> the 'msgid' and 'dlrurls' columns in the storage table can
>>>>>>>>>>>>>>> have different maxiumum lengths, allowing truncation of the
>>>>>>>>>>>>>>> msgid. Here's my proposed fix - add the following bit of 
>>>>>>>>>>>>>>> code
>>>>>>>>>>>>>>> to msg_to_pdu:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> gwlist_destroy(parts, octstr_destroy_item); parts =
>>>>>>>>>>>>>>> octstr_split(dlr->sms.dlr_url, octstr_imm(";"));
>>>>>>>>>>>>>>> gwlist_extract_first(parts);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> right above the following bit:
>>>>>>>>>>>>>>> if (gwlist_len(parts) > 0) {
>>>>>>>>>>>>>>>    while ((msgid2 = gwlist_extract_first(parts)) != NULL) {
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Best regards,
>>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Best regards,
>>>>>>>>>>  Victor Luchitz
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best regards,
>>>>>>>>>  Victor Luchitz
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards,
>>>>>>>>  Victor Luchitz
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best regards,
>>>>>>>  Victor Luchitz
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards,
>>>>>>  Victor Luchitz
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards,
>>>>>  Victor Luchitz
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>>  Victor Luchitz
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Best regards,
>>>  Victor Luchitz
>>>
>>>
>>>
>>>
>>
>>
>>
>> -- 
>> Best regards,
>> Victor Luchitz
>>
>>
>>
>>
>>
>>
>>
>
>
>