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 > >

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.

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

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