[Asterisk-code-review] chan pjsip: transfers with direct media reinvite has wrong a... (asterisk[master])
Kevin Harwell
asteriskteam at digium.com
Wed Mar 16 13:40:53 CDT 2016
Kevin Harwell has uploaded a new change for review.
https://gerrit.asterisk.org/2418
Change subject: chan_pjsip: transfers with direct media reinvite has wrong address/port
......................................................................
chan_pjsip: transfers with direct media reinvite has wrong address/port
During a transfer involving direct media a race occurs between when the
transferer channel is swapped out, initiating rtp changes/updates, and the
subsequent reinvites.
When Alice, after speaking with Charlie (Bob is on hold), connects Bob and
Charlie invites are sent to each in order to establish the call between them.
Bob is taken off hold and Charlie is told to have his media flow through
Asterisk. However, if before those invites go out the bridge updates Bob's
and/or Charlie's rtp information with direct media data (i.e. address, port)
then the invite(s) will contain the remote data in the SDP instead of the
Asterisk data.
The race occurs in the native bridge glue code when updating the peer. The
direct_media_address can get set twice before sending out the first invite
during call connection. This can happen because the checking/setting of the
direct_media_address happened in one thread while the sending of the invite(s)
happened in another thread.
This fix removes the race condition by moving the checking/setting of the
direct_media_address to be in the same thread as the sending of the invites(s).
This serializes the checking/setting and sending so they can no longer happen
out of order.
ASTERISK-25849 #close
Change-Id: Idfea590175e74f401929a601dba0c91ca1a7f873
---
M channels/chan_pjsip.c
1 file changed, 78 insertions(+), 31 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/18/2418/1
diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 32291e5..d2566f0 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -223,17 +223,6 @@
ast_format_cap_append_from_cap(result, channel->session->endpoint->media.codecs, AST_MEDIA_TYPE_UNKNOWN);
}
-static int send_direct_media_request(void *data)
-{
- struct ast_sip_session *session = data;
- int res;
-
- res = ast_sip_session_refresh(session, NULL, NULL, NULL,
- session->endpoint->media.direct_media.method, 1);
- ao2_ref(session, -1);
- return res;
-}
-
/*! \brief Destructor function for \ref transport_info_data */
static void transport_info_destroy(void *obj)
{
@@ -302,6 +291,78 @@
return changed;
}
+struct rtp_direct_media_data {
+ struct ast_channel *chan;
+ struct ast_rtp_instance *rtp;
+ struct ast_rtp_instance *vrtp;
+ struct ast_format_cap *cap;
+ struct ast_sip_session *session;
+ int changed;
+};
+
+static void rtp_direct_media_data_destroy(void *data)
+{
+ struct rtp_direct_media_data *cdata = data;
+
+ ao2_cleanup(cdata->session);
+ ao2_cleanup(cdata->cap);
+ ao2_cleanup(cdata->vrtp);
+ ao2_cleanup(cdata->rtp);
+ ao2_cleanup(cdata->chan);
+}
+
+static struct rtp_direct_media_data *rtp_direct_media_data_create(
+ struct ast_channel *chan, struct ast_rtp_instance *rtp, struct ast_rtp_instance *vrtp,
+ const struct ast_format_cap *cap, struct ast_sip_session *session)
+{
+ struct rtp_direct_media_data *cdata = ao2_alloc(sizeof(*cdata), rtp_direct_media_data_destroy);
+
+ if (!cdata) {
+ return NULL;
+ }
+
+ cdata->chan = ao2_bump(chan);
+ cdata->rtp = ao2_bump(rtp);
+ cdata->vrtp = ao2_bump(vrtp);
+ cdata->cap = ao2_bump((struct ast_format_cap *)cap);
+ cdata->session = ao2_bump(session);
+
+ return cdata;
+}
+
+static int send_direct_media_request(void *data)
+{
+ struct rtp_direct_media_data *cdata = data;
+ struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(cdata->chan);
+ struct chan_pjsip_pvt *pvt = channel->pvt;
+ int changed = 0, res = 0;
+
+ if (pvt->media[SIP_MEDIA_AUDIO]) {
+ changed |= check_for_rtp_changes(
+ cdata->chan, cdata->rtp, pvt->media[SIP_MEDIA_AUDIO], 1);
+ }
+ if (pvt->media[SIP_MEDIA_VIDEO]) {
+ changed |= check_for_rtp_changes(
+ cdata->chan, cdata->vrtp, pvt->media[SIP_MEDIA_VIDEO], 3);
+ }
+
+ if (cdata->cap && ast_format_cap_count(cdata->cap) &&
+ !ast_format_cap_identical(cdata->session->direct_media_cap, cdata->cap)) {
+ ast_format_cap_remove_by_type(cdata->session->direct_media_cap, AST_MEDIA_TYPE_UNKNOWN);
+ ast_format_cap_append_from_cap(cdata->session->direct_media_cap, cdata->cap, AST_MEDIA_TYPE_UNKNOWN);
+ changed = 1;
+ }
+
+ if (changed) {
+ ast_debug(4, "RTP changed on %s; initiating direct media update\n", ast_channel_name(cdata->chan));
+ res = ast_sip_session_refresh(cdata->session, NULL, NULL, NULL,
+ cdata->session->endpoint->media.direct_media.method, 1);
+ }
+
+ ao2_ref(cdata, -1);
+ return res;
+}
+
/*! \brief Function called by RTP engine to change where the remote party should send media */
static int chan_pjsip_set_rtp_peer(struct ast_channel *chan,
struct ast_rtp_instance *rtp,
@@ -311,9 +372,8 @@
int nat_active)
{
struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(chan);
- struct chan_pjsip_pvt *pvt = channel->pvt;
struct ast_sip_session *session = channel->session;
- int changed = 0;
+ struct rtp_direct_media_data *cdata;
/* Don't try to do any direct media shenanigans on early bridges */
if ((rtp || vrtp || tpeer) && !ast_channel_is_bridged(chan)) {
@@ -326,31 +386,18 @@
return 0;
}
- if (pvt->media[SIP_MEDIA_AUDIO]) {
- changed |= check_for_rtp_changes(chan, rtp, pvt->media[SIP_MEDIA_AUDIO], 1);
- }
- if (pvt->media[SIP_MEDIA_VIDEO]) {
- changed |= check_for_rtp_changes(chan, vrtp, pvt->media[SIP_MEDIA_VIDEO], 3);
- }
-
if (direct_media_mitigate_glare(session)) {
ast_debug(4, "Disregarding setting RTP on %s: mitigating re-INVITE glare\n", ast_channel_name(chan));
return 0;
}
- if (cap && ast_format_cap_count(cap) && !ast_format_cap_identical(session->direct_media_cap, cap)) {
- ast_format_cap_remove_by_type(session->direct_media_cap, AST_MEDIA_TYPE_UNKNOWN);
- ast_format_cap_append_from_cap(session->direct_media_cap, cap, AST_MEDIA_TYPE_UNKNOWN);
- changed = 1;
+ if (!(cdata = rtp_direct_media_data_create(chan, rtp, vrtp, cap, session))) {
+ return 0;
}
- if (changed) {
- ao2_ref(session, +1);
-
- ast_debug(4, "RTP changed on %s; initiating direct media update\n", ast_channel_name(chan));
- if (ast_sip_push_task(session->serializer, send_direct_media_request, session)) {
- ao2_cleanup(session);
- }
+ if (ast_sip_push_task(session->serializer, send_direct_media_request, cdata)) {
+ ast_log(LOG_ERROR, "Unable to send direct media request for channel %s\n", ast_channel_name(chan));
+ ao2_ref(cdata, -1);
}
return 0;
--
To view, visit https://gerrit.asterisk.org/2418
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfea590175e74f401929a601dba0c91ca1a7f873
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
More information about the asterisk-code-review
mailing list