[sr-dev] siptrace inconsistency

[Federico Cabiddu]

Hi Daniel,
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.
But tracing and flagging every request would work without duplicated
messages. Something like this

request_route {
    ....
    if (!is_method("OPTIONS")) {
        setflag(CAPTURE_FLAG);
        sip_trace();
    }
}

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.
I will make some tests removing the trace_is_off check for the negative ACK
and make a PR.
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).
What do you and other devs think?

Cheers,

Federico



On Tue, Mar 31, 2020 at 4:25 PM Daniel-Constantin Mierla <miconda at gmail.com>
wrote:

> Hi Federico,
>
> 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.
>
> 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.
>
> 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.
>
> Cheers,
> Daniel
> On 31.03.20 09:09, Federico Cabiddu wrote:
>
> Hi all,
> 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:
> 1) sip_trace_mode("t") on the initial INVITE only: received ACK for
> negative replies not captured
> 2) sip_trace_mode("t") on the initial INVITE and on neg ACK: received ACK
> captured twice
> 3) setflag and sip_trace() on the initial INVITE only: received CANCEL and
> ACK not captured (outgoing yes)
> 4) setflag and sip_trace() on the initial INVITE and ACK: received CANCEL
> not captured, received ACK captured twice
> 5) setflag and sip_trace() for each message (legacy): received CANCEL and
> 200 captured twice, received ACK captured twice
>
> Digging into the module's code the "culprit" looks to be trace_is_off
> function (
> https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L66)
> and the places where it is called.
> 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 (
> https://github.com/kamailio/kamailio/blob/2768f8ce1cf6da242674e7e40c8e76eb6c630f6b/src/modules/siptrace/siptrace.c#L1661),
> 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
> https://github.com/kamailio/kamailio/commit/40e09d8625184f19ff5666a2848cbb8c6212db26
> .
>
> 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.
>
> Best regards,
>
> Federico
>
> _______________________________________________
> Kamailio (SER) - Development Mailing Listsr-dev at lists.kamailio.orghttps://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
>
> --
> Daniel-Constantin Mierla -- www.asipto.comwww.twitter.com/miconda -- www.linkedin.com/in/miconda
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20200401/3ae44292/attachment.html>