[asterisk-dev] [Code Review] 3601: accountcode: Slightly change accountcode propagation.

rmudgett reviewboard at asterisk.org
Fri Jun 27 16:20:32 CDT 2014 


> On June 25, 2014, 6:16 p.m., Matt Jordan wrote:
> > /branches/12/main/event.c, lines 146-167
> > <https://reviewboard.asterisk.org/r/3601/diff/2/?file=60432#file60432line146>
> >
> >     For the tweaks in this section that are unrelated to the accountcode changes, please commit them on a separate patch.
> >     
> >     They don't need to be put up for review again.
> >     
> >     Note that changes in datatypes in event.h are typically dangerous - interactions with other Asterisk systems will break (badly) if we change the data types here. Luckily, CEL and Security events are not shared across corosync, so this should be okay.

The changes are because the IE_PLTYPE put on generated events don't match the values in this table.  For v12+ the values in this table only seem to be used by unit tests.  However, v1.8 and v11 have the same type mismatch values in their table.  I haven't looked too hard at what the consequences are in those versions.


> On June 25, 2014, 6:16 p.m., Matt Jordan wrote:
> > /branches/12/main/bridge_basic.c, lines 3330-3336
> > <https://reviewboard.asterisk.org/r/3601/diff/2/?file=60426#file60426line3330>
> >
> >     Why did these personalities lose the ast_bridge_base_v_table?
> >     
> >     This means, for example, that the 'normal' personality will no longer get a vtable method for pulling, nor will it get a vtable method for bridge destruction.

The personality vtable is used as an alteration to the ast_bridge_basic_v_table routines and not as a stand-alone bridge vtable so the normal rules don't apply.  I'm fairly certain that I'm the reason those lines got put into the code during its code review in the first place.  Before this change, the basic bridge pull routine called the normal personality pull routine which was the base bridge pull routine.  The basic bridge pull routine then directly called the base bridge pull routine again.

I'll add a comment.


- rmudgett


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


On June 23, 2014, 3:56 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3601/
> -----------------------------------------------------------
> 
> (Updated June 23, 2014, 3:56 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-65
>     https://issues.asterisk.org/jira/browse/AFS-65
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Accountcode propagation:
> 
> The current behavior is to simply set the accountcode of an outgoing
> channel to the accountcode of the channel initiating the call.  It was
> done this way a long time ago to allow the accountcode set on the SIP/100
> channel to be propagated to a local channel so the dialplan execution on
> the Local;2 channel would have the SIP/100 accountcode available.
> 
> SIP/100 -> Local;1/Local;2 -> SIP/200
> 
> Propagating the SIP/100 accountcode to the local channels is very useful.
> However, a side effect of this is it will overwrite any accountcode set by
> the SIP channel driver configuration for the SIP/200 channel with the
> accountcode from the Local;2 channel.  Without any dialplan manipulation,
> all channels in this call would have the same accountcode.
> 
> Using dialplan, you can set a different accountcode on the SIP/200 channel
> either by setting the accountcode on the Local;2 channel or by the Dial
> application's b(pre-dial), M(macro) or U(gosub) options, or by the
> FollowMe application's b(pre-dial) option, or by the Queue application's
> macro or gosub options.  The altered accountcode on SIP/200 will remain
> until the local channels optimized out when the accountcode would change
> to the SIP/100 accountcode.
> 
> Asterisk v1.8 attempted to add peeraccount support but ultimately had to
> punt on the support.  The peeraccount support was rendered useless because
> of how the CDR code needed to unconditionally force the caller's
> accountcode onto the peer channel's accountcode.  The CEL events were thus
> intentionally made to always use the channel's accountcode as the
> peeraccount value.
> 
> With the arrival of Asterisk v12, the situation has improved so
> peeraccount support can be made to work.  Using the indicated example,
> the goal is to have the accountcode values become as follows:
> 
> SIP/100 ---> Local;1 ---- Local;2 ---> SIP/200
> acct: 100 \/ acct: 200 \/ acct: 100 \/ acct: 200
> peer: 200 /\ peer: 100 /\ peer: 200 /\ peer: 100
> 
> If a channel already has an accountcode it can only change by explicit
> user command by the following methods:
> 
> 1) A channel originate method that can specify an accountcode to use.
> 
> 2) Dialplan using CHANNEL(accountcode).
> 
> 3) Dialplan using CHANNEL(peeraccount) on the other end of a local
> channel pair.
> 
> If a channel does not have an accountcode it can get one from the
> following places:
> 
> 1) The channel driver's configuration at channel creation.
> 
> 2) Before dialing from the calling channel's peeraccount value; or as a
> legacy fallback the calling channel's accountcode.
> 
> 3) Explicit user command as already indicated.
> 
> 4) Entering a bridge from a peer channel's peeraccount value.
> 
> You can specify the accountcode for an outgoing channel that does not have
> one by setting the CHANNEL(peeraccount) before using the Dial, FollowMe,
> and Queue applications.
> 
> Accountcode and peeraccount values propagate to an outgoing channel before
> dialing.  Accountcodes also propagate when channels enter or leave a
> bridge.  The peeraccount value only makes sense for channels in two party
> bridges.
> 
> * Made peeraccount functional by changing accountcode propagation as
> described above.
> 
> * Fixed the basic bridge sub-class to update peeraccount codes when the
> number of channels in the bridge drops back down to two parties.
> 
> * Refactored ast_bridge_channel_update_accountcodes() to handle channels
> joining/leaving the bridge.
> 
> * Fixed the basic bridge to not call the base pull method twice.
> 
> * Fixed CEL extracting the wrong ie value for the peeraccount.  This was
> done intentionally in Asterisk v1.8 when that version had to punt on
> peeraccount.
> 
> * Fixed some incorrect CEL event ie types.  These seem to only be used
> by the unit tests now.
> 
> * Fixed a few places dealing with accountcodes that were reading from
> channels without the lock held.
> 
> 
> NOTE: This patch is against v12 because that was the original target for
> the patch.  However, because the peeraccount changes were more extensive
> than anticipated and resulted in a larger behavior change it will only be
> checked into trunk.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/parking/parking_bridge_features.c 417163 
>   /branches/12/main/pbx.c 417163 
>   /branches/12/main/event.c 417163 
>   /branches/12/main/dial.c 417163 
>   /branches/12/main/core_unreal.c 417163 
>   /branches/12/main/channel.c 417163 
>   /branches/12/main/cel.c 417163 
>   /branches/12/main/bridge_channel.c 417163 
>   /branches/12/main/bridge_basic.c 417163 
>   /branches/12/main/bridge.c 417163 
>   /branches/12/include/asterisk/channel.h 417163 
>   /branches/12/include/asterisk/bridge_channel.h 417163 
>   /branches/12/apps/app_queue.c 417163 
>   /branches/12/apps/app_followme.c 417163 
>   /branches/12/apps/app_dial.c 417163 
>   /branches/12/UPGRADE.txt 417163 
> 
> Diff: https://reviewboard.asterisk.org/r/3601/diff/
> 
> 
> Testing
> -------
> 
> Set the accountcode on the initial channel and let the channel driver supply or not the accountcode for the other channel.
> Without the channel driver supplying the accountcode, the outgoing channel got the same accountcode as the initial channel.
> With the channel driver supplying the accountcode, the outgoing channel kept its accountcode.
> 
> Let the channel driver supply the accountcode for all endpoint channels.
> Performed a DTMF attended transfer with consultation and creation of a three party bridge.
> When the transferrer channel left the three party bridge, the remaining two channels got the peeraccount updated to the other party.
> 
> Throughout each of these tests, the CEL log had the expected peeraccount value.  Note the peeraccount value is meaningless if the bridge contains more than two parties.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140627/604744f6/attachment-0001.html>

More information about the asterisk-dev mailing list