[RFC] [PATCH 0/4] Protocol family negotiation in mount.nfs

[RFC] [PATCH 0/4] Protocol family negotiation in mount.nfs

[Chuck Lever]

Hi Jeff-

For the record, here are patches that add support to mount.nfs for
negotiating the protocol family correctly with servers that have both
IPv4 and IPv6 addresses.

I've done some basic testing here against Solaris NFS servers, Linux
with rpcbind (Fedora 12), and Linux with portmap (Ubuntu 9.10).

---

Chuck Lever (4):
      text-based mount: Support protocol family negotiation
      text-based mount: Set addr= option in nfs_try_mount_foo()
      text-based mount: Replace nfs_lookup() with getaddrinfo(3)
      text-based mount: Retry when server can't be reached


 utils/mount/nfs.man   |    6 ++
 utils/mount/stropts.c |  120 ++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 107 insertions(+), 19 deletions(-)

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

[PATCH 1/4] text-based mount: Retry when server can't be reached

[Chuck Lever]

We want new default behavior from mount.nfs when the server refuses a
connection.  Since connection refusal can be spurious (for example,
if the server is rebooting), mount.nfs should retry.

NFS shares that are automatically mounted by /etc/fstab at boot
time may be problematic.  The new behavior can be disabled by
specifying the "retry=0" mount option, or these mounts can be changed
to background mounts by specifying the "bg" option.

A kernel code change is still required for the mount(2) system call to
return ECONNREFUSED for NFSv4 mounts (see 2.6.33).  For v2/v3, the
version and transport negotiation logic in mount.nfs should drive a
retry if the server's rpcbind can't be reached.

Note that if a v2/v3 mount request encounters an unregistered NFS
service, it will still fail immediately.  That wouldn't be too hard
to change as well, but there are many more corner cases there where
failing immediately is appropriate.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/nfs.man   |    6 +++++-
 utils/mount/stropts.c |    4 ++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index 93bd642..c64de5f 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -363,7 +363,11 @@ The number of minutes that the
 command retries an NFS mount operation
 in the foreground or background before giving up.
 If this option is not specified, the default value for foreground mounts
-is 2 minutes, and the default value for background mounts is 10000 minutes (80 minutes shy of one week).
+is 2 minutes, and the default value for background mounts is 10000 minutes
+(80 minutes shy of one week).
+If a value of zero is specified, the
+.BR mount (8)
+command exits immediately after the first failure.
 .TP 1.5i
 .BI sec= mode
 The RPCGSS security flavor to use for accessing files on this mount point.
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 57a4b32..74224ff 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -515,6 +515,10 @@ nfs_rewrite_pmap_mount_options(struct mount_options *options)
 	if (!nfs_probe_bothports(mnt_saddr, mnt_salen, &mnt_pmap,
 				 nfs_saddr, nfs_salen, &nfs_pmap)) {
 		errno = ESPIPE;
+		if (rpc_createerr.cf_stat == RPC_PROGNOTREGISTERED)
+			errno = EOPNOTSUPP;
+		else if (rpc_createerr.cf_error.re_errno != 0)
+			errno = rpc_createerr.cf_error.re_errno;
 		return 0;
 	}
 

[PATCH 2/4] text-based mount: Replace nfs_lookup() with getaddrinfo(3)

[Chuck Lever]

Originally I thought it would be best to share the DNS query code
between the legacy mount code and the new text-based code, hence
the introduction of nfs_lookup().  However, it now appears we want
the text-based code to do a little more than take the first address
returned by the query.

So, let's invoke getaddrinfo(3) directly in stropts.c, and save
the returned addrinfo struct until the end of processing.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/stropts.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 74224ff..4ffee48 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -49,6 +49,10 @@
 #include "parse_dev.h"
 #include "conffile.h"
 
+#ifndef HAVE_DECL_AI_ADDRCONFIG
+#define AI_ADDRCONFIG	0
+#endif
+
 #ifndef NFS_PROGRAM
 #define NFS_PROGRAM	(100003)
 #endif
