[asterisk-dev] [Code Review] 4483: Separate QoS settings for trunk packets

Matt Jordan reviewboard at asterisk.org
Mon Mar 23 14:52:59 CDT 2015


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



trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/4483/#comment25353>

    This lock concerns me a lot.
    
    Because you are looking to toggle the QOS setting on the socket, you have to protect access to the socket across all threads. That substantially increases the locking on the socket in chan_iax2.
    
    At a minimum, this is called from iax2_write, which is called by the PBX threads servicing the channels. As a result, this would globally lock all IAX2 channels on every frame written to any IAX2 channel.
    
    While this is an interesting idea, the impact of the implementation here feels rather dramatic.


- Matt Jordan


On March 12, 2015, 3:11 p.m., Y Ateya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4483/
> -----------------------------------------------------------
> 
> (Updated March 12, 2015, 3:11 p.m.)
> 
> 
> Review request for Asterisk Developers and mattjordan.
> 
> 
> Bugs: ASTERISK-24866
>     https://issues.asterisk.org/jira/browse/ASTERISK-24866
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently the packet priority is assigned to all packets sent from IAX socket; Which minimizes the benefit of having packet priority.
> 
> This patch adds separate qos settings for IAX trunk packets.
> 
> Summary:
>  - Added trunk_cos and trunk_tos configuraiton variables (similar to cos and tos) to set qos values for trunk packets.
>  - Added dynamic_qos configuration variables (true/false) to enable/disable changing qos dynamically. (default disabled).
>  - Before calling ast_send (and if dynamic_qos is enabled) update qos settings of the socket (if it needs change only).
>  - Lock a mutex before setting qos and unlock it after ast_send. To prevent thread1 setting qos, thread two setting qos and send, then thread1 uses thread2 qos.
> 
> Notes:
>  - Protecting `qos_dynamic_enabled` in reload configurations complicated locking mechanism a little bit.
>  - More qos classes can be added to different packet types later.
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_iax2.c 432806 
> 
> Diff: https://reviewboard.asterisk.org/r/4483/diff/
> 
> 
> Testing
> -------
> 
> Run 100 calls (over IAX trunk) between client and server. Checked that:
>  - If cos, tos are set but dynamic_qos is disabled, tos and cos values are used.
>  - If tos, cos, dynamic_qos, trunk_cos and trunk_tos are set; trunk packets uses trunk_cos/trunk_tos while all other packets use cos/tos.
> 
> 
> Thanks,
> 
> Y Ateya
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150323/33d1e714/attachment.html>


More information about the asterisk-dev mailing list