All of lore.kernel.org
 help / color / mirror / Atom feed
* 4.1 secinfo patches
@ 2010-12-17 19:01 J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 1/3] nfsd4: 4.1 SECINFO should consume filehandle J. Bruce Fields
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: J. Bruce Fields @ 2010-12-17 19:01 UTC (permalink / raw)
  To: linux-nfs

The following patches implement secinfo_no_name and update secinfo to
meet a 4.1 requirement.

I'll apply them for 2.6.38 if nobody sees a problem.

They look simple enough.  But I don't have a test for them.  So they're
probably wrong.  Two pynfs41 tests would do it:

	- send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
	  legal result.
	- send PUTROOTFH+SECINFO+GETFH, and/or
	  PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
	  NOFILEHANDLE.

I'll get around to it some day if nobody volunteers.

There are plenty of little things like this that could use doing or
testing before server-side 4.1 is correct; see:

	http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues

--b.

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/3] nfsd4: 4.1 SECINFO should consume filehandle
  2010-12-17 19:01 4.1 secinfo patches J. Bruce Fields
@ 2010-12-17 19:01 ` J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 2/3] nfsd4: move guts of nfsd4_lookupp into helper J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 3/3] nfsd4: implement secinfo_no_name J. Bruce Fields
  2 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2010-12-17 19:01 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

See the referenced spec language; an attempt by a 4.1 client to use the
current filehandle after a secinfo call should result in a NOFILEHANDLE
error.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0cdfd02..bad4bf8 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -769,6 +769,9 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	} else
 		secinfo->si_exp = exp;
 	dput(dentry);
+	if (cstate->minorversion)
+		/* See rfc 5661 section 2.6.3.1.1.8 */
+		fh_put(&cstate->current_fh);
 	return err;
 }
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/3] nfsd4: move guts of nfsd4_lookupp into helper
  2010-12-17 19:01 4.1 secinfo patches J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 1/3] nfsd4: 4.1 SECINFO should consume filehandle J. Bruce Fields
@ 2010-12-17 19:01 ` J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 3/3] nfsd4: implement secinfo_no_name J. Bruce Fields
  2 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2010-12-17 19:01 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

We'll reuse this code in secinfo_no_name.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index bad4bf8..095431a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -604,9 +604,7 @@ nfsd4_link(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	return status;
 }
 
