[PATCH RFC Version 1 0/6] Request for Comment: NFS4.1 Session Trunking

[PATCH RFC Version 1 0/6] Request for Comment: NFS4.1 Session Trunking

[andros]

From: Andy Adamson <andros@netapp.com>

This early patch set implements NFSv4.1 session trunking utilizing Tronds
multipath feature.  This works against an upstream linux server with
two NICs as the EXCHANGE_ID info is the same for both NICs.

Unmodified code will create a new nfs_client structure for the mount as
nfs_match_client fails due to the different full server address.
Tunking is detected by comparing the eir_server_owner.major/minor/scope
and the eir_server_impl_id to an existing mount. When trunking is detected,
the 2nd mount point uses the existing  (not new) nfs_client for the new
mount point.

When trunking is detected, this patch set adds an rpc_xprt for the new server
address to the exisiting mount, calls BIND_CONN_TO_SESSION, and destroys
the new clientid (no second mount).

We then get round robin use of the two NICs off one mount point all
using a single session.


Is using mount a good way to setup NFSv4.1 session trunking?

The first two patches fix issues with the multipath code.

SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net
    This patch adds a flag to allow multipathing within the origial net.
    I don't understand why this was not allowed, so I expect a different
    solution for this issue.


Comments please.

Thanks

-->Andy

Andy Adamson (6):
  SUNRPC fix rpc_clnt_add_xprt setup return
  SUNRPC fix rpc_clnt_xprt_iter_init infinite loop
  SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net
  NFS session trunking support: add trunk xprt to nfs4.1 mount
  NFS add callback_ops to nfs4_proc_bind_conn_to_session_callback
  NFS schedule bind_conn_to_session for new NFSv4.1 transport

 fs/nfs/client.c                      |  2 ++
 fs/nfs/nfs4client.c                  | 29 +++++++++++++++++++++++++++--
 fs/nfs/nfs4proc.c                    | 10 ++++++++++
 include/linux/nfs_fs_sb.h            |  1 +
 include/linux/sunrpc/clnt.h          |  1 +
 include/linux/sunrpc/xprtmultipath.h |  6 +++++-
 net/sunrpc/clnt.c                    | 20 +++++++++++++-------
 net/sunrpc/xprtmultipath.c           | 35 +++++++++++++++++++++++++++++++++--
 8 files changed, 92 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[PATCH RFC Version 1 1/6] SUNRPC fix rpc_clnt_add_xprt setup return

[andros]

From: Andy Adamson <andros@netapp.com>

Caller rpc_clnt_add_xprt() expects zero on success.

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

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index efd6ffc..c0469d1 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2573,7 +2573,7 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 	if (IS_ERR(task))
 		return PTR_ERR(task);
 	rpc_put_task(task);
-	return 1;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
 
-- 
1.8.3.1

[PATCH RFC Version 1 2/6] SUNRPC fix rpc_clnt_xprt_iter_init infinite loop

[andros]

From: Andy Adamson <andros@netapp.com>

