Hello,

hope to get to it later today/tomorrow. Being busy after the return from Astricon.

Cheers, Daniel

On 10/16/13 2:42 PM, Øyvind Kolbu wrote:

On 08.10.2013 03:15, Øyvind Kolbu wrote:

...

Ref the thread from september, http://lists.sip-router.org/pipermail/sr-users/2013-September/079760.html

I've fixed SRV lookups to work when use_dns_failover is on and the domain has no _sip._udp record. An example is cisco.com:

% host -t SRV _sip._udp.cisco.com Host _sip._udp.cisco.com not found: 3(NXDOMAIN) % host -t SRV _sip._tcp.cisco.com _sip._tcp.cisco.com has SRV record 1 0 5060 vcsgw.cisco.com. % host -t SRV _sips._tcp.cisco.com _sips._tcp.cisco.com has SRV record 1 0 5061 vcsgw.cisco.com.

Currently attempting to call an uri at cisco.com will fail, unless manually forcing the protocol.

While fixing the the issue I refactored a lot of code and fixed a plenty of comments as the lookup code has clearly been copy pasted more than once.

Patch is attached and should probably be reviewed thoroughly as it touches pretty core usage.

Any feedback?

Hello,

On 10/16/13 2:54 PM, Øyvind Kolbu wrote:

On 16.10.2013 14:45, Daniel-Constantin Mierla wrote:

...

Hello,

hope to get to it later today/tomorrow. Being busy after the return from Astricon.

looking at the first patch, I see that PROTO_NONE was removed from those switch statements on proto variables. I guess it was the reason of the issue, as you pointed it out, that a non-existing udp srv records results in failure. Isn't it?

To be sure I got properly the new logic, if proto var is PROTO_NONE, then create_srv_pref_list(..) will build a list of protocols to try based on config preferences. The it will be a dns lookup of the values in the list. Right?

Thanks, Daniel

I applied the two patches related to this matter. I only renamed the structure you defined, stripping '_t' from its name as this suffix is commonly used across the code as a typedef'ed struct name.

Cheers, Daniel

On 10/18/13 10:01 AM, Øyvind Kolbu wrote:

On 17.10.2013 19:09, Daniel-Constantin Mierla wrote:

...

looking at the first patch, I see that PROTO_NONE was removed from those switch statements on proto variables. I guess it was the reason of the issue, as you pointed it out, that a non-existing udp srv records results in failure. Isn't it?

No, the reason was that it can never be true, as just before *proto is set to PROTO_UDP if not set.

    if (proto){ /* makes sure we have a protocol set*/
            if (*proto==0)
                    *proto=srv_proto=PROTO_UDP; /* default */
            else
                    srv_proto=*proto;
    }else{
            srv_proto=PROTO_UDP;
    }

The problem with the old code was that, unless you specified a protocol, it would only attempt to search up _sip._udp and not continue with _sip._tcp and possible other protocols.

...

To be sure I got properly the new logic, if proto var is PROTO_NONE, then create_srv_pref_list(..) will build a list of protocols to try based on config preferences. The it will be a dns lookup of the values in the list. Right?

Yes, if the original *proto is PROTO_NONE, then call create_srv_pref_list and then stop on the first match in the generated list.