[asterisk-ss7] chan_ss7 1.4.3 bug with overlapping CIC numbers on combined linksets

[Jasper van der Neut - Stulen]

Gregory Massel wrote:
> Hi Robert
> 
> I think the write buffer notice you're getting is normal audio loss due to the jitter buffer and
> jitter associated with the SIP channel.
> 
> It seems that remove_from_idlelist() in l4isup.c may be the issue:
 >
 > static void remove_from_idlelist(struct ss7_chan *pvt) {
 >   struct linkset* linkset = pvt->link->linkset;
 >   struct ss7_chan *prev, *cur;
 >
 >
 >   cur = linkset->group_linkset->idle_list;
 >   prev = NULL;
 >   while(cur != NULL) {
 >     if(pvt->cic == cur->cic) {
 >       if(prev == NULL) {
 >         linkset->group_linkset->idle_list = pvt->next_idle;
 >       } else {
 >         prev->next_idle = pvt->next_idle;
 >       }
 >       pvt->next_idle = NULL;
 >       return;
 >     }
 >     prev = cur;
 >     cur = cur->next_idle;
 >   }
 >   ast_log(LOG_NOTICE, "Trying to remove CIC=%d from idle list, but not found?!?.\n", pvt->cic);
 > }
 >
> 
> What concerns me is the following line:
> 
> if(pvt->cic == cur->cic) {
> 
> I get the impression that it should read something like:
> 
> if(pvt->cic == cur->cic && linkset->dpc == cur->linkset->dpc) {
> 
> In my mind, this should, in theory, ensure that the CIC is matched not just on the CIC number but
> also on the DPC.

Although the above seems buggy indeed, this only is an use when you use a 'group' statement in your
configuration, which you don't according to your first email.

I think the problem is caused by the various cic_hunt functions, for example:

/* This implements the policy: Sequential low to high CICs */
static struct ss7_chan *cic_hunt_seq_lth_htl(struct linkset* linkset, int lth, int first_cic, int 
last_cic)
{
   struct ss7_chan *cur, *prev, *best = NULL, *best_prev = NULL;

   for(cur = linkset->group_linkset->idle_list, prev = NULL; cur != NULL; prev = cur, cur = 
cur->next_idle) {
     /* Don't select lines that are resetting or blocked. */
     if(!cur->reset_done || (cur->blocked & (BL_LH|BL_RM|BL_RH|BL_UNEQUIPPED|BL_LINKDOWN|BL_NOUSE))) {
       continue;
     }
     /* is this cic within the selected range? */
     if(cur->cic < first_cic || cur->cic > last_cic) {
       continue;
                                   }
     if (!best) {
       best = cur;
       continue;
     }
     if (lth) {
       if (cur->cic < best->cic) {
         best = cur;
         best_prev = prev;
       }
     }
     else {
       if (cur->cic > best->cic) {
         best = cur;
         best_prev = prev;
       }
     }
   }

   if(best != NULL) {
     if(best_prev == NULL) {
       linkset->group_linkset->idle_list = best->next_idle;
     } else {
       best_prev->next_idle = best->next_idle;
     }
     best->next_idle = NULL;
     return best;
   } else {
     ast_log(LOG_WARNING, "No idle circuit found.\n");
     return NULL;
   }
}

If "best" is the n-th entry and the first - (n-1)th entry are either blocked for
some reason or outside the range we want, "best_prev" will still be NULL when
we remove "best" from the idle_list, which all cause all entries before "best"
to also be removed from the idle_list.
Using this will fix that:

   if(best != NULL) {
     remove_from_idlelist(best);
     return best;
   } else {
     ast_log(LOG_WARNING, "No idle circuit found.\n");
     return NULL;
   }


-- 
Regards

Jasper van der Neut
SpeakUp BV
T: 088-SPEAKUP (088-7732587)
F: 088-7732588