[Home]

Summary:ASTERISK-13107: [patch] Add packet information to debug
Reporter:Damien Wedhorn (wedhorn)Labels:
Date Opened:2008-11-22 14:38:25.000-0600Date Closed:2008-12-04 10:37:10.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_skinny/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 2008112400_13952_packetdebug.diff
( 1) packetdebug.diff
( 2) packetdebug3.diff
Description:Adds "packet" option to "skinny set debug" which results in skinny packet types and destination/source being displayed on the CLI. Has been very helpful for debugging.

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

Some interesting info: on hangup, chan_skinny sends the various packets associated with the hangup 3 times instead of once!
Comments:By: Leif Madsen (lmadsen) 2008-11-24 09:36:42.000-0600

Assigning to mvanbaak for now. Are you able to take a quick look at this patch and determine whether it is useful, and if so, then perhaps post on reviewboard after your initial review?

Thanks!
By: Michiel van Baak (mvanbaak) 2008-11-24 14:08:20.000-0600

it's usefull, but we need a menuselect entry for this.
I'll look into it and put it on reviewboard.

Thanks for the headsup blitz
By: Michiel van Baak (mvanbaak) 2008-11-24 15:29:35.000-0600

@wedhorn: Can you test this patch ?
In order to get the new packetdebug stuff, run ./configure --enable-dev-mode and select it in 'compiler flags'

If you agree, I'll post it on reviewboard to get a code review
By: Damien Wedhorn (wedhorn) 2008-11-24 16:23:11.000-0600

I'll test it when I get home but in the meantime a couple of comments.

At about line 2020 there's an issue regarding logging the packets. If SKINNY_PACKETDEBUG is not set, skinnydebug can't be greater than 1 and so the logging will never occur. Do we even need this logging?

There's some stuff at about line 5200 (us and ourip) that shouldn't be part of that patch (leftovers from the axe patch).

Rather than SKINNY_PACKETDEBUG, it may be more sensible to change it to SKINNY_DEVMODE or similar so we can add further dev type stuff. I've partially written a bit of code to craft and transmit a packet on the fly which is helpful for protocol work. Again, this wouldn't be something you would want to inflict on users.
By: Damien Wedhorn (wedhorn) 2008-11-25 03:27:23.000-0600

Updated version added (v3). Removed the packet logging at about line 2020 and removed the leftovers at about 5200.

Change option to SKINNY_DEVMODE and used a macro (SKINNY_DEVONLY) for displaying packets. The macro is just an idea but I think it makes the code easier to read.

Tested with the compiler flag on and off. Works fine.

ps I suppose putting it in the dev-mode compiler flags is your way of making me use dev-mode ;)
By: Digium Subversion (svnbot) 2008-12-04 10:37:09.000-0600

Repository: asterisk
Revision: 160938

U   trunk/build_tools/cflags-devmode.xml
U   trunk/channels/chan_skinny.c

------------------------------------------------------------------------
r160938 | mvanbaak | 2008-12-04 10:37:08 -0600 (Thu, 04 Dec 2008) | 9 lines

Add debug flag so skinny debug will show information about packets.
We dont want to scare users with this, so we added a devmode compile flag

(closes issue ASTERISK-13107)
Reported by: wedhorn
Patches:
     packetdebug3.diff uploaded by wedhorn (license 30)
Tested by: mvanbaak, wedhorn

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

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