[pnfs] [PATCH 11/19] nfs41: Refactor svc_process()

[J. Bruce Fields]

On Wed, Jul 16, 2008 at 12:29:45PM +0300, Benny Halevy wrote:
> From: Ricardo Labiaga <ricardo.labiaga at netapp.com>
> 
> net/sunrpc/svc.c:svc_process() is used by the NFSv4 callback service
> to process RPC requests arriving over connections initiated by the
> server.  NFSv4.1 supports callbacks over the backchannel on connections
> initiated by the client.  This patch refactors svc_process() so that
> common code can also be used by the backchannel.
> 
> Signed-off-by: Ricardo Labiaga <ricardo.labiaga at netapp.com>
> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> ---
>  net/sunrpc/svc.c |   80 +++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 50 insertions(+), 30 deletions(-)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 01c7e31..ef03377 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -824,20 +824,18 @@ svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
>  }
>  
>  /*
> - * Process the RPC request.
> + * Common routine for processing the RPC request.
>   */
> -int
> -svc_process(struct svc_rqst *rqstp)
> +static int
> +svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
>  {
>  	struct svc_program	*progp;
>  	struct svc_version	*versp = NULL;	/* compiler food */
>  	struct svc_procedure	*procp = NULL;
> -	struct kvec *		argv = &rqstp->rq_arg.head[0];
> -	struct kvec *		resv = &rqstp->rq_res.head[0];
>  	struct svc_serv		*serv = rqstp->rq_server;
>  	kxdrproc_t		xdr;
>  	__be32			*statp;
> -	u32			dir, prog, vers, proc;
> +	u32			prog, vers, proc;
>  	__be32			auth_stat, rpc_stat;
>  	int			auth_res;
>  	__be32			*reply_statp;
> @@ -847,36 +845,19 @@ svc_process(struct svc_rqst *rqstp)
>  	if (argv->iov_len < 6*4)
>  		goto err_short_len;
>  
> -	/* setup response xdr_buf.
> -	 * Initially it has just one page
> -	 */
> -	rqstp->rq_resused = 1;
> -	resv->iov_base = page_address(rqstp->rq_respages[0]);
> -	resv->iov_len = 0;
> -	rqstp->rq_res.pages = rqstp->rq_respages + 1;
> -	rqstp->rq_res.len = 0;
> -	rqstp->rq_res.page_base = 0;
> -	rqstp->rq_res.page_len = 0;
> -	rqstp->rq_res.buflen = PAGE_SIZE;
> -	rqstp->rq_res.tail[0].iov_base = NULL;
> -	rqstp->rq_res.tail[0].iov_len = 0;
>  	/* Will be turned off only in gss privacy case: */
>  	rqstp->rq_splice_ok = 1;
>  
>  	/* Setup reply header */
>  	rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp);
>  
> -	rqstp->rq_xid = svc_getu32(argv);
>  	svc_putu32(resv, rqstp->rq_xid);
>  
> -	dir  = svc_getnl(argv);
>  	vers = svc_getnl(argv);
>  
>  	/* First words of reply: */
>  	svc_putnl(resv, 1);		/* REPLY */
>  
> -	if (dir != 0)		/* direction != CALL */
> -		goto err_bad_dir;
>  	if (vers != 2)		/* RPC version number */
>  		goto err_bad_rpc;
>  
> @@ -1000,7 +981,7 @@ svc_process(struct svc_rqst *rqstp)
>   sendit:
>  	if (svc_authorise(rqstp))
>  		goto dropit;
> -	return svc_send(rqstp);
> +	return 1;		/* Caller can now send it */
>  
>   dropit:
>  	svc_authorise(rqstp);	/* doesn't hurt to call this twice */
> @@ -1014,12 +995,6 @@ err_short_len:
>  
>  	goto dropit;			/* drop request */
>  
> -err_bad_dir:
> -	svc_printk(rqstp, "bad direction %d, dropping request\n", dir);
> -
> -	serv->sv_stats->rpcbadfmt++;
> -	goto dropit;			/* drop request */
> -
>  err_bad_rpc:
>  	serv->sv_stats->rpcbadfmt++;
>  	svc_putnl(resv, 1);	/* REJECT */
> @@ -1073,6 +1048,51 @@ err_bad:
>  EXPORT_SYMBOL(svc_process);
>  
>  /*
> + * Process the RPC request.
> + */
> +int
> +svc_process(struct svc_rqst *rqstp)
> +{
> +	struct kvec		*argv = &rqstp->rq_arg.head[0];
> +	struct kvec		*resv = &rqstp->rq_res.head[0];
> +	struct svc_serv		*serv = rqstp->rq_server;
> +	u32			dir;
> +	int			error;
> +
> +	/*
> +	 * Setup response xdr_buf.
> +	 * Initially it has just one page
> +	 */
> +	rqstp->rq_resused = 1;
> +	resv->iov_base = page_address(rqstp->rq_respages[0]);
> +	resv->iov_len = 0;
> +	rqstp->rq_res.pages = rqstp->rq_respages + 1;
> +	rqstp->rq_res.len = 0;
> +	rqstp->rq_res.page_base = 0;
> +	rqstp->rq_res.page_len = 0;
> +	rqstp->rq_res.buflen = PAGE_SIZE;
> +	rqstp->rq_res.tail[0].iov_base = NULL;
> +	rqstp->rq_res.tail[0].iov_len = 0;
> +
> +	rqstp->rq_xid = svc_getu32(argv);
> +
> +	dir  = svc_getnl(argv);
> +	if (dir != 0) {
> +		/* direction != CALL */
> +		svc_printk(rqstp, "bad direction %d, dropping request\n", dir);
> +		serv->sv_stats->rpcbadfmt++;
> +		svc_drop(rqstp);
> +		return 0;
> +	}
> +
> +	error = svc_process_common(rqstp, argv, resv);
> +	if (error <= 0)
> +		return error;
> +
> +	return svc_send(rqstp);
> +}

So the return value here is:
	- -ERRNO or 0 if already dropped
	- 1 to send

Could we do the svc_send() inside svc_process_common()?  If not, let's
at least add a quick sentence to the comment on svc_process_comment()
documenting the convention.

Non-standard conventions for return values seem to be an occasional
cause of bugs, so I've got a slight phobia.

--b.