[PATCH Version 8 00/12] pNFS file layout session trunking

[PATCH Version 8 00/12] pNFS file layout session trunking

[andros]

From: Andy Adamson <andros@netapp.com>

Responded to version-7 comments:

- Removed call to rpc_task_release_client from rpc_task_set_client

- Don't leak the rpc_xprt_switch to the NFS layer
  Added wrappers in net/sunrpc/clnt.c  including a new
  rpc_clnt_add_xprt setup function to call into the NFS
  layer.

- Check if a transport has already been added to the xprt switch
  This is done in rpc_xprt_switch_add_xprt. Also done in the first
  new rpc_clnt_add_xprt setup function to avoid an unnecessary
  EXCHANGE_ID.

- Added rpc_xprt_switch test in nfs_match_client so we find an
  nfs_client that is using an rpc_xprt_switch address.

Also:
- The new rpc_clnt_add_xprt setup function returns 1 and added a
  call to rpc_xprt_switch_add_xprt to the session trunk
  case of nfs4_exchange_id_done.

- Saved the mount exchange id verifier in cl_confirm and use it
  in session trunking exchange id's

Testing: pynfs server configured with pnfs mulitpath data servers
- tested working, unconfigured, and repeated ipv4 multipath
   addresses

Andy Adamson (12):
  NFS setup async exchange_id
  NFS refactor nfs4_match_clientids
  NFS refactor nfs4_check_serverowner_major_id
  NFS detect session trunking
  SUNRPC remove rpc_task_release_client from rpc_task_set_client
  SUNRPC rpc_clnt_xprt_switch_put
  SUNRPC rpc_clnt_xprt_switch_add_xprt
  SUNRPC search xprt switch for sockaddr
  SUNRPC: rpc_clnt_add_xprt setup function for NFS layer
  NFS add xprt switch addrs test to match client
  NFS test session trunking with exchange id
  NFS pnfs data server multipath session trunking

 fs/nfs/client.c                      |   5 +-
 fs/nfs/internal.h                    |   3 +
 fs/nfs/nfs4_fs.h                     |   8 +
 fs/nfs/nfs4client.c                  | 114 ++++++++++++--
 fs/nfs/nfs4proc.c                    | 294 +++++++++++++++++++++++++----------
 fs/nfs/pnfs_nfs.c                    |  56 +++++--
 include/linux/sunrpc/clnt.h          |  17 ++
 include/linux/sunrpc/xprtmultipath.h |   2 +
 net/sunrpc/clnt.c                    |  93 ++++++++++-
 net/sunrpc/xprtmultipath.c           |  24 ++-
 10 files changed, 502 insertions(+), 114 deletions(-)

-- 
1.8.3.1

[PATCH Version 8 01/12] 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 8 02/12] 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 8 03/12] 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 8 04/12] 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..5907b14 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 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 8 04/12] NFS detect session trunking

[Anna Schumaker]

Hi Andy,

I noticed a few typos:

On 08/11/2016 05:39 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..5907b14 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 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
                                                ^^^^^^^^
You have an extra "s" at the beginning of "session".


> + * after a successuful EXCHANGE_ID testing a multi-addr connection to be
              ^^^^^^^^^^^
Should be "successful", without the extra "u".

Thanks,
Anna

> + * 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 8 05/12] SUNRPC remove rpc_task_release_client from rpc_task_set_client

[andros]

From: Andy Adamson <andros@netapp.com>

rpc_task_set_client is only called from rpc_run_task after
rpc_new_task and rpc_task_release_client is not needed as the
task is new.

When called from rpc_new_task, rpc_task_set_client also removed the
assigned rpc_xprt which is not desired.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 net/sunrpc/clnt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index cb49898..163b8f4 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -988,7 +988,6 @@ void rpc_task_set_client(struct rpc_task *task, struct rpc_clnt *clnt)
 {
 
 	if (clnt != NULL) {
-		rpc_task_release_client(task);
 		if (task->tk_xprt == NULL)
 			task->tk_xprt = xprt_iter_get_next(&clnt->cl_xpi);
 		task->tk_client = clnt;
-- 
1.8.3.1

[PATCH Version 8 06/12] SUNRPC rpc_clnt_xprt_switch_put

[andros]

From: Andy Adamson <andros@netapp.com>

Give the NFS layer access to the xprt_switch_put function

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

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index b6810c9..6005095 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -197,5 +197,7 @@ int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
 			void *data);
 
 const char *rpc_proc_name(const struct rpc_task *task);
+
+void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SUNRPC_CLNT_H */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 163b8f4..83ab914 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2672,6 +2672,12 @@ out_put_switch:
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
 
+void rpc_clnt_xprt_switch_put(struct rpc_clnt *clnt)
+{
+	xprt_switch_put(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_put);
+
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 static void rpc_show_header(void)
 {
-- 
1.8.3.1

[PATCH Version 8 07/12] SUNRPC rpc_clnt_xprt_switch_add_xprt

[andros]

From: Andy Adamson <andros@netapp.com>

Give the NFS layer access to the rpc_xprt_switch_add_xprt function

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

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 6005095..28cbc81 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -199,5 +199,6 @@ int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
 const char *rpc_proc_name(const struct rpc_task *task);
 
 void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
+void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SUNRPC_CLNT_H */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 83ab914..35df635 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2678,6 +2678,13 @@ void rpc_clnt_xprt_switch_put(struct rpc_clnt *clnt)
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_put);
 
+void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+{
+	rpc_xprt_switch_add_xprt(rcu_dereference(clnt->cl_xpi.xpi_xpswitch),
+				 xprt);
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_add_xprt);
+
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 static void rpc_show_header(void)
 {
-- 
1.8.3.1

[PATCH Version 8 08/12] SUNRPC search xprt switch for sockaddr

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 include/linux/sunrpc/clnt.h          |  2 ++
 include/linux/sunrpc/xprtmultipath.h |  2 ++
 net/sunrpc/clnt.c                    | 15 +++++++++++++++
 net/sunrpc/xprtmultipath.c           | 24 +++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 28cbc81..2db687c 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -200,5 +200,7 @@ const char *rpc_proc_name(const struct rpc_task *task);
 
 void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
 void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
+bool rpc_clnt_xprt_switch_find_addr(struct rpc_clnt *clnt,
+			const struct sockaddr *sap);
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SUNRPC_CLNT_H */
diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index 5a9acff..bc354bd 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -66,4 +66,6 @@ extern struct rpc_xprt *xprt_iter_xprt(struct rpc_xprt_iter *xpi);
 extern struct rpc_xprt *xprt_iter_get_xprt(struct rpc_xprt_iter *xpi);
 extern struct rpc_xprt *xprt_iter_get_next(struct rpc_xprt_iter *xpi);
 
+extern bool rpc_xprt_switch_find_addr(struct rpc_xprt_switch *xps,
+		const struct sockaddr *sap);
 #endif
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 35df635..e3cfced 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2685,6 +2685,21 @@ void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_add_xprt);
 
+bool rpc_clnt_xprt_switch_find_addr(struct rpc_clnt *clnt,
+				    const struct sockaddr *sap)
+{
+	struct rpc_xprt_switch *xps;
+	bool ret;
+
+	xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
+
+	rcu_read_lock();
+	ret = rpc_xprt_switch_find_addr(xps, sap);
+	rcu_read_unlock();
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_find_addr);
+
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 static void rpc_show_header(void)
 {
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 66c9d63..8798726 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -15,6 +15,7 @@
 #include <asm/cmpxchg.h>
 #include <linux/spinlock.h>
 #include <linux/sunrpc/xprt.h>
+#include <linux/sunrpc/addr.h>
 #include <linux/sunrpc/xprtmultipath.h>
 
 typedef struct rpc_xprt *(*xprt_switch_find_xprt_t)(struct list_head *head,
@@ -49,7 +50,8 @@ void rpc_xprt_switch_add_xprt(struct rpc_xprt_switch *xps,
 	if (xprt == NULL)
 		return;
 	spin_lock(&xps->xps_lock);
-	if (xps->xps_net == xprt->xprt_net || xps->xps_net == NULL)
+	if ((xps->xps_net == xprt->xprt_net || xps->xps_net == NULL) &&
+	    !rpc_xprt_switch_find_addr(xps, (struct sockaddr *)&xprt->addr))
 		xprt_switch_add_xprt_locked(xps, xprt);
 	spin_unlock(&xps->xps_lock);
 }
@@ -232,6 +234,26 @@ struct rpc_xprt *xprt_iter_current_entry(struct rpc_xprt_iter *xpi)
 	return xprt_switch_find_current_entry(head, xpi->xpi_cursor);
 }
 
+bool rpc_xprt_switch_find_addr(struct rpc_xprt_switch *xps,
+			       const struct sockaddr *sap)
+{
+	struct list_head *head;
+	struct rpc_xprt *pos;
+
+	if (xps == NULL || sap == NULL)
+		return false;
+
+	head = &xps->xps_xprt_list;
+	list_for_each_entry_rcu(pos, head, xprt_switch) {
+		if (rpc_cmp_addr_port(sap, (struct sockaddr *)&pos->addr)) {
+			pr_info("RPC:   addr %s already in xprt switch\n",
+				pos->address_strings[RPC_DISPLAY_ADDR]);
+			return true;
+		}
+	}
+	return false;
+}
+
 static
 struct rpc_xprt *xprt_switch_find_next_entry(struct list_head *head,
 		const struct rpc_xprt *cur)
-- 
1.8.3.1

Re: [PATCH Version 8 08/12] SUNRPC search xprt switch for sockaddr

[Anna Schumaker]

Hi Andy,

On 08/11/2016 05:39 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 ++
>  include/linux/sunrpc/xprtmultipath.h |  2 ++
>  net/sunrpc/clnt.c                    | 15 +++++++++++++++
>  net/sunrpc/xprtmultipath.c           | 24 +++++++++++++++++++++++-
>  4 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 28cbc81..2db687c 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -200,5 +200,7 @@ const char *rpc_proc_name(const struct rpc_task *task);
>  
>  void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
>  void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
> +bool rpc_clnt_xprt_switch_find_addr(struct rpc_clnt *clnt,
> +			const struct sockaddr *sap);
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_SUNRPC_CLNT_H */
> diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
> index 5a9acff..bc354bd 100644
> --- a/include/linux/sunrpc/xprtmultipath.h
> +++ b/include/linux/sunrpc/xprtmultipath.h
> @@ -66,4 +66,6 @@ extern struct rpc_xprt *xprt_iter_xprt(struct rpc_xprt_iter *xpi);
>  extern struct rpc_xprt *xprt_iter_get_xprt(struct rpc_xprt_iter *xpi);
>  extern struct rpc_xprt *xprt_iter_get_next(struct rpc_xprt_iter *xpi);
>  
> +extern bool rpc_xprt_switch_find_addr(struct rpc_xprt_switch *xps,
> +		const struct sockaddr *sap);
>  #endif
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 35df635..e3cfced 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2685,6 +2685,21 @@ void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_add_xprt);
>  
> +bool rpc_clnt_xprt_switch_find_addr(struct rpc_clnt *clnt,
> +				    const struct sockaddr *sap)

Since this is only returning a yes/no answer, can you rename it to rpc_clnt_xprt_switch_has_addr()?

> +{
> +	struct rpc_xprt_switch *xps;
> +	bool ret;
> +
> +	xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
> +
> +	rcu_read_lock();
> +	ret = rpc_xprt_switch_find_addr(xps, sap);
> +	rcu_read_unlock();
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_find_addr);
> +
>  #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>  static void rpc_show_header(void)
>  {
> diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
> index 66c9d63..8798726 100644
> --- a/net/sunrpc/xprtmultipath.c
> +++ b/net/sunrpc/xprtmultipath.c
> @@ -15,6 +15,7 @@
>  #include <asm/cmpxchg.h>
>  #include <linux/spinlock.h>
>  #include <linux/sunrpc/xprt.h>
> +#include <linux/sunrpc/addr.h>
>  #include <linux/sunrpc/xprtmultipath.h>
>  
>  typedef struct rpc_xprt *(*xprt_switch_find_xprt_t)(struct list_head *head,
> @@ -49,7 +50,8 @@ void rpc_xprt_switch_add_xprt(struct rpc_xprt_switch *xps,
>  	if (xprt == NULL)
>  		return;
>  	spin_lock(&xps->xps_lock);
> -	if (xps->xps_net == xprt->xprt_net || xps->xps_net == NULL)
> +	if ((xps->xps_net == xprt->xprt_net || xps->xps_net == NULL) &&
> +	    !rpc_xprt_switch_find_addr(xps, (struct sockaddr *)&xprt->addr))
>  		xprt_switch_add_xprt_locked(xps, xprt);
>  	spin_unlock(&xps->xps_lock);
>  }
> @@ -232,6 +234,26 @@ struct rpc_xprt *xprt_iter_current_entry(struct rpc_xprt_iter *xpi)
>  	return xprt_switch_find_current_entry(head, xpi->xpi_cursor);
>  }
>  
> +bool rpc_xprt_switch_find_addr(struct rpc_xprt_switch *xps,
> +			       const struct sockaddr *sap)

Same here:  rpc_xprt_switch_has_addr()

Thanks,
Anna

> +{
> +	struct list_head *head;
> +	struct rpc_xprt *pos;
> +
> +	if (xps == NULL || sap == NULL)
> +		return false;
> +
> +	head = &xps->xps_xprt_list;
> +	list_for_each_entry_rcu(pos, head, xprt_switch) {
> +		if (rpc_cmp_addr_port(sap, (struct sockaddr *)&pos->addr)) {
> +			pr_info("RPC:   addr %s already in xprt switch\n",
> +				pos->address_strings[RPC_DISPLAY_ADDR]);
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
>  static
>  struct rpc_xprt *xprt_switch_find_next_entry(struct list_head *head,
>  		const struct rpc_xprt *cur)
> 

[PATCH Version 8 09/12] SUNRPC: rpc_clnt_add_xprt setup function for NFS layer

[andros]

From: Andy Adamson <andros@netapp.com>

Use a setup function to call into the NFS layer to test an rpc_xprt
for session trunking so as to not leak the rpc_xprt_switch into
the nfs layer.

Search for the address in the rpc_xprt_switch first so as not to
put an unnecessary EXCHANGE_ID on the wire.

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

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 2db687c..c05c351 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -125,6 +125,13 @@ struct rpc_create_args {
 	struct svc_xprt		*bc_xprt;	/* NFSv4.1 backchannel */
 };
 
+struct rpc_add_xprt_test {
+	int (*add_xprt_test)(struct rpc_clnt *,
+		struct rpc_xprt *,
+		void *calldata);
+	void *data;
+};
+
 /* Values for "flags" field */
 #define RPC_CLNT_CREATE_HARDRTRY	(1UL << 0)
 #define RPC_CLNT_CREATE_AUTOBIND	(1UL << 2)
@@ -196,6 +203,11 @@ int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
 				void *),
 			void *data);
 
+int		rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
+			struct rpc_xprt_switch *,
+			struct rpc_xprt *,
+			void *);
+
 const char *rpc_proc_name(const struct rpc_task *task);
 
 void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index e3cfced..09d36d6 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2615,6 +2615,70 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
 
 /**
+ * rpc_clnt_setup_test_and_add_xprt()
+ *
+ * This is an rpc_clnt_add_xprt setup() function which returns 1 so:
+ *   1) caller of the test function must dereference the rpc_xprt_switch
+ *   and the rpc_xprt.
+ *   2) test function must call rpc_xprt_switch_add_xprt, usually in
+ *   the rpc_call_done routine.
+ *
+ * Upon success (return of 1), the test function adds the new
+ * transport to the rpc_clnt xprt switch
+ *
+ * @clnt: struct rpc_clnt to get the new transport
+ * @xps:  the rpc_xprt_switch to hold the new transport
+ * @xprt: the rpc_xprt to test
+ * @data: a struct rpc_add_xprt_test pointer that holds the test function
+ *        and test function call data
+ */
+int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *clnt,
+				     struct rpc_xprt_switch *xps,
+				     struct rpc_xprt *xprt,
+				     void *data)
+{
+	struct rpc_cred *cred;
+	struct rpc_task *task;
+	struct rpc_add_xprt_test *xtest = (struct rpc_add_xprt_test *)data;
+	int status = -EADDRINUSE;
+
+	xprt = xprt_get(xprt);
+	xprt_switch_get(xps);
+
+	if (rpc_xprt_switch_find_addr(xps, (struct sockaddr *)&xprt->addr))
+		goto out_err;
+
+	/* Test the connection */
+	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)) {
+		status = PTR_ERR(task);
+		goto out_err;
+	}
+	status = task->tk_status;
+	rpc_put_task(task);
+
+	if (status < 0)
+		goto out_err;
+
+	/* rpc_xprt_switch and rpc_xprt are deferrenced by add_xprt_test() */
+	xtest->add_xprt_test(clnt, xprt, xtest->data);
+
+	/* so that rpc_clnt_add_xprt does not call rpc_xprt_switch_add_xprt */
+	return 1;
+out_err:
+	xprt_put(xprt);
+	xprt_switch_put(xps);
+	pr_info("RPC:   rpc_clnt_test_xprt failed: %d addr %s not added\n",
+		status, xprt->address_strings[RPC_DISPLAY_ADDR]);
+	return status;
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_setup_test_and_add_xprt);
+
+/**
  * rpc_clnt_add_xprt - Add a new transport to a rpc_clnt
  * @clnt: pointer to struct rpc_clnt
  * @xprtargs: pointer to struct xprt_create
-- 
1.8.3.1

[PATCH Version 8 10/12] NFS add xprt switch addrs test to match client

[andros]

From: Andy Adamson <andros@netapp.com>

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

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4849d0f..21413a9 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -313,7 +313,10 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 			continue;
 		/* Match the full socket address */
 		if (!rpc_cmp_addr_port(sap, clap))
