Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 23 Nov 2020 15:55:12 -0500
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH v2 087/118] NFSD: Update READDIR3args decoders to use
 struct xdr_stream
Message-ID: <20201123205512.GI32599@fieldses.org>
References: <160590425404.1340.8850646771948736468.stgit@klimt.1015granger.net>
 <160590488919.1340.2035631615670807514.stgit@klimt.1015granger.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <160590488919.1340.2035631615670807514.stgit@klimt.1015granger.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
From:   bfields@fieldses.org (J. Bruce Fields)
Precedence: bulk

On Fri, Nov 20, 2020 at 03:41:29PM -0500, Chuck Lever wrote:
> As an additional clean up, neither nfsd3_proc_readdir() nor
> nfsd3_proc_readdirplus() make use of the dircount argument, so
> remove it.

Are we technically violating the protocol if we return more than
dircount entries?

https://tools.ietf.org/html/rfc1813#page-82 doesn't say it's optional.
I suppose we'd know by now if any client actually cared.  Seems like
it'd be easy to implement, though.

Anyway, for now:

>  int
>  nfs3svc_decode_readdirplusargs(struct svc_rqst *rqstp, __be32 *p)
>  {
> +	struct xdr_stream *xdr = &rqstp->rq_xdr_stream;
>  	struct nfsd3_readdirargs *args = rqstp->rq_argp;

maybe just add "/* ignored */" to make it clear it's inentional we're
not actually using the value read into this variable?:

> +	u32 dircount;

--b.

>  
> -	p = decode_fh(p, &args->fh);
> -	if (!p)
> -		return 0;
> -	p = xdr_decode_hyper(p, &args->cookie);
> -	args->verf     = p; p += 2;
> -	args->dircount = ntohl(*p++);
> -	args->count    = ntohl(*p++);
> +	if (!svcxdr_decode_nfs_fh3(xdr, &args->fh))
> +		return XDR_DECODE_FAILED;
> +	if (xdr_stream_decode_u64(xdr, &args->cookie) < 0)
> +		return XDR_DECODE_FAILED;
> +	args->verf = xdr_inline_decode(xdr, NFS3_COOKIEVERFSIZE);
> +	if (!args->verf)
> +		return XDR_DECODE_FAILED;
> +	if (xdr_stream_decode_u32(xdr, &dircount) < 0)
> +		return XDR_DECODE_FAILED;
> +	if (xdr_stream_decode_u32(xdr, &args->count) < 0)
> +		return XDR_DECODE_FAILED;
>  
> -	return xdr_argsize_check(rqstp, p);
> +	return XDR_DECODE_DONE;
>  }
>  
>  int
> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
> index 789a364d5e69..64af5b01c5d7 100644
> --- a/fs/nfsd/xdr3.h
> +++ b/fs/nfsd/xdr3.h
> @@ -90,7 +90,6 @@ struct nfsd3_symlinkargs {
>  struct nfsd3_readdirargs {
>  	struct svc_fh		fh;
>  	__u64			cookie;
> -	__u32			dircount;
>  	__u32			count;
>  	__be32 *		verf;
>  };
>