[Asterisk-code-review] res_pjsip_registrar: Remove unavailable contacts if exceeds max_contacts (asterisk[16])

Joe asteriskteam at digium.com
Wed Jul 21 16:48:12 CDT 2021


Joe has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/16160 )


Change subject: res_pjsip_registrar: Remove unavailable contacts if exceeds max_contacts
......................................................................

res_pjsip_registrar: Remove unavailable contacts if exceeds max_contacts

The behavior of max_contacts and remove_existing are connected.  If
remove_existing is enabled, the soonest expiring contacts are removed.
This may occur when there is an unavailable contact.  Similarly,
when remove_existing is not enabled, registrations from good
endpoints are rejected in favor of retaining unavailable contacts.

This commit changes this behavior, to remove unavailable contacts
when they exceed max_contacts, if there are any, regardless of the
setting of remove_existing.

ASTERISK-29525

Change-Id: Ia2711b08f2b4d1177411b1be23e970d7fdff5784
---
A doc/CHANGES-staging/res_pjsip_registrar.txt
M include/asterisk/res_pjsip.h
M res/res_pjsip/location.c
M res/res_pjsip_registrar.c
4 files changed, 85 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/60/16160/1

diff --git a/doc/CHANGES-staging/res_pjsip_registrar.txt b/doc/CHANGES-staging/res_pjsip_registrar.txt
new file mode 100644
index 0000000..b1aa86e
--- /dev/null
+++ b/doc/CHANGES-staging/res_pjsip_registrar.txt
@@ -0,0 +1,5 @@
+Subject: res_pjsip_registrar
+
+Proritize removal of unavailable contacts when remove_existing
+is set.  Remove unavailable contacts when exceeding max_contacts
+even when remove_existing is not set.
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 3d5b504..dd63719 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -945,6 +945,9 @@
 
 	/*! \brief Return only reachable or unknown contacts */
 	AST_SIP_CONTACT_FILTER_REACHABLE = (1 << 0),
+
+	/*! \brief Return only unreachable contacts */
+	AST_SIP_CONTACT_FILTER_UNREACHABLE = (1 << 4),
 };
 
 /*!
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index cef5ad7..2953e7b 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -191,6 +191,24 @@
 	return unreachable ? CMP_MATCH : 0;
 }
 
+/*! \brief Internal callback function which contains only contact which is unreachable */
+static int contact_only_unreachable(void *obj, void *arg, int flags)
+{
+	struct ast_sip_contact *contact = obj;
+	struct ast_sip_contact_status *status;
+	int unreachable;
+
+	status = ast_sip_get_contact_status(contact);
+	if (!status) {
+		return 0;
+	}
+
+	unreachable = (status->status == UNAVAILABLE);
+	ao2_ref(status, -1);
+
+	return unreachable ? 0 : CMP_MATCH;
+}
+
 struct ast_sip_contact *ast_sip_location_retrieve_first_aor_contact(const struct ast_sip_aor *aor)
 {
 	return ast_sip_location_retrieve_first_aor_contact_filtered(aor, AST_SIP_CONTACT_FILTER_DEFAULT);
@@ -237,6 +255,10 @@
 		ao2_callback(aor->permanent_contacts, OBJ_NODATA, contact_link_static, contacts);
 	}
 
+	if (flags & AST_SIP_CONTACT_FILTER_UNREACHABLE) {
+		ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK, contact_only_unreachable, NULL);
+	}
+
 	if (flags & AST_SIP_CONTACT_FILTER_REACHABLE) {
 		ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK, contact_remove_unreachable, NULL);
 	}
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 6fe4058..ce7611d 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -481,8 +481,31 @@
 {
 	struct ast_sip_contact *left_contact = left;
 	struct ast_sip_contact *right_contact = right;
+	struct ast_sip_contact_status *left_status;
+	struct ast_sip_contact_status *right_status;
+	int left_unreachable;
+	int right_unreachable;
 
-	/* Sort from soonest to expire to last to expire */
+	/* If contact is unreachable move to top of list */
+	left_status = ast_sip_get_contact_status(left_contact);
+	right_status = ast_sip_get_contact_status(right_contact);
+	if (left_status && right_status) {
+		left_unreachable = (left_status->status == UNAVAILABLE);
+		right_unreachable = (right_status->status == UNAVAILABLE);
+		ao2_cleanup(left_status);
+		ao2_cleanup(right_status);
+		if (left_unreachable == right_unreachable) {
+			/* Either both available or both unavailable */
+			/* Sort from soonest to expire to last to expire */
+			return ast_tvcmp(left_contact->expiration_time, right_contact->expiration_time);
+		}
+		if (left_unreachable) return -1;
+		if (right_unreachable) return 1;
+	}
+
+	/* No status from one or both contacts, return by expiration */
+	ao2_cleanup(left_status);
+	ao2_cleanup(right_status);
 	return ast_tvcmp(left_contact->expiration_time, right_contact->expiration_time);
 }
 