-			continue;
+			/* Match all xprt_switch full socket addresses */
+			if (!rpc_clnt_xprt_switch_find_addr(clp->cl_rpcclient,
+							    sap))
+				continue;
 
 		atomic_inc(&clp->cl_count);
 		return clp;
-- 
1.8.3.1

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

[andros]

From: Andy Adamson <andros@netapp.com>

Use an async exchange id call to test for session trunking

To conform with RFC 5661 section 18.35.4, the Non-Update on
Existing Clientid case, save the exchange id verifier in
cl_confirm and use it for the session trunking exhange id test.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4_fs.h  |  5 +++++
 fs/nfs/nfs4proc.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 49 insertions(+), 8 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..ffe578c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7054,6 +7054,7 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
 struct nfs41_exchange_id_data {
 	struct nfs41_exchange_id_res res;
 	struct nfs41_exchange_id_args args;
+	struct rpc_xprt *xprt;
 	int rpc_status;
 };
 
@@ -7068,6 +7069,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->xprt)
+		goto session_trunk;
+
 	if (status  == 0)
 		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
 
@@ -7104,8 +7109,23 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
 			clp->cl_serverscope = cdata->res.server_scope;
 			cdata->res.server_scope = NULL;
 		}
+		/* Save the EXCHANGE_ID verifier session trunk tests */
+		memcpy(clp->cl_confirm.data, cdata->args.verifier->data,
+		       sizeof(clp->cl_confirm.data));
 	}
