[PATCH_V6] NFSv4 callback find client fix Version 6

[PATCH_V6] NFSv4 callback find client fix Version 6

[andros]

Version 6 of these patches
Applies to Tronds nfs-for-next branch

Responded to Version 5 comments.

The back channel server svc_process_common RPC layer pg_authenticate call
[nfs_callback_authenticate] is shared by both the NFSv4.0 and the NFSv4.1
callback threads. It authenticates the incoming request by finding (and
referencing) an nfs_client struct based on the incoming request address
and the NFS version (4). This is akin to the NFS server which authenticates
requests by matching the server address to the exports file client list.

Since there is no minorversion in the search, it may find the wrong
nfs_client struct. For the nfsv4.0 callback service thread, this means it
could find an NFSv4.1 nfs_client. For the NFSv4.1 callback service thread, it
could find an NFSv4.0 instead of v4.1, or find an NFSv4.1 nfs_client with the
wrong session.

The nfs_client is dereferenced at the end of pg_authenticate. Another
nfs_find_client call is done in the NFS layouer per operation dispatcher
routines for NFSv4.0 and in the cb_sequence operation dispatcher routine for
NFSv4.1 after decoding.

This means the callback server could start processing a callback, passing
the pg_authenticate test, and have the nfs_client struct freed between the
pg_authenticate call and the dispatcher operation call. Or, it could have
found the wrong nfs_client in the pg_authenticate call.

The current code has this behavior: If the nfs_client is not found in
pg_authenticate, the request is simply dropped (SVC_DROP). If an nfs_client
is not found in the dispatcher routines NFS4ERR_BADSESSION is returned for
v4.1 requests and NFS4ERR_BADHANDLE for v4.0 requests.

The fix is to implement the v4.0 SETCLIENTID callback_ident and use it to find
the one and only client for v4.0 callbacks, and to associate the sessionid
with the sessions based callback service (v4.1) and use it to identify the
one and only client. This can be done in the NFS layer, in CB_COMPOUND header
processing for v4.0 and in CB_SEQUENCE processing in v4.1.
In both cases, a reference to the found client is held across CB_COMPOUND
processing.
The pg_authenticate method does not have the callback_ident for CB_NULL or
CB_COMPOUND v4.0 processing, so just the server address, nfsversion and
minorversion is used for the client search

For sessions based callback service, the sessionID can't be set until the
return of the CREATE_SESSION call, yet the CB_NULL ping from a server can
be initiated by the server at the receipt of the CREATE_SESSION call before
the response is sent. So for CB_NULL, the sessionid is (usually) not set, and
the sessionid is not used for CB_NULL pg_authenticate client searches.

New transport for back channel and bug fixes
0001-SUNRPC-move-svc_drop-to-caller-of-svc_process_common.patch
0002-SUNRPC-fix-bc_send-print.patch
0003-SUNRPC-new-transport-for-the-NFSv4.1-shared-back-cha.patch
0004-NFS-use-svc_create_xprt-for-NFSv4.1-callback-service.patch
0005-NFS-do-not-clear-minor-version-at-nfs_client-free.patch

New callback functionality
0006-NFS-implement-v4.0-callback_ident.patch
0007-NFS-associate-sessionid-with-callback-connection.patch
0008-NFS-refactor-nfs_find_client-and-reference-nfs_clien.patch
0009-NFS-RPC_AUTH_GSS-unsupported-on-v4.1-back-channel.patch
0010-NFS-add-session-back-channel-draining.patch

Rename
0011-NFS-rename-client-back-channel-transport-field.patch

-->Andy

[PATCH_V6 01/11] SUNRPC move svc_drop to caller of svc_process_common

[andros]

From: Andy Adamson <andros@netapp.com>

The NFSv4.1 shared back channel does not need to call svc_drop because the
callback service never outlives the single connection it services, and it
reuses it's buffers and keeps the trasport.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 net/sunrpc/svc.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 6359c42..606d182 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1147,7 +1147,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
  dropit:
 	svc_authorise(rqstp);	/* doesn't hurt to call this twice */
 	dprintk("svc: svc_process dropit\n");
-	svc_drop(rqstp);
 	return 0;
 
 err_short_len:
@@ -1218,7 +1217,6 @@ svc_process(struct svc_rqst *rqstp)
 	struct kvec		*resv = &rqstp->rq_res.head[0];
 	struct svc_serv		*serv = rqstp->rq_server;
 	u32			dir;
-	int			error;
 
 	/*
 	 * Setup response xdr_buf.
@@ -1246,11 +1244,13 @@ svc_process(struct svc_rqst *rqstp)
 		return 0;
 	}
 
-	error = svc_process_common(rqstp, argv, resv);
-	if (error <= 0)
-		return error;
-
-	return svc_send(rqstp);
+	/* Returns 1 for send, 0 for drop */
+	if (svc_process_common(rqstp, argv, resv))
+		return svc_send(rqstp);
+	else {
+		svc_drop(rqstp);
+		return 0;
+	}
 }
 
 #if defined(CONFIG_NFS_V4_1)
