Minor problem in nfsv3 write

Minor problem in nfsv3 write

[Steven]

There is a minor bug in the way writes are handled in nfs3.  If the
client requests a write for more than 32k of data the server will only
write 32k but report that it wrote the whole amount.

This happens when nfs3svc_decode_writeargs caps the amount that can be
written to NFSSVC_MAXBLKSIZE (which is 32k), by limiting the amount that
it copies into the args object (fs/nfsd/nfs3xdr.c:375).  The amount that
is reported to the client is copied from the request packet, which is
the amount the client requested (fs/nfsd/nfs3proc.c:209) not the amount
which was written.

I think the right thing to do is adjust the amount in the request object
to the amount that will be written in the decoder. 

Following is an modified version of nfs3svc_decode_writeargs and diffs
against a 2.6.13 kernel which does this.

--Steven

int
nfs3svc_decode_writeargs(struct svc_rqst *rqstp, u32 *p,
                                        struct nfsd3_writeargs *args)
{
        unsigned int len, v, hdr;

        if (!(p = decode_fh(p, &args->fh))
         || !(p = xdr_decode_hyper(p, &args->offset)))
                return 0;

        args->count = ntohl(*p++);
        args->stable = ntohl(*p++);
        len = args->len = ntohl(*p++);

        hdr = (void*)p - rqstp->rq_arg.head[0].iov_base;
        if (rqstp->rq_arg.len < len + hdr)
                return 0;

        args->vec[0].iov_base = (void*)p;
        args->vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;

        if (len > NFSSVC_MAXBLKSIZE) {
                len = NFSSVC_MAXBLKSIZE;
                /* adjust count to match the amount that will be written */
                args->count = len;
        }
        v=  0;
        while (len > args->vec[v].iov_len) {
                len -= args->vec[v].iov_len;
                v++;
                args->vec[v].iov_base = page_address(rqstp->rq_argpages[v]);
                args->vec[v].iov_len = PAGE_SIZE;
        }
        args->vec[v].iov_len = len;
        args->vlen = v+1;

        /* args->count may be less than args->len since it is capped */
        return args->count <= args->len && args->vec[0].iov_len > 0;
}

*** fs/nfsd/nfs3xdr.c.orig	Fri Sep 16 18:02:12 2005
--- fs/nfsd/nfs3xdr.c	Mon Sep 19 17:35:22 2005
***************
*** 372,379 ****
  	args->vec[0].iov_base = (void*)p;
  	args->vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
  
! 	if (len > NFSSVC_MAXBLKSIZE)
  		len = NFSSVC_MAXBLKSIZE;
  	v=  0;
  	while (len > args->vec[v].iov_len) {
  		len -= args->vec[v].iov_len;
--- 372,382 ----
  	args->vec[0].iov_base = (void*)p;
  	args->vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
  
! 	if (len > NFSSVC_MAXBLKSIZE) {
  		len = NFSSVC_MAXBLKSIZE;
+ 		/* adjust count to match the amount that will be written */
+ 		args->count = len;
+ 	}
  	v=  0;
  	while (len > args->vec[v].iov_len) {
  		len -= args->vec[v].iov_len;
***************
*** 384,390 ****
  	args->vec[v].iov_len = len;
  	args->vlen = v+1;
  
! 	return args->count == args->len && args->vec[0].iov_len > 0;
  }
  
  int
--- 387,394 ----
  	args->vec[v].iov_len = len;
  	args->vlen = v+1;
  
! 	/* args->count may be less than args->len since it is capped */
! 	return args->count <= args->len && args->vec[0].iov_len > 0;
  }
  
  int


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Minor problem in nfsv3 write

[J. Bruce Fields]

On Mon, Sep 19, 2005 at 06:10:46PM -0700, Steven wrote:
> 
> There is a minor bug in the way writes are handled in nfs3.  If the
> client requests a write for more than 32k of data the server will only
> write 32k but report that it wrote the whole amount.

Our server also reports 32k as the maximum write size.  So the bug is on
the client side here, isn't it?

