[PATCH v2 0/2] NFSD: Add v4.2 SEEK support

[PATCH v2 0/2] NFSD: Add v4.2 SEEK support

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

These patches add server support for the NFS v4.2 operation SEEK.  The first
patch adds basic NFS v4.2 infrastructure that all future operations will rely
on.  The second patch adds the SEEK operation.

Changes in v2:
- Remove CONFIG_NFSD_V4_2_SEEK.
- Change the call to preprocess_stateid_op() to only use RD_STATE instead of
  RD_STATE | WR_STATE.

These patches and the corresponding client patch are avaliable in the [seek]
branch of:

	git://git.linux-nfs.org/projects/anna/linux-nfs.git


Questions?  Comments?  Thoughts?

Anna


Anna Schumaker (2):
  NFSD: Add generic v4.2 infrastructure
  NFSD: Implement SEEK

 fs/nfsd/nfs4proc.c   | 43 +++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/xdr4.h       | 14 ++++++++++++
 include/linux/nfs4.h | 23 ++++++++++++++++++--
 4 files changed, 138 insertions(+), 2 deletions(-)

-- 
2.1.0

[PATCH v2 1/2] NFSD: Add generic v4.2 infrastructure

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@netapp.com>

It's cleaner to introduce everything at once and have the server reply
with "not supported" than it would be to introduce extra operations when
implementing a specific one in the middle of the list.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfsd/nfs4xdr.c    | 28 ++++++++++++++++++++++++++++
 include/linux/nfs4.h | 18 ++++++++++++++++--
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f9821ce..94dde7b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1593,6 +1593,20 @@ static nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_WANT_DELEGATION]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_DESTROY_CLIENTID]	= (nfsd4_dec)nfsd4_decode_destroy_clientid,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_dec)nfsd4_decode_reclaim_complete,
+
+	/* new operations for NFSv4.2 */
+	[OP_ALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_COPY]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_COPY_NOTIFY]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_DEALLOCATE]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_IO_ADVISE]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_LAYOUTERROR]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_LAYOUTSTATS]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_OFFLOAD_CANCEL]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_OFFLOAD_STATUS]	= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_READ_PLUS]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
 };
 
 static inline bool
@@ -3822,6 +3836,20 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_WANT_DELEGATION]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_DESTROY_CLIENTID]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_RECLAIM_COMPLETE]	= (nfsd4_enc)nfsd4_encode_noop,
+
+	/* NFSv4.2 operations */
+	[OP_ALLOCATE]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_COPY]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_COPY_NOTIFY]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_DEALLOCATE]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_IO_ADVISE]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_LAYOUTERROR]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_LAYOUTSTATS]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_OFFLOAD_CANCEL]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_OFFLOAD_STATUS]	= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_READ_PLUS]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_WRITE_SAME]		= (nfsd4_enc)nfsd4_encode_noop,
 };
 
 /*
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 79b2a0f..cf38224 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -110,6 +110,20 @@ enum nfs_opnum4 {
 	OP_DESTROY_CLIENTID = 57,
 	OP_RECLAIM_COMPLETE = 58,
 
+	/* nfs42 */
+	OP_ALLOCATE = 59,
+	OP_COPY = 60,
+	OP_COPY_NOTIFY = 61,
+	OP_DEALLOCATE = 62,
+	OP_IO_ADVISE = 63,
+	OP_LAYOUTERROR = 64,
+	OP_LAYOUTSTATS = 65,
+	OP_OFFLOAD_CANCEL = 66,
+	OP_OFFLOAD_STATUS = 67,
+	OP_READ_PLUS = 68,
+	OP_SEEK = 69,
+	OP_WRITE_SAME = 70,
+
 	OP_ILLEGAL = 10044,
 };
 
@@ -117,10 +131,10 @@ enum nfs_opnum4 {
 Needs to be updated if more operations are defined in future.*/
 
 #define FIRST_NFS4_OP	OP_ACCESS
-#define LAST_NFS4_OP 	OP_RECLAIM_COMPLETE
+#define LAST_NFS4_OP 	OP_WRITE_SAME
 #define LAST_NFS40_OP	OP_RELEASE_LOCKOWNER
 #define LAST_NFS41_OP	OP_RECLAIM_COMPLETE
-#define LAST_NFS42_OP	OP_RECLAIM_COMPLETE
+#define LAST_NFS42_OP	OP_WRITE_SAME
 
 enum nfsstat4 {
 	NFS4_OK = 0,
-- 
2.1.0

Re: [PATCH v2 1/2] NFSD: Add generic v4.2 infrastructure

[Christoph Hellwig]

On Tue, Sep 02, 2014 at 01:30:37PM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@netapp.com>
> 
> It's cleaner to introduce everything at once and have the server reply
> with "not supported" than it would be to introduce extra operations when
> implementing a specific one in the middle of the list.
> 
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 2/2] NFSD: Implement SEEK

[Anna.Schumaker]

From: Anna Schumaker <Anna.Schumaker@netapp.com>

This patch adds server support for the NFS v4.2 operation SEEK, which
returns the position of the next hole or data segment in a file.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfsd/nfs4proc.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c    | 36 ++++++++++++++++++++++++++++++++++--
 fs/nfsd/xdr4.h       | 14 ++++++++++++++
 include/linux/nfs4.h |  5 +++++
 4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5e0dc52..aed5b88 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1013,6 +1013,43 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	return status;
 }
 
