<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6633">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
</div><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 option can<br>block re-registrations because the source port from the endpoint can be<br>random. 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 allows a registration to succeed by<br>displacing any existing contacts that now exceed the "max_contacts" count.<br>Any removed contacts are the next to expire. The behaviour change is<br>beneficial when "rewrite_contact" is enabled and "max_contacts" is greater<br>than one. The removed contact is likely the old contact created by<br>"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 include/asterisk/vector.h<br>M res/res_pjsip.c<br>M res/res_pjsip_registrar.c<br>5 files changed, 198 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index dde24e2..4d1395f 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -21,6 +21,19 @@<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 allows a registration to succeed by<br>+ displacing any existing contacts that now exceed the "max_contacts" count.<br>+ Any removed contacts are the next to expire. The behaviour change is<br>+ beneficial when "rewrite_contact" is enabled and "max_contacts" is greater<br>+ than one. The removed contact is likely the old contact created by<br>+ "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..c535aaf 100644<br>--- a/configs/samples/pjsip.conf.sample<br>+++ b/configs/samples/pjsip.conf.sample<br>@@ -918,7 +918,13 @@<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 ; Allow a registration to succeed by displacing any existing<br>+ ; contacts that now exceed the max_contacts count. Any<br>+ ; removed contacts are the next to expire. The behaviour is<br>+ ; beneficial when rewrite_contact is enabled and max_contacts<br>+ ; is greater than one. The removed contact is likely the old<br>+ ; contact created by rewrite_contact that the device is<br>+ ; refreshing.<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>@@ -1157,7 +1163,7 @@<br> <br> ;outbound_auth= ; Authentication object(s) to be used for outbound<br> ; publishes.<br>- ; This is a comma-delimited list of auth sections<br>+ ; This is a comma-delimited list of auth sections<br> ; defined in pjsip.conf used to respond to outbound<br> ; authentication challenges.<br> ; Using the same auth section for inbound and<br>diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h<br>index 1e6fe03..68ce130 100644<br>--- a/include/asterisk/vector.h<br>+++ b/include/asterisk/vector.h<br>@@ -548,6 +548,14 @@<br> #define AST_VECTOR_SIZE(vec) (vec)->current<br> <br> /*!<br>+ * \brief Get the maximum number of elements the vector can currently hold.<br>+ *<br>+ * \param vec Vector to query.<br>+ * \return Maximum number of elements the vector can currently hold.<br>+ */<br>+#define AST_VECTOR_MAX_SIZE(vec) (vec)->max<br>+<br>+/*!<br> * \brief Reset vector.<br> *<br> * \param vec Vector to reset.<br>diff --git a/res/res_pjsip.c b/res/res_pjsip.c<br>index 6c1f776..cbeb5ea 100644<br>--- a/res/res_pjsip.c<br>+++ b/res/res_pjsip.c<br>@@ -1448,6 +1448,18 @@<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 the source address as the contact address to help with<br>+ NAT and reusing connection oriented transports such as TCP and<br>+ TLS. Unfortunately, refreshing a registration may register a<br>+ different contact address and exceed<br>+ <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 <replaceable>rewrite_contact</replaceable> contact source<br>+ address 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 +1469,29 @@<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 the source address as the contact address to help with<br>+ NAT and reusing connection oriented transports such as TCP and<br>+ TLS. Unfortunately, refreshing a registration may register a<br>+ different contact address and exceed<br>+ <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 <replaceable>rewrite_contact</replaceable> contact source<br>+ address 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..3290601 100644<br>--- a/res/res_pjsip_registrar.c<br>+++ b/res/res_pjsip_registrar.c<br>@@ -129,7 +129,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 +141,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 +181,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 +225,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 +357,96 @@<br> ao2_ref(aor, -1);<br> }<br> <br>+AST_VECTOR(excess_contact_vector, struct ast_sip_contact *);<br>+<br>+static int vec_contact_cmp(struct ast_sip_contact *left, struct ast_sip_contact *right)<br>+{<br>+ struct ast_sip_contact *left_contact = left;<br>+ struct ast_sip_contact *right_contact = right;<br>+<br>+ /* Sort from soonest to expire to last to expire */<br>+ return ast_tvcmp(left_contact->expiration_time, right_contact->expiration_time);<br>+}<br>+<br>+static int vec_contact_add(void *obj, void *arg, int flags)<br>+{<br>+ struct ast_sip_contact *contact = obj;<br>+ struct excess_contact_vector *contact_vec = arg;<br>+<br>+ /*<br>+ * Performance wise, an insertion sort is fine because we<br>+ * shouldn't need to remove more than a handful of contacts.<br>+ * I expect we'll typically be removing only one contact.<br>+ */<br>+ AST_VECTOR_ADD_SORTED(contact_vec, contact, vec_contact_cmp);<br>+ if (AST_VECTOR_SIZE(contact_vec) == AST_VECTOR_MAX_SIZE(contact_vec)) {<br>+ /*<br>+ * We added a contact over the number we need to remove.<br>+ * Remove the longest to expire contact from the vector<br>+ * which is the last element in the vector. It may be<br>+ * the one we just added or the one we just added pushed<br>+ * out an earlier contact from removal consideration.<br>+ */<br>+ --AST_VECTOR_SIZE(contact_vec);<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 excess_contact_vector contact_vec;<br>+<br>+ /*<br>+ * Create a sorted vector to hold the to_remove soonest to<br>+ * expire contacts. The vector has an extra space to<br>+ * temporarily hold the longest to expire contact that we<br>+ * won't remove.<br>+ */<br>+ if (AST_VECTOR_INIT(&contact_vec, to_remove + 1)) {<br>+ return;<br>+ }<br>+ ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, vec_contact_add, &contact_vec);<br>+<br>+ /*<br>+ * The vector should always be populated with the number<br>+ * of contacts we need to remove. Just in case, we will<br>+ * remove all contacts in the vector even if the contacts<br>+ * container had fewer contacts than there should be.<br>+ */<br>+ ast_assert(AST_VECTOR_SIZE(&contact_vec) == to_remove);<br>+ to_remove = AST_VECTOR_SIZE(&contact_vec);<br>+<br>+ /* Remove the excess contacts that expire the soonest */<br>+ while (to_remove--) {<br>+ struct ast_sip_contact *contact;<br>+<br>+ contact = AST_VECTOR_GET(&contact_vec, to_remove);<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_VECTOR_FREE(&contact_vec);<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 +455,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 +495,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 +511,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 +554,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 +568,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 +710,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: merged </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: 3 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>