@@ -83,8 +87,7 @@ struct nfsmount_info {
 				*node,		/* mounted-on dir */
 				*type;		/* "nfs" or "nfs4" */
 	char			*hostname;	/* server's hostname */
-	union nfs_sockaddr	address;
-	socklen_t		salen;		/* size of server's address */
+	struct addrinfo		*address;	/* server's addresses */
 
 	struct mount_options	*options;	/* parsed mount options */
 	char			**extra_opts;	/* string for /etc/mtab */
@@ -333,17 +336,27 @@ static int nfs_set_version(struct nfsmount_info *mi)
  */
 static int nfs_validate_options(struct nfsmount_info *mi)
 {
-	struct sockaddr *sap = &mi->address.sa;
+	struct addrinfo hint = {
+		.ai_protocol	= (int)IPPROTO_UDP,
+		.ai_flags	= AI_ADDRCONFIG,
+	};
 	sa_family_t family;
+	int error;
 
 	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))
+
+	hint.ai_family = (int)family;
+	error = getaddrinfo(mi->hostname, NULL, &hint, &mi->address);
+	if (error != 0) {
+		nfs_error(_("%s: Failed to resolve server %s: %s"),
+			progname, mi->hostname, gai_strerror(error));
+		mi->address = NULL;
 		return 0;
+	}
 
 	if (!nfs_set_version(mi))
 		return 0;
@@ -351,7 +364,8 @@ 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->ai_addr,
+					mi->address->ai_addrlen, mi->options))
 		return 0;
 
 	return 1;
@@ -614,7 +628,7 @@ out_fail:
  */
 static int nfs_try_mount_v4(struct nfsmount_info *mi)
 {
-	struct sockaddr *sap = &mi->address.sa;
+	struct addrinfo *ai = mi->address;
 	struct mount_options *options = po_dup(mi->options);
 	int result = 0;
 
@@ -642,7 +656,7 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
 		}
 	}
 
-	if (!nfs_append_clientaddr_option(sap, mi->salen, options)) {
+	if (!nfs_append_clientaddr_option(ai->ai_addr, ai->ai_addrlen, options)) {
 		errno = EINVAL;
 		goto out_fail;
 	}
@@ -882,6 +896,7 @@ int nfsmount_string(const char *spec, const char *node, const char *type,
 	struct nfsmount_info mi = {
 		.spec		= spec,
 		.node		= node,
+		.address	= NULL,
 		.type		= type,
 		.extra_opts	= extra_opts,
 		.flags		= flags,
@@ -897,6 +912,7 @@ int nfsmount_string(const char *spec, const char *node, const char *type,
 	} else
 		nfs_error(_("%s: internal option parsing error"), progname);
 
+	freeaddrinfo(mi.address);
 	free(mi.hostname);
 	return retval;
 }

[PATCH 3/4] text-based mount: Set addr= option in nfs_try_mount_foo()

[Chuck Lever]

When retrying a mount request with a different server address, the
addr= option may change each time through the fg/bg loop.

Instead of setting the addr= option in nfs_validate_options(), set it
in nfs_try_mount_v2v3() and nfs_try_mount_v4().  This is much the
same thing we did recently with the version-specific mount options
which might change each time through the fg/bg retry loop.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/stropts.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 4ffee48..fc1b0da 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -563,10 +563,6 @@ static int nfs_sys_mount(struct nfsmount_info *mi, struct mount_options *opts)
 		return 0;
 	}
 
-	if (verbose)
-		printf(_("%s: trying text-based options '%s'\n"),
-			progname, options);
-
 	if (mi->fake)
 		return 1;
 
@@ -585,6 +581,7 @@ static int nfs_sys_mount(struct nfsmount_info *mi, struct mount_options *opts)
  */
 static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
 {
+	struct addrinfo *ai = mi->address;
 	struct mount_options *options = po_dup(mi->options);
 	int result = 0;
 
@@ -593,6 +590,11 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
 		return result;
 	}
 
+	if (!nfs_append_addr_option(ai->ai_addr, ai->ai_addrlen, options)) {
+		errno = EINVAL;
+		goto out_fail;
+	}
+
 	if (!nfs_fix_mounthost_option(options, mi->hostname)) {
 		errno = EINVAL;
 		goto out_fail;
@@ -613,6 +615,10 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
 		goto out_fail;
 	}
 
+	if (verbose)
+		printf(_("%s: trying text-based options '%s'\n"),
+			progname, *mi->extra_opts);
+
 	if (!nfs_rewrite_pmap_mount_options(options))
 		goto out_fail;
 
@@ -656,6 +662,11 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
 		}
 	}
 
+	if (!nfs_append_addr_option(ai->ai_addr, ai->ai_addrlen, options)) {
+		errno = EINVAL;
+		goto out_fail;
+	}
+
 	if (!nfs_append_clientaddr_option(ai->ai_addr, ai->ai_addrlen, options)) {
 		errno = EINVAL;
 		goto out_fail;
@@ -669,6 +680,10 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
 		goto out_fail;
 	}
 
+	if (verbose)
+		printf(_("%s: trying text-based options '%s'\n"),
+			progname, *mi->extra_opts);
+
 	result = nfs_sys_mount(mi, options);
 
 out_fail:

[PATCH 4/4] text-based mount: Support protocol family negotiation

[Chuck Lever]

Jeff Layton pointed out that the current negotiation logic in
stropts.c simply doesn't handle the case where a server may have an
IPv6 address and an IPv4 address, but only NFS/IPv4 is supported.
This is typical of all currently deployed Linux servers.

Add support for trying all addresses returned from DNS when
"proto=" is not specified on the command line.
---

 utils/mount/stropts.c |   71 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index fc1b0da..9b8c38f 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -576,12 +576,9 @@ static int nfs_sys_mount(struct nfsmount_info *mi, struct mount_options *opts)
 	return !result;
 }
 
-/*
- * For "-t nfs vers=2" or "-t nfs vers=3" mounts.
- */
-static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
+static int nfs_do_mount_v3v2(struct nfsmount_info *mi,
+		struct sockaddr *sap, socklen_t salen)
 {
-	struct addrinfo *ai = mi->address;
 	struct mount_options *options = po_dup(mi->options);
 	int result = 0;
 
@@ -590,7 +587,7 @@ static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
 		return result;
 	}
 
-	if (!nfs_append_addr_option(ai->ai_addr, ai->ai_addrlen, options)) {
+	if (!nfs_append_addr_option(sap, salen, options)) {
 		errno = EINVAL;
 		goto out_fail;
 	}
@@ -630,11 +627,36 @@ out_fail:
 }
 
 /*
- * For "-t nfs -o vers=4" or "-t nfs4" mounts.
+ * Attempt a "-t nfs vers=2" or "-t nfs vers=3" mount.
+ *
+ * Returns TRUE if successful, otherwise FALSE.
+ * "errno" is set to reflect the individual error.
  */
-static int nfs_try_mount_v4(struct nfsmount_info *mi)
+static int nfs_try_mount_v3v2(struct nfsmount_info *mi)
+{
+	struct addrinfo *ai;
+	int ret;
+
+	for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+		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:
+			break;
+		}
+	}
+	return ret;
+}
+
+static int nfs_do_mount_v4(struct nfsmount_info *mi,
+		struct sockaddr *sap, socklen_t salen)
 {
-	struct addrinfo *ai = mi->address;
 	struct mount_options *options = po_dup(mi->options);
 	int result = 0;
 
@@ -662,12 +684,12 @@ static int nfs_try_mount_v4(struct nfsmount_info *mi)
 		}
 	}
 
-	if (!nfs_append_addr_option(ai->ai_addr, ai->ai_addrlen, options)) {
+	if (!nfs_append_addr_option(sap, salen, options)) {
 		errno = EINVAL;
 		goto out_fail;
 	}
 
-	if (!nfs_append_clientaddr_option(ai->ai_addr, ai->ai_addrlen, options)) {
+	if (!nfs_append_clientaddr_option(sap, salen, options)) {
 		errno = EINVAL;
 		goto out_fail;
 	}
@@ -692,6 +714,33 @@ out_fail:
 }
 
 /*
+ * Attempt a "-t nfs -o vers=4" or "-t nfs4" mount.
+ *
+ * Returns TRUE if successful, otherwise FALSE.
+ * "errno" is set to reflect the individual error.
+ */
+static int nfs_try_mount_v4(struct nfsmount_info *mi)
+{
+	struct addrinfo *ai;
+	int ret;
+
+	for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+		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:
+			break;
+		}
+	}
+	return ret;
+}
+
+/*
  * This is a single pass through the fg/bg loop.
  *
  * Returns TRUE if successful, otherwise FALSE.

Re: [PATCH 1/4] text-based mount: Retry when server can't be reached

[Jeff Layton]

On Fri, 22 Jan 2010 16:06:21 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:

> We want new default behavior from mount.nfs when the server refuses a
> connection.  Since connection refusal can be spurious (for example,
> if the server is rebooting), mount.nfs should retry.
> 
> NFS shares that are automatically mounted by /etc/fstab at boot
> time may be problematic.  The new behavior can be disabled by
> specifying the "retry=0" mount option, or these mounts can be changed
> to background mounts by specifying the "bg" option.
> 
> A kernel code change is still required for the mount(2) system call to
> return ECONNREFUSED for NFSv4 mounts (see 2.6.33).  For v2/v3, the
> version and transport negotiation logic in mount.nfs should drive a
> retry if the server's rpcbind can't be reached.
> 
> Note that if a v2/v3 mount request encounters an unregistered NFS
> service, it will still fail immediately.  That wouldn't be too hard
> to change as well, but there are many more corner cases there where
> failing immediately is appropriate.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  utils/mount/nfs.man   |    6 +++++-
>  utils/mount/stropts.c |    4 ++++
>  2 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> index 93bd642..c64de5f 100644
> --- a/utils/mount/nfs.man
> +++ b/utils/mount/nfs.man
> @@ -363,7 +363,11 @@ The number of minutes that the
>  command retries an NFS mount operation
>  in the foreground or background before giving up.
>  If this option is not specified, the default value for foreground mounts
> -is 2 minutes, and the default value for background mounts is 10000 minutes (80 minutes shy of one week).
> +is 2 minutes, and the default value for background mounts is 10000 minutes
> +(80 minutes shy of one week).
> +If a value of zero is specified, the
> +.BR mount (8)
> +command exits immediately after the first failure.
>  .TP 1.5i
>  .BI sec= mode
>  The RPCGSS security flavor to use for accessing files on this mount point.
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 57a4b32..74224ff 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -515,6 +515,10 @@ nfs_rewrite_pmap_mount_options(struct mount_options *options)
>  	if (!nfs_probe_bothports(mnt_saddr, mnt_salen, &mnt_pmap,
>  				 nfs_saddr, nfs_salen, &nfs_pmap)) {
>  		errno = ESPIPE;
> +		if (rpc_createerr.cf_stat == RPC_PROGNOTREGISTERED)
> +			errno = EOPNOTSUPP;
> +		else if (rpc_createerr.cf_error.re_errno != 0)
> +			errno = rpc_createerr.cf_error.re_errno;
>  		return 0;
>  	}
>  
> 


Apologies for the long delay in testing these. I gave the most recent
set of these in your git tree some testing this morning and they seem
to do the right thing.

I've also posted a patch this morning that's similar to the one
currently in Fedora to make the Defaultproto= option affect the address
family selection as well. I see that as complimentary to this set, and
it seems to work properly in conjunction with it.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [RFC] [PATCH 0/4] Protocol family negotiation in mount.nfs

[Steve Dickson]

On 01/22/2010 04:06 PM, Chuck Lever wrote:
> Hi Jeff-
> 
> For the record, here are patches that add support to mount.nfs for
> negotiating the protocol family correctly with servers that have both
> IPv4 and IPv6 addresses.
> 
> I've done some basic testing here against Solaris NFS servers, Linux
> with rpcbind (Fedora 12), and Linux with portmap (Ubuntu 9.10).
> 
> ---
> 
> Chuck Lever (4):
>       text-based mount: Support protocol family negotiation
>       text-based mount: Set addr= option in nfs_try_mount_foo()
>       text-based mount: Replace nfs_lookup() with getaddrinfo(3)
>       text-based mount: Retry when server can't be reached
> 
> 
>  utils/mount/nfs.man   |    6 ++
>  utils/mount/stropts.c |  120 ++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 107 insertions(+), 19 deletions(-)
> 
All four patches tested and committed...

steved.