[asterisk-commits] twilson: branch 10 r353397 - in /branches/10: ./ channels/ include/asterisk/ ...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Jan 30 17:28:17 CST 2012
Author: twilson
Date: Mon Jan 30 17:28:10 2012
New Revision: 353397
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=353397
Log:
Re-link peers by IP when dnsmgr changes the IP
Asterisk's dnsmgr currently takes a pointer to an ast_sockaddr and updates it
anytime an address resolves to something different. There are a couple of
issues with this. First, the ast_sockaddr is usually the address of an
ast_sockaddr inside a refcounted struct and we never bump the refcount of those
structs when using dnsmgr. This makes it possible that a refresh could happen
after the destructor for that object is called (despite ast_dnsmgr_release
being called in that destructor). Second, the module using dnsmgr cannot be
aware of an address changing without polling for it in the code. If an action
needs to be taken on address update (like re-linking a SIP peer in the
peers_by_ip table), then polling for this change negates many of the benefits
of having dnsmgr in the first place.
This patch adds a function to the dnsmgr API that calls an update callback
instead of blindly updating the address itself. It also moves calls to
ast_dnsmgr_release outside of the destructor functions and into cleanup
functions that are called when we no longer need the objects and increments the
refcount of the objects using dnsmgr since those objects are stored on the
ast_dnsmgr_entry struct. A helper function for returning the proper default SIP
port (non-tls vs tls) is also added and used.
This patch also incorporates changes from a patch posted by Timo Teräs to
ASTERISK-19106 for related dnsmgr issues.
(closes issue ASTERISK-19106)
Review: https://reviewboard.asterisk.org/r/1691/
........
Merged revisions 353371 from http://svn.asterisk.org/svn/asterisk/branches/1.8
Modified:
branches/10/ (props changed)
branches/10/channels/chan_sip.c
branches/10/include/asterisk/dnsmgr.h
branches/10/main/dnsmgr.c
Propchange: branches/10/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: branches/10/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/channels/chan_sip.c?view=diff&rev=353397&r1=353396&r2=353397
==============================================================================
--- branches/10/channels/chan_sip.c (original)
+++ branches/10/channels/chan_sip.c Mon Jan 30 17:28:10 2012
@@ -2894,6 +2894,11 @@
if (which == SIP_PEERS_ALL || peer->the_mark) {
peer_sched_cleanup(peer);
+ if (peer->dnsmgr) {
+ ast_dnsmgr_release(peer->dnsmgr);
+ peer->dnsmgr = NULL;
+ sip_unref_peer(peer, "Release peer from dnsmgr");
+ }
return CMP_MATCH;
}
return 0;
@@ -4677,8 +4682,6 @@
ao2_t_ref(peer->auth, -1, "Removing peer authentication");
peer->auth = NULL;
}
- if (peer->dnsmgr)
- ast_dnsmgr_release(peer->dnsmgr);
if (peer->socket.tcptls_session) {
ao2_ref(peer->socket.tcptls_session, -1);
@@ -5418,6 +5421,12 @@
return 0;
}
+/*! \brief The default sip port for the given transport */
+static inline int default_sip_port(enum sip_transport type)
+{
+ return type == SIP_TRANSPORT_TLS ? STANDARD_TLS_PORT : STANDARD_SIP_PORT;
+}
+
/*! \brief create address structure from device name
* Or, if peer not found, find it in the global DNS
* returns TRUE (-1) on failure, FALSE on success */
@@ -5520,9 +5529,7 @@
}
if (!ast_sockaddr_port(&dialog->sa)) {
- ast_sockaddr_set_port(&dialog->sa,
- (dialog->socket.type == SIP_TRANSPORT_TLS) ?
- STANDARD_TLS_PORT : STANDARD_SIP_PORT);
+ ast_sockaddr_set_port(&dialog->sa, default_sip_port(dialog->socket.type));
}
ast_sockaddr_copy(&dialog->recv, &dialog->sa);
return 0;
@@ -5742,7 +5749,6 @@
ast_string_field_free_memory(reg);
ast_atomic_fetchadd_int(®objs, -1);
- ast_dnsmgr_release(reg->dnsmgr);
ast_free(reg);
}
@@ -5756,7 +5762,6 @@
AST_SCHED_DEL(sched, mwi->resub);
ast_string_field_free_memory(mwi);
- ast_dnsmgr_release(mwi->dnsmgr);
ast_free(mwi);
}
@@ -12686,6 +12691,72 @@
return 0;
}
+static void on_dns_update_registry(struct ast_sockaddr *old, struct ast_sockaddr *new, void *data)
+{
+ struct sip_registry *reg = data;
+ const char *old_str;
+
+ /* This shouldn't happen, but just in case */
+ if (ast_sockaddr_isnull(new)) {
+ ast_debug(1, "Empty sockaddr change...ignoring!\n");
+ return;
+ }
+
+ if (!ast_sockaddr_port(new)) {
+ ast_sockaddr_set_port(new, reg->portno);
+ }
+
+ old_str = ast_strdupa(ast_sockaddr_stringify(old));
+
+ ast_debug(1, "Changing registry %s from %s to %s\n", S_OR(reg->peername, reg->hostname), old_str, ast_sockaddr_stringify(new));
+ ast_sockaddr_copy(®->us, new);
+}
+
+static void on_dns_update_peer(struct ast_sockaddr *old, struct ast_sockaddr *new, void *data)
+{
+ struct sip_peer *peer = data;
+ const char *old_str;
+
+ /* This shouldn't happen, but just in case */
+ if (ast_sockaddr_isnull(new)) {
+ ast_debug(1, "Empty sockaddr change...ignoring!\n");
+ return;
+ }
+
+ if (!ast_sockaddr_isnull(&peer->addr)) {
+ ao2_unlink(peers_by_ip, peer);
+ }
+
+ if (!ast_sockaddr_port(new)) {
+ ast_sockaddr_set_port(new, default_sip_port(peer->socket.type));
+ }
+
+ old_str = ast_strdupa(ast_sockaddr_stringify(old));
+ ast_debug(1, "Changing peer %s address from %s to %s\n", peer->name, old_str, ast_sockaddr_stringify(new));
+
+ ao2_lock(peer);
+ ast_sockaddr_copy(&peer->addr, new);
+ ao2_unlock(peer);
+
+ ao2_link(peers_by_ip, peer);
+}
+
+static void on_dns_update_mwi(struct ast_sockaddr *old, struct ast_sockaddr *new, void *data)
+{
+ struct sip_subscription_mwi *mwi = data;
+ const char *old_str;
+
+ /* This shouldn't happen, but just in case */
+ if (ast_sockaddr_isnull(new)) {
+ ast_debug(1, "Empty sockaddr change...ignoring!\n");
+ return;
+ }
+
+ old_str = ast_strdupa(ast_sockaddr_stringify(old));
+ ast_debug(1, "Changing mwi %s from %s to %s\n", mwi->hostname, old_str, ast_sockaddr_stringify(new));
+ ast_sockaddr_copy(&mwi->us, new);
+}
+
/*! \brief Actually setup an MWI subscription or resubscribe */
static int __sip_subscribe_mwi_do(struct sip_subscription_mwi *mwi)
{
@@ -12695,7 +12766,11 @@
snprintf(transport, sizeof(transport), "_%s._%s", get_srv_service(mwi->transport), get_srv_protocol(mwi->transport));
mwi->us.ss.ss_family = get_address_family_filter(&bindaddr); /* Filter address family */
- ast_dnsmgr_lookup(mwi->hostname, &mwi->us, &mwi->dnsmgr, sip_cfg.srvlookup ? transport : NULL);
+ ASTOBJ_REF(mwi); /* Add a ref for storing the mwi on the dnsmgr for updates */
+ ast_dnsmgr_lookup_cb(mwi->hostname, &mwi->us, &mwi->dnsmgr, sip_cfg.srvlookup ? transport : NULL, on_dns_update_mwi, mwi);
+ if (!mwi->dnsmgr) {
+ ASTOBJ_UNREF(mwi, sip_subscribe_mwi_destroy); /* dnsmgr disabled, remove reference */
+ }
}
/* If we already have a subscription up simply send a resubscription */
@@ -13376,17 +13451,8 @@
}
if (r->dnsmgr) {
- struct sip_peer *peer;
/* If the registration has timed out, maybe the IP changed. Force a refresh. */
ast_dnsmgr_refresh(r->dnsmgr);
- /* If we are resolving a peer, we have to make sure the refreshed address gets copied */
- if ((peer = sip_find_peer(r->hostname, NULL, TRUE, FINDPEERS, FALSE, 0))) {
- ast_sockaddr_copy(&peer->addr, &r->us);
- if (r->portno) {
- ast_sockaddr_set_port(&peer->addr, r->portno);
- }
- peer = sip_unref_peer(peer, "unref after sip_find_peer");
- }
}
/* If the initial tranmission failed, we may not have an existing dialog,
@@ -13475,7 +13541,12 @@
* or peer NULL. Since we're only concerned with its existence, we're not going to
* bother getting a ref to the proxy*/
if (!obproxy_get(r->call, peer)) {
- ast_dnsmgr_lookup(peer ? peer->tohost : r->hostname, &r->us, &r->dnsmgr, sip_cfg.srvlookup ? transport : NULL);
+ registry_addref(r, "add reg ref for dnsmgr");
+ ast_dnsmgr_lookup_cb(peer ? peer->tohost : r->hostname, &r->us, &r->dnsmgr, sip_cfg.srvlookup ? transport : NULL, on_dns_update_registry, r);
+ if (!r->dnsmgr) {
+ /*dnsmgr refresh disabled, no reference added! */
+ registry_unref(r, "remove reg ref, dnsmgr disabled");
+ }
}
if (peer) {
peer = sip_unref_peer(peer, "removing peer ref for dnsmgr_lookup");
@@ -13508,18 +13579,21 @@
}
/* Use port number specified if no SRV record was found */
- if (!ast_sockaddr_port(&r->us) && r->portno) {
- ast_sockaddr_set_port(&r->us, r->portno);
- }
-
- /* It is possible that DNS is unavailable at the time the peer is created. Here, if
- * we've updated the address in the registry, we copy it to the peer so that
- * create_addr() can copy it to the dialog via create_addr_from_peer */
- if ((peer = sip_find_peer(r->hostname, NULL, TRUE, FINDPEERS, FALSE, 0))) {
- if (ast_sockaddr_isnull(&peer->addr) && !(ast_sockaddr_isnull(&r->us))) {
- ast_sockaddr_copy(&peer->addr, &r->us);
- }
- peer = sip_unref_peer(peer, "unref after sip_find_peer");
+ if (!ast_sockaddr_isnull(&r->us)) {
+ if (!ast_sockaddr_port(&r->us) && r->portno) {
+ ast_sockaddr_set_port(&r->us, r->portno);
+ }
+
+ /* It is possible that DNS was unavailable at the time the peer was created.
+ * Here, if we've updated the address in the registry via manually calling
+ * ast_dnsmgr_lookup_cb() above, then we call the same function that dnsmgr would
+ * call if it was updating a peer's address */
+ if ((peer = sip_find_peer(S_OR(r->peername, r->hostname), NULL, TRUE, FINDPEERS, FALSE, 0))) {
+ if (ast_sockaddr_cmp(&peer->addr, &r->us)) {
+ on_dns_update_peer(&peer->addr, &r->us, peer);
+ }
+ peer = sip_unref_peer(peer, "unref after sip_find_peer");
+ }
}
/* Find address to hostname */
@@ -14357,9 +14431,7 @@
peer->portinuri = ast_sockaddr_port(&testsa) ? TRUE : FALSE;
if (!ast_sockaddr_port(&testsa)) {
- ast_sockaddr_set_port(&testsa,
- transport_type == SIP_TRANSPORT_TLS ?
- STANDARD_TLS_PORT : STANDARD_SIP_PORT);
+ ast_sockaddr_set_port(&testsa, default_sip_port(transport_type));
}
ast_sockaddr_copy(&peer->addr, &testsa);
@@ -28550,10 +28622,16 @@
snprintf(transport, sizeof(transport), "_%s._%s", get_srv_service(peer->socket.type), get_srv_protocol(peer->socket.type));
peer->addr.ss.ss_family = get_address_family_filter(&bindaddr); /* Filter address family */
- if (ast_dnsmgr_lookup(_srvlookup, &peer->addr, &peer->dnsmgr, sip_cfg.srvlookup && !peer->portinuri ? transport : NULL)) {
+ if (ast_dnsmgr_lookup_cb(_srvlookup, &peer->addr, &peer->dnsmgr, sip_cfg.srvlookup && !peer->portinuri ? transport : NULL,
+ on_dns_update_peer, sip_ref_peer(peer, "Store peer on dnsmgr"))) {
ast_log(LOG_ERROR, "srvlookup failed for host: %s, on peer %s, removing peer\n", _srvlookup, peer->name);
+ sip_unref_peer(peer, "dnsmgr lookup failed, getting rid of peer dnsmgr ref");
sip_unref_peer(peer, "getting rid of a peer pointer");
return NULL;
+ }
+ if (!peer->dnsmgr) {
+ /* dnsmgr refresh disabeld, release reference */
+ sip_unref_peer(peer, "dnsmgr disabled, unref peer");
}
ast_string_field_set(peer, tohost, srvlookup);
@@ -28686,7 +28764,7 @@
/* First, destroy all outstanding registry calls */
/* This is needed, since otherwise active registry entries will not be destroyed */
ASTOBJ_CONTAINER_TRAVERSE(®l, 1, do { /* regl is locked */
- ASTOBJ_RDLOCK(iterator); /* now regl is locked, and the object is also locked */
+ ASTOBJ_WRLOCK(iterator); /* now regl is locked, and the object is also locked */
if (iterator->call) {
ast_debug(3, "Destroying active SIP dialog for registry %s@%s\n", iterator->username, iterator->hostname);
/* This will also remove references to the registry */
@@ -28698,6 +28776,11 @@
}
if (iterator->timeout > -1) {
AST_SCHED_DEL_UNREF(sched, iterator->timeout, registry_unref(iterator, "reg ptr unref from reload config"));
+ }
+ if (iterator->dnsmgr) {
+ ast_dnsmgr_release(iterator->dnsmgr);
+ iterator->dnsmgr = NULL;
+ registry_unref(iterator, "reg ptr unref from dnsmgr");
}
ASTOBJ_UNLOCK(iterator);
} while(0));
@@ -31344,6 +31427,16 @@
cleanup_all_regs();
ASTOBJ_CONTAINER_DESTROYALL(®l, sip_registry_destroy);
ASTOBJ_CONTAINER_DESTROY(®l);
+
+ ASTOBJ_CONTAINER_TRAVERSE(&submwil, 1, do {
+ ASTOBJ_WRLOCK(iterator);
+ if (iterator->dnsmgr) {
+ ast_dnsmgr_release(iterator->dnsmgr);
+ iterator->dnsmgr = NULL;
+ ASTOBJ_UNREF(iterator, sip_subscribe_mwi_destroy);
+ }
+ ASTOBJ_UNLOCK(iterator);
+ } while(0));
ASTOBJ_CONTAINER_DESTROYALL(&submwil, sip_subscribe_mwi_destroy);
ASTOBJ_CONTAINER_DESTROY(&submwil);
Modified: branches/10/include/asterisk/dnsmgr.h
URL: http://svnview.digium.com/svn/asterisk/branches/10/include/asterisk/dnsmgr.h?view=diff&rev=353397&r1=353396&r2=353397
==============================================================================
--- branches/10/include/asterisk/dnsmgr.h (original)
+++ branches/10/include/asterisk/dnsmgr.h Mon Jan 30 17:28:10 2012
@@ -36,6 +36,8 @@
* This is an opaque type.
*/
struct ast_dnsmgr_entry;
+
+typedef void (*dns_update_func)(struct ast_sockaddr *old_addr, struct ast_sockaddr *new_addr, void *data);
/*!
* \brief Allocate a new DNS manager entry
@@ -105,6 +107,31 @@
int ast_dnsmgr_lookup(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service);
/*!
+ * \brief Allocate and initialize a DNS manager entry, with update callback
+ *
+ * \param name the hostname
+ * \param result The addr which is intended to be updated in the update callback when DNS manager calls it on refresh.
+ * The address family is used as an input parameter to filter the returned addresses.
+ * If it is 0, both IPv4 and IPv6 addresses can be returned.
+ * \param dnsmgr Where to store the allocate DNS manager entry
+ * \param service
+ * \param func The update callback function
+ * The update callback will be called when DNS manager detects that an IP address has been changed.
+ * Instead of updating the addr itself, DNS manager will call this callback function with the old
+ * and new addresses. It is the responsibility of the callback to perform any updates
+ * \param data A pointer to data that will be passed through to the callback function
+ *
+ * \note
+ * This function allocates a new DNS manager entry object, and fills it with
+ * the provided hostname and IP address. This function _does_ force an initial
+ * lookup, so it may block for some period of time.
+ *
+ * \retval 0 success
+ * \retval non-zero failure
+ */
+int ast_dnsmgr_lookup_cb(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service, dns_update_func func, void *data);
+
+/*!
* \brief Force a refresh of a dnsmgr entry
*
* \retval non-zero if the result is different than the previous result
Modified: branches/10/main/dnsmgr.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/main/dnsmgr.c?view=diff&rev=353397&r1=353396&r2=353397
==============================================================================
--- branches/10/main/dnsmgr.c (original)
+++ branches/10/main/dnsmgr.c Mon Jan 30 17:28:10 2012
@@ -58,6 +58,10 @@
unsigned int family;
/*! Set to 1 if the entry changes */
unsigned int changed:1;
+ /*! Data to pass back to update_func */
+ void *data;
+ /*! The callback function to execute on address update */
+ dns_update_func update_func;
ast_mutex_t lock;
AST_RWLIST_ENTRY(ast_dnsmgr_entry) list;
/*! just 1 here, but we use calloc to allocate the correct size */
@@ -129,7 +133,7 @@
ast_free(entry);
}
-int ast_dnsmgr_lookup(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service)
+static int internal_dnsmgr_lookup(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service, dns_update_func func, void *data)
{
unsigned int family;
@@ -164,7 +168,19 @@
ast_verb(3, "adding dns manager for '%s'\n", name);
*dnsmgr = ast_dnsmgr_get_family(name, result, service, family);
+ (*dnsmgr)->update_func = func;
+ (*dnsmgr)->data = data;
return !*dnsmgr;
+}
+
+int ast_dnsmgr_lookup(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service)
+{
+ return internal_dnsmgr_lookup(name, result, dnsmgr, service, NULL, NULL);
+}
+
+int ast_dnsmgr_lookup_cb(const char *name, struct ast_sockaddr *result, struct ast_dnsmgr_entry **dnsmgr, const char *service, dns_update_func func, void *data)
+{
+ return internal_dnsmgr_lookup(name, result, dnsmgr, service, func, data);
}
/*
@@ -186,16 +202,19 @@
if (!ast_sockaddr_port(&tmp)) {
ast_sockaddr_set_port(&tmp, ast_sockaddr_port(entry->result));
}
-
if (ast_sockaddr_cmp(&tmp, entry->result)) {
const char *old_addr = ast_strdupa(ast_sockaddr_stringify(entry->result));
const char *new_addr = ast_strdupa(ast_sockaddr_stringify(&tmp));
- ast_log(LOG_NOTICE, "dnssrv: host '%s' changed from %s to %s\n",
- entry->name, old_addr, new_addr);
-
- ast_sockaddr_copy(entry->result, &tmp);
- changed = entry->changed = 1;
+ if (entry->update_func) {
+ entry->update_func(entry->result, &tmp, entry->data);
+ } else {
+ ast_log(LOG_NOTICE, "dnssrv: host '%s' changed from %s to %s\n",
+ entry->name, old_addr, new_addr);
+
+ ast_sockaddr_copy(entry->result, &tmp);
+ changed = entry->changed = 1;
+ }
}
}
More information about the asterisk-commits
mailing list