[PATCH Version 7 0/8] pNFS file layout session trunking

[PATCH Version 7 0/8] pNFS file layout session trunking

[andros]

From: Andy Adamson <andros@netapp.com>

This patch set differs from previous session trunking patch sets in that it
only addresses pNFS filelayout data server session trunking.

Andy Adamson (8):
  NFS setup async exchange_id
  NFS refactor nfs4_match_clientids
  NFS refactor nfs4_check_serverowner_major_id
  NFS detect session trunking
  SUNRPC add remove xprt flag to rpc_task_release_client
  SUNRPC add an RPC null call to test session trunking connection
  NFS test session trunking with exchange id
  NFS pnfs data server multipath session trunking

 fs/nfs/nfs4_fs.h            |   9 ++
 fs/nfs/nfs4client.c         | 114 ++++++++++++++--
 fs/nfs/nfs4proc.c           | 310 ++++++++++++++++++++++++++++++++------------
 fs/nfs/pnfs_nfs.c           |  52 ++++++--
 include/linux/sunrpc/clnt.h |   4 +-
 net/sunrpc/clnt.c           |  25 +++-
 net/sunrpc/sched.c          |   2 +-
 net/sunrpc/xprtmultipath.c  |   2 +
 8 files changed, 402 insertions(+), 116 deletions(-)

-- 
1.8.3.1

[PATCH Version 7 1/8] NFS setup async exchange_id

[andros]

From: Andy Adamson <andros@netapp.com>

Testing an rpc_xprt for session trunking should not delay application
progress over already established transports.
Setup exchange_id to be able to be an async call to test an rpc_xprt
for session trunking use.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4proc.c | 215 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 134 insertions(+), 81 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index da5c9e5..e29f5ce 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7051,6 +7051,80 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
 	return 0;
 }
 
+struct nfs41_exchange_id_data {
+	struct nfs41_exchange_id_res res;
+	struct nfs41_exchange_id_args args;
+	int rpc_status;
+};
+
+static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
+{
+	struct nfs41_exchange_id_data *cdata =
+					(struct nfs41_exchange_id_data *)data;
+	struct nfs_client *clp = cdata->args.client;
+	int status = task->tk_status;
+
+	trace_nfs4_exchange_id(clp, status);
+
+	if (status == 0)
+		status = nfs4_check_cl_exchange_flags(cdata->res.flags);
+	if (status  == 0)
+		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
+
+	if (status == 0) {
+		clp->cl_clientid = cdata->res.clientid;
+		clp->cl_exchange_flags = cdata->res.flags;
+		/* Client ID is not confirmed */
+		if (!(cdata->res.flags & EXCHGID4_FLAG_CONFIRMED_R)) {
+			clear_bit(NFS4_SESSION_ESTABLISHED,
+			&clp->cl_session->session_state);
+			clp->cl_seqid = cdata->res.seqid;
+		}
+
+		kfree(clp->cl_serverowner);
+		clp->cl_serverowner = cdata->res.server_owner;
+		cdata->res.server_owner = NULL;
+
+		/* use the most recent implementation id */
+		kfree(clp->cl_implid);
+		clp->cl_implid = cdata->res.impl_id;
+		cdata->res.impl_id = NULL;
+
+		if (clp->cl_serverscope != NULL &&
+		    !nfs41_same_server_scope(clp->cl_serverscope,
+					cdata->res.server_scope)) {
+			dprintk("%s: server_scope mismatch detected\n",
+				__func__);
+			set_bit(NFS4CLNT_SERVER_SCOPE_MISMATCH, &clp->cl_state);
+			kfree(clp->cl_serverscope);
+			clp->cl_serverscope = NULL;
+		}
+
+		if (clp->cl_serverscope == NULL) {
+			clp->cl_serverscope = cdata->res.server_scope;
+			cdata->res.server_scope = NULL;
+		}
+	}
+	cdata->rpc_status = status;
+}
+
+static void nfs4_exchange_id_release(void *data)
+{
+	struct nfs41_exchange_id_data *cdata =
+					(struct nfs41_exchange_id_data *)data;
+
+	nfs_put_client(cdata->args.client);
+	kfree(cdata->res.impl_id);
+	kfree(cdata->res.server_scope);
+	kfree(cdata->res.server_owner);
+	kfree(cdata);
+}
+
+static const struct rpc_call_ops nfs4_exchange_id_call_ops = {
+	.rpc_call_done = nfs4_exchange_id_done,
+	.rpc_release = nfs4_exchange_id_release,
+};
+
 /*
  * _nfs4_proc_exchange_id()
  *
@@ -7060,66 +7134,60 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	u32 sp4_how)
 {
 	nfs4_verifier verifier;
-	struct nfs41_exchange_id_args args = {
-		.verifier = &verifier,
-		.client = clp,
-#ifdef CONFIG_NFS_V4_1_MIGRATION
-		.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
-			 EXCHGID4_FLAG_BIND_PRINC_STATEID |
-			 EXCHGID4_FLAG_SUPP_MOVED_MIGR,
-#else
-		.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
-			 EXCHGID4_FLAG_BIND_PRINC_STATEID,
-#endif
-	};
-	struct nfs41_exchange_id_res res = {
-		0
-	};
-	int status;
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_EXCHANGE_ID],
-		.rpc_argp = &args,
-		.rpc_resp = &res,
 		.rpc_cred = cred,
 	};
+	struct rpc_task_setup task_setup_data = {
+		.rpc_client = clp->cl_rpcclient,
+		.callback_ops = &nfs4_exchange_id_call_ops,
+		.rpc_message = &msg,
+		.flags = RPC_TASK_ASYNC | RPC_TASK_TIMEOUT,
+	};
+	struct nfs41_exchange_id_data *calldata;
+	struct rpc_task *task;
+	int status = -EIO;
+
+	if (!atomic_inc_not_zero(&clp->cl_count))
+		goto out;
+
+	status = -ENOMEM;
+	calldata = kzalloc(sizeof(*calldata), GFP_NOFS);
+	if (!calldata)
+		goto out;
 
 	nfs4_init_boot_verifier(clp, &verifier);
 
 	status = nfs4_init_uniform_client_string(clp);
 	if (status)
-		goto out;
+		goto out_calldata;
 
 	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
 		clp->cl_owner_id);
 
-	res.server_owner = kzalloc(sizeof(struct nfs41_server_owner),
-					GFP_NOFS);
-	if (unlikely(res.server_owner == NULL)) {
-		status = -ENOMEM;
-		goto out;
-	}
+	calldata->res.server_owner = kzalloc(sizeof(struct nfs41_server_owner),
+						GFP_NOFS);
+	status = -ENOMEM;
+	if (unlikely(calldata->res.server_owner == NULL))
+		goto out_calldata;
 
-	res.server_scope = kzalloc(sizeof(struct nfs41_server_scope),
+	calldata->res.server_scope = kzalloc(sizeof(struct nfs41_server_scope),
 					GFP_NOFS);
-	if (unlikely(res.server_scope == NULL)) {
-		status = -ENOMEM;
+	if (unlikely(calldata->res.server_scope == NULL))
 		goto out_server_owner;
-	}
 
-	res.impl_id = kzalloc(sizeof(struct nfs41_impl_id), GFP_NOFS);
-	if (unlikely(res.impl_id == NULL)) {
-		status = -ENOMEM;
+	calldata->res.impl_id = kzalloc(sizeof(struct nfs41_impl_id), GFP_NOFS);
+	if (unlikely(calldata->res.impl_id == NULL))
 		goto out_server_scope;
-	}
 
 	switch (sp4_how) {
 	case SP4_NONE:
-		args.state_protect.how = SP4_NONE;
+		calldata->args.state_protect.how = SP4_NONE;
 		break;
 
 	case SP4_MACH_CRED:
-		args.state_protect = nfs4_sp4_mach_cred_request;
+		calldata->args.state_protect = nfs4_sp4_mach_cred_request;
 		break;
 
 	default:
@@ -7129,55 +7197,30 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		goto out_impl_id;
 	}
 
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
-	trace_nfs4_exchange_id(clp, status);
-	if (status == 0)
-		status = nfs4_check_cl_exchange_flags(res.flags);
-
-	if (status == 0)
-		status = nfs4_sp4_select_mode(clp, &res.state_protect);
-
-	if (status == 0) {
-		clp->cl_clientid = res.clientid;
-		clp->cl_exchange_flags = res.flags;
-		/* Client ID is not confirmed */
-		if (!(res.flags & EXCHGID4_FLAG_CONFIRMED_R)) {
-			clear_bit(NFS4_SESSION_ESTABLISHED,
-					&clp->cl_session->session_state);
-			clp->cl_seqid = res.seqid;
-		}
-
-		kfree(clp->cl_serverowner);
-		clp->cl_serverowner = res.server_owner;
-		res.server_owner = NULL;
-
-		/* use the most recent implementation id */
-		kfree(clp->cl_implid);
-		clp->cl_implid = res.impl_id;
-		res.impl_id = NULL;
-
-		if (clp->cl_serverscope != NULL &&
-		    !nfs41_same_server_scope(clp->cl_serverscope,
-					     res.server_scope)) {
-			dprintk("%s: server_scope mismatch detected\n",
-				__func__);
-			set_bit(NFS4CLNT_SERVER_SCOPE_MISMATCH, &clp->cl_state);
-			kfree(clp->cl_serverscope);
-			clp->cl_serverscope = NULL;
-		}
+	calldata->args.verifier = &verifier;
+	calldata->args.client = clp;
+#ifdef CONFIG_NFS_V4_1_MIGRATION
+	calldata->args.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
+	EXCHGID4_FLAG_BIND_PRINC_STATEID |
+	EXCHGID4_FLAG_SUPP_MOVED_MIGR,
+#else
+	calldata->args.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
+	EXCHGID4_FLAG_BIND_PRINC_STATEID,
+#endif
+	msg.rpc_argp = &calldata->args;
+	msg.rpc_resp = &calldata->res;
+	task_setup_data.callback_data = calldata;
 
-		if (clp->cl_serverscope == NULL) {
-			clp->cl_serverscope = res.server_scope;
-			res.server_scope = NULL;
-		}
+	task = rpc_run_task(&task_setup_data);
+	if (IS_ERR(task)) {
+	status = PTR_ERR(task);
+		goto out_impl_id;
 	}
 
-out_impl_id:
-	kfree(res.impl_id);
-out_server_scope:
-	kfree(res.server_scope);
-out_server_owner:
-	kfree(res.server_owner);
+	status = rpc_wait_for_completion_task(task);
+	if (!status)
+		status = calldata->rpc_status;
+	rpc_put_task(task);
 out:
 	if (clp->cl_implid != NULL)
 		dprintk("NFS reply exchange_id: Server Implementation ID: "
@@ -7187,6 +7230,16 @@ out:
 			clp->cl_implid->date.nseconds);
 	dprintk("NFS reply exchange_id: %d\n", status);
 	return status;
+
+out_impl_id:
+	kfree(calldata->res.impl_id);
+out_server_scope:
+	kfree(calldata->res.server_scope);
+out_server_owner:
+	kfree(calldata->res.server_owner);
+out_calldata:
+	kfree(calldata);
+	goto out;
 }
 
 /*
-- 
1.8.3.1

[PATCH Version 7 2/8] NFS refactor nfs4_match_clientids

[andros]

From: Andy Adamson <andros@netapp.com>

For session trunking, to compare nfs41_exchange_id_res with
exiting nfs_client.

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

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 8d7d08d..d77a1ad 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -562,15 +562,15 @@ out:
 /*
  * Returns true if the client IDs match
  */
-static bool nfs4_match_clientids(struct nfs_client *a, struct nfs_client *b)
+static bool nfs4_match_clientids(u64 a, u64 b)
 {
-	if (a->cl_clientid != b->cl_clientid) {
+	if (a != b) {
 		dprintk("NFS: --> %s client ID %llx does not match %llx\n",
-			__func__, a->cl_clientid, b->cl_clientid);
+			__func__, a, b);
 		return false;
 	}
 	dprintk("NFS: --> %s client ID %llx matches %llx\n",
-		__func__, a->cl_clientid, b->cl_clientid);
+		__func__, a, b);
 	return true;
 }
 
@@ -650,7 +650,7 @@ int nfs41_walk_client_list(struct nfs_client *new,
 		if (pos->cl_cons_state != NFS_CS_READY)
 			continue;
 
-		if (!nfs4_match_clientids(pos, new))
+		if (!nfs4_match_clientids(pos->cl_clientid, new->cl_clientid))
 			continue;
 
 		/*
-- 
1.8.3.1

[PATCH Version 7 3/8] NFS refactor nfs4_check_serverowner_major_id

[andros]

From: Andy Adamson <andros@netapp.com>

For session trunking, to compare nfs41_exchange_id_res with
existing nfs_client

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4client.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index d77a1ad..f356d50 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -578,17 +578,15 @@ static bool nfs4_match_clientids(u64 a, u64 b)
  * Returns true if the server major ids match
  */
 static bool
-nfs4_check_clientid_trunking(struct nfs_client *a, struct nfs_client *b)
+nfs4_check_serverowner_major_id(struct nfs41_server_owner *o1,
+				struct nfs41_server_owner *o2)
 {
-	struct nfs41_server_owner *o1 = a->cl_serverowner;
-	struct nfs41_server_owner *o2 = b->cl_serverowner;
-
 	if (o1->major_id_sz != o2->major_id_sz)
 		goto out_major_mismatch;
 	if (memcmp(o1->major_id, o2->major_id, o1->major_id_sz) != 0)
 		goto out_major_mismatch;
 
-	dprintk("NFS: --> %s server owners match\n", __func__);
+	dprintk("NFS: --> %s server owner major IDs match\n", __func__);
 	return true;
 
 out_major_mismatch:
@@ -658,7 +656,8 @@ int nfs41_walk_client_list(struct nfs_client *new,
 		 * client id trunking. In either case, we want to fall back
 		 * to using the existing nfs_client.
 		 */
-		if (!nfs4_check_clientid_trunking(pos, new))
+		if (!nfs4_check_serverowner_major_id(pos->cl_serverowner,
+						     new->cl_serverowner))
 			continue;
 
 		/* Unlike NFSv4.0, we know that NFSv4.1 always uses the
-- 
1.8.3.1

[PATCH Version 7 4/8] NFS detect session trunking

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4_fs.h    |  2 ++
 fs/nfs/nfs4client.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4be567a..eb315e1 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -277,6 +277,8 @@ extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
 		struct nfs_fsinfo *fsinfo);
 extern int nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data,
 				  bool sync);
+extern int nfs4_detect_session_trunking(struct nfs_client *clp,
+		struct nfs41_exchange_id_res *res, struct rpc_xprt *xprt);
 
 static inline bool
 is_ds_only_client(struct nfs_client *clp)
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index f356d50..37a9416 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -595,6 +595,99 @@ out_major_mismatch:
 	return false;
 }
 
+/*
+ * Returns true if server minor ids match
+ */
+static bool
+nfs4_check_serverowner_minor_id(struct nfs41_server_owner *o1,
+				struct nfs41_server_owner *o2)
+{
+	/* Check eir_server_owner so_minor_id */
+	if (o1->minor_id != o2->minor_id)
+		goto out_minor_mismatch;
+
+	dprintk("NFS: --> %s server owner minor IDs s match\n", __func__);
+	return true;
+
+out_minor_mismatch:
+	dprintk("NFS: --> %s server owner minor IDs do not match\n", __func__);
+	return false;
+}
+
+/*
+ * Returns true if the server scopes match
+ */
+static bool
+nfs4_check_server_scope(struct nfs41_server_scope *s1,
+			struct nfs41_server_scope *s2)
+{
+	if (s1->server_scope_sz != s2->server_scope_sz)
+		goto out_scope_mismatch;
+	if (memcmp(s1->server_scope, s2->server_scope,
+		   s1->server_scope_sz) != 0)
+		goto out_scope_mismatch;
+
+	dprintk("NFS: --> %s server scopes match\n", __func__);
+	return true;
+
+out_scope_mismatch:
+	dprintk("NFS: --> %s server scopes do not match\n",
+		__func__);
+	return false;
+}
+
+/**
+ * nfs4_detect_session_trunking - Checks for ssession trunking called
+ * after a successuful EXCHANGE_ID testing a multi-addr connection to be
+ * potentially added as a session trunk
+ *
+ * @clp:    original mount nfs_client
+ * @res:    result structure from an exchange_id using the original mount
+ *          nfs_client with a new multi_addr transport
+ *
+ * Returns zero on success, otherwise -EINVAL
+ *
+ * Note: since the exchange_id for the new multi_addr transport uses the
+ * same nfs_client from the original mount, the cl_owner_id is reused,
+ * so eir_clientowner is the same.
+ */
+int nfs4_detect_session_trunking(struct nfs_client *clp,
+				 struct nfs41_exchange_id_res *res,
+				 struct rpc_xprt *xprt)
+{
+	int status = -EINVAL;
+	/* Check eir_clientid */
+	if (!nfs4_match_clientids(clp->cl_clientid, res->clientid))
+		goto out;
+
+	/* Check eir_server_owner so_major_id */
+	if (!nfs4_check_serverowner_major_id(clp->cl_serverowner,
+					     res->server_owner))
+		goto out;
+
+	/* Check eir_server_owner so_minor_id */
+	if (!nfs4_check_serverowner_minor_id(clp->cl_serverowner,
+					     res->server_owner))
+		goto out;
+
+	/* Check eir_server_scope */
+	if (!nfs4_check_server_scope(clp->cl_serverscope, res->server_scope))
+		goto out;
+
+	status = 0;
+out:
+	if (status)
+		pr_info("NFS:  %s: Session trunking failed for %s status %d\n",
+			clp->cl_hostname,
+			xprt->address_strings[RPC_DISPLAY_ADDR], status);
+	else
+		pr_info("NFS:  %s: Session trunking succeeded for %s\n",
+			clp->cl_hostname,
+			xprt->address_strings[RPC_DISPLAY_ADDR]);
+
+	return status;
+}
+
 /**
  * nfs41_walk_client_list - Find nfs_client that matches a client/server owner
  *
-- 
1.8.3.1

Re: [PATCH Version 7 4/8] NFS detect session trunking

[Anna Schumaker]

Hi Andy,

One small typo below:

On 07/27/2016 02:43 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4_fs.h    |  2 ++
>  fs/nfs/nfs4client.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
> 
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 4be567a..eb315e1 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -277,6 +277,8 @@ extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
>  		struct nfs_fsinfo *fsinfo);
>  extern int nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data,
>  				  bool sync);
> +extern int nfs4_detect_session_trunking(struct nfs_client *clp,
> +		struct nfs41_exchange_id_res *res, struct rpc_xprt *xprt);
>  
>  static inline bool
>  is_ds_only_client(struct nfs_client *clp)
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index f356d50..37a9416 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -595,6 +595,99 @@ out_major_mismatch:
>  	return false;
>  }
>  
> +/*
> + * Returns true if server minor ids match
> + */
> +static bool
> +nfs4_check_serverowner_minor_id(struct nfs41_server_owner *o1,
> +				struct nfs41_server_owner *o2)
> +{
> +	/* Check eir_server_owner so_minor_id */
> +	if (o1->minor_id != o2->minor_id)
> +		goto out_minor_mismatch;
> +
> +	dprintk("NFS: --> %s server owner minor IDs s match\n", __func__);
                                                   ^^^
You have an extra "s" in this dprintk().

Thanks,
Anna

> +	return true;
> +
> +out_minor_mismatch:
> +	dprintk("NFS: --> %s server owner minor IDs do not match\n", __func__);
> +	return false;
> +}
> +
> +/*
> + * Returns true if the server scopes match
> + */
> +static bool
> +nfs4_check_server_scope(struct nfs41_server_scope *s1,
> +			struct nfs41_server_scope *s2)
> +{
> +	if (s1->server_scope_sz != s2->server_scope_sz)
> +		goto out_scope_mismatch;
> +	if (memcmp(s1->server_scope, s2->server_scope,
> +		   s1->server_scope_sz) != 0)
> +		goto out_scope_mismatch;
> +
> +	dprintk("NFS: --> %s server scopes match\n", __func__);
> +	return true;
> +
> +out_scope_mismatch:
> +	dprintk("NFS: --> %s server scopes do not match\n",
> +		__func__);
> +	return false;
> +}
> +
> +/**
> + * nfs4_detect_session_trunking - Checks for ssession trunking called
> + * after a successuful EXCHANGE_ID testing a multi-addr connection to be
> + * potentially added as a session trunk
> + *
> + * @clp:    original mount nfs_client
> + * @res:    result structure from an exchange_id using the original mount
> + *          nfs_client with a new multi_addr transport
> + *
> + * Returns zero on success, otherwise -EINVAL
> + *
> + * Note: since the exchange_id for the new multi_addr transport uses the
> + * same nfs_client from the original mount, the cl_owner_id is reused,
> + * so eir_clientowner is the same.
> + */
> +int nfs4_detect_session_trunking(struct nfs_client *clp,
> +				 struct nfs41_exchange_id_res *res,
> +				 struct rpc_xprt *xprt)
> +{
> +	int status = -EINVAL;
> +	/* Check eir_clientid */
> +	if (!nfs4_match_clientids(clp->cl_clientid, res->clientid))
> +		goto out;
> +
> +	/* Check eir_server_owner so_major_id */
> +	if (!nfs4_check_serverowner_major_id(clp->cl_serverowner,
> +					     res->server_owner))
> +		goto out;
> +
> +	/* Check eir_server_owner so_minor_id */
> +	if (!nfs4_check_serverowner_minor_id(clp->cl_serverowner,
> +					     res->server_owner))
> +		goto out;
> +
> +	/* Check eir_server_scope */
> +	if (!nfs4_check_server_scope(clp->cl_serverscope, res->server_scope))
> +		goto out;
> +
> +	status = 0;
> +out:
> +	if (status)
> +		pr_info("NFS:  %s: Session trunking failed for %s status %d\n",
> +			clp->cl_hostname,
> +			xprt->address_strings[RPC_DISPLAY_ADDR], status);
> +	else
> +		pr_info("NFS:  %s: Session trunking succeeded for %s\n",
> +			clp->cl_hostname,
> +			xprt->address_strings[RPC_DISPLAY_ADDR]);
> +
> +	return status;
> +}
> +
>  /**
>   * nfs41_walk_client_list - Find nfs_client that matches a client/server owner
>   *
> 

[PATCH Version 7 5/8] SUNRPC add remove xprt flag to rpc_task_release_client

[andros]

From: Andy Adamson <andros@netapp.com>

Want to specify which rpc_xprt to use in rpc_run_task.

Don't pass in an rpc_xprt in rpc_init_task just to have it not used as it
is removed in rpc_task_release_client.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 include/linux/sunrpc/clnt.h | 2 +-
 net/sunrpc/clnt.c           | 7 +++----
 net/sunrpc/sched.c          | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index b6810c9..99410bb 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -148,7 +148,7 @@ int		rpc_switch_client_transport(struct rpc_clnt *,
 
 void		rpc_shutdown_client(struct rpc_clnt *);
 void		rpc_release_client(struct rpc_clnt *);
-void		rpc_task_release_client(struct rpc_task *);
+void		rpc_task_release_client(struct rpc_task *, int);
 
 int		rpcb_create_local(struct net *);
 void		rpcb_put_local(struct net *);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index cb49898..459f9b1 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -961,7 +961,7 @@ out:
 }
 EXPORT_SYMBOL_GPL(rpc_bind_new_program);
 
-void rpc_task_release_client(struct rpc_task *task)
+void rpc_task_release_client(struct rpc_task *task, int rm_xprt)
 {
 	struct rpc_clnt *clnt = task->tk_client;
 	struct rpc_xprt *xprt = task->tk_xprt;
@@ -976,9 +976,8 @@ void rpc_task_release_client(struct rpc_task *task)
 		rpc_release_client(clnt);
 	}
 
-	if (xprt != NULL) {
+	if (rm_xprt && xprt) {
 		task->tk_xprt = NULL;
-
 		xprt_put(xprt);
 	}
 }
@@ -988,7 +987,7 @@ void rpc_task_set_client(struct rpc_task *task, struct rpc_clnt *clnt)
 {
 
 	if (clnt != NULL) {
-		rpc_task_release_client(task);
+		rpc_task_release_client(task, 0);
 		if (task->tk_xprt == NULL)
 			task->tk_xprt = xprt_iter_get_next(&clnt->cl_xpi);
 		task->tk_client = clnt;
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 9ae5885..575b254 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -1017,7 +1017,7 @@ static void rpc_release_resources_task(struct rpc_task *task)
 		put_rpccred(task->tk_msg.rpc_cred);
 		task->tk_msg.rpc_cred = NULL;
 	}
-	rpc_task_release_client(task);
+	rpc_task_release_client(task, 1);
 }
 
 static void rpc_final_put_task(struct rpc_task *task,
-- 
1.8.3.1

Re: [PATCH Version 7 5/8] SUNRPC add remove xprt flag to rpc_task_release_client

[Anna Schumaker]

Hi Andy,

On 07/27/2016 02:43 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Want to specify which rpc_xprt to use in rpc_run_task.
> 
> Don't pass in an rpc_xprt in rpc_init_task just to have it not used as it
> is removed in rpc_task_release_client.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  include/linux/sunrpc/clnt.h | 2 +-
>  net/sunrpc/clnt.c           | 7 +++----
>  net/sunrpc/sched.c          | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index b6810c9..99410bb 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -148,7 +148,7 @@ int		rpc_switch_client_transport(struct rpc_clnt *,
>  
>  void		rpc_shutdown_client(struct rpc_clnt *);
>  void		rpc_release_client(struct rpc_clnt *);
> -void		rpc_task_release_client(struct rpc_task *);
> +void		rpc_task_release_client(struct rpc_task *, int);

Can you change the new parameter to a bool instead of an int, please?

Thanks,
Anna

>  
>  int		rpcb_create_local(struct net *);
>  void		rpcb_put_local(struct net *);
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index cb49898..459f9b1 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -961,7 +961,7 @@ out:
>  }
>  EXPORT_SYMBOL_GPL(rpc_bind_new_program);
>  
> -void rpc_task_release_client(struct rpc_task *task)
> +void rpc_task_release_client(struct rpc_task *task, int rm_xprt)
>  {
>  	struct rpc_clnt *clnt = task->tk_client;
>  	struct rpc_xprt *xprt = task->tk_xprt;
> @@ -976,9 +976,8 @@ void rpc_task_release_client(struct rpc_task *task)
>  		rpc_release_client(clnt);
>  	}
>  
> -	if (xprt != NULL) {
> +	if (rm_xprt && xprt) {
>  		task->tk_xprt = NULL;
> -
>  		xprt_put(xprt);
>  	}
>  }
> @@ -988,7 +987,7 @@ void rpc_task_set_client(struct rpc_task *task, struct rpc_clnt *clnt)
>  {
>  
>  	if (clnt != NULL) {
> -		rpc_task_release_client(task);
> +		rpc_task_release_client(task, 0);
>  		if (task->tk_xprt == NULL)
>  			task->tk_xprt = xprt_iter_get_next(&clnt->cl_xpi);
>  		task->tk_client = clnt;
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index 9ae5885..575b254 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -1017,7 +1017,7 @@ static void rpc_release_resources_task(struct rpc_task *task)
>  		put_rpccred(task->tk_msg.rpc_cred);
>  		task->tk_msg.rpc_cred = NULL;
>  	}
> -	rpc_task_release_client(task);
> +	rpc_task_release_client(task, 1);
>  }
>  
>  static void rpc_final_put_task(struct rpc_task *task,
> 

Re: [PATCH Version 7 5/8] SUNRPC add remove xprt flag to rpc_task_release_client

[Trond Myklebust]

> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
> 
> From: Andy Adamson <andros@netapp.com>
> 
> Want to specify which rpc_xprt to use in rpc_run_task.
> 
> Don't pass in an rpc_xprt in rpc_init_task just to have it not used as it
> is removed in rpc_task_release_client.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> include/linux/sunrpc/clnt.h | 2 +-
> net/sunrpc/clnt.c           | 7 +++----
> net/sunrpc/sched.c          | 2 +-
> 3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index b6810c9..99410bb 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -148,7 +148,7 @@ int		rpc_switch_client_transport(struct rpc_clnt *,
> 
> void		rpc_shutdown_client(struct rpc_clnt *);
> void		rpc_release_client(struct rpc_clnt *);
> -void		rpc_task_release_client(struct rpc_task *);
> +void		rpc_task_release_client(struct rpc_task *, int);
> 
> int		rpcb_create_local(struct net *);
> void		rpcb_put_local(struct net *);
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index cb49898..459f9b1 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -961,7 +961,7 @@ out:
> }
> EXPORT_SYMBOL_GPL(rpc_bind_new_program);
> 
> -void rpc_task_release_client(struct rpc_task *task)
> +void rpc_task_release_client(struct rpc_task *task, int rm_xprt)
> {
> 	struct rpc_clnt *clnt = task->tk_client;
> 	struct rpc_xprt *xprt = task->tk_xprt;
> @@ -976,9 +976,8 @@ void rpc_task_release_client(struct rpc_task *task)
> 		rpc_release_client(clnt);
> 	}
> 
> -	if (xprt != NULL) {
> +	if (rm_xprt && xprt) {
> 		task->tk_xprt = NULL;
> -
> 		xprt_put(xprt);
> 	}
> }
> @@ -988,7 +987,7 @@ void rpc_task_set_client(struct rpc_task *task, struct rpc_clnt *clnt)
> {
> 
> 	if (clnt != NULL) {
> -		rpc_task_release_client(task);
> +		rpc_task_release_client(task, 0);

Can we rather just kill this call to rpc_task_release_client)? I’m not aware of any remaining code paths that make it necessary. Am I missing something?

> 		if (task->tk_xprt == NULL)
> 			task->tk_xprt = xprt_iter_get_next(&clnt->cl_xpi);
> 		task->tk_client = clnt;
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index 9ae5885..575b254 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -1017,7 +1017,7 @@ static void rpc_release_resources_task(struct rpc_task *task)
> 		put_rpccred(task->tk_msg.rpc_cred);
> 		task->tk_msg.rpc_cred = NULL;
> 	}
> -	rpc_task_release_client(task);
> +	rpc_task_release_client(task, 1);
> }
> 
> static void rpc_final_put_task(struct rpc_task *task,
> -- 
> 1.8.3.1
> 

[PATCH Version 7 6/8] SUNRPC add an RPC null call to test session trunking connection

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 include/linux/sunrpc/clnt.h |  2 ++
 net/sunrpc/clnt.c           | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 99410bb..ebc83df 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -189,6 +189,8 @@ int 		rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 			struct rpc_xprt_switch *xps,
 			struct rpc_xprt *xprt,
 			void *dummy);
+int		rpc_clnt_test_xprt(struct rpc_clnt *clnt,
+			struct rpc_xprt *xprt);
 int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
 			int (*setup)(struct rpc_clnt *,
 				struct rpc_xprt_switch *,
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 459f9b1..822060f 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2614,6 +2614,24 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
 
+int rpc_clnt_test_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+{
+	struct rpc_cred *cred;
+	struct rpc_task *task;
+	int status;
+
+	cred = authnull_ops.lookup_cred(NULL, NULL, 0);
+	task = rpc_call_null_helper(clnt, xprt, cred,
+				RPC_TASK_SOFT | RPC_TASK_SOFTCONN, NULL, NULL);
+	put_rpccred(cred);
+	if (IS_ERR(task))
+		return PTR_ERR(task);
+	status = task->tk_status;
+	rpc_put_task(task);
+	return status;
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_test_xprt);
+
 /**
  * rpc_clnt_add_xprt - Add a new transport to a rpc_clnt
  * @clnt: pointer to struct rpc_clnt
-- 
1.8.3.1

Re: [PATCH Version 7 6/8] SUNRPC add an RPC null call to test session trunking connection

[Anna Schumaker]

Hi Andy,

On 07/27/2016 02:43 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  include/linux/sunrpc/clnt.h |  2 ++
>  net/sunrpc/clnt.c           | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 99410bb..ebc83df 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -189,6 +189,8 @@ int 		rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>  			struct rpc_xprt_switch *xps,
>  			struct rpc_xprt *xprt,
>  			void *dummy);
> +int		rpc_clnt_test_xprt(struct rpc_clnt *clnt,
> +			struct rpc_xprt *xprt);
>  int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
>  			int (*setup)(struct rpc_clnt *,
>  				struct rpc_xprt_switch *,
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 459f9b1..822060f 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2614,6 +2614,24 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
>  
> +int rpc_clnt_test_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)

Looks like rpc_clnt_test_and_add_xprt() runs the same testing code.  Can you swap the order of these functions in clnt.c and then have test_and_add_xprt() call test_xprt()?

Thanks,
Anna

> +{
> +	struct rpc_cred *cred;
> +	struct rpc_task *task;
> +	int status;
> +
> +	cred = authnull_ops.lookup_cred(NULL, NULL, 0);
> +	task = rpc_call_null_helper(clnt, xprt, cred,
> +				RPC_TASK_SOFT | RPC_TASK_SOFTCONN, NULL, NULL);
> +	put_rpccred(cred);
> +	if (IS_ERR(task))
> +		return PTR_ERR(task);
> +	status = task->tk_status;
> +	rpc_put_task(task);
> +	return status;
> +}
> +EXPORT_SYMBOL_GPL(rpc_clnt_test_xprt);
> +
>  /**
>   * rpc_clnt_add_xprt - Add a new transport to a rpc_clnt
>   * @clnt: pointer to struct rpc_clnt
> 

Re: [PATCH Version 7 6/8] SUNRPC add an RPC null call to test session trunking connection

[Adamson, Andy]

DQo+IE9uIEF1ZyA0LCAyMDE2LCBhdCAzOjI2IFBNLCBTY2h1bWFrZXIsIEFubmEgPEFubmEuU2No
dW1ha2VyQG5ldGFwcC5jb20+IHdyb3RlOg0KPiANCj4gSGkgQW5keSwNCj4gDQo+IE9uIDA3LzI3
LzIwMTYgMDI6NDMgUE0sIGFuZHJvc0BuZXRhcHAuY29tIHdyb3RlOg0KPj4gRnJvbTogQW5keSBB
ZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4+IA0KPj4gU2lnbmVkLW9mZi1ieTogQW5keSBB
ZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4+IC0tLQ0KPj4gaW5jbHVkZS9saW51eC9zdW5y
cGMvY2xudC5oIHwgIDIgKysNCj4+IG5ldC9zdW5ycGMvY2xudC5jICAgICAgICAgICB8IDE4ICsr
KysrKysrKysrKysrKysrKw0KPj4gMiBmaWxlcyBjaGFuZ2VkLCAyMCBpbnNlcnRpb25zKCspDQo+
PiANCj4+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9jbG50LmggYi9pbmNsdWRl
L2xpbnV4L3N1bnJwYy9jbG50LmgNCj4+IGluZGV4IDk5NDEwYmIuLmViYzgzZGYgMTAwNjQ0DQo+
PiAtLS0gYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9jbG50LmgNCj4+ICsrKyBiL2luY2x1ZGUvbGlu
dXgvc3VucnBjL2NsbnQuaA0KPj4gQEAgLTE4OSw2ICsxODksOCBAQCBpbnQgCQlycGNfY2xudF90
ZXN0X2FuZF9hZGRfeHBydChzdHJ1Y3QgcnBjX2NsbnQgKmNsbnQsDQo+PiAJCQlzdHJ1Y3QgcnBj
X3hwcnRfc3dpdGNoICp4cHMsDQo+PiAJCQlzdHJ1Y3QgcnBjX3hwcnQgKnhwcnQsDQo+PiAJCQl2
b2lkICpkdW1teSk7DQo+PiAraW50CQlycGNfY2xudF90ZXN0X3hwcnQoc3RydWN0IHJwY19jbG50
ICpjbG50LA0KPj4gKwkJCXN0cnVjdCBycGNfeHBydCAqeHBydCk7DQo+PiBpbnQJCXJwY19jbG50
X2FkZF94cHJ0KHN0cnVjdCBycGNfY2xudCAqLCBzdHJ1Y3QgeHBydF9jcmVhdGUgKiwNCj4+IAkJ
CWludCAoKnNldHVwKShzdHJ1Y3QgcnBjX2NsbnQgKiwNCj4+IAkJCQlzdHJ1Y3QgcnBjX3hwcnRf
c3dpdGNoICosDQo+PiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy9jbG50LmMgYi9uZXQvc3VucnBj
L2NsbnQuYw0KPj4gaW5kZXggNDU5ZjliMS4uODIyMDYwZiAxMDA2NDQNCj4+IC0tLSBhL25ldC9z
dW5ycGMvY2xudC5jDQo+PiArKysgYi9uZXQvc3VucnBjL2NsbnQuYw0KPj4gQEAgLTI2MTQsNiAr
MjYxNCwyNCBAQCBpbnQgcnBjX2NsbnRfdGVzdF9hbmRfYWRkX3hwcnQoc3RydWN0IHJwY19jbG50
ICpjbG50LA0KPj4gfQ0KPj4gRVhQT1JUX1NZTUJPTF9HUEwocnBjX2NsbnRfdGVzdF9hbmRfYWRk
X3hwcnQpOw0KPj4gDQo+PiAraW50IHJwY19jbG50X3Rlc3RfeHBydChzdHJ1Y3QgcnBjX2NsbnQg
KmNsbnQsIHN0cnVjdCBycGNfeHBydCAqeHBydCkNCj4gDQo+IExvb2tzIGxpa2UgcnBjX2NsbnRf
dGVzdF9hbmRfYWRkX3hwcnQoKSBydW5zIHRoZSBzYW1lIHRlc3RpbmcgY29kZS4gIENhbiB5b3Ug
c3dhcCB0aGUgb3JkZXIgb2YgdGhlc2UgZnVuY3Rpb25zIGluIGNsbnQuYyBhbmQgdGhlbiBoYXZl
IHRlc3RfYW5kX2FkZF94cHJ0KCkgY2FsbCB0ZXN0X3hwcnQoKT8NCg0KcnBjX2NsbnRfdGVzdF94
cHJ0IHVzZXMgYSBTWU5DIG51bGwgY2FsbCB3aGlsZSB0ZXN0X2FuZF9hZGRfeHBydCB1c2VzIGFu
IEFTWU5DIGNhbGwuIEZ1dGhlcm1vcmUsIHdoaWxlIGJvdGggdGVzdF94cHJ0IGFuZCB0ZXN0X2Fu
ZF9hZGRfeHBydCByZXR1cm4gYW4gZXJyb3IgaWYgdGhlIHJwY19jYWxsX251bGxfaGVscGVyIHRh
c2sgY2Fubm90IGJlIGFsbG9jYXRlZCAoRVJSX1BUUih0YXNrKSBpbiBycGNfbmV3X3Rhc2spLCB0
ZXN0X2FuZF9hZGRfeHBydCBpZ25vcmVzIHRoZSB0a19zdGF0dXMgYW5kIHJldHVybnMgMSB3aGls
ZSB0ZXN0X3hwcnQgcmV0dXJucyB0aGUgdGtfc3RhdHVzLg0KDQpGb3IgdGhlc2UgcmVhc29ucyBJ
IHRoaW5rIGl0IGlzIGNsZWFuZXIgYW5kIGVhc2llciB0byByZWFkIHRvIGtlZXAgdGhlbSBzZXBh
cmF0ZSBmdW5jdGlvbnMuDQoNCuKAlD5BbmR5DQo6DQo+IA0KPiBUaGFua3MsDQo+IEFubmENCj4g
DQo+PiArew0KPj4gKwlzdHJ1Y3QgcnBjX2NyZWQgKmNyZWQ7DQo+PiArCXN0cnVjdCBycGNfdGFz
ayAqdGFzazsNCj4+ICsJaW50IHN0YXR1czsNCj4+ICsNCj4+ICsJY3JlZCA9IGF1dGhudWxsX29w
cy5sb29rdXBfY3JlZChOVUxMLCBOVUxMLCAwKTsNCj4+ICsJdGFzayA9IHJwY19jYWxsX251bGxf
aGVscGVyKGNsbnQsIHhwcnQsIGNyZWQsDQo+PiArCQkJCVJQQ19UQVNLX1NPRlQgfCBSUENfVEFT
S19TT0ZUQ09OTiwgTlVMTCwgTlVMTCk7DQo+PiArCXB1dF9ycGNjcmVkKGNyZWQpOw0KPj4gKwlp
ZiAoSVNfRVJSKHRhc2spKQ0KPj4gKwkJcmV0dXJuIFBUUl9FUlIodGFzayk7DQo+PiArCXN0YXR1
cyA9IHRhc2stPnRrX3N0YXR1czsNCj4+ICsJcnBjX3B1dF90YXNrKHRhc2spOw0KPj4gKwlyZXR1
cm4gc3RhdHVzOw0KPj4gK30NCj4+ICtFWFBPUlRfU1lNQk9MX0dQTChycGNfY2xudF90ZXN0X3hw
cnQpOw0KPj4gKw0KPj4gLyoqDQo+PiAgKiBycGNfY2xudF9hZGRfeHBydCAtIEFkZCBhIG5ldyB0
cmFuc3BvcnQgdG8gYSBycGNfY2xudA0KPj4gICogQGNsbnQ6IHBvaW50ZXIgdG8gc3RydWN0IHJw
Y19jbG50DQoNCg==

Re: [PATCH Version 7 6/8] SUNRPC add an RPC null call to test session trunking connection

[Anna Schumaker]

On 08/05/2016 12:41 PM, Adamson, Andy wrote:
> 
>> On Aug 4, 2016, at 3:26 PM, Schumaker, Anna <Anna.Schumaker@netapp.com> wrote:
>>
>> Hi Andy,
>>
>> On 07/27/2016 02:43 PM, andros@netapp.com wrote:
>>> From: Andy Adamson <andros@netapp.com>
>>>
>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>> ---
>>> include/linux/sunrpc/clnt.h |  2 ++
>>> net/sunrpc/clnt.c           | 18 ++++++++++++++++++
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
>>> index 99410bb..ebc83df 100644
>>> --- a/include/linux/sunrpc/clnt.h
>>> +++ b/include/linux/sunrpc/clnt.h
>>> @@ -189,6 +189,8 @@ int 		rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>>> 			struct rpc_xprt_switch *xps,
>>> 			struct rpc_xprt *xprt,
>>> 			void *dummy);
>>> +int		rpc_clnt_test_xprt(struct rpc_clnt *clnt,
>>> +			struct rpc_xprt *xprt);
>>> int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
>>> 			int (*setup)(struct rpc_clnt *,
>>> 				struct rpc_xprt_switch *,
>>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>>> index 459f9b1..822060f 100644
>>> --- a/net/sunrpc/clnt.c
>>> +++ b/net/sunrpc/clnt.c
>>> @@ -2614,6 +2614,24 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>>> }
>>> EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
>>>
>>> +int rpc_clnt_test_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
>>
>> Looks like rpc_clnt_test_and_add_xprt() runs the same testing code.  Can you swap the order of these functions in clnt.c and then have test_and_add_xprt() call test_xprt()?
> 
> rpc_clnt_test_xprt uses a SYNC null call while test_and_add_xprt uses an ASYNC call. Futhermore, while both test_xprt and test_and_add_xprt return an error if the rpc_call_null_helper task cannot be allocated (ERR_PTR(task) in rpc_new_task), test_and_add_xprt ignores the tk_status and returns 1 while test_xprt returns the tk_status.

Ah, I missed that one is sync and the other isn't.  Thanks for pointing that out!  I still wish there was a common function they both could tall to handle the cred lookup, but maybe that's not as easy as it seems.

Anna

> 
> For these reasons I think it is cleaner and easier to read to keep them separate functions.
> 
> —>Andy
> :
>>
>> Thanks,
>> Anna
>>
>>> +{
>>> +	struct rpc_cred *cred;
>>> +	struct rpc_task *task;
>>> +	int status;
>>> +
>>> +	cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>>> +	task = rpc_call_null_helper(clnt, xprt, cred,
>>> +				RPC_TASK_SOFT | RPC_TASK_SOFTCONN, NULL, NULL);
>>> +	put_rpccred(cred);
>>> +	if (IS_ERR(task))
>>> +		return PTR_ERR(task);
>>> +	status = task->tk_status;
>>> +	rpc_put_task(task);
>>> +	return status;
>>> +}
>>> +EXPORT_SYMBOL_GPL(rpc_clnt_test_xprt);
>>> +
>>> /**
>>>  * rpc_clnt_add_xprt - Add a new transport to a rpc_clnt
>>>  * @clnt: pointer to struct rpc_clnt
> 

[PATCH Version 7 7/8] NFS test session trunking with exchange id

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4_fs.h           |  5 +++
 fs/nfs/nfs4proc.c          | 97 +++++++++++++++++++++++++++++++++++++++++++---
 net/sunrpc/xprtmultipath.c |  2 +
 3 files changed, 98 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index eb315e1..b2a94ca 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -203,6 +203,11 @@ struct nfs4_state_recovery_ops {
 		struct rpc_cred *);
 };
 
+struct nfs4_add_xprt_data {
+	struct nfs_client	*clp;
+	struct rpc_cred		*cred;
+};
+
 struct nfs4_state_maintenance_ops {
 	int (*sched_state_renewal)(struct nfs_client *, struct rpc_cred *, unsigned);
 	struct rpc_cred * (*get_state_renewal_cred_locked)(struct nfs_client *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e29f5ce..fa9ae75 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -44,6 +44,7 @@
 #include <linux/printk.h>
 #include <linux/slab.h>
 #include <linux/sunrpc/clnt.h>
+#include <linux/sunrpc/addr.h>
 #include <linux/nfs.h>
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
@@ -7050,10 +7051,16 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
 
 	return 0;
 }
+struct nfs41_test_xprt_data {
+	struct rpc_xprt *xprt;
+	struct rpc_xprt_switch *xps;
+};
 
 struct nfs41_exchange_id_data {
 	struct nfs41_exchange_id_res res;
 	struct nfs41_exchange_id_args args;
+	struct nfs41_test_xprt_data xdata;
+	int session_trunk;
 	int rpc_status;
 };
 
@@ -7068,6 +7075,10 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
 
 	if (status == 0)
 		status = nfs4_check_cl_exchange_flags(cdata->res.flags);
+
+	if (cdata->session_trunk)
+		goto session_trunk;
+
 	if (status  == 0)
 		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
 
@@ -7105,7 +7116,15 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
 			cdata->res.server_scope = NULL;
 		}
 	}
+out:
 	cdata->rpc_status = status;
+	return;
+
+session_trunk:
+	if (status == 0)
+		status = nfs4_detect_session_trunking(clp, &cdata->res,
+						      cdata->xdata.xprt);
+	goto out;
 }
 
 static void nfs4_exchange_id_release(void *data)
@@ -7114,6 +7133,10 @@ static void nfs4_exchange_id_release(void *data)
 					(struct nfs41_exchange_id_data *)data;
 
 	nfs_put_client(cdata->args.client);
+	if (cdata->session_trunk) {
+		xprt_put(cdata->xdata.xprt);
+		xprt_switch_put(cdata->xdata.xps);
+	}
 	kfree(cdata->res.impl_id);
 	kfree(cdata->res.server_scope);
 	kfree(cdata->res.server_owner);
@@ -7131,7 +7154,7 @@ static const struct rpc_call_ops nfs4_exchange_id_call_ops = {
  * Wrapper for EXCHANGE_ID operation.
  */
 static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
-	u32 sp4_how)
+			u32 sp4_how, struct nfs41_test_xprt_data *xdata)
 {
 	nfs4_verifier verifier;
 	struct rpc_message msg = {
@@ -7151,6 +7174,11 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	if (!atomic_inc_not_zero(&clp->cl_count))
 		goto out;
 
+	/* Don't test session trunking against the established mount rpc_xprt */
+	if (xdata &&
+	    xdata->xprt == rcu_access_pointer(clp->cl_rpcclient->cl_xprt))
+		return 1;
+
 	status = -ENOMEM;
 	calldata = kzalloc(sizeof(*calldata), GFP_NOFS);
 	if (!calldata)
@@ -7196,7 +7224,14 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		status = -EINVAL;
 		goto out_impl_id;
 	}
-
+	if (xdata) {
+		calldata->session_trunk = 1;
+		calldata->xdata.xprt = xdata->xprt;
+		calldata->xdata.xps = xdata->xps;
+		task_setup_data.rpc_xprt = xdata->xprt;
+		task_setup_data.flags =
+				RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC;
+	}
 	calldata->args.verifier = &verifier;
 	calldata->args.client = clp;
 #ifdef CONFIG_NFS_V4_1_MIGRATION
@@ -7217,9 +7252,13 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		goto out_impl_id;
 	}
 
-	status = rpc_wait_for_completion_task(task);
-	if (!status)
+	if (!xdata) {
+		status = rpc_wait_for_completion_task(task);
+		if (!status)
+			status = calldata->rpc_status;
+	} else	/* session trunking test */
 		status = calldata->rpc_status;
+
 	rpc_put_task(task);
 out:
 	if (clp->cl_implid != NULL)
@@ -7262,13 +7301,59 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 	/* try SP4_MACH_CRED if krb5i/p	*/
 	if (authflavor == RPC_AUTH_GSS_KRB5I ||
 	    authflavor == RPC_AUTH_GSS_KRB5P) {
-		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED);
+		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED, NULL);
 		if (!status)
 			return 0;
 	}
 
 	/* try SP4_NONE */