xprt_iter_get_next() will restart at top of list and never stop.
Only process xps_nxprts xprts.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 net/sunrpc/clnt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index c0469d1..a56a44c8 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -740,6 +740,7 @@ static
 int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
 {
 	struct rpc_xprt_switch *xps;
+	int nxprt;
 
 	rcu_read_lock();
 	xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
@@ -747,8 +748,9 @@ int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
 	if (xps == NULL)
 		return -EAGAIN;
 	xprt_iter_init_listall(xpi, xps);
+	nxprt = xps->xps_nxprts;
 	xprt_switch_put(xps);
-	return 0;
+	return nxprt;
 }
 
 /**
@@ -767,10 +769,11 @@ int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
 		void *data)
 {
 	struct rpc_xprt_iter xpi;
-	int ret;
+	int ret = 0, nxprts, i;
 
-	ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
-	for (;;) {
+	nxprts = rpc_clnt_xprt_iter_init(clnt, &xpi);
+	dprintk("RPC:   processing nxprts %d\n", nxprts);
+	for (i = 0; i < nxprts; i++) {
 		struct rpc_xprt *xprt = xprt_iter_get_next(&xpi);
 
 		if (!xprt)
-- 
1.8.3.1

[PATCH RFC Version 1 3/6] SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net

[andros]

From: Andy Adamson <andros@netapp.com>

NFSv4.1 session trunking wants to add multiple xprts to the same net.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/client.c                      |  2 ++
 fs/nfs/nfs4client.c                  |  4 +++-
 include/linux/nfs_fs_sb.h            |  1 +
 include/linux/sunrpc/clnt.h          |  1 +
 include/linux/sunrpc/xprtmultipath.h |  6 +++++-
 net/sunrpc/clnt.c                    |  7 +++++--
 net/sunrpc/xprtmultipath.c           | 33 +++++++++++++++++++++++++++++++--
 7 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d6d5d2a..f07d639 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -494,6 +494,8 @@ int nfs_create_rpc_client(struct nfs_client *clp,
 		args.flags |= RPC_CLNT_CREATE_NONPRIVPORT;
 	if (test_bit(NFS_CS_INFINITE_SLOTS, &clp->cl_flags))
 		args.flags |= RPC_CLNT_CREATE_INFINITE_SLOTS;
+	if (test_bit(NFS_CS_SAME_NET_OK, &clp->cl_flags))
+		args.flags |= RPC_CLNT_CREATE_SAME_NET_OK;
 
 	if (!IS_ERR(clp->cl_rpcclient))
 		return 0;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 10410e8..5f20acf 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -365,8 +365,10 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 	/* Check NFS protocol revision and initialize RPC op vector */
 	clp->rpc_ops = &nfs_v4_clientops;
 