-static __be32
-nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
-	      void *arg)
+static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
 {
 	struct svc_fh tmp_fh;
 	__be32 ret;
@@ -615,13 +613,19 @@ nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	ret = exp_pseudoroot(rqstp, &tmp_fh);
 	if (ret)
 		return ret;
-	if (tmp_fh.fh_dentry == cstate->current_fh.fh_dentry) {
+	if (tmp_fh.fh_dentry == fh->fh_dentry) {
 		fh_put(&tmp_fh);
 		return nfserr_noent;
 	}
 	fh_put(&tmp_fh);
-	return nfsd_lookup(rqstp, &cstate->current_fh,
-			   "..", 2, &cstate->current_fh);
+	return nfsd_lookup(rqstp, fh, "..", 2, fh);
+}
+
+static __be32
+nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	      void *arg)
+{
+	return nfsd4_do_lookupp(rqstp, &cstate->current_fh);
 }
 
 static __be32
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 3/3] nfsd4: implement secinfo_no_name
  2010-12-17 19:01 4.1 secinfo patches J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 1/3] nfsd4: 4.1 SECINFO should consume filehandle J. Bruce Fields
  2010-12-17 19:01 ` [PATCH 2/3] nfsd4: move guts of nfsd4_lookupp into helper J. Bruce Fields
@ 2010-12-17 19:01 ` J. Bruce Fields
  2010-12-27  6:29   ` Mi Jinlong
  2 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2010-12-17 19:01 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

Implementation of this operation is mandatory for NFSv4.1.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c   |   27 +++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c    |   15 +++++++++++++--
 fs/nfsd/xdr4.h       |    5 +++++
 include/linux/nfs4.h |    3 +++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 095431a..f80c399 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -780,6 +780,29 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 }
 
 static __be32
+nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	      struct nfsd4_secinfo_no_name *sin)
+{
+	__be32 err;
+
+	switch (sin->sin_style) {
+	case NFS4_SECINFO_STYLE4_CURRENT_FH:
+		break;
+	case NFS4_SECINFO_STYLE4_PARENT:
+		err = nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+		if (err)
+			return err;
+		break;
+	default:
+		return nfserr_inval;
+	}
+	exp_get(cstate->current_fh.fh_export);
+	sin->sin_exp = cstate->current_fh.fh_export;
+	fh_put(&cstate->current_fh);
+	return nfs_ok;
+}
+
+static __be32
 nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	      struct nfsd4_setattr *setattr)
 {
@@ -1327,6 +1350,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = ALLOWED_WITHOUT_FH,
 		.op_name = "OP_RECLAIM_COMPLETE",
 	},
+	[OP_SECINFO_NO_NAME] = {
+		.op_func = (nfsd4op_func)nfsd4_secinfo_no_name,
+		.op_name = "OP_SECINFO_NO_NAME",
+	},
 };
 
 static const char *nfsd4_op_name(unsigned opnum)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 71d7d33..b543b24 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -847,6 +847,17 @@ nfsd4_decode_secinfo(struct nfsd4_compoundargs *argp,
 }
 
 static __be32
+nfsd4_decode_secinfo_no_name(struct nfsd4_compoundargs *argp,
+		     struct nfsd4_secinfo_no_name *sin)
+{
+	DECODE_HEAD;
+
+	READ_BUF(4);
+	READ32(sin->sin_style);
+	DECODE_TAIL;
+}
+
+static __be32
 nfsd4_decode_setattr(struct nfsd4_compoundargs *argp, struct nfsd4_setattr *setattr)
 {
 	__be32 status;
@@ -1358,7 +1369,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
 	[OP_LAYOUTCOMMIT]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTGET]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_LAYOUTRETURN]	= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_secinfo_no_name,
 	[OP_SEQUENCE]		= (nfsd4_dec)nfsd4_decode_sequence,
 	[OP_SET_SSV]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
@@ -3162,7 +3173,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_LAYOUTCOMMIT]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_LAYOUTGET]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_LAYOUTRETURN]	= (nfsd4_enc)nfsd4_encode_noop,
-	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo,
 	[OP_SEQUENCE]		= (nfsd4_enc)nfsd4_encode_sequence,
 	[OP_SET_SSV]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 60fce3d..799c30c 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -311,6 +311,11 @@ struct nfsd4_secinfo {
 	struct svc_export *si_exp;			/* response */
 };
 
+struct nfsd4_secinfo_no_name {
+	u32 sin_style;					/* request */
+	struct svc_export *sin_exp;			/* response */
+};
+
 struct nfsd4_setattr {
 	stateid_t	sa_stateid;         /* request */
 	u32		sa_bmval[3];        /* request */
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 4925b22..26afa30 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -136,6 +136,9 @@
 #define SEQ4_STATUS_CB_PATH_DOWN_SESSION	0x00000200
 #define SEQ4_STATUS_BACKCHANNEL_FAULT		0x00000400
 
+#define NFS4_SECINFO_STYLE4_CURRENT_FH	0
+#define NFS4_SECINFO_STYLE4_PARENT	1
+
 #define NFS4_MAX_UINT64	(~(u64)0)
 
 /* An NFS4 sessions server must support at least NFS4_MAX_OPS operations.
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2010-12-17 19:01 ` [PATCH 3/3] nfsd4: implement secinfo_no_name J. Bruce Fields
@ 2010-12-27  6:29   ` Mi Jinlong
  2010-12-29 18:56     ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Mi Jinlong @ 2010-12-27  6:29 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs



J. Bruce Fields 写道:
> Implementation of this operation is mandatory for NFSv4.1.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfsd/nfs4proc.c   |   27 +++++++++++++++++++++++++++
>  fs/nfsd/nfs4xdr.c    |   15 +++++++++++++--
>  fs/nfsd/xdr4.h       |    5 +++++
>  include/linux/nfs4.h |    3 +++
>  4 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 095431a..f80c399 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -780,6 +780,29 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  }
>  
>  static __be32
> +nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> +	      struct nfsd4_secinfo_no_name *sin)
> +{
> +	__be32 err;
> +
> +	switch (sin->sin_style) {
> +	case NFS4_SECINFO_STYLE4_CURRENT_FH:
> +		break;
> +	case NFS4_SECINFO_STYLE4_PARENT:
> +		err = nfsd4_do_lookupp(rqstp, &cstate->current_fh);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return nfserr_inval;
> +	}
> +	exp_get(cstate->current_fh.fh_export);
> +	sin->sin_exp = cstate->current_fh.fh_export;
> +	fh_put(&cstate->current_fh);
> +	return nfs_ok;
> +}
> +
> +static __be32
>  nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	      struct nfsd4_setattr *setattr)
>  {
> @@ -1327,6 +1350,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
>  		.op_flags = ALLOWED_WITHOUT_FH,
>  		.op_name = "OP_RECLAIM_COMPLETE",
>  	},
> +	[OP_SECINFO_NO_NAME] = {
> +		.op_func = (nfsd4op_func)nfsd4_secinfo_no_name,
> +		.op_name = "OP_SECINFO_NO_NAME",
> +	},
>  };
>  
>  static const char *nfsd4_op_name(unsigned opnum)
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 71d7d33..b543b24 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -847,6 +847,17 @@ nfsd4_decode_secinfo(struct nfsd4_compoundargs *argp,
>  }
>  
>  static __be32
> +nfsd4_decode_secinfo_no_name(struct nfsd4_compoundargs *argp,
> +		     struct nfsd4_secinfo_no_name *sin)
> +{
> +	DECODE_HEAD;
> +
> +	READ_BUF(4);
> +	READ32(sin->sin_style);
> +	DECODE_TAIL;
> +}
> +
> +static __be32
>  nfsd4_decode_setattr(struct nfsd4_compoundargs *argp, struct nfsd4_setattr *setattr)
>  {
>  	__be32 status;
> @@ -1358,7 +1369,7 @@ static nfsd4_dec nfsd41_dec_ops[] = {
>  	[OP_LAYOUTCOMMIT]	= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_LAYOUTGET]		= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_LAYOUTRETURN]	= (nfsd4_dec)nfsd4_decode_notsupp,
> -	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_notsupp,
> +	[OP_SECINFO_NO_NAME]	= (nfsd4_dec)nfsd4_decode_secinfo_no_name,
>  	[OP_SEQUENCE]		= (nfsd4_dec)nfsd4_decode_sequence,
>  	[OP_SET_SSV]		= (nfsd4_dec)nfsd4_decode_notsupp,
>  	[OP_TEST_STATEID]	= (nfsd4_dec)nfsd4_decode_notsupp,
> @@ -3162,7 +3173,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>  	[OP_LAYOUTCOMMIT]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_LAYOUTGET]		= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_LAYOUTRETURN]	= (nfsd4_enc)nfsd4_encode_noop,
> -	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_noop,
> +	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo,

hi Bruce,

When testing this patch, oops appears.

We should implement a nfsd4_encode_secinfo_no_name() instead using 
nfsd4_encode_secinfo(). 

With the following patch, kernel will run correctly.

Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
 fs/nfsd/nfs4xdr.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index b543b24..437b462 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2845,11 +2845,10 @@ nfsd4_encode_rename(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 }
 
 static __be32
-nfsd4_encode_secinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
-		     struct nfsd4_secinfo *secinfo)
+nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
+			 __be32 nfserr,struct svc_export *exp)
 {
 	int i = 0;
-	struct svc_export *exp = secinfo->si_exp;
 	u32 nflavs;
 	struct exp_flavor_info *flavs;
 	struct exp_flavor_info def_flavs[2];
@@ -2911,6 +2910,20 @@ out:
 	return nfserr;
 }
 
+static __be32
+nfsd4_encode_secinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
+		     struct nfsd4_secinfo *secinfo)
+{
+	return nfsd4_do_encode_secinfo(resp, nfserr, secinfo->si_exp);
+}
+
+static __be32
+nfsd4_encode_secinfo_no_name(struct nfsd4_compoundres *resp, __be32 nfserr,
+		     struct nfsd4_secinfo_no_name *secinfo)
+{
+	return nfsd4_do_encode_secinfo(resp, nfserr, secinfo->sin_exp);
+}
+
 /*
  * The SETATTR encode routine is special -- it always encodes a bitmap,
  * regardless of the error status.
@@ -3173,7 +3186,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_LAYOUTCOMMIT]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_LAYOUTGET]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_LAYOUTRETURN]	= (nfsd4_enc)nfsd4_encode_noop,
-	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo,
+	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo_no_name,
 	[OP_SEQUENCE]		= (nfsd4_enc)nfsd4_encode_sequence,
 	[OP_SET_SSV]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
-- 
1.7.3.3

==========================================================================
BUG: unable to handle kernel NULL pointer dereference at 00000044
IP: [<e2bd239a>] nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd]
*pdpt = 000000001d59c001 *pde = 0000000000000000
Oops: 0000 [#1] SMP
last sysfs file: /sys/kernel/mm/ksm/run
Modules linked in: ipt_MASQUERADE iptable_nat nf_nat bridge stp llc nfsd lockd nfs_acl auth_rpcgss exportfs sunrpc ipv6 snd_ens1371 gameport snd_rawmidi snd_ac97_codec ac97_bus snd_seq snd_seq_device ppdev parport_pc snd_pcm snd_timer i2c_piix4 i2c_core snd soundcore snd_page_alloc pcnet32 mii parport microcode pcspkr BusLogic floppy [last unloaded: mperf]

Pid: 1285, comm: nfsd Not tainted 2.6.37-rc7+ #50 440BX Desktop Reference Platform/VMware Virtual Platform
EIP: 0060:[<e2bd239a>] EFLAGS: 00010246 CPU: 0
EIP is at nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd]
EAX: ddd30000 EBX: ddd30000 ECX: 00000000 EDX: 00000000
ESI: ddbaba18 EDI: e2bd237e EBP: dd48befc ESP: dd48bec4
 DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Process nfsd (pid: 1285, ti=dd48a000 task=dd9d3240 task.ti=dd48a000)
Stack:
 00000000 deb18198 db078ef8 0fd00000 db07f4c8 00000000 00000000 db07f4c8
 de054600 dd48bef8 c04e17af ddd30000 ddbaba18 e2bd237e dd48bf1c e2bd1e2b
 ddbaba20 e2bd0b0c ddb1c068 ddd30000 ddbaba18 e2bd0b0c dd48bf40 e2bd096c
Call Trace:
 [<c04e17af>] ? dput+0x36/0xfc
 [<e2bd237e>] ? nfsd4_encode_secinfo+0x0/0x1c1 [nfsd]
 [<e2bd1e2b>] ? nfsd4_encode_operation+0x56/0x11b [nfsd]
 [<e2bd0b0c>] ? nfsd4_secinfo_no_name+0x0/0x4a [nfsd]
 [<e2bd0b0c>] ? nfsd4_secinfo_no_name+0x0/0x4a [nfsd]
 [<e2bd096c>] ? nfsd4_proc_compound+0x252/0x370 [nfsd]
 [<e2bc42de>] ? nfsd_dispatch+0xd1/0x19d [nfsd]
 [<e248912f>] ? svc_process_common+0x283/0x46c [sunrpc]
 [<e24894dd>] ? svc_process+0xde/0xf1 [sunrpc]
 [<e2bc47cf>] ? nfsd+0xd6/0x115 [nfsd]
 [<e2bc46f9>] ? nfsd+0x0/0x115 [nfsd]
 [<c0454d22>] ? kthread+0x62/0x67
 [<c0454cc0>] ? kthread+0x0/0x67
 [<c0409a3e>] ? kernel_thread_helper+0x6/0x10
Code: 83 c0 04 89 01 31 c0 83 c4 14 5b 5e 5f 5d c3 55 89 e5 57 56 53 89 c3 83 ec 2c 85 d2 89 55 c8 8b 49 08 89 4d dc 0f 85 68 01 00 00 <8b> 41 44 85 c0 74 0b 83 c1 48 89 4d d4 89 45 d8 eb 4d 8b 55 dc
EIP: [<e2bd239a>] nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd] SS:ESP 0068:dd48bec4
CR2: 0000000000000044
---[ end trace 9c31555ffcc1f3e8 ]---

thanks, 
Mi Jinlong


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2010-12-27  6:29   ` Mi Jinlong
@ 2010-12-29 18:56     ` J. Bruce Fields
  2010-12-30  4:13       ` Mi Jinlong
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2010-12-29 18:56 UTC (permalink / raw)
  To: Mi Jinlong; +Cc: J. Bruce Fields, linux-nfs

