[sr-dev] Is there a sense in calling run_onsend() after the msg_send() function?

[Lucian Balaceanu]

Hi Daniel,

Indeed the case with the state full replies seems not to be taken care 
of. I attach a patch with the proposed solution.
Could you please provide feedback?

1. I am unsure if the place I introduced the run_onsend  call is 
appropriate  since the buf used for msg_send is constructed
build_res_buf_from_sip_req and build_res_buf_from_sip_res calls.

2. Also, we can maybe unite these if call branches I created:
     send_res = msg_send(&uas_rb->dst, buf, res_len);
     send_res = SEND_PR_BUFFER( uas_rb, buf, res_len );

3. Do you think a get_send_socket snippet as follows should be inserted 
before the /*if (onsend_route_enabled(SIP_REPLY)){*/ :

     if(dst.send_sock == NULL) {
         dst.send_sock=get_send_socket(msg, &dst.to, dst.proto);
         if (dst.send_sock==0){
             LM_ERR("cannot forward reply\n");
         }
     }

Thank you,
Lucian

On 10/29/2014 02:15 PM, Daniel-Constantin Mierla wrote:
> Hello Lucian,
>
> I applied your patch with some fixes.
>
> I haven't checked with stateful replies, at some point a function from 
> core should be used. You can go ahead and see if it works, if not, let 
> me know and I can look into it as well. You can follow the callbacks 
> for TMCB_RESPONSE_OUT or TMCB_RESPONSE_FWDED inside tm code, they 
> should lead to the place where a sip response is going to be sent out.
>
> Cheers,
> Daniel
>
> On 27/10/14 12:51, Lucian Balaceanu wrote:
>> Hello Daniel,
>>
>> I must admit I only saw your mail last Friday. Until the 10th of 
>> October I was also on vacation. I know that you actually committed 
>> some of the changes together with your comments on the 12th this month.
>>
>> I don't know if we can consider the topic of the patch closed. As far 
>> as I understand, the state-full replies have not been addressed, 
>> right? (There should be a change in the t_reply.c) I followed the 
>> code to the relay_reply but I did not yet come to find the send 
>> function. Should I pursue further?
>>
>> Thank you,
>> Lucian Balaceanu
>>
>>> Hi Lucian,
>>>
>>> somehow I forgot to follow up on this. But we need to get sorted out 
>>> soon, before we release, so it works as expected with the new 
>>> version. See more comments inline.
>>>
>>>
>>> On 17/09/14 18:09, Lucian Balaceanu wrote:
>>>> Hi Daniel,
>>>>
>>>> Please forgive me for my delay in responding to your mail.
>>>> Please find attached a second version of the onsend_route_reply 
>>>> patch (which again has some problems). As per your previous 
>>>> indications I did the following:
>>>>
>>>> *Issue1*
>>>>> From performances point of view, there can be added a config 
>>>>> parameter to enable running of onsend_route for replies:
>>>>>
>>>>> onsend_route_reply = 0|1
>>>>
>>>> Following 
>>>> http://www.asipto.com/pub/kamailio-devel-guide/#c08add_parameters I 
>>>> have tried to add onsend_route_reply parameter. The code compiles, 
>>>> but when trying to start kamailio with this parameter inside, the 
>>>> parsing fails with syntax errors signaling:
>>>>
>>>> / 0(1321) :<core> [cfg.y:3423]: yyerror_at(): parse error in config 
>>>> file kamailio-basic.cfg.4.1, from line 107, column 1 to line 108, 
>>>> column 0: syntax error
>>>>  0(1321) : <core> [cfg.y:3423]: yyerror_at(): parse error in config 
>>>> file kamailio-basic.cfg.4.1, from line 107, column 1 to line 108, 
>>>> column 0:
>>>> ERROR: bad config file (2 errors)/
>>>
>>> The issue is:
>>>
>>> +<INITIAL>{ONSEND_RT_REPLY}	{  yylval.intval=atoi(yytext);
>>> +						yy_number_str=yytext; return NUMBER; }
>>>
>>> It should be:
>>>
>>> +<INITIAL>{ONSEND_RT_REPLY}	{  yylval.intval=atoi(yytext);
>>> +						yy_number_str=yytext; return ONSEND_RT_REPLY; }
>>>
>>>>
>>>> *Issue2*
>>>>> #define onsend_enabled(rtype) 
>>>>> (onsend_rt.rlist[DEFAULT_RT]?((rtype==SIP_REPLY)?onsend_route_reply:1):0)
>>>> That is to say you see it best to take the chek for 
>>>> onsend_rt.list[DEFAULT_RT] from inside run_onsend() function and 
>>>> call this onsend_enabled(...) before the run_onsend()?
>>>
>>> This is to detect whether the onsend_route should be executed for 
>>> SIP replies. The condition being:
>>>
>>> - if is a sip reply and onsend_route is set and the 
>>> onsend_route_reply parameter is 1
>>>>
>>>> *Issue3*
>>>>> On the other hand, is onsend_route also executed for local 
>>>>> requests? I had in mind it is only for received requests that are 
>>>>> forwarded ... Iirc, on onsend_route, the sip message is the one 
>>>>> received, the outgoing content being accessible via $snd(buf).
>>>>>
>>>> I agree with you with taking out the locally generated requests and 
>>>> only left the run_onsend call in do_forward_reply function (inside 
>>>> forward.c).
>>>> Could you point me to the reply relaying function that is called 
>>>> for state-full processing?
>>> Stateful processing for replies is mainly done in t_reply.c from tm 
>>> module. At some point there should be a send buffer function call.
>>>
>>> Cheers,
>>> Daniel
>>>>
>>>> Thank you and sorry again for my late answer,
>>>> Lucian
>>>
>>> -- 
>>> Daniel-Constantin Mierla
>>> http://twitter.com/#!/miconda  -http://www.linkedin.com/in/miconda
>>>
>>>
>>
>
> -- 
> Daniel-Constantin Mierla
> http://twitter.com/#!/miconda  -http://www.linkedin.com/in/miconda

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20141104/25648cc6/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: onsend_route_reply_tm.patch
Type: text/x-patch
Size: 3030 bytes
Desc: not available
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20141104/25648cc6/attachment-0001.bin>