-	if (clp->cl_minorversion != 0)
+	if (clp->cl_minorversion != 0) {
+		__set_bit(NFS_CS_SAME_NET_OK, &clp->cl_flags);
 		__set_bit(NFS_CS_INFINITE_SLOTS, &clp->cl_flags);
+	}
 	__set_bit(NFS_CS_DISCRTRY, &clp->cl_flags);
 	__set_bit(NFS_CS_NO_RETRANS_TIMEOUT, &clp->cl_flags);
 
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 7fcc13c..8a63993 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -42,6 +42,7 @@ struct nfs_client {
 #define NFS_CS_MIGRATION	2		/* - transparent state migr */
 #define NFS_CS_INFINITE_SLOTS	3		/* - don't limit TCP slots */
 #define NFS_CS_NO_RETRANS_TIMEOUT	4	/* - Disable retransmit timeouts */
+#define NFS_CS_SAME_NET_OK	5		/* same net multiple xprts ok */
 	struct sockaddr_storage	cl_addr;	/* server identifier */
 	size_t			cl_addrlen;
 	char *			cl_hostname;	/* hostname of server */
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 9a7ddba..d8fa359 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -135,6 +135,7 @@ struct rpc_create_args {
 #define RPC_CLNT_CREATE_INFINITE_SLOTS	(1UL << 7)
 #define RPC_CLNT_CREATE_NO_IDLE_TIMEOUT	(1UL << 8)
 #define RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT	(1UL << 9)
+#define RPC_CLNT_CREATE_SAME_NET_OK	(1UL << 10)
 
 struct rpc_clnt *rpc_create(struct rpc_create_args *args);
 struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index 5a9acff..05697cf 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -18,6 +18,10 @@ struct rpc_xprt_switch {
 
 	struct net *		xps_net;
 
+#define XPRT_SWITCH_SAME_NET_OK		(1UL << 0)
+
+	int			xps_flags;
+
 	const struct rpc_xprt_iter_ops *xps_iter_ops;
 
 	struct rcu_head		xps_rcu;
@@ -38,7 +42,7 @@ struct rpc_xprt_iter_ops {
 };
 
 extern struct rpc_xprt_switch *xprt_switch_alloc(struct rpc_xprt *xprt,
-		gfp_t gfp_flags);
+		gfp_t gfp_flags, int flags);
 
 extern struct rpc_xprt_switch *xprt_switch_get(struct rpc_xprt_switch *xps);
 extern void xprt_switch_put(struct rpc_xprt_switch *xps);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index a56a44c8..38676a8 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -451,8 +451,11 @@ struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
 {
 	struct rpc_clnt *clnt = NULL;
 	struct rpc_xprt_switch *xps;
+	int flags = 0;
 
-	xps = xprt_switch_alloc(xprt, GFP_KERNEL);
+	if (args->flags & RPC_CLNT_CREATE_SAME_NET_OK)
+		flags |= XPRT_SWITCH_SAME_NET_OK;
+	xps = xprt_switch_alloc(xprt, GFP_KERNEL, flags);
 	if (xps == NULL)
 		return ERR_PTR(-ENOMEM);
 
@@ -683,7 +686,7 @@ int rpc_switch_client_transport(struct rpc_clnt *clnt,
 		return PTR_ERR(xprt);
 	}
 
-	xps = xprt_switch_alloc(xprt, GFP_KERNEL);
+	xps = xprt_switch_alloc(xprt, GFP_KERNEL, 0);
 	if (xps == NULL) {
 		xprt_put(xprt);
 		return -ENOMEM;
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 8b95ad1..7be5bd2 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -16,6 +16,7 @@
 #include <linux/spinlock.h>
 #include <linux/sunrpc/xprt.h>
 #include <linux/sunrpc/xprtmultipath.h>
+#include <linux/sunrpc/addr.h>
 
 typedef struct rpc_xprt *(*xprt_switch_find_xprt_t)(struct list_head *head,
 		const struct rpc_xprt *cur);
@@ -37,6 +38,22 @@ static void xprt_switch_add_xprt_locked(struct rpc_xprt_switch *xps,
 }
 
 /**
+ * See if xprt->addr is already in switch
+ */
+static int
+rpc_xprt_switch_find_addr(struct list_head *head, struct rpc_xprt *xprt)
+{
+	struct rpc_xprt *local;
+
+	list_for_each_entry_rcu(local, head, xprt_switch) {
+		if (rpc_cmp_addr_port((struct sockaddr *)&local->addr,
+				      (struct sockaddr *)&xprt->addr))
+				return 1;
+	}
+	return 0;
+}
+
+/**
  * rpc_xprt_switch_add_xprt - Add a new rpc_xprt to an rpc_xprt_switch
  * @xps: pointer to struct rpc_xprt_switch
  * @xprt: pointer to struct rpc_xprt
@@ -49,8 +66,19 @@ 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) {
+		pr_info("RPC:    ADD NEW NET xprt %p servername %s\n", xprt,
+			xprt->servername);
 		xprt_switch_add_xprt_locked(xps, xprt);
+	}
+	if (xps->xps_flags & XPRT_SWITCH_SAME_NET_OK &&
+	    xps->xps_net == xprt->xprt_net) {
+		if (!rpc_xprt_switch_find_addr(&xps->xps_xprt_list, xprt)) {
+			pr_info("RPC:    ADD SAME NET xprt %p servername %s\n",
+				__func__, xprt, xprt->servername);
+			xprt_switch_add_xprt_locked(xps, xprt);
+		}
+	}
 	spin_unlock(&xps->xps_lock);
 }
 
@@ -91,7 +119,7 @@ void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
  * the entry xprt. Returns NULL on failure.
  */
 struct rpc_xprt_switch *xprt_switch_alloc(struct rpc_xprt *xprt,
-		gfp_t gfp_flags)
+		gfp_t gfp_flags, int flags)
 {
 	struct rpc_xprt_switch *xps;
 
@@ -102,6 +130,7 @@ struct rpc_xprt_switch *xprt_switch_alloc(struct rpc_xprt *xprt,
 		xps->xps_nxprts = 0;
 		INIT_LIST_HEAD(&xps->xps_xprt_list);
 		xps->xps_iter_ops = &rpc_xprt_iter_singular;
+		xps->xps_flags = flags;
 		xprt_switch_add_xprt_locked(xps, xprt);
 	}
 
-- 
1.8.3.1

[PATCH RFC Version 1 4/6] NFS session trunking support: add trunk xprt to nfs4.1 mount

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4client.c        | 16 +++++++++++++++-
 net/sunrpc/xprtmultipath.c |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 5f20acf..6736805 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -412,8 +412,22 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 	if (error < 0)
 		goto error;
 
-	if (clp != old)
+	if (clp != old) {
+		if (clp->cl_minorversion != 0) {
+			struct rpc_xprt *add = clp->cl_rpcclient->cl_xprt;
+			struct xprt_create xprt_args = {
+				.ident = XPRT_TRANSPORT_TCP,
+				.net = old->cl_net, /* clp->cl_net ??*/
+				.dstaddr = (struct sockaddr *)&add->addr,
+				.addrlen = add->addrlen,
+				.servername = clp->cl_hostname,
+			};
+			/* Add this address as an alias */
+			rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
+					rpc_clnt_test_and_add_xprt, NULL);
+		}
 		clp->cl_preserve_clid = true;
+	}
 	nfs_put_client(clp);
 	return old;
 
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 7be5bd2..f13419e 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -35,6 +35,8 @@ static void xprt_switch_add_xprt_locked(struct rpc_xprt_switch *xps,
 	if (xps->xps_nxprts == 0)
 		xps->xps_net = xprt->xprt_net;
 	xps->xps_nxprts++;
+	pr_info("RPC:  ADDED %s to xps %p xps_nxprts %d\n",
+		xprt->servername, xps, xps->xps_nxprts);
 }
 
 /**
-- 
1.8.3.1

[PATCH RFC Version 1 5/6] NFS add callback_ops to nfs4_proc_bind_conn_to_session_callback

[andros]

From: Andy Adamson <andros@netapp.com>

Fix oops when NULL callback_ops pointer accessed in rpc_init_task

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a2da473..ea07707 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6782,6 +6782,15 @@ nfs41_same_server_scope(struct nfs41_server_scope *a,
 	return false;
 }
 
+static void
+nfs4_bind_one_conn_to_session_done(struct rpc_task *task, void *calldata)
+{
+}
+
+static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops = {
+	.rpc_call_done =  &nfs4_bind_one_conn_to_session_done,
+};
+
 /*
  * nfs4_proc_bind_one_conn_to_session()
  *
@@ -6810,6 +6819,7 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
 	struct rpc_task_setup task_setup_data = {
 		.rpc_client = clnt,
 		.rpc_xprt = xprt,
+		.callback_ops = &nfs4_bind_one_conn_to_session_ops,
 		.rpc_message = &msg,
 		.flags = RPC_TASK_TIMEOUT,
 	};
-- 
1.8.3.1

[PATCH RFC Version 1 6/6] NFS schedule bind_conn_to_session for new NFSv4.1 transport

[andros]

From: Andy Adamson <andros@netapp.com>

Complete session trunking with the BIND_CONN_TO_SESSION call.
NFS cleanup after session trunking transport added

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

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 6736805..b0423f7 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -423,8 +423,17 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 				.servername = clp->cl_hostname,
 			};
 			/* Add this address as an alias */
-			rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
-					rpc_clnt_test_and_add_xprt, NULL);
+			error = rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
+					     rpc_clnt_test_and_add_xprt, NULL);
+			if (!error) {
+				nfs4_schedule_session_recovery(old->cl_session,
+					-NFS4ERR_CONN_NOT_BOUND_TO_SESSION);
+
+				/** Cancel the 2nd mount */
+				pr_info("NFS:  Session trunk. CANCEL MOUNT\n");
+				error = -EINVAL;
+				goto error;
+			}
 		}
 		clp->cl_preserve_clid = true;
 	}
-- 
1.8.3.1

Re: [PATCH RFC Version 1 0/6] Request for Comment: NFS4.1 Session Trunking

[Matt Benjamin]

Looks like a good thing.

Matt

----- Original Message -----
> From: andros@netapp.com
> To: "trond myklebust" <trond.myklebust@primarydata.com>
> Cc: linux-nfs@vger.kernel.org, "Andy Adamson" <andros@netapp.com>
> Sent: Friday, February 5, 2016 4:08:32 PM
> Subject: [PATCH RFC Version 1 0/6] Request for Comment: NFS4.1 Session Trunking
> 
> From: Andy Adamson <andros@netapp.com>
> 
> This early patch set implements NFSv4.1 session trunking utilizing Tronds
> multipath feature.  This works against an upstream linux server with
> two NICs as the EXCHANGE_ID info is the same for both NICs.
> 
> Unmodified code will create a new nfs_client structure for the mount as
> nfs_match_client fails due to the different full server address.
> Tunking is detected by comparing the eir_server_owner.major/minor/scope
> and the eir_server_impl_id to an existing mount. When trunking is detected,
> the 2nd mount point uses the existing  (not new) nfs_client for the new
> mount point.
> 
> When trunking is detected, this patch set adds an rpc_xprt for the new server
> address to the exisiting mount, calls BIND_CONN_TO_SESSION, and destroys
> the new clientid (no second mount).
> 
> We then get round robin use of the two NICs off one mount point all
> using a single session.
> 
> 
> Is using mount a good way to setup NFSv4.1 session trunking?
> 
> The first two patches fix issues with the multipath code.
> 
> SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net
>     This patch adds a flag to allow multipathing within the origial net.
>     I don't understand why this was not allowed, so I expect a different
>     solution for this issue.
> 
> 
> Comments please.
> 
> Thanks
> 
> -->Andy
> 
> Andy Adamson (6):
>   SUNRPC fix rpc_clnt_add_xprt setup return
>   SUNRPC fix rpc_clnt_xprt_iter_init infinite loop
>   SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net
>   NFS session trunking support: add trunk xprt to nfs4.1 mount
>   NFS add callback_ops to nfs4_proc_bind_conn_to_session_callback
>   NFS schedule bind_conn_to_session for new NFSv4.1 transport
> 
>  fs/nfs/client.c                      |  2 ++
>  fs/nfs/nfs4client.c                  | 29 +++++++++++++++++++++++++++--
>  fs/nfs/nfs4proc.c                    | 10 ++++++++++
>  include/linux/nfs_fs_sb.h            |  1 +
>  include/linux/sunrpc/clnt.h          |  1 +
>  include/linux/sunrpc/xprtmultipath.h |  6 +++++-
>  net/sunrpc/clnt.c                    | 20 +++++++++++++-------
>  net/sunrpc/xprtmultipath.c           | 35
>  +++++++++++++++++++++++++++++++++--
>  8 files changed, 92 insertions(+), 12 deletions(-)
> 
> --
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-- 
Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-707-0660
fax.  734-769-8938
cel.  734-216-5309