@@ -1264,7 +1264,6 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 {
 	struct kvec	*argv = &rqstp->rq_arg.head[0];
 	struct kvec	*resv = &rqstp->rq_res.head[0];
-	int 		error;
 
 	/* Build the svc_rqst used by the common processing routine */
 	rqstp->rq_xprt = serv->bc_xprt;
@@ -1292,12 +1291,15 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	svc_getu32(argv);	/* XID */
 	svc_getnl(argv);	/* CALLDIR */
 
-	error = svc_process_common(rqstp, argv, resv);
-	if (error <= 0)
-		return error;
-
-	memcpy(&req->rq_snd_buf, &rqstp->rq_res, sizeof(req->rq_snd_buf));
-	return bc_send(req);
+	/* Returns 1 for send, 0 for drop */
+	if (svc_process_common(rqstp, argv, resv)) {
+		memcpy(&req->rq_snd_buf, &rqstp->rq_res,
+						sizeof(req->rq_snd_buf));
+		return bc_send(req);
+	} else {
+		/* Nothing to do to drop request */
+		return 0;
+	}
 }
 EXPORT_SYMBOL(bc_svc_process);
 #endif /* CONFIG_NFS_V4_1 */
-- 
1.6.6

[PATCH_V6 02/11] SUNRPC fix bc_send print

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 net/sunrpc/bc_svc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/bc_svc.c b/net/sunrpc/bc_svc.c
index 7dcfe0c..1dd1a68 100644
--- a/net/sunrpc/bc_svc.c
+++ b/net/sunrpc/bc_svc.c
@@ -59,8 +59,8 @@ int bc_send(struct rpc_rqst *req)
 		ret = task->tk_status;
 		rpc_put_task(task);
 	}
-	return ret;
 	dprintk("RPC:       bc_send ret= %d\n", ret);
+	return ret;
 }
 
 #endif /* CONFIG_NFS_V4_1 */
-- 
1.6.6

[PATCH_V6 03/11] SUNRPC new transport for the NFSv4.1 shared back channel

[andros]

From: Andy Adamson <andros@netapp.com>

Move the current sock create and destroy routines into the new transport ops.
Back channel socket will be destroyed by the svc_closs_all call in svc_destroy.

Added check: only TCP supported on shared back channel.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 include/linux/sunrpc/svcsock.h |    1 +
 net/sunrpc/svc.c               |    4 --
 net/sunrpc/svcsock.c           |  118 +++++++++++++++++++++++++++++++++-------
 3 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
index 1b353a7..3a45a80 100644
--- a/include/linux/sunrpc/svcsock.h
+++ b/include/linux/sunrpc/svcsock.h
@@ -45,6 +45,7 @@ int		svc_sock_names(struct svc_serv *serv, char *buf,
 int		svc_addsock(struct svc_serv *serv, const int fd,
 					char *name_return, const size_t len);
 void		svc_init_xprt_sock(void);
+void		svc_init_bc_xprt_sock(void);
 void		svc_cleanup_xprt_sock(void);
 struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot);
 void		svc_sock_destroy(struct svc_xprt *);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 606d182..261e2d1 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -488,10 +488,6 @@ svc_destroy(struct svc_serv *serv)
 	if (svc_serv_is_pooled(serv))
 		svc_pool_map_put();
 
-#if defined(CONFIG_NFS_V4_1)
-	svc_sock_destroy(serv->bc_xprt);
-#endif /* CONFIG_NFS_V4_1 */
-
 	svc_unregister(serv);
 	kfree(serv->sv_pools);
 	kfree(serv);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 07919e1..7c29be6 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -66,6 +66,13 @@ static void		svc_sock_free(struct svc_xprt *);
 static struct svc_xprt *svc_create_socket(struct svc_serv *, int,
 					  struct net *, struct sockaddr *,
 					  int, int);
+#if defined(CONFIG_NFS_V4_1)
+static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
+					     struct net *, struct sockaddr *,
+					     int, int);
+static void svc_bc_sock_free(struct svc_xprt *xprt);
+#endif /* CONFIG_NFS_V4_1 */
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 static struct lock_class_key svc_key[2];
 static struct lock_class_key svc_slock_key[2];
@@ -1184,6 +1191,75 @@ static struct svc_xprt *svc_tcp_create(struct svc_serv *serv,
 	return svc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
 }
 
+#if defined(CONFIG_NFS_V4_1)
+static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
+					     struct net *, struct sockaddr *,
+					     int, int);
+static void svc_bc_sock_free(struct svc_xprt *xprt);
+
+static struct svc_xprt *svc_bc_tcp_create(struct svc_serv *serv,
+				       struct net *net,
+				       struct sockaddr *sa, int salen,
+				       int flags)
+{
+	return svc_bc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
+}
+
+static void svc_bc_tcp_sock_detach(struct svc_xprt *xprt)
+{
+}
+
+/* These bc ops are never called. The shared fore channel is used instead */
+static int svc_bc_tcp_recvfrom(struct svc_rqst *rqstp)
+{
+	return 0;
+}
+
+static int svc_bc_tcp_sendto(struct svc_rqst *rqstp)
+{
+	return 0;
+}
+
+static void svc_bc_release_skb(struct svc_rqst *rqstp)
+{
+}
+
+static int svc_bc_tcp_has_wspace(struct svc_xprt *xprt)
+{
+	return 0;
+}
+
+static struct svc_xprt *svc_bc_tcp_accept(struct svc_xprt *xprt)
+{
+	return NULL;
+}
+
+static struct svc_xprt_ops svc_tcp_bc_ops = {
+	.xpo_create = svc_bc_tcp_create,
+	.xpo_recvfrom = svc_bc_tcp_recvfrom,
+	.xpo_sendto = svc_bc_tcp_sendto,
+	.xpo_release_rqst = svc_bc_release_skb,
+	.xpo_detach = svc_bc_tcp_sock_detach,
+	.xpo_free = svc_bc_sock_free,
+	.xpo_prep_reply_hdr = svc_tcp_prep_reply_hdr,
+	.xpo_has_wspace = svc_bc_tcp_has_wspace,
+	.xpo_accept = svc_bc_tcp_accept,
+};
+
+static struct svc_xprt_class svc_tcp_bc_class = {
+	.xcl_name = "tcp-bc",
+	.xcl_owner = THIS_MODULE,
+	.xcl_ops = &svc_tcp_bc_ops,
+	.xcl_max_payload = RPCSVC_MAXPAYLOAD_TCP,
+};
+
+void svc_init_bc_xprt_sock(void)
+{
+	svc_reg_xprt_class(&svc_tcp_bc_class);
+}
+EXPORT_SYMBOL_GPL(svc_init_bc_xprt_sock);
+#endif /* CONFIG_NFS_V4_1 */
+
 static struct svc_xprt_ops svc_tcp_ops = {
 	.xpo_create = svc_tcp_create,
 	.xpo_recvfrom = svc_tcp_recvfrom,
@@ -1509,41 +1585,43 @@ static void svc_sock_free(struct svc_xprt *xprt)
 	kfree(svsk);
 }
 
+#if defined(CONFIG_NFS_V4_1)
 /*
- * Create a svc_xprt.
- *
- * For internal use only (e.g. nfsv4.1 backchannel).
- * Callers should typically use the xpo_create() method.
+ * Create a back channel svc_xprt which shares the fore channel socket.
  */
-struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot)
+static struct svc_xprt *svc_bc_create_socket(struct svc_serv *serv,
+					     int protocol,
+					     struct net *net,
+					     struct sockaddr *sin, int len,
+					     int flags)
 {
 	struct svc_sock *svsk;
-	struct svc_xprt *xprt = NULL;
+	struct svc_xprt *xprt;
+
+	if (protocol != IPPROTO_TCP) {
+		printk(KERN_WARNING "svc: only TCP sockets"
+			" supported on shared back channel\n");
+		return ERR_PTR(-EINVAL);
+	}
 
-	dprintk("svc: %s\n", __func__);
 	svsk = kzalloc(sizeof(*svsk), GFP_KERNEL);
 	if (!svsk)
-		goto out;
+		return ERR_PTR(-ENOMEM);
 
 	xprt = &svsk->sk_xprt;
-	if (prot == IPPROTO_TCP)
-		svc_xprt_init(&svc_tcp_class, xprt, serv);
-	else if (prot == IPPROTO_UDP)
-		svc_xprt_init(&svc_udp_class, xprt, serv);
-	else
-		BUG();
-out:
-	dprintk("svc: %s return %p\n", __func__, xprt);
+	svc_xprt_init(&svc_tcp_bc_class, xprt, serv);
+
+	serv->bc_xprt = xprt;
+
 	return xprt;
 }
-EXPORT_SYMBOL_GPL(svc_sock_create);
 
 /*
- * Destroy a svc_sock.
+ * Free a back channel svc_sock.
  */
-void svc_sock_destroy(struct svc_xprt *xprt)
+static void svc_bc_sock_free(struct svc_xprt *xprt)
 {
 	if (xprt)
 		kfree(container_of(xprt, struct svc_sock, sk_xprt));
 }
-EXPORT_SYMBOL_GPL(svc_sock_destroy);
+#endif /* CONFIG_NFS_V4_1 */
-- 
1.6.6

[PATCH_V6 04/11] NFS use svc_create_xprt for NFSv4.1 callback service

[andros]

From: Andy Adamson <andros@netapp.com>

The new back channel transport means we call the normal creation routine as
well as svc_xprt_put.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/callback.c |   31 +++++++++++++++++++++----------
 1 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 93a8b3b..343d543 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -177,30 +177,41 @@ nfs41_callback_svc(void *vrqstp)
 struct svc_rqst *
 nfs41_callback_up(struct svc_serv *serv, struct rpc_xprt *xprt)
 {
-	struct svc_xprt *bc_xprt;
-	struct svc_rqst *rqstp = ERR_PTR(-ENOMEM);
+	struct svc_rqst *rqstp;
+	int ret;
 
-	dprintk("--> %s\n", __func__);
-	/* Create a svc_sock for the service */
-	bc_xprt = svc_sock_create(serv, xprt->prot);
-	if (!bc_xprt)
+	/* register the back channel transport class */
+	svc_init_bc_xprt_sock();
+
+	/*
+	 * 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
+	 */
+	ret = svc_create_xprt(serv, "tcp-bc", &init_net, PF_INET, 0,
+			      SVC_SOCK_ANONYMOUS);
+	if (ret < 0) {
+		rqstp = ERR_PTR(ret);
 		goto out;
+	}
 
 	/*
 	 * Save the svc_serv in the transport so that it can
 	 * be referenced when the session backchannel is initialized
 	 */
-	serv->bc_xprt = bc_xprt;
 	xprt->bc_serv = serv;
 
 	INIT_LIST_HEAD(&serv->sv_cb_list);
 	spin_lock_init(&serv->sv_cb_lock);
 	init_waitqueue_head(&serv->sv_cb_waitq);
 	rqstp = svc_prepare_thread(serv, &serv->sv_pools[0]);
-	if (IS_ERR(rqstp))
-		svc_sock_destroy(bc_xprt);
+	if (IS_ERR(rqstp)) {
+		svc_xprt_put(serv->bc_xprt);
+		serv->bc_xprt = NULL;
+	}
 out:
-	dprintk("--> %s return %p\n", __func__, rqstp);
+	dprintk("--> %s return %ld\n", __func__,
+		IS_ERR(rqstp) ? PTR_ERR(rqstp) : 0);
 	return rqstp;
 }
 
-- 
1.6.6

[PATCH_V6 05/11] NFS do not clear minor version at nfs_client free

[andros]

From: Andy Adamson <andros@netapp.com>

Resetting the client minor version operations causes nfs4_destroy_callback
to fail to shutdown the NFSv4.1 callback service.

There is no reason to reset the client minorversion operations when the
nfs_client struct is being freed.

Remove the minorverion reset and rename the function.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/client.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 0870d0d..855add6 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -170,21 +170,17 @@ error_0:
 }
 
 #ifdef CONFIG_NFS_V4
