Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:16279 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750922Ab0ATN3R (ORCPT ); Wed, 20 Jan 2010 08:29:17 -0500 Date: Wed, 20 Jan 2010 08:29:05 -0500 From: Jeff Layton To: Chuck Lever Cc: linux-nfs@vger.kernel.org, steved@redhat.com, trond.myklebust@fys.uio.no Subject: Re: [PATCH] mount.nfs: prefer IPv4 addresses over IPv6 (try #2) Message-ID: <20100120082905.1825f806@tlielax.poochiereds.net> In-Reply-To: <1667647A-2BB1-478D-8881-CE8EA2191F97@oracle.com> References: <1263907662-19107-1-git-send-email-jlayton@redhat.com> <1667647A-2BB1-478D-8881-CE8EA2191F97@oracle.com> Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Tue, 19 Jan 2010 10:43:34 -0500 Chuck Lever 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 --- 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