Re: [PATCH RFC Version 1 1/6] SUNRPC fix rpc_clnt_add_xprt setup return

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> Caller rpc_clnt_add_xprt() expects zero on success.
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  net/sunrpc/clnt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index efd6ffc..c0469d1 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2573,7 +2573,7 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>         if (IS_ERR(task))
>                 return PTR_ERR(task);
>         rpc_put_task(task);
> -       return 1;
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);


Which is why we return 1. :-)

The point of doing so is to ensure we do not add the xprt until the
connection has been tested.

Cheers
  Trond

Re: [PATCH RFC Version 1 3/6] SUNRPC: allow rpc_xprt_switch_add_xprt to add xprts on the same net

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> NFSv4.1 session trunking wants to add multiple xprts to the same net.

So does the original code. We never want to add xprts from different nets.

I'll fix the typo in the original patch.

Cheers
  Trond

Re: [PATCH RFC Version 1 4/6] NFS session trunking support: add trunk xprt to nfs4.1 mount

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4client.c        | 16 +++++++++++++++-
>  net/sunrpc/xprtmultipath.c |  2 ++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 5f20acf..6736805 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -412,8 +412,22 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
>         if (error < 0)
>                 goto error;
>
> -       if (clp != old)
> +       if (clp != old) {
> +               if (clp->cl_minorversion != 0) {
> +                       struct rpc_xprt *add = clp->cl_rpcclient->cl_xprt;
> +                       struct xprt_create xprt_args = {
> +                               .ident = XPRT_TRANSPORT_TCP,
> +                               .net = old->cl_net, /* clp->cl_net ??*/

We already know that old->cl_net == clp->cl_net. That has been
established in nfs41_walk_client_list().

> +                               .dstaddr = (struct sockaddr *)&add->addr,
> +                               .addrlen = add->addrlen,
> +                               .servername = clp->cl_hostname,
> +                       };
> +                       /* Add this address as an alias */
> +                       rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> +                                       rpc_clnt_test_and_add_xprt, NULL);

You have to ascertain that the server supports session trunking before
you can do this. nfs41_walk_client_list() establishes that 'old' and
'clp' have the same owner major ID, but it does nothing to ensure that
the minor IDs match, nor does it ensure that the scope is the same.

> +               }
>                 clp->cl_preserve_clid = true;
> +       }
>         nfs_put_client(clp);
>         return old;
>
> diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
> index 7be5bd2..f13419e 100644
> --- a/net/sunrpc/xprtmultipath.c
> +++ b/net/sunrpc/xprtmultipath.c
> @@ -35,6 +35,8 @@ static void xprt_switch_add_xprt_locked(struct rpc_xprt_switch *xps,
>         if (xps->xps_nxprts == 0)
>                 xps->xps_net = xprt->xprt_net;
>         xps->xps_nxprts++;
> +       pr_info("RPC:  ADDED %s to xps %p xps_nxprts %d\n",
> +               xprt->servername, xps, xps->xps_nxprts);

Doesn't belong in this patch.

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

Re: [PATCH RFC Version 1 6/6] NFS schedule bind_conn_to_session for new NFSv4.1 transport

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> Complete session trunking with the BIND_CONN_TO_SESSION call.
> NFS cleanup after session trunking transport added
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4client.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 6736805..b0423f7 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -423,8 +423,17 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
>                                 .servername = clp->cl_hostname,
>                         };
>                         /* Add this address as an alias */
> -                       rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> -                                       rpc_clnt_test_and_add_xprt, NULL);
> +                       error = rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> +                                            rpc_clnt_test_and_add_xprt, NULL);
> +                       if (!error) {
> +                               nfs4_schedule_session_recovery(old->cl_session,
> +                                       -NFS4ERR_CONN_NOT_BOUND_TO_SESSION);
> +
> +                               /** Cancel the 2nd mount */
> +                               pr_info("NFS:  Session trunk. CANCEL MOUNT\n");
> +                               error = -EINVAL;

You can't do this. What if we're not mounting the same path?

> +                               goto error;
> +                       }
>                 }
>                 clp->cl_preserve_clid = true;
>         }
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC Version 1 5/6] NFS add callback_ops to nfs4_proc_bind_conn_to_session_callback

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> Fix oops when NULL callback_ops pointer accessed in rpc_init_task
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4proc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index a2da473..ea07707 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6782,6 +6782,15 @@ nfs41_same_server_scope(struct nfs41_server_scope *a,
>         return false;
>  }
>
> +static void
> +nfs4_bind_one_conn_to_session_done(struct rpc_task *task, void *calldata)
> +{
> +}
> +
> +static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops = {
> +       .rpc_call_done =  &nfs4_bind_one_conn_to_session_done,
> +};
> +
>  /*
>   * nfs4_proc_bind_one_conn_to_session()
>   *
> @@ -6810,6 +6819,7 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
>         struct rpc_task_setup task_setup_data = {
>                 .rpc_client = clnt,
>                 .rpc_xprt = xprt,
> +               .callback_ops = &nfs4_bind_one_conn_to_session_ops,
>                 .rpc_message = &msg,
>                 .flags = RPC_TASK_TIMEOUT,
>         };

Thanks! Applied to the series.

Re: [PATCH RFC Version 1 2/6] SUNRPC fix rpc_clnt_xprt_iter_init infinite loop

[Trond Myklebust]

On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> xprt_iter_get_next() will restart at top of list and never stop.
> Only process xps_nxprts xprts.
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  net/sunrpc/clnt.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index c0469d1..a56a44c8 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -740,6 +740,7 @@ static
>  int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
>  {
>         struct rpc_xprt_switch *xps;
> +       int nxprt;
>
>         rcu_read_lock();
>         xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
> @@ -747,8 +748,9 @@ int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
>         if (xps == NULL)
>                 return -EAGAIN;
>         xprt_iter_init_listall(xpi, xps);
> +       nxprt = xps->xps_nxprts;
>         xprt_switch_put(xps);
> -       return 0;
> +       return nxprt;
>  }
>
>  /**
> @@ -767,10 +769,11 @@ int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
>                 void *data)
>  {
>         struct rpc_xprt_iter xpi;
> -       int ret;
> +       int ret = 0, nxprts, i;
>
> -       ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
> -       for (;;) {
> +       nxprts = rpc_clnt_xprt_iter_init(clnt, &xpi);
> +       dprintk("RPC:   processing nxprts %d\n", nxprts);
> +       for (i = 0; i < nxprts; i++) {
>                 struct rpc_xprt *xprt = xprt_iter_get_next(&xpi);
>
>                 if (!xprt)
> --

I'd prefer not to export xps->xps_nxprts. It's not guaranteed to
remain stable unless you hold the spinlock.
Please see the change to xprt_switch_set_next_cursor().

Re: [PATCH RFC Version 1 6/6] NFS schedule bind_conn_to_session for new NFSv4.1 transport

[Adamson, Andy]

DQo+IE9uIEZlYiA1LCAyMDE2LCBhdCA4OjA0IFBNLCBUcm9uZCBNeWtsZWJ1c3QgPHRyb25kLm15
a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20+IHdyb3RlOg0KPiANCj4gT24gRnJpLCBGZWIgNSwgMjAx
NiBhdCA0OjA4IFBNLCAgPGFuZHJvc0BuZXRhcHAuY29tPiB3cm90ZToNCj4+IEZyb206IEFuZHkg
QWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQo+PiANCj4+IENvbXBsZXRlIHNlc3Npb24gdHJ1
bmtpbmcgd2l0aCB0aGUgQklORF9DT05OX1RPX1NFU1NJT04gY2FsbC4NCj4+IE5GUyBjbGVhbnVw
IGFmdGVyIHNlc3Npb24gdHJ1bmtpbmcgdHJhbnNwb3J0IGFkZGVkDQo+PiANCj4+IFNpZ25lZC1v
ZmYtYnk6IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQo+PiAtLS0NCj4+IGZzL25m
cy9uZnM0Y2xpZW50LmMgfCAxMyArKysrKysrKysrKy0tDQo+PiAxIGZpbGUgY2hhbmdlZCwgMTEg
aW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkNCj4+IA0KPj4gZGlmZiAtLWdpdCBhL2ZzL25m
cy9uZnM0Y2xpZW50LmMgYi9mcy9uZnMvbmZzNGNsaWVudC5jDQo+PiBpbmRleCA2NzM2ODA1Li5i
MDQyM2Y3IDEwMDY0NA0KPj4gLS0tIGEvZnMvbmZzL25mczRjbGllbnQuYw0KPj4gKysrIGIvZnMv
bmZzL25mczRjbGllbnQuYw0KPj4gQEAgLTQyMyw4ICs0MjMsMTcgQEAgc3RydWN0IG5mc19jbGll
bnQgKm5mczRfaW5pdF9jbGllbnQoc3RydWN0IG5mc19jbGllbnQgKmNscCwNCj4+ICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAuc2VydmVybmFtZSA9IGNscC0+Y2xfaG9zdG5hbWUsDQo+
PiAgICAgICAgICAgICAgICAgICAgICAgIH07DQo+PiAgICAgICAgICAgICAgICAgICAgICAgIC8q
IEFkZCB0aGlzIGFkZHJlc3MgYXMgYW4gYWxpYXMgKi8NCj4+IC0gICAgICAgICAgICAgICAgICAg
ICAgIHJwY19jbG50X2FkZF94cHJ0KG9sZC0+Y2xfcnBjY2xpZW50LCAmeHBydF9hcmdzLA0KPj4g
LSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHJwY19jbG50X3Rlc3RfYW5k
X2FkZF94cHJ0LCBOVUxMKTsNCj4+ICsgICAgICAgICAgICAgICAgICAgICAgIGVycm9yID0gcnBj
X2NsbnRfYWRkX3hwcnQob2xkLT5jbF9ycGNjbGllbnQsICZ4cHJ0X2FyZ3MsDQo+PiArICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBycGNfY2xudF90ZXN0X2FuZF9h
ZGRfeHBydCwgTlVMTCk7DQo+PiArICAgICAgICAgICAgICAgICAgICAgICBpZiAoIWVycm9yKSB7
DQo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIG5mczRfc2NoZWR1bGVfc2Vzc2lv
bl9yZWNvdmVyeShvbGQtPmNsX3Nlc3Npb24sDQo+PiArICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgLU5GUzRFUlJfQ09OTl9OT1RfQk9VTkRfVE9fU0VTU0lPTik7DQo+PiAr
DQo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC8qKiBDYW5jZWwgdGhlIDJuZCBt
b3VudCAqLw0KPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBwcl9pbmZvKCJORlM6
ICBTZXNzaW9uIHRydW5rLiBDQU5DRUwgTU9VTlRcbiIpOw0KPj4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBlcnJvciA9IC1FSU5WQUw7DQo+IA0KPiBZb3UgY2FuJ3QgZG8gdGhpcy4g
V2hhdCBpZiB3ZSdyZSBub3QgbW91bnRpbmcgdGhlIHNhbWUgcGF0aD8NCg0KDQpUaGlzIGlzIHRo
ZSBxdWVzdGlvbiBJIGFza2VkIGluIHRoZSBjb3ZlciBsZXR0ZXI6IElzIHVzaW5nIG1vdW50IGEg
Z29vZCB3YXkgdG8gc2V0dXAgTkZTdjQuMSBzZXNzaW9uIHRydW5raW5nPw0KSG93IGRvIHlvdSBz
ZWUgc2Vzc2lvbiB0cnVua2luZyBiZWluZyByZXF1ZXN0ZWQ/IElzIG1vdW50aW5nIHRoZSBleHBv
cnQgcGF0aCBvbiB0aGUgc2VydmVyLCBidXQgYSBkaWZmZXJlbnQgbG9jYWwgbW91bnQgcG9pbnQg
Z29vZCBlbm91Z2g/DQoNClRoYW5rcw0KDQrigJQ+QW5keQ0KDQo+IA0KPj4gKyAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBnb3RvIGVycm9yOw0KPj4gKyAgICAgICAgICAgICAgICAgICAg
ICAgfQ0KPj4gICAgICAgICAgICAgICAgfQ0KPj4gICAgICAgICAgICAgICAgY2xwLT5jbF9wcmVz
ZXJ2ZV9jbGlkID0gdHJ1ZTsNCj4+ICAgICAgICB9DQo+PiAtLQ0KPj4gMS44LjMuMQ0KPj4gDQo+
PiAtLQ0KPj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVu
c3Vic2NyaWJlIGxpbnV4LW5mcyIgaW4NCj4+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpv
cmRvbW9Admdlci5rZXJuZWwub3JnDQo+PiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8v
dmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCg0K

Re: [PATCH RFC Version 1 6/6] NFS schedule bind_conn_to_session for new NFSv4.1 transport

[Trond Myklebust]

On Mon, Feb 8, 2016 at 12:48 PM, Adamson, Andy
<William.Adamson@netapp.com> wrote:
>
>
> > On Feb 5, 2016, at 8:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> >
> > On Fri, Feb 5, 2016 at 4:08 PM,  <andros@netapp.com> wrote:
> >> From: Andy Adamson <andros@netapp.com>
> >>
> >> Complete session trunking with the BIND_CONN_TO_SESSION call.
> >> NFS cleanup after session trunking transport added
> >>
> >> Signed-off-by: Andy Adamson <andros@netapp.com>
> >> ---
> >> fs/nfs/nfs4client.c | 13 +++++++++++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> >> index 6736805..b0423f7 100644
> >> --- a/fs/nfs/nfs4client.c
> >> +++ b/fs/nfs/nfs4client.c
> >> @@ -423,8 +423,17 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
> >>                                .servername = clp->cl_hostname,
> >>                        };
> >>                        /* Add this address as an alias */
> >> -                       rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> >> -                                       rpc_clnt_test_and_add_xprt, NULL);
> >> +                       error = rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> >> +                                            rpc_clnt_test_and_add_xprt, NULL);
> >> +                       if (!error) {
> >> +                               nfs4_schedule_session_recovery(old->cl_session,
> >> +                                       -NFS4ERR_CONN_NOT_BOUND_TO_SESSION);
> >> +
> >> +                               /** Cancel the 2nd mount */
> >> +                               pr_info("NFS:  Session trunk. CANCEL MOUNT\n");
> >> +                               error = -EINVAL;
> >
> > You can't do this. What if we're not mounting the same path?
>
>
> This is the question I asked in the cover letter: Is using mount a good way to setup NFSv4.1 session trunking?
> How do you see session trunking being requested? Is mounting the export path on the server, but a different local mount point good enough?

The "standard" way of doing it is to supply multiple hostnames, in other words:

mount -t nfs foo,bar,baz:/export /mnt

We can then either supply the network addresses for foo, bar and baz
in the form of multiple 'addr=<address>' options (and possibly
prefixed by 'proto=<netid>' if necessary), or we could have the DNS
resolver do the translation for us.

Cheers
  Trond

>
> Thanks
>
> —>Andy
>
> >
> >> +                               goto error;
> >> +                       }
> >>                }
> >>                clp->cl_preserve_clid = true;
> >>        }
> >> --
> >> 1.8.3.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>