[sr-dev] Is there a sense in calling run_onsend() after the msg_send() function?
Lucian Balaceanu
lucian.balaceanu at 1and1.ro
Wed Dec 3 18:14:11 CET 2014
Hi Daniel,
Indeed, locally generated replies provoke a segmentation fault in my
code because of the missing msg pointer in such cases.
I am currently testing a version where I generate the msg from the buf
to be sent by using the parse_msg() function.
Conceptually, do you see any problems with this approach?
Thank you,
Lucian
On 11/27/2014 10:37 AM, Daniel-Constantin Mierla wrote:
> Hi Lucian,
>
> the patch is still in radar, just that last month was crazy busy from
> various aspects and didn't get the time to handle it.
>
> On 27/11/14 09:09, Lucian Balaceanu wrote:
>> Hi Daniel,
>>
>> I must admit this run_onsend() patch for stateful replies creation
>> was not quite a success story. However, I think it serves a practical
>> purpose, for example in Homer tracing and could be useful for the
>> community. Again, I propose my past solution, with some questions:
>>
>> 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.
>
> These functions are to build the reply from request (local generated
> reply) or from the incoming reply (forwarding reply).
>
> I am wondering what would take to call onsend route for both forwarded
> and local generated messages (no matter is request or reply) -- might
> become simpler by having the code in the wrapper function sending to
> the network from core, on the other hand could break scripting as no
> msg structure is available for local generated messages. Otherwise the
> benefit can be also in coherency.
>
>
>>
>> 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 );
>
> SEND_PR_BUFFER seems to be a wrapper around msg_send() for safety and
> debugging processes, so it can be used.
>
>>
>> 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");
>> }
>> }
>
> Is the above referring to the patch attached? If the send_sock is
> needed elsewhere after the IF block, then should be inserted before.
>
> One remark, new variables should be declared at the beginning of the
> blocks, you mixed the style with two new vars you added -- this is
> more a suggestions as we try to stay compatible with old c standards
> -- I know it is not everywhere in the code, but we should aim at it.
>
> Cheers,
> Daniel
>>
>> 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
>>
>
> --
> 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/20141203/a52731f3/attachment-0001.html>
More information about the sr-dev
mailing list