[Linux] Enable connecting to localhost when offline.

[Linux] Enable connecting to localhost when offline.

Add a utility function to determine if only loopback address are configured.
Add a mechanism to add supplemental HostResolverFlags in the HostResolver.
Change the resolver on Linux to not use AI_ADDRCONFIG if only loopback addresses are configured.

BUG=41408
TEST=localhost works when offline

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53487

[eroman, 2010-07-22 21:26:03]

@wtc: I wander if we should just remove AI_ADDRCONFIG on linux the same as we do
on windows?

http://codereview.chromium.org/3036011/diff/3001/4001
File net/base/address_family.h (right):

http://codereview.chromium.org/3036011/diff/3001/4001#newcode25
net/base/address_family.h:25: HOST_RESOLVER_LOOPBACK_ONLY = 1 << 15,
What is the significance of 15?

http://codereview.chromium.org/3036011/diff/3001/4002
File net/base/host_resolver_impl.cc (right):

http://codereview.chromium.org/3036011/diff/3001/4002#newcode1060
net/base/host_resolver_impl.cc:1060: additional_resolver_flags_ |=
HOST_RESOLVER_LOOPBACK_ONLY;
This test should also be done for first time initialization (OnIPAddressChanged
is not necessarily called before the first request is issued).

http://codereview.chromium.org/3036011/diff/3001/4005
File net/base/net_util.cc (right):

http://codereview.chromium.org/3036011/diff/3001/4005#newcode1801
net/base/net_util.cc:1801: struct sockaddr* addr = interface->ifa_addr;
nit: const?

[vandebo (ex-Chrome), 2010-07-22 21:43:51]

On 2010/07/22 21:26:03, eroman wrote:
> @wtc: I wander if we should just remove AI_ADDRCONFIG on linux the same as we
do
> on windows?

As I understand it, windows has a different behavior - it always does what you'd
*like* AI_ADDRCONFIG to do.

http://codereview.chromium.org/3036011/diff/3001/4001
File net/base/address_family.h (right):

http://codereview.chromium.org/3036011/diff/3001/4001#newcode25
net/base/address_family.h:25: HOST_RESOLVER_LOOPBACK_ONLY = 1 << 15,
On 2010/07/22 21:26:03, eroman wrote:
> What is the significance of 15?

largest AI_foo defined on linux is AI_NUMERICSERV 0x0400, so the enum will be 16
bits. 1 << 15 is the highest bit.

http://codereview.chromium.org/3036011/diff/3001/4002
File net/base/host_resolver_impl.cc (right):

http://codereview.chromium.org/3036011/diff/3001/4002#newcode1060
net/base/host_resolver_impl.cc:1060: additional_resolver_flags_ |=
HOST_RESOLVER_LOOPBACK_ONLY;
On 2010/07/22 21:26:03, eroman wrote:
> This test should also be done for first time initialization
(OnIPAddressChanged
> is not necessarily called before the first request is issued).

Done.

http://codereview.chromium.org/3036011/diff/3001/4005
File net/base/net_util.cc (right):

http://codereview.chromium.org/3036011/diff/3001/4005#newcode1801
net/base/net_util.cc:1801: struct sockaddr* addr = interface->ifa_addr;
On 2010/07/22 21:26:03, eroman wrote:
> nit: const?

Done.

[eroman, 2010-07-22 22:17:11]

http://codereview.chromium.org/3036011/diff/3001/4001
File net/base/address_family.h (right):

http://codereview.chromium.org/3036011/diff/3001/4001#newcode25
net/base/address_family.h:25: HOST_RESOLVER_LOOPBACK_ONLY = 1 << 15,
On 2010/07/22 21:43:51, vandebo wrote:
> On 2010/07/22 21:26:03, eroman wrote:
> > What is the significance of 15?
> 
> largest AI_foo defined on linux is AI_NUMERICSERV 0x0400, so the enum will be
16
> bits. 1 << 15 is the highest bit.

Since in host_resolver_proc.cc you are testing for HOST_RESOLVER_LOOPBACK_ONLY
and substituting the AI_ADDRCONFIG, this can safely be 1 << 1.

(This is consistent with the HOST_RESOLVER_CANONNAME flag which is also not tied
to the system integer value).

