Re: Testers wanted: reentrant resolver

Valentin Nechayev <netch_at_iv.nn.kiev.ua> wrote:
>  Fri, Feb 20, 2004 at 18:46:36, green (Brian Fundakowski Feldman) wrote about "Testers wanted: reentrant resolver": 
> 
> BFF> New and improved, my reentrant resolver patch even includes documentation!
> BFF> What does it do, you ask?  Why, it simply makes tabbed browsing in Mozilla 
> BFF> an order of magnitude faster!  How does it do that, you ask?  Well, it 
> BFF> simply makes h_errno and the getaddrinfo(3)/getipnodebyname(3) functions 
> BFF> thread-safe in a more intelligent way.  There is still a global lock around 
> 
> The main question: why do you do it this way instead of import of newer
> ISC resolver which already has thread-safe resolver without any requirement
> of TLS? ISC resolver moreover has new options (attempts, timeouts) which
> are useful for some networks, and in any case current policy says
> that vendor branch features are preferrable than local hacks.

It's simple; the software API expected of FreeBSD right now is a thread-safe 
getaddrinfo(), and no gethostbyname_r().  I don't feel that it's a good idea 
to break that unless something revolutionary is going to happen to replace 
them all.  Keeping support of the old API is very desirable, and the safest 
thing, unless the new suite is 100% safe.  Giant changes to the resolver, 
even switching it to a new official version, should be taken with fair 
caution!

> For example of such import, tested working in real environment:
> ftp://segfault.kiev.ua/pub/freebsd/newresolv/
> Well, this is simple import (so I think there is no real reason to read
> files on the URL), and this was without testing on mozilla. But mozilla
> isn't the
> only application which uses resolver in MT mode, and carefully configured
> application will use new ISC interface having seen res_ninit(). This also
> should be true for Mozilla: GNU libc has new ISC style, not internal-MT
> style resolver.

It needs to have a lot of testing by a lot of people.

> Adding multithreading support internally to the current interface
> will burn the path to return to more portable approach: the more locally
> patched code is, the harder is to synchronize it with vendor branch,
> and too much code begins to be bound to local bug-o-features.
> For now it requires much work to port KAME-related options from FreeBSD
> local version. With MTization, it can become impossible.
> I think it should _not_ be quietly tested-and-committed, but requires
> explicit approval from core_at_ or arch_at_.

It will not burn the path.  At most, a reference to the old names for the
(*accessor()) will be needed, and that's an extra line of "code" to make the 
old name continue to work.  This is far from risky compared to the 
alternatives.

> And, after all, you seems to be ignorable for bin/29581:
> 
> ==={{{
> proposed gethostbyXXXX_r() implementation
> Responsible
>     nectar_at_FreeBSD.org
> Arrival-Date
>     Thu Aug 09 13:10:19 PDT 2001 
> Last-Modified
>     Sat Feb 22 09:51:52 PST 2003 
> Originator
>     Alexander Litvin <archer_at_whichever.org>
> [...]
> Description
> 
> 	Below is a patch, which adds implementation of gethostbyname_r()
> 	and gethostbyaddr_r() reentrant functions to FreeBSD libc.
> 	Notes:
> 	 * Patch essentially eliminates usage of static buffers
> 		 when gethostbyXXXX_r are called.
> 	 * Also, global _res and h_errno are replaced by
> 		 function calls (similar to errno) and for threaded
> 		 applications are stored in thread-specific data
> 	 * Although all static variables relevant to gethostbyXXXX()
> 		 and resolver library were eliminated, to clame that
> 		 this implementation is thread-safe it is necessary
> 		 to check all possible code paths.
> 	 * Patch implements reentrant functions only for dns/files
> 		 configuration. In case of nis implementation is not
> 		 reentrant. This can be easily done (it looks easier
> 		 then to modify dns/files code) , but I chose to omit
> 		 this, because I have no environment to test it.
> ===}}}
> 
> I don't like this approach, but it exists and is tested to be working,
> and is only little more cumbersome than yours.

It won't apply probably 80% of it to -CURRENT.  It has been superceded, 
since the ideas have been reused, and it would just be pointless 
gratuitously throwing away the work to use the older copy of the work that 
would just need to be mostly replaced.

> BFF> some pieces of data, but for the most part the global lock (which is what 
> BFF> serialized EVERY hostname lookup in threaded programs) is not contested.  
> BFF> It's still used for some NIS-related tasks, looking up hostents, and looking 
> BFF> up server port names, but that's not comparatively very important.  The 
> BFF> resolver itself is a thread-safe backend where each thread gets its own 
> BFF> resolver that can run concurrently with the rest, so the time spent blocked 
> BFF> waiting for DNS responses is gone.
> 
> Do you say that problem with MT and NSS external lookups is solved?

Yes, read the documentation changes.  All accesses to those will be 
guaranteed not to collide, because they're all protected with a mutex around 
all API calls out that are not thread-safe.

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green_at_FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\