+static __be32
+nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+		struct nfsd4_seek *seek)
+{
+	struct file *file;
+	__be32 status = nfs_ok;
+
+	status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
+					    &seek->seek_stateid,
+					    RD_STATE, &file);
+	if (status) {
+		dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n");
+		return status;
+	}
+
+	switch (seek->seek_whence) {
+	case NFS4_CONTENT_DATA:
+		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
+		break;
+	case NFS4_CONTENT_HOLE:
+		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
+		break;
+	default:
+		status = nfserr_union_notsupp;
+		goto out;
+	}
+
+	if (seek->seek_pos < 0)
+		status = nfserrno(seek->seek_pos);
+	else if (seek->seek_pos >= i_size_read(file_inode(file)))
+		seek->seek_eof = true;
+
+out:
+	fput(file);
+	return status;
+}
+
 /* This routine never returns NFS_OK!  If there are no other errors, it
  * will return NFSERR_SAME or NFSERR_NOT_SAME depending on whether the
  * attributes matched.  VERIFY is implemented by mapping NFSERR_SAME
@@ -1881,6 +1918,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
 		.op_get_currentstateid = (stateid_getter)nfsd4_get_freestateid,
 		.op_rsize_bop = (nfsd4op_rsize)nfsd4_only_status_rsize,
 	},
+
+	/* NFSv4.2 operations */
+	[OP_SEEK] = {
+		.op_func = (nfsd4op_func)nfsd4_seek,
+		.op_name = "OP_SEEK",
+	},
 };
 
 int nfsd4_max_reply(struct svc_rqst *rqstp, struct nfsd4_op *op)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 94dde7b..57b3973 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1521,6 +1521,22 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
 }
 
 static __be32
+nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
+{
+	DECODE_HEAD;
+
+	status = nfsd4_decode_stateid(argp, &seek->seek_stateid);
+	if (status)
+		return status;
+
+	READ_BUF(12);
+	p = xdr_decode_hyper(p, &seek->seek_offset);
+	seek->seek_whence = be32_to_cpup(p);
+
+	DECODE_TAIL;
+}
+
+static __be32
 nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
 {
 	return nfs_ok;
@@ -1605,7 +1621,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_OFFLOAD_CANCEL]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_OFFLOAD_STATUS]	= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_READ_PLUS]		= (nfsd4_dec)nfsd4_decode_notsupp,
-	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
 };
 
@@ -3765,6 +3781,22 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
 }
 
 static __be32
+nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
+		  struct nfsd4_seek *seek)
+{
+	__be32 *p;
+
+	if (nfserr)
+		return nfserr;
+
+	p = xdr_reserve_space(&resp->xdr, 12);
+	*p++ = cpu_to_be32(seek->seek_eof);
+	p = xdr_encode_hyper(p, seek->seek_pos);
+
+	return nfserr;
+}
+
+static __be32
 nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
 {
 	return nfserr;
@@ -3848,7 +3880,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_OFFLOAD_CANCEL]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_OFFLOAD_STATUS]	= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_READ_PLUS]		= (nfsd4_enc)nfsd4_encode_noop,
-	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_enc)nfsd4_encode_noop,
 };
 
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 465e779..5720e94 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -428,6 +428,17 @@ struct nfsd4_reclaim_complete {
 	u32 rca_one_fs;
 };
 
+struct nfsd4_seek {
+	/* request */
+	stateid_t	seek_stateid;
+	loff_t		seek_offset;
+	u32		seek_whence;
+
+	/* response */
+	u32		seek_eof;
+	loff_t		seek_pos;
+};
+
 struct nfsd4_op {
 	int					opnum;
 	__be32					status;
@@ -473,6 +484,9 @@ struct nfsd4_op {
 		struct nfsd4_reclaim_complete	reclaim_complete;
 		struct nfsd4_test_stateid	test_stateid;
 		struct nfsd4_free_stateid	free_stateid;
+
+		/* NFSv4.2 */
+		struct nfsd4_seek		seek;
 	} u;
 	struct nfs4_replay *			replay;
 };
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index cf38224..026b0c0 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -550,4 +550,9 @@ struct nfs4_deviceid {
 	char data[NFS4_DEVICEID4_SIZE];
 };
 
+enum data_content4 {
+	NFS4_CONTENT_DATA		= 0,
+	NFS4_CONTENT_HOLE		= 1,
+};
+
 #endif
-- 
2.1.0

Re: [PATCH v2 2/2] NFSD: Implement SEEK

[Christoph Hellwig]