+out:
 	cdata->rpc_status = status;
+	return;
+
+session_trunk:
+	if (status == 0) {
+		status = nfs4_detect_session_trunking(clp, &cdata->res,
+						      cdata->xprt);
+		if (status == 0)
+			rpc_clnt_xprt_switch_add_xprt(clp->cl_rpcclient,
+						      cdata->xprt);
+	}
+	goto out;
 }
 
 static void nfs4_exchange_id_release(void *data)
@@ -7114,6 +7134,10 @@ static void nfs4_exchange_id_release(void *data)
 					(struct nfs41_exchange_id_data *)data;
 
 	nfs_put_client(cdata->args.client);
+	if (cdata->xprt) {
+		xprt_put(cdata->xprt);
+		rpc_clnt_xprt_switch_put(cdata->args.client->cl_rpcclient);
+	}
 	kfree(cdata->res.impl_id);
 	kfree(cdata->res.server_scope);
 	kfree(cdata->res.server_owner);
@@ -7131,7 +7155,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 rpc_xprt *xprt)
 {
 	nfs4_verifier verifier;
 	struct rpc_message msg = {
@@ -7156,7 +7180,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	if (!calldata)
 		goto out;
 
-	nfs4_init_boot_verifier(clp, &verifier);
+	if (!xprt)
+		nfs4_init_boot_verifier(clp, &verifier);
 
 	status = nfs4_init_uniform_client_string(clp);
 	if (status)
@@ -7196,8 +7221,15 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		status = -EINVAL;
 		goto out_impl_id;
 	}
-
-	calldata->args.verifier = &verifier;
+	if (xprt) {
+		calldata->xprt = xprt;
+		task_setup_data.rpc_xprt = xprt;
+		task_setup_data.flags =
+				RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC;
+		calldata->args.verifier = &clp->cl_confirm;
+	} else {
+		calldata->args.verifier = &verifier;
+	}
 	calldata->args.client = clp;
 #ifdef CONFIG_NFS_V4_1_MIGRATION
 	calldata->args.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
@@ -7217,9 +7249,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 (!xprt) {
+		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 +7298,13 @@ 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);
 }
 
 static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