-/*
- * Clears/puts all minor version specific parts from an nfs_client struct
- * reverting it to minorversion 0.
- */
-static void nfs4_clear_client_minor_version(struct nfs_client *clp)
-{
 #ifdef CONFIG_NFS_V4_1
-	if (nfs4_has_session(clp)) {
+static void nfs4_shutdown_session(struct nfs_client *clp)
+{
+	if (nfs4_has_session(clp))
 		nfs4_destroy_session(clp->cl_session);
-		clp->cl_session = NULL;
-	}
-
-	clp->cl_mvops = nfs_v4_minor_ops[0];
-#endif /* CONFIG_NFS_V4_1 */
 }
+#else /* CONFIG_NFS_V4_1 */
+static void nfs4_shutdown_session(struct nfs_client *clp)
+{
+}
+#endif /* CONFIG_NFS_V4_1 */
 
 /*
  * Destroy the NFS4 callback service
@@ -199,7 +195,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
 {
 	if (__test_and_clear_bit(NFS_CS_RENEWD, &clp->cl_res_state))
 		nfs4_kill_renewd(clp);
-	nfs4_clear_client_minor_version(clp);
+	nfs4_shutdown_session(clp);
 	nfs4_destroy_callback(clp);
 	if (__test_and_clear_bit(NFS_CS_IDMAP, &clp->cl_res_state))
 		nfs_idmap_delete(clp);
-- 
1.6.6

[PATCH_V6 06/11] NFS implement v4.0 callback_ident

[andros]

From: Andy Adamson <andros@netapp.com>

Provide a unique callback identifier per SETCLIENTID call used to identify the
v4.0 callback service associated with the clientid.

Do not worry about wrap around. Zero value means unset.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4proc.c         |    5 +++++
 fs/nfs/nfs4state.c        |    1 +
 include/linux/nfs_fs_sb.h |    1 +
 include/linux/nfs_xdr.h   |    1 +
 4 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 78b0899..6652e39 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3470,6 +3470,8 @@ do_state_recovery:
 	return -EAGAIN;
 }
 
+static u32 current_cb_ident = 1;
+
 int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 		unsigned short port, struct rpc_cred *cred,
 		struct nfs4_setclientid_res *res)
@@ -3478,6 +3480,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 	struct nfs4_setclientid setclientid = {
 		.sc_verifier = &sc_verifier,
 		.sc_prog = program,
+		.sc_cb_ident = current_cb_ident++,
 	};
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
@@ -3522,6 +3525,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 			if (++clp->cl_id_uniquifier == 0)
 				break;
 	}
+	if (status == NFS_OK)
+		res->cb_ident = setclientid.sc_cb_ident;
 	return status;
 }
 
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index f575a31..fe61422 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -79,6 +79,7 @@ int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
 	if (status != 0)
 		goto out;
 	clp->cl_clientid = clid.clientid;
+	clp->cl_cb_ident = clid.cb_ident;
 	nfs4_schedule_state_renewal(clp);
 out:
 	return status;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 452d964..1eaa054 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -71,6 +71,7 @@ struct nfs_client {
 	 */
 	char			cl_ipaddr[48];
 	unsigned char		cl_id_uniquifier;
+	u32			cl_cb_ident;	/* v4.0 callback identifier */
 	const struct nfs4_minor_version_ops *cl_mvops;
 #endif /* CONFIG_NFS_V4 */
 
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 236e7e4..d325def 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -869,6 +869,7 @@ struct nfs4_setclientid {
 struct nfs4_setclientid_res {
 	u64				clientid;
 	nfs4_verifier			confirm;
+	u32				cb_ident;
 };
 
 struct nfs4_statfs_arg {
-- 
1.6.6

[PATCH_V6 07/11] NFS associate sessionid with callback connection

[andros]

From: Andy Adamson <andros@netapp.com>

The sessions based callback service is started prior to the CREATE_SESSION call
so that it can handle CB_NULL requests which can be sent before the
CREATE_SESSION call returns and the session ID is known.

Set the callback sessionid after a sucessful CREATE_SESSION.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/callback.c               |   31 +++++++++++++++++++++++++++++++
 fs/nfs/callback.h               |    1 +
 fs/nfs/nfs4state.c              |    6 ++++++
 include/linux/sunrpc/svc_xprt.h |    1 +
 net/sunrpc/svcsock.c            |    1 +
 5 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 343d543..8353c4a 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -137,6 +137,33 @@ out_err:
 
 #if defined(CONFIG_NFS_V4_1)
 /*
+ *  * CB_SEQUENCE operations will fail until the callback sessionid is set.
+ *   */
+int nfs4_set_callback_sessionid(struct nfs_client *clp)
+{
+	struct svc_serv *serv = clp->cl_rpcclient->cl_xprt->bc_serv;
+	struct nfs4_sessionid *bc_sid;
+
+	if (!serv->bc_xprt)
+		return -EINVAL;
+
+	/* on success freed in xprt_free */
+	bc_sid = kmalloc(sizeof(struct nfs4_sessionid), GFP_KERNEL);
+	if (!bc_sid)
+		return -ENOMEM;
+	memcpy(bc_sid->data, &clp->cl_session->sess_id.data,
+		NFS4_MAX_SESSIONID_LEN);
+	spin_lock_bh(&serv->sv_cb_lock);
+	serv->bc_xprt->xpt_bc_sid = bc_sid;
+	spin_unlock_bh(&serv->sv_cb_lock);
+	dprintk("%s set xpt_bc_sid=%u:%u:%u:%u for bc_xprt %p\n", __func__,
+		((u32 *)bc_sid->data)[0], ((u32 *)bc_sid->data)[1],
+		((u32 *)bc_sid->data)[2], ((u32 *)bc_sid->data)[3],
+		serv->bc_xprt);
+	return 0;
+}
+
+/*
  * The callback service for NFSv4.1 callbacks
  */
 static int
@@ -244,6 +271,10 @@ static inline void nfs_callback_bc_serv(u32 minorversion, struct rpc_xprt *xprt,
 		struct nfs_callback_data *cb_info)
 {
 }
+int nfs4_set_callback_sessionid(struct nfs_client *clp)
+{
+	return 0;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 /*
diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
index 85a7cfd..58d61a8 100644
--- a/fs/nfs/callback.h
+++ b/fs/nfs/callback.h
@@ -137,6 +137,7 @@ extern int nfs_callback_up(u32 minorversion, struct rpc_xprt *xprt);
 extern void nfs_callback_down(int minorversion);
 extern int nfs4_validate_delegation_stateid(struct nfs_delegation *delegation,
 					    const nfs4_stateid *stateid);
+extern int nfs4_set_callback_sessionid(struct nfs_client *clp);
 #endif /* CONFIG_NFS_V4 */
 /*
  * nfs41: Callbacks are expected to not cause substantial latency,
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index fe61422..11290de 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -193,6 +193,12 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
 	status = nfs4_proc_create_session(clp);
 	if (status != 0)
 		goto out;
+	status = nfs4_set_callback_sessionid(clp);
+	if (status != 0) {
+		printk(KERN_WARNING "Sessionid not set. No callback service\n");
+		nfs_callback_down(1);
+		status = 0;
+	}
 	nfs41_setup_state_renewal(clp);
 	nfs_mark_client_ready(clp, NFS_CS_READY);
 out:
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index aea0d43..357da5e 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -78,6 +78,7 @@ struct svc_xprt {
 	size_t			xpt_remotelen;	/* length of address */
 	struct rpc_wait_queue	xpt_bc_pending;	/* backchannel wait queue */
 	struct list_head	xpt_users;	/* callbacks on free */
+	void			*xpt_bc_sid;	/* back channel session ID */
 
 	struct net		*xpt_net;
 };
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 7c29be6..87e6f28 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1622,6 +1622,7 @@ static struct svc_xprt *svc_bc_create_socket(struct svc_serv *serv,
 static void svc_bc_sock_free(struct svc_xprt *xprt)
 {
 	if (xprt)
+		kfree(xprt->xpt_bc_sid);
 		kfree(container_of(xprt, struct svc_sock, sk_xprt));
 }
 #endif /* CONFIG_NFS_V4_1 */
-- 
1.6.6

Re: [PATCH_V6 01/11] SUNRPC move svc_drop to caller of svc_process_common

[J. Bruce Fields]

On Tue, Jan 04, 2011 at 12:10:18PM -0500, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> The NFSv4.1 shared back channel does not need to call svc_drop because the
> callback service never outlives the single connection it services, and it
> reuses it's buffers and keeps the trasport.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>

Looks good to me.

Acked-by: J. Bruce Fields <bfields@redhat.com>

--b.

> ---
>  net/sunrpc/svc.c |   30 ++++++++++++++++--------------
>  1 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 6359c42..606d182 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -1147,7 +1147,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
>   dropit:
>  	svc_authorise(rqstp);	/* doesn't hurt to call this twice */
>  	dprintk("svc: svc_process dropit\n");
> -	svc_drop(rqstp);
>  	return 0;
>  
>  err_short_len:
> @@ -1218,7 +1217,6 @@ svc_process(struct svc_rqst *rqstp)
>  	struct kvec		*resv = &rqstp->rq_res.head[0];
>  	struct svc_serv		*serv = rqstp->rq_server;
>  	u32			dir;
> -	int			error;
>  
>  	/*
>  	 * Setup response xdr_buf.
> @@ -1246,11 +1244,13 @@ svc_process(struct svc_rqst *rqstp)
>  		return 0;
>  	}
>  
> -	error = svc_process_common(rqstp, argv, resv);
> -	if (error <= 0)
> -		return error;
> -
> -	return svc_send(rqstp);
> +	/* Returns 1 for send, 0 for drop */
> +	if (svc_process_common(rqstp, argv, resv))
> +		return svc_send(rqstp);
> +	else {
> +		svc_drop(rqstp);
> +		return 0;
> +	}
>  }
>  
>  #if defined(CONFIG_NFS_V4_1)
> @@ -1264,7 +1264,6 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
>  {
>  	struct kvec	*argv = &rqstp->rq_arg.head[0];
>  	struct kvec	*resv = &rqstp->rq_res.head[0];
> -	int 		error;
>  
>  	/* Build the svc_rqst used by the common processing routine */
>  	rqstp->rq_xprt = serv->bc_xprt;
> @@ -1292,12 +1291,15 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
>  	svc_getu32(argv);	/* XID */
>  	svc_getnl(argv);	/* CALLDIR */
>  
> -	error = svc_process_common(rqstp, argv, resv);
> -	if (error <= 0)
> -		return error;
> -
> -	memcpy(&req->rq_snd_buf, &rqstp->rq_res, sizeof(req->rq_snd_buf));
> -	return bc_send(req);
> +	/* Returns 1 for send, 0 for drop */
> +	if (svc_process_common(rqstp, argv, resv)) {
> +		memcpy(&req->rq_snd_buf, &rqstp->rq_res,
> +						sizeof(req->rq_snd_buf));
> +		return bc_send(req);
> +	} else {
> +		/* Nothing to do to drop request */
> +		return 0;
> +	}
>  }
>  EXPORT_SYMBOL(bc_svc_process);
>  #endif /* CONFIG_NFS_V4_1 */
> -- 
> 1.6.6
> 
> --
> 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

Re: [PATCH_V6 03/11] SUNRPC new transport for the NFSv4.1 shared back channel

[J. Bruce Fields]

On Tue, Jan 04, 2011 at 12:10:20PM -0500, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Move the current sock create and destroy routines into the new transport ops.
> Back channel socket will be destroyed by the svc_closs_all call in svc_destroy.
> 
> Added check: only TCP supported on shared back channel.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  include/linux/sunrpc/svcsock.h |    1 +
>  net/sunrpc/svc.c               |    4 --
>  net/sunrpc/svcsock.c           |  118 +++++++++++++++++++++++++++++++++-------
>  3 files changed, 99 insertions(+), 24 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
> index 1b353a7..3a45a80 100644
> --- a/include/linux/sunrpc/svcsock.h
> +++ b/include/linux/sunrpc/svcsock.h
> @@ -45,6 +45,7 @@ int		svc_sock_names(struct svc_serv *serv, char *buf,
>  int		svc_addsock(struct svc_serv *serv, const int fd,
>  					char *name_return, const size_t len);
>  void		svc_init_xprt_sock(void);
> +void		svc_init_bc_xprt_sock(void);
>  void		svc_cleanup_xprt_sock(void);
>  struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot);
>  void		svc_sock_destroy(struct svc_xprt *);
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 606d182..261e2d1 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -488,10 +488,6 @@ svc_destroy(struct svc_serv *serv)
>  	if (svc_serv_is_pooled(serv))
>  		svc_pool_map_put();
>  
> -#if defined(CONFIG_NFS_V4_1)
> -	svc_sock_destroy(serv->bc_xprt);
> -#endif /* CONFIG_NFS_V4_1 */
> -
>  	svc_unregister(serv);
>  	kfree(serv->sv_pools);
>  	kfree(serv);
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 07919e1..7c29be6 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -66,6 +66,13 @@ static void		svc_sock_free(struct svc_xprt *);
>  static struct svc_xprt *svc_create_socket(struct svc_serv *, int,
>  					  struct net *, struct sockaddr *,
>  					  int, int);
> +#if defined(CONFIG_NFS_V4_1)
> +static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
> +					     struct net *, struct sockaddr *,
> +					     int, int);
> +static void svc_bc_sock_free(struct svc_xprt *xprt);
> +#endif /* CONFIG_NFS_V4_1 */
> +
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  static struct lock_class_key svc_key[2];
>  static struct lock_class_key svc_slock_key[2];
> @@ -1184,6 +1191,75 @@ static struct svc_xprt *svc_tcp_create(struct svc_serv *serv,
>  	return svc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
>  }
>  
> +#if defined(CONFIG_NFS_V4_1)
> +static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
> +					     struct net *, struct sockaddr *,
> +					     int, int);
> +static void svc_bc_sock_free(struct svc_xprt *xprt);
> +
> +static struct svc_xprt *svc_bc_tcp_create(struct svc_serv *serv,
> +				       struct net *net,
> +				       struct sockaddr *sa, int salen,
> +				       int flags)
> +{
> +	return svc_bc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
> +}
> +
> +static void svc_bc_tcp_sock_detach(struct svc_xprt *xprt)
> +{
> +}
> +
> +/* These bc ops are never called. The shared fore channel is used instead */

If these are never called, then you should be able to just leave these
fields of the xprt_ops NULL and save a few lines of code.

> +static int svc_bc_tcp_recvfrom(struct svc_rqst *rqstp)
> +{
> +	return 0;
> +}
> +
> +static int svc_bc_tcp_sendto(struct svc_rqst *rqstp)
> +{
> +	return 0;
> +}
> +
> +static void svc_bc_release_skb(struct svc_rqst *rqstp)
> +{
> +}
> +
> +static int svc_bc_tcp_has_wspace(struct svc_xprt *xprt)
> +{
> +	return 0;
> +}
> +
> +static struct svc_xprt *svc_bc_tcp_accept(struct svc_xprt *xprt)
> +{
> +	return NULL;
> +}
> +
> +static struct svc_xprt_ops svc_tcp_bc_ops = {
> +	.xpo_create = svc_bc_tcp_create,
> +	.xpo_recvfrom = svc_bc_tcp_recvfrom,
> +	.xpo_sendto = svc_bc_tcp_sendto,
> +	.xpo_release_rqst = svc_bc_release_skb,
> +	.xpo_detach = svc_bc_tcp_sock_detach,
> +	.xpo_free = svc_bc_sock_free,
> +	.xpo_prep_reply_hdr = svc_tcp_prep_reply_hdr,
> +	.xpo_has_wspace = svc_bc_tcp_has_wspace,
> +	.xpo_accept = svc_bc_tcp_accept,
> +};
> +
> +static struct svc_xprt_class svc_tcp_bc_class = {
> +	.xcl_name = "tcp-bc",
> +	.xcl_owner = THIS_MODULE,
> +	.xcl_ops = &svc_tcp_bc_ops,
> +	.xcl_max_payload = RPCSVC_MAXPAYLOAD_TCP,
> +};
> +
> +void svc_init_bc_xprt_sock(void)
> +{
> +	svc_reg_xprt_class(&svc_tcp_bc_class);
> +}
> +EXPORT_SYMBOL_GPL(svc_init_bc_xprt_sock);

We shouldn't introduce this until the patch where it's used.

--b.
> +#endif /* CONFIG_NFS_V4_1 */
> +
>  static struct svc_xprt_ops svc_tcp_ops = {
>  	.xpo_create = svc_tcp_create,
>  	.xpo_recvfrom = svc_tcp_recvfrom,
> @@ -1509,41 +1585,43 @@ static void svc_sock_free(struct svc_xprt *xprt)
>  	kfree(svsk);
>  }
>  
> +#if defined(CONFIG_NFS_V4_1)
>  /*
> - * Create a svc_xprt.
> - *
> - * For internal use only (e.g. nfsv4.1 backchannel).
> - * Callers should typically use the xpo_create() method.
> + * Create a back channel svc_xprt which shares the fore channel socket.
>   */
> -struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot)
> +static struct svc_xprt *svc_bc_create_socket(struct svc_serv *serv,
> +					     int protocol,
> +					     struct net *net,
> +					     struct sockaddr *sin, int len,
> +					     int flags)
>  {
>  	struct svc_sock *svsk;
> -	struct svc_xprt *xprt = NULL;
> +	struct svc_xprt *xprt;
> +
> +	if (protocol != IPPROTO_TCP) {
> +		printk(KERN_WARNING "svc: only TCP sockets"
> +			" supported on shared back channel\n");
> +		return ERR_PTR(-EINVAL);
> +	}
>  
> -	dprintk("svc: %s\n", __func__);
>  	svsk = kzalloc(sizeof(*svsk), GFP_KERNEL);
>  	if (!svsk)
> -		goto out;
> +		return ERR_PTR(-ENOMEM);
>  
>  	xprt = &svsk->sk_xprt;
> -	if (prot == IPPROTO_TCP)
> -		svc_xprt_init(&svc_tcp_class, xprt, serv);
> -	else if (prot == IPPROTO_UDP)
> -		svc_xprt_init(&svc_udp_class, xprt, serv);
> -	else
> -		BUG();
> -out:
> -	dprintk("svc: %s return %p\n", __func__, xprt);
> +	svc_xprt_init(&svc_tcp_bc_class, xprt, serv);
> +
> +	serv->bc_xprt = xprt;
> +
>  	return xprt;
>  }
> -EXPORT_SYMBOL_GPL(svc_sock_create);
>  
>  /*
> - * Destroy a svc_sock.
> + * Free a back channel svc_sock.
>   */
> -void svc_sock_destroy(struct svc_xprt *xprt)
> +static void svc_bc_sock_free(struct svc_xprt *xprt)
>  {
>  	if (xprt)
>  		kfree(container_of(xprt, struct svc_sock, sk_xprt));
>  }
> -EXPORT_SYMBOL_GPL(svc_sock_destroy);
> +#endif /* CONFIG_NFS_V4_1 */
> -- 
> 1.6.6
> 
> --
> 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