-	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE);
+	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE, NULL);
+}
+
+/**
+ * nfs4_test_session_trunk
+ * Test the connection with an rpc null ping.
+ * Test for session trunking with an asynchronous exchange_id call.
+ * Upon success, add a new transport
+ * to the rpc_clnt
+ *
+ * @clnt: struct rpc_clnt to get new transport
+ * @xps:  the rpc_xprt_switch to hold the new transport
+ * @xprt: the rpc_xprt to test
+ * @data: call data for _nfs4_proc_exchange_id.
+ */
+int nfs4_test_session_trunk(struct rpc_clnt *clnt, struct rpc_xprt_switch *xps,
+			    struct rpc_xprt *xprt, void *data)
+{
+	struct nfs4_add_xprt_data *adata = (struct nfs4_add_xprt_data *)data;
+	struct nfs41_test_xprt_data xdata = {
+		.xprt = xprt,
+		.xps = xps,
+	};
+	u32 sp4_how;
+	int status;
+
+	dprintk("--> %s try %s\n", __func__,
+		xprt->address_strings[RPC_DISPLAY_ADDR]);
+
+	xprt = xprt_get(xprt);
+	/* Test the connection */
+	status = rpc_clnt_test_xprt(clnt, xprt);
+	if (status < 0) {
+		dprintk(" %s rpc_clnt_test_xprt failed for %s\n", __func__,
+			xprt->address_strings[RPC_DISPLAY_ADDR]);
+		xprt_put(xprt);
+		goto out;
+	}
+	xps = xprt_switch_get(xps);
+
+	sp4_how = (adata->clp->cl_sp4_flags == 0 ? SP4_NONE : SP4_MACH_CRED);
+
+	/* Test connection for session trunking. Async exchange_id call */
+	status = _nfs4_proc_exchange_id(adata->clp, adata->cred, sp4_how,
+					&xdata);
+out:
+	return status;
 }
 
 static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 66c9d63..505d3b9 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -145,6 +145,7 @@ struct rpc_xprt_switch *xprt_switch_get(struct rpc_xprt_switch *xps)
 		return xps;
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(xprt_switch_get);
 
 /**
  * xprt_switch_put - Release a reference to a rpc_xprt_switch
@@ -157,6 +158,7 @@ void xprt_switch_put(struct rpc_xprt_switch *xps)
 	if (xps != NULL)
 		kref_put(&xps->xps_kref, xprt_switch_free);
 }
+EXPORT_SYMBOL_GPL(xprt_switch_put);
 
 /**
  * rpc_xprt_switch_set_roundrobin - Set a round-robin policy on rpc_xprt_switch
-- 
1.8.3.1

Re: [PATCH Version 7 7/8] NFS test session trunking with exchange id

[Trond Myklebust]

> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
> 
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> fs/nfs/nfs4_fs.h           |  5 +++
> fs/nfs/nfs4proc.c          | 97 +++++++++++++++++++++++++++++++++++++++++++---
> net/sunrpc/xprtmultipath.c |  2 +
> 3 files changed, 98 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index eb315e1..b2a94ca 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -203,6 +203,11 @@ struct nfs4_state_recovery_ops {
> 		struct rpc_cred *);
> };
> 
> +struct nfs4_add_xprt_data {
> +	struct nfs_client	*clp;
> +	struct rpc_cred		*cred;
> +};
> +
> struct nfs4_state_maintenance_ops {
> 	int (*sched_state_renewal)(struct nfs_client *, struct rpc_cred *, unsigned);
> 	struct rpc_cred * (*get_state_renewal_cred_locked)(struct nfs_client *);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index e29f5ce..fa9ae75 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -44,6 +44,7 @@
> #include <linux/printk.h>
> #include <linux/slab.h>
> #include <linux/sunrpc/clnt.h>
> +#include <linux/sunrpc/addr.h>
> #include <linux/nfs.h>
> #include <linux/nfs4.h>
> #include <linux/nfs_fs.h>
> @@ -7050,10 +7051,16 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
> 
> 	return 0;
> }
> +struct nfs41_test_xprt_data {
> +	struct rpc_xprt *xprt;
> +	struct rpc_xprt_switch *xps;
> +};
> 
> struct nfs41_exchange_id_data {
> 	struct nfs41_exchange_id_res res;
> 	struct nfs41_exchange_id_args args;
> +	struct nfs41_test_xprt_data xdata;
> +	int session_trunk;
> 	int rpc_status;
> };
> 
> @@ -7068,6 +7075,10 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
> 
> 	if (status == 0)
> 		status = nfs4_check_cl_exchange_flags(cdata->res.flags);
> +
> +	if (cdata->session_trunk)
> +		goto session_trunk;
> +
> 	if (status  == 0)
> 		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
> 
> @@ -7105,7 +7116,15 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
> 			cdata->res.server_scope = NULL;
> 		}
> 	}
> +out:
> 	cdata->rpc_status = status;
> +	return;
> +
> +session_trunk:
> +	if (status == 0)
> +		status = nfs4_detect_session_trunking(clp, &cdata->res,
> +						      cdata->xdata.xprt);
> +	goto out;
> }
> 
> static void nfs4_exchange_id_release(void *data)
> @@ -7114,6 +7133,10 @@ static void nfs4_exchange_id_release(void *data)
> 					(struct nfs41_exchange_id_data *)data;
> 
> 	nfs_put_client(cdata->args.client);
> +	if (cdata->session_trunk) {
> +		xprt_put(cdata->xdata.xprt);
> +		xprt_switch_put(cdata->xdata.xps);
> +	}
> 	kfree(cdata->res.impl_id);
> 	kfree(cdata->res.server_scope);
> 	kfree(cdata->res.server_owner);
> @@ -7131,7 +7154,7 @@ static const struct rpc_call_ops nfs4_exchange_id_call_ops = {
>  * Wrapper for EXCHANGE_ID operation.
>  */
> static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> -	u32 sp4_how)
> +			u32 sp4_how, struct nfs41_test_xprt_data *xdata)
> {
> 	nfs4_verifier verifier;
> 	struct rpc_message msg = {
> @@ -7151,6 +7174,11 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> 	if (!atomic_inc_not_zero(&clp->cl_count))
> 		goto out;
> 
> +	/* Don't test session trunking against the established mount rpc_xprt */
> +	if (xdata &&
> +	    xdata->xprt == rcu_access_pointer(clp->cl_rpcclient->cl_xprt))
> +		return 1;
> +
> 	status = -ENOMEM;
> 	calldata = kzalloc(sizeof(*calldata), GFP_NOFS);
> 	if (!calldata)
> @@ -7196,7 +7224,14 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> 		status = -EINVAL;
> 		goto out_impl_id;
> 	}
> -
> +	if (xdata) {
> +		calldata->session_trunk = 1;
> +		calldata->xdata.xprt = xdata->xprt;
> +		calldata->xdata.xps = xdata->xps;
> +		task_setup_data.rpc_xprt = xdata->xprt;
> +		task_setup_data.flags =
> +				RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC;
> +	}
> 	calldata->args.verifier = &verifier;
> 	calldata->args.client = clp;
> #ifdef CONFIG_NFS_V4_1_MIGRATION
> @@ -7217,9 +7252,13 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> 		goto out_impl_id;
> 	}
> 
> -	status = rpc_wait_for_completion_task(task);
> -	if (!status)
> +	if (!xdata) {
> +		status = rpc_wait_for_completion_task(task);
> +		if (!status)
> +			status = calldata->rpc_status;
> +	} else	/* session trunking test */
> 		status = calldata->rpc_status;
> +
> 	rpc_put_task(task);
> out:
> 	if (clp->cl_implid != NULL)
> @@ -7262,13 +7301,59 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
> 	/* try SP4_MACH_CRED if krb5i/p	*/
> 	if (authflavor == RPC_AUTH_GSS_KRB5I ||
> 	    authflavor == RPC_AUTH_GSS_KRB5P) {
> -		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED);
> +		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED, NULL);
> 		if (!status)
> 			return 0;
> 	}
> 
> 	/* try SP4_NONE */
> -	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE);
> +	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE, NULL);
> +}
> +
> +/**
> + * nfs4_test_session_trunk
> + * Test the connection with an rpc null ping.
> + * Test for session trunking with an asynchronous exchange_id call.
> + * Upon success, add a new transport
> + * to the rpc_clnt
> + *
> + * @clnt: struct rpc_clnt to get new transport
> + * @xps:  the rpc_xprt_switch to hold the new transport

I’d strongly prefer that we don’t let struct rpc_xprt_switch leak out of the RPC layer.
Can't we replace this with struct rpc_clnt?

> + * @xprt: the rpc_xprt to test
> + * @data: call data for _nfs4_proc_exchange_id.
> + */
> +int nfs4_test_session_trunk(struct rpc_clnt *clnt, struct rpc_xprt_switch *xps,
> +			    struct rpc_xprt *xprt, void *data)
> +{
> +	struct nfs4_add_xprt_data *adata = (struct nfs4_add_xprt_data *)data;
> +	struct nfs41_test_xprt_data xdata = {
> +		.xprt = xprt,
> +		.xps = xps,
> +	};
> +	u32 sp4_how;
> +	int status;
> +
> +	dprintk("--> %s try %s\n", __func__,
> +		xprt->address_strings[RPC_DISPLAY_ADDR]);
> +
> +	xprt = xprt_get(xprt);
> +	/* Test the connection */
> +	status = rpc_clnt_test_xprt(clnt, xprt);
> +	if (status < 0) {
> +		dprintk(" %s rpc_clnt_test_xprt failed for %s\n", __func__,
> +			xprt->address_strings[RPC_DISPLAY_ADDR]);
> +		xprt_put(xprt);
> +		goto out;
> +	}
> +	xps = xprt_switch_get(xps);
> +
> +	sp4_how = (adata->clp->cl_sp4_flags == 0 ? SP4_NONE : SP4_MACH_CRED);
> +
> +	/* Test connection for session trunking. Async exchange_id call */
> +	status = _nfs4_proc_exchange_id(adata->clp, adata->cred, sp4_how,
> +					&xdata);
> +out:
> +	return status;
> }
> 
> static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
> diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
> index 66c9d63..505d3b9 100644
> --- a/net/sunrpc/xprtmultipath.c
> +++ b/net/sunrpc/xprtmultipath.c
> @@ -145,6 +145,7 @@ struct rpc_xprt_switch *xprt_switch_get(struct rpc_xprt_switch *xps)
> 		return xps;
> 	return NULL;
> }
> +EXPORT_SYMBOL_GPL(xprt_switch_get);
> 
> /**
>  * xprt_switch_put - Release a reference to a rpc_xprt_switch
> @@ -157,6 +158,7 @@ void xprt_switch_put(struct rpc_xprt_switch *xps)
> 	if (xps != NULL)
> 		kref_put(&xps->xps_kref, xprt_switch_free);
> }
> +EXPORT_SYMBOL_GPL(xprt_switch_put);
> 
> /**
>  * rpc_xprt_switch_set_roundrobin - Set a round-robin policy on rpc_xprt_switch
> -- 
> 1.8.3.1
> 

Re: [PATCH Version 7 7/8] NFS test session trunking with exchange id

[Adamson, Andy]

> On Aug 5, 2016, at 1:34 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> 
>> 
>> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
>> 
>> From: Andy Adamson <andros@netapp.com>
>> 
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfs/nfs4_fs.h           |  5 +++
>> fs/nfs/nfs4proc.c          | 97 +++++++++++++++++++++++++++++++++++++++++++---
>> net/sunrpc/xprtmultipath.c |  2 +
>> 3 files changed, 98 insertions(+), 6 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index eb315e1..b2a94ca 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -203,6 +203,11 @@ struct nfs4_state_recovery_ops {
>> 		struct rpc_cred *);
>> };
>> 
>> +struct nfs4_add_xprt_data {
>> +	struct nfs_client	*clp;
>> +	struct rpc_cred		*cred;
>> +};
>> +
>> struct nfs4_state_maintenance_ops {
>> 	int (*sched_state_renewal)(struct nfs_client *, struct rpc_cred *, unsigned);
>> 	struct rpc_cred * (*get_state_renewal_cred_locked)(struct nfs_client *);
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index e29f5ce..fa9ae75 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -44,6 +44,7 @@
>> #include <linux/printk.h>
>> #include <linux/slab.h>
>> #include <linux/sunrpc/clnt.h>
>> +#include <linux/sunrpc/addr.h>
>> #include <linux/nfs.h>
>> #include <linux/nfs4.h>
>> #include <linux/nfs_fs.h>
>> @@ -7050,10 +7051,16 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
>> 
>> 	return 0;
>> }
>> +struct nfs41_test_xprt_data {
>> +	struct rpc_xprt *xprt;
>> +	struct rpc_xprt_switch *xps;
>> +};
>> 
>> struct nfs41_exchange_id_data {
>> 	struct nfs41_exchange_id_res res;
>> 	struct nfs41_exchange_id_args args;
>> +	struct nfs41_test_xprt_data xdata;
>> +	int session_trunk;
>> 	int rpc_status;
>> };
>> 
>> @@ -7068,6 +7075,10 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
>> 
>> 	if (status == 0)
>> 		status = nfs4_check_cl_exchange_flags(cdata->res.flags);
>> +
>> +	if (cdata->session_trunk)
>> +		goto session_trunk;
>> +
>> 	if (status  == 0)
>> 		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
>> 
>> @@ -7105,7 +7116,15 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
>> 			cdata->res.server_scope = NULL;
>> 		}
>> 	}
>> +out:
>> 	cdata->rpc_status = status;
>> +	return;
>> +
>> +session_trunk:
>> +	if (status == 0)
>> +		status = nfs4_detect_session_trunking(clp, &cdata->res,
>> +						      cdata->xdata.xprt);
>> +	goto out;
>> }
>> 
>> static void nfs4_exchange_id_release(void *data)
>> @@ -7114,6 +7133,10 @@ static void nfs4_exchange_id_release(void *data)
>> 					(struct nfs41_exchange_id_data *)data;
>> 
>> 	nfs_put_client(cdata->args.client);
>> +	if (cdata->session_trunk) {
>> +		xprt_put(cdata->xdata.xprt);
>> +		xprt_switch_put(cdata->xdata.xps);
>> +	}
>> 	kfree(cdata->res.impl_id);
>> 	kfree(cdata->res.server_scope);
>> 	kfree(cdata->res.server_owner);
>> @@ -7131,7 +7154,7 @@ static const struct rpc_call_ops nfs4_exchange_id_call_ops = {
>> * Wrapper for EXCHANGE_ID operation.
>> */
>> static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>> -	u32 sp4_how)
>> +			u32 sp4_how, struct nfs41_test_xprt_data *xdata)
>> {
>> 	nfs4_verifier verifier;
>> 	struct rpc_message msg = {
>> @@ -7151,6 +7174,11 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>> 	if (!atomic_inc_not_zero(&clp->cl_count))
>> 		goto out;
>> 
>> +	/* Don't test session trunking against the established mount rpc_xprt */
>> +	if (xdata &&
>> +	    xdata->xprt == rcu_access_pointer(clp->cl_rpcclient->cl_xprt))
>> +		return 1;
>> +
>> 	status = -ENOMEM;
>> 	calldata = kzalloc(sizeof(*calldata), GFP_NOFS);
>> 	if (!calldata)
>> @@ -7196,7 +7224,14 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>> 		status = -EINVAL;
>> 		goto out_impl_id;
>> 	}
>> -
>> +	if (xdata) {
>> +		calldata->session_trunk = 1;
>> +		calldata->xdata.xprt = xdata->xprt;
>> +		calldata->xdata.xps = xdata->xps;
>> +		task_setup_data.rpc_xprt = xdata->xprt;
>> +		task_setup_data.flags =
>> +				RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC;
>> +	}
>> 	calldata->args.verifier = &verifier;
>> 	calldata->args.client = clp;
>> #ifdef CONFIG_NFS_V4_1_MIGRATION
>> @@ -7217,9 +7252,13 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>> 		goto out_impl_id;
>> 	}
>> 
>> -	status = rpc_wait_for_completion_task(task);
>> -	if (!status)
>> +	if (!xdata) {
>> +		status = rpc_wait_for_completion_task(task);
>> +		if (!status)
>> +			status = calldata->rpc_status;
>> +	} else	/* session trunking test */
>> 		status = calldata->rpc_status;
>> +
>> 	rpc_put_task(task);
>> out:
>> 	if (clp->cl_implid != NULL)
>> @@ -7262,13 +7301,59 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>> 	/* try SP4_MACH_CRED if krb5i/p	*/
>> 	if (authflavor == RPC_AUTH_GSS_KRB5I ||
>> 	    authflavor == RPC_AUTH_GSS_KRB5P) {
>> -		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED);
>> +		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED, NULL);
>> 		if (!status)
>> 			return 0;
>> 	}
>> 
>> 	/* try SP4_NONE */
>> -	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE);
>> +	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE, NULL);
>> +}
>> +
>> +/**
>> + * nfs4_test_session_trunk
>> + * Test the connection with an rpc null ping.
>> + * Test for session trunking with an asynchronous exchange_id call.
>> + * Upon success, add a new transport
>> + * to the rpc_clnt
>> + *
>> + * @clnt: struct rpc_clnt to get new transport
>> + * @xps:  the rpc_xprt_switch to hold the new transport
> 
> I’d strongly prefer that we don’t let struct rpc_xprt_switch leak out of the RPC layer.
> Can't we replace this with struct rpc_clnt?


Yes - you’re right. The rpc_clnt does hold the rpc_xprt_switch. I should be able to do that.

—>Andy

> 
>> + * @xprt: the rpc_xprt to test
>> + * @data: call data for _nfs4_proc_exchange_id.
>> + */
>> +int nfs4_test_session_trunk(struct rpc_clnt *clnt, struct rpc_xprt_switch *xps,
>> +			    struct rpc_xprt *xprt, void *data)
>> +{
>> +	struct nfs4_add_xprt_data *adata = (struct nfs4_add_xprt_data *)data;
>> +	struct nfs41_test_xprt_data xdata = {
>> +		.xprt = xprt,
>> +		.xps = xps,
>> +	};
>> +	u32 sp4_how;
>> +	int status;
>> +
>> +	dprintk("--> %s try %s\n", __func__,
>> +		xprt->address_strings[RPC_DISPLAY_ADDR]);
>> +
>> +	xprt = xprt_get(xprt);
>> +	/* Test the connection */
>> +	status = rpc_clnt_test_xprt(clnt, xprt);
>> +	if (status < 0) {
>> +		dprintk(" %s rpc_clnt_test_xprt failed for %s\n", __func__,
>> +			xprt->address_strings[RPC_DISPLAY_ADDR]);
>> +		xprt_put(xprt);
>> +		goto out;
>> +	}
>> +	xps = xprt_switch_get(xps);
>> +
>> +	sp4_how = (adata->clp->cl_sp4_flags == 0 ? SP4_NONE : SP4_MACH_CRED);
>> +
>> +	/* Test connection for session trunking. Async exchange_id call */
>> +	status = _nfs4_proc_exchange_id(adata->clp, adata->cred, sp4_how,
>> +					&xdata);
>> +out:
>> +	return status;
>> }
>> 
>> static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
>> diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
>> index 66c9d63..505d3b9 100644
>> --- a/net/sunrpc/xprtmultipath.c
>> +++ b/net/sunrpc/xprtmultipath.c
>> @@ -145,6 +145,7 @@ struct rpc_xprt_switch *xprt_switch_get(struct rpc_xprt_switch *xps)
>> 		return xps;
>> 	return NULL;
>> }
>> +EXPORT_SYMBOL_GPL(xprt_switch_get);
>> 
>> /**
>> * xprt_switch_put - Release a reference to a rpc_xprt_switch
>> @@ -157,6 +158,7 @@ void xprt_switch_put(struct rpc_xprt_switch *xps)
>> 	if (xps != NULL)
>> 		kref_put(&xps->xps_kref, xprt_switch_free);
>> }
>> +EXPORT_SYMBOL_GPL(xprt_switch_put);
>> 
>> /**
>> * rpc_xprt_switch_set_roundrobin - Set a round-robin policy on rpc_xprt_switch
>> -- 
>> 1.8.3.1

[PATCH Version 7 8/8] NFS pnfs data server multipath session trunking

[andros]

From: Andy Adamson <andros@netapp.com>

Try all multipath addresses for a data server. The first address that
successfully connects and creates a session is the DS mount address.
All subsequent addresses are tested for session trunking and
added as aliases.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4_fs.h  |  2 ++
 fs/nfs/nfs4proc.c |  2 ++
 fs/nfs/pnfs_nfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b2a94ca..8e4343f 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -59,6 +59,8 @@ struct nfs4_minor_version_ops {
 			struct nfs4_lock_state *);
 	struct nfs_seqid *
 		(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
+	int	(*session_trunk)(struct rpc_clnt *, struct rpc_xprt_switch *,
+			struct rpc_xprt *, void *);
 	const struct rpc_call_ops *call_sync_ops;
 	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
 	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index fa9ae75..8767720 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8934,6 +8934,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
 	.find_root_sec = nfs41_find_root_sec,
 	.free_lock_state = nfs41_free_lock_state,
 	.alloc_seqid = nfs_alloc_no_seqid,
+	.session_trunk = nfs4_test_session_trunk,
 	.call_sync_ops = &nfs41_call_sync_ops,
 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
@@ -8963,6 +8964,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
 	.free_lock_state = nfs41_free_lock_state,
 	.call_sync_ops = &nfs41_call_sync_ops,
 	.alloc_seqid = nfs_alloc_no_seqid,
+	.session_trunk = nfs4_test_session_trunk,
 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
 	.state_renewal_ops = &nfs41_state_renewal_ops,
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index f3468b5..7cc1c4d 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -690,13 +690,44 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 		dprintk("%s: DS %s: trying address %s\n",
 			__func__, ds->ds_remotestr, da->da_remotestr);
 
-		clp = nfs4_set_ds_client(mds_srv,
-					(struct sockaddr *)&da->da_addr,
-					da->da_addrlen, IPPROTO_TCP,
-					timeo, retrans, minor_version,
-					au_flavor);
-		if (!IS_ERR(clp))
-			break;
+		if (!IS_ERR(clp) && clp->cl_mvops->session_trunk) {
+			struct xprt_create xprt_args = {
+				.ident = XPRT_TRANSPORT_TCP,
+				.net = clp->cl_net,
+				.dstaddr = (struct sockaddr *)&da->da_addr,
+				.addrlen = da->da_addrlen,
+				.servername = clp->cl_hostname,
+			};
+			struct nfs4_add_xprt_data xprtdata = {
+				.clp = clp,
+			};
+			xprtdata.cred = nfs4_get_clid_cred(clp);
+
+			/**
+			* Test this address for session trunking and
+			* add as an alias
+			*/
+			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
+					  clp->cl_mvops->session_trunk,
+					  &xprtdata);
+		} else {
+			clp = nfs4_set_ds_client(mds_srv,
+						(struct sockaddr *)&da->da_addr,
+						da->da_addrlen, IPPROTO_TCP,
+						timeo, retrans, minor_version,
+						au_flavor);
+			if (IS_ERR(clp))
+				continue;
+
+			status = nfs4_init_ds_session(clp,
+					mds_srv->nfs_client->cl_lease_time);
+			if (status) {
+				nfs_put_client(clp);
+				clp = ERR_PTR(-EIO);
+				continue;
+			}
+
+		}
 	}
 
 	if (IS_ERR(clp)) {
@@ -704,18 +735,11 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 		goto out;
 	}
 
-	status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
-	if (status)
-		goto out_put;
-
 	smp_wmb();
 	ds->ds_clp = clp;
 	dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
 out:
 	return status;
-out_put:
-	nfs_put_client(clp);
-	goto out;
 }
 
 /*
-- 
1.8.3.1

Re: [PATCH Version 7 8/8] NFS pnfs data server multipath session trunking

[Trond Myklebust]

> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
> 
> From: Andy Adamson <andros@netapp.com>
> 
> Try all multipath addresses for a data server. The first address that
> successfully connects and creates a session is the DS mount address.
> All subsequent addresses are tested for session trunking and
> added as aliases.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> fs/nfs/nfs4_fs.h  |  2 ++
> fs/nfs/nfs4proc.c |  2 ++
> fs/nfs/pnfs_nfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
> 3 files changed, 42 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index b2a94ca..8e4343f 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -59,6 +59,8 @@ struct nfs4_minor_version_ops {
> 			struct nfs4_lock_state *);
> 	struct nfs_seqid *
> 		(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
> +	int	(*session_trunk)(struct rpc_clnt *, struct rpc_xprt_switch *,
> +			struct rpc_xprt *, void *);
> 	const struct rpc_call_ops *call_sync_ops;
> 	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
> 	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index fa9ae75..8767720 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8934,6 +8934,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
> 	.find_root_sec = nfs41_find_root_sec,
> 	.free_lock_state = nfs41_free_lock_state,
> 	.alloc_seqid = nfs_alloc_no_seqid,
> +	.session_trunk = nfs4_test_session_trunk,
> 	.call_sync_ops = &nfs41_call_sync_ops,
> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
> @@ -8963,6 +8964,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
> 	.free_lock_state = nfs41_free_lock_state,
> 	.call_sync_ops = &nfs41_call_sync_ops,
> 	.alloc_seqid = nfs_alloc_no_seqid,
> +	.session_trunk = nfs4_test_session_trunk,
> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
> 	.state_renewal_ops = &nfs41_state_renewal_ops,
> diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
> index f3468b5..7cc1c4d 100644
> --- a/fs/nfs/pnfs_nfs.c
> +++ b/fs/nfs/pnfs_nfs.c
> @@ -690,13 +690,44 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
> 		dprintk("%s: DS %s: trying address %s\n",
> 			__func__, ds->ds_remotestr, da->da_remotestr);
> 
> -		clp = nfs4_set_ds_client(mds_srv,
> -					(struct sockaddr *)&da->da_addr,
> -					da->da_addrlen, IPPROTO_TCP,
> -					timeo, retrans, minor_version,
> -					au_flavor);
> -		if (!IS_ERR(clp))
> -			break;
> +		if (!IS_ERR(clp) && clp->cl_mvops->session_trunk) {
> +			struct xprt_create xprt_args = {
> +				.ident = XPRT_TRANSPORT_TCP,
> +				.net = clp->cl_net,
> +				.dstaddr = (struct sockaddr *)&da->da_addr,
> +				.addrlen = da->da_addrlen,
> +				.servername = clp->cl_hostname,
> +			};
> +			struct nfs4_add_xprt_data xprtdata = {
> +				.clp = clp,
> +			};
> +			xprtdata.cred = nfs4_get_clid_cred(clp);
> +
> +			/**
> +			* Test this address for session trunking and
> +			* add as an alias
> +			*/
> +			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
> +					  clp->cl_mvops->session_trunk,
> +					  &xprtdata);
> +		} else {
> +			clp = nfs4_set_ds_client(mds_srv,
> +						(struct sockaddr *)&da->da_addr,
> +						da->da_addrlen, IPPROTO_TCP,
> +						timeo, retrans, minor_version,
> +						au_flavor);
> +			if (IS_ERR(clp))
> +				continue;
> +
> +			status = nfs4_init_ds_session(clp,
> +					mds_srv->nfs_client->cl_lease_time);
> +			if (status) {
> +				nfs_put_client(clp);
> +				clp = ERR_PTR(-EIO);
> +				continue;
> +			}
> +
> +		}
> 	}
> 
> 	if (IS_ERR(clp)) {
> @@ -704,18 +735,11 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
> 		goto out;
> 	}
> 
> -	status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
> -	if (status)
> -		goto out_put;
> -
> 	smp_wmb();
> 	ds->ds_clp = clp;
> 	dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
> out:
> 	return status;
> -out_put:
> -	nfs_put_client(clp);
> -	goto out;
> }

