Summary:	ASTERISK-06145: [patch] iax2 registry does not work correctly with a dynamic dns server
Reporter:	Ivan F. Martinez (ivanfm)	Labels:
Date Opened:	2006-01-20 08:56:27.000-0600	Date Closed:	2006-06-07 12:44:57
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) asterisk_iax2_dns_v5.patch ( 1) asterisk_iax2_dns_v6.patch ( 2) asterisk_iax2_dns_v7.patch
Description:	when using register => in iax.conf it resolves de hostrname only at startup, and if sometime later the server changes the IP associated by the name, the connection was lost because chann_iax2 does not try to resolve IP again. ****** ADDITIONAL INFORMATION ****** I made a small patch to solve this. Can I send a scanned disclaimer by email instead of FAX or standard mail as defined in digium and voip-info.org sites?
Comments:	By: Russell Bryant (russell) 2006-01-20 11:51:32.000-0600 This is what the dnsmgr is for.  See configs/dnsmgr.conf.sample for information on how to configure managed dns lookups. Thank you for your contribution, anyway, though! By: Ivan F. Martinez (ivanfm) 2006-01-20 13:30:00.000-0600 thanks for showing dnsmgr : it is a very nice feature. but the iax2 register structure does not use it get the IP address. The peer code has support for dnsmgr but it does not affect the registrations list. I will try to change my patch to use dnsmgr also for registrations. By: Russell Bryant (russell) 2006-01-20 15:46:02.000-0600 Oops!  Sorry for 'jumping the gun' on this one. As far as the disclaimer goes, we really don't like to take them by email.  Hopefully one of the other methods can work for you. By: Ivan F. Martinez (ivanfm) 2006-01-20 18:33:55.000-0600 I have made the changes to use dnsmgr on register, I will test it better during the week and send a new patch. And I will check to send the disclaimer too. By: Ivan F. Martinez (ivanfm) 2006-01-21 07:41:56.000-0600 this new patch uses dnsmgr. some adjusts are made on dnsmgr code too, and cli command dnsmgr list was added. By: Ivan F. Martinez (ivanfm) 2006-01-21 18:21:26.000-0600 now I have added the patch with dnsmgr support. I have signed a disclaimer, and I will try to send it by fax. By: Ivan F. Martinez (ivanfm) 2006-01-25 07:30:10.000-0600 disclaimer sent by FAX yesterday. By: Olle Johansson (oej) 2006-03-10 07:12:53.000-0600 russell: Any thoughts? ivanfm: Please remove the // comments. (Read the coding guide lines). Also, maybe the cli command should be a separate patch in a separate report. Thanks. By: Ivan F. Martinez (ivanfm) 2006-03-14 04:20:42.000-0600 comments adjusted, cli code removed from this patch. please check what you think about the need for locking in ast_dnsmgr_refresh. In my mind probably it will not be necessary, but I make the comment to remember to check with asterisk developers. It will not be necessary because the chann_code use the data without concurrency. By: Russell Bryant (russell) 2006-05-03 21:03:47 Unfortunately, due to changes in the trunk, this patch no longer cleanly applies.    ivanfm, if you or someone else interested in this patch could update this to apply cleanly to the trunk, we can review it for inclusion before the 1.4 release. By: Serge Vecher (serge-v) 2006-05-26 10:50:36 ivanfm: need an updated patch here soon to make it into 1.4. Obrigado! By: Ivan F. Martinez (ivanfm) 2006-05-30 20:03:50 updated using tunk By: Joshua C. Colp (jcolp) 2006-06-05 10:45:38 Put into trunk as of revision 32304. Thanks! By: Russell Bryant (russell) 2006-06-05 11:13:28 After reviewing this code, I do not feel it was ready to be committed. ivanfm made comments here in this bug as well as in the code that he was not sure if ast_dnsmgr_refresh is threadsafe.  It is in fact, not threadsafe.  chan_iax2 can call ast_dnsmgr_refresh whenever it would like to, and the dnsmgr refresh thread could be doing it at the same time. It's not quite as simple as just locking the refresh lock around ast_dnsmgr_refresh when being called from an external module, because the dnsmgr refresh thread checks to see if the refresh lock is locked, and if it is, assumes that a full refresh is already in progress.  This means that a call to ast_dnsmgr_refresh could prevent the rest of the entries from getting refreshed. The mechanism for detecting when an entry has changed is also not threadsafe.  I think allowing a callback to be registered for when an entry changed would be a better solution for this.  That would allow chan_iax2 to immediately redo the registration, instead of waiting until the next retry on the registration to do it. I have reverted this code for now, until these issues can be addressed. By: Ivan F. Martinez (ivanfm) 2006-06-05 14:54:05 what you think about creating a new lock for each entry ? If you think this is safe I can change the code in ast_dnsmgr_lookup to use this entry lock. This make safe to external module call to lookup and also the internal refresh. the ast_dnsmgr_changed will be called on the routine wich uses the dnsmgr entry. The routine must be safe because the flag is zeroed when ast_dnsmgr_changed is called. An dnsmgr entry can't be shared between 2 modules. By: Russell Bryant (russell) 2006-06-06 09:12:26 I am fine with adding a lock on each dnsmgr entry. ast_dnsmgr_changed() will need to use it as well.  This is why ... + int ret = entry->changed; Say for example at this point, changed is zero because the entry has not changed. + entry->changed = 0; Meanwhile, the dnsmgr refresh thread is refreshing all of the entries and this entry may change.  Then, that thread could set it to one before this operation is completed, and ast_dnsmgr_changed immediately sets it back to zero. The worst thing that could happen is that a change is never detected.  But, it's still a race condition that can be easily handled by locking the entry in this function. By: Ivan F. Martinez (ivanfm) 2006-06-07 07:00:21 the new patch working fine here at this moment. By: Russell Bryant (russell) 2006-06-07 12:44:56 added to the trunk with some small modifications, thanks! rev 32817