[asterisk-bugs] [LibSS7 0013495]: [patch] isup timers + q.764 compatibility + new cli commands

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=13495 
====================================================================== 
Reported By:                adomjan
Assigned To:                mattf
====================================================================== 
Project:                    LibSS7
Issue ID:                   13495
Category:                   New/Feature
Reproducibility:            always
Severity:                   feature
Priority:                   normal
Status:                     assigned
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases): 1.6.0 
SVN Revision (number only!): 140434 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             2008-09-16 12:43 CDT
Last Modified:              2008-11-26 18:07 CST
====================================================================== 
Summary:                    [patch] isup timers + q.764 compatibility + new cli
commands
Description: 
This path introduce isup timers, + handle the abnormal situation as the
q.764 require.

not complete, isup_gra and the others have to move to call oriantated
model.

enable isup timers, put those lines after the sigchan lines(values just
example for testing):

isup_timer.t1 = 3000
isup_timer.t5 = 7000
isup_timer.t7 = 20000

isup_timer.t12 = 5000 
isup_timer.t13 = 17000
isup_timer.t14 = 5000 
isup_timer.t15 = 17000

isup_timer.t16 = 5000
isup_timer.t17 = 17000

isup_timer.t18 = 5000
isup_timer.t19 = 17000

isup_timer.t20 = 5000
isup_timer.t21 = 17000

isup_timer.t22 = 5000
isup_timer.t23 = 17000

======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
related to          0012036 [patch] RFC 3372 SIP-T receive implemen...
====================================================================== 

---------------------------------------------------------------------- 
 (0095561) mattf (administrator) - 2008-11-26 18:07
 http://bugs.digium.com/view.php?id=13495#c95561 
---------------------------------------------------------------------- 
Aside from some code style mismatches that are not architecturally
important (I fixed most of those already), here is what I have found so far
as I've been going through this patch:

chan_dahdi.c:

In ss7_linkset() for case ISUP_EVENT_COT:

                                        if (e->cot.got_sent_msg &
ISUP_GOT_IAM) {
                                                if (e->cot.passed |
p->called_complete) 
                                                        ss7_start_call(p,
linkset);
                                        } else
                                                isup_clear_callflags(ss7,
p->ss7call, ISUP_GOT_IAM); /* we will got a new IAM if cot failed */

Do we need to actually clear the callflags here, since they don't appear
to be set anyways...?

Your original code was:

+                                       /* Don't start call if we didn't
get IAM or COT failed! */
+                                       if (e->cot.got_sent_msg &
ISUP_GOT_IAM) {
+                                               if (e->cot.passed |
p->called_complete)
+                                                              
ss7_start_call(p, linkset);
+                                               } else
+                                                      
isup_clear_callflags(ss7, p->ss7call, ISUP_GOT_IAM); /* we will got a new
IAM if cot failed */

In my working version to be committed, I formatted it so that the code
looks more like it behaves.  Perhaps you didn't put a bracket in the right
place and this is a bug....

Also, is it even necessary to call isup_clear_callflags in chan_dahdi.c? 
Couldn't we just put this in the event handler code in isup.c?

We can probably remove the lock parameter from dahdi_ss7_call_null since
we are using recursive mutexes (which allow locks to be called more than
once) to simplify the interface.

isup.c:
This looks a lot like a bug:

isup_clear_callflags - this function checks for if (flags | ISUP_XXXX....)
which will always be true...

Should this be "if (flags & ISUP_SENT_XXX)" instead?



I am going to keep going through the patch, as there is a lot to think
about.  I'll post more comments as I get further.

Thanks. 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2008-11-26 18:07 mattf          Note Added: 0095561                          
======================================================================