Re: [PATCH_V3 3/7] NFS associate sessionid with callback connection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Dec 14, 2010, at 1:26 PM, J. Bruce Fields wrote:

On Mon, Dec 13, 2010 at 03:19:41PM -0500, Andy Adamson wrote:
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@xxxxxxxxxx>
---
fs/nfs/callback.c | 31 +++++++++++++++++++++++++++++ ++
fs/nfs/callback.h               |    1 +
fs/nfs/nfs4state.c              |    6 ++++++
include/linux/sunrpc/svc_xprt.h |    1 +
net/sunrpc/xprt.c               |    2 ++
5 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 023a9eb..558d858 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(&serv->sv_cb_lock);
+	serv->bc_xprt->xpt_bc_sid = bc_sid;
+	spin_unlock(&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
@@ -236,6 +263,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 600c669..aa15126 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -79,6 +79,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/xprt.c b/net/sunrpc/xprt.c
index 4c8f18a..18783cb 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -985,6 +985,8 @@ void xprt_free(struct rpc_xprt *xprt)
{
	put_net(xprt->xprt_net);
	kfree(xprt->slot);
+	if (xprt->bc_xprt)
+		kfree(xprt->bc_xprt->xpt_bc_sid);

If bc_xprt points to an svc_xprt, shouldn't you be waiting till you free
that svc_xprt before freeing anything it points to?

The idea of leaving around that bc_xprt with its xpt_bc_sid field now
pointing at freed memory makes me nervous....

You are correct - the xpt_bc_sid should be freed in svc_sock_destroy.
I'll resend.

Thanks for the review!

-->Andy


--b.

	kfree(xprt);
}
EXPORT_SYMBOL_GPL(xprt_free);
--
1.6.6

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

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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux