[vlc-devel] Patch: accelerate net_Connect
Leonid Rosenboim, CERTARETE
Leonid at certarete.com
Mon Aug 29 23:49:26 CEST 2011
Many thanks for taking a deep look into the proposed change, and sending us your detailed feedback.
Regarding your comment with respect to "getaddrinfo()" usage, the on-line POSIX reference I found states this:
"If the query is successful, a pointer to a linked list of one or more addrinfo structures is returned via the fourth argument."
This basically means to me that all DNS records matching the requested domain name will be returned, and in case this normative definition is not enough, let me assure you that I have tested the "smart connect" code under several operating systems including MacOS, Ubuntu and Windows XP, and all of them with no exception followed the normative requirement and returned all matching address records in the returned linked list. I did not however have an opportunity to test it with a DNS name that has a mix of "A" and "AAA" records for the same domain (server) name.
I have reviewed the "Happy Eyeballs" IETF draft you referenced with great interest, and I think it is a rather impressive body of work. However, I do not agree with paragraph 4.1 and its justification. The authors assume that the user of the computer is sufficiently competent to configure the address policy, and thus puts precedence on reducing load on some network elements (i.e. machines). In my experience the human will invariably be the weakest link, and cause for misconfiguration that will result in malfunction. To always put preference on IPv6 does not seem to me like the proper solution either. One should anticipate that in a dual-stack scenario one of the two stacks may not be properly configured and there will be connectivity issues. Hence statically selecting one protocol version over the other in failures and usability issues requiring competent manual intervention. It seems the authors of the "Happy Eyeballs" have not considered an option of automatically selecting the best of the two protocol versions in a fully dynamic manner, which is at the core of our proposition.
If however you remain unconvinced, an option to filter the result returned by "getaddrinfo()" based on protocol family can be easily added to comply with static protocol precedence, which will reduce the effectiveness of "smart connect" to a degree, but not entirely, as it will still be able to dynamically select the closest server in a given protocol family.
The issue if effectively directing client traffic to a server that is closest to it in terms of network topology has been around for many years, but mostly it was addressed from the server side, e.g. by adjusting the order of address records returned by DNS (assuming that clients will attempt in-order connection), but these have many shortcomings and can never be as precise or as optimal as this client-centric approach.
May I also add that a similar concurrent-connect approach has been successfully deployed in several IPTV networks to date, and it is even more potent for OTT scenarios where the network topology is much more diverse in distance and performance.
Thank you for your consideration,
- Leonid Rosenboim
On Aug 27, 2011, at 1:58 AM, Rémi Denis-Courmont wrote:
> Le samedi 27 août 2011 03:36:44 Kiran Kotla, vous avez écrit :
>> Here is a patch that makes the net_Connect function connect
>> to the fastest server attempting to connect to multiple servers from
>> DNS resolution in parallel by making the sockets non-blocking and
>> using select to choose the best among them. We call this new
>> mechanism 'smart connect'. It also stores the fastest server from the
>> previous resolution of a DNS name into a cache (with a configurable
>> timeout) and connects to it directly without performing DNS resolution.
> Is there any OS that returns more than one addrinfo structure per family per
> protocol? As far as I know, if the DNS query returned multiple addresses in
> the same familty, getaddrinfo() picks one pseudo-randomly. This being the
> case, this patch will basically change the logic to determine which of IPv4 or
> IPv6 is used, right?
> That is covered by http://tools.ietf.org/html/draft-ietf-v6ops-happy-eyeballs
> and I suspect your proposed implementation does not adhere to the address
> policy, as required by §4.1. Of course, this is only a draft, but I doubt this
> requirement is ever going to be dropped. Probably the simplest solution is the
> short timeout reportedly implemented by Chrome and Firefox (§6.0).
> I am also not sure that caching DNS lookups in VLC is a good idea. VLC does
> not know the lifetime of the DNS responses, so it cannot perform correct cache
> expiration. DNS cache really ought to be maintained by the DNS resolver, and
> indeed some operating systems do have a caching resolver (e.g. nscd on Linux).
>> The patch file is attached. We would appreciate any comments or
>> suggestions on the changes described.
> * net_Connect() normally returns sockets in non-blocking mode.
> * select() is not used due to design bugs (FD_SET() does not check for buffer
> overflow). poll() is used instead.
> * net_Connect() is thread-safe. It cannot use unprotected non-constant global
> * strerror() is forbidden in VLC code base due to not being thread-safe.
> * memset() is preferred over bzero().
> * Identifiers with leading underscores are deprecated due for conformance with
> the C standard.
> Rémi Denis-Courmont
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel