[sr-dev] Migration of Open IMS Core to sip-router

[Andrei Pelinescu-Onciul]

On Jul 23, 2009 at 19:59, Dragos Vingarzan <dragos.vingarzan at gmail.com> wrote:
> Hi devs,
> 
[...]
>   * cdp - The CDiameterPeer
>   * icscf - I-CSCF functions
>   * isc - IMS Service Control - AS triggering and forwarding
>   * mgcf - some helpers for making some MGCF nodes attached to
>     softswitches (this is just an experimental collection of hacks)
>   * pcscf - P-CSCF functions
>   * scscf - S-CSCF functions
> 
> Other patches:
>   * dialog - small typos
>   * enum - support for query on the originating side, useful in PSTN
>     inbound processing
>   * ratelimit - a much extended ratelimit module, with multiple
>     queues, dynamic limiting capabilities based on internal/external
>     indicators, random retry-after capabilities, etc; which although
>     sent towards the SER trunk, never made it

IIRC initially the author thought it was not ready for ser. After that
it probably got lost in the noise (up until now I was sure it _was_ in
ser :-)).

>   * tls - some hashing functions
>   * tm - not really sure for now, but I hope we can reconcile without
>     changes on the sip-router side
>   * Core - support for emergency URI, some mem logging improvements, etc.

I remember the mem logging stuff. IIRC an extra config option for
turning it on/off is all that's missing (there are people for which the
 restart speed matters a lot and in some corner cases the mem logging
 summary can eat some time).
I'm not sure about he emergency URI, but we could add a compile def for
it (or even support it if it doesn't affect anything else).

> 
> I hope that we could integrate as much as possible from the changes that 
> we did as really we would like to converge rather than go our own 
> separate path. Sure, many parts are crazy-experimental and they would 
> probably stay like that unless someone would like to do a serious 
> re-implementation, but others I think would benefit outside of the IMS 
> targets that we had.
> 
> And here comes my first issue then. Whether you guys would be open to 
> host this in the same git repository or should we still maintain our own 
> separate svn tree? I started by adding a modules_osims directory and 
> putting there the differences. And so far it seems to be working for me 
> locally. Sure that the crazy-experimental parts won't make it to master, 
> but hopefully the Open IMS Core could leave entirely as just one modules 
> directory inside sip-router tree.

As Jan said there are several options. From my point of view the
separate branch doesn't make much sense, it's equivalent to a separate
git repo from the point of view of everyday work complexity, but a
separate repo would be more clean. So (again from my point of view),
it's either a separate git repo or a new modules_* directory.
I like the modules_osims directory idea, but if you choose to go this
path you should be aware that if you need to make changes to something
not in that directory and in particular core+tm there might be some
"conflicts". In particular you should take into account _slow_ response
times and/or integration for new features in core or tm.
> 
> If we'd stay separated, I would then try and find a way to link the git 
> repository as external in our svn one and overwrite with our changes.
> 
> 
> Then I am attaching some small patches for now, with more to come in the 
> next days as I will progress:
> 
> - sip-router_modules_s_dialog.diff - just some typos in logging
> - sip-router_pt.diff
>   - added a drop_my_process() function - in the cdp module (Diameter) 
> we do have dynamic processes, which fork and exit distinctly from the 
> ser ones, so we need this to clean-up. Without it, such usages would not 
> be possible as the process table would fill and then new forks would be 
> denied

I haven't looked at the patch yet, but why do you use ser fork_process()
 for these dynamic processes and not normal fork()?

>   - I have commented the pt.c 
>   <mailbox:///root/private/_mail/Mail/common/Sent?number=587016624#pt.c> 
>   line 210. I am not sure if this is a bug or I just used the fork_process 
> wrongly, but my process which was forked from a mod_init() not a 
> mod_child_init() opens some sockets, which are mistakenly closed on 
> fork_process() by the close_extra_socks(), which uses the pt[].unix_sock 
> values from other processes, which overlap. So please advise on what would 
> the best way be to still allow for the forked processes to open some 
> sockets without them being closed on fork.

I don't see how it could happen. All the new forked processes inherit
the unix socket used for communicating with tcp_main from all the
processes forked before. close_extra_socks() closes all this unneeded fd
in the new forked process.
If you use fork_process from mod_init, the most likely the process_count
is 0 at the time of forking => close_extra_socks() won't do anything.
Could you provide me with more info on how exactly you fork the
processes and what fds overlap?


> 
> Both these patches are required in order to get the Diameter stack working.
> 
> Then there is this one that we use quite a lot for tracking memory 
> leaks: http://tracker.iptel.org/browse/SER-224 . It's not there yet I 
> guess, so should I resubmit it or forget about it?  ;-) 
> 
> 
> I am looking forward to your feedback on how to proceed and on the 
> attached patches.

I'm for the separate module directory, with a suggestive name (like
modules_osims, modules_ims, osims a.s.o.), but if you need to make a lot
of experimental or bad-for-noims-use changes, the separate git repo it's
probably better.



Andrei