-- 
1.8.3.1

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

[Anna Schumaker]

Hi Andy,

On 08/11/2016 05:39 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Use an async exchange id call to test for session trunking
> 
> To conform with RFC 5661 section 18.35.4, the Non-Update on
> Existing Clientid case, save the exchange id verifier in
> cl_confirm and use it for the session trunking exhange id test.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4_fs.h  |  5 +++++
>  fs/nfs/nfs4proc.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 49 insertions(+), 8 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..ffe578c 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7054,6 +7054,7 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
>  struct nfs41_exchange_id_data {
>  	struct nfs41_exchange_id_res res;
>  	struct nfs41_exchange_id_args args;
> +	struct rpc_xprt *xprt;
>  	int rpc_status;
>  };
>  
> @@ -7068,6 +7069,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->xprt)
> +		goto session_trunk;
> +
>  	if (status  == 0)
>  		status = nfs4_sp4_select_mode(clp, &cdata->res.state_protect);
>  
> @@ -7104,8 +7109,23 @@ static void nfs4_exchange_id_done(struct rpc_task *task, void *data)
>  			clp->cl_serverscope = cdata->res.server_scope;
>  			cdata->res.server_scope = NULL;
>  		}
> +		/* Save the EXCHANGE_ID verifier session trunk tests */
> +		memcpy(clp->cl_confirm.data, cdata->args.verifier->data,
> +		       sizeof(clp->cl_confirm.data));
>  	}
> +out:
>  	cdata->rpc_status = status;
> +	return;
> +
> +session_trunk:
> +	if (status == 0) {
> +		status = nfs4_detect_session_trunking(clp, &cdata->res,
> +						      cdata->xprt);
> +		if (status == 0)
> +			rpc_clnt_xprt_switch_add_xprt(clp->cl_rpcclient,
> +						      cdata->xprt);

Would it make sense to have nfs4_detect_session_trunking() call rpc_clnt_xprt_switch_add_xprt() on success?  Then you might be able to get by without calling it directly from your if (cdata->xprt) check (above), rather than adding the extra goto.

Thoughts?
Anna

> +	}
> +	goto out;
>  }
>  
>  static void nfs4_exchange_id_release(void *data)
> @@ -7114,6 +7134,10 @@ static void nfs4_exchange_id_release(void *data)
>  					(struct nfs41_exchange_id_data *)data;
>  
>  	nfs_put_client(cdata->args.client);
> +	if (cdata->xprt) {
> +		xprt_put(cdata->xprt);
> +		rpc_clnt_xprt_switch_put(cdata->args.client->cl_rpcclient);
> +	}
>  	kfree(cdata->res.impl_id);
>  	kfree(cdata->res.server_scope);
>  	kfree(cdata->res.server_owner);
> @@ -7131,7 +7155,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 rpc_xprt *xprt)
>  {
>  	nfs4_verifier verifier;
>  	struct rpc_message msg = {
> @@ -7156,7 +7180,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>  	if (!calldata)
>  		goto out;
>  
> -	nfs4_init_boot_verifier(clp, &verifier);
> +	if (!xprt)
> +		nfs4_init_boot_verifier(clp, &verifier);
>  
>  	status = nfs4_init_uniform_client_string(clp);
>  	if (status)
> @@ -7196,8 +7221,15 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>  		status = -EINVAL;
>  		goto out_impl_id;
>  	}
> -
> -	calldata->args.verifier = &verifier;
> +	if (xprt) {
> +		calldata->xprt = xprt;
> +		task_setup_data.rpc_xprt = xprt;
> +		task_setup_data.flags =
> +				RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC;
> +		calldata->args.verifier = &clp->cl_confirm;
> +	} else {
> +		calldata->args.verifier = &verifier;
> +	}
>  	calldata->args.client = clp;
>  #ifdef CONFIG_NFS_V4_1_MIGRATION
>  	calldata->args.flags = EXCHGID4_FLAG_SUPP_MOVED_REFER |
> @@ -7217,9 +7249,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 (!xprt) {
> +		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 +7298,13 @@ 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);
>  }
>  
>  static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
> 

