<div dir="ltr">Hi Daniel,<div>thanks for the feedback. About your questions: received ACK and CANCEL were not captured in old versions of the module, so no changes from this point of view.</div><div>But tracing and flagging every request would work without duplicated messages. Something like this</div><div><br></div>request_route {<div> ....<br><div> if (!is_method("OPTIONS")) {<br></div><div> setflag(CAPTURE_FLAG);<br> sip_trace();<br> }</div><div>}</div><div><br><div>I would expect the legacy behavior preserved, but I need to check better the code to understand how to do. If not possible we should add it to the documentation.</div><div>I will make some tests removing the trace_is_off check for the negative ACK and make a PR.</div><div>Finally I think that we also should align case 3) (siptrace + flag) with case 1) (transaction tracing) so that the behavior is the same (as I think was the intention originally).</div><div>What do you and other devs think?</div><div><br></div><div>Cheers,</div><div><br></div><div>Federico</div><div><br></div><div> </div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 31, 2020 at 4:25 PM Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com">miconda@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Hi Federico,</p>
<p>were the received ack and cancel captured automatically in the
old version when sip trace was set for invite? There were many
changes in the past years, but I remember that the flag was mainly
for outgoing requests and matching replies of the transaction for
which the flag was set. For incoming requests sip_trace() function
had to be used.</p>
<p>Based on your remark, I think that trace_tm_neg_ack_in() should
not check if the trace-is-off(). It should be set when trace-is-on
and that's it for the transaction.</p>
<p>Feel free to clarify (or propose) the wanted behaviour and then
we can work together to have it as expected. I used sip trace
lately for tracing all traffic (trace_mode=1), no longer doing any
filtering for transactions/dialogs.</p>
<p>Cheers,<br>
Daniel<br>
</p>
<div>On 31.03.20 09:09, Federico Cabiddu
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,
<div>I've been recently testing 5.3.x/master siptrace module, in
particular the new trace mode "t" vs the legacy flag +
sip_trace() mode and I've found some issues with the handling
of CANCEL. Specifically, I've tested the following scenarios:</div>
1) sip_trace_mode("t") on the initial INVITE only: received ACK
for negative replies not captured<br>
2) sip_trace_mode("t") on the initial INVITE and on neg ACK:
received ACK captured twice<br>
3) setflag and sip_trace() on the initial INVITE only: received
CANCEL and ACK not captured (outgoing yes)<br>
4) setflag and sip_trace() on the initial INVITE and ACK:
received CANCEL not captured, received ACK captured twice<br>
5) setflag and sip_trace() for each message (legacy): received
CANCEL and 200 captured twice, received ACK captured twice
<div><br>
</div>
<div>Digging into the module's code the "culprit" looks to be
trace_is_off function (<a href="https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L66" target="_blank">https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L66</a>)
and the places where it is called.</div>
<div>E.g.: for the case 1), when a negative reply is
received, trace_tm_neg_ack_in is called, which calls inside
trace_is_off (<a href="https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L1661" target="_blank">https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L1661</a>),
which cannot be true unless the ACK has been marked for
capture in the script, in which case it will be capture twice
(case 2). The same applies to the CANCEL for case 3),
in trace_onreq_out (callback for TMCB_E2ECANCEL_IN)
trace_is_off because the incoming message is not flagged. Case
3) should theoretically behave like case 1) according to
commit <a href="https://github.com/kamailio/kamailio/commit/40e09d8625184f19ff5666a2848cbb8c6212db26" target="_blank">https://github.com/kamailio/kamailio/commit/40e09d8625184f19ff5666a2848cbb8c6212db26</a>.</div>
<div><br>
</div>
<div>I'm not really sure if (and how) modify the trace_is_off
function or not calling it in specific cases. E.g.: why
calling it in trace_tm_neg_ack_in? This callback is set when
we explicity want to trace a transaction, so why checking
inside if tracing is on? Maybe I'm missing something, but I
think that probably the different behaviors of the modes
should be better specified/decided.</div>
<div><br>
</div>
<div>Best regards,</div>
<div><br>
</div>
<div>Federico</div>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
Kamailio (SER) - Development Mailing List
<a href="mailto:sr-dev@lists.kamailio.org" target="_blank">sr-dev@lists.kamailio.org</a>
<a href="https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev</a>
</pre>
</blockquote>
<pre cols="72">--
Daniel-Constantin Mierla -- <a href="http://www.asipto.com" target="_blank">www.asipto.com</a>
<a href="http://www.twitter.com/miconda" target="_blank">www.twitter.com/miconda</a> -- <a href="http://www.linkedin.com/in/miconda" target="_blank">www.linkedin.com/in/miconda</a></pre>
</div>
</blockquote></div>