[Home]

Summary:ASTERISK-17209: [patch] Codec negotiation fails on IAX calls from 1.8.1.1 to 1.8.1.1
Reporter:John Covert (jcovert)Labels:
Date Opened:2011-01-06 08:41:47.000-0600Date Closed:2011-02-16 02:47:49.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_iax2
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20110110__issue18583.diff.txt
( 1) failure.txt
( 2) iax2-parser.c.patch
( 3) success.txt
Description:In the example below, an originating 1.8.1.1 system with the ONLY change in iax.conf being changing one line from bandwidth=low to bandwidth=high is not able to connect to a terminating 1.8.1.1 system with the only CODEC change relevant to a guest user being (a) the change of bandwidth=low to bandwidth=high and (b) codecpriority=caller inserted.

An originating 1.6.1.6 system with a configuration identical to the failing originating 1.8.1.1 system is able to connect to the same terminating 1.8.1.1 system, no problem.


****** ADDITIONAL INFORMATION ******

I have uploaded logs of the failure case and the success case.
Comments:By: John Covert (jcovert) 2011-01-06 08:45:33.000-0600

Removing codecpriority=caller from the terminating system's config file has no effect on the problem.
By: John Covert (jcovert) 2011-01-06 10:25:55.000-0600

Further testing indicates that is a problem only when

a.  Both systems are 1.8

AND

b. One system is Linux and the other system is Mac OS X Power PC.

i.e. Mac OS X Power PC systems running 1.8 can only communicate with other Mac OS X Power PC systems running 1.8, or with non-Mac systems running 1.6 or below.


By: John Covert (jcovert) 2011-01-06 10:42:08.000-0600

This appears related to issue ASTERISK-1755804

I am guessing at this point that Endianness was not considered when moving the uint64 into and out of the protocol message, and I am reading the code now to see if this guess is correct.
By: John Covert (jcovert) 2011-01-07 20:07:38.000-0600

Endianness is definitely the problem.

On BIG_ENDIAN machines, the ntohll and htonll functions do nothing, since network byte order is big endian.

The fix is to still do the byte swap, even on BIG_ENDIAN machines (or at least on Darwin PPC).  The implication is that ntohll and htonll should not have ever been called.

The patch takes care of this for Darwin.  I do not have any other BIG_ENDIAN machines to test against.

/john
By: Tilghman Lesher (tilghman) 2011-01-10 13:28:40.000-0600

It appears the problem is actually in include/asterisk/compat.h, where it assumes that all machines that are not SPARC are little endian.
By: Tilghman Lesher (tilghman) 2011-01-10 13:45:54.000-0600

Oh, while that is a problem, it's actually in strcompat.c, where the implementation of htonll is faulty.  This will work correctly on platforms which have that function, so we need to apply the correct fix.  We can certainly increment the version number on the capability2 and format2 IEs to compensate for this change.
By: John Covert (jcovert) 2011-01-10 14:18:30.000-0600

Not so fast.  That's only if it's undef in compat.h, and it won't be because of things which are included before the #ifndef in compat.h

x.c:
#include "include/asterisk/autoconfig.h"
#include "include/asterisk/compat.h"
main() {printf("Big=%d BYTE_ORDER=%d\n",BIG_ENDIAN,BYTE_ORDER);}

asterisk:~/asterisk/asterisk-1.8.1.1 root# cc x.c
asterisk:~/asterisk/asterisk-1.8.1.1 root# ./a.out

Big=4321 Endian=4321
By: John Covert (jcovert) 2011-01-10 14:34:58.000-0600

Yes, you are correct about the little endian faulty implementation of both htonll and ntohll.

That means that my Darwin patch makes me compatible with little endian machines which don't have the xtoyll functions, but incompatible with any which do.

Since this is iax only, and iax appears to really not be ready for prime time in 1.8.1.1 for other reasons, it might not be worth a version change in the protocol, since only little endian machines without xtoyll create the faulty packets.

/john
By: John Covert (jcovert) 2011-01-10 14:40:29.000-0600

Did you test your patch?

In the current version of your patch, you change only one of the xtoyll implementations; they are both wrong.

/john
By: Digium Subversion (svnbot) 2011-01-10 16:39:33.000-0600

Repository: asterisk
Revision: 301263

U   branches/1.8/main/strcompat.c

------------------------------------------------------------------------
r301263 | tilghman | 2011-01-10 16:39:33 -0600 (Mon, 10 Jan 2011) | 8 lines

Little endian machines were not converted properly.

(closes issue ASTERISK-17209)
Reported by: jcovert
Patches:
     20110110__issue18583.diff.txt uploaded by tilghman (license 14)
Tested by: jcovert

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=301263
By: Digium Subversion (svnbot) 2011-01-10 16:40:24.000-0600

Repository: asterisk
Revision: 301264

_U  trunk/
U   trunk/main/strcompat.c

------------------------------------------------------------------------
r301264 | tilghman | 2011-01-10 16:40:23 -0600 (Mon, 10 Jan 2011) | 15 lines

Merged revisions 301263 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r301263 | tilghman | 2011-01-10 16:39:31 -0600 (Mon, 10 Jan 2011) | 8 lines
 
 Little endian machines were not converted properly.
 
 (closes issue ASTERISK-17209)
 Reported by: jcovert
 Patches:
       20110110__issue18583.diff.txt uploaded by tilghman (license 14)
 Tested by: jcovert
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=301264