2011-11-08 03:14:56

by Yang Bai

[permalink] [raw]
Subject: [PATCH] mount.nfs: ensure AF priority when mounting

On different architectures, getaddrinfo() may return different
orders of address families. This may confuse the users.
This patch fixes this problem by ensuring that we first try
through IPv6(if present), then through IPv4.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=750729

Signed-off-by: Yang Bai <[email protected]>
---
utils/mount/stropts.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 4032bf3..aa5ee86 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -654,7 +654,28 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
struct addrinfo *ai;
int ret = 0;

+ /* first, try IPv6 */
for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+ if (ai->ai_family != AF_INET6)
+ continue;
+ ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
+ if (ret != 0)
+ return ret;
+
+ switch (errno) {
+ case ECONNREFUSED:
+ case EOPNOTSUPP:
+ case EHOSTUNREACH:
+ continue;
+ default:
+ goto out;
+ }
+ }
+
+ /* then, try IPv4 */
+ for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+ if (ai->ai_family != AF_INET)
+ continue;
ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
if (ret != 0)
return ret;
@@ -742,7 +763,27 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
struct addrinfo *ai;
int ret = 0;

+ /* first, try IPv6 */
for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+ if (ai->ai_family != AF_INET6)
+ continue;
+ ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
+ if (ret != 0)
+ return ret;
+
+ switch (errno) {
+ case ECONNREFUSED:
+ case EHOSTUNREACH:
+ continue;
+ default:
+ goto out;
+ }
+ }
+
+ /* then, try IPv4 */
+ for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+ if (ai->ai_family != AF_INET)
+ continue;
ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
if (ret != 0)
return ret;
--
1.7.1


2011-11-08 04:33:38

by Yang Bai

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: ensure AF priority when mounting

On Tue, Nov 8, 2011 at 12:01 PM, Chuck Lever <[email protected]> wrote:
>
> On Nov 7, 2011, at 10:14 PM, Hamo wrote:
>
>> On different architectures, getaddrinfo() may return different
>> orders of address families. This may confuse the users.
>> This patch fixes this problem by ensuring that we first try
>> through IPv6(if present), then through IPv4.
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=750729
>
> By the way, this bug isn't public (I can't access it, anyway). ?If RH can't make it public, perhaps a copy of the relevant parts of the bug can be created in bugzilla.linux-nfs.org? ?Otherwise, it's hard to tell what you are trying to fix so we can do a thorough patch review.
>
> (Also, I'm not sure why you copied lkml and linux-fsdevel on this NFS-specific user space patch).
>

Hi Chuck,
added you in the CC list so that you can view this bug.
I miss-added those two mailing lists, thanks for pointing out this.

Thanks,
Yang

2011-11-08 11:32:29

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: ensure AF priority when mounting

On Tue, 8 Nov 2011 12:51:21 +0800
Hamo <[email protected]> wrote:

> On Tue, Nov 8, 2011 at 11:57 AM, Chuck Lever <[email protected]> wrote:
> >
> > On Nov 7, 2011, at 10:14 PM, Hamo wrote:
> >
> >> On different architectures, getaddrinfo() may return different
> >> orders of address families. This may confuse the users.
> >
> > Isn't this controlled by /etc/gai.conf ?  I'm not sure we want mount.nfs to override that.
> >
>
> After digging into this file, I found the order can be controlled by
> this file, but without it, the order is not predictable.
> Should we first try IPv6 then IPv4? This sounds reasonable.
> So now, I am also not sure should we override this. Waiting for the
> opinions from others.


I don't think we ought to try and do this in nfs-utils. I see no reason
that the architecture should have any effect at all on the sort order of
address families being returned. If there is, then that sounds like a
(libc?) bug on those arches.

--
Jeff Layton <[email protected]>

2011-11-08 04:51:42

by Yang Bai

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: ensure AF priority when mounting

On Tue, Nov 8, 2011 at 11:57 AM, Chuck Lever <[email protected]> wrote:
>
> On Nov 7, 2011, at 10:14 PM, Hamo wrote:
>
>> On different architectures, getaddrinfo() may return different
>> orders of address families. This may confuse the users.
>
> Isn't this controlled by /etc/gai.conf ? ?I'm not sure we want mount.nfs to override that.
>

After digging into this file, I found the order can be controlled by
this file, but without it, the order is not predictable.
Should we first try IPv6 then IPv4? This sounds reasonable.
So now, I am also not sure should we override this. Waiting for the
opinions from others.

2011-11-08 04:01:20

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: ensure AF priority when mounting


On Nov 7, 2011, at 10:14 PM, Hamo wrote:

> On different architectures, getaddrinfo() may return different
> orders of address families. This may confuse the users.
> This patch fixes this problem by ensuring that we first try
> through IPv6(if present), then through IPv4.
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=750729

By the way, this bug isn't public (I can't access it, anyway). If RH can't make it public, perhaps a copy of the relevant parts of the bug can be created in bugzilla.linux-nfs.org? Otherwise, it's hard to tell what you are trying to fix so we can do a thorough patch review.

