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

[Dragos Vingarzan]

Hi Andrei,

Andrei Pelinescu-Onciul wrote:
>>   * 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 :-)).
>   
it seems that it made it though here and even with improvements as 
Ovidiu Sas imported in in Kamailio, so the functionality, although not 
code-line identical should all be in modules_k/ratelimit

>>   * 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 see. OK, then I'll come back with the added flag ;-). Do you think 
that it would be useful if we'd demand some unfix parameter functions 
for the memory that the module fixes do? AFAICT, this is still one of 
the main roadblocks (if not the only) in having an empty mem list on ser 
nice exit...
> 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 already posted the patch once, so here it is again. Not a big deal, 
just some sensible support for parsing and then some more cases in some 
switches before declaring it as invalid. I guess it won't hurt to have 
it in even without flags.
> 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.
>   
I really hope that we won't make changes to core+tm... We did some for 
the core as we needed this for the diameter integration and there are 
some other stuff that we might be somehow in front, like the emergency 
URN. But this are small occurrences for which, like you said, I would 
not sacrifice the advantages of not being on a different branch of sr.

And after all... OSIMS is not much more than some other way of 
configuring and running sr... really, IMS not reinvent the wheel or 
anything :-p.

>> - 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()?
>   
well... I like to have all the advantages that a ser process has, like 
being able to process SIP messages, without having a big interface back 
to ser for pushing back events and actions.
>>   - 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?
>   
Actually, now that I looked again, I see that I am only initializing 
some variables from mod_init() and then forking from mod_child_init(), 
but only from one from PROC_MAIN.

/**
 *	Child init function.
 * - starts the DiameterPeer by forking the processes
 * @param rank - id of the child 
 */
static int cdp_child_init( int rank )
{
	if (rank == PROC_MAIN) { 
		LOG(L_INFO,"INFO:"M_NAME":cdp_child_init(): CDiameterPeer starting ...\n");		
		diameter_peer_start(0);
		LOG(L_INFO,"INFO:"M_NAME":cdp_child_init(): ... CDiameterPeer started\n");		
	}
	
	return 0;
}

Then one of the processes will later on get a fd on a newly opened 
socket and then fork, which fd looks like already in pt[].unix_sock for 
another process. So these are the sockets for communicating with the 
"owner" of the SIP TCP sockets? So there are a lot of assumptions there 
about what you do with the forked processes... more than I actually took 
into consideration.

I'll try and debug this further too and come-up with a better 
description of the issue. In case you'd like to try it for yourself, the 
module cdp is here: 
http://svn.berlios.de/svnroot/repos/openimscore/ser_ims/trunk/modules/cdp/ 
. It should be compilable under sr with the previous sip-router_pt.diff 
which adds the drop_my_process() function.


> 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.
>   
I am leaning also towards the modules_osims. Then if you have an 
experimental flag that you could put on the entire directory, it would 
be great. For some parts, like the cdp, if the users like it we could 
just as well get it stabilized and moved out. But some other parts would 
probably be just too crazy to be used in a real exploitation environment 
and I hope that this won't cause support confusions. Ultimately, we just 
want that our users could pull the osims together with the latest sr and 
have cross benefits.

Cheers,
-Dragos

-------------- next part --------------
A non-text attachment was scrubbed...
Name: sip-router_urn.diff
Type: text/x-patch
Size: 2556 bytes
Desc: not available
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20090729/4edfffd6/attachment.bin>