Re: [PATCH_V6 04/11] NFS use svc_create_xprt for NFSv4.1 callback service

[J. Bruce Fields]

On Tue, Jan 04, 2011 at 12:10:21PM -0500, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> The new back channel transport means we call the normal creation routine as
> well as svc_xprt_put.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/callback.c |   31 +++++++++++++++++++++----------
>  1 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 93a8b3b..343d543 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -177,30 +177,41 @@ nfs41_callback_svc(void *vrqstp)
>  struct svc_rqst *
>  nfs41_callback_up(struct svc_serv *serv, struct rpc_xprt *xprt)
>  {
> -	struct svc_xprt *bc_xprt;
> -	struct svc_rqst *rqstp = ERR_PTR(-ENOMEM);
> +	struct svc_rqst *rqstp;
> +	int ret;
>  
> -	dprintk("--> %s\n", __func__);
> -	/* Create a svc_sock for the service */
> -	bc_xprt = svc_sock_create(serv, xprt->prot);
> -	if (!bc_xprt)
> +	/* register the back channel transport class */
> +	svc_init_bc_xprt_sock();

We really only need to register the transport class once, not each time
we set up a new backchannel.

(OK, it's harmless to register it multiple times--the registration
detects that case and does nothing--but still I'd prefer to register
just once if possible.)

Also: I think you're forgetting to unregister the transport class?

I think the correct place to put the registration and unregistration may
be init_nfs_fs() and exit_nfs_fs().

(And define it as a no-op function depending on config options to avoid
ifdef's in init_nfs_fs/exit_nfs_fs, in the usual way.)

--b.

> +
> +	/*
> +	 * 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
> +	 */
> +	ret = svc_create_xprt(serv, "tcp-bc", &init_net, PF_INET, 0,
> +			      SVC_SOCK_ANONYMOUS);
> +	if (ret < 0) {
> +		rqstp = ERR_PTR(ret);
>  		goto out;
> +	}
>  
>  	/*
>  	 * Save the svc_serv in the transport so that it can
>  	 * be referenced when the session backchannel is initialized
>  	 */
> -	serv->bc_xprt = bc_xprt;
>  	xprt->bc_serv = serv;
>  
>  	INIT_LIST_HEAD(&serv->sv_cb_list);
>  	spin_lock_init(&serv->sv_cb_lock);
>  	init_waitqueue_head(&serv->sv_cb_waitq);
>  	rqstp = svc_prepare_thread(serv, &serv->sv_pools[0]);
> -	if (IS_ERR(rqstp))
> -		svc_sock_destroy(bc_xprt);
> +	if (IS_ERR(rqstp)) {
> +		svc_xprt_put(serv->bc_xprt);
> +		serv->bc_xprt = NULL;
> +	}
>  out:
> -	dprintk("--> %s return %p\n", __func__, rqstp);
> +	dprintk("--> %s return %ld\n", __func__,
> +		IS_ERR(rqstp) ? PTR_ERR(rqstp) : 0);
>  	return rqstp;
>  }
>  
> -- 
> 1.6.6
> 
> --
> 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