* [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn
@ 2013-06-27 19:54 Jeff Layton
2013-06-27 19:54 ` [PATCH v4 1/4] nfs: refactor "need_mount" code out of nfs_try_mount Jeff Layton
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Jeff Layton @ 2013-06-27 19:54 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs, chuck.lever, Weston.Adamson
Changes:
v4:
- fake up server list in nfs_mount rather than in nfs_try_mount_request.
This simplifies the auth selection code by allowing it to always
assume that it has a non-empty authlist from the mount request.
v3:
- fix some signed vs. unsigned type comparisons
- change how an empty server_authlist is handled. Instead of picking an
authflavor to try at that point, just munge the list to contain only
RPC_AUTH_NULL. The rest of the logic can take over at that point.
I got a report of a regression in recent kernels. Windows 2012 servers
support v3 and v4.1. They also return a list of authflavors that starts
with AUTH_GSS flavors and ends with AUTH_SYS.
Since commit 4580a92 (NFS: Use server-recommended security flavor by
default (NFSv3)) mounting this server with nfsv3 fails unless you
specify sec=sys. I can replicate the problem with a Linux NFS server
by exporing a filesystem with "sec=krb5:sys".
This patchset overhauls the NFSv3 auth code to try each authflavor in
the list provided by the server in the order that it specified them.
With this, I'm again able to mount the server without needing any
special mount options.
Thanks to Chuck Lever for suggestions thus far...
Jeff Layton (4):
nfs: refactor "need_mount" code out of nfs_try_mount
nfs: move server_authlist into nfs_try_mount_request
nfs: have nfs_mount fake up a auth_flavs list when the server didn't
provide it
nfs: have NFSv3 try server-specified auth flavors in turn
fs/nfs/mount_clnt.c | 18 +++++-
fs/nfs/super.c | 175 ++++++++++++++++++++++++++++------------------------
2 files changed, 110 insertions(+), 83 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/4] nfs: refactor "need_mount" code out of nfs_try_mount
2013-06-27 19:54 [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn Jeff Layton
@ 2013-06-27 19:54 ` Jeff Layton
2013-06-27 19:54 ` [PATCH v4 2/4] nfs: move server_authlist into nfs_try_mount_request Jeff Layton
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2013-06-27 19:54 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs, chuck.lever, Weston.Adamson
This looks like pointless refactoring for now, but we'll flesh out
the need_mount case a little more in a later patch.
Cc: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/super.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2d7525f..afeee81 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1759,21 +1759,29 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
return nfs_select_flavor(args, &request);
}
+static struct nfs_server *nfs_try_mount_request(struct nfs_mount_info *mount_info,
+ struct nfs_subversion *nfs_mod)
+{
+ int status;
+
+ status = nfs_request_mount(mount_info->parsed, mount_info->mntfh);
+ if (status)
+ return ERR_PTR(status);
+
+ return nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
+}
+
struct dentry *nfs_try_mount(int flags, const char *dev_name,
struct nfs_mount_info *mount_info,
struct nfs_subversion *nfs_mod)
{
- int status;
struct nfs_server *server;
- if (mount_info->parsed->need_mount) {
- status = nfs_request_mount(mount_info->parsed, mount_info->mntfh);
- if (status)
- return ERR_PTR(status);
- }
+ if (mount_info->parsed->need_mount)
+ server = nfs_try_mount_request(mount_info, nfs_mod);
+ else
+ server = nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
- /* Get a volume representation */
- server = nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
if (IS_ERR(server))
return ERR_CAST(server);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/4] nfs: move server_authlist into nfs_try_mount_request
2013-06-27 19:54 [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn Jeff Layton
2013-06-27 19:54 ` [PATCH v4 1/4] nfs: refactor "need_mount" code out of nfs_try_mount Jeff Layton
@ 2013-06-27 19:54 ` Jeff Layton
2013-06-27 19:54 ` [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it Jeff Layton
2013-06-27 19:54 ` [PATCH v4 4/4] nfs: have NFSv3 try server-specified auth flavors in turn Jeff Layton
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2013-06-27 19:54 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs, chuck.lever, Weston.Adamson
In a later patch we're going to want to cycle over this list and attempt
to call ->create_server for each different flavor until one succeeds.
Move the list allocation to the stack of nfs_try_mount_request() and
pass a pointer to it and its length to nfs_request_mount().
Cc: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/super.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index afeee81..a0949f5 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1701,10 +1701,10 @@ out_err:
* corresponding to the provided path.
*/
static int nfs_request_mount(struct nfs_parsed_mount_data *args,
- struct nfs_fh *root_fh)
+ struct nfs_fh *root_fh,
+ rpc_authflavor_t *server_authlist,
+ unsigned int *server_authlist_len)
{
- rpc_authflavor_t server_authlist[NFS_MAX_SECFLAVORS];
- unsigned int server_authlist_len = ARRAY_SIZE(server_authlist);
struct nfs_mount_request request = {
.sap = (struct sockaddr *)
&args->mount_server.address,
@@ -1712,7 +1712,7 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
.protocol = args->mount_server.protocol,
.fh = root_fh,
.noresvport = args->flags & NFS_MOUNT_NORESVPORT,
- .auth_flav_len = &server_authlist_len,
+ .auth_flav_len = server_authlist_len,
.auth_flavs = server_authlist,
.net = args->net,
};
@@ -1763,8 +1763,12 @@ static struct nfs_server *nfs_try_mount_request(struct nfs_mount_info *mount_inf
struct nfs_subversion *nfs_mod)
{
int status;
+ struct nfs_parsed_mount_data *args = mount_info->parsed;
+ rpc_authflavor_t authlist[NFS_MAX_SECFLAVORS];
+ unsigned int authlist_len = ARRAY_SIZE(authlist);
- status = nfs_request_mount(mount_info->parsed, mount_info->mntfh);
+ status = nfs_request_mount(args, mount_info->mntfh, authlist,
+ &authlist_len);
if (status)
return ERR_PTR(status);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it
2013-06-27 19:54 [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn Jeff Layton
2013-06-27 19:54 ` [PATCH v4 1/4] nfs: refactor "need_mount" code out of nfs_try_mount Jeff Layton
2013-06-27 19:54 ` [PATCH v4 2/4] nfs: move server_authlist into nfs_try_mount_request Jeff Layton
@ 2013-06-27 19:54 ` Jeff Layton
2013-06-28 15:06 ` Chuck Lever
2013-06-27 19:54 ` [PATCH v4 4/4] nfs: have NFSv3 try server-specified auth flavors in turn Jeff Layton
3 siblings, 1 reply; 7+ messages in thread
From: Jeff Layton @ 2013-06-27 19:54 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs, chuck.lever, Weston.Adamson
Instead of handling this as a special case in the auth-selection code,
we can simply fake up an auth_flavs list when the server doesn't
provide it.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/mount_clnt.c | 18 +++++++++++++++++-
fs/nfs/super.c | 13 -------------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
index 91a6faf..0f925f3 100644
--- a/fs/nfs/mount_clnt.c
+++ b/fs/nfs/mount_clnt.c
@@ -139,7 +139,10 @@ struct mnt_fhstatus {
* nfs_mount - Obtain an NFS file handle for the given host and path
* @info: pointer to mount request arguments
*
- * Uses default timeout parameters specified by underlying transport.
+ * Uses default timeout parameters specified by underlying transport. On
+ * successful return, the auth_flavs list and auth_flav_len will be populated
+ * with the list from the server or a faked-up list if the server didn't
+ * provide one.
*/
int nfs_mount(struct nfs_mount_request *info)
{
@@ -195,6 +198,19 @@ int nfs_mount(struct nfs_mount_request *info)
dprintk("NFS: MNT request succeeded\n");
status = 0;
+ /*
+ * The NFSv2 MNT operation does not return a flavor list. Also, some
+ * NFSv3 servers (older Linux servers in particular) return an empty
+ * list.
+ *
+ * In that event fake up a list that just has RPC_AUTH_NULL in it since
+ * we have no way to know what the server actually supports.
+ */
+ if (info->version != NFS_MNT3_VERSION || *info->auth_flav_len == 0) {
+ dprintk("NFS: Faking up auth_flavs list\n");
+ info->auth_flavs[0] = RPC_AUTH_NULL;
+ *info->auth_flav_len = 1;
+ }
out:
return status;
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index a0949f5..ceb60c7 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1620,19 +1620,6 @@ static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
rpc_authflavor_t flavor;
/*
- * The NFSv2 MNT operation does not return a flavor list.
- */
- if (args->mount_server.version != NFS_MNT3_VERSION)
- goto out_default;
-
- /*
- * Certain releases of Linux's mountd return an empty
- * flavor list in some cases.
- */
- if (count == 0)
- goto out_default;
-
- /*
* If the sec= mount option is used, the specified flavor or AUTH_NULL
* must be in the list returned by the server.
*
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 4/4] nfs: have NFSv3 try server-specified auth flavors in turn
2013-06-27 19:54 [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn Jeff Layton
` (2 preceding siblings ...)
2013-06-27 19:54 ` [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it Jeff Layton
@ 2013-06-27 19:54 ` Jeff Layton
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2013-06-27 19:54 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs, chuck.lever, Weston.Adamson
The current scheme is to try and pick the auth flavor that the server
prefers. In some cases though, we may find that we're not actually
able to use that auth flavor later. For instance, the server may
prefer an AUTH_GSS flavor, but we may not be able to get GSSAPI creds.
The current code just gives up at that point. Change it instead to
try the ->create_server call using each of the different authflavors
in the server's list if one was not specified at mount time. Once
we have a successful ->create_server call, return the result. Only
give up and return error if all attempts fail.
Cc: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/super.c | 126 +++++++++++++++++++++++++++++++--------------------------
1 file changed, 69 insertions(+), 57 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ceb60c7..8d51101 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1608,16 +1608,13 @@ out_security_failure:
}
/*
- * Select a security flavor for this mount. The selected flavor
- * is planted in args->auth_flavors[0].
- *
- * Returns 0 on success, -EACCES on failure.
+ * Ensure that the specified authtype in args->auth_flavors[0] is supported by
+ * the server. Returns 0 if it's ok, and -EACCES if not.
*/
-static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
- struct nfs_mount_request *request)
+static int nfs_verify_authflavor(struct nfs_parsed_mount_data *args,
+ rpc_authflavor_t *server_authlist, unsigned int count)
{
- unsigned int i, count = *(request->auth_flav_len);
- rpc_authflavor_t flavor;
+ unsigned int i;
/*
* If the sec= mount option is used, the specified flavor or AUTH_NULL
@@ -1627,60 +1624,19 @@ static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
* means that the server will ignore the rpc creds, so any flavor
* can be used.
*/
- if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
- for (i = 0; i < count; i++) {
- if (args->auth_flavors[0] == request->auth_flavs[i] ||
- request->auth_flavs[i] == RPC_AUTH_NULL)
- goto out;
- }
- dfprintk(MOUNT, "NFS: auth flavor %d not supported by server\n",
- args->auth_flavors[0]);
- goto out_err;
- }
-
- /*
- * RFC 2623, section 2.7 suggests we SHOULD prefer the
- * flavor listed first. However, some servers list
- * AUTH_NULL first. Avoid ever choosing AUTH_NULL.
- */
for (i = 0; i < count; i++) {
- struct rpcsec_gss_info info;
-
- flavor = request->auth_flavs[i];
- switch (flavor) {
- case RPC_AUTH_UNIX:
- goto out_set;
- case RPC_AUTH_NULL:
- continue;
- default:
- if (rpcauth_get_gssinfo(flavor, &info) == 0)
- goto out_set;
- }
+ if (args->auth_flavors[0] == server_authlist[i] ||
+ server_authlist[i] == RPC_AUTH_NULL)
+ goto out;
}
- /*
- * As a last chance, see if the server list contains AUTH_NULL -
- * if it does, use the default flavor.
- */
- for (i = 0; i < count; i++) {
- if (request->auth_flavs[i] == RPC_AUTH_NULL)
- goto out_default;
- }
-
- dfprintk(MOUNT, "NFS: no auth flavors in common with server\n");
- goto out_err;
+ dfprintk(MOUNT, "NFS: auth flavor %u not supported by server\n",
+ args->auth_flavors[0]);
+ return -EACCES;
-out_default:
- /* use default if flavor not already set */
- flavor = (args->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) ?
- RPC_AUTH_UNIX : args->auth_flavors[0];
-out_set:
- args->auth_flavors[0] = flavor;
out:
- dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]);
+ dfprintk(MOUNT, "NFS: using auth flavor %u\n", args->auth_flavors[0]);
return 0;
-out_err:
- return -EACCES;
}
/*
@@ -1743,13 +1699,17 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args,
return status;
}
- return nfs_select_flavor(args, &request);
+ return 0;
}
static struct nfs_server *nfs_try_mount_request(struct nfs_mount_info *mount_info,
struct nfs_subversion *nfs_mod)
{
int status;
+ unsigned int i;
+ bool tried_auth_unix = false;
+ bool auth_null_in_list = false;
+ struct nfs_server *server = ERR_PTR(-EACCES);
struct nfs_parsed_mount_data *args = mount_info->parsed;
rpc_authflavor_t authlist[NFS_MAX_SECFLAVORS];
unsigned int authlist_len = ARRAY_SIZE(authlist);
@@ -1759,6 +1719,58 @@ static struct nfs_server *nfs_try_mount_request(struct nfs_mount_info *mount_inf
if (status)
return ERR_PTR(status);
+ /*
+ * Was a sec= authflavor specified in the options? First, verify
+ * whether the server supports it, and then just try to use it if so.
+ */
+ if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) {
+ status = nfs_verify_authflavor(args, authlist, authlist_len);
+ dfprintk(MOUNT, "NFS: using auth flavor %u\n", args->auth_flavors[0]);
+ if (status)
+ return ERR_PTR(status);
+ return nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
+ }
+
+ /*
+ * No sec= option was provided. RFC 2623, section 2.7 suggests we
+ * SHOULD prefer the flavor listed first. However, some servers list
+ * AUTH_NULL first. Avoid ever choosing AUTH_NULL.
+ */
+ for (i = 0; i < authlist_len; ++i) {
+ rpc_authflavor_t flavor;
+ struct rpcsec_gss_info info;
+
+ flavor = authlist[i];
+ switch (flavor) {
+ case RPC_AUTH_UNIX:
+ tried_auth_unix = true;
+ break;
+ case RPC_AUTH_NULL:
+ auth_null_in_list = true;
+ continue;
+ default:
+ if (rpcauth_get_gssinfo(flavor, &info) != 0)
+ continue;
+ /* Fallthrough */
+ }
+ dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor);
+ args->auth_flavors[0] = flavor;
+ server = nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
+ if (!IS_ERR(server))
+ return server;
+ }
+
+ /*
+ * Nothing we tried so far worked. At this point, give up if we've
+ * already tried AUTH_UNIX or if the server's list doesn't contain
+ * AUTH_NULL
+ */
+ if (tried_auth_unix || !auth_null_in_list)
+ return server;
+
+ /* Last chance! Try AUTH_UNIX */
+ dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", RPC_AUTH_UNIX);
+ args->auth_flavors[0] = RPC_AUTH_UNIX;
return nfs_mod->rpc_ops->create_server(mount_info, nfs_mod);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it
2013-06-27 19:54 ` [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it Jeff Layton
@ 2013-06-28 15:06 ` Chuck Lever
2013-06-28 15:18 ` Jeff Layton
0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2013-06-28 15:06 UTC (permalink / raw)
To: Jeff Layton; +Cc: trond.myklebust, linux-nfs, Weston.Adamson
On Jun 27, 2013, at 3:54 PM, Jeff Layton <jlayton@redhat.com> wrote:
> Instead of handling this as a special case in the auth-selection code,
> we can simply fake up an auth_flavs list when the server doesn't
> provide it.
The series looks like a reasonable approach. One nit picked below.
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/mount_clnt.c | 18 +++++++++++++++++-
> fs/nfs/super.c | 13 -------------
> 2 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> index 91a6faf..0f925f3 100644
> --- a/fs/nfs/mount_clnt.c
> +++ b/fs/nfs/mount_clnt.c
> @@ -139,7 +139,10 @@ struct mnt_fhstatus {
> * nfs_mount - Obtain an NFS file handle for the given host and path
> * @info: pointer to mount request arguments
> *
> - * Uses default timeout parameters specified by underlying transport.
> + * Uses default timeout parameters specified by underlying transport. On
> + * successful return, the auth_flavs list and auth_flav_len will be populated
> + * with the list from the server or a faked-up list if the server didn't
> + * provide one.
> */
> int nfs_mount(struct nfs_mount_request *info)
> {
> @@ -195,6 +198,19 @@ int nfs_mount(struct nfs_mount_request *info)
> dprintk("NFS: MNT request succeeded\n");
> status = 0;
>
> + /*
> + * The NFSv2 MNT operation does not return a flavor list. Also, some
> + * NFSv3 servers (older Linux servers in particular) return an empty
> + * list.
> + *
> + * In that event fake up a list that just has RPC_AUTH_NULL in it since
> + * we have no way to know what the server actually supports.
> + */
I see the checks are folded together again. I'm not going to win that battle, am I? ;-)
The comment repeats most of what the code already says. How about:
/*
* If the server didn't provide a flavor list, allow the
* client to try any flavor.
*/
Blue is my favorite color, by the way.
> + if (info->version != NFS_MNT3_VERSION || *info->auth_flav_len == 0) {
> + dprintk("NFS: Faking up auth_flavs list\n");
> + info->auth_flavs[0] = RPC_AUTH_NULL;
> + *info->auth_flav_len = 1;
> + }
> out:
> return status;
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index a0949f5..ceb60c7 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1620,19 +1620,6 @@ static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
> rpc_authflavor_t flavor;
>
> /*
> - * The NFSv2 MNT operation does not return a flavor list.
> - */
> - if (args->mount_server.version != NFS_MNT3_VERSION)
> - goto out_default;
> -
> - /*
> - * Certain releases of Linux's mountd return an empty
> - * flavor list in some cases.
> - */
> - if (count == 0)
> - goto out_default;
> -
> - /*
> * If the sec= mount option is used, the specified flavor or AUTH_NULL
> * must be in the list returned by the server.
> *
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it
2013-06-28 15:06 ` Chuck Lever
@ 2013-06-28 15:18 ` Jeff Layton
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2013-06-28 15:18 UTC (permalink / raw)
To: Chuck Lever; +Cc: trond.myklebust, linux-nfs, Weston.Adamson
On Fri, 28 Jun 2013 11:06:08 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Jun 27, 2013, at 3:54 PM, Jeff Layton <jlayton@redhat.com> wrote:
>
> > Instead of handling this as a special case in the auth-selection code,
> > we can simply fake up an auth_flavs list when the server doesn't
> > provide it.
>
> The series looks like a reasonable approach. One nit picked below.
>
> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
>
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/mount_clnt.c | 18 +++++++++++++++++-
> > fs/nfs/super.c | 13 -------------
> > 2 files changed, 17 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> > index 91a6faf..0f925f3 100644
> > --- a/fs/nfs/mount_clnt.c
> > +++ b/fs/nfs/mount_clnt.c
> > @@ -139,7 +139,10 @@ struct mnt_fhstatus {
> > * nfs_mount - Obtain an NFS file handle for the given host and path
> > * @info: pointer to mount request arguments
> > *
> > - * Uses default timeout parameters specified by underlying transport.
> > + * Uses default timeout parameters specified by underlying transport. On
> > + * successful return, the auth_flavs list and auth_flav_len will be populated
> > + * with the list from the server or a faked-up list if the server didn't
> > + * provide one.
> > */
> > int nfs_mount(struct nfs_mount_request *info)
> > {
> > @@ -195,6 +198,19 @@ int nfs_mount(struct nfs_mount_request *info)
> > dprintk("NFS: MNT request succeeded\n");
> > status = 0;
> >
> > + /*
> > + * The NFSv2 MNT operation does not return a flavor list. Also, some
> > + * NFSv3 servers (older Linux servers in particular) return an empty
> > + * list.
> > + *
> > + * In that event fake up a list that just has RPC_AUTH_NULL in it since
> > + * we have no way to know what the server actually supports.
> > + */
>
> I see the checks are folded together again. I'm not going to win that battle, am I? ;-)
>
Yeah, sorry...
In the other case it made sense since we just did a "goto".
Here, it's either fold the checks together, repeat the list fakeup code
twice, do some goto shenanigans, or stick the list fakeup in a separate
function.
This seemed like the lesser of evils.
> The comment repeats most of what the code already says. How about:
>
> /*
> * If the server didn't provide a flavor list, allow the
> * client to try any flavor.
> */
>
Sounds reasonable. I'll fix it up in my repo. I hate to repost it again
for a comment change, so maybe Trond can fix that up if he finds the
series otherwise acceptable?
> Blue is my favorite color, by the way.
>
> > + if (info->version != NFS_MNT3_VERSION || *info->auth_flav_len == 0) {
> > + dprintk("NFS: Faking up auth_flavs list\n");
> > + info->auth_flavs[0] = RPC_AUTH_NULL;
> > + *info->auth_flav_len = 1;
> > + }
> > out:
> > return status;
> >
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index a0949f5..ceb60c7 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -1620,19 +1620,6 @@ static int nfs_select_flavor(struct nfs_parsed_mount_data *args,
> > rpc_authflavor_t flavor;
> >
> > /*
> > - * The NFSv2 MNT operation does not return a flavor list.
> > - */
> > - if (args->mount_server.version != NFS_MNT3_VERSION)
> > - goto out_default;
> > -
> > - /*
> > - * Certain releases of Linux's mountd return an empty
> > - * flavor list in some cases.
> > - */
> > - if (count == 0)
> > - goto out_default;
> > -
> > - /*
> > * If the sec= mount option is used, the specified flavor or AUTH_NULL
> > * must be in the list returned by the server.
> > *
> > --
> > 1.8.1.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-28 15:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-27 19:54 [PATCH v4 0/4] nfs: teach NFSv3 mount code to try each authflavor in turn Jeff Layton
2013-06-27 19:54 ` [PATCH v4 1/4] nfs: refactor "need_mount" code out of nfs_try_mount Jeff Layton
2013-06-27 19:54 ` [PATCH v4 2/4] nfs: move server_authlist into nfs_try_mount_request Jeff Layton
2013-06-27 19:54 ` [PATCH v4 3/4] nfs: have nfs_mount fake up a auth_flavs list when the server didn't provide it Jeff Layton
2013-06-28 15:06 ` Chuck Lever
2013-06-28 15:18 ` Jeff Layton
2013-06-27 19:54 ` [PATCH v4 4/4] nfs: have NFSv3 try server-specified auth flavors in turn Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).