2013-02-21 22:15:17

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 0/2] NFS v4.2 support to both the server and client

This is the first attempt to added v4.2 support to both
the server and client. Basically the v4.1 code is reused,
but the minor version is needed to enable features like
label NFS.

Steve Dickson (2):
NFSv4.2: Added NFS v4.2 support to the NFS client
NFSDv4.2: Added NFS v4.2 support to the NFS server

fs/nfs/Kconfig | 11 ++++++++++-
fs/nfs/callback.c | 3 +++
fs/nfs/nfs4client.c | 5 +++++
fs/nfs/nfs4proc.c | 3 +++
fs/nfs/super.c | 7 ++++++-
fs/nfsd/nfs4xdr.c | 1 +
fs/nfsd/nfsd.h | 2 +-
include/linux/nfs4.h | 4 ++++
8 files changed, 33 insertions(+), 3 deletions(-)

--
1.8.1.2



2013-02-22 15:26:04

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFSDv4.2: Added NFS v4.2 support to the NFS server

On Thu, Feb 21, 2013 at 05:15:11PM -0500, Steve Dickson wrote:
> This enable NFSv4.2 support for the server. To enable this
> code do the following:
> echo "+4.2" >/proc/fs/nfsd/versions
>
> after the nfsd kernel module is loaded.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 1 +
> fs/nfsd/nfsd.h | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index d2ae8db..86be853 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1621,6 +1621,7 @@ struct nfsd4_minorversion_ops {
> static struct nfsd4_minorversion_ops nfsd4_minorversion[] = {
> [0] = { nfsd4_dec_ops, ARRAY_SIZE(nfsd4_dec_ops) },
> [1] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> + [2] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> };
>
> static __be32
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 26a457b..0e3ccd1 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -24,7 +24,7 @@
> /*
> * nfsd version
> */
> -#define NFSD_SUPPORTED_MINOR_VERSION 1
> +#define NFSD_SUPPORTED_MINOR_VERSION 2
> /*
> * Maximum blocksizes supported by daemon under various circumstances.
> */

Looks OK to me, except this should be behind a config for now.

On a grep for "minorversion" I notice two oversights, below: one just a
comment, one might result in getting the wrong minorversion on
callbacks.

--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 503e15e..b86cf07 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1665,7 +1665,7 @@ out_new:
status = nfserr_jukebox;
goto out;
}
- new->cl_minorversion = 1;
+ new->cl_minorversion = cstate->minorversion;

gen_clid(new);
add_to_unconfirmed(new, strhashval);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e036894..28d41ef 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -108,7 +108,7 @@ struct nfs4_cb_conn {
u32 cb_prog; /* used only in 4.0 case;
per-session otherwise */
u32 cb_ident; /* minorversion 0 only */
- struct svc_xprt *cb_xprt; /* minorversion 1 only */
+ struct svc_xprt *cb_xprt; /* minorversion >=1 only */
};

static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)

2013-02-22 15:13:59

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client

On Thu, Feb 21, 2013 at 05:15:10PM -0500, Steve Dickson wrote:
> This enable NFSv4.2 support. To enable this code the
> CONFIG_NFS_V4_2 Kconfig define needs to be set and
> the -o v4.2 mount option need to be used.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> fs/nfs/Kconfig | 11 ++++++++++-
> fs/nfs/callback.c | 3 +++
> fs/nfs/nfs4client.c | 5 +++++
> fs/nfs/nfs4proc.c | 3 +++
> fs/nfs/super.c | 7 ++++++-
> include/linux/nfs4.h | 4 ++++
> 6 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index 3861a1f..247db6d 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -104,6 +104,15 @@ config NFS_V4_1
>
> If unsure, say N.
>
> +config NFS_V4_2
> + bool "NFS client support for NFSv4.2"
> + depends on NFS_V4_1
> + help
> + This option enables support for minor version 1 of the NFSv4 protocol
> + (RFC 5661) in the kernel's NFS client.
> +
> + If unsure, say N.
> +
> config PNFS_FILE_LAYOUT
> tristate
> depends on NFS_V4_1
> @@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>
> config NFS_V4_SECURITY_LABEL
> bool "Provide Security Label support for NFSv4 client"
> - depends on NFS_V4 && SECURITY
> + depends on NFS_V4_2 && SECURITY
> help
>
> Say Y here if you want enable fine-grained security label attribute
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 5088b57..4058ec8 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -281,6 +281,9 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
> case 1:
> ret = nfs41_callback_up_net(serv, net);
> break;
> + case 2:
> + ret = nfs41_callback_up_net(serv, net);
> + break;
> default:
> printk(KERN_ERR "NFS: unknown callback version: %d\n",
> minorversion);
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 2e9779b..2987fd6 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -66,6 +66,11 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
> if (err)
> goto error;
>
> + if (cl_init->minorversion > NFS4_MAX_MINOR_VERSION) {
> + err = -EINVAL;
> + goto error;
> + }

Why wasn't this check needed before?

> +
> spin_lock_init(&clp->cl_lock);
> INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
> rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 40c1d1f..08fc8e2 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7136,6 +7136,9 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
> #if defined(CONFIG_NFS_V4_1)
> [1] = &nfs_v4_1_minor_ops,
> #endif
> +#if defined(CONFIG_NFS_V4_2)
> + [2] = &nfs_v4_1_minor_ops,

But then nfs_v4_minor_ops[2]->minor_version = 1.

I think you want to create another structure--just define an
nfs_v4_2_minor_ops field that's the same as nfs_v4_1_minor_ops except
for the minor_version field.

--b.

> +#endif
> };
>
> const struct inode_operations nfs4_dir_inode_operations = {
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 4e78f93..d35582c 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -269,7 +269,7 @@ static match_table_t nfs_local_lock_tokens = {
>
> enum {
> Opt_vers_2, Opt_vers_3, Opt_vers_4, Opt_vers_4_0,
> - Opt_vers_4_1,
> + Opt_vers_4_1, Opt_vers_4_2,
>
> Opt_vers_err
> };
> @@ -280,6 +280,7 @@ static match_table_t nfs_vers_tokens = {
> { Opt_vers_4, "4" },
> { Opt_vers_4_0, "4.0" },
> { Opt_vers_4_1, "4.1" },
> + { Opt_vers_4_2, "4.2" },
>
> { Opt_vers_err, NULL }
> };
> @@ -1143,6 +1144,10 @@ static int nfs_parse_version_string(char *string,
> mnt->version = 4;
> mnt->minorversion = 1;
> break;
> + case Opt_vers_4_2:
> + mnt->version = 4;
> + mnt->minorversion = 2;
> + break;
> default:
> return 0;
> }
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index aab8bd8..e9c040a 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -394,11 +394,15 @@ enum lock_type4 {
> #define NFS4_VERSION 4
> #define NFS4_MINOR_VERSION 0
>
> +#if defined(CONFIG_NFS_V4_2)
> +#define NFS4_MAX_MINOR_VERSION 2
> +#else
> #if defined(CONFIG_NFS_V4_1)
> #define NFS4_MAX_MINOR_VERSION 1
> #else
> #define NFS4_MAX_MINOR_VERSION 0
> #endif /* CONFIG_NFS_V4_1 */
> +#endif /* CONFIG_NFS_V4_2 */
>
> #define NFS4_DEBUG 1
>
> --
> 1.8.1.2
>

2013-02-21 22:22:51

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client

On Thu, 2013-02-21 at 17:15 -0500, Steve Dickson wrote:
> This enable NFSv4.2 support. To enable this code the
> CONFIG_NFS_V4_2 Kconfig define needs to be set and
> the -o v4.2 mount option need to be used.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> fs/nfs/Kconfig | 11 ++++++++++-
> fs/nfs/callback.c | 3 +++
> fs/nfs/nfs4client.c | 5 +++++
> fs/nfs/nfs4proc.c | 3 +++
> fs/nfs/super.c | 7 ++++++-
> include/linux/nfs4.h | 4 ++++
> 6 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index 3861a1f..247db6d 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -104,6 +104,15 @@ config NFS_V4_1
>
> If unsure, say N.
>
> +config NFS_V4_2
> + bool "NFS client support for NFSv4.2"
> + depends on NFS_V4_1
> + help
> + This option enables support for minor version 1 of the NFSv4 protocol
> + (RFC 5661) in the kernel's NFS client.
> +
> + If unsure, say N.
> +
> config PNFS_FILE_LAYOUT
> tristate
> depends on NFS_V4_1
> @@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>
> config NFS_V4_SECURITY_LABEL
> bool "Provide Security Label support for NFSv4 client"
> - depends on NFS_V4 && SECURITY
> + depends on NFS_V4_2 && SECURITY
> help

Just have NFS_V4_SECURITY_LABEL be automatically selected by NFS_V4_2.
Requiring users to manually select both makes little sense...


> Say Y here if you want enable fine-grained security label attribute
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 5088b57..4058ec8 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -281,6 +281,9 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
> case 1:

case 2:

> ret = nfs41_callback_up_net(serv, net);
> break;
> + case 2:
> + ret = nfs41_callback_up_net(serv, net);
> + break;
> default:
> printk(KERN_ERR "NFS: unknown callback version: %d\n",
> minorversion);
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 2e9779b..2987fd6 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -66,6 +66,11 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
> if (err)
> goto error;
>
> + if (cl_init->minorversion > NFS4_MAX_MINOR_VERSION) {
> + err = -EINVAL;
> + goto error;
> + }
> +
> spin_lock_init(&clp->cl_lock);
> INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
> rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 40c1d1f..08fc8e2 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7136,6 +7136,9 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
> #if defined(CONFIG_NFS_V4_1)
> [1] = &nfs_v4_1_minor_ops,
> #endif
> +#if defined(CONFIG_NFS_V4_2)
> + [2] = &nfs_v4_1_minor_ops,
> +#endif
> };
>
> const struct inode_operations nfs4_dir_inode_operations = {
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 4e78f93..d35582c 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -269,7 +269,7 @@ static match_table_t nfs_local_lock_tokens = {
>
> enum {
> Opt_vers_2, Opt_vers_3, Opt_vers_4, Opt_vers_4_0,
> - Opt_vers_4_1,
> + Opt_vers_4_1, Opt_vers_4_2,
>
> Opt_vers_err
> };
> @@ -280,6 +280,7 @@ static match_table_t nfs_vers_tokens = {
> { Opt_vers_4, "4" },
> { Opt_vers_4_0, "4.0" },
> { Opt_vers_4_1, "4.1" },
> + { Opt_vers_4_2, "4.2" },
>
> { Opt_vers_err, NULL }
> };
> @@ -1143,6 +1144,10 @@ static int nfs_parse_version_string(char *string,
> mnt->version = 4;
> mnt->minorversion = 1;
> break;
> + case Opt_vers_4_2:
> + mnt->version = 4;
> + mnt->minorversion = 2;
> + break;
> default:
> return 0;
> }
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index aab8bd8..e9c040a 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -394,11 +394,15 @@ enum lock_type4 {
> #define NFS4_VERSION 4
> #define NFS4_MINOR_VERSION 0
>
> +#if defined(CONFIG_NFS_V4_2)
> +#define NFS4_MAX_MINOR_VERSION 2
> +#else
> #if defined(CONFIG_NFS_V4_1)
> #define NFS4_MAX_MINOR_VERSION 1
> #else
> #define NFS4_MAX_MINOR_VERSION 0
> #endif /* CONFIG_NFS_V4_1 */
> +#endif /* CONFIG_NFS_V4_2 */
>
> #define NFS4_DEBUG 1
>

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-02-22 15:38:03

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFSDv4.2: Added NFS v4.2 support to the NFS server

On Fri, Feb 22, 2013 at 03:30:08PM +0000, Myklebust, Trond wrote:
> On Fri, 2013-02-22 at 10:26 -0500, J. Bruce Fields wrote:
> > On Thu, Feb 21, 2013 at 05:15:11PM -0500, Steve Dickson wrote:
> > > This enable NFSv4.2 support for the server. To enable this
> > > code do the following:
> > > echo "+4.2" >/proc/fs/nfsd/versions
> > >
> > > after the nfsd kernel module is loaded.
> > >
> > > Signed-off-by: Steve Dickson <[email protected]>
> > > ---
> > > fs/nfsd/nfs4xdr.c | 1 +
> > > fs/nfsd/nfsd.h | 2 +-
> > > 2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > index d2ae8db..86be853 100644
> > > --- a/fs/nfsd/nfs4xdr.c
> > > +++ b/fs/nfsd/nfs4xdr.c
> > > @@ -1621,6 +1621,7 @@ struct nfsd4_minorversion_ops {
> > > static struct nfsd4_minorversion_ops nfsd4_minorversion[] = {
> > > [0] = { nfsd4_dec_ops, ARRAY_SIZE(nfsd4_dec_ops) },
> > > [1] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> > > + [2] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> > > };
> > >
> > > static __be32
> > > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > > index 26a457b..0e3ccd1 100644
> > > --- a/fs/nfsd/nfsd.h
> > > +++ b/fs/nfsd/nfsd.h
> > > @@ -24,7 +24,7 @@
> > > /*
> > > * nfsd version
> > > */
> > > -#define NFSD_SUPPORTED_MINOR_VERSION 1
> > > +#define NFSD_SUPPORTED_MINOR_VERSION 2
> > > /*
> > > * Maximum blocksizes supported by daemon under various circumstances.
> > > */
> >
> > Looks OK to me, except this should be behind a config for now.
> >
> > On a grep for "minorversion" I notice two oversights, below: one just a
> > comment, one might result in getting the wrong minorversion on
> > callbacks.
> >
> > --b.
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 503e15e..b86cf07 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1665,7 +1665,7 @@ out_new:
> > status = nfserr_jukebox;
> > goto out;
> > }
> > - new->cl_minorversion = 1;
> > + new->cl_minorversion = cstate->minorversion;
> >
> > gen_clid(new);
> > add_to_unconfirmed(new, strhashval);
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index e036894..28d41ef 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -108,7 +108,7 @@ struct nfs4_cb_conn {
> > u32 cb_prog; /* used only in 4.0 case;
> > per-session otherwise */
> > u32 cb_ident; /* minorversion 0 only */
> > - struct svc_xprt *cb_xprt; /* minorversion 1 only */
> > + struct svc_xprt *cb_xprt; /* minorversion >=1 only */
>
> Tagging things as being particlar to a give minorversion means that
> we'll never stop having to change the comments.

I don't see how "minorversion >=1 only" is likely to become untrue. Are
we really expecting to remove the session back channel in a future minor
version?

> Why not just label it as being the session back channel?

I don't really care. Happy to take alternate text if you want to
suggest something.

--b.

2013-02-21 22:15:17

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client

This enable NFSv4.2 support. To enable this code the
CONFIG_NFS_V4_2 Kconfig define needs to be set and
the -o v4.2 mount option need to be used.

Signed-off-by: Steve Dickson <[email protected]>
---
fs/nfs/Kconfig | 11 ++++++++++-
fs/nfs/callback.c | 3 +++
fs/nfs/nfs4client.c | 5 +++++
fs/nfs/nfs4proc.c | 3 +++
fs/nfs/super.c | 7 ++++++-
include/linux/nfs4.h | 4 ++++
6 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 3861a1f..247db6d 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -104,6 +104,15 @@ config NFS_V4_1

If unsure, say N.

+config NFS_V4_2
+ bool "NFS client support for NFSv4.2"
+ depends on NFS_V4_1
+ help
+ This option enables support for minor version 1 of the NFSv4 protocol
+ (RFC 5661) in the kernel's NFS client.
+
+ If unsure, say N.
+
config PNFS_FILE_LAYOUT
tristate
depends on NFS_V4_1
@@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN

config NFS_V4_SECURITY_LABEL
bool "Provide Security Label support for NFSv4 client"
- depends on NFS_V4 && SECURITY
+ depends on NFS_V4_2 && SECURITY
help

Say Y here if you want enable fine-grained security label attribute
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 5088b57..4058ec8 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -281,6 +281,9 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
case 1:
ret = nfs41_callback_up_net(serv, net);
break;
+ case 2:
+ ret = nfs41_callback_up_net(serv, net);
+ break;
default:
printk(KERN_ERR "NFS: unknown callback version: %d\n",
minorversion);
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 2e9779b..2987fd6 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -66,6 +66,11 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
if (err)
goto error;

+ if (cl_init->minorversion > NFS4_MAX_MINOR_VERSION) {
+ err = -EINVAL;
+ goto error;
+ }
+
spin_lock_init(&clp->cl_lock);
INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 40c1d1f..08fc8e2 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7136,6 +7136,9 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
#if defined(CONFIG_NFS_V4_1)
[1] = &nfs_v4_1_minor_ops,
#endif
+#if defined(CONFIG_NFS_V4_2)
+ [2] = &nfs_v4_1_minor_ops,
+#endif
};

const struct inode_operations nfs4_dir_inode_operations = {
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 4e78f93..d35582c 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -269,7 +269,7 @@ static match_table_t nfs_local_lock_tokens = {

enum {
Opt_vers_2, Opt_vers_3, Opt_vers_4, Opt_vers_4_0,
- Opt_vers_4_1,
+ Opt_vers_4_1, Opt_vers_4_2,

Opt_vers_err
};
@@ -280,6 +280,7 @@ static match_table_t nfs_vers_tokens = {
{ Opt_vers_4, "4" },
{ Opt_vers_4_0, "4.0" },
{ Opt_vers_4_1, "4.1" },
+ { Opt_vers_4_2, "4.2" },

{ Opt_vers_err, NULL }
};
@@ -1143,6 +1144,10 @@ static int nfs_parse_version_string(char *string,
mnt->version = 4;
mnt->minorversion = 1;
break;
+ case Opt_vers_4_2:
+ mnt->version = 4;
+ mnt->minorversion = 2;
+ break;
default:
return 0;
}
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index aab8bd8..e9c040a 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -394,11 +394,15 @@ enum lock_type4 {
#define NFS4_VERSION 4
#define NFS4_MINOR_VERSION 0

+#if defined(CONFIG_NFS_V4_2)
+#define NFS4_MAX_MINOR_VERSION 2
+#else
#if defined(CONFIG_NFS_V4_1)
#define NFS4_MAX_MINOR_VERSION 1
#else
#define NFS4_MAX_MINOR_VERSION 0
#endif /* CONFIG_NFS_V4_1 */
+#endif /* CONFIG_NFS_V4_2 */

#define NFS4_DEBUG 1

--
1.8.1.2


2013-02-22 16:38:27

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client



On 22/02/13 10:13, J. Bruce Fields wrote:
> On Thu, Feb 21, 2013 at 05:15:10PM -0500, Steve Dickson wrote:
>> This enable NFSv4.2 support. To enable this code the
>> CONFIG_NFS_V4_2 Kconfig define needs to be set and
>> the -o v4.2 mount option need to be used.
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> fs/nfs/Kconfig | 11 ++++++++++-
>> fs/nfs/callback.c | 3 +++
>> fs/nfs/nfs4client.c | 5 +++++
>> fs/nfs/nfs4proc.c | 3 +++
>> fs/nfs/super.c | 7 ++++++-
>> include/linux/nfs4.h | 4 ++++
>> 6 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
>> index 3861a1f..247db6d 100644
>> --- a/fs/nfs/Kconfig
>> +++ b/fs/nfs/Kconfig
>> @@ -104,6 +104,15 @@ config NFS_V4_1
>>
>> If unsure, say N.
>>
>> +config NFS_V4_2
>> + bool "NFS client support for NFSv4.2"
>> + depends on NFS_V4_1
>> + help
>> + This option enables support for minor version 1 of the NFSv4 protocol
>> + (RFC 5661) in the kernel's NFS client.
>> +
>> + If unsure, say N.
>> +
>> config PNFS_FILE_LAYOUT
>> tristate
>> depends on NFS_V4_1
>> @@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>>
>> config NFS_V4_SECURITY_LABEL
>> bool "Provide Security Label support for NFSv4 client"
>> - depends on NFS_V4 && SECURITY
>> + depends on NFS_V4_2 && SECURITY
>> help
>>
>> Say Y here if you want enable fine-grained security label attribute
>> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
>> index 5088b57..4058ec8 100644
>> --- a/fs/nfs/callback.c
>> +++ b/fs/nfs/callback.c
>> @@ -281,6 +281,9 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
>> case 1:
>> ret = nfs41_callback_up_net(serv, net);
>> break;
>> + case 2:
>> + ret = nfs41_callback_up_net(serv, net);
>> + break;
>> default:
>> printk(KERN_ERR "NFS: unknown callback version: %d\n",
>> minorversion);
>> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
>> index 2e9779b..2987fd6 100644
>> --- a/fs/nfs/nfs4client.c
>> +++ b/fs/nfs/nfs4client.c
>> @@ -66,6 +66,11 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
>> if (err)
>> goto error;
>>
>> + if (cl_init->minorversion > NFS4_MAX_MINOR_VERSION) {
>> + err = -EINVAL;
>> + goto error;
>> + }
>
> Why wasn't this check needed before?
It was not needed because the checks in nfs_parse_version_string caught
the mismatch minor version... but since minor version 2 is no longer
a mismatch and those checks in nfs_parse_version_string are not
covered by an ifdef this check is now needed...

>
>> +
>> spin_lock_init(&clp->cl_lock);
>> INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
>> rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 40c1d1f..08fc8e2 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -7136,6 +7136,9 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
>> #if defined(CONFIG_NFS_V4_1)
>> [1] = &nfs_v4_1_minor_ops,
>> #endif
>> +#if defined(CONFIG_NFS_V4_2)
>> + [2] = &nfs_v4_1_minor_ops,
>
> But then nfs_v4_minor_ops[2]->minor_version = 1.
>
> I think you want to create another structure--just define an
> nfs_v4_2_minor_ops field that's the same as nfs_v4_1_minor_ops except
> for the minor_version field.
That's how I started out:

#if defined(CONFIG_NFS_V4_2)
static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
.minor_version = 2,
.call_sync = nfs4_call_sync_sequence,
.match_stateid = nfs41_match_stateid,
.find_root_sec = nfs41_find_root_sec,
.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
.state_renewal_ops = &nfs41_state_renewal_ops,
};
#endif

but it just seem like such a big waste of space... I could switch back...

Trond??

steved.

>
> --b.
>
>> +#endif
>> };
>>
>> const struct inode_operations nfs4_dir_inode_operations = {
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index 4e78f93..d35582c 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -269,7 +269,7 @@ static match_table_t nfs_local_lock_tokens = {
>>
>> enum {
>> Opt_vers_2, Opt_vers_3, Opt_vers_4, Opt_vers_4_0,
>> - Opt_vers_4_1,
>> + Opt_vers_4_1, Opt_vers_4_2,
>>
>> Opt_vers_err
>> };
>> @@ -280,6 +280,7 @@ static match_table_t nfs_vers_tokens = {
>> { Opt_vers_4, "4" },
>> { Opt_vers_4_0, "4.0" },
>> { Opt_vers_4_1, "4.1" },
>> + { Opt_vers_4_2, "4.2" },
>>
>> { Opt_vers_err, NULL }
>> };
>> @@ -1143,6 +1144,10 @@ static int nfs_parse_version_string(char *string,
>> mnt->version = 4;
>> mnt->minorversion = 1;
>> break;
>> + case Opt_vers_4_2:
>> + mnt->version = 4;
>> + mnt->minorversion = 2;
>> + break;
>> default:
>> return 0;
>> }
>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>> index aab8bd8..e9c040a 100644
>> --- a/include/linux/nfs4.h
>> +++ b/include/linux/nfs4.h
>> @@ -394,11 +394,15 @@ enum lock_type4 {
>> #define NFS4_VERSION 4
>> #define NFS4_MINOR_VERSION 0
>>
>> +#if defined(CONFIG_NFS_V4_2)
>> +#define NFS4_MAX_MINOR_VERSION 2
>> +#else
>> #if defined(CONFIG_NFS_V4_1)
>> #define NFS4_MAX_MINOR_VERSION 1
>> #else
>> #define NFS4_MAX_MINOR_VERSION 0
>> #endif /* CONFIG_NFS_V4_1 */
>> +#endif /* CONFIG_NFS_V4_2 */
>>
>> #define NFS4_DEBUG 1
>>
>> --
>> 1.8.1.2
>>

2013-02-21 23:13:42

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client



On 21/02/13 17:22, Myklebust, Trond wrote:
> On Thu, 2013-02-21 at 17:15 -0500, Steve Dickson wrote:
>> > This enable NFSv4.2 support. To enable this code the
>> > CONFIG_NFS_V4_2 Kconfig define needs to be set and
>> > the -o v4.2 mount option need to be used.
>> >
>> > Signed-off-by: Steve Dickson <[email protected]>
>> > ---
>> > fs/nfs/Kconfig | 11 ++++++++++-
>> > fs/nfs/callback.c | 3 +++
>> > fs/nfs/nfs4client.c | 5 +++++
>> > fs/nfs/nfs4proc.c | 3 +++
>> > fs/nfs/super.c | 7 ++++++-
>> > include/linux/nfs4.h | 4 ++++
>> > 6 files changed, 31 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
>> > index 3861a1f..247db6d 100644
>> > --- a/fs/nfs/Kconfig
>> > +++ b/fs/nfs/Kconfig
>> > @@ -104,6 +104,15 @@ config NFS_V4_1
>> >
>> > If unsure, say N.
>> >
>> > +config NFS_V4_2
>> > + bool "NFS client support for NFSv4.2"
>> > + depends on NFS_V4_1
>> > + help
>> > + This option enables support for minor version 1 of the NFSv4 protocol
>> > + (RFC 5661) in the kernel's NFS client.
>> > +
>> > + If unsure, say N.
>> > +
>> > config PNFS_FILE_LAYOUT
>> > tristate
>> > depends on NFS_V4_1
>> > @@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>> >
>> > config NFS_V4_SECURITY_LABEL
>> > bool "Provide Security Label support for NFSv4 client"
>> > - depends on NFS_V4 && SECURITY
>> > + depends on NFS_V4_2 && SECURITY
>> > help
> Just have NFS_V4_SECURITY_LABEL be automatically selected by NFS_V4_2.
> Requiring users to manually select both makes little sense...
>
>
Yeah... that make sense...

steved.

2013-02-21 22:15:17

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/2] NFSDv4.2: Added NFS v4.2 support to the NFS server

This enable NFSv4.2 support for the server. To enable this
code do the following:
echo "+4.2" >/proc/fs/nfsd/versions

after the nfsd kernel module is loaded.

Signed-off-by: Steve Dickson <[email protected]>
---
fs/nfsd/nfs4xdr.c | 1 +
fs/nfsd/nfsd.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index d2ae8db..86be853 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1621,6 +1621,7 @@ struct nfsd4_minorversion_ops {
static struct nfsd4_minorversion_ops nfsd4_minorversion[] = {
[0] = { nfsd4_dec_ops, ARRAY_SIZE(nfsd4_dec_ops) },
[1] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
+ [2] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
};

static __be32
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 26a457b..0e3ccd1 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -24,7 +24,7 @@
/*
* nfsd version
*/
-#define NFSD_SUPPORTED_MINOR_VERSION 1
+#define NFSD_SUPPORTED_MINOR_VERSION 2
/*
* Maximum blocksizes supported by daemon under various circumstances.
*/
--
1.8.1.2


2013-02-22 16:59:00

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv4.2: Added NFS v4.2 support to the NFS client

On Fri, Feb 22, 2013 at 11:38:25AM -0500, Steve Dickson wrote:
>
>
> On 22/02/13 10:13, J. Bruce Fields wrote:
> > On Thu, Feb 21, 2013 at 05:15:10PM -0500, Steve Dickson wrote:
> >> This enable NFSv4.2 support. To enable this code the
> >> CONFIG_NFS_V4_2 Kconfig define needs to be set and
> >> the -o v4.2 mount option need to be used.
> >>
> >> Signed-off-by: Steve Dickson <[email protected]>
> >> ---
> >> fs/nfs/Kconfig | 11 ++++++++++-
> >> fs/nfs/callback.c | 3 +++
> >> fs/nfs/nfs4client.c | 5 +++++
> >> fs/nfs/nfs4proc.c | 3 +++
> >> fs/nfs/super.c | 7 ++++++-
> >> include/linux/nfs4.h | 4 ++++
> >> 6 files changed, 31 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> >> index 3861a1f..247db6d 100644
> >> --- a/fs/nfs/Kconfig
> >> +++ b/fs/nfs/Kconfig
> >> @@ -104,6 +104,15 @@ config NFS_V4_1
> >>
> >> If unsure, say N.
> >>
> >> +config NFS_V4_2
> >> + bool "NFS client support for NFSv4.2"
> >> + depends on NFS_V4_1
> >> + help
> >> + This option enables support for minor version 1 of the NFSv4 protocol
> >> + (RFC 5661) in the kernel's NFS client.
> >> +
> >> + If unsure, say N.
> >> +
> >> config PNFS_FILE_LAYOUT
> >> tristate
> >> depends on NFS_V4_1
> >> @@ -133,7 +142,7 @@ config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
> >>
> >> config NFS_V4_SECURITY_LABEL
> >> bool "Provide Security Label support for NFSv4 client"
> >> - depends on NFS_V4 && SECURITY
> >> + depends on NFS_V4_2 && SECURITY
> >> help
> >>
> >> Say Y here if you want enable fine-grained security label attribute
> >> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> >> index 5088b57..4058ec8 100644
> >> --- a/fs/nfs/callback.c
> >> +++ b/fs/nfs/callback.c
> >> @@ -281,6 +281,9 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
> >> case 1:
> >> ret = nfs41_callback_up_net(serv, net);
> >> break;
> >> + case 2:
> >> + ret = nfs41_callback_up_net(serv, net);
> >> + break;
> >> default:
> >> printk(KERN_ERR "NFS: unknown callback version: %d\n",
> >> minorversion);
> >> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> >> index 2e9779b..2987fd6 100644
> >> --- a/fs/nfs/nfs4client.c
> >> +++ b/fs/nfs/nfs4client.c
> >> @@ -66,6 +66,11 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
> >> if (err)
> >> goto error;
> >>
> >> + if (cl_init->minorversion > NFS4_MAX_MINOR_VERSION) {
> >> + err = -EINVAL;
> >> + goto error;
> >> + }
> >
> > Why wasn't this check needed before?
> It was not needed because the checks in nfs_parse_version_string caught
> the mismatch minor version... but since minor version 2 is no longer
> a mismatch and those checks in nfs_parse_version_string are not
> covered by an ifdef this check is now needed...

NFSv4.1 could be configured out too. How did it manage to return an
error for minorversion == 1 in the !CONFIG_NFS_V4_1 case before?

> >> +
> >> spin_lock_init(&clp->cl_lock);
> >> INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
> >> rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index 40c1d1f..08fc8e2 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -7136,6 +7136,9 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
> >> #if defined(CONFIG_NFS_V4_1)
> >> [1] = &nfs_v4_1_minor_ops,
> >> #endif
> >> +#if defined(CONFIG_NFS_V4_2)
> >> + [2] = &nfs_v4_1_minor_ops,
> >
> > But then nfs_v4_minor_ops[2]->minor_version = 1.
> >
> > I think you want to create another structure--just define an
> > nfs_v4_2_minor_ops field that's the same as nfs_v4_1_minor_ops except
> > for the minor_version field.
> That's how I started out:
>
> #if defined(CONFIG_NFS_V4_2)
> static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
> .minor_version = 2,
> .call_sync = nfs4_call_sync_sequence,
> .match_stateid = nfs41_match_stateid,
> .find_root_sec = nfs41_find_root_sec,
> .reboot_recovery_ops = &nfs41_reboot_recovery_ops,
> .nograce_recovery_ops = &nfs41_nograce_recovery_ops,
> .state_renewal_ops = &nfs41_state_renewal_ops,
> };
> #endif
>
> but it just seem like such a big waste of space...

Correctness first....

--b.

> I could switch back...

2013-02-22 15:30:12

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFSDv4.2: Added NFS v4.2 support to the NFS server

On Fri, 2013-02-22 at 10:26 -0500, J. Bruce Fields wrote:
> On Thu, Feb 21, 2013 at 05:15:11PM -0500, Steve Dickson wrote:
> > This enable NFSv4.2 support for the server. To enable this
> > code do the following:
> > echo "+4.2" >/proc/fs/nfsd/versions
> >
> > after the nfsd kernel module is loaded.
> >
> > Signed-off-by: Steve Dickson <[email protected]>
> > ---
> > fs/nfsd/nfs4xdr.c | 1 +
> > fs/nfsd/nfsd.h | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index d2ae8db..86be853 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -1621,6 +1621,7 @@ struct nfsd4_minorversion_ops {
> > static struct nfsd4_minorversion_ops nfsd4_minorversion[] = {
> > [0] = { nfsd4_dec_ops, ARRAY_SIZE(nfsd4_dec_ops) },
> > [1] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> > + [2] = { nfsd41_dec_ops, ARRAY_SIZE(nfsd41_dec_ops) },
> > };
> >
> > static __be32
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 26a457b..0e3ccd1 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -24,7 +24,7 @@
> > /*
> > * nfsd version
> > */
> > -#define NFSD_SUPPORTED_MINOR_VERSION 1
> > +#define NFSD_SUPPORTED_MINOR_VERSION 2
> > /*
> > * Maximum blocksizes supported by daemon under various circumstances.
> > */
>
> Looks OK to me, except this should be behind a config for now.
>
> On a grep for "minorversion" I notice two oversights, below: one just a
> comment, one might result in getting the wrong minorversion on
> callbacks.
>
> --b.
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 503e15e..b86cf07 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1665,7 +1665,7 @@ out_new:
> status = nfserr_jukebox;
> goto out;
> }
> - new->cl_minorversion = 1;
> + new->cl_minorversion = cstate->minorversion;
>
> gen_clid(new);
> add_to_unconfirmed(new, strhashval);
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index e036894..28d41ef 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -108,7 +108,7 @@ struct nfs4_cb_conn {
> u32 cb_prog; /* used only in 4.0 case;
> per-session otherwise */
> u32 cb_ident; /* minorversion 0 only */
> - struct svc_xprt *cb_xprt; /* minorversion 1 only */
> + struct svc_xprt *cb_xprt; /* minorversion >=1 only */

Tagging things as being particlar to a give minorversion means that
we'll never stop having to change the comments. Why not just label it as
being the session back channel?

> };
>
> static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s)

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com