From: "J. Bruce Fields" <[email protected]>
Commit 3b299709091b "nfsd4: enforce rd_dircount" totally misunderstood
rd_dircount; it refers to total non-attribute bytes returned, not number
of directory entries returned.
Bring the code into agreement with RFC 3530 section 14.2.24.
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4xdr.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f9821ce6658a..e94457c33ad6 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2657,6 +2657,7 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
struct xdr_stream *xdr = cd->xdr;
int start_offset = xdr->buf->len;
int cookie_offset;
+ u32 name_and_cookie;
int entry_bytes;
__be32 nfserr = nfserr_toosmall;
__be64 wire_offset;
@@ -2718,7 +2719,14 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
cd->rd_maxcount -= entry_bytes;
if (!cd->rd_dircount)
goto fail;
- cd->rd_dircount--;
+ /*
+ * RFC 3530 14.2.24 describes rd_dircount as only a "hint", so
+ * let's always let through the first entry, at least:
+ */
+ name_and_cookie = 4 * XDR_QUADLEN(namlen) + 8;
+ if (name_and_cookie > cd->rd_dircount && cd->cookie_offset)
+ goto fail;
+ cd->rd_dircount -= min(cd->rd_dircount, name_and_cookie);
cd->cookie_offset = cookie_offset;
skip_entry:
cd->common.err = nfs_ok;
@@ -3321,6 +3329,10 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
}
maxcount = min_t(int, maxcount-16, bytes_left);
+ /* RFC 3530 14.2.24 allows us to ignore dircount when it's 0: */
+ if (!readdir->rd_dircount)
+ readdir->rd_dircount = INT_MAX;
+
readdir->xdr = xdr;
readdir->rd_maxcount = maxcount;
readdir->common.err = 0;
--
1.9.3
On Fri, Aug 29, 2014 at 2:55 PM, J. Bruce Fields <[email protected]> wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> Commit 3b299709091b "nfsd4: enforce rd_dircount" totally misunderstood
> rd_dircount; it refers to total non-attribute bytes returned, not number
> of directory entries returned.
>
> Bring the code into agreement with RFC 3530 section 14.2.24.
>
> Cc: [email protected]
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index f9821ce6658a..e94457c33ad6 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2657,6 +2657,7 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
> struct xdr_stream *xdr = cd->xdr;
> int start_offset = xdr->buf->len;
> int cookie_offset;
> + u32 name_and_cookie;
> int entry_bytes;
> __be32 nfserr = nfserr_toosmall;
> __be64 wire_offset;
> @@ -2718,7 +2719,14 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
> cd->rd_maxcount -= entry_bytes;
> if (!cd->rd_dircount)
> goto fail;
> - cd->rd_dircount--;
> + /*
> + * RFC 3530 14.2.24 describes rd_dircount as only a "hint", so
> + * let's always let through the first entry, at least:
> + */
> + name_and_cookie = 4 * XDR_QUADLEN(namlen) + 8;
> + if (name_and_cookie > cd->rd_dircount && cd->cookie_offset)
> + goto fail;
> + cd->rd_dircount -= min(cd->rd_dircount, name_and_cookie);
> cd->cookie_offset = cookie_offset;
> skip_entry:
> cd->common.err = nfs_ok;
> @@ -3321,6 +3329,10 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
> }
> maxcount = min_t(int, maxcount-16, bytes_left);
>
> + /* RFC 3530 14.2.24 allows us to ignore dircount when it's 0: */
> + if (!readdir->rd_dircount)
> + readdir->rd_dircount = INT_MAX;
> +
Ah... Time to change the Linux client to always set this value to 0.
It really is useless in our case.
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Fri, Aug 29, 2014 at 03:29:07PM -0400, Trond Myklebust wrote:
> On Fri, Aug 29, 2014 at 2:55 PM, J. Bruce Fields <[email protected]> wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > Commit 3b299709091b "nfsd4: enforce rd_dircount" totally misunderstood
> > rd_dircount; it refers to total non-attribute bytes returned, not number
> > of directory entries returned.
> >
> > Bring the code into agreement with RFC 3530 section 14.2.24.
> >
> > Cc: [email protected]
> > Signed-off-by: J. Bruce Fields <[email protected]>
> > ---
> > fs/nfsd/nfs4xdr.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index f9821ce6658a..e94457c33ad6 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -2657,6 +2657,7 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
> > struct xdr_stream *xdr = cd->xdr;
> > int start_offset = xdr->buf->len;
> > int cookie_offset;
> > + u32 name_and_cookie;
> > int entry_bytes;
> > __be32 nfserr = nfserr_toosmall;
> > __be64 wire_offset;
> > @@ -2718,7 +2719,14 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen,
> > cd->rd_maxcount -= entry_bytes;
> > if (!cd->rd_dircount)
> > goto fail;
> > - cd->rd_dircount--;
> > + /*
> > + * RFC 3530 14.2.24 describes rd_dircount as only a "hint", so
> > + * let's always let through the first entry, at least:
> > + */
> > + name_and_cookie = 4 * XDR_QUADLEN(namlen) + 8;
> > + if (name_and_cookie > cd->rd_dircount && cd->cookie_offset)
> > + goto fail;
> > + cd->rd_dircount -= min(cd->rd_dircount, name_and_cookie);
> > cd->cookie_offset = cookie_offset;
> > skip_entry:
> > cd->common.err = nfs_ok;
> > @@ -3321,6 +3329,10 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
> > }
> > maxcount = min_t(int, maxcount-16, bytes_left);
> >
> > + /* RFC 3530 14.2.24 allows us to ignore dircount when it's 0: */
> > + if (!readdir->rd_dircount)
> > + readdir->rd_dircount = INT_MAX;
> > +
>
> Ah... Time to change the Linux client to always set this value to 0.
> It really is useless in our case.
I didn't look at the client. Hm:
uint32_t dircount = readdir->count >> 1
...
*p++ = cpu_to_be32(dircount);
*p++ = cpu_to_be32(readdir->count);
Yeah, that's kind of arbitrary. This dircount thing is just inherently
weird. We could just revert the server back to ignoring it completely
like it used to--the spec only says it's a "hint", whatever that means.
But presumably somebody must have thought they had a use for it.
--b.