[Home]

Summary:ASTERISK-12652: [patch] Reason header support
Reporter:adomjan (adomjan)Labels:
Date Opened:2008-08-27 10:40:46Date Closed:2009-11-02 09:02:33.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20080829_chan_sip.c-q850reason_header.patch
( 1) chan_sip.c-1.6.0.9-q850reason.patch-fixed
( 2) chan_sip.c-q850reason_header.patch
( 3) chan_sip.c-trunk20090929-reason_q850_atoi_fix.patch
( 4) chan_sip.c-trunk20090929-reason_q850.patch
( 5) chan_sip.c-trunk-q850-reason.patch
( 6) CHANGES-trunk20090929-reason_q850.patch
( 7) sip.conf.sample-trunk20090929-reason_q850.patch
( 8) sip-q850-hangupcause1.diff
Description:Cisco GWs use this header, however it is only a rfc draft.

draft-jesske-sipping-etsi-ngn-reason-03.txt

just set:
use_q850_reason=yes
in sip peer/user configuration and chan_sip.c will insert
Reason: Q.850;cause=17
header.
If the peer send Reason header will be used instead of hangup_sip2cause().

Comments:By: Leif Madsen (lmadsen) 2008-08-27 10:51:04

As this is an RFC draft (and not a standard), seems to be mostly related to Cisco GWs, and does not have a patch attached to implement the feature, I'm closing this issue. If you feel this issue has been closed in error, please find me on IRC in #asterisk-bugs on irc.freenode.net.

Thanks!
By: Leif Madsen (lmadsen) 2008-08-27 11:37:04

Reopening issue per #asterisk-bugs as reporter is providing a patch. Thanks!!
By: adomjan (adomjan) 2008-08-27 12:14:36

btw we run out the bits of the flags...
s/ast_flags/ast_flags64/g ?
By: adomjan (adomjan) 2008-08-29 05:23:50

opps in sip_hangup() unrefer sip pvt from ast_channel before call the transmit_response_reliable(). I added a new parameter into transmit_response_reliable() any better idea?
By: Olle Johansson (oej) 2009-01-23 08:58:07.000-0600

Good stuff. Thanks!
By: Olle Johansson (oej) 2009-01-26 04:26:52.000-0600

I would like a global option as well, thank you. Do you think you can fix that?
By: adomjan (adomjan) 2009-01-26 04:38:51.000-0600

yes of course, I hope I will have time on this week.
btw we run out of the flags, what should we do

- new page of flags
- moving flags to 64bits?
By: Olle Johansson (oej) 2009-01-26 05:58:39.000-0600

We have been moving stuff away from flags, so there should be free flags.
By: Olle Johansson (oej) 2009-01-26 10:28:47.000-0600

Admonjan: We've decided to handle the problem with the lost hangupcause separately in bug ASTERISK-13415 (related to this one). So please wait with your code rework until we've committed that stuff and tested it. If you have time to test it, I'm very grateful.

We needed to separate that issue, since it was a problem going back to 1.4. The reason header part will only be integrated in trunk. Thanks for spotting that issue, btw!
By: Digium Subversion (svnbot) 2009-01-29 02:47:32.000-0600

