[Home]

Summary:ASTERISK-04040: [patch] regseconds is never non-zero in realtime
Reporter:Terry Wilson (twilson)Labels:
Date Opened:2005-05-02 20:04:47Date Closed:2008-01-15 15:36:04.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Registration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) regseconds.patch
Description:Since parse_contacts always sets p->expires = -1 when the SIP_REALTIME flag is set, the update_peer function always passes an expiry time of 0 to realtime_update_peer during a registration.  This in turn makes it impossible to share the sip registration database between two asterisk servers (when you are registering different users to both of them) because then the only place that we look up peers is in the astdb.

****** STEPS TO REPRODUCE ******

Set up two asterisk servers using sip realtime and sharing the same dialplan.
Take two phones that should be able to call each other and register one to box 1 and the other to box 2.  Try calling from one phone to the other.  It doesn't work, because asterisk assumes the registration has expired for the phone that is on the other server.

If they are on the same server, it falls back to the astdb where the expiry time is handled properly.

****** ADDITIONAL INFORMATION ******

As far as I can tell, this is how we've ALWAYS done realtime SIP so it confuses me that no one else has reported this.  If we really only wanted sip peers to register and immediately expire, though, I don't see why we would have the option to pass an expiry to realtime_update_peer in the first place...
Comments:By: Terry Wilson (twilson) 2005-05-02 20:08:04

and disclaimer on file... although do you need a disclaimer for only removing lines of code?  =)
By: Gunnar Harms (speedy) 2005-05-04 01:09:21

Thanks! Works great for me.
By: Kevin P. Fleming (kpfleming) 2005-05-04 13:51:44

That fix is not quite right... this a bit confusing, to say the least.

peer->expire is the ID of a scheduled event to expire the peer's registration.
peer->expiry is the number of seconds that the registration is supposed to last (from when it was received)

parse_contact is not updating peer->expiry at all, and I don't understand that.

You do _not_ want to schedule an expiration event for a SIP_REALTIME peer; that type of peer is temporary, and will go away as soon as the call it's involved in has completed. That's why peer->expire has to be -1 for a SIP_REALTIME peer.

However, I don't understand why update_peer is using that as a flag to force the expiry value to zero; it should just take the expiry value it is passed and do the proper thing with it. Also, I don't see update_peer being called if the peer forcibly de-REGISTERs itself either...

I need to set up a realtime installation here to be able to test this out, but I don't think the proposed solution is correct. There is a flaw here (actually multiple ones) that needs to be fixed, though.
By: Terry Wilson (twilson) 2005-05-04 14:40:03

I'm assuming temporary as in they aren't stored in memory?  Since they do acutally get an entry put into the astdb when they register (currently, atleast) that I'm assuming needs to be removed when the registration has expired--if it is indeed supposed to be written there (currently it looks like these entries stay forever!)
By: Mark Spencer (markster) 2005-05-04 15:16:59

I'm with kevin on this one.  Unless we're caching the peer, we wouldn't want to expire anything.
By: Terry Wilson (twilson) 2005-05-04 17:04:34

If I'm reading it right, a realtime peer is considered unregistered when its regseconds is in the past:

from build_peer
  if ((nowtime - regseconds) > 0) {
     memset(&peer->addr, 0, sizeof(peer->addr));
...

Since we are always setting regseconds to 0, we are saying that the peer is expired as soon as it is registered.  The ONLY reason that realtime sip looks like it works at all right now is that when a client registers, it adds the cached entry to the astdb which never goes away.  This bug has been necessary to provide a fix for the other bug.  :-)

I don't think we should be using the astdb at all for realtime SIP--the data is already in a db (a shareable one, at that).  We look up in the db every time anyway.  Second attempt at patch. regseconds2.patch.txt.


By: Terry Wilson (twilson) 2005-05-10 13:12:18

Is the new patch closer?  Or does it break something else?
By: Kevin P. Fleming (kpfleming) 2005-05-14 23:34:11

Yes, I think it looks fine, but I still haven't had time to set up a Realtime SIP installation here. Can you get some additional testing help from other users (via asterisk-dev or IRC) to confirm that this patch has the desired effect and no side-effects? This area of chan_sip is tricky to modify properly, I'd rather not break it :-)
By: Terry Wilson (twilson) 2005-05-20 10:28:44

I am currently inflicting the patch on my beta customers with no ill-effects as of yet.  Will also request help on IRC.  Updating patch to latest CVS (I think some whitespace changes broke it or something).
By: Mark Spencer (markster) 2005-05-25 13:39:39

Added to CVS.  I'm not convinced this is right, but it seems more right than what we have now.
By: Russell Bryant (russell) 2005-05-30 23:07:04

appears to only be an issue in realtime which is not in 1.0
By: Digium Subversion (svnbot) 2008-01-15 15:36:04.000-0600

Repository: asterisk
Revision: 5767

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r5767 | markster | 2008-01-15 15:36:04 -0600 (Tue, 15 Jan 2008) | 2 lines

Fix SIP registration (bug ASTERISK-4040)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5767