2010-01-19 13:27:44

by Jeff Layton

[permalink] [raw]
Subject: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

We're poised to enable IPv6 in nfs-utils in distros. There is a
potential problem however. mount.nfs will prefer IPv6 addrs.

If someone has a working IPv4 server today that has an IPv6 address,
then clients may start trying to mount over that address. If the server
doesn't support NFS serving over IPv6 (and virtually no linux servers
currently do), then the mount will start failing.

Avoid this problem by making the mount code prefer IPv4 addresses
when they are available and an address family isn't specified.

This is the second attempt at this patch. This moves the changes
into nfs_validate_options. Chuck also mentioned parameterizing this
behavior too. This patch doesn't include that, as I wasn't exactly
clear on what he had in mind.

Signed-off-by: Jeff Layton <[email protected]>
---
utils/mount/stropts.c | 32 +++++++++++++++++++++++++-------
1 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 57a4b32..f0937a2 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -326,6 +326,29 @@ static int nfs_set_version(struct nfsmount_info *mi)
return 1;
}

+static int nfs_addr_option_lookup(struct nfsmount_info *mi)
+{
+ struct sockaddr *sap = &mi->address.sa;
+ sa_family_t family;
+
+ if (!nfs_nfs_proto_family(mi->options, &family))
+ return 0;
+
+ mi->salen = sizeof(mi->address);
+
+#ifdef IPV6_SUPPORTED
+ /*
+ * A lot of servers don't support NFS over IPv6 yet. For now,
+ * IPv4 addresses are preferred.
+ */
+ if (family == AF_UNSPEC &&
+ nfs_lookup(mi->hostname, AF_INET, sap, &mi->salen))
+ return 1;
+#endif /* IPV6_SUPPORTED */
+
+ return nfs_lookup(mi->hostname, family, sap, &mi->salen);
+}
+
/*
* Set up mandatory non-version specific NFS mount options.
*
@@ -333,16 +356,11 @@ static int nfs_set_version(struct nfsmount_info *mi)
*/
static int nfs_validate_options(struct nfsmount_info *mi)
{
- struct sockaddr *sap = &mi->address.sa;
- sa_family_t family;

if (!nfs_parse_devname(mi->spec, &mi->hostname, NULL))
return 0;

- if (!nfs_nfs_proto_family(mi->options, &family))
- return 0;
- mi->salen = sizeof(mi->address);
- if (!nfs_lookup(mi->hostname, family, sap, &mi->salen))
+ if (!nfs_addr_option_lookup(mi))
return 0;

if (!nfs_set_version(mi))
@@ -351,7 +369,7 @@ static int nfs_validate_options(struct nfsmount_info *mi)
if (!nfs_append_sloppy_option(mi->options))
return 0;

- if (!nfs_append_addr_option(sap, mi->salen, mi->options))
+ if (!nfs_append_addr_option(&mi->address.sa, mi->salen, mi->options))
return 0;

return 1;
--
1.6.5.2



2010-01-20 15:37:27

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)


On Jan 20, 2010, at 8:29 AM, Jeff Layton wrote:

> On Tue, 19 Jan 2010 10:43:34 -0500
> Chuck Lever <[email protected]> wrote:
>
>>
>> On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:
>>
>>> We're poised to enable IPv6 in nfs-utils in distros. There is a
>>> potential problem however. mount.nfs will prefer IPv6 addrs.
>>>
>>> If someone has a working IPv4 server today that has an IPv6 address,
>>> then clients may start trying to mount over that address. If the
>>> server
>>> doesn't support NFS serving over IPv6 (and virtually no linux
>>> servers
>>> currently do), then the mount will start failing.
>>>
>>> Avoid this problem by making the mount code prefer IPv4 addresses
>>> when they are available and an address family isn't specified.
>>>
>>> This is the second attempt at this patch. This moves the changes
>>> into nfs_validate_options. Chuck also mentioned parameterizing this
>>> behavior too. This patch doesn't include that, as I wasn't exactly
>>> clear on what he had in mind.
>>>
>
> After playing around with this some more, I'm coming to the conclusion
> that my first patch for this (the one that modified nfs_lookup) is
> really the best one.
>
> As Chuck pointed out, we need to have the address resolution behave
> consistently, so I'd need to have other places that currently call
> nfs_lookup do something similar.
>
> There are 4 callers of nfs_lookup currently:
>
> nfs_gethostbyname
> nfs_umount_do_umnt
> nfs_fix_mounthost_option
> nfs_validate_options
>
> ...all of these with the exception of nfs_gethostbyname will need to
> do
> this "sorting". nfs_gethostbyname passes in AF_INET for the family, so
> it's guaranteed to return a IPv4 address or nothing anyway.
>
> I've tried a more-or-less exhaustive combination of proto=/mountproto=
> options (and lack thereof) and I think with the original patch I
> posted
> it works as expected.
>
> On IRC, Chuck mentioned replacing nfs_lookup with direct calls to
> getaddrinfo, but that's a larger change and I don't really think it's
> necessary.
>
> Chuck also suggested that we should have mount.nfs never attempt to
> use
> an IPv6 address unless someone explicitly adds proto=tcp6|udp6. I'm
> not
> a fan of that solution. I think if a hostname resolves to only an IPv6
> address it ought to "just work" and mount via that address without
> needing extra options.
>
> For discussion, the original patch follows:

For the record, we looked at Solaris behavior yesterday. With bi-
family servers, its mount command tries IPv6 first, but appears smart
enough to fall back to IPv4. One thing we haven't tried is to see how
difficult it would be to fix the real problem by adding proper
protocol family negotiation to our own mount command. This patch is
predicated on the idea that would be hard to implement, which hasn't
been demonstrated.

