<p>Richard Mudgett has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6633">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_registrar.c: Update remove_existing AOR contact handling.<br><br>When "rewrite_contact" is enabled, the "max_contacts" count can block<br>re-registrations because the source port from the endpoint can be random.<br>When the re-registration is blocked, the endpoint may give up<br>re-registering and require manual intervention.<br><br>* The "remove_existing" option now removes the soonest to expire contacts a<br>registration does not affect that are over the "max_contacts" count. The<br>behaviour change is beneficial when "rewrite_contact" is enabled and<br>"max_contacts" is greater than one. The removed contact is likely the old<br>contact created by "rewrite_contact" that the device is refreshing.<br><br>ASTERISK-27192<br><br>Change-Id: I64c107a10b70db1697d17136051ae6bf22b5314b<br>---<br>M CHANGES<br>M configs/samples/pjsip.conf.sample<br>M res/res_pjsip.c<br>M res/res_pjsip_registrar.c<br>4 files changed, 170 insertions(+), 20 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/33/6633/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index dde24e2..47bbacf 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -21,6 +21,18 @@<br> dialplan in the hash TRANSFER_DATA.<br> <br> ------------------------------------------------------------------------------<br>+--- Functionality changes from Asterisk 15.0.0 to Asterisk 15.1.0 ------------<br>+------------------------------------------------------------------------------<br>+<br>+res_pjsip<br>+------------------<br>+ * The "remove_existing" option now removes the soonest to expire contacts a<br>+ registration does not affect that are over the "max_contacts" count. The<br>+ behaviour change is beneficial when "rewrite_contact" is enabled and<br>+ "max_contacts" is greater than one. The removed contact is likely the<br>+ old contact created by "rewrite_contact" that the device is refreshing.<br>+<br>+------------------------------------------------------------------------------<br> --- Functionality changes from Asterisk 14 to Asterisk 15 --------------------<br> ------------------------------------------------------------------------------<br> <br>diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample<br>index 9a2592b..726f2a8 100644<br>--- a/configs/samples/pjsip.conf.sample<br>+++ b/configs/samples/pjsip.conf.sample<br>@@ -918,7 +918,8 @@<br> ;max_contacts=0 ; Maximum number of contacts that can bind to an AoR (default:<br> ; "0")<br> ;minimum_expiration=60 ; Minimum keep alive time for an AoR (default: "60")<br>-;remove_existing=no ; Determines whether new contacts replace existing ones<br>+;remove_existing=no ; Determines whether new contacts remove the number of<br>+ ; existing ones over max_contacts which expire the soonest.<br> ; (default: "no")<br> ;type= ; Must be of type aor (default: "")<br> ;qualify_frequency=0 ; Interval at which to qualify an AoR (default: "0")<br>diff --git a/res/res_pjsip.c b/res/res_pjsip.c<br>index 6c1f776..7bf2b60 100644<br>--- a/res/res_pjsip.c<br>+++ b/res/res_pjsip.c<br>@@ -1448,6 +1448,17 @@<br> It only limits contacts added through external interaction, such as<br> registration.<br> </para><br>+ <note><para>The <replaceable>rewrite_contact</replaceable> option<br>+ registers random contact addresses to help with NAT and reusing<br>+ connection oriented transports such as TCP and TLS. Unfortunately,<br>+ refreshing a registration may create a different random contact<br>+ address and exceed <replaceable>max_contacts</replaceable>. The<br>+ <replaceable>remove_existing</replaceable> option can help by<br>+ removing the soonest to expire contact(s) over<br>+ <replaceable>max_contacts</replaceable> which is likely the<br>+ old random <replaceable>rewrite_contact</replaceable> contact<br>+ being refreshed.<br>+ </para></note><br> <note><para>This should be set to <literal>1</literal> and<br> <replaceable>remove_existing</replaceable> set to <literal>yes</literal> if you<br> wish to stick with the older <literal>chan_sip</literal> behaviour.<br>@@ -1457,15 +1468,28 @@<br> <configOption name="minimum_expiration" default="60"><br> <synopsis>Minimum keep alive time for an AoR</synopsis><br> <description><para><br>- Minimum time to keep a peer with an explict expiration. Time in seconds.<br>+ Minimum time to keep a peer with an explicit expiration. Time in seconds.<br> </para></description><br> </configOption><br> <configOption name="remove_existing" default="no"><br> <synopsis>Determines whether new contacts replace existing ones.</synopsis><br> <description><para><br>- On receiving a new registration to the AoR should it remove<br>- the existing contact that was registered against it?<br>+ On receiving a new registration to the AoR should it remove enough<br>+ existing contacts not added or updated by the registration to<br>+ satisfy <replaceable>max_contacts</replaceable>? Any removed<br>+ contacts will expire the soonest.<br> </para><br>+ <note><para>The <replaceable>rewrite_contact</replaceable> option<br>+ registers random contact addresses to help with NAT and reusing<br>+ connection oriented transports such as TCP and TLS. Unfortunately,<br>+ refreshing a registration may create a different random contact<br>+ address and exceed <replaceable>max_contacts</replaceable>. The<br>+ <replaceable>remove_existing</replaceable> option can help by<br>+ removing the soonest to expire contact(s) over<br>+ <replaceable>max_contacts</replaceable> which is likely the<br>+ old random <replaceable>rewrite_contact</replaceable> contact<br>+ being refreshed.<br>+ </para></note><br> <note><para>This should be set to <literal>yes</literal> and<br> <replaceable>max_contacts</replaceable> set to <literal>1</literal> if you<br> wish to stick with the older <literal>chan_sip</literal> behaviour.<br>diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c<br>index ba1c074..e26ad3e 100644<br>--- a/res/res_pjsip_registrar.c<br>+++ b/res/res_pjsip_registrar.c<br>@@ -32,6 +32,7 @@<br> #include "asterisk/module.h"<br> #include "asterisk/paths.h"<br> #include "asterisk/test.h"<br>+#include "asterisk/heap.h"<br> #include "asterisk/taskprocessor.h"<br> #include "asterisk/manager.h"<br> #include "asterisk/named_locks.h"<br>@@ -129,7 +130,8 @@<br> /*! \brief Internal function which validates provided Contact headers to confirm that they are acceptable, and returns number of contacts */<br> static int registrar_validate_contacts(const pjsip_rx_data *rdata, struct ao2_container *contacts, struct ast_sip_aor *aor, int *added, int *updated, int *deleted)<br> {<br>- pjsip_contact_hdr *previous = NULL, *contact = (pjsip_contact_hdr *)&rdata->msg_info.msg->hdr;<br>+ pjsip_contact_hdr *previous = NULL;<br>+ pjsip_contact_hdr *contact = (pjsip_contact_hdr *)&rdata->msg_info.msg->hdr;<br> struct registrar_contact_details details = {<br> .pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Contact Comparison", 256, 256),<br> };<br>@@ -140,15 +142,18 @@<br> <br> while ((contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next))) {<br> int expiration = registrar_get_expiration(aor, contact, rdata);<br>- RAII_VAR(struct ast_sip_contact *, existing, NULL, ao2_cleanup);<br>+ struct ast_sip_contact *existing;<br> char contact_uri[pjsip_max_url_size];<br> <br> if (contact->star) {<br> /* The expiration MUST be 0 when a '*' contact is used and there must be no other contact */<br>- if ((expiration != 0) || previous) {<br>+ if (expiration != 0 || previous) {<br> pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);<br> return -1;<br> }<br>+ /* Count all contacts to delete */<br>+ *deleted = ao2_container_count(contacts);<br>+ previous = contact;<br> continue;<br> } else if (previous && previous->star) {<br> /* If there is a previous contact and it is a '*' this is a deal breaker */<br>@@ -177,14 +182,16 @@<br> }<br> <br> /* Determine if this is an add, update, or delete for policy enforcement purposes */<br>- if (!(existing = ao2_callback(contacts, 0, registrar_find_contact, &details))) {<br>+ existing = ao2_callback(contacts, 0, registrar_find_contact, &details);<br>+ ao2_cleanup(existing);<br>+ if (!existing) {<br> if (expiration) {<br>- (*added)++;<br>+ ++*added;<br> }<br> } else if (expiration) {<br>- (*updated)++;<br>+ ++*updated;<br> } else {<br>- (*deleted)++;<br>+ ++*deleted;<br> }<br> }<br> <br>@@ -219,7 +226,7 @@<br> contact->user_agent);<br> }<br> <br>- return 0;<br>+ return CMP_MATCH;<br> }<br> <br> /*! \brief Internal function which adds a contact to a response */<br>@@ -351,6 +358,84 @@<br> ao2_ref(aor, -1);<br> }<br> <br>+static unsigned int heap_calc_height(unsigned int num_elements)<br>+{<br>+ unsigned int height = 0;<br>+<br>+ while (num_elements) {<br>+ num_elements >>= 1;<br>+ ++height;<br>+ }<br>+ return height;<br>+}<br>+<br>+static int heap_min_contact_expire_cmp(void *left, void *right)<br>+{<br>+ struct ast_sip_contact *left_contact = left;<br>+ struct ast_sip_contact *right_contact = right;<br>+<br>+ /* Swap left and right to get minimum expiration time remaining heap. */<br>+ return ast_tvcmp(right_contact->expiration_time, left_contact->expiration_time);<br>+}<br>+<br>+static int heap_push_contact(void *obj, void *arg, int flags)<br>+{<br>+ struct ast_sip_contact *contact = obj;<br>+ struct ast_heap *contact_heap = arg;<br>+<br>+ ast_heap_push(contact_heap, contact);<br>+<br>+ return 0;<br>+}<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Remove excess existing contacts that expire the soonest.<br>+ * \since 13.18.0<br>+ *<br>+ * \param contacts Container of unmodified contacts that could remove.<br>+ * \param to_remove Maximum number of contacts to remove.<br>+ *<br>+ * \return Nothing<br>+ */<br>+static void remove_excess_contacts(struct ao2_container *contacts, unsigned int to_remove)<br>+{<br>+ struct ast_heap *contact_heap;<br>+<br>+ ast_assert(0 < to_remove);<br>+<br>+ /* Create a minimum contact expiration time remaining heap */<br>+ contact_heap = ast_heap_create(heap_calc_height(ao2_container_count(contacts)),<br>+ heap_min_contact_expire_cmp, -1);<br>+ if (!contact_heap) {<br>+ return;<br>+ }<br>+ ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, heap_push_contact, contact_heap);<br>+<br>+ /* Remove the excess contacts that expire the soonest */<br>+ while (to_remove--) {<br>+ struct ast_sip_contact *contact;<br>+<br>+ contact = ast_heap_pop(contact_heap);<br>+ if (!contact) {<br>+ break;<br>+ }<br>+<br>+ ast_sip_location_delete_contact(contact);<br>+ ast_verb(3, "Removed contact '%s' from AOR '%s' due to remove_existing\n",<br>+ contact->uri, contact->aor);<br>+ ast_test_suite_event_notify("AOR_CONTACT_REMOVED",<br>+ "Contact: %s\r\n"<br>+ "AOR: %s\r\n"<br>+ "UserAgent: %s",<br>+ contact->uri,<br>+ contact->aor,<br>+ contact->user_agent);<br>+ }<br>+<br>+ ast_heap_destroy(contact_heap);<br>+}<br>+<br> static int register_aor_core(pjsip_rx_data *rdata,<br> struct ast_sip_endpoint *endpoint,<br> struct ast_sip_aor *aor,<br>@@ -359,7 +444,10 @@<br> {<br> static const pj_str_t USER_AGENT = { "User-Agent", 10 };<br> <br>- int added = 0, updated = 0, deleted = 0;<br>+ int added = 0;<br>+ int updated = 0;<br>+ int deleted = 0;<br>+ int contact_count;<br> pjsip_contact_hdr *contact_hdr = NULL;<br> struct registrar_contact_details details = { 0, };<br> pjsip_tx_data *tdata;<br>@@ -396,7 +484,14 @@<br> return PJ_TRUE;<br> }<br> <br>- if ((MAX(added - deleted, 0) + (!aor->remove_existing ? ao2_container_count(contacts) : 0)) > aor->max_contacts) {<br>+ if (aor->remove_existing) {<br>+ /* Cumulative number of contacts affected by this registration */<br>+ contact_count = MAX(updated + added - deleted, 0);<br>+ } else {<br>+ /* Total contacts after this registration */<br>+ contact_count = ao2_container_count(contacts) + added - deleted;<br>+ }<br>+ if (contact_count > aor->max_contacts) {<br> /* Enforce the maximum number of contacts */<br> pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 403, NULL, NULL, NULL);<br> ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");<br>@@ -405,7 +500,9 @@<br> return PJ_TRUE;<br> }<br> <br>- if (!(details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Contact Comparison", 256, 256))) {<br>+ details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),<br>+ "Contact Comparison", 256, 256);<br>+ if (!details.pool) {<br> pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);<br> return PJ_TRUE;<br> }<br>@@ -446,7 +543,8 @@<br> <br> if (contact_hdr->star) {<br> /* A star means to unregister everything, so do so for the possible contacts */<br>- ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, registrar_delete_contact, (void *)aor_name);<br>+ ao2_callback(contacts, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,<br>+ registrar_delete_contact, (void *)aor_name);<br> break;<br> }<br> <br>@@ -459,7 +557,8 @@<br> details.uri = pjsip_uri_get_uri(contact_hdr->uri);<br> pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri));<br> <br>- if (!(contact = ao2_callback(contacts, OBJ_UNLINK, registrar_find_contact, &details))) {<br>+ contact = ao2_callback(contacts, OBJ_UNLINK, registrar_find_contact, &details);<br>+ if (!contact) {<br> int prune_on_boot = 0;<br> pj_str_t host_name;<br> <br>@@ -600,15 +699,29 @@<br> <br> pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);<br> <br>- /* If the AOR is configured to remove any existing contacts that have not been updated/added as a result of this REGISTER<br>- * do so<br>+ /*<br>+ * If the AOR is configured to remove any contacts over max_contacts<br>+ * that have not been updated/added/deleted as a result of this<br>+ * REGISTER do so.<br>+ *<br>+ * The contacts container currently holds the existing contacts that<br>+ * were not affected by this REGISTER.<br> */<br> if (aor->remove_existing) {<br>- ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, registrar_delete_contact, NULL);<br>+ /* Total contacts after this registration */<br>+ contact_count = ao2_container_count(contacts) + updated + added;<br>+ if (contact_count > aor->max_contacts) {<br>+ /* Remove excess existing contacts that expire the soonest */<br>+ remove_excess_contacts(contacts, contact_count - aor->max_contacts);<br>+ }<br> }<br> <br> /* Re-retrieve contacts. Caller will clean up the original container. */<br> contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);<br>+ if (!contacts) {<br>+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);<br>+ return PJ_TRUE;<br>+ }<br> response_contact = ao2_callback(contacts, 0, NULL, NULL);<br> <br> /* Send a response containing all of the contacts (including static) that are present on this AOR */<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6633">change 6633</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6633"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I64c107a10b70db1697d17136051ae6bf22b5314b </div>
<div style="display:none"> Gerrit-Change-Number: 6633 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>