Hi Michel,

only callid matching is not enough - you should also check the tags. In the mean while, please upload the patch on the tracker. As I said in the previous email, my only concern is about performance, as you will have to check all requests and not the only marked for dialog tracking.

regards,
bogdan

Michel Bensoussan wrote:

Hi
If I understand your solution, you don't allow the session. And what if ($dlg_count != 0) ?

I tried this. It seems to work but I just quick check it.
The key in "Callid" but I'm not sure it is a good choice. Maybe "To" is better.

Add the following function in dlg_hash.c:
(based on release 1.1.1)

struct dlg_cell* lookup_dlg_callid(str* callid)
{
 struct dlg_cell *dlg;
 struct dlg_entry *d_entry;
 int i;

 if (d_table==0)
   goto not_found;

 for( i=0 ; i<d_table->size; i++ ) {

   d_entry = &(d_table->entries[i]);

   dlg_lock( d_table, d_entry);

   for( dlg=d_entry->first ; dlg ; dlg=dlg->next ) {
     if (!memcmp(dlg->callid.s, callid->s, dlg->callid.len)) {
       if (dlg->state==DLG_STATE_DELETED) {
         dlg_unlock( d_table, d_entry);
         goto not_found;
       }
       dlg->ref++;
       dlg_unlock( d_table, d_entry);
       DBG("DEBUG:dialog:lookup_dlg_callid: dialog callid='%.*s' found on entry %u.%u\n",
           callid->len, ZSW(callid->s), dlg->h_id, dlg->h_entry);
       return dlg;
     }
   }

   dlg_unlock( d_table, d_entry);
 }

not_found:
 DBG("DEBUG:dialog:lookup_dlg: no dialog callid='%.*s' found\n",
     callid->len, ZSW(callid->s));
 return 0;
}


In dlg_handlers.c, modify dlg_onroute() as following:

void dlg_onroute(struct sip_msg* req, str *route_params, void *param)
{
   struct dlg_cell *dlg = 0;
   str val;
   str callid;
   int h_entry;
   int h_id;

/* If find rm_param do  as  previously */
if (d_rrb.get_route_param( req, &rr_param, &val) == 0) {
   DBG("DEBUG:dialog:dlg_onroute: route param is '%.*s' (len=%d)\n",
     val.len, val.s, val.len);

   if ( parse_dlg_rr_param( val.s, val.s+val.len, &h_entry, &h_id)<0 )
     return;

   dlg = lookup_dlg( h_entry, h_id);
   if (dlg==0) {
     LOG(L_WARN,"WARNING:dialog:dlg_onroute: unable to find dialog\n");
     return;
   }
 }else if (req->callid){
/* If rm_param not found, look for callid */
   dlg = lookup_dlg_callid(&req->callid->body);
   if (!dlg){
     DBG("DEBUG:dialog:dlg_onroute: Callid '%.*s' not found\n",
       req->callid->body.len, req->callid->body.s);
     return;
   }
 }

   if (use_tight_match) {
       if ((!req->callid && parse_headers(req,HDR_CALLID_F,0)<0) ||
       !req->callid) {
           LOG(L_ERR, "ERROR:dialog:dlg_onroute: bad request or "
               "missing CALLID hdr :-/\n");
           return;
       }
       callid = req->callid->body;
       trim(&callid);
       if (dlg->callid.len!=callid.len ||
       strncmp(dlg->callid.s,callid.s,callid.len)!=0) {
           LOG(L_WARN,"WARNING:dialog:dlg_onroute: tight matching failed\n");
           return;
       }
   }

   if (req->first_line.u.request.method_value==METHOD_BYE) {
       if (remove_dlg_timer(&dlg->tl)!=0) {
           unref_dlg( dlg , 1, 0);
           return;
       }
       if (dlg->state!=DLG_STATE_CONFIRMED)
           LOG(L_WARN, "WARNING:dialog:dlg_onroute: BYE for "
               "unconfirmed dialog ?!\n");

       /* dialog terminated (BYE) */
       run_dlg_callbacks( DLGCB_TERMINATED, dlg, req);

       unref_dlg(dlg, 2, 1);
       if_update_stat( dlg_enable_stats, active_dlgs, -1);
       return;
   } else {
       /* within dialog request */
       run_dlg_callbacks( DLGCB_REQ_WITHIN, dlg, req);
   }

   if (req->first_line.u.request.method_value!=METHOD_ACK) {
       dlg->lifetime = get_dlg_timeout(req);
       update_dlg_timer( &dlg->tl, dlg->lifetime );
   }

   unref_dlg( dlg , 1, 0);
   return;
}




Andy Pyles wrote:

HI Michel,

Well here's what I'm doing:

if ( loose_route() {
    if($DLG_status == 0 && method!="BYE" ){ *** assuming latest patch accepted
         xlog("L_INFO", "broken ua, dropping packet\n");
         exit();
  }
}

This isn't the most elegant solution, but in my case works ok.
Basically Lets say the UAC is broken. The first opportunity to tell it
is "broken" is when it sends an ACK to the 200 OK.
At this point, it would be very nice to have an option to send "BYE",
but as this functionality isn't there yet, we just reject the ACK, and
eventually the broken UAC will send a bye.

There may be other corner cases as well, but you get the general idea.

Andy


On 2/26/07, Michel Bensoussan <michel@extricom.com> wrote:

Hi

"I think the best approach is to force the UAC vendors to align to the
SIP specifications."

Well..... Good luck.
I don't know why but I think if I call and tell them "get back all you
products and upgrade them to align to the SIP specification" they won't
listen to me. But may be I'm wrong.

Any way. I have to deal with it. Do you have some suggestions? What is
the best dialog module version to start with?

I'm not familiar with SIP but I understand that the "To" header won't
change during a session. Is that right? I'm not sure the "CallID" will
be them same in case of re-INVITE.
So I can save the "To" header in the dialog table and check it if the
rm_param is not found.
I'm not sure how to do this.

Andy, If you have some suggestions too...

Regards,
Michel.

Bogdan-Andrei Iancu wrote:
> Hi Michael,
>
> as you give you the same answer as to Andy :) :
>
> "To be honest I'm not fan of complicating my life just to support
> broken stuff :)....doing dialog matching in traditional way (without
> using RR stuff) is very costly and since complete, un-altered RR
> mirroring is mandatory by RFC3261, I see no point of doing it
> different. "
>
> I think the best approach is to force the UAC vendors to align to the
> SIP specifications.
> regards,
> bogdan
>
>
> Michel Bensoussan wrote:
>> Hi Bogdan
>> I'm agree with you, but we cannot control the UAS devices so we have
>> to handle the case it doesn't correctly mirror the RR header. Can we
>> base the dialog states on From and To headers? or Callid? I
>> understand the the rr_param is used for fast dialog matching (dialog
>> README). Checking dialog matching with headers (From, To, ...),
>> will consequently slowing the transaction?
>>
>> Regards,
>> Michel.
>>
>> Bogdan-Andrei Iancu wrote:
>>> Hi Michel,
>>>
>>> looking at the net capture, it seams that the UAS device
>>> (User-Agent: WLAN660-S VoIP PHONE) does not correctly mirror the RR
>>> header - it is removing the hdr parameters, mirroring only the URI,
>>> which is bogus.
>>>
>>> regards,
>>> bogdan
>
>
>