I'm worried about putting this preference behavior in released code,
and then changing it yet again later. I don't know how much time we
have to fix this, but if we have a few days, I could try implementing
real protocol family negotiation.

If you go forward with this solution, it should be made clear in the
documenting comments (and in the patch description) that this is a
temporary workaround. If we intend to further correct this behavior
down the road, we should avoid building on the new behavior, even by
accident.

> -----------------[snip]-------------------
>
> mount.nfs: prefer IPv4 addresses over IPv6
>
> We're poised to enable IPv6 in nfs-utils in distros. There is a
> potential
> problem however. mount.nfs will prefer IPv6 addrs.
>
> If someone has a working IPv4 server today that has an IPv6 address,
> then clients may start trying to mount over that address. If the
> server
> doesn't support NFS serving over IPv6 (and virtually no linux servers
> currently do), then the mount will start failing.
>
> Avoid this problem by making the mount.nfs code prefer IPv4 addresses
> when they are available.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> utils/mount/network.c | 27 ++++++++++++++++++++++-----
> 1 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 92bba2d..6723f8f 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -203,7 +203,7 @@ static const unsigned int *nfs_default_proto()
> int nfs_lookup(const char *hostname, const sa_family_t family,
> struct sockaddr *sap, socklen_t *salen)
> {
> - struct addrinfo *gai_results;
> + struct addrinfo *gai_results, *gai_pref;
> struct addrinfo gai_hint = {
> #ifdef HAVE_DECL_AI_ADDRCONFIG
> .ai_flags = AI_ADDRCONFIG,
> @@ -229,12 +229,29 @@ int nfs_lookup(const char *hostname, const
> sa_family_t family,
> return ret;
> }
>
> - switch (gai_results->ai_addr->sa_family) {
> + /*
> + * It will probably be quite a while before we have enough IPv6
> + * capable servers to be able to prefer using IPv6. For now, we
> + * only use IPv6 when there is no IPv4 address available in the
> + * results.
> + */
> + gai_pref = gai_results;
> + while (gai_pref) {
> + if (gai_pref->ai_addr->sa_family == AF_INET)
> + break;
> + gai_pref = gai_pref->ai_next;
> + }
> +
> + /* no IPv4 addr found, just use first on the list */
> + if (gai_pref == NULL)
> + gai_pref = gai_results;
> +
> + switch (gai_pref->ai_addr->sa_family) {
> case AF_INET:
> case AF_INET6:
> - if (len >= gai_results->ai_addrlen) {
> - *salen = gai_results->ai_addrlen;
> - memcpy(sap, gai_results->ai_addr, *salen);
> + if (len >= gai_pref->ai_addrlen) {
> + *salen = gai_pref->ai_addrlen;
> + memcpy(sap, gai_pref->ai_addr, *salen);
> ret = 1;
> }
> break;
> --
> 1.6.5.2
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2010-01-19 15:44:40

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)


On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:

> We're poised to enable IPv6 in nfs-utils in distros. There is a
> potential problem however. mount.nfs will prefer IPv6 addrs.
>
> If someone has a working IPv4 server today that has an IPv6 address,
> then clients may start trying to mount over that address. If the
> server
> doesn't support NFS serving over IPv6 (and virtually no linux servers
> currently do), then the mount will start failing.
>
> Avoid this problem by making the mount code prefer IPv4 addresses
> when they are available and an address family isn't specified.
>
> This is the second attempt at this patch. This moves the changes
> into nfs_validate_options. Chuck also mentioned parameterizing this
> behavior too. This patch doesn't include that, as I wasn't exactly
> clear on what he had in mind.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> utils/mount/stropts.c | 32 +++++++++++++++++++++++++-------
> 1 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 57a4b32..f0937a2 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -326,6 +326,29 @@ static int nfs_set_version(struct nfsmount_info
> *mi)
> return 1;
> }
>
> +static int nfs_addr_option_lookup(struct nfsmount_info *mi)
> +{
> + struct sockaddr *sap = &mi->address.sa;
> + sa_family_t family;
> +
> + if (!nfs_nfs_proto_family(mi->options, &family))
> + return 0;

The real problem is that nfs_nfs_proto_family() returns AF_UNSPEC if
"proto=" was not specified. The negotiation logic here in this file
can't handle that return properly.

We could change nfs_nfs_proto_family(), or we could decide that
returning AF_UNSPEC is the right thing to do when "proto=" isn't
specified. If the latter, then nfs_nfs_proto_family() (and
nfs_mount_proto_family) should get a documenting comment that
describes the meaning of the AF_UNSPEC return.

Also, do you need to look at how nfs_mount_proto_family is used in
other nearby routines? It has the same problem. What do you do when
"mountproto=udp6" is specified?

> +
> + mi->salen = sizeof(mi->address);
> +
> +#ifdef IPV6_SUPPORTED
> + /*
> + * A lot of servers don't support NFS over IPv6 yet. For now,
> + * IPv4 addresses are preferred.
> + */

I think that doesn't describe this workaround adequately. This is a
temporary crutch that prevents us from using IPv6 if "proto=" isn't
specified. The underlying problem here is that nfs_lookup() returns
just one address.

> + if (family == AF_UNSPEC &&
> + nfs_lookup(mi->hostname, AF_INET, sap, &mi->salen))
> + return 1;
> +#endif /* IPV6_SUPPORTED */
> +
> + return nfs_lookup(mi->hostname, family, sap, &mi->salen);
> +}
> +
> /*
> * Set up mandatory non-version specific NFS mount options.
> *
> @@ -333,16 +356,11 @@ static int nfs_set_version(struct
> nfsmount_info *mi)
> */
> static int nfs_validate_options(struct nfsmount_info *mi)
> {
> - struct sockaddr *sap = &mi->address.sa;
> - sa_family_t family;
>
> if (!nfs_parse_devname(mi->spec, &mi->hostname, NULL))
> return 0;
>
> - if (!nfs_nfs_proto_family(mi->options, &family))
> - return 0;
> - mi->salen = sizeof(mi->address);
> - if (!nfs_lookup(mi->hostname, family, sap, &mi->salen))
> + if (!nfs_addr_option_lookup(mi))
> return 0;
>
> if (!nfs_set_version(mi))
> @@ -351,7 +369,7 @@ static int nfs_validate_options(struct
> nfsmount_info *mi)
> if (!nfs_append_sloppy_option(mi->options))
> return 0;
>
> - if (!nfs_append_addr_option(sap, mi->salen, mi->options))
> + if (!nfs_append_addr_option(&mi->address.sa, mi->salen, mi-
> >options))
> return 0;
>
> return 1;
> --
> 1.6.5.2
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2010-01-21 19:39:09

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On 01/21/2010 02:15 PM, J. Bruce Fields wrote:
> On Wed, Jan 20, 2010 at 10:36:36AM -0500, Chuck Lever wrote:
>> For the record, we looked at Solaris behavior yesterday. With bi-family
>> servers, its mount command tries IPv6 first, but appears smart enough to
>> fall back to IPv4. One thing we haven't tried is to see how difficult it
>> would be to fix the real problem by adding proper protocol family
>> negotiation to our own mount command.
>
> Sorry, I probably just haven't been following: what's "proper protocol
> family negotiation"? I thought the only ways to negotiate were either
> rpcbind (v2, v3) or trial and error (v4)?

In TI-RPC parlance, a "protocol" is the transport protocol (UDP, for
example), and a "protocol family" is the address family ("inet6", for
example). A netid represents a particular combination of the two: the
netid "udp6" represents UDP over "inet6".

The "protocol family" is really the value that is passed to socket(2).
This call generally takes PF_INET or something like that as its first
argument. All of the PF_FOO thingies have the same integer value as
their AF_FOO counterparts. For TI-RPC, we have "inet" and "inet6",
which are strings that match up with the AF_FOO and PF_FOO names.

rpcb_getaddr(3t) is designed to use the rpcbind protocol to determine
the address and transport to use when contacting a remote service. Our
mount command has its own negotiation mechanism that is a superset of
rpcbind calls, in addition to having a faster timeout than rpcb_getaddr(3t).

Until now, mount.nfs hasn't needed to negotiate the protocol family in
addition to the NFS and mount versions and transports. It always
assumed "inet" (or AF_INET, or PF_INET), and until recently, used only
rpcbind protocol version 2.

--
chuck[dot]lever[at]oracle[dot]com

2010-01-20 16:34:39

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Wed, 20 Jan 2010 10:36:36 -0500
Chuck Lever <[email protected]> wrote:

>
> On Jan 20, 2010, at 8:29 AM, Jeff Layton wrote:
>
> > On Tue, 19 Jan 2010 10:43:34 -0500
> > Chuck Lever <[email protected]> wrote:
> >
> >>
> >> On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:
> >>
> >>> We're poised to enable IPv6 in nfs-utils in distros. There is a
> >>> potential problem however. mount.nfs will prefer IPv6 addrs.
> >>>
> >>> If someone has a working IPv4 server today that has an IPv6 address,
> >>> then clients may start trying to mount over that address. If the
> >>> server
> >>> doesn't support NFS serving over IPv6 (and virtually no linux
> >>> servers
> >>> currently do), then the mount will start failing.
> >>>
> >>> Avoid this problem by making the mount code prefer IPv4 addresses
> >>> when they are available and an address family isn't specified.
> >>>
> >>> This is the second attempt at this patch. This moves the changes
> >>> into nfs_validate_options. Chuck also mentioned parameterizing this
> >>> behavior too. This patch doesn't include that, as I wasn't exactly
> >>> clear on what he had in mind.
> >>>
> >
> > After playing around with this some more, I'm coming to the conclusion
> > that my first patch for this (the one that modified nfs_lookup) is
> > really the best one.
> >
> > As Chuck pointed out, we need to have the address resolution behave
> > consistently, so I'd need to have other places that currently call
> > nfs_lookup do something similar.
> >
> > There are 4 callers of nfs_lookup currently:
> >
> > nfs_gethostbyname
> > nfs_umount_do_umnt
> > nfs_fix_mounthost_option
> > nfs_validate_options
> >
> > ...all of these with the exception of nfs_gethostbyname will need to
> > do
> > this "sorting". nfs_gethostbyname passes in AF_INET for the family, so
> > it's guaranteed to return a IPv4 address or nothing anyway.
> >
> > I've tried a more-or-less exhaustive combination of proto=/mountproto=
> > options (and lack thereof) and I think with the original patch I
> > posted
> > it works as expected.
> >
> > On IRC, Chuck mentioned replacing nfs_lookup with direct calls to
> > getaddrinfo, but that's a larger change and I don't really think it's
> > necessary.
> >
> > Chuck also suggested that we should have mount.nfs never attempt to
> > use
> > an IPv6 address unless someone explicitly adds proto=tcp6|udp6. I'm
> > not
> > a fan of that solution. I think if a hostname resolves to only an IPv6
> > address it ought to "just work" and mount via that address without
> > needing extra options.
> >
> > For discussion, the original patch follows:
>
> For the record, we looked at Solaris behavior yesterday. With bi-
> family servers, its mount command tries IPv6 first, but appears smart
> enough to fall back to IPv4. One thing we haven't tried is to see how
> difficult it would be to fix the real problem by adding proper
> protocol family negotiation to our own mount command. This patch is
> predicated on the idea that would be hard to implement, which hasn't
> been demonstrated.
>
> I'm worried about putting this preference behavior in released code,
> and then changing it yet again later. I don't know how much time we
> have to fix this, but if we have a few days, I could try implementing
> real protocol family negotiation.
>
> If you go forward with this solution, it should be made clear in the
> documenting comments (and in the patch description) that this is a
> temporary workaround. If we intend to further correct this behavior
> down the road, we should avoid building on the new behavior, even by
> accident.
>

Changing behavior is a valid concern and something we generally like to
avoid. OTOH, if someone doesn't specify proto=/mountproto= options
explicitly, then I think it's fair game to change the address
preference if we think it's reasonable and necessary.

I agree that doing proper negotiation would be ideal. I'll note that
with a 2.6.33-ish kernel, I quick -ECONNREFUSED errors when attempting
a NFSv4 mount to a server that doesn't support NFS over IPv6, so it
seems doable.

If you think you can get proper negotiation done within a few days,
then I say go for it and I'll hold off on asking Steve to take this
patch. If it turns out to be more difficult, we can always revisit this
patch.

--
Jeff Layton <[email protected]>

2010-01-20 13:29:17

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Tue, 19 Jan 2010 10:43:34 -0500
Chuck Lever <[email protected]> wrote:

>
> On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:
>
> > We're poised to enable IPv6 in nfs-utils in distros. There is a
> > potential problem however. mount.nfs will prefer IPv6 addrs.
> >
> > If someone has a working IPv4 server today that has an IPv6 address,
> > then clients may start trying to mount over that address. If the
> > server
> > doesn't support NFS serving over IPv6 (and virtually no linux servers
> > currently do), then the mount will start failing.
> >
> > Avoid this problem by making the mount code prefer IPv4 addresses
> > when they are available and an address family isn't specified.
> >
> > This is the second attempt at this patch. This moves the changes
> > into nfs_validate_options. Chuck also mentioned parameterizing this
> > behavior too. This patch doesn't include that, as I wasn't exactly
> > clear on what he had in mind.
> >

After playing around with this some more, I'm coming to the conclusion
that my first patch for this (the one that modified nfs_lookup) is
really the best one.

As Chuck pointed out, we need to have the address resolution behave
consistently, so I'd need to have other places that currently call
nfs_lookup do something similar.

There are 4 callers of nfs_lookup currently:

nfs_gethostbyname
nfs_umount_do_umnt
nfs_fix_mounthost_option
nfs_validate_options

...all of these with the exception of nfs_gethostbyname will need to do
this "sorting". nfs_gethostbyname passes in AF_INET for the family, so
it's guaranteed to return a IPv4 address or nothing anyway.

I've tried a more-or-less exhaustive combination of proto=/mountproto=
options (and lack thereof) and I think with the original patch I posted
it works as expected.

On IRC, Chuck mentioned replacing nfs_lookup with direct calls to
getaddrinfo, but that's a larger change and I don't really think it's
necessary.

Chuck also suggested that we should have mount.nfs never attempt to use
an IPv6 address unless someone explicitly adds proto=tcp6|udp6. I'm not
a fan of that solution. I think if a hostname resolves to only an IPv6
address it ought to "just work" and mount via that address without
needing extra options.

For discussion, the original patch follows:

-----------------[snip]-------------------

mount.nfs: prefer IPv4 addresses over IPv6

We're poised to enable IPv6 in nfs-utils in distros. There is a potential
problem however. mount.nfs will prefer IPv6 addrs.

If someone has a working IPv4 server today that has an IPv6 address,
then clients may start trying to mount over that address. If the server
doesn't support NFS serving over IPv6 (and virtually no linux servers
currently do), then the mount will start failing.

