[Home]

Summary:ASTERISK-07083: [patch] add T309 to libpri : maintain calls in case of brief alarms
Reporter:Frederic LE FOLL (flefoll)Labels:
Date Opened:2006-06-02 09:32:31Date Closed:2006-07-06 16:52:19
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) patch_T309_chan_zap.c_rev32848.txt
( 1) patch_T309_pri_q931.h.txt
( 2) patch_T309_pri.c.txt
( 3) patch_T309_q921.c_rev350.txt
( 4) patch_T309_q931.c_rev352.txt
( 5) patch_T309_zapata.conf.sample_rev31413.txt
Description:Asterisk/libpri currently does implement T309, although it is mandatory when running in Network mode ... and quite comfortable when running in User mode.

Instead, currently, Asterisk has an applicative "kill'em all" workaround when chan_zap receives an alarm notification.

I propose a patch to implement T309. Unluckily, this patch touches a number of files, but most changes are minor.

* Patch description

- Asterisk chan_zap.c : Deactivate kill'em all workaround if T309 is configured (zt_handle_events() case ZT_EVENT_ALARM and pri_dchannel() case PRI_EVENT_DCHAN_DOWN).

- pri.c : Display T309 value in pri_dump_info_str().

- q921.c : Add calls to new function q931_dl_indication() in q921_dchannel_down() and ..._up().

- pri_q931.h : Add the prototype for q931_dl_indication().

- q931.c : the biggest part ! Add new functions for T309 ...
. q931_dl_indication() receives UP/DOWN indications from L2, parses calls in pri->callpool, arms/inhibits T309, or cancel non active calls.
. pri_dl_down_cancelcall() is designed to be scheduled with 0 delay for non active calls when L2 goes down.
. pri_dl_down_timeout() is designed to be scheduled with T309 delay for active calls when L2 goes down.
. pri_internal_clear() is designed to be called by previous functions to clear a call internally (i.e. while nobody hung up). It mostly is a copy/paste of the code that is executed when libpri receives a Q931_RELEASE_COMPLETE.

Additionally, q931.c patch includes :
- a macro UPDATE_OURCALLSTATE() that traces call state changes when PRI_DEBUG_Q931_STATE flag is set.
- the fix for call state that must be ACTIVE when Asterisk in Network mode sends CONNECT (same as bug ASTERISK-7081 for branch/1.2).

At the moment :
- pri_timer.h is not changed. So, T309 will be active only if it is declared in zapata.conf.
- new functions in q931.c probably include a little too many debug traces. Some should probably removed if the patch is accepted.
- the patch sends STATUS messages for calls in establishment phase when it receives an L2 UP indication. This is not is Q.931 or ETSI, but it protects from misunderstandings on both sides of the link.
- the patch has been tested in user mode and network mode.
Comments:By: Brian Degenhardt (bmdhacks) 2006-06-02 11:34:22

you should probably wrap UPDATE_OURCALLSTATE in a do { } while 0;
Otherwise, somebody's gonna come along and do something like this:

if(pri->localtype == PRI_CPE)
  UPDATE_OURCALLSTATE(pri, c, Q931_CALL_STATE_CONNECT_REQUEST);
else
  UPDATE_OURCALLSTATE(pri, c, Q931_CALL_STATE_ACTIVE);
By: Frederic LE FOLL (flefoll) 2006-06-02 12:09:11

bmdhacks : That's why I inserted a "Beware" comment beneath the macro definition. But you're right : people don't always read comments (me included), and your workaround with do {} while(0) may help prevent unlucky accolades removals.

Matt : for bug ASTERISK-7081 (go ACTIVE after CONNECT), I sent a q931.c patch for branches/1.2, but I see that you applied it to trunk too. So, I'm afraid that current q931.c T309 patch for trunk, which also includes the ACTIVE state fix, won't apply 100% correctly. Sorry I mixed these two things ...
By: Frederic LE FOLL (flefoll) 2006-06-05 03:30:52

I uploaded patch_q931.C_rev348.txt that is adapted to lastest trunk head q931.c rev 348.
Thus, in q931_connect() it replaces both "c->ourcallstate =" with "UPDATE_OURCALLSTATE(".
This new version of patch for q931.c also includes bmdhacks's suggestion to embed UPDATE_OURCALLSTATE instructions in a "do {} while(0)".
By: Matthew Fredrickson (mattf) 2006-06-12 11:21:44

I think I would like to see this in before we make the 1.4 release if at all possible.  Can you update the patch to the most recent version of trunk, since I put in a few other of your patches that may have been contained within this.  Thanks!
By: Matthew Fredrickson (mattf) 2006-06-12 11:25:00

Oh, and perhaps more important, have you tested this patch yet?
By: Frederic LE FOLL (flefoll) 2006-06-13 02:23:23

I uploaded refreshed patches for current trunk head, i.e. chan_zap.c rev 32848, q921.C rev 350 and q931.c rev 352.
Older patches patch_T309_chan_zap.c.txt, patch_T309_q921.c.txt, patch_T309_q931.c_rev348.txt now are obsolete, but I don't have the permission to delete them :-(.

I also included a small comment fix in chan_zap.c:setup_zap(), since it now returns 0 instead of -1 when ast_config_load() fails.

And sure it's important: yes, I've tested the patch. But it currently has been tested and used with a limited set of pbx'es versions in front of Asterisk, always in simple EuroISDN protocol on E1 trunks (no multi-trunk shared D-channel, no Q.SIG, no T1, ...).

I'd like to insist on one thing : as provided, the patch does not enable T309 by default. Thus, q931_dl_indication() does nothing, and chan_zap continues to "kill'em all" upon alarm events.
At the moment, you have to declare T309 in zapata.conf to enable it.

An additional patch for pri_timers.h will be necessary to enable T309 by default. ETSI 300 403 says T309 should be 6 to 12 seconds, according to the formula "(N200 + 1) × T200 + 2 seconds", that is, for libpri defaults : (3 + 1) x 1000 + 2000 = 6000 ms. In real life, it seems that some/many(?) "EuroISDN" pbx'es still use T309 old value (recommended by Q.931 or ETSI 300 102) 90 seconds, but having different values on opposite sides does not matter, because STATUS messages allow both sides to resynchronize their calls states when L2 comes back.
By: Serge Vecher (serge-v) 2006-06-13 08:44:10

old patches removed as requested
By: Frederic LE FOLL (flefoll) 2006-06-14 05:46:29

I propose an additional - optional - patch for zapata.conf.sample, in order to explain what ISDN timers are for. Especially T309 ...
By: Matthew Fredrickson (mattf) 2006-07-06 16:52:08

Put in trunk