[Asterisk-code-review] res pjsip / res pjsip multihomed: Use the correct transport ... (asterisk[certified/13.1])

Joshua Colp asteriskteam at digium.com
Tue May 5 10:36:55 CDT 2015


Joshua Colp has uploaded a new change for review.

  https://gerrit.asterisk.org/364

Change subject: res_pjsip / res_pjsip_multihomed: Use the correct transport and addressing information on UAS sessions.
......................................................................

res_pjsip / res_pjsip_multihomed: Use the correct transport and addressing information on UAS sessions.

The first thing this patch fixes is UAS dialogs. Previously if a transport was
configured on an endpoint and an inbound session was created there was no guarantee
that requests sent on the dialog would use the correct transport and address
information. This has now been fixed so an explicitly configured transport
is taken into account.

The second thing this patch fixes is res_pjsip_multihomed. The res_pjsip_multihomed
module attempts to determine what transport a message should go out on and what
addressing information should go into the message itself. In a scenario where
multiple transports exist bound to the same IP address but a different port the
code would incorrectly alter the transport and change the message to the wrong
transport. This change makes the res_pjsip_multihomed module smarter so it will
only change the transport and address information in the message when it is
possible and makes sense.

ASTERISK-24615 #close
Reported by: David Justl

Change-Id: I5b57362201cc8c6555834ec8707e9fbddeff7904
---
M res/res_pjsip.c
M res/res_pjsip_multihomed.c
2 files changed, 45 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/64/364/1

diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 39e61a8..2fb669e 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2379,17 +2379,29 @@
 	pjsip_dialog *dlg;
 	pj_str_t contact;
 	pjsip_transport_type_e type = rdata->tp_info.transport->key.type;
+	pjsip_tpselector selector = { .type = PJSIP_TPSELECTOR_NONE, };
+	pjsip_transport *transport;
 
 	ast_assert(status != NULL);
+
+	if (sip_get_tpselector_from_endpoint(endpoint, &selector)) {
+		return NULL;
+	}
+
+	transport = rdata->tp_info.transport;
+	if (selector.type == PJSIP_TPSELECTOR_TRANSPORT) {
+		transport = selector.u.transport;
+	}
+	type = transport->key.type;
 
 	contact.ptr = pj_pool_alloc(rdata->tp_info.pool, PJSIP_MAX_URL_SIZE);
 	contact.slen = pj_ansi_snprintf(contact.ptr, PJSIP_MAX_URL_SIZE,
 			"<sip:%s%.*s%s:%d%s%s>",
 			(type & PJSIP_TRANSPORT_IPV6) ? "[" : "",
-			(int)rdata->tp_info.transport->local_name.host.slen,
-			rdata->tp_info.transport->local_name.host.ptr,
+			(int)transport->local_name.host.slen,
+			transport->local_name.host.ptr,
 			(type & PJSIP_TRANSPORT_IPV6) ? "]" : "",
-			rdata->tp_info.transport->local_name.port,
+			transport->local_name.port,
 			(type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? ";transport=" : "",
 			(type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? pjsip_transport_get_type_name(type) : "");
 
@@ -2403,6 +2415,10 @@
 		return NULL;
 	}
 
+	dlg->sess_count++;
+	pjsip_dlg_set_transport(dlg, &selector);
+	dlg->sess_count--;
+
 	return dlg;
 }
 
diff --git a/res/res_pjsip_multihomed.c b/res/res_pjsip_multihomed.c
index e0ee53e..ca56b7c 100644
--- a/res/res_pjsip_multihomed.c
+++ b/res/res_pjsip_multihomed.c
@@ -83,21 +83,15 @@
 	return 0;
 }
 
