[asterisk-bugs] [JIRA] (ASTERISK-24873) Cancelling transmission of IAX2 packet loop wrap around

[Y Ateya (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-24873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=225569#comment-225569 ] 

Y Ateya commented on ASTERISK-24873:
------------------------------------

@ibercom After investigating more, it seems that the problem is not with the canceling for loop.

The problem is accepting control frames with {{fr->iseqno}} less than {code}iaxs[fr->callno]->rseqno{code}. It happens more with VNAK; but this is [correlation not causation|https://xkcd.com/552/].

{code}
* Rx <ACK> with 008 ISeqno: 007
--------------------------------
[2015-03-12 13:26:03.071] VERBOSE[6960] chan_iax2.c: Rx-Frame Retry[ No] -- OSeqno: 008 ISeqno: 007 Type: IAX     Subclass: ACK    
[2015-03-12 13:26:03.071] VERBOSE[6960] chan_iax2.c:    Timestamp: 19717ms  SCall: 07431  DCall: 25289 45.56.114.174:4566

* Rx <LAGRP> with OSeqno: 008 ISeqno: 006
------------------------------------------
[2015-03-12 13:26:03.099] VERBOSE[6960] chan_iax2.c: Rx-Frame Retry[ No] -- OSeqno: 008 ISeqno: 006 Type: IAX     Subclass: LAGRP  
[2015-03-12 13:26:03.099] VERBOSE[6960] chan_iax2.c:    Timestamp: 20007ms  SCall: 07431  DCall: 25289 45.56.114.174:4566
{code}
The cause of this problem is some packet arrives out-of-order; so {code}iaxs[fr->callno]->rseqno{code} (highest recieved {{fr->iseqno}}) is higher than {{fr->iseqno}}). The following steps occurs:

- Peer receives frame with {{fr->iseqno= N, fr->oseqno=Y}}, it cancels all waiting-for-ack packets before N (N-1, N-2, ... )
- Peer receives out-of-sequence frame with {{fr->iseqno = N-1, fr->oseqno=Y}} (same oseqno), frame is accepted and trying to cancel all waiting-for-ack packets before N; 
**but wait** N-1 was acknowledged before!

Either we :
 - Send VNAK for out-of-order {{fr->iseqno}} as we do for {{fr->oseqno}} (this might violate RFC).
 - Prevent starting two control messages at same time (ex. don't send PING if you are waiting for LAGRP); but solution seems too complex.
 - Add special handling for this case to prevent this case specifically.

> Cancelling transmission of IAX2 packet loop wrap around
> -------------------------------------------------------
>
>                 Key: ASTERISK-24873
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-24873
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_iax2
>    Affects Versions: 13.2.0
>            Reporter: Y Ateya
>            Severity: Minor
>         Attachments: client_logs_cancelling_packet_issue.txt.bz2, use_case.txt
>
>
> In IAX, when a packet is received with sequence number, all not-yet-ACKed packets with lower sequence number shall be considered ACKed.
> Sometimes after VNAK is recieved, the first packet sequence number makes the for loop run for useless 255 times.
> check attachment: use_case.txt for extracted sequence of packets.
> check attachment: client_logs_cancelling_packet_issue.txt.bz2 for a full log with many occurrences of this issue.
> {code:title=The affected for loop|borderStyle=solid}
> 			/* First we have to qualify that the ACKed value is within our window */
> 			if (iaxs[fr->callno]->rseqno >= iaxs[fr->callno]->oseqno || (fr->iseqno >= iaxs[fr->callno]->rseqno && fr->iseqno < iaxs[fr->callno]->oseqno))
> 				x = fr->iseqno;
> 			else
> 				x = iaxs[fr->callno]->oseqno;
> 			if ((x != iaxs[fr->callno]->oseqno) || (iaxs[fr->callno]->oseqno == fr->iseqno)) {
> 				/* The acknowledgement is within our window.  Time to acknowledge everything
> 				   that it says to */
> 				for (x=iaxs[fr->callno]->rseqno; x != fr->iseqno; x++) {
> 					/* Ack the packet with the given timestamp */
> 					if (iaxdebug)
> 						ast_debug(1, "Cancelling transmission of packet %d\n", x);
> {code}
> "Cancelling transmission of packet X" shall be printed only for packets with lower sequence number than received packet.
> Simple solution of using `<` for checking end of loop is not enough as it don't handle sequence number wrap around.
> Note that there is not functional effect of this issue. It just blocks its thread in useless 255 loop.



--
This message was sent by Atlassian JIRA
(v6.2#6252)