Is there something which checks whether or not a particular transport has already been added?
Also, doesn’t something need to change in nfs_match_client() so that we can find the struct nfs_client using any of the IP addresses that are registered to it?

Re: [PATCH Version 7 8/8] NFS pnfs data server multipath session trunking

[Adamson, Andy]

> On Aug 5, 2016, at 6:05 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
> 
> 
>> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
>> 
>> From: Andy Adamson <andros@netapp.com>
>> 
>> Try all multipath addresses for a data server. The first address that
>> successfully connects and creates a session is the DS mount address.
>> All subsequent addresses are tested for session trunking and
>> added as aliases.
>> 
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfs/nfs4_fs.h  |  2 ++
>> fs/nfs/nfs4proc.c |  2 ++
>> fs/nfs/pnfs_nfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
>> 3 files changed, 42 insertions(+), 14 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index b2a94ca..8e4343f 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -59,6 +59,8 @@ struct nfs4_minor_version_ops {
>> 			struct nfs4_lock_state *);
>> 	struct nfs_seqid *
>> 		(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
>> +	int	(*session_trunk)(struct rpc_clnt *, struct rpc_xprt_switch *,
>> +			struct rpc_xprt *, void *);
>> 	const struct rpc_call_ops *call_sync_ops;
>> 	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
>> 	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index fa9ae75..8767720 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -8934,6 +8934,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
>> 	.find_root_sec = nfs41_find_root_sec,
>> 	.free_lock_state = nfs41_free_lock_state,
>> 	.alloc_seqid = nfs_alloc_no_seqid,
>> +	.session_trunk = nfs4_test_session_trunk,
>> 	.call_sync_ops = &nfs41_call_sync_ops,
>> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
>> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
>> @@ -8963,6 +8964,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
>> 	.free_lock_state = nfs41_free_lock_state,
>> 	.call_sync_ops = &nfs41_call_sync_ops,
>> 	.alloc_seqid = nfs_alloc_no_seqid,
>> +	.session_trunk = nfs4_test_session_trunk,
>> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
>> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
>> 	.state_renewal_ops = &nfs41_state_renewal_ops,
>> diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
>> index f3468b5..7cc1c4d 100644
>> --- a/fs/nfs/pnfs_nfs.c
>> +++ b/fs/nfs/pnfs_nfs.c
>> @@ -690,13 +690,44 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
>> 		dprintk("%s: DS %s: trying address %s\n",
>> 			__func__, ds->ds_remotestr, da->da_remotestr);
>> 
>> -		clp = nfs4_set_ds_client(mds_srv,
>> -					(struct sockaddr *)&da->da_addr,
>> -					da->da_addrlen, IPPROTO_TCP,
>> -					timeo, retrans, minor_version,
>> -					au_flavor);
>> -		if (!IS_ERR(clp))
>> -			break;
>> +		if (!IS_ERR(clp) && clp->cl_mvops->session_trunk) {
>> +			struct xprt_create xprt_args = {
>> +				.ident = XPRT_TRANSPORT_TCP,
>> +				.net = clp->cl_net,
>> +				.dstaddr = (struct sockaddr *)&da->da_addr,
>> +				.addrlen = da->da_addrlen,
>> +				.servername = clp->cl_hostname,
>> +			};
>> +			struct nfs4_add_xprt_data xprtdata = {
>> +				.clp = clp,
>> +			};
>> +			xprtdata.cred = nfs4_get_clid_cred(clp);
>> +
>> +			/**
>> +			* Test this address for session trunking and
>> +			* add as an alias
>> +			*/
>> +			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
>> +					  clp->cl_mvops->session_trunk,
>> +					  &xprtdata);
>> +		} else {
>> +			clp = nfs4_set_ds_client(mds_srv,
>> +						(struct sockaddr *)&da->da_addr,
>> +						da->da_addrlen, IPPROTO_TCP,
>> +						timeo, retrans, minor_version,
>> +						au_flavor);
>> +			if (IS_ERR(clp))
>> +				continue;
>> +
>> +			status = nfs4_init_ds_session(clp,
>> +					mds_srv->nfs_client->cl_lease_time);
>> +			if (status) {
>> +				nfs_put_client(clp);
>> +				clp = ERR_PTR(-EIO);
>> +				continue;
>> +			}
>> +
>> +		}
>> 	}
>> 
>> 	if (IS_ERR(clp)) {
>> @@ -704,18 +735,11 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
>> 		goto out;
>> 	}
>> 
>> -	status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
>> -	if (status)
>> -		goto out_put;
>> -
>> 	smp_wmb();
>> 	ds->ds_clp = clp;
>> 	dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
>> out:
>> 	return status;
>> -out_put:
>> -	nfs_put_client(clp);
>> -	goto out;
>> }
> 
> Is there something which checks whether or not a particular transport has already been added?