Avoid this problem by making the mount.nfs code prefer IPv4 addresses
when they are available.

Signed-off-by: Jeff Layton <[email protected]>
---
utils/mount/network.c | 27 ++++++++++++++++++++++-----
1 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 92bba2d..6723f8f 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -203,7 +203,7 @@ static const unsigned int *nfs_default_proto()
int nfs_lookup(const char *hostname, const sa_family_t family,
struct sockaddr *sap, socklen_t *salen)
{
- struct addrinfo *gai_results;
+ struct addrinfo *gai_results, *gai_pref;
struct addrinfo gai_hint = {
#ifdef HAVE_DECL_AI_ADDRCONFIG
.ai_flags = AI_ADDRCONFIG,
@@ -229,12 +229,29 @@ int nfs_lookup(const char *hostname, const sa_family_t family,
return ret;
}

- switch (gai_results->ai_addr->sa_family) {
+ /*
+ * It will probably be quite a while before we have enough IPv6
+ * capable servers to be able to prefer using IPv6. For now, we
+ * only use IPv6 when there is no IPv4 address available in the
+ * results.
+ */
+ gai_pref = gai_results;
+ while (gai_pref) {
+ if (gai_pref->ai_addr->sa_family == AF_INET)
+ break;
+ gai_pref = gai_pref->ai_next;
+ }
+
+ /* no IPv4 addr found, just use first on the list */
+ if (gai_pref == NULL)
+ gai_pref = gai_results;
+
+ switch (gai_pref->ai_addr->sa_family) {
case AF_INET:
case AF_INET6:
- if (len >= gai_results->ai_addrlen) {
- *salen = gai_results->ai_addrlen;
- memcpy(sap, gai_results->ai_addr, *salen);
+ if (len >= gai_pref->ai_addrlen) {
+ *salen = gai_pref->ai_addrlen;
+ memcpy(sap, gai_pref->ai_addr, *salen);
ret = 1;
}
break;
--
1.6.5.2


2010-01-20 13:13:22

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Tue, 19 Jan 2010 15:51:44 -0500
Trond Myklebust <[email protected]> wrote:

> On Tue, 2010-01-19 at 15:38 -0500, Jeff Layton wrote:
> > On Tue, 19 Jan 2010 10:43:34 -0500
> > Chuck Lever <[email protected]> wrote:
> > > I think that doesn't describe this workaround adequately. This is a
> > > temporary crutch that prevents us from using IPv6 if "proto=" isn't
> > > specified. The underlying problem here is that nfs_lookup() returns
> > > just one address.
> > >
> >
> > Yes. The best solution would be to somehow try all addresses in the
> > list until one works. That's a larger project however and we'll
> > probably need some significant kernel changes to handle that anyway.
>
> Why would that involve kernel changes? I'm assuming that we can just
> retry the mount call if we see that the server isn't listening on a
> particular ip address+port combination.
>
> Cheers
> Trond
>

True, it's not required. We could just return to userspace and have it
try again.

I think that there might be advantages to being able to pass a list of
addresses to the kernel, or overhaul the mount process to have the
kernel upcall for addresses, but it's not strictly required for this.

--
Jeff Layton <[email protected]>

2010-01-21 20:29:28

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On 01/21/2010 02:57 PM, J. Bruce Fields wrote:
> On Thu, Jan 21, 2010 at 02:37:58PM -0500, Chuck Lever wrote:
>> On 01/21/2010 02:15 PM, J. Bruce Fields wrote:
>>> On Wed, Jan 20, 2010 at 10:36:36AM -0500, Chuck Lever wrote:
>>>> For the record, we looked at Solaris behavior yesterday. With bi-family
>>>> servers, its mount command tries IPv6 first, but appears smart enough to
>>>> fall back to IPv4. One thing we haven't tried is to see how difficult it
>>>> would be to fix the real problem by adding proper protocol family
>>>> negotiation to our own mount command.
>>>
>>> Sorry, I probably just haven't been following: what's "proper protocol
>>> family negotiation"? I thought the only ways to negotiate were either
>>> rpcbind (v2, v3) or trial and error (v4)?
>>
>> In TI-RPC parlance, a "protocol" is the transport protocol (UDP, for
>> example), and a "protocol family" is the address family ("inet6", for
>> example). A netid represents a particular combination of the two: the
>> netid "udp6" represents UDP over "inet6".
>>
>> The "protocol family" is really the value that is passed to socket(2).
>> This call generally takes PF_INET or something like that as its first
>> argument. All of the PF_FOO thingies have the same integer value as
>> their AF_FOO counterparts. For TI-RPC, we have "inet" and "inet6",
>> which are strings that match up with the AF_FOO and PF_FOO names.
>>
>> rpcb_getaddr(3t) is designed to use the rpcbind protocol to determine
>> the address and transport to use when contacting a remote service. Our
>> mount command has its own negotiation mechanism that is a superset of
>> rpcbind calls, in addition to having a faster timeout than
>> rpcb_getaddr(3t).
>
> What does "is a superset of rpcbind calls" mean?

rpcb_getaddr(3t) performs a single specific rpcbind query with a long
fixed timeout. mount.nfs uses several rpcbind queries, in a particular
order, to identify which NFS-related services are available. mount.nfs
uses individual queries rather than a single DUMPALL in order to enable
firewalls to detect which ports should be opened.

> I still don't
> understand what the proper protocol family negotiation is: what actually
> happens on the wire?

If a particular RPC service (including rpcbind) cannot be contacted via
"inet6," and the server has an "inet" address listed in DNS, then
mount.nfs should be smart enough to try the mount request via the "inet"
address too. This is in addition to support for rpcbind queries that
can return a netid, which would include information about which protocol
family to use).

Currently our mount.nfs command fails if the target server has at least
one IPv6 address listed in DNS in addition to an IPv4 address, but does
not support NFS/IPv6.

For NFSv4, a server that has an IPv6 address but does not support
NFS/IPv6 will refuse connection to port 2049 over IPv6. In that case,
mount.nfs should tell the kernel to retry the mount with the server's
IPv4 address, if it has one.

For NFSv3, a server that has an IPv6 address, but does not support
NFS/IPv6, will not register any inet6 netids in its rpcbind database.
Thus the mount.nfs command has to be smart enough to retry
PROGNOTREGISTERED results with the server's IPv4 address, if it has one.

If the server has an IPv6 address, but is running portmap instead of
rpcbind, the initial rpcbind query connection will be refused (portmap
does not set up an IPv6 listener). In that case, the mount request
should be retried with the server's IPv4 address, if it has one.

Note that in any of these cases, if an NFS server does not have any IPv6
addresses listed in DNS, then behavior should be the same as before.

--
chuck[dot]lever[at]oracle[dot]com

2010-01-19 20:38:38

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Tue, 19 Jan 2010 10:43:34 -0500
Chuck Lever <[email protected]> wrote:

>
> On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:
>
> > We're poised to enable IPv6 in nfs-utils in distros. There is a
> > potential problem however. mount.nfs will prefer IPv6 addrs.
> >
> > If someone has a working IPv4 server today that has an IPv6 address,
> > then clients may start trying to mount over that address. If the
> > server
> > doesn't support NFS serving over IPv6 (and virtually no linux servers
> > currently do), then the mount will start failing.
> >
> > Avoid this problem by making the mount code prefer IPv4 addresses
> > when they are available and an address family isn't specified.
> >
> > This is the second attempt at this patch. This moves the changes
> > into nfs_validate_options. Chuck also mentioned parameterizing this
> > behavior too. This patch doesn't include that, as I wasn't exactly
> > clear on what he had in mind.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > utils/mount/stropts.c | 32 +++++++++++++++++++++++++-------
> > 1 files changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 57a4b32..f0937a2 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -326,6 +326,29 @@ static int nfs_set_version(struct nfsmount_info
> > *mi)
> > return 1;
> > }
> >
> > +static int nfs_addr_option_lookup(struct nfsmount_info *mi)
> > +{
> > + struct sockaddr *sap = &mi->address.sa;
> > + sa_family_t family;
> > +
> > + if (!nfs_nfs_proto_family(mi->options, &family))
> > + return 0;
>
> The real problem is that nfs_nfs_proto_family() returns AF_UNSPEC if
> "proto=" was not specified.

...and if it was built with libtirpc support.

> The negotiation logic here in this file
> can't handle that return properly.
>
> We could change nfs_nfs_proto_family(), or we could decide that
> returning AF_UNSPEC is the right thing to do when "proto=" isn't
> specified. If the latter, then nfs_nfs_proto_family() (and
> nfs_mount_proto_family) should get a documenting comment that
> describes the meaning of the AF_UNSPEC return.
>

Yep, and we also will need to fix it up for the non-libtirpc case.

> Also, do you need to look at how nfs_mount_proto_family is used in
> other nearby routines? It has the same problem. What do you do when
> "mountproto=udp6" is specified?
>

That is a problem we'll have to deal with, yes...

> > +
> > + mi->salen = sizeof(mi->address);
> > +
> > +#ifdef IPV6_SUPPORTED
> > + /*
> > + * A lot of servers don't support NFS over IPv6 yet. For now,
> > + * IPv4 addresses are preferred.
> > + */
>
> I think that doesn't describe this workaround adequately. This is a
> temporary crutch that prevents us from using IPv6 if "proto=" isn't
> specified. The underlying problem here is that nfs_lookup() returns
> just one address.
>

Yes. The best solution would be to somehow try all addresses in the
list until one works. That's a larger project however and we'll
probably need some significant kernel changes to handle that anyway.

> > + if (family == AF_UNSPEC &&
> > + nfs_lookup(mi->hostname, AF_INET, sap, &mi->salen))
> > + return 1;
> > +#endif /* IPV6_SUPPORTED */
> > +
> > + return nfs_lookup(mi->hostname, family, sap, &mi->salen);
> > +}
> > +
> > /*
> > * Set up mandatory non-version specific NFS mount options.
> > *
> > @@ -333,16 +356,11 @@ static int nfs_set_version(struct
> > nfsmount_info *mi)
> > */
> > static int nfs_validate_options(struct nfsmount_info *mi)
> > {
> > - struct sockaddr *sap = &mi->address.sa;
> > - sa_family_t family;
> >
> > if (!nfs_parse_devname(mi->spec, &mi->hostname, NULL))
> > return 0;
> >
> > - if (!nfs_nfs_proto_family(mi->options, &family))
> > - return 0;
> > - mi->salen = sizeof(mi->address);
> > - if (!nfs_lookup(mi->hostname, family, sap, &mi->salen))
> > + if (!nfs_addr_option_lookup(mi))
> > return 0;
> >
> > if (!nfs_set_version(mi))
> > @@ -351,7 +369,7 @@ static int nfs_validate_options(struct
> > nfsmount_info *mi)
> > if (!nfs_append_sloppy_option(mi->options))
> > return 0;
> >
> > - if (!nfs_append_addr_option(sap, mi->salen, mi->options))
> > + if (!nfs_append_addr_option(&mi->address.sa, mi->salen, mi-
> > >options))
> > return 0;
> >
> > return 1;
> > --
> > 1.6.5.2
> >
>


--
Jeff Layton <[email protected]>

2010-01-19 20:51:53

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Tue, 2010-01-19 at 15:38 -0500, Jeff Layton wrote:
> On Tue, 19 Jan 2010 10:43:34 -0500
> Chuck Lever <[email protected]> wrote:
> > I think that doesn't describe this workaround adequately. This is a
> > temporary crutch that prevents us from using IPv6 if "proto=" isn't
> > specified. The underlying problem here is that nfs_lookup() returns
> > just one address.
> >
>
> Yes. The best solution would be to somehow try all addresses in the
> list until one works. That's a larger project however and we'll
> probably need some significant kernel changes to handle that anyway.

Why would that involve kernel changes? I'm assuming that we can just
retry the mount call if we see that the server isn't listening on a
particular ip address+port combination.

Cheers
Trond


2010-01-19 21:07:11

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)


On Jan 19, 2010, at 3:51 PM, Trond Myklebust wrote:

> On Tue, 2010-01-19 at 15:38 -0500, Jeff Layton wrote:
>> On Tue, 19 Jan 2010 10:43:34 -0500
>> Chuck Lever <[email protected]> wrote:
>>> I think that doesn't describe this workaround adequately. This is a
>>> temporary crutch that prevents us from using IPv6 if "proto=" isn't
>>> specified. The underlying problem here is that nfs_lookup() returns
>>> just one address.
>>>
>>
>> Yes. The best solution would be to somehow try all addresses in the
>> list until one works. That's a larger project however and we'll
>> probably need some significant kernel changes to handle that anyway.
>
> Why would that involve kernel changes? I'm assuming that we can just
> retry the mount call if we see that the server isn't listening on a
> particular ip address+port combination.

For NFSv2/v3, the mount command can poke at the server's rpcbind
service to discover that the server won't support IPv6. For NFSv4,
the kernel jumps straight for a TCP connection, without consulting
rpcbind

I think Jeff is using a pre-2.6.33 kernel, which basically hangs when
the server refuses to connect in this case.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2010-01-20 19:11:08

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)


On Jan 20, 2010, at 11:34 AM, Jeff Layton wrote:

> On Wed, 20 Jan 2010 10:36:36 -0500
> Chuck Lever <[email protected]> wrote:
>
>>
>> On Jan 20, 2010, at 8:29 AM, Jeff Layton wrote:
>>
>>> On Tue, 19 Jan 2010 10:43:34 -0500
>>> Chuck Lever <[email protected]> wrote:
>>>
>>>>
>>>> On Jan 19, 2010, at 8:27 AM, Jeff Layton wrote:
>>>>
>>>>> We're poised to enable IPv6 in nfs-utils in distros. There is a
>>>>> potential problem however. mount.nfs will prefer IPv6 addrs.
>>>>>
>>>>> If someone has a working IPv4 server today that has an IPv6
>>>>> address,
>>>>> then clients may start trying to mount over that address. If the
>>>>> server
>>>>> doesn't support NFS serving over IPv6 (and virtually no linux
>>>>> servers
>>>>> currently do), then the mount will start failing.
>>>>>
>>>>> Avoid this problem by making the mount code prefer IPv4 addresses
>>>>> when they are available and an address family isn't specified.
>>>>>
>>>>> This is the second attempt at this patch. This moves the changes
>>>>> into nfs_validate_options. Chuck also mentioned parameterizing
>>>>> this
>>>>> behavior too. This patch doesn't include that, as I wasn't exactly
>>>>> clear on what he had in mind.
>>>>>
>>>
>>> After playing around with this some more, I'm coming to the
>>> conclusion
>>> that my first patch for this (the one that modified nfs_lookup) is
>>> really the best one.
>>>
>>> As Chuck pointed out, we need to have the address resolution behave
>>> consistently, so I'd need to have other places that currently call
>>> nfs_lookup do something similar.
>>>
>>> There are 4 callers of nfs_lookup currently:
>>>
>>> nfs_gethostbyname
>>> nfs_umount_do_umnt
>>> nfs_fix_mounthost_option
>>> nfs_validate_options
>>>
>>> ...all of these with the exception of nfs_gethostbyname will need to
>>> do
>>> this "sorting". nfs_gethostbyname passes in AF_INET for the
>>> family, so
>>> it's guaranteed to return a IPv4 address or nothing anyway.
>>>
>>> I've tried a more-or-less exhaustive combination of proto=/
>>> mountproto=
>>> options (and lack thereof) and I think with the original patch I
>>> posted
>>> it works as expected.
>>>
>>> On IRC, Chuck mentioned replacing nfs_lookup with direct calls to
>>> getaddrinfo, but that's a larger change and I don't really think
>>> it's
>>> necessary.
>>>
>>> Chuck also suggested that we should have mount.nfs never attempt to
>>> use
>>> an IPv6 address unless someone explicitly adds proto=tcp6|udp6. I'm
>>> not
>>> a fan of that solution. I think if a hostname resolves to only an
>>> IPv6
>>> address it ought to "just work" and mount via that address without
>>> needing extra options.
>>>
>>> For discussion, the original patch follows:
>>
>> For the record, we looked at Solaris behavior yesterday. With bi-
>> family servers, its mount command tries IPv6 first, but appears smart
>> enough to fall back to IPv4. One thing we haven't tried is to see
>> how
>> difficult it would be to fix the real problem by adding proper
>> protocol family negotiation to our own mount command. This patch is
>> predicated on the idea that would be hard to implement, which hasn't
>> been demonstrated.
>>
>> I'm worried about putting this preference behavior in released code,
>> and then changing it yet again later. I don't know how much time we
>> have to fix this, but if we have a few days, I could try implementing
>> real protocol family negotiation.
>>
>> If you go forward with this solution, it should be made clear in the
>> documenting comments (and in the patch description) that this is a
>> temporary workaround. If we intend to further correct this behavior
>> down the road, we should avoid building on the new behavior, even by
>> accident.
>>
>
> Changing behavior is a valid concern and something we generally like
> to
> avoid. OTOH, if someone doesn't specify proto=/mountproto= options
> explicitly, then I think it's fair game to change the address
> preference if we think it's reasonable and necessary.
>
> I agree that doing proper negotiation would be ideal. I'll note that
> with a 2.6.33-ish kernel, I quick -ECONNREFUSED errors when attempting
> a NFSv4 mount to a server that doesn't support NFS over IPv6, so it
> seems doable.
>
> If you think you can get proper negotiation done within a few days,
> then I say go for it and I'll hold off on asking Steve to take this
> patch. If it turns out to be more difficult, we can always revisit
> this
> patch.