[PATCH Version 8 12/12] 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/internal.h |  3 +++
 fs/nfs/nfs4_fs.h  |  1 +
 fs/nfs/nfs4proc.c | 33 ++++++++++++++++++++++++++++++++
 fs/nfs/pnfs_nfs.c | 56 +++++++++++++++++++++++++++++++++++++++++--------------
 4 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 8de509b..4f4e775 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -569,6 +569,9 @@ extern int nfs40_walk_client_list(struct nfs_client *clp,
 extern int nfs41_walk_client_list(struct nfs_client *clp,
 				struct nfs_client **result,
 				struct rpc_cred *cred);
+extern int nfs4_test_session_trunk(struct rpc_clnt *,
+				struct rpc_xprt *,
+				void *);
 
 static inline struct inode *nfs_igrab_and_active(struct inode *inode)
 {
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b2a94ca..e7d2efe 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -59,6 +59,7 @@ 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 *, 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 ffe578c..3770f03 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7307,6 +7307,37 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE, NULL);
 }
 
+/**
+ * nfs4_test_session_trunk
+ *
+ * This is an add_xprt_test() test function called from
+ * rpc_clnt_setup_test_and_add_xprt.
+ *
+ * The rpc_xprt_switch is referrenced by rpc_clnt_setup_test_and_add_xprt
+ * and is dereferrenced in nfs4_exchange_id_release
+ *
+ * Upon success, add the new transport to the rpc_clnt
+ *
+ * @clnt: struct rpc_clnt to get 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 *xprt,
+			    void *data)
+{
+	struct nfs4_add_xprt_data *adata = (struct nfs4_add_xprt_data *)data;
+	u32 sp4_how;
+
+	dprintk("--> %s try %s\n", __func__,
+		xprt->address_strings[RPC_DISPLAY_ADDR]);
+
+	sp4_how = (adata->clp->cl_sp4_flags == 0 ? SP4_NONE : SP4_MACH_CRED);
+
+	/* Test connection for session trunking. Async exchange_id call */
+	return  _nfs4_proc_exchange_id(adata->clp, adata->cred, sp4_how, xprt);
+}
+EXPORT_SYMBOL_GPL(nfs4_test_session_trunk);
+
 static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
 		struct rpc_cred *cred)
 {
@@ -8885,6 +8916,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,
@@ -8914,6 +8946,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..242ce87 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -690,13 +690,48 @@ 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,
+				.cred = nfs4_get_clid_cred(clp),
+			};
+			struct rpc_add_xprt_test rpcdata = {
+				.add_xprt_test = clp->cl_mvops->session_trunk,
+				.data = &xprtdata,
+			};
+
+			/**
+			* Test this address for session trunking and
+			* add as an alias
+			*/
+			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
+					  rpc_clnt_setup_test_and_add_xprt,
+					  &rpcdata);
+		} 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 +739,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