[OpenSER-Devel] discussion: issues with local_route

[Dan Pascu]

On Tuesday 24 June 2008, Daniel-Constantin Mierla wrote:
> >
> > What exactly is broken? If I run openser with my 1.3 script will it
> > work exactly the same or not?
>
> Could you please review the messages in the beginning of this email
> thread? I listed couple of them there, giving examples and details.

I've reviewed it and I've seen nothing broken if local_route is not used 
at all, and a few (minor IMO) items that are fixable if it is used.

> Again in summary...
>
>  local_route changed the internal architecture of openser -- while so
> far there was only one SIP message context while running the route
> blocks (incoming sip message), now we have nested message contexts.
> Entire code of openser has to be reviewed to ensure proper
> functionality. Functions and functionalities so far were designed and
> implemented within the old architecture.

You keep repeating this big architectural change argument, but I've seen 
no evidence of such a thing. Not in the examples you gave.

> When local_route was introduced I asked time for other developers to
> review their code and properly integrate with local_route -- I saw that
> was understood as a run over the modules and enabling local_route for
> many functions, not letting the developers to do it.

Here I agree with you. I feel the functions shouldn't have been mass 
enabled, instead they should have been gradually enabled over time as 
appropriate.

> I have never said to remove the code, but to enclose it in defines and
> disable it for 1.4. This is, in my opinion, the safest solution to have
> a coherent and robust 1.4 release. 

Which I support if I see any example of a thing that is so broken that it 
cannot be fixed. So far I've only seen examples of things that can be 
fixed and things that that do not work there because they have no 
relationship with the local_route at all.

> If decision is to go for fixing the 
> code with local_route enabled, then will be kind of chaos, each
> developer will fix as it considers his part of code, either disabling
> it for local_route, because code was not designed for nested
> environments and does not want to load unnecessary bug reports, the
> problem being the design of local_route, or making other workarounds.
> Probably the local_route will be pretty much unusable.

I can log outgoing NOTIFY and BYE messages and I can enable accounting for 
the internally generated BYEs from the dialog module. I'd say it's pretty 
much usable for me. In fact this was the main reason I asked for this 
functionality and for a first incarnation, it manages to cover it pretty 
well.

> > Yet, we have the same issues with reply_route and error_route. There
> > are operations that make no sense there either. If you try them, they
> > are either silently ignored or you get an error. What makes the
> > local_route inconsistencies more relevant, while the other
> > inconsistencies do not bother you?
>
> Just read the initial messages and you will understand. There are side
> effects, big ones and critical.

I've read them, but I can't find any big and critical side effect. I see 
some examples about minor issues that can be fixed easily. Can you be so 
kind as to pinpoint exactly to such an example which you find the most 
critical?

>
> > You do remember that when first introduced, AVPs could not be
> > accessed in the reply_route.
>
> Let me expose the right history. When avps where introduced, the only
> functions managing them in config were from avpops where the functions
> flags denied usage in reply_route.
>
> When the avps could be used directly in the config file, you could use
> them anywhere and they were and still are safe. I believe you mix the
> avp attached to transaction (those attached to request) and avps
> attached to reply. If you play with the parameter of the TM module,
> then you get in onreply_route the transaction avps, otherwise you get
> the avps attached to the reply.

Yes Daniel, I meant the transaction avps and there is not need to nitpick 
on definitions, you understand pretty well what I meant. The TM module 
parameter was not there 2 releases ago, which means avps were not safe in 
the reply route, in fact they weren't even exposed for this reason, which 
makes my point that some functionality was committed incomplete in the 
beginning and only extended later.

> > If it has to go down this path, then why don't we pick up on some
> > functionality that is already in openser for 4+ generations and it
> > has issues. TCP/TLS support is in there since 0.9.0 and today we
> > still can't use it for end user devices. The consensus when this
> > issue is raised, is that it is only supported for server connections,
> > but I can also say that why should the end user device use case be
> > labeled "inappropriate usage" when we target to release quality
> > software?
> >
> > So what do you say, if I connect 20000 SIP phone via TCP/TLS to the
> > proxy will it be able to work properly as it does with UDP? If not,
> > what should we do, should we remove TCP/TLS support because it is
> > incomplete for this use case?
>
> I do not understand the point and relation with the discussion. Sounds
> like, just as stupid example, 

It's not a stupid example, it's about the same thing: some incomplete 
functionality (caused by design this time) which by your definition 
reduces the quality of the software, but against which you did not chose 
to wage a war.

> if 10 billions processes do better than 
> 10billions threads, shall the  multi-thread support in kernel be
> removed?

Exactly my point. If local_route is not as complete as it can be in the 
future, should it be bashed for this reason?

> >> To go further and complete the 1.4 release, the solution I see is to
> >> enclose local_route in defines in disable it by default. Who wants
> >> to use it, can recompile and reinstall from sources.
> >
> > As I already said, I will support you in this direction if we can
> > find any problems that break the existing proxy functionality and
> > cannot be fixed easily. However if there are no show stoppers, only
> > incomplete stuff, I have to disagree. Every new functionality was
> > incomplete in the beginning, and required multiple releases until it
> > reached maturity and sometimes even required design changes between
> > those releases to allow it to be implemented in a more complete form.
> > Nothing new with local_route.
>
> Again, it is nothing about completeness, it is about breaking lot of
> other things, changing the internal architecture. 

You keep saying this. But please, for those of us that do not see it, 
provide one example (the most important in your opinion from the things 
you mentioned) that has such a big unfixable impact.

> It is not about 
> supporting one or another, any developer has the right to rise and
> expose the problems, not agree when something is broken 

> and take care of its code to be protected.

Sure, which is exactly why I'd like to see this functionality made 
available because it provides means to do some very needed stuff and to 
date I have not seen a single shred of evidence about it breaking 
something fundamental that cannot be fixed as bugs.

So please provide one such example, and I'm with you in making this a 
compile time option and disabling it by default in 1.4

-- 
Dan