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