I'm trying to merge up with the current state of upstream nfs-utils,
at the moment. Bruce's pseudofs work is now in the upstream tree, and
that's requiring some by-hand merging of the mountd/exportfs IPv6
patches.

I should be able to start on this later this afternoon, and promise to
report on or before Friday.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2010-01-21 19:13:53

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Wed, Jan 20, 2010 at 10:36:36AM -0500, Chuck Lever wrote:
> For the record, we looked at Solaris behavior yesterday. With bi-family
> servers, its mount command tries IPv6 first, but appears smart enough to
> fall back to IPv4. One thing we haven't tried is to see how difficult it
> would be to fix the real problem by adding proper protocol family
> negotiation to our own mount command.

Sorry, I probably just haven't been following: what's "proper protocol
family negotiation"? I thought the only ways to negotiate were either
rpcbind (v2, v3) or trial and error (v4)?

--b.

2010-01-21 19:56:25

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Thu, Jan 21, 2010 at 02:37:58PM -0500, Chuck Lever wrote:
> On 01/21/2010 02:15 PM, J. Bruce Fields wrote:
>> On Wed, Jan 20, 2010 at 10:36:36AM -0500, Chuck Lever wrote:
>>> For the record, we looked at Solaris behavior yesterday. With bi-family
>>> servers, its mount command tries IPv6 first, but appears smart enough to
>>> fall back to IPv4. One thing we haven't tried is to see how difficult it
>>> would be to fix the real problem by adding proper protocol family
>>> negotiation to our own mount command.
>>
>> Sorry, I probably just haven't been following: what's "proper protocol
>> family negotiation"? I thought the only ways to negotiate were either
>> rpcbind (v2, v3) or trial and error (v4)?
>
> In TI-RPC parlance, a "protocol" is the transport protocol (UDP, for
> example), and a "protocol family" is the address family ("inet6", for
> example). A netid represents a particular combination of the two: the
> netid "udp6" represents UDP over "inet6".
>
> The "protocol family" is really the value that is passed to socket(2).
> This call generally takes PF_INET or something like that as its first
> argument. All of the PF_FOO thingies have the same integer value as
> their AF_FOO counterparts. For TI-RPC, we have "inet" and "inet6",
> which are strings that match up with the AF_FOO and PF_FOO names.
>
> rpcb_getaddr(3t) is designed to use the rpcbind protocol to determine
> the address and transport to use when contacting a remote service. Our
> mount command has its own negotiation mechanism that is a superset of
> rpcbind calls, in addition to having a faster timeout than
> rpcb_getaddr(3t).

