[asterisk-dev] [Code Review] 2511: Add core API for attended transfers and partially hide masquerade usage.

Mark Michelson reviewboard at asterisk.org
Mon May 13 16:06:25 CDT 2013 


> On May 9, 2013, 11:51 p.m., rmudgett wrote:
> > /team/mmichelson/transfer/bridges/bridge_builtin_features.c, line 176
> > <https://reviewboard.asterisk.org/r/2511/diff/1/?file=37436#file37436line176>
> >
> >     This callback needs to know if it is redirecting a channel or the bridge.  If you are redirecting a channel, you don't want to do the data copy.  If you are transferring the bridge, then you do want to copy the data.
> >     
> >     The called party of a bridge transfer is going to see the transferrer as the calling party and be slightly surprised when he finds out that the caller is not there with the parties in the bridge.  In this case though what caller information can be used since an ad-hock bridge has some difficulty in knowing its party id.
> 
> Mark Michelson wrote:
>     I've been giving this some thought. The reason this callback is here is because originally, the blind transfer code called dial_transfer() in order to create a local channel and dial the extension and context. dial_transfer() does this same copying of information. dial_transfer() is also called for the builtin attended transfer code. For attended transfers, since the call is being made from the transferer to the transfer target, the copying of caller data to the outbound local channel makes sense.
>     
>     For blind transfers, I think that copying datastores and channel variables really just does not make sense at all since the call is not supposed to be coming from the transferer channel. In addition, I think the connected line information for the outbound call should be handled by the bridge core instead of in a callback.
>     
>     In other words, I think the callback should be removed entirely. What do you think?
> 
> rmudgett wrote:
>     In general, I think the callback still should indicate if the transfer is a redirect or bridge transfer anyway.  It seems like a useful piece of information the callback may need.
>     
>     We still should copy channel variables and datastores from the transferrer channel.  We just need to determine what to use as a caller-id to send out.  In this case, the callback is not needed because it seems like a general requirement for blind transferring a bridge.
>     
>     What COLP should the bridge supply?  For two-party bridges it is easy: the caller-id of the peer.  For multi-party bridges, that is not defined yet.  How can a multi-party bridge get a COLP identity representing the members of the bridge?  For ConfBridges, it easy since the bridge profile could be expanded to include an identity.  Ad-hock multi-party bridges don't have an obvious way to get an identity.  We could define bridge identities for specific scenarios:  "Bridge Transfer" <???>, "Multi-party" <???>.  We could take the caller-id of the oldest channel in the bridge.
> 
> Mark Michelson wrote:
>     What I'll do is modify core blind transfer code in the following way:
>     
>     1) For two-party transfers, I will set the connected line on the outbound call to be the transferee's information.
>     2) For multi-party transfers (i.e. bridge transfers) I will add a BUGBUG comment where the connected line info should be set so that it can be addressed.

I did some more thinking about this, and I've realized that your initial comment about connected line is partially incorrect.

"The called party of a bridge transfer is going to see the transferrer as the calling party"

This actually is not the case. The transferer's caller identity is being copied to the *connected line* information of the channel that will be running dialplan. This does not change the *caller* information of the channel that will run dialplan. Thus, in a two-party transfer, the called party will see the transferee's identity when he is called. What is problematic is that the transferee has the transferer's caller identity set as his connected line. While this likely would get overwritten once the transferee is connected to the transfer target, there is no point in copying the transferer's caller information because the transferee should *already* have the transferer's identity set as his connected line.

So my approach for fixing this now is going to be just to remove the copying of the transferer's connected line information to the channel running dialplan in the blind transfer callback. This way, things should resolve themselves properly by the bridging core once the final bridge is established.

I have also modified the blind transfer callback so that it will indicate the type of transfer being performed (i.e. single-party vs. multi-party).


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2511/#review8569
-----------------------------------------------------------