> +	switch (seek->seek_whence) {
> +	case NFS4_CONTENT_DATA:
> +		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
> +		break;
> +	case NFS4_CONTENT_HOLE:
> +		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
> +		break;
> +	default:
> +		status = nfserr_union_notsupp;
> +		goto out;
> +	}

nipick: maybe just assign pos in the switch, and have a single
vfs_llseek call?

Also this might want a comment that vfs_llseek is changing file->f_pos,
but nothing in NFSD should ever rely on file->f_pos.

>  static __be32
> +nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
> +		  struct nfsd4_seek *seek)
> +{
> +	__be32 *p;
> +
> +	if (nfserr)
> +		return nfserr;
> +
> +	p = xdr_reserve_space(&resp->xdr, 12);

nipick: can you replace the 12 by a "4 + 8"?  I think having one
literal for each field later encoded helps reading the XDR code a lot.
And although it might not matter for a trivial encoder like this it
set standards for future copy and paste code.

Re: [PATCH v2 2/2] NFSD: Implement SEEK

[Anna Schumaker]

On 09/09/2014 01:56 PM, Christoph Hellwig wrote:
>> +	switch (seek->seek_whence) {
>> +	case NFS4_CONTENT_DATA:
>> +		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
>> +		break;
>> +	case NFS4_CONTENT_HOLE:
>> +		seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
>> +		break;
>> +	default:
>> +		status = nfserr_union_notsupp;
>> +		goto out;
>> +	}
> 
> nipick: maybe just assign pos in the switch, and have a single
> vfs_llseek call?
> 
> Also this might want a comment that vfs_llseek is changing file->f_pos,
> but nothing in NFSD should ever rely on file->f_pos.

Sure.

> 
>>  static __be32
>> +nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
>> +		  struct nfsd4_seek *seek)
>> +{
>> +	__be32 *p;
>> +
>> +	if (nfserr)
>> +		return nfserr;
>> +
>> +	p = xdr_reserve_space(&resp->xdr, 12);
> 
> nipick: can you replace the 12 by a "4 + 8"?  I think having one
> literal for each field later encoded helps reading the XDR code a lot.
> And although it might not matter for a trivial encoder like this it
> set standards for future copy and paste code.

I can change that, too.  Thanks for looking!

Anna
> 

Re: [PATCH v2 0/2] NFSD: Add v4.2 SEEK support

[J. Bruce Fields]

On Tue, Sep 02, 2014 at 01:30:36PM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> These patches add server support for the NFS v4.2 operation SEEK.  The first
> patch adds basic NFS v4.2 infrastructure that all future operations will rely
> on.  The second patch adds the SEEK operation.
> 
> Changes in v2:
> - Remove CONFIG_NFSD_V4_2_SEEK.
> - Change the call to preprocess_stateid_op() to only use RD_STATE instead of
>   RD_STATE | WR_STATE.
> 
> These patches and the corresponding client patch are avaliable in the [seek]
> branch of:
> 
> 	git://git.linux-nfs.org/projects/anna/linux-nfs.git
> 
> 
> Questions?  Comments?  Thoughts?

They look OK to me.  I'll probably merge them once there's evidence
they're likely to be merged on the client side too.

Also: has anyone done wireshark support for this yet?

--b.

> 
> Anna
> 
> 
> Anna Schumaker (2):
>   NFSD: Add generic v4.2 infrastructure
>   NFSD: Implement SEEK
> 
>  fs/nfsd/nfs4proc.c   | 43 +++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4xdr.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/xdr4.h       | 14 ++++++++++++
>  include/linux/nfs4.h | 23 ++++++++++++++++++--
>  4 files changed, 138 insertions(+), 2 deletions(-)
> 
> -- 
> 2.1.0
> 
> --
> 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

Re: [PATCH v2 0/2] NFSD: Add v4.2 SEEK support

[J. Bruce Fields]

On Tue, Sep 02, 2014 at 03:10:43PM -0400, J. Bruce Fields wrote:
> On Tue, Sep 02, 2014 at 01:30:36PM -0400, Anna.Schumaker@netapp.com wrote:
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > 
> > These patches add server support for the NFS v4.2 operation SEEK.  The first
> > patch adds basic NFS v4.2 infrastructure that all future operations will rely
> > on.  The second patch adds the SEEK operation.
> > 
> > Changes in v2:
> > - Remove CONFIG_NFSD_V4_2_SEEK.
> > - Change the call to preprocess_stateid_op() to only use RD_STATE instead of
> >   RD_STATE | WR_STATE.
> > 
> > These patches and the corresponding client patch are avaliable in the [seek]
> > branch of:
> > 
> > 	git://git.linux-nfs.org/projects/anna/linux-nfs.git
> > 
> > 
> > Questions?  Comments?  Thoughts?
> 
> They look OK to me.  I'll probably merge them once there's evidence
> they're likely to be merged on the client side too.
> 
> Also: has anyone done wireshark support for this yet?

Are you sending a new version of these?

(And ditto for the allocate patches.)

As far as I can tell review comments were mostly nits.

--b.