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

[Anna Schumaker <Anna.Schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>]

On 11/02/2015 04:16 PM, Chuck Lever wrote:
> 
>> On Nov 2, 2015, at 3:43 PM, Anna Schumaker <Anna.Schumaker-HgOvQuBEEgRhl2p70BpVqQ@public.gmane.orgm> wrote:
>>
>> 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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> 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;
> 
> Adding
> 
>   struct net;
> 
> here seems to help.

Yep.  I'll fix up the patch and send to Trond in a few minutes!

Anna
> 
> 
>>> /*
>>>  * 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> —
> Chuck Lever
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html