[asterisk-dev] [Code Review] 3174: chan_iax2: Block unnecessary control frames to/from the wire.

[Matt Jordan]

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



/branches/1.8/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/3174/#comment20294>

    I'm curious - why not define the parameter as an enum ast_control_frame_type - and have the cast be done when the function is called?
    
    An int should convert to an enum without any issues, and that would resolve the two variables with the same name and needing to do an additional assignment.



/branches/1.8/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/3174/#comment20293>

    I can't say I'm a big fan of parameters that begin with _. While there are plenty of functions in chan_iax2 that use the '__' prefix, there are no functions that I could find that take in a parameter with a leading '_'.
    
    I'd rename this to something else - maybe frame_subtype, since that's what it is?



/branches/1.8/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/3174/#comment20295>

    This shouldn't be needed if you have a 'default' label that disallows the frame.



/branches/1.8/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/3174/#comment20296>

    This structure feels a little weird, since we handle both this frametype and this subclass earlier. It feels like this should be handled in a single block.


- Matt Jordan


On Feb. 3, 2014, 5:31 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3174/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2014, 5:31 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AST-1302
>     https://issues.asterisk.org/jira/browse/AST-1302
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Establishing an IAX2 call between Asterisk v1.4 and v1.8 (or later) results in an unexpected call disconnect.  The problem happens because newer values in the enum ast_control_frame_type are not consistent between the branch versions of Asterisk.
> 
> For example:
> 1) v1.4 calls v1.8 (or later) using IAX2
> 2) v1.8 answers and sends a connected line update control frame. (on v1.8 AST_CONTROL_CONNECTED_LINE = 22)
> 3) v1.4 receives the control frame as an end-of-q (on v1.4 AST_CONTROL_END_OF_Q = 22)
> 4) v1.4 disconnects the call once the receive queue becomes empty.
> 
> Several things are done by this patch to fix the problem and attempt to prevent it from happening again in the future:
> * Added a warning at the definition of enum ast_control_frame_type about how to add new control frame values.
> 
> * Made block sending and receiving control frames that have no reason to go over the wire.
> 
> * Extended the connectedline iax.conf parameter to also include the redirecting information updates.
> 
> * Updated the connectedline iax.conf parameter documentation to include a notice that the parameter must be "no" when the peer is an Asterisk v1.4 instance.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/include/asterisk/frame.h 407196 
>   /branches/1.8/configs/iax.conf.sample 407196 
>   /branches/1.8/channels/chan_iax2.c 407196 
> 
> Diff: https://reviewboard.asterisk.org/r/3174/diff/
> 
> 
> Testing
> -------
> 
> Without the patch the IAX2 call gets disconnected when the call initially connects.
> With the patch the call stays up.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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