Re: [PATCH v4 16/16] NFS: Enable client side NFSv4.1 backchannel to use other transports

[Anna Schumaker <Anna.Schumaker@netapp.com>]

Hi Chuck,

Sorry for the delay in reviewing.

On 10/24/2015 05:28 PM, Chuck Lever wrote:
> Forechannel transports get their own "bc_up" method to create an
> endpoint for the backchannel service.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfs/callback.c                 |   40 +++++++------------------------------
>  include/linux/sunrpc/xprt.h       |    2 ++
>  net/sunrpc/xprtrdma/backchannel.c |   21 +++++++++++++++++++
>  net/sunrpc/xprtrdma/transport.c   |    1 +
>  net/sunrpc/xprtrdma/xprt_rdma.h   |    1 +
>  net/sunrpc/xprtsock.c             |   12 +++++++++++
>  6 files changed, 45 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 75f7c0a..a7f2e6e 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -99,17 +99,6 @@ nfs4_callback_up(struct svc_serv *serv)
>  }
>  
>  #if defined(CONFIG_NFS_V4_1)
> -static int nfs41_callback_up_net(struct svc_serv *serv, struct net *net)
> -{
> -	/*
> -	 * Create an svc_sock for the back channel service that shares the
> -	 * fore channel connection.
> -	 * Returns the input port (0) and sets the svc_serv bc_xprt on success
> -	 */
> -	return svc_create_xprt(serv, "tcp-bc", net, PF_INET, 0,
> -			      SVC_SOCK_ANONYMOUS);
> -}
> -
>  /*
>   * The callback service for NFSv4.1 callbacks
>   */
> @@ -184,11 +173,6 @@ static inline void nfs_callback_bc_serv(u32 minorversion, struct rpc_xprt *xprt,
>  		xprt->bc_serv = serv;
>  }
>  #else
> -static int nfs41_callback_up_net(struct svc_serv *serv, struct net *net)
> -{
> -	return 0;
> -}
> -
>  static void nfs_minorversion_callback_svc_setup(struct svc_serv *serv,
>  		struct svc_rqst **rqstpp, int (**callback_svc)(void *vrqstp))
>  {
> @@ -259,7 +243,8 @@ static void nfs_callback_down_net(u32 minorversion, struct svc_serv *serv, struc
>  	svc_shutdown_net(serv, net);
>  }
>  
> -static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct net *net)
> +static int nfs_callback_up_net(int minorversion, struct svc_serv *serv,
> +			       struct net *net, struct rpc_xprt *xprt)
>  {
>  	struct nfs_net *nn = net_generic(net, nfs_net_id);
>  	int ret;
> @@ -275,20 +260,11 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, struct n
>  		goto err_bind;
>  	}
>  
> -	switch (minorversion) {
> -		case 0:
> -			ret = nfs4_callback_up_net(serv, net);
> -			break;
> -		case 1:
> -		case 2:
> -			ret = nfs41_callback_up_net(serv, net);
> -			break;
> -		default:
> -			printk(KERN_ERR "NFS: unknown callback version: %d\n",
> -					minorversion);
> -			ret = -EINVAL;
> -			break;
> -	}
> +	ret = -EPROTONOSUPPORT;
> +	if (minorversion == 0)
> +		ret = nfs4_callback_up_net(serv, net);
> +	else if (xprt->ops->bc_up)
> +		ret = xprt->ops->bc_up(serv, net);
>  
>  	if (ret < 0) {
>  		printk(KERN_ERR "NFS: callback service start failed\n");
> @@ -364,7 +340,7 @@ int nfs_callback_up(u32 minorversion, struct rpc_xprt *xprt)
>  		goto err_create;
>  	}
>  
> -	ret = nfs_callback_up_net(minorversion, serv, net);
> +	ret = nfs_callback_up_net(minorversion, serv, net, xprt);
>  	if (ret < 0)
>  		goto err_net;
>  
> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> index 82c0839..9d22f43 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -54,6 +54,7 @@ enum rpc_display_format_t {
>  struct rpc_task;
>  struct rpc_xprt;
>  struct seq_file;
> +struct svc_serv;
>  
>  /*
>   * This describes a complete RPC request
> @@ -138,6 +139,7 @@ struct rpc_xprt_ops {
>  	void		(*inject_disconnect)(struct rpc_xprt *xprt);
>  	int		(*bc_setup)(struct rpc_xprt *xprt,
>  				    unsigned int min_reqs);
> +	int		(*bc_up)(struct svc_serv *serv, struct net *net);

Disabling CONFIG_SUNRPC_DEBUG gives me several:

  CC [M]  fs/nfsd/nfs4acl.o
  CC [M]  fs/nfsd/nfs4callback.o
In file included from include/linux/sunrpc/clnt.h:19:0,
                 from fs/nfsd/nfs4callback.c:34:
include/linux/sunrpc/xprt.h:142:46: error: 'struct net' declared inside parameter list [-Werror]
  int  (*bc_up)(struct svc_serv *serv, struct net *net);
                                              ^
include/linux/sunrpc/xprt.h:142:46: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]

I'm guessing an include file is missing somewhere?

Anna


>  	void		(*bc_free_rqst)(struct rpc_rqst *rqst);
>  	void		(*bc_destroy)(struct rpc_xprt *xprt,
>  				      unsigned int max_reqs);
> diff --git a/net/sunrpc/xprtrdma/backchannel.c b/net/sunrpc/xprtrdma/backchannel.c
> index 0b3387f..2dcb44f 100644
> --- a/net/sunrpc/xprtrdma/backchannel.c
> +++ b/net/sunrpc/xprtrdma/backchannel.c
> @@ -7,6 +7,7 @@
>  #include <linux/module.h>
>  #include <linux/sunrpc/xprt.h>
>  #include <linux/sunrpc/svc.h>
> +#include <linux/sunrpc/svc_xprt.h>
>  
>  #include "xprt_rdma.h"
>  
> @@ -174,6 +175,26 @@ out_err:
>  }
>  
>  /**
> + * xprt_rdma_bc_up - Create transport endpoint for backchannel service
> + * @serv: server endpoint
> + * @net: network namespace
> + *
> + * The "xprt" is an implied argument: it supplies the name of the
> + * backchannel transport class.
> + *
> + * Returns zero on success, negative errno on failure
> + */
> +int xprt_rdma_bc_up(struct svc_serv *serv, struct net *net)
> +{
> +	int ret;
> +
> +	ret = svc_create_xprt(serv, "rdma-bc", net, PF_INET, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +/**
>   * rpcrdma_bc_marshal_reply - Send backwards direction reply
>   * @rqst: buffer containing RPC reply data
>   *
> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
> index 845278e..8c545f7 100644
> --- a/net/sunrpc/xprtrdma/transport.c
> +++ b/net/sunrpc/xprtrdma/transport.c
> @@ -708,6 +708,7 @@ static struct rpc_xprt_ops xprt_rdma_procs = {
>  	.inject_disconnect	= xprt_rdma_inject_disconnect,
>  #if defined(CONFIG_SUNRPC_BACKCHANNEL)
>  	.bc_setup		= xprt_rdma_bc_setup,
> +	.bc_up			= xprt_rdma_bc_up,
>  	.bc_free_rqst		= xprt_rdma_bc_free_rqst,
>  	.bc_destroy		= xprt_rdma_bc_destroy,
>  #endif
> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
> index eb87d96e8..f8dd17b 100644
> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
> @@ -520,6 +520,7 @@ void xprt_rdma_cleanup(void);
>   */
>  #if defined(CONFIG_SUNRPC_BACKCHANNEL)
>  int xprt_rdma_bc_setup(struct rpc_xprt *, unsigned int);
> +int xprt_rdma_bc_up(struct svc_serv *, struct net *);
>  int rpcrdma_bc_post_recv(struct rpcrdma_xprt *, unsigned int);
>  void rpcrdma_bc_receive_call(struct rpcrdma_xprt *, struct rpcrdma_rep *);
>  int rpcrdma_bc_marshal_reply(struct rpc_rqst *);
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 44a81e4..dc47067 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1306,6 +1306,17 @@ static inline int _xs_tcp_read_data(struct rpc_xprt *xprt,
>  		xs_tcp_read_reply(xprt, desc) :
>  		xs_tcp_read_callback(xprt, desc);
>  }
> +
> +static int xs_tcp_bc_up(struct svc_serv *serv, struct net *net)
> +{
> +	int ret;
> +
> +	ret = svc_create_xprt(serv, "tcp-bc", net, PF_INET, 0,
> +			      SVC_SOCK_ANONYMOUS);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
>  #else
>  static inline int _xs_tcp_read_data(struct rpc_xprt *xprt,
>  					struct xdr_skb_reader *desc)
> @@ -2582,6 +2593,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
>  	.inject_disconnect	= xs_inject_disconnect,
>  #ifdef CONFIG_SUNRPC_BACKCHANNEL
>  	.bc_setup		= xprt_setup_bc,
> +	.bc_up			= xs_tcp_bc_up,
>  	.bc_free_rqst		= xprt_free_bc_rqst,
>  	.bc_destroy		= xprt_destroy_bc,
>  #endif
> 
> --
> 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
>