Is there a reason that rpc_switch_xprt_add_xprt() doesn’t check to see if the xprt has already been added? IOW should I add the check to rpc_switch_xprt_add_xprt() or have a new interface?


> Also, doesn’t something need to change in nfs_match_client() so that we can find the struct nfs_client using any of the IP addresses that are registered to it?

OK

—>Andy
 
> 
> 

Re: [PATCH Version 7 8/8] NFS pnfs data server multipath session trunking

[Trond Myklebust]

> On Aug 8, 2016, at 15:57, Adamson, Andy <William.Adamson@netapp.com> wrote:
> 
>> 
>> On Aug 5, 2016, at 6:05 PM, Trond Myklebust <trondmy@primarydata.com> wrote:
>> 
>> 
>>> On Jul 27, 2016, at 14:43, andros@netapp.com wrote:
>>> 
>>> From: Andy Adamson <andros@netapp.com>
>>> 
>>> Try all multipath addresses for a data server. The first address that
>>> successfully connects and creates a session is the DS mount address.
>>> All subsequent addresses are tested for session trunking and
>>> added as aliases.
>>> 
>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>> ---
>>> fs/nfs/nfs4_fs.h  |  2 ++
>>> fs/nfs/nfs4proc.c |  2 ++
>>> fs/nfs/pnfs_nfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
>>> 3 files changed, 42 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>>> index b2a94ca..8e4343f 100644
>>> --- a/fs/nfs/nfs4_fs.h
>>> +++ b/fs/nfs/nfs4_fs.h
>>> @@ -59,6 +59,8 @@ struct nfs4_minor_version_ops {
>>> 			struct nfs4_lock_state *);
>>> 	struct nfs_seqid *
>>> 		(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
>>> +	int	(*session_trunk)(struct rpc_clnt *, struct rpc_xprt_switch *,
>>> +			struct rpc_xprt *, void *);
>>> 	const struct rpc_call_ops *call_sync_ops;
>>> 	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
>>> 	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index fa9ae75..8767720 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -8934,6 +8934,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
>>> 	.find_root_sec = nfs41_find_root_sec,
>>> 	.free_lock_state = nfs41_free_lock_state,
>>> 	.alloc_seqid = nfs_alloc_no_seqid,
>>> +	.session_trunk = nfs4_test_session_trunk,
>>> 	.call_sync_ops = &nfs41_call_sync_ops,
>>> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
>>> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
>>> @@ -8963,6 +8964,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
>>> 	.free_lock_state = nfs41_free_lock_state,
>>> 	.call_sync_ops = &nfs41_call_sync_ops,
>>> 	.alloc_seqid = nfs_alloc_no_seqid,
>>> +	.session_trunk = nfs4_test_session_trunk,
>>> 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
>>> 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
>>> 	.state_renewal_ops = &nfs41_state_renewal_ops,
>>> diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
>>> index f3468b5..7cc1c4d 100644
>>> --- a/fs/nfs/pnfs_nfs.c
>>> +++ b/fs/nfs/pnfs_nfs.c
>>> @@ -690,13 +690,44 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
>>> 		dprintk("%s: DS %s: trying address %s\n",
>>> 			__func__, ds->ds_remotestr, da->da_remotestr);
>>> 
>>> -		clp = nfs4_set_ds_client(mds_srv,
>>> -					(struct sockaddr *)&da->da_addr,
>>> -					da->da_addrlen, IPPROTO_TCP,
>>> -					timeo, retrans, minor_version,
>>> -					au_flavor);
>>> -		if (!IS_ERR(clp))
>>> -			break;
>>> +		if (!IS_ERR(clp) && clp->cl_mvops->session_trunk) {
>>> +			struct xprt_create xprt_args = {
>>> +				.ident = XPRT_TRANSPORT_TCP,
>>> +				.net = clp->cl_net,
>>> +				.dstaddr = (struct sockaddr *)&da->da_addr,
>>> +				.addrlen = da->da_addrlen,
>>> +				.servername = clp->cl_hostname,
>>> +			};
>>> +			struct nfs4_add_xprt_data xprtdata = {
>>> +				.clp = clp,
>>> +			};
>>> +			xprtdata.cred = nfs4_get_clid_cred(clp);
>>> +
>>> +			/**
>>> +			* Test this address for session trunking and
>>> +			* add as an alias
>>> +			*/
>>> +			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
>>> +					  clp->cl_mvops->session_trunk,
>>> +					  &xprtdata);
>>> +		} else {
>>> +			clp = nfs4_set_ds_client(mds_srv,
>>> +						(struct sockaddr *)&da->da_addr,
>>> +						da->da_addrlen, IPPROTO_TCP,
>>> +						timeo, retrans, minor_version,
>>> +						au_flavor);
>>> +			if (IS_ERR(clp))
>>> +				continue;
>>> +
>>> +			status = nfs4_init_ds_session(clp,
>>> +					mds_srv->nfs_client->cl_lease_time);
>>> +			if (status) {
>>> +				nfs_put_client(clp);
>>> +				clp = ERR_PTR(-EIO);
>>> +				continue;
>>> +			}
>>> +
>>> +		}
>>> 	}
>>> 
>>> 	if (IS_ERR(clp)) {
>>> @@ -704,18 +735,11 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
>>> 		goto out;
>>> 	}
>>> 
>>> -	status = nfs4_init_ds_session(clp, mds_srv->nfs_client->cl_lease_time);
>>> -	if (status)
>>> -		goto out_put;
>>> -
>>> 	smp_wmb();
>>> 	ds->ds_clp = clp;
>>> 	dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
>>> out:
>>> 	return status;
>>> -out_put:
>>> -	nfs_put_client(clp);
>>> -	goto out;
>>> }
>> 
>> Is there something which checks whether or not a particular transport has already been added?
> 
> Is there a reason that rpc_switch_xprt_add_xprt() doesn’t check to see if the xprt has already been added? IOW should I add the check to rpc_switch_xprt_add_xprt() or have a new interface?
> 

Adding it to rpc_switch_xprt_add_xprt() should be OK.

> 
>> Also, doesn’t something need to change in nfs_match_client() so that we can find the struct nfs_client using any of the IP addresses that are registered to it?
> 
> OK
> 
> —>Andy