What does "is a superset of rpcbind calls" mean? I still don't
understand what the proper protocol family negotiation is: what actually
happens on the wire?

--b.

>
> Until now, mount.nfs hasn't needed to negotiate the protocol family in
> addition to the NFS and mount versions and transports. It always
> assumed "inet" (or AF_INET, or PF_INET), and until recently, used only
> rpcbind protocol version 2.

2010-01-21 21:50:47

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2)

On Thu, Jan 21, 2010 at 03:28:46PM -0500, Chuck Lever wrote:
> On 01/21/2010 02:57 PM, J. Bruce Fields wrote:
>> On Thu, Jan 21, 2010 at 02:37:58PM -0500, Chuck Lever wrote:
>>> rpcb_getaddr(3t) is designed to use the rpcbind protocol to determine
>>> the address and transport to use when contacting a remote service. Our
>>> mount command has its own negotiation mechanism that is a superset of
>>> rpcbind calls, in addition to having a faster timeout than
>>> rpcb_getaddr(3t).
>>
>> What does "is a superset of rpcbind calls" mean?
>
> rpcb_getaddr(3t) performs a single specific rpcbind query with a long
> fixed timeout. mount.nfs uses several rpcbind queries, in a particular
> order, to identify which NFS-related services are available. mount.nfs
> uses individual queries rather than a single DUMPALL in order to enable
> firewalls to detect which ports should be opened.
>
>> I still don't
>> understand what the proper protocol family negotiation is: what actually
>> happens on the wire?
>
> If a particular RPC service (including rpcbind) cannot be contacted via
> "inet6," and the server has an "inet" address listed in DNS, then
> mount.nfs should be smart enough to try the mount request via the "inet"
> address too. This is in addition to support for rpcbind queries that
> can return a netid, which would include information about which protocol
> family to use).
>
> Currently our mount.nfs command fails if the target server has at least
> one IPv6 address listed in DNS in addition to an IPv4 address, but does
> not support NFS/IPv6.
>
> For NFSv4, a server that has an IPv6 address but does not support
> NFS/IPv6 will refuse connection to port 2049 over IPv6. In that case,
> mount.nfs should tell the kernel to retry the mount with the server's
> IPv4 address, if it has one.
>
> For NFSv3, a server that has an IPv6 address, but does not support
> NFS/IPv6, will not register any inet6 netids in its rpcbind database.
> Thus the mount.nfs command has to be smart enough to retry
> PROGNOTREGISTERED results with the server's IPv4 address, if it has one.
>
> If the server has an IPv6 address, but is running portmap instead of
> rpcbind, the initial rpcbind query connection will be refused (portmap
> does not set up an IPv6 listener). In that case, the mount request
> should be retried with the server's IPv4 address, if it has one.
>
> Note that in any of these cases, if an NFS server does not have any IPv6
> addresses listed in DNS, then behavior should be the same as before.

OK, got it. (I think.) Thanks!

--b.