--b.


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Minor problem in nfsv3 write

[Peter Staubach]

J. Bruce Fields wrote:

>On Mon, Sep 19, 2005 at 06:10:46PM -0700, Steven wrote:
>  
>
>>There is a minor bug in the way writes are handled in nfs3.  If the
>>client requests a write for more than 32k of data the server will only
>>write 32k but report that it wrote the whole amount.
>>    
>>
>
>Our server also reports 32k as the maximum write size.  So the bug is on
>the client side here, isn't it?
>

This would be a bug in the client, but if the server is going to attempt
to write some of the data anyway, then it should correctly report the
number of bytes written.  Otherwise, the server should reject the entire
request.

    Thanx...

       ps


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Minor problem in nfsv3 write

[Steven]

> This would be a bug in the client, but if the server is going to attempt
> to write some of the data anyway, then it should correctly report the
> number of bytes written.  Otherwise, the server should reject the entire
> request.

I agree.  I don't think it is a good policy to tell the client that all
of the data was written when some was not.

For what it's worth, the RFC says that if the write size exceeds wtmax
then the server may write only wtmax bytes of the request.

--Steven


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Minor problem in nfsv3 write

[J. Bruce Fields]

On Tue, Sep 20, 2005 at 11:30:54AM -0700, Steven wrote:
> I agree.  I don't think it is a good policy to tell the client that all
> of the data was written when some was not.
> 
> For what it's worth, the RFC says that if the write size exceeds wtmax
> then the server may write only wtmax bytes of the request.

OK.  Could you regenerate that patch with diff -up?  (See
Documentation/SubmittingPatches.)  Don't bother sending the modified
function, it's the patch that people want.

--b.


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[PATCH] Minor problem in nfsv3 write

[Steven]

The following changes the nfs write procedure so that it reports the
number of bytes written correctly when the requested write size execeeds
the server's maximum write transfer size.  The server was reporting that
it wrote the number of bytes the client requested, but in fact it only
wrote the max transfer size.

For linux-2.6.13.2.

--Steven Procter

--- fs/nfsd/nfs3xdr.c.orig	2005-09-20 13:49:52.000000000 -0700
+++ fs/nfsd/nfs3xdr.c	2005-09-20 13:49:53.000000000 -0700
@@ -372,8 +372,10 @@ nfs3svc_decode_writeargs(struct svc_rqst
 	args->vec[0].iov_base = (void*)p;
 	args->vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
 
-	if (len > NFSSVC_MAXBLKSIZE)
+	if (len > NFSSVC_MAXBLKSIZE) {
 		len = NFSSVC_MAXBLKSIZE;
+		args->count = len;
+	}
 	v=  0;
 	while (len > args->vec[v].iov_len) {
 		len -= args->vec[v].iov_len;
@@ -384,7 +386,7 @@ nfs3svc_decode_writeargs(struct svc_rqst
 	args->vec[v].iov_len = len;
 	args->vlen = v+1;
 
-	return args->count == args->len && args->vec[0].iov_len > 0;
+	return args->count <= args->len && args->vec[0].iov_len > 0;
 }
 
 int


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Minor problem in nfsv3 write

[J. Bruce Fields]

On Tue, Sep 20, 2005 at 03:19:10PM -0700, Steven wrote:
> 
> The following changes the nfs write procedure so that it reports the
> number of bytes written correctly when the requested write size execeeds
> the server's maximum write transfer size.  The server was reporting that
> it wrote the number of bytes the client requested, but in fact it only
> wrote the max transfer size.
> 
> For linux-2.6.13.2.
> 
> --Steven Procter

Thanks.  For next time; note that these should apply with -p1, so should
have another pathname component at the beginning:

> --- fs/nfsd/nfs3xdr.c.orig	2005-09-20 13:49:52.000000000 -0700
> +++ fs/nfsd/nfs3xdr.c	2005-09-20 13:49:53.000000000 -0700

Again, this is in Documentation/SubmittingPatches.

(We all have scripts to automate the process, which are happier if
everyone follows the same conventions.)

--b.


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs