<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hello,<br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 03/12/14 18:14, Lucian Balaceanu
      wrote:<br>
    </div>
    <blockquote cite="mid:547F44E3.3070607@1and1.ro" type="cite">
      <meta content="text/html; charset=windows-1252"
        http-equiv="Content-Type">
      Hi Daniel,<br>
      <br>
      Indeed, locally generated replies provoke a segmentation fault in
      my code because of the missing msg pointer in such cases.<br>
      I am currently testing a version where I generate the msg from the
      buf to be sent by using the parse_msg() function.<br>
      Conceptually, do you see any problems with this approach?<br>
    </blockquote>
    <br>
    there are other attributes that might not be set in this case, such
    as source ip/port, local ip/port...<br>
    <br>
    Cheers,<br>
    Daniel<br>
    <br>
    <blockquote cite="mid:547F44E3.3070607@1and1.ro" type="cite"> <br>
      Thank you,<br>
      Lucian<br>
      <br>
      On 11/27/2014 10:37 AM, Daniel-Constantin Mierla wrote:
      <blockquote cite="mid:5476E2E1.3050207@gmail.com" type="cite">
        <meta http-equiv="Content-Type" content="text/html;
          charset=windows-1252">
        Hi Lucian,<br>
        <br>
        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.<br>
        <br>
        <div class="moz-cite-prefix">On 27/11/14 09:09, Lucian Balaceanu
          wrote:<br>
        </div>
        <blockquote cite="mid:5476DC41.8010208@1and1.ro" type="cite">
           Hi Daniel,<br>
          <br>
          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:<br>
          <br>
          1. I am unsure if the place I introduced the run_onsend  call
          is appropriate  since the buf used for msg_send is constructed
          <br>
          build_res_buf_from_sip_req and build_res_buf_from_sip_res
          calls.<br>
        </blockquote>
        <br>
        These functions are to build the reply from request (local
        generated reply) or from the incoming reply (forwarding reply).<br>
        <br>
        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.<br>
        <br>
        <br>
        <blockquote cite="mid:5476DC41.8010208@1and1.ro" type="cite"> <br>
          2. Also, we can maybe unite these if call branches I created:<br>
              send_res = msg_send(&uas_rb->dst, buf, res_len);<br>
              send_res = SEND_PR_BUFFER( uas_rb, buf, res_len );<br>
        </blockquote>
        <br>
        SEND_PR_BUFFER seems to be a wrapper around msg_send() for
        safety and debugging processes, so it can be used.<br>
        <br>
        <blockquote cite="mid:5476DC41.8010208@1and1.ro" type="cite"> <br>
          3. Do you think a get_send_socket snippet as follows should be
          inserted before the <i><b>if
              (onsend_route_enabled(SIP_REPLY)){</b></i> :<br>
          <br>
              if(dst.send_sock == NULL) {<br>
                  dst.send_sock=get_send_socket(msg, &dst.to,
          dst.proto);<br>
                  if (dst.send_sock==0){<br>
                      LM_ERR("cannot forward reply\n");<br>
                  }<br>
              }<br>
        </blockquote>
        <br>
        Is the above referring to the patch attached? If the send_sock
        is needed elsewhere after the IF block, then should be inserted
        before.<br>
        <br>
        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.<br>
        <br>
        Cheers,<br>
        Daniel<br>
        <blockquote cite="mid:5476DC41.8010208@1and1.ro" type="cite"> <br>
          Thank you,<br>
          Lucian<br>
          <br>
          On 10/29/2014 02:15 PM, Daniel-Constantin Mierla wrote:
          <blockquote cite="mid:5450DA4B.9050906@gmail.com" type="cite">
            Hello Lucian,<br>
            <br>
            I applied your patch with some fixes.<br>
            <br>
            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.<br>
            <br>
            Cheers,<br>
            Daniel<br>
            <br>
            <div class="moz-cite-prefix">On 27/10/14 12:51, Lucian
              Balaceanu wrote:<br>
            </div>
            <blockquote cite="mid:544E31B9.9060802@1and1.ro" type="cite">
              Hello Daniel,<br>
              <br>
              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. <br>
              <br>
              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?<br>
              <br>
              Thank you,<br>
              Lucian Balaceanu<br>
              <br>
              <blockquote cite="mid:5448B8C5.5030007@1und1.de"
                type="cite">
                <div class="moz-forward-container"> Hi Lucian,<br>
                  <br>
                  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.<br>
                  <br>
                  <br>
                  <div class="moz-cite-prefix">On 17/09/14 18:09, Lucian
                    Balaceanu wrote:<br>
                  </div>
                  <blockquote cite="mid:5419B240.5000005@1and1.ro"
                    type="cite"> Hi Daniel,<br>
                    <br>
                    Please forgive me for my delay in responding to your
                    mail.<br>
                    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:<br>
                    <br>
                    <b>Issue1</b><br>
                    <blockquote cite="mid:53FDC46E.7050909@gmail.com"
                      type="cite"> From performances point of view,
                      there can be added a config parameter to enable
                      running of onsend_route for replies:<br>
                      <br>
                      onsend_route_reply = 0|1<br>
                    </blockquote>
                    <br>
                    Following <a moz-do-not-send="true"
                      class="moz-txt-link-freetext"
                      href="http://www.asipto.com/pub/kamailio-devel-guide/#c08add_parameters">http://www.asipto.com/pub/kamailio-devel-guide/#c08add_parameters</a>
                    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:<br>
                    <br>
                    <i> 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<br>
                       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: <br>
                      ERROR: bad config file (2 errors)</i><br>
                  </blockquote>
                  <br>
                  The issue is:<br>
                  <br>
                  <pre wrap="">+<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; }