On May 8, 2013, 6:36 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2511/
> -----------------------------------------------------------
> 
> (Updated May 8, 2013, 6:36 p.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Bugs: ASTERISK-21334 and ASTERISK-21336
>     https://issues.asterisk.org/jira/browse/ASTERISK-21334
>     https://issues.asterisk.org/jira/browse/ASTERISK-21336
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This changeset contains two separate items
> 
> First, there is a new bridge function called ast_bridge_transfer_attended(). In review 2470, this was a stubbed out function, but here, the actual content has been filled in. It performs an attended transfer given the two transferer channels involved. This function is intended to handle cases where both transferer channels are in bridges, or where only one transferer channel is in a bridge. There is currently one piece of functionality missing, and it is dependent on review 2508. I cannot currently perform an attended transfer of a bridge to an unbridged call since I cannot manipulate a local channel in the appropriate way. The addition of the unreal channels in 2508 will allow me to do this.
> 
> Second, the larger of the changes was to go through places where masquerades are performed and eliminate or hide them as much as possible. Two new functions have been created in the channel API, ast_channel_yank() and ast_channel_move(). ast_channel_yank() is intended to be used when code needs to gain local control of a channel in order to redirect it someplace new. ast_channel_move() is intended to be used as a replacement for old uses of ast_channel_masquerade() and ast_do_masquerade() in order to move one channel into where another channel is executing. Neither of these functions should be needed on bridged channels since the bridging API provides ways for channels to move between bridges or perform a task upon exiting a bridge.
> 
> The highlights of this second change include changing the Bridge AMI action and Bridge dialplan applications to no longer require explicit masquerade calls in order to operate. Similarly, the masquerade call made in ast_do_pickup() has changed to use ast_channel_move() instead. ast_async_goto() has been modified to use ast_channel_yank() when redirecting an unbridged channel.
> 
> In the summary for the review, I mentioned that masquerade usage is only partially handled. There are some road blocks preventing complete masquerade hiding:
> 1) The blind transfer and attended transfer code in features.c currently uses masquerades. This code is more-or-less dead at this point, but since the code is still in the tree, it's still using masquerade calls. This code is slated for eventual removal.
> 2) There are a number of channel drivers that make use of ast_channel_masquerade() and ast_channel_transfer_masquerade() in order to perform transfers. These are slated to eventually make use of ast_bridge_transfer_blind() and ast_bridge_transfer_attended() in order to actually perform their transfers.
> 3) There are several channel API calls that check ast_channel_masq(chan) and will call ast_do_masquerade() if the result is non-NULL. Unfortunately, this is still needed because of the channel drivers mentioned in item 2. They simply set up the masquerade and expect the channel thread to actually perform the masquerade.
> 
> Once these three points are addressed, then ast_channel_transfer_masquerade() can be removed entirely, and ast_channel_masquerade() and ast_do_masquerade() can be combined into a single function that simply performs the masquerade.
> 
> As a final note, I also, because I was thinking about these sorts of things, changed the bridge builtin blind transfer code to use ast_bridge_transfer_blind() instead of using what it was previously using.
> 
> 
> Diffs
> -----
> 
>   /team/mmichelson/transfer/bridges/bridge_builtin_features.c 388000 
>   /team/mmichelson/transfer/include/asterisk/bridging.h 388000 
>   /team/mmichelson/transfer/include/asterisk/channel.h 388000 
>   /team/mmichelson/transfer/main/bridging.c 388000 
>   /team/mmichelson/transfer/main/channel.c 388000 
>   /team/mmichelson/transfer/main/features.c 388000 
>   /team/mmichelson/transfer/main/pbx.c 388000 
> 
> Diff: https://reviewboard.asterisk.org/r/2511/diff/
> 
> 
> Testing
> -------
> 
> I tested the Manager Bridge action and the Bridge dialplan application with these changes applied. I also did manager redirects of bridged and unbridged channels to ensure they were redirected properly.
> 
> I tested attended transfer code using chan_sip. The chan_sip changes will be in a separate review.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130513/cfda4cae/attachment-0001.htm>

More information about the asterisk-dev mailing list