[Home]

Summary:ASTERISK-24873: Cancelling transmission of IAX2 packet loop wrap around
Reporter:Y Ateya (yateya)Labels:
Date Opened:2015-03-13 07:55:09Date Closed:
Priority:MajorRegression?
Status:Open/NewComponents:Channels/chan_iax2
Versions:13.2.0 Frequency of
Occurrence		Frequent
Related
Issues:
Environment:Attachments:( 0) client_logs_cancelling_packet_issue.txt.bz2
( 1) use_case.txt
Description: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.
Comments:By: ibercom (ibercom) 2015-03-23 13:10:53.035-0500

[~yateya]: do you have a patch for this ?
By: Y Ateya (yateya) 2015-03-23 15:48:28.046-0500

@ibercom After investigating more, it seems that the problem is not -only- 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.