2012-05-24 21:19:36

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation

This patch adds the BIND_CONN_TO_SESSION operation which is needed for
upcoming SP4_MACH_CRED work and useful for recovering from broken connections
without destroying the session.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
Updated with comment from Trond

fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++
fs/nfs/nfs4xdr.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/nfs4.h | 5 ++
include/linux/nfs_xdr.h | 6 +++
5 files changed, 161 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index edeef71..ad90bc7 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -215,6 +215,7 @@ struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *);
extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *);
extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
+extern int nfs4_proc_bind_conn_to_session(struct nfs_client *);
extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 78784e5..1d1bef6 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5062,6 +5062,60 @@ nfs41_same_server_scope(struct server_scope *a, struct server_scope *b)
}

/*
+ * nfs4_proc_bind_conn_to_session()
+ *
+ * The 4.1 client currently uses the same TCP connection for the
+ * fore and backchannel.
+ */
+int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
+{
+ int status;
+ struct nfs41_bind_conn_to_session_res res;
+ struct rpc_message msg = {
+ .rpc_proc =
+ &nfs4_procedures[NFSPROC4_CLNT_BIND_CONN_TO_SESSION],
+ .rpc_argp = clp,
+ .rpc_resp = &res,
+ };
+
+ dprintk("--> %s\n", __func__);
+ BUG_ON(clp == NULL);
+
+ res.session = kzalloc(sizeof(struct nfs4_session), GFP_NOFS);
+ if (unlikely(res.session == NULL)) {
+ status = -ENOMEM;
+ goto out;
+ }
+
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+ if (status == 0) {
+ if (memcmp(res.session->sess_id.data,
+ clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
+ dprintk("NFS: %s: Session ID mismatch\n", __func__);
+ status = -EIO;
+ goto out_session;
+ }
+ if (res.dir != NFS4_CDFS4_BOTH) {
+ dprintk("NFS: %s: Unexpected direction from server\n",
+ __func__);
+ status = -EIO;
+ goto out_session;
+ }
+ if (res.use_conn_in_rdma_mode) {
+ dprintk("NFS: %s: Server returned RDMA mode = true\n",
+ __func__);
+ status = -EIO;
+ goto out_session;
+ }
+ }
+out_session:
+ kfree(res.session);
+out:
+ dprintk("<-- %s status= %d\n", __func__, status);
+ return status;
+}
+
+/*
* nfs4_proc_exchange_id()
*
* Since the clientid has expired, all compounds using sessions
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index db040e9..e9be74f 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -321,6 +321,16 @@ static int nfs4_stat_to_errno(int);
1 /* csr_flags */ + \
decode_channel_attrs_maxsz + \
decode_channel_attrs_maxsz)
+#define encode_bind_conn_to_session_maxsz (op_encode_hdr_maxsz + \
+ /* bctsa_sessid */ \
+ XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+ 1 /* bctsa_dir */ + \
+ 1 /* bctsa_use_conn_in_rdma_mode */)
+#define decode_bind_conn_to_session_maxsz (op_decode_hdr_maxsz + \
+ /* bctsr_sessid */ \
+ XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+ 1 /* bctsr_dir */ + \
+ 1 /* bctsr_use_conn_in_rdma_mode */)
#define encode_destroy_session_maxsz (op_encode_hdr_maxsz + 4)
#define decode_destroy_session_maxsz (op_decode_hdr_maxsz)
#define encode_sequence_maxsz (op_encode_hdr_maxsz + \
@@ -714,6 +724,12 @@ static int nfs4_stat_to_errno(int);
decode_putfh_maxsz + \
decode_secinfo_maxsz)
#if defined(CONFIG_NFS_V4_1)
+#define NFS4_enc_bind_conn_to_session_sz \
+ (compound_encode_hdr_maxsz + \
+ encode_bind_conn_to_session_maxsz)
+#define NFS4_dec_bind_conn_to_session_sz \
+ (compound_decode_hdr_maxsz + \
+ decode_bind_conn_to_session_maxsz)
#define NFS4_enc_exchange_id_sz \
(compound_encode_hdr_maxsz + \
encode_exchange_id_maxsz)
@@ -1654,6 +1670,20 @@ static void encode_secinfo(struct xdr_stream *xdr, const struct qstr *name, stru

#if defined(CONFIG_NFS_V4_1)
/* NFSv4.1 operations */
+static void encode_bind_conn_to_session(struct xdr_stream *xdr,
+ struct nfs4_session *session,
+ struct compound_hdr *hdr)
+{
+ __be32 *p;
+
+ encode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION,
+ decode_bind_conn_to_session_maxsz, hdr);
+ encode_opaque_fixed(xdr, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
+ p = xdr_reserve_space(xdr, 8);
+ *p++ = cpu_to_be32(NFS4_CDFC4_BACK_OR_BOTH);
+ *p = 0; /* use_conn_in_rdma_mode = False */
+}
+
static void encode_exchange_id(struct xdr_stream *xdr,
struct nfs41_exchange_id_args *args,
struct compound_hdr *hdr)
@@ -2614,6 +2644,22 @@ static void nfs4_xdr_enc_secinfo(struct rpc_rqst *req,

#if defined(CONFIG_NFS_V4_1)
/*
+ * BIND_CONN_TO_SESSION request
+ */
+static void nfs4_xdr_enc_bind_conn_to_session(struct rpc_rqst *req,
+ struct xdr_stream *xdr,
+ struct nfs_client *clp)
+{
+ struct compound_hdr hdr = {
+ .minorversion = clp->cl_mvops->minor_version,
+ };
+
+ encode_compound_hdr(xdr, req, &hdr);
+ encode_bind_conn_to_session(xdr, clp->cl_session, &hdr);
+ encode_nops(&hdr);
+}
+
+/*
* EXCHANGE_ID request
*/
static void nfs4_xdr_enc_exchange_id(struct rpc_rqst *req,
@@ -5239,6 +5285,37 @@ static int decode_sessionid(struct xdr_stream *xdr, struct nfs4_sessionid *sid)
return decode_opaque_fixed(xdr, sid->data, NFS4_MAX_SESSIONID_LEN);
}

+static int decode_bind_conn_to_session(struct xdr_stream *xdr,
+ struct nfs41_bind_conn_to_session_res *res)
+{
+ __be32 *p;
+ int status;
+
+ status = decode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION);
+ if (!status)
+ status = decode_sessionid(xdr, &res->session->sess_id);
+ if (unlikely(status))
+ return status;
+
+ /* dir flags, rdma mode bool */
+ p = xdr_inline_decode(xdr, 8);
+ if (unlikely(!p))
+ goto out_overflow;
+
+ res->dir = be32_to_cpup(p++);
+ if (res->dir == 0 || res->dir > NFS4_CDFS4_BOTH)
+ return -EIO;
+ if (be32_to_cpup(p) == 0)
+ res->use_conn_in_rdma_mode = false;
+ else
+ res->use_conn_in_rdma_mode = true;
+
+ return 0;
+out_overflow:
+ print_overflow_msg(__func__, xdr);
+ return -EIO;
+}
+
static int decode_create_session(struct xdr_stream *xdr,
struct nfs41_create_session_res *res)
{
@@ -6521,6 +6598,22 @@ out:

#if defined(CONFIG_NFS_V4_1)
/*
+ * Decode BIND_CONN_TO_SESSION response
+ */
+static int nfs4_xdr_dec_bind_conn_to_session(struct rpc_rqst *rqstp,
+ struct xdr_stream *xdr,
+ void *res)
+{
+ struct compound_hdr hdr;
+ int status;
+
+ status = decode_compound_hdr(xdr, &hdr);
+ if (!status)
+ status = decode_bind_conn_to_session(xdr, res);
+ return status;
+}
+
+/*
* Decode EXCHANGE_ID response
*/
static int nfs4_xdr_dec_exchange_id(struct rpc_rqst *rqstp,
@@ -7001,6 +7094,8 @@ struct rpc_procinfo nfs4_procedures[] = {
PROC(RELEASE_LOCKOWNER, enc_release_lockowner, dec_release_lockowner),
PROC(SECINFO, enc_secinfo, dec_secinfo),
#if defined(CONFIG_NFS_V4_1)
+ PROC(BIND_CONN_TO_SESSION,
+ enc_bind_conn_to_session, dec_bind_conn_to_session),
PROC(EXCHANGE_ID, enc_exchange_id, dec_exchange_id),
PROC(CREATE_SESSION, enc_create_session, dec_create_session),
PROC(DESTROY_SESSION, enc_destroy_session, dec_destroy_session),
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 0987146..5b1a504 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -69,6 +69,10 @@
#define NFS4_CDFC4_FORE_OR_BOTH 0x3
#define NFS4_CDFC4_BACK_OR_BOTH 0x7

+#define NFS4_CDFS4_FORE 0x1
+#define NFS4_CDFS4_BACK 0x2
+#define NFS4_CDFS4_BOTH 0x3
+
#define NFS4_SET_TO_SERVER_TIME 0
#define NFS4_SET_TO_CLIENT_TIME 1

@@ -582,6 +586,7 @@ enum {
NFSPROC4_CLNT_SECINFO,

/* nfs41 */
+ NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
NFSPROC4_CLNT_EXCHANGE_ID,
NFSPROC4_CLNT_CREATE_SESSION,
NFSPROC4_CLNT_DESTROY_SESSION,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 2e53a3f..aa2a3bb 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1115,6 +1115,12 @@ struct nfs41_impl_id {
struct nfstime4 date;
};

+struct nfs41_bind_conn_to_session_res {
+ struct nfs4_session *session;
+ u32 dir;
+ bool use_conn_in_rdma_mode;
+};
+
struct nfs41_exchange_id_res {
struct nfs_client *client;
u32 flags;
--
1.7.4.4



2012-05-24 16:49:55

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 12:46 PM, Chuck Lever wrote:

>
> On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:
>
>> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
>> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
>> without destroying the session.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> fs/nfs/nfs4_fs.h | 1 +
>> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
>> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/nfs4.h | 5 +++
>> include/linux/nfs_xdr.h | 6 +++
>> 5 files changed, 156 insertions(+), 0 deletions(-)
>
> [ ... snipped ... ]
>
>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>> index 0987146..80fbbfc6 100644
>> --- a/include/linux/nfs4.h
>> +++ b/include/linux/nfs4.h
>> @@ -69,6 +69,10 @@
>> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
>> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>>
>> +#define CDFS4_FORE 0x1
>> +#define CDFS4_BACK 0x2
>> +#define CDFS4_BOTH 0x3
>
> A few more nits:
>
> I'm looking back at the patch I did last year to add BIND_CONN_TO_SESS for NFSv4.1 migration support... I used NFS4_CDFS4_FORE and so on here, to match the defines in the paragraph above.

Ah, good catch! I'll change to NFS4_CDFS4_*

>
> In the XDR decoder function, I check the "dir_from_server" field to ensure it is equal to or less than NFS4_CDFS4_BOTH (-EIO if not).

Ok, that can't hurt! Note I'm already returning EIO in the proc handler if dir != NFS4_CDFS4_BOTH, but I think it makes sense to add this sanity check.

Thanks!
-dros

>
>> +
>> #define NFS4_SET_TO_SERVER_TIME 0
>> #define NFS4_SET_TO_CLIENT_TIME 1
>>
>> @@ -582,6 +586,7 @@ enum {
>> NFSPROC4_CLNT_SECINFO,
>>
>> /* nfs41 */
>> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
>> NFSPROC4_CLNT_EXCHANGE_ID,
>> NFSPROC4_CLNT_CREATE_SESSION,
>> NFSPROC4_CLNT_DESTROY_SESSION,
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>


Attachments:
smime.p7s (1.34 kB)

2012-05-24 17:08:08

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 1:06 PM, Chuck Lever wrote:

>
> On May 24, 2012, at 12:49 PM, Adamson, Dros wrote:
>
>>
>> On May 24, 2012, at 12:46 PM, Chuck Lever wrote:
>>
>>>
>>> On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:
>>>
>>>> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
>>>> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
>>>> without destroying the session.
>>>>
>>>> Signed-off-by: Weston Andros Adamson <[email protected]>
>>>> ---
>>>> fs/nfs/nfs4_fs.h | 1 +
>>>> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
>>>> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/nfs4.h | 5 +++
>>>> include/linux/nfs_xdr.h | 6 +++
>>>> 5 files changed, 156 insertions(+), 0 deletions(-)
>>>
>>> [ ... snipped ... ]
>>>
>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>>>> index 0987146..80fbbfc6 100644
>>>> --- a/include/linux/nfs4.h
>>>> +++ b/include/linux/nfs4.h
>>>> @@ -69,6 +69,10 @@
>>>> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
>>>> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>>>>
>>>> +#define CDFS4_FORE 0x1
>>>> +#define CDFS4_BACK 0x2
>>>> +#define CDFS4_BOTH 0x3
>>>
>>> A few more nits:
>>>
>>> I'm looking back at the patch I did last year to add BIND_CONN_TO_SESS for NFSv4.1 migration support... I used NFS4_CDFS4_FORE and so on here, to match the defines in the paragraph above.
>>
>> Ah, good catch! I'll change to NFS4_CDFS4_*
>>
>>>
>>> In the XDR decoder function, I check the "dir_from_server" field to ensure it is equal to or less than NFS4_CDFS4_BOTH (-EIO if not).
>>
>> Ok, that can't hurt! Note I'm already returning EIO in the proc handler if dir != NFS4_CDFS4_BOTH,
>
> Right, that's an implementation-imposed limit. proc is the right place for that. The XDR check is to sanity check servers, and should allow all possible values of that field, and no more. FWIW.

I'm right there with you :)

-dros

>
>> but I think it makes sense to add this sanity check.
>>
>> Thanks!
>> -dros
>>
>>>
>>>> +
>>>> #define NFS4_SET_TO_SERVER_TIME 0
>>>> #define NFS4_SET_TO_CLIENT_TIME 1
>>>>
>>>> @@ -582,6 +586,7 @@ enum {
>>>> NFSPROC4_CLNT_SECINFO,
>>>>
>>>> /* nfs41 */
>>>> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
>>>> NFSPROC4_CLNT_EXCHANGE_ID,
>>>> NFSPROC4_CLNT_CREATE_SESSION,
>>>> NFSPROC4_CLNT_DESTROY_SESSION,
>>>
>>> --
>>> Chuck Lever
>>> chuck[dot]lever[at]oracle[dot]com
>>>
>>>
>>>
>>>
>>
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>


Attachments:
smime.p7s (1.34 kB)

2012-05-24 16:35:54

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 12:30 PM, Chuck Lever wrote:

>
> On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:
>
>> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
>> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
>> without destroying the session.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> fs/nfs/nfs4_fs.h | 1 +
>> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
>> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/nfs4.h | 5 +++
>> include/linux/nfs_xdr.h | 6 +++
>> 5 files changed, 156 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index edeef71..ad90bc7 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -215,6 +215,7 @@ struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
>> extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *);
>> extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *);
>> extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
>> +extern int nfs4_proc_bind_conn_to_session(struct nfs_client *);
>> extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
>> extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
>> extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 78784e5..7a4b320 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5062,6 +5062,60 @@ nfs41_same_server_scope(struct server_scope *a, struct server_scope *b)
>> }
>>
>> /*
>> + * nfs4_proc_bind_conn_to_session()
>> + *
>> + * The 4.1 client currently uses the same TCP connection for the
>> + * fore and backchannel.
>> + */
>> +int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
>> +{
>> + int status;
>> + struct nfs41_bind_conn_to_session_res res;
>> + struct rpc_message msg = {
>> + .rpc_proc =
>> + &nfs4_procedures[NFSPROC4_CLNT_BIND_CONN_TO_SESSION],
>> + .rpc_argp = clp,
>> + .rpc_resp = &res,
>> + };
>> +
>> + dprintk("--> %s\n", __func__);
>> + BUG_ON(clp == NULL);
>> +
>> + res.session = kzalloc(sizeof(struct nfs4_session), GFP_KERNEL);
>> + if (unlikely(res.session == NULL)) {
>> + status = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> + if (status == 0) {
>> + if (memcmp(res.session->sess_id.data,
>> + clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
>> + dprintk("NFS: %s: Session ID mismatch\n", __func__);
>> + status = -EIO;
>> + goto out_session;
>> + }
>> + if (res.dir != CDFS4_BOTH) {
>> + dprintk("NFS: %s: Unexpected direction from server\n",
>> + __func__);
>> + status = -EIO;
>> + goto out_session;
>> + }
>> + if (res.use_conn_in_rdma_mode != 0) {
>> + dprintk("NFS: %s: Server returned RDMA mode = true\n",
>> + __func__);
>> + status = -EIO;
>> + goto out_session;
>> + }
>> + }
>> +out_session:
>> + kfree(res.session);
>> +out:
>> + dprintk("<-- %s status= %d\n", __func__, status);
>> + return status;
>> +}
>> +
>> +/*
>> * nfs4_proc_exchange_id()
>> *
>> * Since the clientid has expired, all compounds using sessions
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index db040e9..37e09c8 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -321,6 +321,16 @@ static int nfs4_stat_to_errno(int);
>> 1 /* csr_flags */ + \
>> decode_channel_attrs_maxsz + \
>> decode_channel_attrs_maxsz)
>> +#define encode_bind_conn_to_session_maxsz (op_encode_hdr_maxsz + \
>> + /* bctsa_sessid */ \
>> + XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
>> + 1 /* bctsa_dir */ + \
>> + 1 /* bctsa_use_conn_in_rdma_mode */)
>> +#define decode_bind_conn_to_session_maxsz (op_decode_hdr_maxsz + \
>> + /* bctsr_sessid */ \
>> + XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
>> + 1 /* bctsr_dir */ + \
>> + 1 /* bctsr_use_conn_in_rdma_mode */)
>> #define encode_destroy_session_maxsz (op_encode_hdr_maxsz + 4)
>> #define decode_destroy_session_maxsz (op_decode_hdr_maxsz)
>> #define encode_sequence_maxsz (op_encode_hdr_maxsz + \
>> @@ -714,6 +724,12 @@ static int nfs4_stat_to_errno(int);
>> decode_putfh_maxsz + \
>> decode_secinfo_maxsz)
>> #if defined(CONFIG_NFS_V4_1)
>> +#define NFS4_enc_bind_conn_to_session_sz \
>> + (compound_encode_hdr_maxsz + \
>> + encode_bind_conn_to_session_maxsz)
>> +#define NFS4_dec_bind_conn_to_session_sz \
>> + (compound_decode_hdr_maxsz + \
>> + decode_bind_conn_to_session_maxsz)
>> #define NFS4_enc_exchange_id_sz \
>> (compound_encode_hdr_maxsz + \
>> encode_exchange_id_maxsz)
>> @@ -1654,6 +1670,20 @@ static void encode_secinfo(struct xdr_stream *xdr, const struct qstr *name, stru
>>
>> #if defined(CONFIG_NFS_V4_1)
>> /* NFSv4.1 operations */
>> +static void encode_bind_conn_to_session(struct xdr_stream *xdr,
>> + struct nfs4_session *session,
>> + struct compound_hdr *hdr)
>> +{
>> + __be32 *p;
>> +
>> + encode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION,
>> + decode_bind_conn_to_session_maxsz, hdr);
>> + encode_opaque_fixed(xdr, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
>> + p = xdr_reserve_space(xdr, 8);
>> + *p++ = cpu_to_be32(NFS4_CDFC4_BACK_OR_BOTH);
>> + *p = 0; /* use_conn_in_rdma_mode = False */
>> +}
>> +
>> static void encode_exchange_id(struct xdr_stream *xdr,
>> struct nfs41_exchange_id_args *args,
>> struct compound_hdr *hdr)
>> @@ -2614,6 +2644,22 @@ static void nfs4_xdr_enc_secinfo(struct rpc_rqst *req,
>>
>> #if defined(CONFIG_NFS_V4_1)
>> /*
>> + * BIND_CONN_TO_SESSION request
>> + */
>> +static void nfs4_xdr_enc_bind_conn_to_session(struct rpc_rqst *req,
>> + struct xdr_stream *xdr,
>> + struct nfs_client *clp)
>> +{
>> + struct compound_hdr hdr = {
>> + .minorversion = clp->cl_mvops->minor_version,
>> + };
>> +
>> + encode_compound_hdr(xdr, req, &hdr);
>> + encode_bind_conn_to_session(xdr, clp->cl_session, &hdr);
>> + encode_nops(&hdr);
>> +}
>> +
>> +/*
>> * EXCHANGE_ID request
>> */
>> static void nfs4_xdr_enc_exchange_id(struct rpc_rqst *req,
>> @@ -5239,6 +5285,32 @@ static int decode_sessionid(struct xdr_stream *xdr, struct nfs4_sessionid *sid)
>> return decode_opaque_fixed(xdr, sid->data, NFS4_MAX_SESSIONID_LEN);
>> }
>>
>> +static int decode_bind_conn_to_session(struct xdr_stream *xdr,
>> + struct nfs41_bind_conn_to_session_res *res)
>> +{
>> + __be32 *p;
>> + int status;
>> +
>> + status = decode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION);
>> + if (!status)
>> + status = decode_sessionid(xdr, &res->session->sess_id);
>> + if (unlikely(status))
>> + return status;
>> +
>> + /* dir flags, rdma mode bool */
>> + p = xdr_inline_decode(xdr, 8);
>> + if (unlikely(!p))
>> + goto out_overflow;
>> +
>> + res->dir = be32_to_cpup(p++);
>> + res->use_conn_in_rdma_mode = be32_to_cpup(p);
>> +
>> + return 0;
>> +out_overflow:
>> + print_overflow_msg(__func__, xdr);
>> + return -EIO;
>> +}
>> +
>> static int decode_create_session(struct xdr_stream *xdr,
>> struct nfs41_create_session_res *res)
>> {
>> @@ -6521,6 +6593,22 @@ out:
>>
>> #if defined(CONFIG_NFS_V4_1)
>> /*
>> + * Decode BIND_CONN_TO_SESSION response
>> + */
>> +static int nfs4_xdr_dec_bind_conn_to_session(struct rpc_rqst *rqstp,
>> + struct xdr_stream *xdr,
>> + void *res)
>> +{
>> + struct compound_hdr hdr;
>> + int status;
>> +
>> + status = decode_compound_hdr(xdr, &hdr);
>> + if (!status)
>> + status = decode_bind_conn_to_session(xdr, res);
>> + return status;
>> +}
>> +
>> +/*
>> * Decode EXCHANGE_ID response
>> */
>> static int nfs4_xdr_dec_exchange_id(struct rpc_rqst *rqstp,
>> @@ -7001,6 +7089,8 @@ struct rpc_procinfo nfs4_procedures[] = {
>> PROC(RELEASE_LOCKOWNER, enc_release_lockowner, dec_release_lockowner),
>> PROC(SECINFO, enc_secinfo, dec_secinfo),
>> #if defined(CONFIG_NFS_V4_1)
>> + PROC(BIND_CONN_TO_SESSION,
>> + enc_bind_conn_to_session, dec_bind_conn_to_session),
>> PROC(EXCHANGE_ID, enc_exchange_id, dec_exchange_id),
>> PROC(CREATE_SESSION, enc_create_session, dec_create_session),
>> PROC(DESTROY_SESSION, enc_destroy_session, dec_destroy_session),
>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>> index 0987146..80fbbfc6 100644
>> --- a/include/linux/nfs4.h
>> +++ b/include/linux/nfs4.h
>> @@ -69,6 +69,10 @@
>> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
>> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>>
>> +#define CDFS4_FORE 0x1
>> +#define CDFS4_BACK 0x2
>> +#define CDFS4_BOTH 0x3
>> +
>> #define NFS4_SET_TO_SERVER_TIME 0
>> #define NFS4_SET_TO_CLIENT_TIME 1
>>
>> @@ -582,6 +586,7 @@ enum {
>> NFSPROC4_CLNT_SECINFO,
>>
>> /* nfs41 */
>> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
>> NFSPROC4_CLNT_EXCHANGE_ID,
>> NFSPROC4_CLNT_CREATE_SESSION,
>> NFSPROC4_CLNT_DESTROY_SESSION,
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 2e53a3f..dd3bc2f 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -1115,6 +1115,12 @@ struct nfs41_impl_id {
>> struct nfstime4 date;
>> };
>>
>> +struct nfs41_bind_conn_to_session_res {
>> + struct nfs4_session *session;
>> + u32 dir;
>> + u32 use_conn_in_rdma_mode;
>
> Would it make sense to use a "bool" here instead?

I'm not against it. It's part of the result struct to ensure that it's always 0 returned from the server.

-dros

>
>> +};
>> +
>> struct nfs41_exchange_id_res {
>> struct nfs_client *client;
>> u32 flags;
>> --
>> 1.7.4.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>


Attachments:
smime.p7s (1.34 kB)

2012-05-24 20:36:12

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation

T24gVGh1LCAyMDEyLTA1LTI0IGF0IDEzOjIyIC0wNDAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IFRoaXMgcGF0Y2ggYWRkcyB0aGUgQklORF9DT05OX1RPX1NFU1NJT04gb3BlcmF0
aW9uIHdoaWNoIGlzIG5lZWRlZCBmb3INCj4gdXBjb21pbmcgU1A0X01BQ0hfQ1JFRCB3b3JrIGFu
ZCB1c2VmdWwgZm9yIHJlY292ZXJpbmcgZnJvbSBicm9rZW4gY29ubmVjdGlvbnMNCj4gd2l0aG91
dCBkZXN0cm95aW5nIHRoZSBzZXNzaW9uLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogV2VzdG9uIEFu
ZHJvcyBBZGFtc29uIDxkcm9zQG5ldGFwcC5jb20+DQo+IC0tLQ0KPiBSZXBvc3RlZCB3aXRoIGNo
YW5nZXMgYWRkcmVzc2luZyBDaHVjaydzIGNvbW1lbnRzLg0KPiANCg0KQ29tbWl0dGVkIGFmdGVy
IGNoYW5naW5nIHRoZSBhbGxvY2F0aW9uIG1vZGUgdG8gR0ZQX05PRlMuIEdGUF9LRVJORUwNCmFs
bG9jYXRpb25zIG1heSBlbmQgdXAgZGVhZGxvY2tpbmcgaW4gYSByZWNvdmVyeSBzaXR1YXRpb24g
aWYgdGhleQ0KcmVzdWx0IGluIGFuIGF0dGVtcHQgdG8gd3JpdGUgYmFjayBkaXJ0eSBORlMgcGFn
ZXMuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIN
Cg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

2012-05-24 17:06:46

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 12:49 PM, Adamson, Dros wrote:

>
> On May 24, 2012, at 12:46 PM, Chuck Lever wrote:
>
>>
>> On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:
>>
>>> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
>>> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
>>> without destroying the session.
>>>
>>> Signed-off-by: Weston Andros Adamson <[email protected]>
>>> ---
>>> fs/nfs/nfs4_fs.h | 1 +
>>> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
>>> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/nfs4.h | 5 +++
>>> include/linux/nfs_xdr.h | 6 +++
>>> 5 files changed, 156 insertions(+), 0 deletions(-)
>>
>> [ ... snipped ... ]
>>
>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>>> index 0987146..80fbbfc6 100644
>>> --- a/include/linux/nfs4.h
>>> +++ b/include/linux/nfs4.h
>>> @@ -69,6 +69,10 @@
>>> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
>>> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>>>
>>> +#define CDFS4_FORE 0x1
>>> +#define CDFS4_BACK 0x2
>>> +#define CDFS4_BOTH 0x3
>>
>> A few more nits:
>>
>> I'm looking back at the patch I did last year to add BIND_CONN_TO_SESS for NFSv4.1 migration support... I used NFS4_CDFS4_FORE and so on here, to match the defines in the paragraph above.
>
> Ah, good catch! I'll change to NFS4_CDFS4_*
>
>>
>> In the XDR decoder function, I check the "dir_from_server" field to ensure it is equal to or less than NFS4_CDFS4_BOTH (-EIO if not).
>
> Ok, that can't hurt! Note I'm already returning EIO in the proc handler if dir != NFS4_CDFS4_BOTH,

Right, that's an implementation-imposed limit. proc is the right place for that. The XDR check is to sanity check servers, and should allow all possible values of that field, and no more. FWIW.

> but I think it makes sense to add this sanity check.
>
> Thanks!
> -dros
>
>>
>>> +
>>> #define NFS4_SET_TO_SERVER_TIME 0
>>> #define NFS4_SET_TO_CLIENT_TIME 1
>>>
>>> @@ -582,6 +586,7 @@ enum {
>>> NFSPROC4_CLNT_SECINFO,
>>>
>>> /* nfs41 */
>>> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
>>> NFSPROC4_CLNT_EXCHANGE_ID,
>>> NFSPROC4_CLNT_CREATE_SESSION,
>>> NFSPROC4_CLNT_DESTROY_SESSION,
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>
>>
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2012-05-24 21:19:38

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH 2/2] nfs41: Use BIND_CONN_TO_SESSION for CB_PATH_DOWN*

The state manager can handle SEQ4_STATUS_CB_PATH_DOWN* flags with a
BIND_CONN_TO_SESSION instead of destroying the session and creating a new one.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
Updated with comments from Trond.

I got to remember to test with different CONFIG options *after* I clean up the
patch :)

fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4state.c | 31 +++++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index ad90bc7..9feff0f 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -24,6 +24,7 @@ enum nfs4_client_state {
NFS4CLNT_RECALL_SLOT,
NFS4CLNT_LEASE_CONFIRM,
NFS4CLNT_SERVER_SCOPE_MISMATCH,
+ NFS4CLNT_BIND_CONN_TO_SESSION,
};

enum nfs4_session_state {
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 7f0fcfc..4f41144 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1639,13 +1639,20 @@ static void nfs41_handle_recallable_state_revoked(struct nfs_client *clp)
nfs_expire_all_delegations(clp);
}

-static void nfs41_handle_cb_path_down(struct nfs_client *clp)
+static void nfs41_handle_backchannel_fault(struct nfs_client *clp)
{
nfs_expire_all_delegations(clp);
if (test_and_set_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state) == 0)
nfs4_schedule_state_manager(clp);
}

+static void nfs41_handle_cb_path_down(struct nfs_client *clp)
+{
+ if (test_and_set_bit(NFS4CLNT_BIND_CONN_TO_SESSION,
+ &clp->cl_state) == 0)
+ nfs4_schedule_state_manager(clp);
+}
+
void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
{
if (!flags)
@@ -1659,9 +1666,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
nfs41_handle_state_revoked(clp);
if (flags & SEQ4_STATUS_RECALLABLE_STATE_REVOKED)
nfs41_handle_recallable_state_revoked(clp);
- if (flags & (SEQ4_STATUS_CB_PATH_DOWN |
- SEQ4_STATUS_BACKCHANNEL_FAULT |
- SEQ4_STATUS_CB_PATH_DOWN_SESSION))
+ if (flags & SEQ4_STATUS_BACKCHANNEL_FAULT)
+ nfs41_handle_backchannel_fault(clp);
+ else if (flags & (SEQ4_STATUS_CB_PATH_DOWN |
+ SEQ4_STATUS_CB_PATH_DOWN_SESSION))
nfs41_handle_cb_path_down(clp);
}

@@ -1686,6 +1694,7 @@ static int nfs4_reset_session(struct nfs_client *clp)
clear_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state);
/* create_session negotiated new slot table */
clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
+ clear_bit(NFS4CLNT_BIND_CONN_TO_SESSION, &clp->cl_state);

/* Let the state manager reestablish state */
if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
@@ -1722,10 +1731,15 @@ static int nfs4_recall_slot(struct nfs_client *clp)
return 0;
}

+static int nfs4_do_bind_conn_to_session(struct nfs_client *clp)
+{
+ return nfs4_proc_bind_conn_to_session(clp);
+}
#else /* CONFIG_NFS_V4_1 */
static int nfs4_reset_session(struct nfs_client *clp) { return 0; }
static int nfs4_end_drain_session(struct nfs_client *clp) { return 0; }
static int nfs4_recall_slot(struct nfs_client *clp) { return 0; }
+static int nfs4_do_bind_conn_to_session(struct nfs_client *clp) { return 0; }
#endif /* CONFIG_NFS_V4_1 */

/* Set NFS4CLNT_LEASE_EXPIRED for all v4.0 errors and for recoverable errors
@@ -1803,6 +1817,15 @@ static void nfs4_state_manager(struct nfs_client *clp)
goto out_error;
}

+ /* Send BIND_CONN_TO_SESSION */
+ if (clp->cl_mvops->minor_version == 1 &&
+ test_and_clear_bit(NFS4CLNT_BIND_CONN_TO_SESSION,
+ &clp->cl_state) && nfs4_has_session(clp)) {
+ status = nfs4_do_bind_conn_to_session(clp);
+ if (status < 0)
+ goto out_error;
+ }
+
/* First recover reboot state... */
if (test_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state)) {
status = nfs4_do_reclaim(clp,
--
1.7.4.4


2012-05-24 16:30:19

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:

> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
> without destroying the session.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4_fs.h | 1 +
> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/nfs4.h | 5 +++
> include/linux/nfs_xdr.h | 6 +++
> 5 files changed, 156 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index edeef71..ad90bc7 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -215,6 +215,7 @@ struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
> extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *);
> extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *);
> extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
> +extern int nfs4_proc_bind_conn_to_session(struct nfs_client *);
> extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
> extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
> extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 78784e5..7a4b320 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5062,6 +5062,60 @@ nfs41_same_server_scope(struct server_scope *a, struct server_scope *b)
> }
>
> /*
> + * nfs4_proc_bind_conn_to_session()
> + *
> + * The 4.1 client currently uses the same TCP connection for the
> + * fore and backchannel.
> + */
> +int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
> +{
> + int status;
> + struct nfs41_bind_conn_to_session_res res;
> + struct rpc_message msg = {
> + .rpc_proc =
> + &nfs4_procedures[NFSPROC4_CLNT_BIND_CONN_TO_SESSION],
> + .rpc_argp = clp,
> + .rpc_resp = &res,
> + };
> +
> + dprintk("--> %s\n", __func__);
> + BUG_ON(clp == NULL);
> +
> + res.session = kzalloc(sizeof(struct nfs4_session), GFP_KERNEL);
> + if (unlikely(res.session == NULL)) {
> + status = -ENOMEM;
> + goto out;
> + }
> +
> + status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> + if (status == 0) {
> + if (memcmp(res.session->sess_id.data,
> + clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) {
> + dprintk("NFS: %s: Session ID mismatch\n", __func__);
> + status = -EIO;
> + goto out_session;
> + }
> + if (res.dir != CDFS4_BOTH) {
> + dprintk("NFS: %s: Unexpected direction from server\n",
> + __func__);
> + status = -EIO;
> + goto out_session;
> + }
> + if (res.use_conn_in_rdma_mode != 0) {
> + dprintk("NFS: %s: Server returned RDMA mode = true\n",
> + __func__);
> + status = -EIO;
> + goto out_session;
> + }
> + }
> +out_session:
> + kfree(res.session);
> +out:
> + dprintk("<-- %s status= %d\n", __func__, status);
> + return status;
> +}
> +
> +/*
> * nfs4_proc_exchange_id()
> *
> * Since the clientid has expired, all compounds using sessions
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index db040e9..37e09c8 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -321,6 +321,16 @@ static int nfs4_stat_to_errno(int);
> 1 /* csr_flags */ + \
> decode_channel_attrs_maxsz + \
> decode_channel_attrs_maxsz)
> +#define encode_bind_conn_to_session_maxsz (op_encode_hdr_maxsz + \
> + /* bctsa_sessid */ \
> + XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
> + 1 /* bctsa_dir */ + \
> + 1 /* bctsa_use_conn_in_rdma_mode */)
> +#define decode_bind_conn_to_session_maxsz (op_decode_hdr_maxsz + \
> + /* bctsr_sessid */ \
> + XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
> + 1 /* bctsr_dir */ + \
> + 1 /* bctsr_use_conn_in_rdma_mode */)
> #define encode_destroy_session_maxsz (op_encode_hdr_maxsz + 4)
> #define decode_destroy_session_maxsz (op_decode_hdr_maxsz)
> #define encode_sequence_maxsz (op_encode_hdr_maxsz + \
> @@ -714,6 +724,12 @@ static int nfs4_stat_to_errno(int);
> decode_putfh_maxsz + \
> decode_secinfo_maxsz)
> #if defined(CONFIG_NFS_V4_1)
> +#define NFS4_enc_bind_conn_to_session_sz \
> + (compound_encode_hdr_maxsz + \
> + encode_bind_conn_to_session_maxsz)
> +#define NFS4_dec_bind_conn_to_session_sz \
> + (compound_decode_hdr_maxsz + \
> + decode_bind_conn_to_session_maxsz)
> #define NFS4_enc_exchange_id_sz \
> (compound_encode_hdr_maxsz + \
> encode_exchange_id_maxsz)
> @@ -1654,6 +1670,20 @@ static void encode_secinfo(struct xdr_stream *xdr, const struct qstr *name, stru
>
> #if defined(CONFIG_NFS_V4_1)
> /* NFSv4.1 operations */
> +static void encode_bind_conn_to_session(struct xdr_stream *xdr,
> + struct nfs4_session *session,
> + struct compound_hdr *hdr)
> +{
> + __be32 *p;
> +
> + encode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION,
> + decode_bind_conn_to_session_maxsz, hdr);
> + encode_opaque_fixed(xdr, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
> + p = xdr_reserve_space(xdr, 8);
> + *p++ = cpu_to_be32(NFS4_CDFC4_BACK_OR_BOTH);
> + *p = 0; /* use_conn_in_rdma_mode = False */
> +}
> +
> static void encode_exchange_id(struct xdr_stream *xdr,
> struct nfs41_exchange_id_args *args,
> struct compound_hdr *hdr)
> @@ -2614,6 +2644,22 @@ static void nfs4_xdr_enc_secinfo(struct rpc_rqst *req,
>
> #if defined(CONFIG_NFS_V4_1)
> /*
> + * BIND_CONN_TO_SESSION request
> + */
> +static void nfs4_xdr_enc_bind_conn_to_session(struct rpc_rqst *req,
> + struct xdr_stream *xdr,
> + struct nfs_client *clp)
> +{
> + struct compound_hdr hdr = {
> + .minorversion = clp->cl_mvops->minor_version,
> + };
> +
> + encode_compound_hdr(xdr, req, &hdr);
> + encode_bind_conn_to_session(xdr, clp->cl_session, &hdr);
> + encode_nops(&hdr);
> +}
> +
> +/*
> * EXCHANGE_ID request
> */
> static void nfs4_xdr_enc_exchange_id(struct rpc_rqst *req,
> @@ -5239,6 +5285,32 @@ static int decode_sessionid(struct xdr_stream *xdr, struct nfs4_sessionid *sid)
> return decode_opaque_fixed(xdr, sid->data, NFS4_MAX_SESSIONID_LEN);
> }
>
> +static int decode_bind_conn_to_session(struct xdr_stream *xdr,
> + struct nfs41_bind_conn_to_session_res *res)
> +{
> + __be32 *p;
> + int status;
> +
> + status = decode_op_hdr(xdr, OP_BIND_CONN_TO_SESSION);
> + if (!status)
> + status = decode_sessionid(xdr, &res->session->sess_id);
> + if (unlikely(status))
> + return status;
> +
> + /* dir flags, rdma mode bool */
> + p = xdr_inline_decode(xdr, 8);
> + if (unlikely(!p))
> + goto out_overflow;
> +
> + res->dir = be32_to_cpup(p++);
> + res->use_conn_in_rdma_mode = be32_to_cpup(p);
> +
> + return 0;
> +out_overflow:
> + print_overflow_msg(__func__, xdr);
> + return -EIO;
> +}
> +
> static int decode_create_session(struct xdr_stream *xdr,
> struct nfs41_create_session_res *res)
> {
> @@ -6521,6 +6593,22 @@ out:
>
> #if defined(CONFIG_NFS_V4_1)
> /*
> + * Decode BIND_CONN_TO_SESSION response
> + */
> +static int nfs4_xdr_dec_bind_conn_to_session(struct rpc_rqst *rqstp,
> + struct xdr_stream *xdr,
> + void *res)
> +{
> + struct compound_hdr hdr;
> + int status;
> +
> + status = decode_compound_hdr(xdr, &hdr);
> + if (!status)
> + status = decode_bind_conn_to_session(xdr, res);
> + return status;
> +}
> +
> +/*
> * Decode EXCHANGE_ID response
> */
> static int nfs4_xdr_dec_exchange_id(struct rpc_rqst *rqstp,
> @@ -7001,6 +7089,8 @@ struct rpc_procinfo nfs4_procedures[] = {
> PROC(RELEASE_LOCKOWNER, enc_release_lockowner, dec_release_lockowner),
> PROC(SECINFO, enc_secinfo, dec_secinfo),
> #if defined(CONFIG_NFS_V4_1)
> + PROC(BIND_CONN_TO_SESSION,
> + enc_bind_conn_to_session, dec_bind_conn_to_session),
> PROC(EXCHANGE_ID, enc_exchange_id, dec_exchange_id),
> PROC(CREATE_SESSION, enc_create_session, dec_create_session),
> PROC(DESTROY_SESSION, enc_destroy_session, dec_destroy_session),
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 0987146..80fbbfc6 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -69,6 +69,10 @@
> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>
> +#define CDFS4_FORE 0x1
> +#define CDFS4_BACK 0x2
> +#define CDFS4_BOTH 0x3
> +
> #define NFS4_SET_TO_SERVER_TIME 0
> #define NFS4_SET_TO_CLIENT_TIME 1
>
> @@ -582,6 +586,7 @@ enum {
> NFSPROC4_CLNT_SECINFO,
>
> /* nfs41 */
> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
> NFSPROC4_CLNT_EXCHANGE_ID,
> NFSPROC4_CLNT_CREATE_SESSION,
> NFSPROC4_CLNT_DESTROY_SESSION,
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 2e53a3f..dd3bc2f 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1115,6 +1115,12 @@ struct nfs41_impl_id {
> struct nfstime4 date;
> };
>
> +struct nfs41_bind_conn_to_session_res {
> + struct nfs4_session *session;
> + u32 dir;
> + u32 use_conn_in_rdma_mode;

Would it make sense to use a "bool" here instead?

> +};
> +
> struct nfs41_exchange_id_res {
> struct nfs_client *client;
> u32 flags;
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2012-05-24 16:46:07

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation


On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:

> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
> without destroying the session.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4_fs.h | 1 +
> fs/nfs/nfs4proc.c | 54 ++++++++++++++++++++++++++++
> fs/nfs/nfs4xdr.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/nfs4.h | 5 +++
> include/linux/nfs_xdr.h | 6 +++
> 5 files changed, 156 insertions(+), 0 deletions(-)

[ ... snipped ... ]

> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 0987146..80fbbfc6 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -69,6 +69,10 @@
> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>
> +#define CDFS4_FORE 0x1
> +#define CDFS4_BACK 0x2
> +#define CDFS4_BOTH 0x3

A few more nits:

I'm looking back at the patch I did last year to add BIND_CONN_TO_SESS for NFSv4.1 migration support... I used NFS4_CDFS4_FORE and so on here, to match the defines in the paragraph above.

In the XDR decoder function, I check the "dir_from_server" field to ensure it is equal to or less than NFS4_CDFS4_BOTH (-EIO if not).

> +
> #define NFS4_SET_TO_SERVER_TIME 0
> #define NFS4_SET_TO_CLIENT_TIME 1
>
> @@ -582,6 +586,7 @@ enum {
> NFSPROC4_CLNT_SECINFO,
>
> /* nfs41 */
> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
> NFSPROC4_CLNT_EXCHANGE_ID,
> NFSPROC4_CLNT_CREATE_SESSION,
> NFSPROC4_CLNT_DESTROY_SESSION,

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com