On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
> When testing this patch, oops appears.
> 
> We should implement a nfsd4_encode_secinfo_no_name() instead using 
> nfsd4_encode_secinfo(). 
> 
> With the following patch, kernel will run correctly.

Whoops, yes, you're correct.  I've applied your patch.  Thanks!

(What are you using for testing?)

--b.

> 
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  fs/nfsd/nfs4xdr.c |   21 +++++++++++++++++----
>  1 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index b543b24..437b462 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2845,11 +2845,10 @@ nfsd4_encode_rename(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
>  }
>  
>  static __be32
> -nfsd4_encode_secinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
> -		     struct nfsd4_secinfo *secinfo)
> +nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
> +			 __be32 nfserr,struct svc_export *exp)
>  {
>  	int i = 0;
> -	struct svc_export *exp = secinfo->si_exp;
>  	u32 nflavs;
>  	struct exp_flavor_info *flavs;
>  	struct exp_flavor_info def_flavs[2];
> @@ -2911,6 +2910,20 @@ out:
>  	return nfserr;
>  }
>  
> +static __be32
> +nfsd4_encode_secinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
> +		     struct nfsd4_secinfo *secinfo)
> +{
> +	return nfsd4_do_encode_secinfo(resp, nfserr, secinfo->si_exp);
> +}
> +
> +static __be32
> +nfsd4_encode_secinfo_no_name(struct nfsd4_compoundres *resp, __be32 nfserr,
> +		     struct nfsd4_secinfo_no_name *secinfo)
> +{
> +	return nfsd4_do_encode_secinfo(resp, nfserr, secinfo->sin_exp);
> +}
> +
>  /*
>   * The SETATTR encode routine is special -- it always encodes a bitmap,
>   * regardless of the error status.
> @@ -3173,7 +3186,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>  	[OP_LAYOUTCOMMIT]	= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_LAYOUTGET]		= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_LAYOUTRETURN]	= (nfsd4_enc)nfsd4_encode_noop,
> -	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo,
> +	[OP_SECINFO_NO_NAME]	= (nfsd4_enc)nfsd4_encode_secinfo_no_name,
>  	[OP_SEQUENCE]		= (nfsd4_enc)nfsd4_encode_sequence,
>  	[OP_SET_SSV]		= (nfsd4_enc)nfsd4_encode_noop,
>  	[OP_TEST_STATEID]	= (nfsd4_enc)nfsd4_encode_noop,
> -- 
> 1.7.3.3
> 
> ==========================================================================
> BUG: unable to handle kernel NULL pointer dereference at 00000044
> IP: [<e2bd239a>] nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd]
> *pdpt = 000000001d59c001 *pde = 0000000000000000
> Oops: 0000 [#1] SMP
> last sysfs file: /sys/kernel/mm/ksm/run
> Modules linked in: ipt_MASQUERADE iptable_nat nf_nat bridge stp llc nfsd lockd nfs_acl auth_rpcgss exportfs sunrpc ipv6 snd_ens1371 gameport snd_rawmidi snd_ac97_codec ac97_bus snd_seq snd_seq_device ppdev parport_pc snd_pcm snd_timer i2c_piix4 i2c_core snd soundcore snd_page_alloc pcnet32 mii parport microcode pcspkr BusLogic floppy [last unloaded: mperf]
> 
> Pid: 1285, comm: nfsd Not tainted 2.6.37-rc7+ #50 440BX Desktop Reference Platform/VMware Virtual Platform
> EIP: 0060:[<e2bd239a>] EFLAGS: 00010246 CPU: 0
> EIP is at nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd]
> EAX: ddd30000 EBX: ddd30000 ECX: 00000000 EDX: 00000000
> ESI: ddbaba18 EDI: e2bd237e EBP: dd48befc ESP: dd48bec4
>  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> Process nfsd (pid: 1285, ti=dd48a000 task=dd9d3240 task.ti=dd48a000)
> Stack:
>  00000000 deb18198 db078ef8 0fd00000 db07f4c8 00000000 00000000 db07f4c8
>  de054600 dd48bef8 c04e17af ddd30000 ddbaba18 e2bd237e dd48bf1c e2bd1e2b
>  ddbaba20 e2bd0b0c ddb1c068 ddd30000 ddbaba18 e2bd0b0c dd48bf40 e2bd096c
> Call Trace:
>  [<c04e17af>] ? dput+0x36/0xfc
>  [<e2bd237e>] ? nfsd4_encode_secinfo+0x0/0x1c1 [nfsd]
>  [<e2bd1e2b>] ? nfsd4_encode_operation+0x56/0x11b [nfsd]
>  [<e2bd0b0c>] ? nfsd4_secinfo_no_name+0x0/0x4a [nfsd]
>  [<e2bd0b0c>] ? nfsd4_secinfo_no_name+0x0/0x4a [nfsd]
>  [<e2bd096c>] ? nfsd4_proc_compound+0x252/0x370 [nfsd]
>  [<e2bc42de>] ? nfsd_dispatch+0xd1/0x19d [nfsd]
>  [<e248912f>] ? svc_process_common+0x283/0x46c [sunrpc]
>  [<e24894dd>] ? svc_process+0xde/0xf1 [sunrpc]
>  [<e2bc47cf>] ? nfsd+0xd6/0x115 [nfsd]
>  [<e2bc46f9>] ? nfsd+0x0/0x115 [nfsd]
>  [<c0454d22>] ? kthread+0x62/0x67
>  [<c0454cc0>] ? kthread+0x0/0x67
>  [<c0409a3e>] ? kernel_thread_helper+0x6/0x10
> Code: 83 c0 04 89 01 31 c0 83 c4 14 5b 5e 5f 5d c3 55 89 e5 57 56 53 89 c3 83 ec 2c 85 d2 89 55 c8 8b 49 08 89 4d dc 0f 85 68 01 00 00 <8b> 41 44 85 c0 74 0b 83 c1 48 89 4d d4 89 45 d8 eb 4d 8b 55 dc
> EIP: [<e2bd239a>] nfsd4_encode_secinfo+0x1c/0x1c1 [nfsd] SS:ESP 0068:dd48bec4
> CR2: 0000000000000044
> ---[ end trace 9c31555ffcc1f3e8 ]---
> 
> thanks, 
> Mi Jinlong
> 
> --
> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2010-12-29 18:56     ` J. Bruce Fields
@ 2010-12-30  4:13       ` Mi Jinlong
  2011-01-05  1:05         ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Mi Jinlong @ 2010-12-30  4:13 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: J. Bruce Fields, linux-nfs



J. Bruce Fields:
> On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
>> When testing this patch, oops appears.
>>
>> We should implement a nfsd4_encode_secinfo_no_name() instead using 
>> nfsd4_encode_secinfo(). 
>>
>> With the following patch, kernel will run correctly.
> 
> Whoops, yes, you're correct.  I've applied your patch.  Thanks!
> 
> (What are you using for testing?)

Hi Bruce:

  I test it by the pynfs41 at your tree, and add some simple
  test case as following.

  I have a question about the op.secinfo_no_name(), should it take 
  a argument? 
  Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
  or I can't find how to set the argument.

thanks,
Mi Jinlong  

>From 31ae25c04295888fa54ea531cdfaa4cfdefbe877 Mon Sep 17 00:00:00 2001
From: Mi Jinlong <mijinlong@cn.fujitsu.com>
Date: Wed, 29 Dec 2010 17:23:35 +0800
Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests

 Add two simple secinfo_no_name tests as Bruce said:
    - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
      legal result.
    - send PUTROOTFH+SECINFO+GETFH, and/or
      PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
      NOFILEHANDLE.

 Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
 nfs4.1/server41tests/__init__.py           |    1 +
 nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py

diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
index 22cd664..6aee88e 100644
--- a/nfs4.1/server41tests/__init__.py
+++ b/nfs4.1/server41tests/__init__.py
@@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
            "st_compound.py",
            "st_create_session.py",
            "st_destroy_session.py",
+           "st_secinfo_no_name.py",
            "st_sequence.py",
 	   "st_trunking.py",
            "st_open.py",
diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
new file mode 100644
index 0000000..a4b148d
--- /dev/null
+++ b/nfs4.1/server41tests/st_secinfo_no_name.py
@@ -0,0 +1,35 @@
+from st_create_session import create_session
+from nfs4_const import *
+from environment import check, fail, bad_sessionid, create_file
+from nfs4_type import channel_attrs4
+import nfs4_ops as op
+import nfs4lib
+
+def testSupported(t, env):
+    """Do a simple SECINFO_NO_NAME
+       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
+
+    FLAGS: all 
+    CODE: SECNN1
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name()])
+    check(res);
+
+def testSupported2(t, env):
+    """GETFH after do a SECINFO_NO_NAME or SECINFO
+       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
+
+    FLAGS: all
+    CODE: SECNN2
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name(), op.getfh()])
+    print res
+    check(res, NFS4ERR_NOFILEHANDLE);
+
+    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
+    print res
+    check(res, NFS4ERR_NOFILEHANDLE);
-- 
1.7.3.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2010-12-30  4:13       ` Mi Jinlong
@ 2011-01-05  1:05         ` J. Bruce Fields
  2011-01-05 15:15           ` Fred Isaman
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2011-01-05  1:05 UTC (permalink / raw)
  To: Mi Jinlong; +Cc: J. Bruce Fields, linux-nfs

On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
> 
> 
> J. Bruce Fields:
> > On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
> >> When testing this patch, oops appears.
> >>
> >> We should implement a nfsd4_encode_secinfo_no_name() instead using 
> >> nfsd4_encode_secinfo(). 
> >>
> >> With the following patch, kernel will run correctly.
> > 
> > Whoops, yes, you're correct.  I've applied your patch.  Thanks!
> > 
> > (What are you using for testing?)
> 
> Hi Bruce:
> 
>   I test it by the pynfs41 at your tree, and add some simple
>   test case as following.
> 
>   I have a question about the op.secinfo_no_name(), should it take 
>   a argument? 
>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
>   or I can't find how to set the argument.

Hm, I don't know, I guess I'd look at how arguments are passed to some
of the other operations when constructing compounds in other tests....

--b.

> 
> thanks,
> Mi Jinlong  
> 
> >From 31ae25c04295888fa54ea531cdfaa4cfdefbe877 Mon Sep 17 00:00:00 2001
> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
> Date: Wed, 29 Dec 2010 17:23:35 +0800
> Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests
> 
>  Add two simple secinfo_no_name tests as Bruce said:
>     - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
>       legal result.
>     - send PUTROOTFH+SECINFO+GETFH, and/or
>       PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
>       NOFILEHANDLE.
> 
>  Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  nfs4.1/server41tests/__init__.py           |    1 +
>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
> 
> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
> index 22cd664..6aee88e 100644
> --- a/nfs4.1/server41tests/__init__.py
> +++ b/nfs4.1/server41tests/__init__.py
> @@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
>             "st_compound.py",
>             "st_create_session.py",
>             "st_destroy_session.py",
> +           "st_secinfo_no_name.py",
>             "st_sequence.py",
>  	   "st_trunking.py",
>             "st_open.py",
> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
> new file mode 100644
> index 0000000..a4b148d
> --- /dev/null
> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
> @@ -0,0 +1,35 @@
> +from st_create_session import create_session
> +from nfs4_const import *
> +from environment import check, fail, bad_sessionid, create_file
> +from nfs4_type import channel_attrs4
> +import nfs4_ops as op
> +import nfs4lib
> +
> +def testSupported(t, env):
> +    """Do a simple SECINFO_NO_NAME
> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
> +
> +    FLAGS: all 
> +    CODE: SECNN1
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name()])
> +    check(res);
> +
> +def testSupported2(t, env):
> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> +
> +    FLAGS: all
> +    CODE: SECNN2
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(), op.getfh()])
> +    print res
> +    check(res, NFS4ERR_NOFILEHANDLE);
> +
> +    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
> +    print res
> +    check(res, NFS4ERR_NOFILEHANDLE);
> -- 
> 1.7.3.3
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-05  1:05         ` J. Bruce Fields
@ 2011-01-05 15:15           ` Fred Isaman
  2011-01-05 15:37             ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Fred Isaman @ 2011-01-05 15:15 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs

On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
>>
>>
>> J. Bruce Fields:
>> > On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
>> >> When testing this patch, oops appears.
>> >>
>> >> We should implement a nfsd4_encode_secinfo_no_name() instead using
>> >> nfsd4_encode_secinfo().
>> >>
>> >> With the following patch, kernel will run correctly.
>> >
>> > Whoops, yes, you're correct.  I've applied your patch.  Thanks!
>> >
>> > (What are you using for testing?)
>>
>> Hi Bruce:
>>
>>   I test it by the pynfs41 at your tree, and add some simple
>>   test case as following.
>>
>>   I have a question about the op.secinfo_no_name(), should it take
>>   a argument?
>>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
>>   or I can't find how to set the argument.
>

There was a bug in how the op.secinfo_no_name() function was auto
constructed.  I've pushed a fix to
my tree at git://linux-nfs.org/~iisaman/newpynfs.git.

Fred

> Hm, I don't know, I guess I'd look at how arguments are passed to some
> of the other operations when constructing compounds in other tests....
>
> --b.
>
>>
>> thanks,
>> Mi Jinlong
>>
>> >From 31ae25c04295888fa54ea531cdfaa4cfdefbe877 Mon Sep 17 00:00:00 2001
>> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
>> Date: Wed, 29 Dec 2010 17:23:35 +0800
>> Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests
>>
>>  Add two simple secinfo_no_name tests as Bruce said:
>>     - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
>>       legal result.
>>     - send PUTROOTFH+SECINFO+GETFH, and/or
>>       PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
>>       NOFILEHANDLE.
>>
>>  Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>> ---
>>  nfs4.1/server41tests/__init__.py           |    1 +
>>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+), 0 deletions(-)
>>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
>>
>> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
>> index 22cd664..6aee88e 100644
>> --- a/nfs4.1/server41tests/__init__.py
>> +++ b/nfs4.1/server41tests/__init__.py
>> @@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
>>             "st_compound.py",
>>             "st_create_session.py",
>>             "st_destroy_session.py",
>> +           "st_secinfo_no_name.py",
>>             "st_sequence.py",
>>          "st_trunking.py",
>>             "st_open.py",
>> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
>> new file mode 100644
>> index 0000000..a4b148d
>> --- /dev/null
>> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
>> @@ -0,0 +1,35 @@
>> +from st_create_session import create_session
>> +from nfs4_const import *
>> +from environment import check, fail, bad_sessionid, create_file
>> +from nfs4_type import channel_attrs4
>> +import nfs4_ops as op
>> +import nfs4lib
>> +
>> +def testSupported(t, env):
>> +    """Do a simple SECINFO_NO_NAME
>> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
>> +
>> +    FLAGS: all
>> +    CODE: SECNN1
>> +    """
>> +    c = env.c1.new_client(env.testname(t))
>> +    sess = c.create_session()
>> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name()])
>> +    check(res);
>> +
>> +def testSupported2(t, env):
>> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
>> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
>> +
>> +    FLAGS: all
>> +    CODE: SECNN2
>> +    """
>> +    c = env.c1.new_client(env.testname(t))
>> +    sess = c.create_session()
>> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(), op.getfh()])
>> +    print res
>> +    check(res, NFS4ERR_NOFILEHANDLE);
>> +
>> +    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
>> +    print res
>> +    check(res, NFS4ERR_NOFILEHANDLE);
>> --
>> 1.7.3.3
>>
> --
> 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
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-05 15:15           ` Fred Isaman
@ 2011-01-05 15:37             ` J. Bruce Fields
  2011-01-05 17:29               ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2011-01-05 15:37 UTC (permalink / raw)
  To: Fred Isaman; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs

On Wed, Jan 05, 2011 at 10:15:11AM -0500, Fred Isaman wrote:
> On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
> >>
> >>
> >> J. Bruce Fields:
> >> > On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
> >> >> When testing this patch, oops appears.
> >> >>
> >> >> We should implement a nfsd4_encode_secinfo_no_name() instead using
> >> >> nfsd4_encode_secinfo().
> >> >>
> >> >> With the following patch, kernel will run correctly.
> >> >
> >> > Whoops, yes, you're correct.  I've applied your patch.  Thanks!
> >> >
> >> > (What are you using for testing?)
> >>
> >> Hi Bruce:
> >>
> >>   I test it by the pynfs41 at your tree, and add some simple
> >>   test case as following.
> >>
> >>   I have a question about the op.secinfo_no_name(), should it take
> >>   a argument?
> >>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
> >>   or I can't find how to set the argument.
> >
> 
> There was a bug in how the op.secinfo_no_name() function was auto
> constructed.  I've pushed a fix to
> my tree at git://linux-nfs.org/~iisaman/newpynfs.git.

Thanks, Fred.  I've pulled that into my repository too.

--b.

> 
> Fred
> 
> > Hm, I don't know, I guess I'd look at how arguments are passed to some
> > of the other operations when constructing compounds in other tests....
> >
> > --b.
> >
> >>
> >> thanks,
> >> Mi Jinlong
> >>
> >> >From 31ae25c04295888fa54ea531cdfaa4cfdefbe877 Mon Sep 17 00:00:00 2001
> >> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
> >> Date: Wed, 29 Dec 2010 17:23:35 +0800
> >> Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests
> >>
> >>  Add two simple secinfo_no_name tests as Bruce said:
> >>     - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
> >>       legal result.
> >>     - send PUTROOTFH+SECINFO+GETFH, and/or
> >>       PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
> >>       NOFILEHANDLE.
> >>
> >>  Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> >> ---
> >>  nfs4.1/server41tests/__init__.py           |    1 +
> >>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
> >>  2 files changed, 36 insertions(+), 0 deletions(-)
> >>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
> >>
> >> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
> >> index 22cd664..6aee88e 100644
> >> --- a/nfs4.1/server41tests/__init__.py
> >> +++ b/nfs4.1/server41tests/__init__.py
> >> @@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
> >>             "st_compound.py",
> >>             "st_create_session.py",
> >>             "st_destroy_session.py",
> >> +           "st_secinfo_no_name.py",
> >>             "st_sequence.py",
> >>          "st_trunking.py",
> >>             "st_open.py",
> >> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
> >> new file mode 100644
> >> index 0000000..a4b148d
> >> --- /dev/null
> >> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
> >> @@ -0,0 +1,35 @@
> >> +from st_create_session import create_session
> >> +from nfs4_const import *
> >> +from environment import check, fail, bad_sessionid, create_file
> >> +from nfs4_type import channel_attrs4
> >> +import nfs4_ops as op
> >> +import nfs4lib
> >> +
> >> +def testSupported(t, env):
> >> +    """Do a simple SECINFO_NO_NAME
> >> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
> >> +
> >> +    FLAGS: all
> >> +    CODE: SECNN1
> >> +    """
> >> +    c = env.c1.new_client(env.testname(t))
> >> +    sess = c.create_session()
> >> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name()])
> >> +    check(res);
> >> +
> >> +def testSupported2(t, env):
> >> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> >> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> >> +
> >> +    FLAGS: all
> >> +    CODE: SECNN2
> >> +    """
> >> +    c = env.c1.new_client(env.testname(t))
> >> +    sess = c.create_session()
> >> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(), op.getfh()])
> >> +    print res
> >> +    check(res, NFS4ERR_NOFILEHANDLE);
> >> +
> >> +    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
> >> +    print res
> >> +    check(res, NFS4ERR_NOFILEHANDLE);
> >> --
> >> 1.7.3.3
> >>
> > --
> > 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
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-05 15:37             ` J. Bruce Fields
@ 2011-01-05 17:29               ` J. Bruce Fields
  2011-01-06  3:54                 ` Mi Jinlong
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2011-01-05 17:29 UTC (permalink / raw)
  To: Fred Isaman; +Cc: Mi Jinlong, J. Bruce Fields, linux-nfs

On Wed, Jan 05, 2011 at 10:37:13AM -0500, J. Bruce Fields wrote:
> On Wed, Jan 05, 2011 at 10:15:11AM -0500, Fred Isaman wrote:
> > On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > > On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
> > >>
> > >>
> > >> J. Bruce Fields:
> > >> > On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
> > >> >> When testing this patch, oops appears.
> > >> >>
> > >> >> We should implement a nfsd4_encode_secinfo_no_name() instead using
> > >> >> nfsd4_encode_secinfo().
> > >> >>
> > >> >> With the following patch, kernel will run correctly.
> > >> >
> > >> > Whoops, yes, you're correct.  I've applied your patch.  Thanks!
> > >> >
> > >> > (What are you using for testing?)
> > >>
> > >> Hi Bruce:
> > >>
> > >>   I test it by the pynfs41 at your tree, and add some simple
> > >>   test case as following.
> > >>
> > >>   I have a question about the op.secinfo_no_name(), should it take
> > >>   a argument?
> > >>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
> > >>   or I can't find how to set the argument.
> > >
> > 
> > There was a bug in how the op.secinfo_no_name() function was auto
> > constructed.  I've pushed a fix to
> > my tree at git://linux-nfs.org/~iisaman/newpynfs.git.
> 
> Thanks, Fred.  I've pulled that into my repository too.

But then these secinfo_no_name tests don't work ("TypeError:
data.opsecinfo_no_name == None"); Mi Jinlong, would you mind looking
into that?

--b.

> 
> --b.
> 
> > 
> > Fred
> > 
> > > Hm, I don't know, I guess I'd look at how arguments are passed to some
> > > of the other operations when constructing compounds in other tests....
> > >
> > > --b.
> > >
> > >>
> > >> thanks,
> > >> Mi Jinlong
> > >>
> > >> >From 31ae25c04295888fa54ea531cdfaa4cfdefbe877 Mon Sep 17 00:00:00 2001
> > >> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
> > >> Date: Wed, 29 Dec 2010 17:23:35 +0800
> > >> Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests
> > >>
> > >>  Add two simple secinfo_no_name tests as Bruce said:
> > >>     - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
> > >>       legal result.
> > >>     - send PUTROOTFH+SECINFO+GETFH, and/or
> > >>       PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
> > >>       NOFILEHANDLE.
> > >>
> > >>  Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> > >> ---
> > >>  nfs4.1/server41tests/__init__.py           |    1 +
> > >>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
> > >>  2 files changed, 36 insertions(+), 0 deletions(-)
> > >>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
> > >>
> > >> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
> > >> index 22cd664..6aee88e 100644
> > >> --- a/nfs4.1/server41tests/__init__.py
> > >> +++ b/nfs4.1/server41tests/__init__.py
> > >> @@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
> > >>             "st_compound.py",
> > >>             "st_create_session.py",
> > >>             "st_destroy_session.py",
> > >> +           "st_secinfo_no_name.py",
> > >>             "st_sequence.py",
> > >>          "st_trunking.py",
> > >>             "st_open.py",
> > >> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
> > >> new file mode 100644
> > >> index 0000000..a4b148d
> > >> --- /dev/null
> > >> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
> > >> @@ -0,0 +1,35 @@
> > >> +from st_create_session import create_session
> > >> +from nfs4_const import *
> > >> +from environment import check, fail, bad_sessionid, create_file
> > >> +from nfs4_type import channel_attrs4
> > >> +import nfs4_ops as op
> > >> +import nfs4lib
> > >> +
> > >> +def testSupported(t, env):
> > >> +    """Do a simple SECINFO_NO_NAME
> > >> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
> > >> +
> > >> +    FLAGS: all
> > >> +    CODE: SECNN1
> > >> +    """
> > >> +    c = env.c1.new_client(env.testname(t))
> > >> +    sess = c.create_session()
> > >> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name()])
> > >> +    check(res);
> > >> +
> > >> +def testSupported2(t, env):
> > >> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> > >> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> > >> +
> > >> +    FLAGS: all
> > >> +    CODE: SECNN2
> > >> +    """
> > >> +    c = env.c1.new_client(env.testname(t))
> > >> +    sess = c.create_session()
> > >> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(), op.getfh()])
> > >> +    print res
> > >> +    check(res, NFS4ERR_NOFILEHANDLE);
> > >> +
> > >> +    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
> > >> +    print res
> > >> +    check(res, NFS4ERR_NOFILEHANDLE);
> > >> --
> > >> 1.7.3.3
> > >>
> > > --
> > > 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
> > >

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-05 17:29               ` J. Bruce Fields
@ 2011-01-06  3:54                 ` Mi Jinlong
  2011-01-11 23:32                   ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Mi Jinlong @ 2011-01-06  3:54 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Fred Isaman, J. Bruce Fields, linux-nfs



J. Bruce Fields 写道:
> On Wed, Jan 05, 2011 at 10:37:13AM -0500, J. Bruce Fields wrote:
>> On Wed, Jan 05, 2011 at 10:15:11AM -0500, Fred Isaman wrote:
>>> On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
>>>> On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
>>>>>
>>>>> J. Bruce Fields:
>>>>>> On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
>>>>>>> When testing this patch, oops appears.
>>>>>>>
>>>>>>> We should implement a nfsd4_encode_secinfo_no_name() instead using
>>>>>>> nfsd4_encode_secinfo().
>>>>>>>
>>>>>>> With the following patch, kernel will run correctly.
>>>>>> Whoops, yes, you're correct.  I've applied your patch.  Thanks!
>>>>>>
>>>>>> (What are you using for testing?)
>>>>> Hi Bruce:
>>>>>
>>>>>   I test it by the pynfs41 at your tree, and add some simple
>>>>>   test case as following.
>>>>>
>>>>>   I have a question about the op.secinfo_no_name(), should it take
>>>>>   a argument?
>>>>>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
>>>>>   or I can't find how to set the argument.
>>> There was a bug in how the op.secinfo_no_name() function was auto
>>> constructed.  I've pushed a fix to
>>> my tree at git://linux-nfs.org/~iisaman/newpynfs.git.
>> Thanks, Fred.  I've pulled that into my repository too.
> 
> But then these secinfo_no_name tests don't work ("TypeError:
> data.opsecinfo_no_name == None"); Mi Jinlong, would you mind looking
> into that?

  After Fred's patch, we should rebuild the test site at pynfs41 dir 
  with "python setup.py build".

  And update test case, call op.secinfo_no_name() with 0 as following.

thanks,
Mi Jinlong

>From 344f0ee9a625d43b918be5dfc4fc9df8e8cbac28 Mon Sep 17 00:00:00 2001
From: Mi Jinlong <mijinlong@cn.fujitsu.com>
Date: Thu, 30 Dec 2010 12:13:27 +0800
Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests

Add two simple secinfo_no_name tests as Bruce said:
  - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
    legal result.
  - send PUTROOTFH+SECINFO+GETFH, and/or
    PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
    NOFILEHANDLE.

Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
 nfs4.1/server41tests/__init__.py           |    1 +
 nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py

diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
index 22cd664..6aee88e 100644
--- a/nfs4.1/server41tests/__init__.py
+++ b/nfs4.1/server41tests/__init__.py
@@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
            "st_compound.py",
            "st_create_session.py",
            "st_destroy_session.py",
+           "st_secinfo_no_name.py",
            "st_sequence.py",
 	   "st_trunking.py",
            "st_open.py",
diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
new file mode 100644
index 0000000..b66fdb6
--- /dev/null
+++ b/nfs4.1/server41tests/st_secinfo_no_name.py
@@ -0,0 +1,35 @@
+from st_create_session import create_session
+from nfs4_const import *
+from environment import check, fail, bad_sessionid, create_file
+from nfs4_type import channel_attrs4
+import nfs4_ops as op
+import nfs4lib
+
+def testSupported(t, env):
+    """Do a simple SECINFO_NO_NAME
+       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
+
+    FLAGS: all
+    CODE: SECNN1
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0)])
+    check(res);
+
+def testSupported2(t, env):
+    """GETFH after do a SECINFO_NO_NAME or SECINFO
+       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
+
+    FLAGS: all
+    CODE: SECNN2
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0), op.getfh()])
+    print res
+    check(res, NFS4ERR_NOFILEHANDLE);
+
+    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
+    print res
+    check(res, NFS4ERR_NOFILEHANDLE);
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-06  3:54                 ` Mi Jinlong
@ 2011-01-11 23:32                   ` J. Bruce Fields
  2011-01-13  3:20                     ` Mi Jinlong
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2011-01-11 23:32 UTC (permalink / raw)
  To: Mi Jinlong; +Cc: Fred Isaman, J. Bruce Fields, linux-nfs

On Thu, Jan 06, 2011 at 11:54:02AM +0800, Mi Jinlong wrote:
> 
> 
> J. Bruce Fields 写道:
> > On Wed, Jan 05, 2011 at 10:37:13AM -0500, J. Bruce Fields wrote:
> >> On Wed, Jan 05, 2011 at 10:15:11AM -0500, Fred Isaman wrote:
> >>> On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> >>>> On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
> >>>>>
> >>>>> J. Bruce Fields:
> >>>>>> On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
> >>>>>>> When testing this patch, oops appears.
> >>>>>>>
> >>>>>>> We should implement a nfsd4_encode_secinfo_no_name() instead using
> >>>>>>> nfsd4_encode_secinfo().
> >>>>>>>
> >>>>>>> With the following patch, kernel will run correctly.
> >>>>>> Whoops, yes, you're correct.  I've applied your patch.  Thanks!
> >>>>>>
> >>>>>> (What are you using for testing?)
> >>>>> Hi Bruce:
> >>>>>
> >>>>>   I test it by the pynfs41 at your tree, and add some simple
> >>>>>   test case as following.
> >>>>>
> >>>>>   I have a question about the op.secinfo_no_name(), should it take
> >>>>>   a argument?
> >>>>>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
> >>>>>   or I can't find how to set the argument.
> >>> There was a bug in how the op.secinfo_no_name() function was auto
> >>> constructed.  I've pushed a fix to
> >>> my tree at git://linux-nfs.org/~iisaman/newpynfs.git.
> >> Thanks, Fred.  I've pulled that into my repository too.
> > 
> > But then these secinfo_no_name tests don't work ("TypeError:
> > data.opsecinfo_no_name == None"); Mi Jinlong, would you mind looking
> > into that?
> 
>   After Fred's patch, we should rebuild the test site at pynfs41 dir 
>   with "python setup.py build".
> 
>   And update test case, call op.secinfo_no_name() with 0 as following.

Thanks!

Could I ask for a few changes?:
	- The second half of testSupported2 should go into a separate
	  test in a separate st_secinfo file.  (It will be a very small
	  file for now!  But we can port some of the 4.0 tests to it
	  some day.)
	- That test should create a file in the same way that e.g. the
	  st_open tests do instead of assuming a file named "/tree"
	  exists.

--b.

> 
> thanks,
> Mi Jinlong
> 
> >From 344f0ee9a625d43b918be5dfc4fc9df8e8cbac28 Mon Sep 17 00:00:00 2001
> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
> Date: Thu, 30 Dec 2010 12:13:27 +0800
> Subject: [PATCH] CLNT: Add some simple secinfo_no_name tests
> 
> Add two simple secinfo_no_name tests as Bruce said:
>   - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
>     legal result.
>   - send PUTROOTFH+SECINFO+GETFH, and/or
>     PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
>     NOFILEHANDLE.
> 
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  nfs4.1/server41tests/__init__.py           |    1 +
>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 ++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
> 
> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
> index 22cd664..6aee88e 100644
> --- a/nfs4.1/server41tests/__init__.py
> +++ b/nfs4.1/server41tests/__init__.py
> @@ -2,6 +2,7 @@ __all__ = ["st_exchange_id.py", # draft 21
>             "st_compound.py",
>             "st_create_session.py",
>             "st_destroy_session.py",
> +           "st_secinfo_no_name.py",
>             "st_sequence.py",
>  	   "st_trunking.py",
>             "st_open.py",
> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
> new file mode 100644
> index 0000000..b66fdb6
> --- /dev/null
> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
> @@ -0,0 +1,35 @@
> +from st_create_session import create_session
> +from nfs4_const import *
> +from environment import check, fail, bad_sessionid, create_file
> +from nfs4_type import channel_attrs4
> +import nfs4_ops as op
> +import nfs4lib
> +
> +def testSupported(t, env):
> +    """Do a simple SECINFO_NO_NAME
> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
> +
> +    FLAGS: all
> +    CODE: SECNN1
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0)])
> +    check(res);
> +
> +def testSupported2(t, env):
> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> +
> +    FLAGS: all
> +    CODE: SECNN2
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0), op.getfh()])
> +    print res
> +    check(res, NFS4ERR_NOFILEHANDLE);
> +
> +    res = sess.compound([op.putrootfh(), op.secinfo("tree"), op.getfh()])
> +    print res
> +    check(res, NFS4ERR_NOFILEHANDLE);
> -- 
> 1.7.3.4
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-11 23:32                   ` J. Bruce Fields
@ 2011-01-13  3:20                     ` Mi Jinlong
  2011-01-18 22:59                       ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Mi Jinlong @ 2011-01-13  3:20 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Fred Isaman, J. Bruce Fields, linux-nfs



J. Bruce Fields 写道:
> On Thu, Jan 06, 2011 at 11:54:02AM +0800, Mi Jinlong wrote:
>>
>> J. Bruce Fields 写道:
>>> On Wed, Jan 05, 2011 at 10:37:13AM -0500, J. Bruce Fields wrote:
>>>> On Wed, Jan 05, 2011 at 10:15:11AM -0500, Fred Isaman wrote:
>>>>> On Tue, Jan 4, 2011 at 8:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
>>>>>> On Thu, Dec 30, 2010 at 12:13:27PM +0800, Mi Jinlong wrote:
>>>>>>> J. Bruce Fields:
>>>>>>>> On Mon, Dec 27, 2010 at 02:29:57PM +0800, Mi Jinlong wrote:
>>>>>>>>> When testing this patch, oops appears.
>>>>>>>>>
>>>>>>>>> We should implement a nfsd4_encode_secinfo_no_name() instead using
>>>>>>>>> nfsd4_encode_secinfo().
>>>>>>>>>
>>>>>>>>> With the following patch, kernel will run correctly.
>>>>>>>> Whoops, yes, you're correct.  I've applied your patch.  Thanks!
>>>>>>>>
>>>>>>>> (What are you using for testing?)
>>>>>>> Hi Bruce:
>>>>>>>
>>>>>>>   I test it by the pynfs41 at your tree, and add some simple
>>>>>>>   test case as following.
>>>>>>>
>>>>>>>   I have a question about the op.secinfo_no_name(), should it take
>>>>>>>   a argument?
>>>>>>>   Right now, maybe it always request with NFS4_SECINFO_STYLE4_CURRENT_FH,
>>>>>>>   or I can't find how to set the argument.
>>>>> There was a bug in how the op.secinfo_no_name() function was auto
>>>>> constructed.  I've pushed a fix to
>>>>> my tree at git://linux-nfs.org/~iisaman/newpynfs.git.
>>>> Thanks, Fred.  I've pulled that into my repository too.
>>> But then these secinfo_no_name tests don't work ("TypeError:
>>> data.opsecinfo_no_name == None"); Mi Jinlong, would you mind looking
>>> into that?
>>   After Fred's patch, we should rebuild the test site at pynfs41 dir 
>>   with "python setup.py build".
>>
>>   And update test case, call op.secinfo_no_name() with 0 as following.
> 
> Thanks!
> 
> Could I ask for a few changes?:
> 	- The second half of testSupported2 should go into a separate
> 	  test in a separate st_secinfo file.  (It will be a very small
> 	  file for now!  But we can port some of the 4.0 tests to it
> 	  some day.)
> 	- That test should create a file in the same way that e.g. the
> 	  st_open tests do instead of assuming a file named "/tree"
> 	  exists.

  Is the following one OK?

thanks,
Mi Jinlong

>From 480d5d5272dd6d3c7bd15adafd62a4a9d2431db2 Mon Sep 17 00:00:00 2001
From: Mi Jinlong <mijinlong@cn.fujitsu.com>
Date: Sun, 2 Jan 2011 17:59:20 +0800
Subject: [PATCH] CLNT: Add some simple secinfo_no_name and secinfo tests

Add two simple secinfo_no_name tests as Bruce said:
  - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
    legal result.
  - send PUTROOTFH+SECINFO+GETFH, and/or
    PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
    NOFILEHANDLE.

also add two simple secinfo tests case.

Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
 nfs4.1/server41tests/__init__.py           |    2 +
 nfs4.1/server41tests/st_secinfo.py         |   57 ++++++++++++++++++++++++++++
 nfs4.1/server41tests/st_secinfo_no_name.py |   35 +++++++++++++++++
 3 files changed, 94 insertions(+), 0 deletions(-)
 create mode 100644 nfs4.1/server41tests/st_secinfo.py
 create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py

diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
index 22cd664..3b4411a 100644
--- a/nfs4.1/server41tests/__init__.py
+++ b/nfs4.1/server41tests/__init__.py
@@ -2,6 +2,8 @@ __all__ = ["st_exchange_id.py", # draft 21
            "st_compound.py",
            "st_create_session.py",
            "st_destroy_session.py",
+           "st_secinfo_no_name.py",
+           "st_secinfo.py",
            "st_sequence.py",
 	   "st_trunking.py",
            "st_open.py",
diff --git a/nfs4.1/server41tests/st_secinfo.py b/nfs4.1/server41tests/st_secinfo.py
new file mode 100644
index 0000000..c0e5b71
--- /dev/null
+++ b/nfs4.1/server41tests/st_secinfo.py
@@ -0,0 +1,57 @@
+from st_create_session import create_session
+from nfs4_const import *
+from environment import check, fail, use_obj, bad_sessionid, create_file
+from nfs4_type import channel_attrs4
+import nfs4_ops as op
+import nfs4lib
+
+def testSupported(t, env):
+    """Do a simple SECINFO
+
+    FLAGS: all
+    CODE: SEC1
+    """
+    name = env.testname(t)
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+
+    # Create a tmpfile for testing
+    owner = "owner_%s" % name
+    path = sess.c.homedir + [name]
+    res = create_file(sess, owner, path, access=OPEN4_SHARE_ACCESS_WRITE)
+    check(res)
+
+    # Get the filehandle of the tmpfile's parent dir
+    res = sess.compound(use_obj(sess.c.homedir) + [op.getfh()])
+    check(res)
+    fh = res.resarray[-1].object
+
+    # Just do a simple SECINFO
+    res = sess.compound([op.putfh(fh), op.secinfo(name)])
+    check(res)
+
+def testSupported2(t, env):
+    """GETFH after do a SECINFO_NO_NAME or SECINFO
+       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
+
+    FLAGS: all
+    CODE: SEC2
+    """
+    name = env.testname(t)
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+
+    # Create a tmpfile for testing
+    owner = "owner_%s" % name
+    path = sess.c.homedir + [name]
+    res = create_file(sess, owner, path, access=OPEN4_SHARE_ACCESS_WRITE)
+    check(res)
+
+    # Get the filehandle of the tmpfile's parent dir
+    res = sess.compound(use_obj(sess.c.homedir) + [op.getfh()])
+    check(res)
+    fh = res.resarray[-1].object
+
+    # GETFH after do a SECINFO should get error NFS4ERR_NOFILEHANDLE
+    res = sess.compound([op.putfh(fh), op.secinfo(name), op.getfh()])
+    check(res, NFS4ERR_NOFILEHANDLE)
diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
new file mode 100644
index 0000000..14bc410
--- /dev/null
+++ b/nfs4.1/server41tests/st_secinfo_no_name.py
@@ -0,0 +1,35 @@
+from st_create_session import create_session
+from nfs4_const import *
+from environment import check, fail, bad_sessionid, create_file
+from nfs4_type import channel_attrs4
+import nfs4_ops as op
+import nfs4lib
+
+def testSupported(t, env):
+    """Do a simple SECINFO_NO_NAME
+       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
+
+    FLAGS: all
+    CODE: SECNN1
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+
+    # Do a simple SECINFO_NO_NAME
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0)])
+    check(res)
+
+def testSupported2(t, env):
+    """GETFH after do a SECINFO_NO_NAME or SECINFO
+       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
+
+    FLAGS: all
+    CODE: SECNN2
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess = c.create_session()
+
+    # GETFH after do a SECINFO_NO_NAME should get error NFS4ERR_NOFILEHANDLE
+    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0), op.getfh()])
+    print res
+    check(res, NFS4ERR_NOFILEHANDLE)
-- 
1.7.3.4




^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nfsd4: implement secinfo_no_name
  2011-01-13  3:20                     ` Mi Jinlong
@ 2011-01-18 22:59                       ` J. Bruce Fields
  0 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2011-01-18 22:59 UTC (permalink / raw)
  To: Mi Jinlong; +Cc: Fred Isaman, J. Bruce Fields, linux-nfs

On Thu, Jan 13, 2011 at 11:20:16AM +0800, Mi Jinlong wrote:
> 
> 
> J. Bruce Fields 写道:
> > Could I ask for a few changes?:
> > 	- The second half of testSupported2 should go into a separate
> > 	  test in a separate st_secinfo file.  (It will be a very small
> > 	  file for now!  But we can port some of the 4.0 tests to it
> > 	  some day.)
> > 	- That test should create a file in the same way that e.g. the
> > 	  st_open tests do instead of assuming a file named "/tree"
> > 	  exists.
> 
>   Is the following one OK?

Yes, applied.

Thanks again for the pynfs help, getting these new features tested is
important.

--b.

> 
> thanks,
> Mi Jinlong
> 
> >From 480d5d5272dd6d3c7bd15adafd62a4a9d2431db2 Mon Sep 17 00:00:00 2001
> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
> Date: Sun, 2 Jan 2011 17:59:20 +0800
> Subject: [PATCH] CLNT: Add some simple secinfo_no_name and secinfo tests
> 
> Add two simple secinfo_no_name tests as Bruce said:
>   - send PUTROOTFH+SECINFO_NO_NAME, check that you get back a
>     legal result.
>   - send PUTROOTFH+SECINFO+GETFH, and/or
>     PUTROOTFH+SECINFO_NO_NAME+GETFH, check that the GETFH returns
>     NOFILEHANDLE.
> 
> also add two simple secinfo tests case.
> 
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  nfs4.1/server41tests/__init__.py           |    2 +
>  nfs4.1/server41tests/st_secinfo.py         |   57 ++++++++++++++++++++++++++++
>  nfs4.1/server41tests/st_secinfo_no_name.py |   35 +++++++++++++++++
>  3 files changed, 94 insertions(+), 0 deletions(-)
>  create mode 100644 nfs4.1/server41tests/st_secinfo.py
>  create mode 100644 nfs4.1/server41tests/st_secinfo_no_name.py
> 
> diff --git a/nfs4.1/server41tests/__init__.py b/nfs4.1/server41tests/__init__.py
> index 22cd664..3b4411a 100644
> --- a/nfs4.1/server41tests/__init__.py
> +++ b/nfs4.1/server41tests/__init__.py
> @@ -2,6 +2,8 @@ __all__ = ["st_exchange_id.py", # draft 21
>             "st_compound.py",
>             "st_create_session.py",
>             "st_destroy_session.py",
> +           "st_secinfo_no_name.py",
> +           "st_secinfo.py",
>             "st_sequence.py",
>  	   "st_trunking.py",
>             "st_open.py",
> diff --git a/nfs4.1/server41tests/st_secinfo.py b/nfs4.1/server41tests/st_secinfo.py
> new file mode 100644
> index 0000000..c0e5b71
> --- /dev/null
> +++ b/nfs4.1/server41tests/st_secinfo.py
> @@ -0,0 +1,57 @@
> +from st_create_session import create_session
> +from nfs4_const import *
> +from environment import check, fail, use_obj, bad_sessionid, create_file
> +from nfs4_type import channel_attrs4
> +import nfs4_ops as op
> +import nfs4lib
> +
> +def testSupported(t, env):
> +    """Do a simple SECINFO
> +
> +    FLAGS: all
> +    CODE: SEC1
> +    """
> +    name = env.testname(t)
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +
> +    # Create a tmpfile for testing
> +    owner = "owner_%s" % name
> +    path = sess.c.homedir + [name]
> +    res = create_file(sess, owner, path, access=OPEN4_SHARE_ACCESS_WRITE)
> +    check(res)
> +
> +    # Get the filehandle of the tmpfile's parent dir
> +    res = sess.compound(use_obj(sess.c.homedir) + [op.getfh()])
> +    check(res)
> +    fh = res.resarray[-1].object
> +
> +    # Just do a simple SECINFO
> +    res = sess.compound([op.putfh(fh), op.secinfo(name)])
> +    check(res)
> +
> +def testSupported2(t, env):
> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> +
> +    FLAGS: all
> +    CODE: SEC2
> +    """
> +    name = env.testname(t)
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +
> +    # Create a tmpfile for testing
> +    owner = "owner_%s" % name
> +    path = sess.c.homedir + [name]
> +    res = create_file(sess, owner, path, access=OPEN4_SHARE_ACCESS_WRITE)
> +    check(res)
> +
> +    # Get the filehandle of the tmpfile's parent dir
> +    res = sess.compound(use_obj(sess.c.homedir) + [op.getfh()])
> +    check(res)
> +    fh = res.resarray[-1].object
> +
> +    # GETFH after do a SECINFO should get error NFS4ERR_NOFILEHANDLE
> +    res = sess.compound([op.putfh(fh), op.secinfo(name), op.getfh()])
> +    check(res, NFS4ERR_NOFILEHANDLE)
> diff --git a/nfs4.1/server41tests/st_secinfo_no_name.py b/nfs4.1/server41tests/st_secinfo_no_name.py
> new file mode 100644
> index 0000000..14bc410
> --- /dev/null
> +++ b/nfs4.1/server41tests/st_secinfo_no_name.py
> @@ -0,0 +1,35 @@
> +from st_create_session import create_session
> +from nfs4_const import *
> +from environment import check, fail, bad_sessionid, create_file
> +from nfs4_type import channel_attrs4
> +import nfs4_ops as op
> +import nfs4lib
> +
> +def testSupported(t, env):
> +    """Do a simple SECINFO_NO_NAME
> +       send PUTROOTFH+SECINFO_NO_NAME, check is result legal
> +
> +    FLAGS: all
> +    CODE: SECNN1
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +
> +    # Do a simple SECINFO_NO_NAME
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0)])
> +    check(res)
> +
> +def testSupported2(t, env):
> +    """GETFH after do a SECINFO_NO_NAME or SECINFO
> +       result in a NOFILEHANDLE error, See rfc 5661 section 2.6.3.1.1.8
> +
> +    FLAGS: all
> +    CODE: SECNN2
> +    """
> +    c = env.c1.new_client(env.testname(t))
> +    sess = c.create_session()
> +
> +    # GETFH after do a SECINFO_NO_NAME should get error NFS4ERR_NOFILEHANDLE
> +    res = sess.compound([op.putrootfh(), op.secinfo_no_name(0), op.getfh()])
> +    print res
> +    check(res, NFS4ERR_NOFILEHANDLE)
> -- 
> 1.7.3.4
> 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-01-18 22:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-17 19:01 4.1 secinfo patches J. Bruce Fields
2010-12-17 19:01 ` [PATCH 1/3] nfsd4: 4.1 SECINFO should consume filehandle J. Bruce Fields
2010-12-17 19:01 ` [PATCH 2/3] nfsd4: move guts of nfsd4_lookupp into helper J. Bruce Fields
2010-12-17 19:01 ` [PATCH 3/3] nfsd4: implement secinfo_no_name J. Bruce Fields
2010-12-27  6:29   ` Mi Jinlong
2010-12-29 18:56     ` J. Bruce Fields
2010-12-30  4:13       ` Mi Jinlong
2011-01-05  1:05         ` J. Bruce Fields
2011-01-05 15:15           ` Fred Isaman
2011-01-05 15:37             ` J. Bruce Fields
2011-01-05 17:29               ` J. Bruce Fields
2011-01-06  3:54                 ` Mi Jinlong
2011-01-11 23:32                   ` J. Bruce Fields
2011-01-13  3:20                     ` Mi Jinlong
2011-01-18 22:59                       ` J. Bruce Fields

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.