[Asterisk-code-review] res pjsip registrar: Improve performance on inbound handling. (asterisk[16])
Kevin Harwell
asteriskteam at digium.com
Wed Aug 8 12:22:34 CDT 2018
Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/9804 )
Change subject: res_pjsip_registrar: Improve performance on inbound handling.
......................................................................
res_pjsip_registrar: Improve performance on inbound handling.
This change removes a sorcery lookup for retrieving all
contacts at the end of the registration process by keeping
track of the contacts that are added/updated/deleted.
This ensures at the end of the process the container of
contacts we have is the current state.
Pool usage has also been reduced by allocating one for
usage throughout the handling of a REGISTER and resetting
it to a clean state. This ensures that in most cases
we allocate once and just reuse it.
ASTERISK-28001
Change-Id: I1a78b2d46f9a2045dbbff1a3fd6dba84b612b3cb
---
M res/res_pjsip_registrar.c
1 file changed, 99 insertions(+), 50 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved; Approved for Submit
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 985933e..efd2bd9 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -122,25 +122,28 @@
{
struct ast_sip_contact *contact = obj;
const struct registrar_contact_details *details = arg;
- pjsip_uri *contact_uri = pjsip_parse_uri(details->pool, (char*)contact->uri, strlen(contact->uri), 0);
+ pjsip_uri *contact_uri;
+
+ if (ast_tvzero(contact->expiration_time)) {
+ return 0;
+ }
+
+ contact_uri = pjsip_parse_uri(details->pool, (char*)contact->uri, strlen(contact->uri), 0);
return (pjsip_uri_cmp(PJSIP_URI_IN_CONTACT_HDR, details->uri, contact_uri) == PJ_SUCCESS) ? CMP_MATCH : 0;
}
/*! \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)
+static int registrar_validate_contacts(const pjsip_rx_data *rdata, pj_pool_t *pool, struct ao2_container *contacts,
+ struct ast_sip_aor *aor, int permanent, int *added, int *updated, int *deleted)
{
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),
+ .pool = pool,
};
- if (!details.pool) {
- return -1;
- }
-
- while ((contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next))) {
+ for (; (contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next)); pj_pool_reset(pool)) {
int expiration = registrar_get_expiration(aor, contact, rdata);
struct ast_sip_contact *existing;
char contact_uri[pjsip_max_url_size];
@@ -148,16 +151,14 @@
if (contact->star) {
/* The expiration MUST be 0 when a '*' contact is used and there must be no other contact */
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);
+ *deleted = ao2_container_count(contacts) - permanent;
previous = contact;
continue;
} else if (previous && previous->star) {
/* If there is a previous contact and it is a '*' this is a deal breaker */
- pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return -1;
}
previous = contact;
@@ -171,13 +172,11 @@
/* pjsip_uri_print returns -1 if there's not enough room in the buffer */
if (pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri)) < 0) {
/* If the total length of the uri is greater than pjproject can handle, go no further */
- pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return -1;
}
if (details.uri->host.slen >= pj_max_hostname) {
/* If the length of the hostname is greater than pjproject can handle, go no further */
- pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return -1;
}
@@ -195,25 +194,20 @@
}
}
- /* The provided contacts are acceptable, huzzah! */
- pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return 0;
}
-/*! \brief Callback function which prunes static contacts */
-static int registrar_prune_static(void *obj, void *arg, int flags)
-{
- struct ast_sip_contact *contact = obj;
-
- return ast_tvzero(contact->expiration_time) ? CMP_MATCH : 0;
-}
-
/*! \brief Internal function used to delete a contact from an AOR */
static int registrar_delete_contact(void *obj, void *arg, int flags)
{
struct ast_sip_contact *contact = obj;
const char *aor_name = arg;
+ /* Permanent contacts can't be deleted */
+ if (ast_tvzero(contact->expiration_time)) {
+ return 0;
+ }
+
ast_sip_location_delete_contact(contact);
if (!ast_strlen_zero(aor_name)) {
ast_verb(3, "Removed contact '%s' from AOR '%s' due to request\n", contact->uri, aor_name);
@@ -270,7 +264,7 @@
}
*path_str = ast_str_create(64);
- if (!path_str) {
+ if (!*path_str) {
return -1;
}
@@ -442,7 +436,8 @@
*
* \return Nothing
*/
-static void remove_excess_contacts(struct ao2_container *contacts, unsigned int to_remove)
+static void remove_excess_contacts(struct ao2_container *contacts, struct ao2_container *response_contacts,
+ unsigned int to_remove)
{
struct excess_contact_vector contact_vec;
@@ -482,11 +477,28 @@
contact->uri,
contact->aor,
contact->user_agent);
+
+ ao2_unlink(response_contacts, contact);
}
AST_VECTOR_FREE(&contact_vec);
}
+/*! \brief Callback function which adds non-permanent contacts to a container */
+static int registrar_add_non_permanent(void *obj, void *arg, int flags)
+{
+ struct ast_sip_contact *contact = obj;
+ struct ao2_container *container = arg;
+
+ if (ast_tvzero(contact->expiration_time)) {
+ return 0;
+ }
+
+ ao2_link(container, contact);
+
+ return 0;
+}
+
struct aor_core_response {
/*! Tx data to use for statefull response. NULL for stateless response. */
pjsip_tx_data *tdata;
@@ -506,8 +518,10 @@
int added = 0;
int updated = 0;
int deleted = 0;
+ int permanent = 0;
int contact_count;
- pjsip_contact_hdr *contact_hdr = NULL;
+ struct ao2_container *existing_contacts = NULL;
+ pjsip_contact_hdr *contact_hdr = (pjsip_contact_hdr *)&rdata->msg_info.msg->hdr;
struct registrar_contact_details details = { 0, };
pjsip_tx_data *tdata;
RAII_VAR(struct ast_str *, path_str, NULL, ast_free);
@@ -523,15 +537,28 @@
char *call_id = NULL;
size_t alloc_size;
- /* So we don't count static contacts against max_contacts we prune them out from the container */
- ao2_callback(contacts, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE, registrar_prune_static, NULL);
+ /* We create a single pool and use it throughout this function where we need one */
+ details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),
+ "Contact Comparison", 1024, 256);
+ if (!details.pool) {
+ response->code = 500;
+ return;
+ }
- if (registrar_validate_contacts(rdata, contacts, aor, &added, &updated, &deleted)) {
+ /* If there are any permanent contacts configured on the AOR we need to take them
+ * into account when counting contacts.
+ */
+ if (aor->permanent_contacts) {
+ permanent = ao2_container_count(aor->permanent_contacts);
+ }
+
+ if (registrar_validate_contacts(rdata, details.pool, contacts, aor, permanent, &added, &updated, &deleted)) {
/* The provided Contact headers do not conform to the specification */
ast_sip_report_failed_acl(endpoint, rdata, "registrar_invalid_contacts_provided");
ast_log(LOG_WARNING, "Failed to validate contacts in REGISTER request from '%s'\n",
ast_sorcery_object_get_id(endpoint));
response->code = 400;
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return;
}
@@ -540,15 +567,29 @@
ast_log(LOG_WARNING, "Invalid modifications made to REGISTER request from '%s' by intervening proxy\n",
ast_sorcery_object_get_id(endpoint));
response->code = 420;
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
return;
}
if (aor->remove_existing) {
/* Cumulative number of contacts affected by this registration */
contact_count = MAX(updated + added - deleted, 0);
+
+ /* We need to keep track of only existing contacts so we can later
+ * remove them if need be.
+ */
+ existing_contacts = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, 0,
+ NULL, ast_sorcery_object_id_compare);
+ if (!existing_contacts) {
+ response->code = 500;
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+ return;
+ }
+
+ ao2_callback(contacts, OBJ_NODATA, registrar_add_non_permanent, existing_contacts);
} else {
/* Total contacts after this registration */
- contact_count = ao2_container_count(contacts) + added - deleted;
+ contact_count = ao2_container_count(contacts) - permanent + added - deleted;
}
if (contact_count > aor->max_contacts) {
/* Enforce the maximum number of contacts */
@@ -556,13 +597,8 @@
ast_log(LOG_WARNING, "Registration attempt from endpoint '%s' to AOR '%s' will exceed max contacts of %u\n",
ast_sorcery_object_get_id(endpoint), aor_name, aor->max_contacts);
response->code = 403;
- return;
- }
-
- details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),
- "Contact Comparison", 256, 256);
- if (!details.pool) {
- response->code = 500;
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+ ao2_cleanup(existing_contacts);
return;
}
@@ -595,7 +631,7 @@
}
/* Iterate each provided Contact header and add, update, or delete */
- while ((contact_hdr = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact_hdr ? contact_hdr->next : NULL))) {
+ for (; (contact_hdr = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact_hdr->next)); pj_pool_reset(details.pool)) {
int expiration;
char contact_uri[pjsip_max_url_size];
RAII_VAR(struct ast_sip_contact *, contact, NULL, ao2_cleanup);
@@ -604,6 +640,13 @@
/* A star means to unregister everything, so do so for the possible contacts */
ao2_callback(contacts, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
registrar_delete_contact, (void *)aor_name);
+ /* If we are keeping track of existing contacts for removal then, well, there is
+ * absolutely nothing left so no need to try to remove any.
+ */
+ if (existing_contacts) {
+ ao2_ref(existing_contacts, -1);
+ existing_contacts = NULL;
+ }
break;
}
@@ -617,6 +660,14 @@
pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri));
contact = ao2_callback(contacts, OBJ_UNLINK, registrar_find_contact, &details);
+
+ /* If a contact was returned and we need to keep track of existing contacts then it
+ * should be removed.
+ */
+ if (contact && existing_contacts) {
+ ao2_unlink(existing_contacts, contact);
+ }
+
if (!contact) {
int prune_on_boot;
@@ -672,6 +723,8 @@
aor_name,
expiration,
user_agent);
+
+ ao2_link(contacts, contact);
} else if (expiration) {
struct ast_sip_contact *contact_update;
@@ -712,6 +765,7 @@
aor_name,
expiration,
contact_update->user_agent);
+ ao2_link(contacts, contact_update);
ao2_cleanup(contact_update);
} else {
if (contact->prune_on_boot) {
@@ -749,24 +803,20 @@
* 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.
+ * The existing contacts container holds all contacts that were not
+ * involved in this REGISTER.
+ * The contacts container holds the current contacts of the AOR.
*/
- if (aor->remove_existing) {
+ if (aor->remove_existing && existing_contacts) {
/* Total contacts after this registration */
- contact_count = ao2_container_count(contacts) + updated + added;
+ contact_count = ao2_container_count(existing_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);
+ remove_excess_contacts(existing_contacts, contacts, contact_count - aor->max_contacts);
}
+ ao2_ref(existing_contacts, -1);
}
- /* Re-retrieve contacts. Caller will clean up the original container. */
- contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);
- if (!contacts) {
- response->code = 500;
- return;
- }
response_contact = ao2_callback(contacts, 0, NULL, NULL);
/* Send a response containing all of the contacts (including static) that are present on this AOR */
@@ -782,7 +832,6 @@
registrar_add_date_header(tdata);
ao2_callback(contacts, 0, registrar_add_contact, tdata);
- ao2_cleanup(contacts);
if ((expires_hdr = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_EXPIRES, NULL))) {
expires_hdr = pjsip_expires_hdr_create(tdata->pool, registrar_get_expiration(aor, NULL, rdata));
--
To view, visit https://gerrit.asterisk.org/9804
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a78b2d46f9a2045dbbff1a3fd6dba84b612b3cb
Gerrit-Change-Number: 9804
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180808/4b766a17/attachment-0001.html>
More information about the asterisk-code-review
mailing list