-/*! \brief Helper function which determines if the existing address has priority over new one */
-static int multihomed_rewrite_header(pj_str_t *source, pjsip_transport *transport)
+/*! \brief Helper function which determines if a transport is bound to any */
+static int multihomed_bound_any(pjsip_transport *transport)
 {
 	pj_uint32_t loop6[4] = {0, 0, 0, 0};
 
-	/* If the transport is bound to any it should always rewrite */
 	if ((transport->local_addr.addr.sa_family == pj_AF_INET() &&
 		transport->local_addr.ipv4.sin_addr.s_addr == PJ_INADDR_ANY) ||
 		(transport->local_addr.addr.sa_family == pj_AF_INET6() &&
 		!pj_memcmp(&transport->local_addr.ipv6.sin6_addr, loop6, sizeof(loop6)))) {
-		return 1;
-	}
-
-	/* If the transport is explicitly bound but the determined source differs favor the transport */
-	if (!pj_strcmp(source, &transport->local_name.host)) {
 		return 1;
 	}
 
@@ -107,7 +101,6 @@
 static pj_status_t multihomed_on_tx_message(pjsip_tx_data *tdata)
 {
 	pjsip_tpmgr_fla2_param prm;
-	pjsip_transport *transport = NULL;
 	pjsip_cseq_hdr *cseq;
 	pjsip_via_hdr *via;
 
@@ -122,24 +115,32 @@
 		return PJ_SUCCESS;
 	}
 
-	/* If the transport it is going out on is different reflect it in the message */
-	if (tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP ||
-		tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP6) {
-		transport = multihomed_get_udp_transport(&prm.ret_addr, prm.ret_port);
-	}
+	/* The port in the message should always be that of the original transport */
+	prm.ret_port = tdata->tp_info.transport->local_name.port;
 
-	/* If no new transport use the one provided by the message */
-	if (!transport) {
-		transport = tdata->tp_info.transport;
-	}
+	/* If the IP source differs from the existing transport see if we need to update it */
+	if (pj_strcmp(&prm.ret_addr, &tdata->tp_info.transport->local_name.host)) {
 
-	/* If the message should not be rewritten then abort early */
-	if (!multihomed_rewrite_header(&prm.ret_addr, transport)) {
-		return PJ_SUCCESS;
-	}
+		/* If the transport it is going out on is different reflect it in the message */
+		if (tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP ||
+			tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP6) {
+			pjsip_transport *transport;
 
-	/* Update the transport in case it has changed - we do this now in case we don't want to touch the message above */
-	tdata->tp_info.transport = transport;
+			transport = multihomed_get_udp_transport(&prm.ret_addr, prm.ret_port);
+
+			if (transport) {
+				tdata->tp_info.transport = transport;
+			}
+		}
+
+		/* If the chosen transport is not bound to any we can't use the source address as it won't get back to us */
+		if (!multihomed_bound_any(tdata->tp_info.transport)) {
+			pj_strassign(&prm.ret_addr, &tdata->tp_info.transport->local_name.host);
+		}
+	} else {
+		/* The transport chosen will deliver this but ensure it is updated with the right information */
+		pj_strassign(&prm.ret_addr, &tdata->tp_info.transport->local_name.host);
+	}
 
 	/* If the message needs to be updated with new address do so */
 	if (tdata->msg->type == PJSIP_REQUEST_MSG || !(cseq = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_CSEQ, NULL)) ||
@@ -148,7 +149,7 @@
 		if (contact && (PJSIP_URI_SCHEME_IS_SIP(contact->uri) || PJSIP_URI_SCHEME_IS_SIPS(contact->uri))) {
 			pjsip_sip_uri *uri = pjsip_uri_get_uri(contact->uri);
 
-			/* prm.ret_addr is allocated from the tdata pool so it is perfectly fine to just do an assignment like this */
+			/* prm.ret_addr is allocated from the tdata pool OR the transport so it is perfectly fine to just do an assignment like this */
 			pj_strassign(&uri->host, &prm.ret_addr);
 			uri->port = prm.ret_port;
 

-- 
To view, visit https://gerrit.asterisk.org/364
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b57362201cc8c6555834ec8707e9fbddeff7904
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>



More information about the asterisk-code-review mailing list