@@ -496,6 +519,7 @@
 	 * 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)) {
 		/*
@@ -512,7 +536,7 @@
 
 /*!
  * \internal
- * \brief Remove excess existing contacts that expire the soonest.
+ * \brief Remove excess existing contacts that are unavailable or expire soonest.
  * \since 13.18.0
  *
  * \param contacts Container of unmodified contacts that could remove.
@@ -534,6 +558,7 @@
 	if (AST_VECTOR_INIT(&contact_vec, to_remove + 1)) {
 		return;
 	}
+
 	ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, vec_contact_add, &contact_vec);
 
 	/*
@@ -545,7 +570,7 @@
 	ast_assert(AST_VECTOR_SIZE(&contact_vec) == to_remove);
 	to_remove = AST_VECTOR_SIZE(&contact_vec);
 
-	/* Remove the excess contacts that expire the soonest */
+	/* Remove the excess contacts that are unavailable or expire the soonest */
 	while (to_remove--) {
 		struct ast_sip_contact *contact;
 
@@ -596,6 +621,7 @@
 	int permanent = 0;
 	int contact_count;
 	struct ao2_container *existing_contacts = NULL;
+	struct ao2_container *unavail_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;
@@ -666,16 +692,32 @@
 		/* Total contacts after this registration */
 		contact_count = ao2_container_count(contacts) - permanent + added - deleted;
 	}
+
 	if (contact_count > aor->max_contacts) {
-		/* Enforce the maximum number of contacts */
-		ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");
-		ast_log(LOG_WARNING, "Registration attempt from endpoint '%s' (%s:%d) to AOR '%s' will exceed max contacts of %u\n",
-				ast_sorcery_object_get_id(endpoint), rdata->pkt_info.src_name, rdata->pkt_info.src_port,
-				aor_name, aor->max_contacts);
-		response->code = 403;
-		pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
-		ao2_cleanup(existing_contacts);
-		return;
+		/* Get unreachable contact total */
+		int unavail_count = 0;
+
+		unavail_contacts = ast_sip_location_retrieve_aor_contacts_filtered(aor, AST_SIP_CONTACT_FILTER_UNREACHABLE);
+		unavail_count = ao2_container_count(unavail_contacts);
+
+		/* Check to see if removing those will not help */
+		if (contact_count - unavail_count > aor->max_contacts) {
+			/* Enforce the maximum number of contacts */
+			ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");
+			ast_log(LOG_WARNING, "Registration attempt from endpoint '%s' (%s:%d) to AOR '%s' will exceed max contacts of %u\n",
+					ast_sorcery_object_get_id(endpoint), rdata->pkt_info.src_name, rdata->pkt_info.src_port,
+					aor_name, aor->max_contacts);
+			response->code = 403;
+			pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+			ao2_cleanup(existing_contacts);
+			return;
+		}
+
+		/* Remove any unreachable contacts */
+		ast_log(LOG_WARNING, "Unreachable contacts being removed from endpoint '%s' so max_contacts will not be exceeded\n",
+			ast_sorcery_object_get_id(endpoint));
+		remove_excess_contacts(unavail_contacts, contacts, aor->max_contacts - contact_count);
+		ao2_cleanup(unavail_contacts);
 	}
 
 	user_agent_hdr = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &USER_AGENT, NULL);
@@ -867,7 +909,7 @@
 		/* Total contacts after this registration */
 		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 existing contacts that are unavailable or expire soonest */
 			remove_excess_contacts(existing_contacts, contacts, contact_count - aor->max_contacts);
 		}
 		ao2_ref(existing_contacts, -1);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16160
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ia2711b08f2b4d1177411b1be23e970d7fdff5784
Gerrit-Change-Number: 16160
Gerrit-PatchSet: 1
Gerrit-Owner: Joe <ynadiv at corpit.xyz>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210721/b6cffca5/attachment-0001.html>


More information about the asterisk-code-review mailing list