[Home]

Summary:ASTERISK-15624: [patch] Endianess problems in skinny messages
Reporter:Philippe Teissier (pipocanaja)Labels:
Date Opened:2010-02-14 17:19:34.000-0600Date Closed:2010-03-01 13:27:36.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_skinny
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_skinny.c_bigendianPatch_20100218.diff
Description:I'm using a sun machine as my asterisk server:
Linux sunsrv 2.6.18-gre #3 SMP Wed Sep 24 22:12:02 CEST 2008 sparc64 GNU/Linux

I faced some endianess problems in the skinny exchanges with my 2 phones (Cisco 7920 and Cisco 7912).

In default code, the phone would not even register completely.

I compared the wireshark trace of the same SVN version (Revision: 246625) on an intel machine and this sun, and based on this, I patched my chan_skinny.c to improve it.
Basically, it means: removing (most of the time) or adding byteswapping. On intel platform, no swapping is needed (the phone expects LittleEndian) so the functions are returning the same value they are called with. So if not tested with a Big Endian machine, it's easy to call those byteswapp functions more than needed.

Basically, I'm now able to register, and to do the echo test. It needs some more testing but I'm now opening the bug report so that if somebody else is facing that issue/working at it, we can share and test patches.

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

<Code Removed>
Comments:By: Philippe Teissier (pipocanaja) 2010-02-14 17:26:46.000-0600

I would add that this issue must be quite old. I did some tests on 1.2 and at this time (almost 2 years ago), chan_skinny was already endianess-dirty as far as I remember.
By: Michiel van Baak (mvanbaak) 2010-02-15 00:54:21.000-0600

I'm looking forward to see your patch.

and you might indeed be the only one running chan_skinny on big endian ;)
By: Philippe Teissier (pipocanaja) 2010-02-17 12:18:59.000-0600

Hi mvanbaak,

I did some more tests here, and for simple operation (register a phone, make a phone call, receive a phone call) it works well on my 3 test phones here (2x7920 and 1x7912).

What kind of tests would you suggest to do ? Is there a "testing procedure" that I should apply to this ?

Concerning the Call Transfer, and other advanced feature, where can I find the official status ? Are they supposed to work ? Call transfer usually ends up in a crash here. I haven't tested it on a little endian platform, in order to see if it's the same.

The patch is the one in the Additionnal Informations above.

Bye for now
By: Michiel van Baak (mvanbaak) 2010-02-17 17:01:43.000-0600

On an unpatched system I have occasional crashes with call transfers as well.
The stub rewrite done by wedhorn fixes most of those, so dont worry about them at the moment (unless you have a fix right now)

The tests you did (register, make call, receive call) are enough for the moment.

We cannot accept patches in the Additional info. You have to upload them as patch to this bug. This is because of the license and all.
Please sign the license if you did not do already and upload the patch to this bug. That way I can look at it, test it, and commit it.

Thanks again for the work and time you spent on this!
By: Michiel van Baak (mvanbaak) 2010-02-18 02:41:44.000-0600

Thanks. As soon as your license is approved I'll have a look.
By: Philippe Teissier (pipocanaja) 2010-02-18 02:41:59.000-0600

Ok

It seems to be all right now. I haven't looked at the issues on call transfers. But I can do some testing if you point me on where to get this patch.

Bye for now,
By: Digium Subversion (svnbot) 2010-03-01 13:27:35.000-0600

Repository: asterisk
Revision: 249669

U   trunk/channels/chan_skinny.c

------------------------------------------------------------------------
r249669 | mvanbaak | 2010-03-01 13:27:35 -0600 (Mon, 01 Mar 2010) | 9 lines

fix endianes issues in chan_skinny

(closes issue ASTERISK-15624)
Reported by: PipoCanaja
Patches:
     chan_skinny.c_bigendianPatch_20100218.diff uploaded by PipoCanaja (license 994)
Tested by: wedhorn


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

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