[asterisk-bugs] [Zaptel 0008290]: [patch] zap hookstate is never set offhook if wctdm module is loaded with an active fxo line

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=8290 
====================================================================== 
Reported By:                tmarkov
Assigned To:                mattf
====================================================================== 
Project:                    Zaptel
Issue ID:                   8290
Category:                   wctdm
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
Zaptel Version:              1.2.10 
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Disclaimer on File?:        Yes 
Request Review:              
====================================================================== 
Date Submitted:             11-03-2006 17:28 CST
Last Modified:              12-04-2007 03:41 CST
====================================================================== 
Summary:                    [patch] zap hookstate is never set offhook if wctdm
module is loaded with an active fxo line
Description: 
If #define ZAP_CHECK_HOOKSTATE is uncommented in chan_zap.c, then the
hookstate stays onhook if the wctdm module is loaded with an active line. 
Removing line and reinserting will cause the hookstate to initialize
properly.  The problem appears to be that wc->mod[card].fxo.battery never
gets initialized to 0, so in wctdm_voicedaa_check_hook() the test "if
(!wc->mod[card].fxo.battery && !wc->mod[card].fxo.battdebounce)" is never
entered, and the hookstate remains onhook upon module startup.  Only a
cycling of line voltage will correct the problem.  I have attached a patch
which initializes the battery variable and fixes the problem for me.
====================================================================== 

---------------------------------------------------------------------- 
 meneault - 12-04-07 03:41  
---------------------------------------------------------------------- 
Hi I am back again,

I just tested wctdm.c with wctdm-race-1.4-rev3211 and
wctdm-hooksig2-1.4-rev3121 patches. This time I checked that zt_hooksig is
called regularly when a line is present.
I only checked the case where battery is present on the line because my
server is not physically accessible for the moment thus I cannot unplug the
line to reverse the test but it should work as well.

I will let this version running on my box so as to be sure there is not a
performance impact, IMHO this is the only issue that could raise the
patch.
This is why I limited calling zt_hooksig every battdebounce ms and not for
every call to voice_daa_check_hook.

Now I remember why I did produce two patches only:

- wctdm-race-1.4-rev3211 was to correct a coding bug which would revert
the initialization of fxo.battery this is an initialization race. And this
must be applied in any case because this is a real bug.
If more details are needed:
When wctdm_init_proslic is called the driver does not know if card is FXO
or FXS but what it does is to set wc->mod[card].fxs.idletxhookstate before
checking if it is a FXS or FXO card. Then If the card appears to be FXO one
then this value won't be reinitialized to 0. When you look at the
corresponding position in the union for a FXO card you can stop at
fxo.battery (if AUDIO_RINGCHECK is not enabled) and at fxo.offhook (if
AUDIO_RINGCHECK is enabled). What it means is that when initializing a FXO
card you get wc->mod[card].fxo.battery set to either 5 or 1 in
wctdm_init_proslic, the apparent result is that the driver thinks that the
battery is already set and won't print BATTERY message the first time
voice_daa_check_hook is called (and zt_hooksig is neither called).
I didn't check what is the result of this bug in case of AUDIO_RINGCHECK
being enabled but I think it could provoke the card to go off hook at
driver load time (which is still worse).

- wctdm-hooksig2-rev3121 is the remaining part of the patch, this one
contrary to the first one is only needed if maintainer of wctdm wants to
correct the bug 8290. I didn't made two patches (one for the messages and
one for zt_hooksig) because they share the same lines of code.
As I said this patch may have performance impact every battdebounce ms but
should be low (cost is a channel's lock locking every battdebounce ms when
calling zt_hooksig). 
If performance impact appears to big when testing we could change the
locking consideration for rxhooksig (either creating a separate lock for
rxhooksig or preventing rxhooksig being modified from user-space context
and thus accessing it without locking). The second solution is clearly more
interesting but at the time being rxhooksig maybe modified from user-space
context in zaptel-base (this is the reason of the bug). Having a separate
lock while accessing rxhooksig could alleviate the load because that way
rxhooksig access wouldn't collide with channel's read method.
Accessing rxhooksig as in flefoll patch (without locking) is not safe but
as rxhooksig is not changed often the probability to cause troubles is very
low and thus could be accepted. I still prefer locking though, for sake of
sanity.

Waiting for feedbacks now.

(llriera I will upload new version for TDM2400 as soon as feedbacks are ok
for TDM400). 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
12-04-07 03:41  meneault       Note Added: 0074730                          
======================================================================