[OpenSER-Devel] discussion: issues with local_route

[Daniel-Constantin Mierla]

Hello everybody,

I want to discuss in this thread the issues the local_route created, 
showing why my concerns about this late commit were expressed.

Although may not be a big patch it is a major change as it introduces 
architectural changes in regard to route executions and related SIP 
messages. As you know openser uses a lot of global variables during 
routes execution (when I say routes I mean all type of routes: route, 
failure_route, branch_route, ...). All these variables create the 
context of the SIP message being processed at that time.

So far it was only the context of one SIP messages there, the message 
that was received on the network. Now we get nested contexts and things 
get really messed up.

The local_route can be triggered because:
- there is a command via MI interface
- there is a non SIP worker process that sends it (timer process: msilo, 
presence, etc)
- a SIP message that was received is processed and functions that send 
new messages are called (e.g., m_dump() call from msilo)

The third case breaks everything. Let me give an example:
- REGISTER is received and its context is created
- m_dump is called
- local_route is executed for each SIP MESSAGE sent by msilo
- the context is still the one from REGISTER for every MESSAGE while 
should be a new one
- changes in the REGISTER context are reflected in MESSAGE context and 
viceversa

To have everything working clear, the context of the REGISTER must be 
parked and restored after local_route execution. That would be very nice 
as it is a big step towards asynchronous processing.

However, here is a list with what I find subject for discussion, to be 
broken, not properly behavior or misleading (considering the timeline of 
last Friday evening):
- exit or drop have no effect, misleading because of their meaning
- setsflag() broken
- resetsflag() broken
- issflagset() broken
- setbflag() broken
- resetbflag() broken
- isbflagset() broken
- initial scripts flags are lost, last local_route script flags are 
getting into initial route script flags
- assignments of following pseudo variables is broken
    - $bf/$bF - branch flags
    - $sf/$sF - script flags
    - $br - adding branch
- usage of following pseudo-variables is broken
    - $Ts - time stamp
    - $Tf - formatted time
    - $br - branch
    - $bR - all branches
    - $time(...) PV class from cfgutils is broken
- avp_pushto("$br", ...) is broken
- avp_pushto("$ru", ...) with many avps for the second parameter is broken
- setdebug() is broken
- append_branch() is broken
- pv_printf(br, ...) (avp_printf(br, ...)) is broken
- have no time to check all the source code, but other developers can 
see if they use that -- the sip_msg->id is changed, so if you call a 
function that checks it to see if some cached data is still valid 
because it is same message you processed by the call of same function or 
another one, once in the initial route, then in the nested local_route, 
when you need it again in the initial route, the cached value is 
considered invalid although it is the message in current processing

The new code for local_route has some behaviors I don't really like, so 
I want discussions about:
- if the buffer with the SIP message required by the local_route to run 
cannot be created (no memory, etc), the message is sent without running 
local route - I would expect to be dropped and error returned upstream
- if the new SIP message after executing local_route cannot be printed 
to buffer, the initial message is sent - I would expect to be dropped 
and error returned upstream
- decide behavior for exit/drop

I haven't added description about all these broken situations because it 
will be long description, but I can provide if someone feels I am not 
right (and yes, I may not be right). It is what I found so far, but 
fixing it is no longer simple. Let's see how many of them are valid and 
decide afterwards what to do.

Waiting for your comments. Thanks!

Cheers,
Daniel

-- 
http://www.asipto.com