(Also, I'm not sure why you copied lkml and linux-fsdevel on this NFS-specific user space patch).

> Signed-off-by: Yang Bai <[email protected]>
> ---
> utils/mount/stropts.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 4032bf3..aa5ee86 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -654,7 +654,28 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
> struct addrinfo *ai;
> int ret = 0;
>
> + /* first, try IPv6 */
> for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET6)
> + continue;
> + ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
> + if (ret != 0)
> + return ret;
> +
> + switch (errno) {
> + case ECONNREFUSED:
> + case EOPNOTSUPP:
> + case EHOSTUNREACH:
> + continue;
> + default:
> + goto out;
> + }
> + }
> +
> + /* then, try IPv4 */
> + for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET)
> + continue;
> ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
> if (ret != 0)
> return ret;
> @@ -742,7 +763,27 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
> struct addrinfo *ai;
> int ret = 0;
>
> + /* first, try IPv6 */
> for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET6)
> + continue;
> + ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
> + if (ret != 0)
> + return ret;
> +
> + switch (errno) {
> + case ECONNREFUSED:
> + case EHOSTUNREACH:
> + continue;
> + default:
> + goto out;
> + }
> + }
> +
> + /* then, try IPv4 */
> + for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET)
> + continue;
> ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
> if (ret != 0)
> return ret;
> --
> 1.7.1

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





2011-11-08 03:57:26

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] mount.nfs: ensure AF priority when mounting


On Nov 7, 2011, at 10:14 PM, Hamo wrote:

> On different architectures, getaddrinfo() may return different
> orders of address families. This may confuse the users.

Isn't this controlled by /etc/gai.conf ? I'm not sure we want mount.nfs to override that.

> This patch fixes this problem by ensuring that we first try
> through IPv6(if present), then through IPv4.
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=750729
>
> Signed-off-by: Yang Bai <[email protected]>
> ---
> utils/mount/stropts.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 4032bf3..aa5ee86 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -654,7 +654,28 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
> struct addrinfo *ai;
> int ret = 0;
>
> + /* first, try IPv6 */
> for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET6)
> + continue;
> + ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
> + if (ret != 0)
> + return ret;
> +
> + switch (errno) {
> + case ECONNREFUSED:
> + case EOPNOTSUPP:
> + case EHOSTUNREACH:
> + continue;
> + default:
> + goto out;
> + }
> + }
> +
> + /* then, try IPv4 */
> + for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET)
> + continue;
> ret = nfs_do_mount_v3v2(mi, ai->ai_addr, ai->ai_addrlen);
> if (ret != 0)
> return ret;
> @@ -742,7 +763,27 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
> struct addrinfo *ai;
> int ret = 0;
>
> + /* first, try IPv6 */
> for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET6)
> + continue;
> + ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
> + if (ret != 0)
> + return ret;
> +
> + switch (errno) {
> + case ECONNREFUSED:
> + case EHOSTUNREACH:
> + continue;
> + default:
> + goto out;
> + }
> + }
> +
> + /* then, try IPv4 */
> + for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> + if (ai->ai_family != AF_INET)
> + continue;
> ret = nfs_do_mount_v4(mi, ai->ai_addr, ai->ai_addrlen);
> if (ret != 0)
> return ret;
> --
> 1.7.1

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