Return-Path: Received: from aserp1040.oracle.com ([141.146.126.69]:48516 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753722AbbKBVQl convert rfc822-to-8bit (ORCPT ); Mon, 2 Nov 2015 16:16:41 -0500 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: [PATCH v4 16/16] NFS: Enable client side NFSv4.1 backchannel to use other transports From: Chuck Lever In-Reply-To: <5637CAFC.7070805@Netapp.com> Date: Mon, 2 Nov 2015 16:16:36 -0500 Cc: linux-rdma@vger.kernel.org, Linux NFS Mailing List Message-Id: <920F59C9-B37D-47AA-ADE8-F72B1958FE68@oracle.com> References: <20151024211707.4715.61650.stgit@manet.1015granger.net> <20151024212832.4715.39528.stgit@manet.1015granger.net> <5637CAFC.7070805@Netapp.com> To: Anna Schumaker Sender: linux-nfs-owner@vger.kernel.org List-ID: > On Nov 2, 2015, at 3:43 PM, Anna Schumaker 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 >> --- >> 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. >> /* >> * 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 >> #include >> #include >> +#include >> >> #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 >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html — Chuck Lever