</pre>
                  <br>
                  <blockquote cite="mid:5419B240.5000005@1and1.ro"
                    type="cite"> <br>
                    <b>Issue2</b><br>
                    <blockquote cite="mid:53FDC46E.7050909@gmail.com"
                      type="cite"> #define onsend_enabled(rtype)
(onsend_rt.rlist[DEFAULT_RT]?((rtype==SIP_REPLY)?onsend_route_reply:1):0)<br>
                    </blockquote>
                    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()? <br>
                  </blockquote>
                  <br>
                  This is to detect whether the onsend_route should be
                  executed for SIP replies. The condition being:<br>
                  <br>
                  - if is a sip reply and onsend_route is set and the
                  onsend_route_reply parameter is 1<br>
                  <blockquote cite="mid:5419B240.5000005@1and1.ro"
                    type="cite"> <br>
                    <b>Issue3</b><br>
                    <blockquote cite="mid:53FDC46E.7050909@gmail.com"
                      type="cite"> 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).<br>
                      <br>
                    </blockquote>
                    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).<br>
                    Could you point me to the reply relaying function
                    that is called for state-full processing? <br>
                  </blockquote>
                  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.<br>
                  <br>
                  Cheers,<br>
                  Daniel<br>
                  <blockquote cite="mid:5419B240.5000005@1and1.ro"
                    type="cite"> <br>
                    Thank you and sorry again for my late answer,<br>
                    Lucian<br>
                  </blockquote>
                  <br>
                  <pre class="moz-signature" cols="72">-- 
Daniel-Constantin Mierla
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://twitter.com/#%21/miconda">http://twitter.com/#!/miconda</a> - <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.linkedin.com/in/miconda">http://www.linkedin.com/in/miconda</a></pre>
                  <br>
                </div>
                <br>
              </blockquote>
              <br>
            </blockquote>
            <br>
            <pre class="moz-signature" cols="72">-- 
Daniel-Constantin Mierla
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://twitter.com/#%21/miconda">http://twitter.com/#!/miconda</a> - <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.linkedin.com/in/miconda">http://www.linkedin.com/in/miconda</a></pre>
          </blockquote>
          <br>
        </blockquote>
        <br>
        <pre class="moz-signature" cols="72">-- 
Daniel-Constantin Mierla
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://twitter.com/#%21/miconda">http://twitter.com/#!/miconda</a> - <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.linkedin.com/in/miconda">http://www.linkedin.com/in/miconda</a></pre>
      </blockquote>
      <br>
    </blockquote>
    <br>
    <pre class="moz-signature" cols="72">-- 
Daniel-Constantin Mierla
<a class="moz-txt-link-freetext" href="http://twitter.com/#!/miconda">http://twitter.com/#!/miconda</a> - <a class="moz-txt-link-freetext" href="http://www.linkedin.com/in/miconda">http://www.linkedin.com/in/miconda</a></pre>
  </body>
</html>