Repository: asterisk
Revision: 172169

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r172169 | oej | 2009-01-29 02:47:31 -0600 (Thu, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


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

http://svn.digium.com/view/asterisk?view=rev&revision=172169
By: Digium Subversion (svnbot) 2009-01-29 03:17:17.000-0600

Repository: asterisk
Revision: 172173

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r172173 | oej | 2009-01-29 03:17:16 -0600 (Thu, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

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

http://svn.digium.com/view/asterisk?view=rev&revision=172173
By: Olle Johansson (oej) 2009-01-29 03:52:15.000-0600

Adomjan, as you see, I've committed a patch based on your input to trunk that solves the hangupcause issue. The code is now ready for you to update your patch. I look forward to working with you on that, as I see it as an important change.

Thanks
/O
By: Digium Subversion (svnbot) 2009-01-29 03:56:34.000-0600

Repository: asterisk
Revision: 172217

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_sip.c

------------------------------------------------------------------------
r172217 | oej | 2009-01-29 03:56:33 -0600 (Thu, 29 Jan 2009) | 32 lines

Merged revisions 172173 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r172173 | oej | 2009-01-29 10:18:01 +0100 (Tor, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

................

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

http://svn.digium.com/view/asterisk?view=rev&revision=172217
By: Digium Subversion (svnbot) 2009-01-29 04:35:50.000-0600

Repository: asterisk
Revision: 172231

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r172231 | oej | 2009-01-29 04:35:50 -0600 (Thu, 29 Jan 2009) | 32 lines

Merged revisions 172173 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r172173 | oej | 2009-01-29 10:18:01 +0100 (Tor, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

................

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

http://svn.digium.com/view/asterisk?view=rev&revision=172231
By: adomjan (adomjan) 2009-01-29 04:48:57.000-0600

Tomorrow I will produce an updated/tested patch.
By: adomjan (adomjan) 2009-01-30 05:48:28.000-0600

Hi,
I produce a patch, but not complete
I worked on current trunk.

problems:

- If in the called dialplan I put only like: Hangup(57); won't be sent the reason code...
- If in the called dialplan I put only Busy() won't be sent the reason code

- If the call answered, and hangup by any cause code, the cause code will be sent, but on the caller side allways will be 16

Will you fix around it, or may I modify the chan_sip deeper?
By: Olle Johansson (oej) 2009-01-30 09:40:56.000-0600

Hmm. I suspected something was wrong with cause codes yesterday. Will check.
By: adomjan (adomjan) 2009-02-11 03:19:13.000-0600

Have you found anything?
By: Olle Johansson (oej) 2009-02-11 03:23:01.000-0600

Sorry, got sidetracked. But I really tried to get other developers to look at it... Will check again soon.
By: Roberto Guerra (robertog) 2009-02-25 15:09:06.000-0600

I tried this patch this week and found why HANGUPCAUSE is not set when the remote party hangs up.
This part of the patch:

               if (ast_test_flag(&p->flags[1], SIP_PAGE2_Q850_REASON) &&
               (reason_header = strstr(get_header(req, "Reason"), "cause=")) &&
               sscanf(reason_header, "cause=%d", &owner->hangupcause))
                       ast_verb(3,"Using SIP Reason header for cause code:%d\n", owner->hangupcause);

Modifies the handle_response() function in chan_sip.c, but the "Reason:" code of interest (to me at least), comes in a BYE message, which is a _request_ SIP message.

If I missed something obvious, please forgive me. I am just an Asterisk newbie.
By: Roberto Guerra (robertog) 2009-03-02 10:41:48.000-0600

I implemented my idea (above) on chan_sip.c and now it processes Q.850 reason headers for BYE _REQUESTS_. I don't know if this is on the scope of this bug and it belongs here.

Note: my patch modifies '&owner->hangupcause' successfully but because the channel expires upon BYE, the HANGUPCAUSE channel variable is not available in the 'h' extension dialplan.
I changed it further to save the value in a global variable with 'builtin_pbx_setvar_helper(NULL,hangupcause<number>,hangupcausevalue)', but that's kind of unelegant. If anyone is interested in this patch, I'll post it.
By: adomjan (adomjan) 2009-04-07 03:16:17

I just wanted to let you know, I'm using this feature in production, if anybody is interested uploaded the patch for 1.6.0.9
By: adomjan (adomjan) 2009-04-07 03:51:33

Sorry I uploaded previously not the right patch chan_sip.c-1.6.0.9-q850reason.patch please drop it!

chan_sip.c-1.6.0.9-q850reason.patch-fixed is ok
By: () 2009-07-18 15:52:05

Huawei SIP GWs also include the Q.850 Reason header in SIP messages.

I haven't tested the patch as yet. I am using SVN-branch-1.6.0-r199301.
By: Matthew Nicholson (mnicholson) 2009-10-21 09:33:56

Is the trunk version of this patch up to date?
By: adomjan (adomjan) 2009-10-21 09:37:29

Sorry not, I'm using this patch in my 1.6.0.x production systems.
By: Matthew Nicholson (mnicholson) 2009-10-21 09:39:07

Please update the trunk version of this patch to include the features from the 1.6 version and to apply cleanly to the trunk branch.  This issue is a feature request and will only be applied to the trunk version of asterisk.
By: adomjan (adomjan) 2009-10-21 09:47:10

Ok, on next week I will produce new patches.
By: Matthew Nicholson (mnicholson) 2009-10-21 10:03:21

After reviewing the patch, I am curious why the 'hangupcause' parameter was added to transmit_response_reliable.  It looks like that parameter is only set to p->hangupcause or 0.  Would it be possible to just use the p->hangupcause pvt member  in __transmit_response() directly?
By: adomjan (adomjan) 2009-10-21 10:08:16

I will examine this. I can't remember.
By: Matthew Nicholson (mnicholson) 2009-10-21 10:08:42

Additionally in your patch, please update the CHANGES file and the sample sip.conf to reflect the new option.
By: adomjan (adomjan) 2009-10-21 10:10:57

and global configuration option...
By: Matthew Nicholson (mnicholson) 2009-10-28 15:43:51

Any progress on this issue?
By: adomjan (adomjan) 2009-10-29 03:47:20

Please give me some days, I have not time for it yet.
By: adomjan (adomjan) 2009-10-29 09:22:04

I cleaned up and updated to the today trunk the patch.
By: Matthew Nicholson (mnicholson) 2009-10-29 14:51:15

This looks good.  There are a few whitespace issues that need to be cleaned up.  Also I found a potential bug.  In the handle_response() function, if 'cause=' is not included in the reason header, strstr will return NULL, and asterisk should crash on the call to atoi.  A header in the format "Reason: Q.850;" should cause a crash.
By: adomjan (adomjan) 2009-10-29 17:42:21

I will fix the potential crash and white spaces (idiot netbeans, however I configured for the right formatting).
By: adomjan (adomjan) 2009-10-30 05:44:14

chan_sip.c-trunk20090929-reason_q850_atoi_fix.patch:

fixed the potential atoi crash
By: Matthew Nicholson (mnicholson) 2009-10-30 13:31:01

Ok, I made some additional minor changes and uploaded a new patch (sip-q850-hangupcause1.diff).  Please test it, if it works, I will commit it.
By: adomjan (adomjan) 2009-10-30 15:03:08

ok, I will test it on next week.
By: adomjan (adomjan) 2009-11-02 03:39:29.000-0600

I tested the sip-q850-hangupcause1.diff, the patch is ok.
By: Digium Subversion (svnbot) 2009-11-02 09:02:29.000-0600

Repository: asterisk
Revision: 226687

U   trunk/CHANGES
U   trunk/channels/chan_sip.c
U   trunk/configs/sip.conf.sample

------------------------------------------------------------------------
r226687 | mnicholson | 2009-11-02 09:02:28 -0600 (Mon, 02 Nov 2009) | 12 lines

This patch adds support for a draft proposal for adding Q.850 reason headers to sip messages.

(closes issue ASTERISK-12652)
Reported by: adomjan
Patches:
     sip.conf.sample-trunk20090929-reason_q850.patch uploaded by adomjan (license 487)
     CHANGES-trunk20090929-reason_q850.patch uploaded by adomjan (license 487)
     chan_sip.c-trunk20090929-reason_q850_atoi_fix.patch uploaded by adomjan (license 487)
     sip-q850-hangupcause1.diff uploaded by mnicholson (license 96)
Tested by: adomjan


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

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