[asterisk-commits] res pjsip registrar.c: Update remove existing AOR contact ha... (asterisk[14])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Oct 11 06:44:15 CDT 2017
Jenkins2 has submitted this change and it was merged. ( 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 option 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 allows a registration to succeed by
displacing any existing contacts that now exceed the "max_contacts" count.
Any removed contacts are the next to expire. 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 include/asterisk/vector.h
M res/res_pjsip.c
M res/res_pjsip_registrar.c
5 files changed, 192 insertions(+), 21 deletions(-)
Approvals:
George Joseph: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/CHANGES b/CHANGES
index dc665ac..2cc8ad1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -34,6 +34,13 @@
unsolicited MWI NOTIFY requests and make them available to other modules via
the stasis message bus.
+ * The "remove_existing" option now allows a registration to succeed by
+ displacing any existing contacts that now exceed the "max_contacts" count.
+ Any removed contacts are the next to expire. 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..8f739d4 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -902,7 +902,13 @@
;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 ; Allow a registration to succeed by displacing any existing
+ ; contacts that now exceed the max_contacts count. Any
+ ; removed contacts are the next to expire. The behaviour 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.
; (default: "no")
;type= ; Must be of type aor (default: "")
;qualify_frequency=0 ; Interval at which to qualify an AoR (default: "0")
@@ -1141,7 +1147,7 @@
;outbound_auth= ; Authentication object(s) to be used for outbound
; publishes.
- ; This is a comma-delimited list of auth sections
+ ; This is a comma-delimited list of auth sections
; defined in pjsip.conf used to respond to outbound
; authentication challenges.
; Using the same auth section for inbound and
diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h
index 1e6fe03..68ce130 100644
--- a/include/asterisk/vector.h
+++ b/include/asterisk/vector.h
@@ -548,6 +548,14 @@
#define AST_VECTOR_SIZE(vec) (vec)->current
/*!
+ * \brief Get the maximum number of elements the vector can currently hold.
+ *
+ * \param vec Vector to query.
+ * \return Maximum number of elements the vector can currently hold.
+ */
+#define AST_VECTOR_MAX_SIZE(vec) (vec)->max
+
+/*!
* \brief Reset vector.
*
* \param vec Vector to reset.
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 4d5c5cb..b1f1d4d 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -1411,6 +1411,18 @@
It only limits contacts added through external interaction, such as
registration.
</para>
+ <note><para>The <replaceable>rewrite_contact</replaceable> option
+ registers the source address as the contact address to help with
+ NAT and reusing connection oriented transports such as TCP and
+ TLS. Unfortunately, refreshing a registration may register a
+ different 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 <replaceable>rewrite_contact</replaceable> contact source
+ address 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 +1432,29 @@
<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 the source address as the contact address to help with
+ NAT and reusing connection oriented transports such as TCP and
+ TLS. Unfortunately, refreshing a registration may register a
+ different 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 <replaceable>rewrite_contact</replaceable> contact source
+ address 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..3290601 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -129,7 +129,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 +141,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 +181,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 +225,7 @@
contact->user_agent);
}
- return 0;
+ return CMP_MATCH;
}
/*! \brief Internal function which adds a contact to a response */
@@ -351,6 +357,96 @@
ao2_ref(aor, -1);
}
+AST_VECTOR(excess_contact_vector, struct ast_sip_contact *);
+
+static int vec_contact_cmp(struct ast_sip_contact *left, struct ast_sip_contact *right)
+{
+ struct ast_sip_contact *left_contact = left;
+ struct ast_sip_contact *right_contact = right;
+
+ /* Sort from soonest to expire to last to expire */
+ return ast_tvcmp(left_contact->expiration_time, right_contact->expiration_time);
+}
+
+static int vec_contact_add(void *obj, void *arg, int flags)
+{
+ struct ast_sip_contact *contact = obj;
+ struct excess_contact_vector *contact_vec = arg;
+
+ /*
+ * Performance wise, an insertion sort is fine because we
+ * shouldn't need to remove more than a handful of contacts.
+ * I expect we'll typically be removing only one contact.
+ */
+ AST_VECTOR_ADD_SORTED(contact_vec, contact, vec_contact_cmp);
+ if (AST_VECTOR_SIZE(contact_vec) == AST_VECTOR_MAX_SIZE(contact_vec)) {
+ /*
+ * We added a contact over the number we need to remove.
+ * Remove the longest to expire contact from the vector
+ * which is the last element in the vector. It may be
+ * the one we just added or the one we just added pushed
+ * out an earlier contact from removal consideration.
+ */
+ --AST_VECTOR_SIZE(contact_vec);
+ }
+ 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 excess_contact_vector contact_vec;
+
+ /*
+ * Create a sorted vector to hold the to_remove soonest to
+ * expire contacts. The vector has an extra space to
+ * temporarily hold the longest to expire contact that we
+ * won't remove.
+ */
+ if (AST_VECTOR_INIT(&contact_vec, to_remove + 1)) {
+ return;
+ }
+ ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, vec_contact_add, &contact_vec);
+
+ /*
+ * The vector should always be populated with the number
+ * of contacts we need to remove. Just in case, we will
+ * remove all contacts in the vector even if the contacts
+ * container had fewer contacts than there should be.
+ */
+ ast_assert(AST_VECTOR_SIZE(&contact_vec) == to_remove);
+ to_remove = AST_VECTOR_SIZE(&contact_vec);
+
+ /* Remove the excess contacts that expire the soonest */
+ while (to_remove--) {
+ struct ast_sip_contact *contact;
+
+ contact = AST_VECTOR_GET(&contact_vec, to_remove);
+
+ 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_VECTOR_FREE(&contact_vec);
+}
+
static int register_aor_core(pjsip_rx_data *rdata,
struct ast_sip_endpoint *endpoint,
struct ast_sip_aor *aor,
@@ -359,7 +455,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 +495,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 +511,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 +554,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 +568,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 +710,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: merged
Gerrit-Change-Id: I64c107a10b70db1697d17136051ae6bf22b5314b
Gerrit-Change-Number: 6631
Gerrit-PatchSet: 3
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-commits/attachments/20171011/34fdc49b/attachment-0001.html>
More information about the asterisk-commits
mailing list