As far as allowing raw AI_* flags to be put in here in the future, I don't think
that is a good idea (since for starters flags and their values can differ
between systems, but it also defeats the purpose of having an application level
wrapper around it if consumers don't know the possible values that can go into
it.

[vandebo (ex-Chrome), 2010-07-22 22:25:31]

http://codereview.chromium.org/3036011/diff/3001/4001
File net/base/address_family.h (right):

http://codereview.chromium.org/3036011/diff/3001/4001#newcode25
net/base/address_family.h:25: HOST_RESOLVER_LOOPBACK_ONLY = 1 << 15,
On 2010/07/22 22:17:11, eroman wrote:
> On 2010/07/22 21:43:51, vandebo wrote:
> > On 2010/07/22 21:26:03, eroman wrote:
> > > What is the significance of 15?
> > 
> > largest AI_foo defined on linux is AI_NUMERICSERV 0x0400, so the enum will
be
> 16
> > bits. 1 << 15 is the highest bit.
> 
> Since in host_resolver_proc.cc you are testing for HOST_RESOLVER_LOOPBACK_ONLY
> and substituting the AI_ADDRCONFIG, this can safely be 1 << 1.
> 
> (This is consistent with the HOST_RESOLVER_CANONNAME flag which is also not
tied
> to the system integer value).
> 
> As far as allowing raw AI_* flags to be put in here in the future, I don't
think
> that is a good idea (since for starters flags and their values can differ
> between systems, but it also defeats the purpose of having an application
level
> wrapper around it if consumers don't know the possible values that can go into
> it.

Done.

[cbentzel, 2010-07-22 22:51:54]

I don't think this is a problem, but the additional HostResolverFlags bit means
that there will be different entries in the HostCache when only loopback is
present versus other interfaces being present.

http://codereview.chromium.org/3036011/diff/13001/14005
File net/base/net_util.cc (right):

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1787
net/base/net_util.cc:1787: #if defined(OS_POSIX)
All of the HostResolver uses of this were OS_LINUX - should those open up to
OS_POSIX, or does AI_ADDRCONFIG work correctly on OSX?

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1791
net/base/net_util.cc:1791: return false;
Could you log errno in the problematic case?

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1797
net/base/net_util.cc:1797: if (!(IFF_UP & interface->ifa_flags))
Nit: Is there a requirement that IFF_UP etc. be a single bit? May be safer to
make this (IFF_UP & interface->ifa_flags) == IFF_UP.

[cbentzel, 2010-07-22 22:53:00]

On 2010/07/22 22:51:54, cbentzel wrote:
> I don't think this is a problem, but the additional HostResolverFlags bit
means
> that there will be different entries in the HostCache when only loopback is
> present versus other interfaces being present.

Oh, and you may want to add additional checks in
HostCacheTest.HostResolverFlagsArePartOfKey just to make sure.

[vandebo (ex-Chrome), 2010-07-22 23:55:01]

On 2010/07/22 22:53:00, cbentzel wrote:
> On 2010/07/22 22:51:54, cbentzel wrote:
> > I don't think this is a problem, but the additional HostResolverFlags bit
> means
> > that there will be different entries in the HostCache when only loopback is
> > present versus other interfaces being present.
> 
> Oh, and you may want to add additional checks in
> HostCacheTest.HostResolverFlagsArePartOfKey just to make sure.

I updated this test to confirm that.

http://codereview.chromium.org/3036011/diff/13001/14005
File net/base/net_util.cc (right):

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1787
net/base/net_util.cc:1787: #if defined(OS_POSIX)
On 2010/07/22 22:51:55, cbentzel wrote:
> All of the HostResolver uses of this were OS_LINUX - should those open up to
> OS_POSIX, or does AI_ADDRCONFIG work correctly on OSX? 

AI_ADDRCONFIG works the way we want it to work on OSX.  I originally had this as
defined(OS_LINUX), but this function is correct for OSX, so I made it OS_POSIX.

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1791
net/base/net_util.cc:1791: return false;
On 2010/07/22 22:51:55, cbentzel wrote:
> Could you log errno in the problematic case?

Done.

http://codereview.chromium.org/3036011/diff/13001/14005#newcode1797
net/base/net_util.cc:1797: if (!(IFF_UP & interface->ifa_flags))
On 2010/07/22 22:51:55, cbentzel wrote:
> Nit: Is there a requirement that IFF_UP etc. be a single bit? May be safer to
> make this (IFF_UP & interface->ifa_flags) == IFF_UP.

Both Linux and OSX have IFF_* as single bit flags.  Especially with a name
including flags, I'm inclined to leave this alone.  Unless you feel strongly.

[wtc, 2010-07-22 23:56:22]

eroman: Ulrich Drepper (a main author of glibc) advised that we use
AI_ADDRCONFIG on Linux.  I now believe that I removed the
AI_ADDRCONFIG flag on Windows because I misunderstood what
the problem was.  I suspect that I actually ran into the same problem
as the Linux/Mac problem that vandebo is trying to fix now.
Unfortunately, I don't have time to look into that.

http://codereview.chromium.org/3036011/diff/13001/14004
File net/base/host_resolver_proc.cc (right):

http://codereview.chromium.org/3036011/diff/13001/14004#newcode223
net/base/host_resolver_proc.cc:223: hints.ai_flags &= ~AI_ADDRCONFIG;
vandebo: thank you for working on this issue.

Strictly speaking, if only the IPv4 loopback interface is configured,
we should filter out any IPv6 loopback address returned by
getaddrinfo.  And vice versa.  Since our network stack tries the
next address when we cannot connect to one address, things
will still work, just might be slower.

Another corner case is that the computer has only an IPv4
outgoing interface configured and /etc/hosts maps "localhost"
to the IPv6 loopback address ::1 only.  In this case,
getaddrinfo(AI_ADDRCONFIG) will fail for "localhost", but the
host resolver should return the IPv6 loopback address.
You should add a comment to note that your solution does not
address this corner case.

[vandebo (ex-Chrome), 2010-07-23 00:10:04]

On 2010/07/22 23:56:22, wtc wrote:
> eroman: Ulrich Drepper (a main author of glibc) advised that we use
> AI_ADDRCONFIG on Linux.  I now believe that I removed the
> AI_ADDRCONFIG flag on Windows because I misunderstood what
> the problem was.  I suspect that I actually ran into the same problem
> as the Linux/Mac problem that vandebo is trying to fix now.
> Unfortunately, I don't have time to look into that.

According to your comment in SystemHostResolverProc, Windows actually does
something smarter.  However your comment could be misleading.
 
> Strictly speaking, if only the IPv4 loopback interface is configured,
> we should filter out any IPv6 loopback address returned by
> getaddrinfo.  And vice versa.  Since our network stack tries the
> next address when we cannot connect to one address, things
> will still work, just might be slower.

If this happens, you can probably consider it a distro bug or a user
configuration bug.  Distros should add a "localhost ::1" entry to /etc/hosts at
the same time they enable IPv6.  If they've added the host entry without
enabling IPv6 then it's a distro's bug.  Similarly, if the user changes this
(adds a ::1 entry or disables IPv6), they should make the corresponding change
to enable IPv6 or remove the ::1 entry.
 
> Another corner case is that the computer has only an IPv4
> outgoing interface configured and /etc/hosts maps "localhost"
> to the IPv6 loopback address ::1 only.  In this case,
> getaddrinfo(AI_ADDRCONFIG) will fail for "localhost", but the
> host resolver should return the IPv6 loopback address.
> You should add a comment to note that your solution does not
> address this corner case.

This is being tracked as a different bug.  One that I am currently working on:
http://crbug.com/39830

[wtc, 2010-07-23 00:30:13]

On 2010/07/23 00:10:04, vandebo wrote:
>
> According to your comment in SystemHostResolverProc, Windows actually does
> something smarter.  However your comment could be misleading.

Right, I believe I wrote that comment with an incorrect understanding
of what Windows actually does.

[eroman, 2010-07-23 00:47:49]

lgtm

http://codereview.chromium.org/3036011/diff/21001/22003
File net/base/host_resolver_impl.cc (right):

http://codereview.chromium.org/3036011/diff/21001/22003#newcode730
net/base/host_resolver_impl.cc:730: if (HaveOnlyLoopbackAddresses())
FYI: be aware this might cause a startup test regression. When jar landed
something similar for IPv6 probing it caused such a regression, and he ended up
having to post an async task for it instead.

[eroman, 2010-07-23 00:48:14]

http://codereview.chromium.org/3036011/diff/21001/22001
File net/base/address_family.h (right):

http://codereview.chromium.org/3036011/diff/21001/22001#newcode22
net/base/address_family.h:22: // Hint to the resolve proc that only loopback
addresses are configured.
typo: resolver

[eroman, 2010-07-23 00:57:50]

http://codereview.chromium.org/3036011/diff/21001/22003
File net/base/host_resolver_impl.cc (right):

http://codereview.chromium.org/3036011/diff/21001/22003#newcode1062
net/base/host_resolver_impl.cc:1062: #if defined(OS_LINUX)
Should we enable this on mac as well?

[vandebo (ex-Chrome), 2010-07-23 01:03:30]

On 2010/07/23 00:47:49, eroman wrote:
> http://codereview.chromium.org/3036011/diff/21001/22003#newcode730
> net/base/host_resolver_impl.cc:730: if (HaveOnlyLoopbackAddresses())
> FYI: be aware this might cause a startup test regression. When jar landed
> something similar for IPv6 probing it caused such a regression, and he ended
up
> having to post an async task for it instead.

There are a couple of other parts to the IPv6 probe; I suspect this isn't the
slow part. But, I will certainly keep an eye on it.

On 2010/07/23 00:57:50, eroman wrote:
> http://codereview.chromium.org/3036011/diff/21001/22003#newcode1062
> net/base/host_resolver_impl.cc:1062: #if defined(OS_LINUX)
> Should we enable this on mac as well?

I tested this on Mac, and it seemed to work already.  If we find it actually is
a problem, it should be easy to change.