[PATCH v1 0/8] sysfs files for multipath transport control

[PATCH v1 0/8] sysfs files for multipath transport control

[Dan Aloni]

Hi Anna,

This patchset builds ontop v2 of your 'sysfs files for changing IP' changeset.

- The patchset adds two more sysfs objects, for one for transport and another
  for multipath.
- Also, `net` renamed to `client`, and `client` now has symlink to its principal
  transport. A client also has a symlink to its `multipath` object.
- The transport interface lets you change `dstaddr` of individual transports,
  when `nconnect` is used (or if it wasn't used and these were added with the
  new interface).
- The interface to add a transport is using a single string written to 'add',
  for example:

       echo 'dstaddr 192.168.40.8 kind rdma' \
               > /sys/kernel/sunrpc/client/0/multipath/add

These changes are independent of the method used to obtain a sunrpc ID for a
mountpoint. For that I've sent a concept patch showing an fspick-based
implementation: https://marc.info/?l=linux-nfs&m=161332454821849&w=4

Thanks

Dan Aloni (8):
  sunrpc: rename 'net' to 'client'
  sunrpc: add xprt id
  sunrpc: add a directory per sunrpc xprt
  sunrpc: have client directory a symlink to the root transport
  sunrpc: add IDs to multipath
  sunrpc: add multipath directory and symlink from client
  sunrpc: change rpc_clnt_add_xprt() to rpc_add_xprt()
  sunrpc: introduce an 'add' node to 'multipath' sysfs directory

 fs/nfs/pnfs_nfs.c                    |  12 +-
 include/linux/sunrpc/clnt.h          |  12 +-
 include/linux/sunrpc/xprt.h          |   3 +
 include/linux/sunrpc/xprtmultipath.h |   6 +
 net/sunrpc/clnt.c                    |  39 +--
 net/sunrpc/sunrpc_syms.c             |   2 +
 net/sunrpc/sysfs.c                   | 403 +++++++++++++++++++++++----
 net/sunrpc/sysfs.h                   |  21 +-
 net/sunrpc/xprt.c                    |  29 ++
 net/sunrpc/xprtmultipath.c           |  37 +++
 10 files changed, 487 insertions(+), 77 deletions(-)

-- 
2.26.2

[PATCH v1 1/8] sunrpc: rename 'net' to 'client'

[Dan Aloni]

This is in preparation to adding a second directory to keep track
of each transport.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 net/sunrpc/clnt.c  |  8 ++++----
 net/sunrpc/sysfs.c | 22 +++++++++++-----------
 net/sunrpc/sysfs.h |  4 ++--
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 02905eae5c0a..0a4811be01cd 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -301,7 +301,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
 	int err;
 
 	rpc_clnt_debugfs_register(clnt);
-	rpc_netns_sysfs_setup(clnt, net);
+	rpc_netns_client_sysfs_setup(clnt, net);
 
 	pipefs_sb = rpc_get_sb_net(net);
 	if (pipefs_sb) {
@@ -329,7 +329,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
 out:
 	if (pipefs_sb)
 		rpc_put_sb_net(net);
-	rpc_netns_sysfs_destroy(clnt);
+	rpc_netns_client_sysfs_destroy(clnt);
 	rpc_clnt_debugfs_unregister(clnt);
 	return err;
 }
@@ -736,7 +736,7 @@ int rpc_switch_client_transport(struct rpc_clnt *clnt,
 
 	rpc_unregister_client(clnt);
 	__rpc_clnt_remove_pipedir(clnt);
-	rpc_netns_sysfs_destroy(clnt);
+	rpc_netns_client_sysfs_destroy(clnt);
 	rpc_clnt_debugfs_unregister(clnt);
 
 	/*
@@ -883,7 +883,7 @@ static void rpc_free_client_work(struct work_struct *work)
 	 * so they cannot be called in rpciod, so they are handled separately
 	 * here.
 	 */
-	rpc_netns_sysfs_destroy(clnt);
+	rpc_netns_client_sysfs_destroy(clnt);
 	rpc_clnt_debugfs_unregister(clnt);
 	rpc_free_clid(clnt);
 	rpc_clnt_remove_pipedir(clnt);
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 8b01b4df64ee..3fe814795ed9 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -9,7 +9,7 @@
 #include "sysfs.h"
 
 struct kobject *rpc_client_kobj;
-static struct kset *rpc_client_kset;
+static struct kset *rpc_sunrpc_kset;
 
 static void rpc_netns_object_release(struct kobject *kobj)
 {
@@ -45,13 +45,13 @@ static struct kobject *rpc_netns_object_alloc(const char *name,
 
 int rpc_sysfs_init(void)
 {
-	rpc_client_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
-	if (!rpc_client_kset)
+	rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
+	if (!rpc_sunrpc_kset)
 		return -ENOMEM;
-	rpc_client_kobj = rpc_netns_object_alloc("net", rpc_client_kset, NULL);
+	rpc_client_kobj = rpc_netns_object_alloc("client", rpc_sunrpc_kset, NULL);
 	if (!rpc_client_kobj) {
-		kset_unregister(rpc_client_kset);
-		rpc_client_kset = NULL;
+		kset_unregister(rpc_sunrpc_kset);
+		rpc_sunrpc_kset = NULL;
 		return -ENOMEM;
 	}
 	return 0;
@@ -119,18 +119,18 @@ static struct kobj_type rpc_netns_client_type = {
 void rpc_sysfs_exit(void)
 {
 	kobject_put(rpc_client_kobj);
-	kset_unregister(rpc_client_kset);
+	kset_unregister(rpc_sunrpc_kset);
 }
 
 static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
-						struct net *net, int clid)
+						       struct net *net, int clid)
 {
 	struct rpc_netns_client *p;
 
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (p) {
 		p->net = net;
-		p->kobject.kset = rpc_client_kset;
+		p->kobject.kset = rpc_sunrpc_kset;
 		if (kobject_init_and_add(&p->kobject, &rpc_netns_client_type,
 					parent, "%d", clid) == 0)
 			return p;
@@ -139,7 +139,7 @@ static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
 	return NULL;
 }
 
-void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
+void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
 {
 	struct rpc_netns_client *rpc_client;
 	struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
@@ -155,7 +155,7 @@ void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
 	}
 }
 
-void rpc_netns_sysfs_destroy(struct rpc_clnt *clnt)
+void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
 {
 	struct rpc_netns_client *rpc_client = clnt->cl_sysfs;
 
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
index 137a12c87954..ab75c3cc91b6 100644
--- a/net/sunrpc/sysfs.h
+++ b/net/sunrpc/sysfs.h
@@ -16,7 +16,7 @@ extern struct kobject *rpc_client_kobj;
 extern int rpc_sysfs_init(void);
 extern void rpc_sysfs_exit(void);
 
-void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
-void rpc_netns_sysfs_destroy(struct rpc_clnt *clnt);
+void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
+void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt);
 
 #endif
-- 
2.26.2

Re: [PATCH v1 1/8] sunrpc: rename 'net' to 'client'

[Anna Schumaker]

Hi Dan,

On Mon, Feb 15, 2021 at 12:42 PM Dan Aloni <dan@kernelim.com> wrote:
>
> This is in preparation to adding a second directory to keep track
> of each transport.

I initially named the directory "net" to match how NFS's sysfs
directories are created (/sys/fs/nfs/net). If naming it "client" makes
more sense then I should probably introduce it that way in my patches.

Thoughts?
Anna

>
> Signed-off-by: Dan Aloni <dan@kernelim.com>
> ---
>  net/sunrpc/clnt.c  |  8 ++++----
>  net/sunrpc/sysfs.c | 22 +++++++++++-----------
>  net/sunrpc/sysfs.h |  4 ++--
>  3 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 02905eae5c0a..0a4811be01cd 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -301,7 +301,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
>         int err;
>
>         rpc_clnt_debugfs_register(clnt);
> -       rpc_netns_sysfs_setup(clnt, net);
> +       rpc_netns_client_sysfs_setup(clnt, net);
>
>         pipefs_sb = rpc_get_sb_net(net);
>         if (pipefs_sb) {
> @@ -329,7 +329,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
>  out:
>         if (pipefs_sb)
>                 rpc_put_sb_net(net);
> -       rpc_netns_sysfs_destroy(clnt);
> +       rpc_netns_client_sysfs_destroy(clnt);
>         rpc_clnt_debugfs_unregister(clnt);
>         return err;
>  }
> @@ -736,7 +736,7 @@ int rpc_switch_client_transport(struct rpc_clnt *clnt,
>
>         rpc_unregister_client(clnt);
>         __rpc_clnt_remove_pipedir(clnt);
> -       rpc_netns_sysfs_destroy(clnt);
> +       rpc_netns_client_sysfs_destroy(clnt);
>         rpc_clnt_debugfs_unregister(clnt);
>
>         /*
> @@ -883,7 +883,7 @@ static void rpc_free_client_work(struct work_struct *work)
>          * so they cannot be called in rpciod, so they are handled separately
>          * here.
>          */
> -       rpc_netns_sysfs_destroy(clnt);
> +       rpc_netns_client_sysfs_destroy(clnt);
>         rpc_clnt_debugfs_unregister(clnt);
>         rpc_free_clid(clnt);
>         rpc_clnt_remove_pipedir(clnt);
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index 8b01b4df64ee..3fe814795ed9 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -9,7 +9,7 @@
>  #include "sysfs.h"
>
>  struct kobject *rpc_client_kobj;
> -static struct kset *rpc_client_kset;
> +static struct kset *rpc_sunrpc_kset;
>
>  static void rpc_netns_object_release(struct kobject *kobj)
>  {
> @@ -45,13 +45,13 @@ static struct kobject *rpc_netns_object_alloc(const char *name,
>
>  int rpc_sysfs_init(void)
>  {
> -       rpc_client_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
> -       if (!rpc_client_kset)
> +       rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
> +       if (!rpc_sunrpc_kset)
>                 return -ENOMEM;
> -       rpc_client_kobj = rpc_netns_object_alloc("net", rpc_client_kset, NULL);
> +       rpc_client_kobj = rpc_netns_object_alloc("client", rpc_sunrpc_kset, NULL);
>         if (!rpc_client_kobj) {
> -               kset_unregister(rpc_client_kset);
> -               rpc_client_kset = NULL;
> +               kset_unregister(rpc_sunrpc_kset);
> +               rpc_sunrpc_kset = NULL;
>                 return -ENOMEM;
>         }
>         return 0;
> @@ -119,18 +119,18 @@ static struct kobj_type rpc_netns_client_type = {
>  void rpc_sysfs_exit(void)
>  {
>         kobject_put(rpc_client_kobj);
> -       kset_unregister(rpc_client_kset);
> +       kset_unregister(rpc_sunrpc_kset);
>  }
>
>  static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
> -                                               struct net *net, int clid)
> +                                                      struct net *net, int clid)
>  {
>         struct rpc_netns_client *p;
>
>         p = kzalloc(sizeof(*p), GFP_KERNEL);
>         if (p) {
>                 p->net = net;
> -               p->kobject.kset = rpc_client_kset;
> +               p->kobject.kset = rpc_sunrpc_kset;
>                 if (kobject_init_and_add(&p->kobject, &rpc_netns_client_type,
>                                         parent, "%d", clid) == 0)
>                         return p;
> @@ -139,7 +139,7 @@ static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
>         return NULL;
>  }
>
> -void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
> +void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
>  {
>         struct rpc_netns_client *rpc_client;
>         struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
> @@ -155,7 +155,7 @@ void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
>         }
>  }
>
> -void rpc_netns_sysfs_destroy(struct rpc_clnt *clnt)
> +void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
>  {
>         struct rpc_netns_client *rpc_client = clnt->cl_sysfs;
>
> diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
> index 137a12c87954..ab75c3cc91b6 100644
> --- a/net/sunrpc/sysfs.h
> +++ b/net/sunrpc/sysfs.h
> @@ -16,7 +16,7 @@ extern struct kobject *rpc_client_kobj;
>  extern int rpc_sysfs_init(void);
>  extern void rpc_sysfs_exit(void);
>
> -void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
> -void rpc_netns_sysfs_destroy(struct rpc_clnt *clnt);
> +void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
> +void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt);
>
>  #endif
> --
> 2.26.2
>

Re: [PATCH v1 1/8] sunrpc: rename 'net' to 'client'

[Dan Aloni]

On Tue, Feb 16, 2021 at 04:24:41PM -0500, Anna Schumaker wrote:
> Hi Dan,
> 
> On Mon, Feb 15, 2021 at 12:42 PM Dan Aloni <dan@kernelim.com> wrote:
> >
> > This is in preparation to adding a second directory to keep track
> > of each transport.
> 
> I initially named the directory "net" to match how NFS's sysfs
> directories are created (/sys/fs/nfs/net). If naming it "client" makes
> more sense then I should probably introduce it that way in my patches.
> 
> Thoughts?

We can keep the "net" directory under "sunrpc" and have the three
directories under it, i.e:

     /sys/kernel/sunrpc/net/client
     /sys/kernel/sunrpc/net/transport
     /sys/kernel/sunrpc/net/multipath

It works for me too, but I no preference either way.

-- 
Dan Aloni

[PATCH v1 2/8] sunrpc: add xprt id

[Dan Aloni]

This adds a unique identifier for a sunrpc transport in sysfs, which is
similarly managed to the unique IDs of clients.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 include/linux/sunrpc/xprt.h |  2 ++
 net/sunrpc/sunrpc_syms.c    |  1 +
 net/sunrpc/xprt.c           | 26 ++++++++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index d2e97ee802af..fbf57a87dc47 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -185,6 +185,7 @@ enum xprt_transports {
 struct rpc_xprt {
 	struct kref		kref;		/* Reference count */
 	const struct rpc_xprt_ops *ops;		/* transport methods */
+	unsigned int		id;		/* transport id */
 
 	const struct rpc_timeout *timeout;	/* timeout parms */
 	struct sockaddr_storage	addr;		/* server address */
@@ -367,6 +368,7 @@ struct rpc_xprt *	xprt_alloc(struct net *net, size_t size,
 				unsigned int num_prealloc,
 				unsigned int max_req);
 void			xprt_free(struct rpc_xprt *);
+void			xprt_cleanup_ids(void);
 
 static inline int
 xprt_enable_swap(struct rpc_xprt *xprt)
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index 3b57efc692ec..b61b74c00483 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -133,6 +133,7 @@ cleanup_sunrpc(void)
 {
 	rpc_sysfs_exit();
 	rpc_cleanup_clids();
+	xprt_cleanup_ids();
 	rpcauth_remove_module();
 	cleanup_socket_xprt();
 	svc_cleanup_xprt_sock();
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 691ccf8049a4..e30acd1f0e31 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1717,6 +1717,30 @@ static void xprt_free_all_slots(struct rpc_xprt *xprt)
 	}
 }
 
+static DEFINE_IDA(rpc_xprt_ids);
+
+void xprt_cleanup_ids(void)
+{
+	ida_destroy(&rpc_xprt_ids);
+}
+
+static int xprt_alloc_id(struct rpc_xprt *xprt)
+{
+	int id;
+
+	id = ida_simple_get(&rpc_xprt_ids, 0, 0, GFP_KERNEL);
+	if (id < 0)
+		return id;
+
+	xprt->id = id;
+	return 0;
+}
+
+static void xprt_free_id(struct rpc_xprt *xprt)
+{
+	ida_simple_remove(&rpc_xprt_ids, xprt->id);
+}
+
 struct rpc_xprt *xprt_alloc(struct net *net, size_t size,
 		unsigned int num_prealloc,
 		unsigned int max_alloc)
@@ -1729,6 +1753,7 @@ struct rpc_xprt *xprt_alloc(struct net *net, size_t size,
 	if (xprt == NULL)
 		goto out;
 
+	xprt_alloc_id(xprt);
 	xprt_init(xprt, net);
 
 	for (i = 0; i < num_prealloc; i++) {
@@ -1757,6 +1782,7 @@ void xprt_free(struct rpc_xprt *xprt)
 {
 	put_net(xprt->xprt_net);
 	xprt_free_all_slots(xprt);
+	xprt_free_id(xprt);
 	kfree_rcu(xprt, rcu);
 }
 EXPORT_SYMBOL_GPL(xprt_free);
-- 
2.26.2

[PATCH v1 3/8] sunrpc: add a directory per sunrpc xprt

[Dan Aloni]

This uses much of the code from the per-client directory, except that
now we have direct access to the transport struct. The per-client
direct is adjusted in a subsequent commit.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 include/linux/sunrpc/xprt.h |   1 +
 net/sunrpc/sysfs.c          | 131 ++++++++++++++++++++++++++++++++++--
 net/sunrpc/sysfs.h          |   9 +++
 net/sunrpc/xprt.c           |   3 +
 4 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index fbf57a87dc47..df0252de58f4 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -260,6 +260,7 @@ struct rpc_xprt {
 						 * items */
 	struct list_head	bc_pa_list;	/* List of preallocated
 						 * backchannel rpc_rqst's */
+	void			*sysfs;         /* /sys/kernel/sunrpc/xprt/<id> */
 #endif /* CONFIG_SUNRPC_BACKCHANNEL */
 
 	struct rb_root		recv_queue;	/* Receive queue */
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 3fe814795ed9..687d4470b90d 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -9,6 +9,7 @@
 #include "sysfs.h"
 
 struct kobject *rpc_client_kobj;
+struct kobject *rpc_xprt_kobj;
 static struct kset *rpc_sunrpc_kset;
 
 static void rpc_netns_object_release(struct kobject *kobj)
@@ -48,13 +49,24 @@ int rpc_sysfs_init(void)
 	rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
 	if (!rpc_sunrpc_kset)
 		return -ENOMEM;
+
 	rpc_client_kobj = rpc_netns_object_alloc("client", rpc_sunrpc_kset, NULL);
-	if (!rpc_client_kobj) {
-		kset_unregister(rpc_sunrpc_kset);
-		rpc_sunrpc_kset = NULL;
-		return -ENOMEM;
-	}
+	if (!rpc_client_kobj)
+		goto err_kset;
+
+	rpc_xprt_kobj = rpc_netns_object_alloc("transport", rpc_sunrpc_kset, NULL);
+	if (!rpc_xprt_kobj)
+		goto err_client;
+
 	return 0;
+
+err_client:
+	kobject_put(rpc_client_kobj);
+	rpc_client_kobj = NULL;
+err_kset:
+	kset_unregister(rpc_sunrpc_kset);
+	rpc_sunrpc_kset = NULL;
+	return -ENOMEM;
 }
 
 static ssize_t rpc_netns_dstaddr_show(struct kobject *kobj,
@@ -118,6 +130,7 @@ static struct kobj_type rpc_netns_client_type = {
 
 void rpc_sysfs_exit(void)
 {
+	kobject_put(rpc_xprt_kobj);
 	kobject_put(rpc_client_kobj);
 	kset_unregister(rpc_sunrpc_kset);
 }
@@ -166,3 +179,111 @@ void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
 		clnt->cl_sysfs = NULL;
 	}
 }
+
+static ssize_t rpc_netns_xprt_dstaddr_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct rpc_netns_xprt *c = container_of(kobj,
+				struct rpc_netns_xprt, kobject);
+	struct rpc_xprt *xprt = c->xprt;
+
+	if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA))) {
+		sprintf(buf, "N/A");
+		return 0;
+	}
+
+	return rpc_ntop((struct sockaddr *)&xprt->addr, buf, PAGE_SIZE);
+}
+
+static ssize_t rpc_netns_xprt_dstaddr_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	struct rpc_netns_xprt *c = container_of(kobj,
+				struct rpc_netns_xprt, kobject);
+	struct rpc_xprt *xprt = c->xprt;
+	struct sockaddr *saddr = (struct sockaddr *)&xprt->addr;
+	int port;
+
+	if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA)))
+		return -EINVAL;
+
+	port = rpc_get_port(saddr);
+
+	xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr, sizeof(*saddr));
+	rpc_set_port(saddr, port);
+
+	kfree(xprt->address_strings[RPC_DISPLAY_ADDR]);
+	xprt->address_strings[RPC_DISPLAY_ADDR] = kstrndup(buf, count - 1, GFP_KERNEL);
+
+	xprt->ops->connect(xprt, NULL);
+	return count;
+}
+
+static void rpc_netns_xprt_release(struct kobject *kobj)
+{
+	struct rpc_netns_xprt *c;
+
+	c = container_of(kobj, struct rpc_netns_xprt, kobject);
+	kfree(c);
+}
+
+static const void *rpc_netns_xprt_namespace(struct kobject *kobj)
+{
+	return container_of(kobj, struct rpc_netns_xprt, kobject)->net;
+}
+
+static struct kobj_attribute rpc_netns_xprt_dstaddr = __ATTR(dstaddr,
+		0644, rpc_netns_xprt_dstaddr_show, rpc_netns_xprt_dstaddr_store);
+
+static struct attribute *rpc_netns_xprt_attrs[] = {
+	&rpc_netns_xprt_dstaddr.attr,
+	NULL,
+};
+
+static struct kobj_type rpc_netns_xprt_type = {
+	.release = rpc_netns_xprt_release,
+	.default_attrs = rpc_netns_xprt_attrs,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.namespace = rpc_netns_xprt_namespace,
+};
+
+static struct rpc_netns_xprt *rpc_netns_xprt_alloc(struct kobject *parent,
+						   struct net *net, int id)
+{
+	struct rpc_netns_xprt *p;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (p) {
+		p->net = net;
+		p->kobject.kset = rpc_sunrpc_kset;
+		if (kobject_init_and_add(&p->kobject, &rpc_netns_xprt_type,
+					 parent, "%d", id) == 0)
+			return p;
+		kobject_put(&p->kobject);
+	}
+	return NULL;
+}
+
+void rpc_netns_xprt_sysfs_setup(struct rpc_xprt *xprt, struct net *net)
+{
+	struct rpc_netns_xprt *rpc_xprt;
+
+	rpc_xprt = rpc_netns_xprt_alloc(rpc_xprt_kobj, net, xprt->id);
+	if (rpc_xprt) {
+		xprt->sysfs = rpc_xprt;
+		rpc_xprt->xprt = xprt;
+		kobject_uevent(&rpc_xprt->kobject, KOBJ_ADD);
+	}
+}
+
+void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt)
+{
+	struct rpc_netns_xprt *rpc_xprt = xprt->sysfs;
+
+	if (rpc_xprt) {
+		kobject_uevent(&rpc_xprt->kobject, KOBJ_REMOVE);
+		kobject_del(&rpc_xprt->kobject);
+		kobject_put(&rpc_xprt->kobject);
+		xprt->sysfs = NULL;
+	}
+}
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
index ab75c3cc91b6..e08dd7f6a1ec 100644
--- a/net/sunrpc/sysfs.h
+++ b/net/sunrpc/sysfs.h
@@ -11,12 +11,21 @@ struct rpc_netns_client {
 	struct rpc_clnt *clnt;
 };
 
+struct rpc_netns_xprt {
+	struct kobject kobject;
+	struct net *net;
+	struct rpc_xprt *xprt;
+};
+
 extern struct kobject *rpc_client_kobj;
+extern struct kobject *rpc_xprt_kobj;
 
 extern int rpc_sysfs_init(void);
 extern void rpc_sysfs_exit(void);
 
 void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
 void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt);
+void rpc_netns_xprt_sysfs_setup(struct rpc_xprt *xprt, struct net *net);
+void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt);
 
 #endif
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index e30acd1f0e31..4098cb6b1453 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -55,6 +55,7 @@
 #include <trace/events/sunrpc.h>
 
 #include "sunrpc.h"
+#include "sysfs.h"
 
 /*
  * Local variables
@@ -1768,6 +1769,7 @@ struct rpc_xprt *xprt_alloc(struct net *net, size_t size,
 		xprt->max_reqs = num_prealloc;
 	xprt->min_reqs = num_prealloc;
 	xprt->num_reqs = num_prealloc;
+	rpc_netns_xprt_sysfs_setup(xprt, net);
 
 	return xprt;
 
@@ -1780,6 +1782,7 @@ EXPORT_SYMBOL_GPL(xprt_alloc);
 
 void xprt_free(struct rpc_xprt *xprt)
 {
+	rpc_netns_xprt_sysfs_destroy(xprt);
 	put_net(xprt->xprt_net);
 	xprt_free_all_slots(xprt);
 	xprt_free_id(xprt);
-- 
2.26.2

Re: [PATCH v1 3/8] sunrpc: add a directory per sunrpc xprt

[Anna Schumaker]

Hi Dan,

On Mon, Feb 15, 2021 at 12:42 PM Dan Aloni <dan@kernelim.com> wrote:
>
> This uses much of the code from the per-client directory, except that
> now we have direct access to the transport struct. The per-client
> direct is adjusted in a subsequent commit.

Just a heads up that I've changed the per-client functions a bit, so
you'll probably need to adjust for that once I post v3 of my patches.


>
> Signed-off-by: Dan Aloni <dan@kernelim.com>
> ---
>  include/linux/sunrpc/xprt.h |   1 +
>  net/sunrpc/sysfs.c          | 131 ++++++++++++++++++++++++++++++++++--
>  net/sunrpc/sysfs.h          |   9 +++
>  net/sunrpc/xprt.c           |   3 +
>  4 files changed, 139 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> index fbf57a87dc47..df0252de58f4 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -260,6 +260,7 @@ struct rpc_xprt {
>                                                  * items */
>         struct list_head        bc_pa_list;     /* List of preallocated
>                                                  * backchannel rpc_rqst's */
> +       void                    *sysfs;         /* /sys/kernel/sunrpc/xprt/<id> */
>  #endif /* CONFIG_SUNRPC_BACKCHANNEL */
>
>         struct rb_root          recv_queue;     /* Receive queue */
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index 3fe814795ed9..687d4470b90d 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -9,6 +9,7 @@
>  #include "sysfs.h"
>
>  struct kobject *rpc_client_kobj;
> +struct kobject *rpc_xprt_kobj;
>  static struct kset *rpc_sunrpc_kset;
>
>  static void rpc_netns_object_release(struct kobject *kobj)
> @@ -48,13 +49,24 @@ int rpc_sysfs_init(void)
>         rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
>         if (!rpc_sunrpc_kset)
>                 return -ENOMEM;
> +
>         rpc_client_kobj = rpc_netns_object_alloc("client", rpc_sunrpc_kset, NULL);
> -       if (!rpc_client_kobj) {
> -               kset_unregister(rpc_sunrpc_kset);
> -               rpc_sunrpc_kset = NULL;
> -               return -ENOMEM;
> -       }
> +       if (!rpc_client_kobj)
> +               goto err_kset;
> +
> +       rpc_xprt_kobj = rpc_netns_object_alloc("transport", rpc_sunrpc_kset, NULL);
> +       if (!rpc_xprt_kobj)
> +               goto err_client;
> +
>         return 0;
> +
> +err_client:
> +       kobject_put(rpc_client_kobj);
> +       rpc_client_kobj = NULL;
> +err_kset:
> +       kset_unregister(rpc_sunrpc_kset);
> +       rpc_sunrpc_kset = NULL;
> +       return -ENOMEM;
>  }
>
>  static ssize_t rpc_netns_dstaddr_show(struct kobject *kobj,
> @@ -118,6 +130,7 @@ static struct kobj_type rpc_netns_client_type = {
>
>  void rpc_sysfs_exit(void)
>  {
> +       kobject_put(rpc_xprt_kobj);
>         kobject_put(rpc_client_kobj);
>         kset_unregister(rpc_sunrpc_kset);
>  }
> @@ -166,3 +179,111 @@ void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
>                 clnt->cl_sysfs = NULL;
>         }
>  }
> +
> +static ssize_t rpc_netns_xprt_dstaddr_show(struct kobject *kobj,
> +               struct kobj_attribute *attr, char *buf)
> +{
> +       struct rpc_netns_xprt *c = container_of(kobj,
> +                               struct rpc_netns_xprt, kobject);
> +       struct rpc_xprt *xprt = c->xprt;
> +
> +       if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA))) {

We might want to change these restrictions later on, so if we're going
to put this into each function then maybe it would make sense to have
a quick inline to check protocol support?

I do the same check in the setup function for my patches, so if you
want I can add the inline function and then it'll just be there for
you to use.

> +               sprintf(buf, "N/A");
> +               return 0;

I'm guessing the point of putting "N/A" here is so userspace tools
don't have to guess which files exist or not for each protocol type?
Should I change my patches to match this style too?

Anna

> +       }
> +
> +       return rpc_ntop((struct sockaddr *)&xprt->addr, buf, PAGE_SIZE);
> +}
> +
> +static ssize_t rpc_netns_xprt_dstaddr_store(struct kobject *kobj,
> +               struct kobj_attribute *attr, const char *buf, size_t count)
> +{
> +       struct rpc_netns_xprt *c = container_of(kobj,
> +                               struct rpc_netns_xprt, kobject);
> +       struct rpc_xprt *xprt = c->xprt;
> +       struct sockaddr *saddr = (struct sockaddr *)&xprt->addr;
> +       int port;
> +
> +       if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA)))
> +               return -EINVAL;
> +
> +       port = rpc_get_port(saddr);
> +
> +       xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr, sizeof(*saddr));
> +       rpc_set_port(saddr, port);
> +
> +       kfree(xprt->address_strings[RPC_DISPLAY_ADDR]);
> +       xprt->address_strings[RPC_DISPLAY_ADDR] = kstrndup(buf, count - 1, GFP_KERNEL);
> +
> +       xprt->ops->connect(xprt, NULL);
> +       return count;
> +}
> +
> +static void rpc_netns_xprt_release(struct kobject *kobj)
> +{
> +       struct rpc_netns_xprt *c;
> +
> +       c = container_of(kobj, struct rpc_netns_xprt, kobject);
> +       kfree(c);
> +}
> +
> +static const void *rpc_netns_xprt_namespace(struct kobject *kobj)
> +{
> +       return container_of(kobj, struct rpc_netns_xprt, kobject)->net;
> +}
> +
> +static struct kobj_attribute rpc_netns_xprt_dstaddr = __ATTR(dstaddr,
> +               0644, rpc_netns_xprt_dstaddr_show, rpc_netns_xprt_dstaddr_store);
> +
> +static struct attribute *rpc_netns_xprt_attrs[] = {
> +       &rpc_netns_xprt_dstaddr.attr,
> +       NULL,
> +};
> +
> +static struct kobj_type rpc_netns_xprt_type = {
> +       .release = rpc_netns_xprt_release,
> +       .default_attrs = rpc_netns_xprt_attrs,
> +       .sysfs_ops = &kobj_sysfs_ops,
> +       .namespace = rpc_netns_xprt_namespace,
> +};
> +
> +static struct rpc_netns_xprt *rpc_netns_xprt_alloc(struct kobject *parent,
> +                                                  struct net *net, int id)
> +{
> +       struct rpc_netns_xprt *p;
> +
> +       p = kzalloc(sizeof(*p), GFP_KERNEL);
> +       if (p) {
> +               p->net = net;
> +               p->kobject.kset = rpc_sunrpc_kset;
> +               if (kobject_init_and_add(&p->kobject, &rpc_netns_xprt_type,
> +                                        parent, "%d", id) == 0)
> +                       return p;
> +               kobject_put(&p->kobject);
> +       }
> +       return NULL;
> +}
> +
> +void rpc_netns_xprt_sysfs_setup(struct rpc_xprt *xprt, struct net *net)
> +{
> +       struct rpc_netns_xprt *rpc_xprt;
> +
> +       rpc_xprt = rpc_netns_xprt_alloc(rpc_xprt_kobj, net, xprt->id);
> +       if (rpc_xprt) {
> +               xprt->sysfs = rpc_xprt;
> +               rpc_xprt->xprt = xprt;
> +               kobject_uevent(&rpc_xprt->kobject, KOBJ_ADD);
> +       }
> +}
> +
> +void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt)
> +{
> +       struct rpc_netns_xprt *rpc_xprt = xprt->sysfs;
> +
> +       if (rpc_xprt) {
> +               kobject_uevent(&rpc_xprt->kobject, KOBJ_REMOVE);
> +               kobject_del(&rpc_xprt->kobject);
> +               kobject_put(&rpc_xprt->kobject);
> +               xprt->sysfs = NULL;
> +       }
> +}
> diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
> index ab75c3cc91b6..e08dd7f6a1ec 100644
> --- a/net/sunrpc/sysfs.h
> +++ b/net/sunrpc/sysfs.h
> @@ -11,12 +11,21 @@ struct rpc_netns_client {
>         struct rpc_clnt *clnt;
>  };
>
> +struct rpc_netns_xprt {
> +       struct kobject kobject;
> +       struct net *net;
> +       struct rpc_xprt *xprt;
> +};
> +
>  extern struct kobject *rpc_client_kobj;
> +extern struct kobject *rpc_xprt_kobj;
>
>  extern int rpc_sysfs_init(void);
>  extern void rpc_sysfs_exit(void);
>
>  void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
>  void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt);
> +void rpc_netns_xprt_sysfs_setup(struct rpc_xprt *xprt, struct net *net);
> +void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt);
>
>  #endif
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index e30acd1f0e31..4098cb6b1453 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -55,6 +55,7 @@
>  #include <trace/events/sunrpc.h>
>
>  #include "sunrpc.h"
> +#include "sysfs.h"
>
>  /*
>   * Local variables
> @@ -1768,6 +1769,7 @@ struct rpc_xprt *xprt_alloc(struct net *net, size_t size,
>                 xprt->max_reqs = num_prealloc;
>         xprt->min_reqs = num_prealloc;
>         xprt->num_reqs = num_prealloc;
> +       rpc_netns_xprt_sysfs_setup(xprt, net);
>
>         return xprt;
>
> @@ -1780,6 +1782,7 @@ EXPORT_SYMBOL_GPL(xprt_alloc);
>
>  void xprt_free(struct rpc_xprt *xprt)
>  {
> +       rpc_netns_xprt_sysfs_destroy(xprt);
>         put_net(xprt->xprt_net);
>         xprt_free_all_slots(xprt);
>         xprt_free_id(xprt);
> --
> 2.26.2
>

Re: [PATCH v1 3/8] sunrpc: add a directory per sunrpc xprt

[Dan Aloni]

On Tue, Feb 16, 2021 at 04:46:55PM -0500, Anna Schumaker wrote:
> > +static ssize_t rpc_netns_xprt_dstaddr_show(struct kobject *kobj,
> > +               struct kobj_attribute *attr, char *buf)
> > +{
> > +       struct rpc_netns_xprt *c = container_of(kobj,
> > +                               struct rpc_netns_xprt, kobject);
> > +       struct rpc_xprt *xprt = c->xprt;
> > +
> > +       if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA))) {
> 
> We might want to change these restrictions later on, so if we're going
> to put this into each function then maybe it would make sense to have
> a quick inline to check protocol support?

Yeah, I agree.

> I do the same check in the setup function for my patches, so if you
> want I can add the inline function and then it'll just be there for
> you to use.

Sure, go ahead.

> 
> > +               sprintf(buf, "N/A");
> > +               return 0;
> 
> I'm guessing the point of putting "N/A" here is so userspace tools
> don't have to guess which files exist or not for each protocol type?
> Should I change my patches to match this style too?

Yes, though I'm not sure what are the common kernel convention here.
Maybe a "-" string is sufficient?

-- 
Dan Aloni

[PATCH v1 4/8] sunrpc: have client directory a symlink to the root transport

[Dan Aloni]

Instead of duplicating `dstaddr` in client directory, we add a symlink
to the relevant transport directory which now hosts a `dstaddr`.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 net/sunrpc/sysfs.c | 44 ++++++--------------------------------------
 1 file changed, 6 insertions(+), 38 deletions(-)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 687d4470b90d..ae608235d7e0 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -69,37 +69,6 @@ int rpc_sysfs_init(void)
 	return -ENOMEM;
 }
 
-static ssize_t rpc_netns_dstaddr_show(struct kobject *kobj,
-		struct kobj_attribute *attr, char *buf)
-{
-	struct rpc_netns_client *c = container_of(kobj,
-				struct rpc_netns_client, kobject);
-	struct rpc_clnt *clnt = c->clnt;
-	struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
-
-	return rpc_ntop((struct sockaddr *)&xprt->addr, buf, PAGE_SIZE);
-}
-
-static ssize_t rpc_netns_dstaddr_store(struct kobject *kobj,
-		struct kobj_attribute *attr, const char *buf, size_t count)
-{
-	struct rpc_netns_client *c = container_of(kobj,
-				struct rpc_netns_client, kobject);
-	struct rpc_clnt *clnt = c->clnt;
-	struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
-	struct sockaddr *saddr = (struct sockaddr *)&xprt->addr;
-	int port = rpc_get_port(saddr);
-
-	xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr, sizeof(*saddr));
-	rpc_set_port(saddr, port);
-
-	kfree(xprt->address_strings[RPC_DISPLAY_ADDR]);
-	xprt->address_strings[RPC_DISPLAY_ADDR] = kstrndup(buf, count - 1, GFP_KERNEL);
-
-	xprt->ops->connect(xprt, NULL);
-	return count;
-}
-
 static void rpc_netns_client_release(struct kobject *kobj)
 {
 	struct rpc_netns_client *c;
@@ -113,11 +82,7 @@ static const void *rpc_netns_client_namespace(struct kobject *kobj)
 	return container_of(kobj, struct rpc_netns_client, kobject)->net;
 }
 
-static struct kobj_attribute rpc_netns_client_dstaddr = __ATTR(dstaddr,
-		0644, rpc_netns_dstaddr_show, rpc_netns_dstaddr_store);
-
 static struct attribute *rpc_netns_client_attrs[] = {
-	&rpc_netns_client_dstaddr.attr,
 	NULL,
 };
 
@@ -156,12 +121,14 @@ void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
 {
 	struct rpc_netns_client *rpc_client;
 	struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
-
-	if (!(xprt->prot & (IPPROTO_TCP | XPRT_TRANSPORT_RDMA)))
-		return;
+	struct rpc_netns_xprt *rpc_xprt;
+	int ret;
 
 	rpc_client = rpc_netns_client_alloc(rpc_client_kobj, net, clnt->cl_clid);
 	if (rpc_client) {
+		rpc_xprt = xprt->sysfs;
+		ret = sysfs_create_link_nowarn(&rpc_client->kobject,
+					 &rpc_xprt->kobject, "transport");
 		clnt->cl_sysfs = rpc_client;
 		rpc_client->clnt = clnt;
 		kobject_uevent(&rpc_client->kobject, KOBJ_ADD);
@@ -173,6 +140,7 @@ void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
 	struct rpc_netns_client *rpc_client = clnt->cl_sysfs;
 
 	if (rpc_client) {
+		sysfs_remove_link(&rpc_client->kobject, "transport");
 		kobject_uevent(&rpc_client->kobject, KOBJ_REMOVE);
 		kobject_del(&rpc_client->kobject);
 		kobject_put(&rpc_client->kobject);
-- 
2.26.2

[PATCH v1 5/8] sunrpc: add IDs to multipath

[Dan Aloni]

This is used to uniquely identify sunrpc multipath objects in /sys.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 include/linux/sunrpc/xprtmultipath.h |  4 ++++
 net/sunrpc/sunrpc_syms.c             |  1 +
 net/sunrpc/xprtmultipath.c           | 27 +++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index c6cce3fbf29d..ef95a6f18ccf 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -14,6 +14,7 @@ struct rpc_xprt_switch {
 	spinlock_t		xps_lock;
 	struct kref		xps_kref;
 
+	unsigned int		xps_id;
 	unsigned int		xps_nxprts;
 	unsigned int		xps_nactive;
 	atomic_long_t		xps_queuelen;
@@ -71,4 +72,7 @@ extern struct rpc_xprt *xprt_iter_get_next(struct rpc_xprt_iter *xpi);
 
 extern bool rpc_xprt_switch_has_addr(struct rpc_xprt_switch *xps,
 		const struct sockaddr *sap);
+
+extern void xprt_multipath_cleanup_ids(void);
+
 #endif
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index b61b74c00483..691c0000e9ea 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -134,6 +134,7 @@ cleanup_sunrpc(void)
 	rpc_sysfs_exit();
 	rpc_cleanup_clids();
 	xprt_cleanup_ids();
+	xprt_multipath_cleanup_ids();
 	rpcauth_remove_module();
 	cleanup_socket_xprt();
 	svc_cleanup_xprt_sock();
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 78c075a68c04..52a9584b23af 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -86,6 +86,31 @@ void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
 	xprt_put(xprt);
 }
 
+static DEFINE_IDA(rpc_xprtswitch_ids);
+
+void xprt_multipath_cleanup_ids(void)
+{
+       ida_destroy(&rpc_xprtswitch_ids);
+}
+
+static int xprt_switch_alloc_id(struct rpc_xprt_switch *xps)
+{
+       int id;
+
+       id = ida_simple_get(&rpc_xprtswitch_ids, 0, 0, GFP_KERNEL);
+       if (id < 0)
+               return id;
+
+       xps->xps_id = id;
+       return 0;
+}
+
+static void xprt_switch_free_id(struct rpc_xprt_switch *xps)
+{
+       ida_simple_remove(&rpc_xprtswitch_ids, xps->xps_id);
+}
+
+
 /**
  * xprt_switch_alloc - Allocate a new struct rpc_xprt_switch
  * @xprt: pointer to struct rpc_xprt
@@ -103,6 +128,7 @@ struct rpc_xprt_switch *xprt_switch_alloc(struct rpc_xprt *xprt,
 	if (xps != NULL) {
 		spin_lock_init(&xps->xps_lock);
 		kref_init(&xps->xps_kref);
+		xprt_switch_alloc_id(xps);
 		xps->xps_nxprts = xps->xps_nactive = 0;
 		atomic_long_set(&xps->xps_queuelen, 0);
 		xps->xps_net = NULL;
@@ -136,6 +162,7 @@ static void xprt_switch_free(struct kref *kref)
 			struct rpc_xprt_switch, xps_kref);
 
 	xprt_switch_free_entries(xps);
+	xprt_switch_free_id(xps);
 	kfree_rcu(xps, xps_rcu);
 }
 
-- 
2.26.2

[PATCH v1 6/8] sunrpc: add multipath directory and symlink from client

[Dan Aloni]

This also adds a `list` attribute to multipath directory that provides
the transport IDs of the transports contained in the multipath object.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 include/linux/sunrpc/xprtmultipath.h |   2 +
 net/sunrpc/sysfs.c                   | 122 +++++++++++++++++++++++++++
 net/sunrpc/sysfs.h                   |   8 ++
 net/sunrpc/xprtmultipath.c           |  10 +++
 4 files changed, 142 insertions(+)

diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index ef95a6f18ccf..2d0832dc10f5 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -24,6 +24,8 @@ struct rpc_xprt_switch {
 
 	const struct rpc_xprt_iter_ops *xps_iter_ops;
 
+	void                    *xps_sysfs; /* /sys/kernel/sunrpc/multipath/<id> */
+
 	struct rcu_head		xps_rcu;
 };
 
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index ae608235d7e0..3592f3b862b2 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -10,6 +10,7 @@
 
 struct kobject *rpc_client_kobj;
 struct kobject *rpc_xprt_kobj;
+struct kobject *rpc_xps_kobj;
 static struct kset *rpc_sunrpc_kset;
 
 static void rpc_netns_object_release(struct kobject *kobj)
@@ -58,8 +59,15 @@ int rpc_sysfs_init(void)
 	if (!rpc_xprt_kobj)
 		goto err_client;
 
+	rpc_xps_kobj = rpc_netns_object_alloc("multipath", rpc_sunrpc_kset, NULL);
+	if (!rpc_xps_kobj)
+		goto err_xprt;
+
 	return 0;
 
+err_xprt:
+	kobject_put(rpc_xprt_kobj);
+	rpc_xprt_kobj = NULL;
 err_client:
 	kobject_put(rpc_client_kobj);
 	rpc_client_kobj = NULL;
@@ -95,6 +103,7 @@ static struct kobj_type rpc_netns_client_type = {
 
 void rpc_sysfs_exit(void)
 {
+	kobject_put(rpc_xps_kobj);
 	kobject_put(rpc_xprt_kobj);
 	kobject_put(rpc_client_kobj);
 	kset_unregister(rpc_sunrpc_kset);
@@ -122,17 +131,29 @@ void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
 	struct rpc_netns_client *rpc_client;
 	struct rpc_xprt *xprt = rcu_dereference(clnt->cl_xprt);
 	struct rpc_netns_xprt *rpc_xprt;
+	struct rpc_netns_multipath *rpc_multipath;
+	struct rpc_xprt_switch *xps;
 	int ret;
 
+	xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
+
 	rpc_client = rpc_netns_client_alloc(rpc_client_kobj, net, clnt->cl_clid);
 	if (rpc_client) {
 		rpc_xprt = xprt->sysfs;
 		ret = sysfs_create_link_nowarn(&rpc_client->kobject,
 					 &rpc_xprt->kobject, "transport");
+		if (xps) {
+			rpc_multipath = xps->xps_sysfs;
+			ret = sysfs_create_link_nowarn(&rpc_client->kobject,
+						       &rpc_multipath->kobject,
+						       "multipath");
+		}
 		clnt->cl_sysfs = rpc_client;
 		rpc_client->clnt = clnt;
 		kobject_uevent(&rpc_client->kobject, KOBJ_ADD);
 	}
+
+	xprt_switch_put(xps);
 }
 
 void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
@@ -141,6 +162,7 @@ void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt)
 
 	if (rpc_client) {
 		sysfs_remove_link(&rpc_client->kobject, "transport");
+		sysfs_remove_link(&rpc_client->kobject, "multipath");
 		kobject_uevent(&rpc_client->kobject, KOBJ_REMOVE);
 		kobject_del(&rpc_client->kobject);
 		kobject_put(&rpc_client->kobject);
@@ -255,3 +277,103 @@ void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt)
 		xprt->sysfs = NULL;
 	}
 }
+
+static void rpc_netns_multipath_release(struct kobject *kobj)
+{
+	struct rpc_netns_multipath *c;
+
+	c = container_of(kobj, struct rpc_netns_multipath, kobject);
+	kfree(c);
+}
+
+static const void *rpc_netns_multipath_namespace(struct kobject *kobj)
+{
+	return container_of(kobj, struct rpc_netns_multipath, kobject)->net;
+}
+
+static ssize_t rpc_netns_multipath_list_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct rpc_netns_multipath *c =
+		container_of(kobj, struct rpc_netns_multipath, kobject);
+	struct rpc_xprt_switch *xps = c->xps;
+	struct rpc_xprt_iter xpi;
+	int pos = 0;
+
+	xprt_iter_init_listall(&xpi, xps);
+	for (;;) {
+		struct rpc_xprt *xprt = xprt_iter_get_next(&xpi);
+		if (!xprt)
+			break;
+
+		snprintf(&buf[pos], PAGE_SIZE - pos, "%d\n", xprt->id);
+		pos += strlen(&buf[pos]);
+		xprt_put(xprt);
+	}
+	xprt_iter_destroy(&xpi);
+
+	return pos;
+}
+
+static ssize_t rpc_netns_multipath_list_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return -EINVAL;
+}
+
+static struct kobj_attribute rpc_netns_multipath_list = __ATTR(list,
+	0644, rpc_netns_multipath_list_show, rpc_netns_multipath_list_store);
+
+
+static struct attribute *rpc_netns_multipath_attrs[] = {
+	&rpc_netns_multipath_list.attr,
+	NULL,
+};
+
+static struct kobj_type rpc_netns_multipath_type = {
+	.release = rpc_netns_multipath_release,
+	.default_attrs = rpc_netns_multipath_attrs,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.namespace = rpc_netns_multipath_namespace,
+};
+
+static struct rpc_netns_multipath *rpc_netns_multipath_alloc(struct kobject *parent,
+							     struct net *net, int id)
+{
+	struct rpc_netns_multipath *p;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (p) {
+		p->net = net;
+		p->kobject.kset = rpc_sunrpc_kset;
+		if (kobject_init_and_add(&p->kobject, &rpc_netns_multipath_type,
+					 parent, "%d", id) == 0)
+			return p;
+		kobject_put(&p->kobject);
+	}
+	return NULL;
+}
+
+void rpc_netns_multipath_sysfs_setup(struct rpc_xprt_switch *xps, struct net *net)
+{
+	struct rpc_netns_multipath *rpc_multipath;
+
+	rpc_multipath = rpc_netns_multipath_alloc(rpc_xps_kobj, net, xps->xps_id);
+	if (rpc_multipath) {
+		xps->xps_sysfs = rpc_multipath;
+		rpc_multipath->xps = xps;
+		kobject_uevent(&rpc_multipath->kobject, KOBJ_ADD);
+	}
+}
+
+void rpc_netns_multipath_sysfs_destroy(struct rpc_xprt_switch *xps)
+{
+	struct rpc_netns_multipath *rpc_multipath = xps->xps_sysfs;
+
+	if (rpc_multipath) {
+		kobject_uevent(&rpc_multipath->kobject, KOBJ_REMOVE);
+		kobject_del(&rpc_multipath->kobject);
+		kobject_put(&rpc_multipath->kobject);
+		xps->xps_sysfs = NULL;
+	}
+}
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
index e08dd7f6a1ec..b2e379f78b91 100644
--- a/net/sunrpc/sysfs.h
+++ b/net/sunrpc/sysfs.h
@@ -17,6 +17,12 @@ struct rpc_netns_xprt {
 	struct rpc_xprt *xprt;
 };
 
+struct rpc_netns_multipath {
+	struct kobject kobject;
+	struct net *net;
+	struct rpc_xprt_switch *xps;
+};
+
 extern struct kobject *rpc_client_kobj;
 extern struct kobject *rpc_xprt_kobj;
 
@@ -27,5 +33,7 @@ void rpc_netns_client_sysfs_setup(struct rpc_clnt *clnt, struct net *net);
 void rpc_netns_client_sysfs_destroy(struct rpc_clnt *clnt);
 void rpc_netns_xprt_sysfs_setup(struct rpc_xprt *xprt, struct net *net);
 void rpc_netns_xprt_sysfs_destroy(struct rpc_xprt *xprt);
+void rpc_netns_multipath_sysfs_setup(struct rpc_xprt_switch *xps, struct net *net);
+void rpc_netns_multipath_sysfs_destroy(struct rpc_xprt_switch *xps);
 
 #endif
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 52a9584b23af..d03fb3bb74ce 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -19,6 +19,8 @@
 #include <linux/sunrpc/addr.h>
 #include <linux/sunrpc/xprtmultipath.h>
 
+#include "sysfs.h"
+
 typedef struct rpc_xprt *(*xprt_switch_find_xprt_t)(struct rpc_xprt_switch *xps,
 		const struct rpc_xprt *cur);
 
@@ -83,6 +85,9 @@ void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
 	spin_lock(&xps->xps_lock);
 	xprt_switch_remove_xprt_locked(xps, xprt);
 	spin_unlock(&xps->xps_lock);
+
+	if (!xps->xps_net)
+		rpc_netns_multipath_sysfs_destroy(xps);
 	xprt_put(xprt);
 }
 
@@ -135,6 +140,10 @@ struct rpc_xprt_switch *xprt_switch_alloc(struct rpc_xprt *xprt,
 		INIT_LIST_HEAD(&xps->xps_xprt_list);
 		xps->xps_iter_ops = &rpc_xprt_iter_singular;
 		xprt_switch_add_xprt_locked(xps, xprt);
+		xps->xps_sysfs = NULL;
+
+		if (xprt->xprt_net != NULL)
+			rpc_netns_multipath_sysfs_setup(xps, xprt->xprt_net);
 	}
 
 	return xps;
@@ -162,6 +171,7 @@ static void xprt_switch_free(struct kref *kref)
 			struct rpc_xprt_switch, xps_kref);
 
 	xprt_switch_free_entries(xps);
+	rpc_netns_multipath_sysfs_destroy(xps);
 	xprt_switch_free_id(xps);
 	kfree_rcu(xps, xps_rcu);
 }
-- 
2.26.2

[PATCH v1 7/8] sunrpc: change rpc_clnt_add_xprt() to rpc_add_xprt()

[Dan Aloni]

This change of API allows adding transports without holding a reference
to an rpc client.

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 fs/nfs/pnfs_nfs.c           | 12 +++++++-----
 include/linux/sunrpc/clnt.h | 12 +++++++-----
 net/sunrpc/clnt.c           | 31 +++++++++++++++++--------------
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 49d3389bd813..1e61626bd0fa 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -878,8 +878,9 @@ static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
 			if (da->da_addr.ss_family != clp->cl_addr.ss_family)
 				continue;
 			/* Add this address as an alias */
-			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
-					rpc_clnt_test_and_add_xprt, NULL);
+			rpc_add_xprt(&clp->cl_rpcclient->cl_xpi,
+				     clp->cl_rpcclient, &xprt_args,
+				     rpc_clnt_test_and_add_xprt, NULL);
 			continue;
 		}
 		clp = get_v3_ds_connect(mds_srv,
@@ -945,9 +946,10 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 			* add as an alias
 			*/
 			xprtdata.cred = nfs4_get_clid_cred(clp),
-			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
-					  rpc_clnt_setup_test_and_add_xprt,
-					  &rpcdata);
+			rpc_add_xprt(&clp->cl_rpcclient->cl_xpi,
+				     clp->cl_rpcclient, &xprt_args,
+				     rpc_clnt_setup_test_and_add_xprt,
+				     &rpcdata);
 			if (xprtdata.cred)
 				put_cred(xprtdata.cred);
 		} else {
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 503653720e18..19bb23143eef 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -210,21 +210,23 @@ int 		rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
 			int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void *),
 			void *data);
 
-int 		rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
+int 		rpc_clnt_test_and_add_xprt(void *clnt,
 			struct rpc_xprt_switch *xps,
 			struct rpc_xprt *xprt,
 			void *dummy);
-int		rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
-			int (*setup)(struct rpc_clnt *,
+int		rpc_add_xprt(struct rpc_xprt_iter *iter,
+		        void *ctx,
+		        struct xprt_create *xprtargs,
+		        int (*setup)(void *ctx,
 				struct rpc_xprt_switch *,
 				struct rpc_xprt *,
 				void *),
-			void *data);
+		        void *data);
 void		rpc_set_connect_timeout(struct rpc_clnt *clnt,
 			unsigned long connect_timeout,
 			unsigned long reconnect_timeout);
 
-int		rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
+int		rpc_clnt_setup_test_and_add_xprt(void *,
 			struct rpc_xprt_switch *,
 			struct rpc_xprt *,
 			void *);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 0a4811be01cd..b94d274a5446 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -598,7 +598,7 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args)
 		return clnt;
 
 	for (i = 0; i < args->nconnect - 1; i++) {
-		if (rpc_clnt_add_xprt(clnt, &xprtargs, NULL, NULL) < 0)
+		if (rpc_add_xprt(&clnt->cl_xpi, NULL, &xprtargs, NULL, NULL) < 0)
 			break;
 	}
 	return clnt;
@@ -2751,10 +2751,11 @@ static const struct rpc_call_ops rpc_cb_add_xprt_call_ops = {
  * @xprt: pointer struct rpc_xprt
  * @dummy: unused
  */
-int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
+int rpc_clnt_test_and_add_xprt(void *ptr,
 		struct rpc_xprt_switch *xps, struct rpc_xprt *xprt,
 		void *dummy)
 {
+	struct rpc_clnt *clnt = ptr;
 	struct rpc_cb_add_xprt_calldata *data;
 	struct rpc_task *task;
 
@@ -2795,11 +2796,12 @@ EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
  * @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,
+int rpc_clnt_setup_test_and_add_xprt(void *ptr,
 				     struct rpc_xprt_switch *xps,
 				     struct rpc_xprt *xprt,
 				     void *data)
 {
+	struct rpc_clnt *clnt = ptr;
 	struct rpc_task *task;
 	struct rpc_add_xprt_test *xtest = (struct rpc_add_xprt_test *)data;
 	int status = -EADDRINUSE;
@@ -2852,13 +2854,14 @@ EXPORT_SYMBOL_GPL(rpc_clnt_setup_test_and_add_xprt);
  * adding the new transport.
  *
  */
-int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
-		struct xprt_create *xprtargs,
-		int (*setup)(struct rpc_clnt *,
-			struct rpc_xprt_switch *,
-			struct rpc_xprt *,
-			void *),
-		void *data)
+int rpc_add_xprt(struct rpc_xprt_iter *iter,
+		 void *ctx,
+		 struct xprt_create *xprtargs,
+		 int (*setup)(void *ctx,
+			      struct rpc_xprt_switch *,
+			      struct rpc_xprt *,
+			      void *),
+		 void *data)
 {
 	struct rpc_xprt_switch *xps;
 	struct rpc_xprt *xprt;
@@ -2868,8 +2871,8 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
 	int ret = 0;
 
 	rcu_read_lock();
-	xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
-	xprt = xprt_iter_xprt(&clnt->cl_xpi);
+	xps = xprt_switch_get(rcu_dereference(iter->xpi_xpswitch));
+	xprt = xprt_iter_xprt(iter);
 	if (xps == NULL || xprt == NULL) {
 		rcu_read_unlock();
 		xprt_switch_put(xps);
@@ -2895,7 +2898,7 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
 
 	rpc_xprt_switch_set_roundrobin(xps);
 	if (setup) {
-		ret = setup(clnt, xps, xprt, data);
+		ret = setup(ctx, xps, xprt, data);
 		if (ret != 0)
 			goto out_put_xprt;
 	}
@@ -2906,7 +2909,7 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
 	xprt_switch_put(xps);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
+EXPORT_SYMBOL_GPL(rpc_add_xprt);
 
 struct connect_timeout_data {
 	unsigned long connect_timeout;
-- 
2.26.2

[PATCH v1 8/8] sunrpc: introduce an 'add' node to 'multipath' sysfs directory

[Dan Aloni]

This allows adding new transports to an existing multipath switch. This
is similar to what the `nconnect` mount parameter does, but instead we
can do this while the NFS mount is live.

For example:

    echo 'dstaddr 192.168.40.8 kind rdma' \
	   > /sys/kernel/sunrpc/client/0/multipath/add

Signed-off-by: Dan Aloni <dan@kernelim.com>
---
 net/sunrpc/sysfs.c | 104 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 101 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 3592f3b862b2..d745dfb7cef4 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -315,18 +315,116 @@ static ssize_t rpc_netns_multipath_list_show(struct kobject *kobj,
 	return pos;
 }
 
-static ssize_t rpc_netns_multipath_list_store(struct kobject *kobj,
+static ssize_t rpc_netns_multipath_add_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
-	return -EINVAL;
+	struct rpc_netns_multipath *c =
+		container_of(kobj, struct rpc_netns_multipath, kobject);
+	struct rpc_xprt_switch *xps = c->xps;
+	struct rpc_xprt_iter xpi;
+	struct sockaddr_storage addr;
+	struct xprt_create xprt_args = {
+		.ident = XPRT_TRANSPORT_RDMA,
+		.net = c->net,
+		.dstaddr = (struct sockaddr *)&addr,
+	};
+	struct rpc_xprt *existing_xprt;
+	char *opt, *str, *arg;
+	bool has_dstaddr = false;
+	bool has_kind = false;
+	int err;
+
+	opt = kstrndup(buf, count, GFP_KERNEL);
+	if (!opt)
+		return -ENOMEM;
+
+	str = strstrip(opt);
+
+	while (1) {
+		arg = strsep(&str, " ");
+		if (!arg)
+			break;
+
+		if (sysfs_streq(arg, "kind")) {
+			arg = strsep(&str, " ");
+			if (!arg) {
+				err = -EINVAL;
+				goto out;
+			}
+			if (has_kind) {
+				err = -EINVAL;
+				goto out;
+			}
+
+			if (sysfs_streq(arg, "rdma")) {
+				xprt_args.ident = XPRT_TRANSPORT_RDMA;
+			} else if (sysfs_streq(arg, "tcp")) {
+				xprt_args.ident = XPRT_TRANSPORT_TCP;
+			} else {
+				err = -EINVAL;
+				goto out;
+			}
+
+			has_kind = true;
+		} else if (sysfs_streq(arg, "dstaddr")) {
+			arg = strsep(&str, " ");
+			if (!arg) {
+				err = -EINVAL;
+				goto out;
+			}
+			xprt_args.addrlen = rpc_pton(c->net,
+				arg, strlen(arg), (struct sockaddr *)&addr,
+				sizeof(addr));
+			if (has_dstaddr || !xprt_args.addrlen) {
+				err = -EINVAL;
+				goto out;
+			}
+
+			has_dstaddr = true;
+		} else {
+			break;
+		}
+	}
+
+	if (!has_dstaddr || !has_kind)  {
+		err = -EINVAL;
+		goto out;
+	}
+
+
+	/* Discover an existing xprt */
+	xprt_iter_init_listall(&xpi, xps);
+	existing_xprt = xprt_iter_get_next(&xpi);
+	xprt_iter_destroy(&xpi);
+
+	if (!existing_xprt) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	xprt_args.servername = existing_xprt->servername;
+	xprt_iter_init_listall(&xpi, xps);
+	rpc_add_xprt(&xpi, NULL, &xprt_args, NULL, NULL);
+	xprt_iter_destroy(&xpi);
+
+	xprt_put(existing_xprt);
+
+	err = count;
+out:
+	kfree(opt);
+
+	return err;
 }
 
 static struct kobj_attribute rpc_netns_multipath_list = __ATTR(list,
-	0644, rpc_netns_multipath_list_show, rpc_netns_multipath_list_store);
+	0444, rpc_netns_multipath_list_show, NULL);
 
+static struct kobj_attribute rpc_netns_multipath_add = __ATTR(add,
+	0200, NULL, rpc_netns_multipath_add_store);
 
 static struct attribute *rpc_netns_multipath_attrs[] = {
 	&rpc_netns_multipath_list.attr,
+	&rpc_netns_multipath_add.attr,
 	NULL,
 };
 
-- 
2.26.2

Re: [PATCH v1 0/8] sysfs files for multipath transport control

[Olga Kornievskaia]

Hi Dan,

On Mon, Feb 15, 2021 at 12:43 PM Dan Aloni <dan@kernelim.com> wrote:
>
> Hi Anna,
>
> This patchset builds ontop v2 of your 'sysfs files for changing IP' changeset.
>
> - The patchset adds two more sysfs objects, for one for transport and another
>   for multipath.
> - Also, `net` renamed to `client`, and `client` now has symlink to its principal
>   transport. A client also has a symlink to its `multipath` object.
> - The transport interface lets you change `dstaddr` of individual transports,
>   when `nconnect` is used (or if it wasn't used and these were added with the
>   new interface).
> - The interface to add a transport is using a single string written to 'add',
>   for example:
>
>        echo 'dstaddr 192.168.40.8 kind rdma' \
>                > /sys/kernel/sunrpc/client/0/multipath/add
>
> These changes are independent of the method used to obtain a sunrpc ID for a
> mountpoint. For that I've sent a concept patch showing an fspick-based
> implementation: https://marc.info/?l=linux-nfs&m=161332454821849&w=4

I'm confused: does this allow adding arbitrary connections between a
client and some server IP to an existing RPC client? Given the above
description, that's how it reads to me, can you clarify please. I
thought it was something specifically for v3 (because it has no
concept of trunking). As for NFSv4 there is a notion of getting server
locations via FS_LOCATION and doing trunking (ie multipathing)? I
don't see how this code restricts the addition of transports to v3.

>
> Thanks
>
> Dan Aloni (8):
>   sunrpc: rename 'net' to 'client'
>   sunrpc: add xprt id
>   sunrpc: add a directory per sunrpc xprt
>   sunrpc: have client directory a symlink to the root transport
>   sunrpc: add IDs to multipath
>   sunrpc: add multipath directory and symlink from client
>   sunrpc: change rpc_clnt_add_xprt() to rpc_add_xprt()
>   sunrpc: introduce an 'add' node to 'multipath' sysfs directory
>
>  fs/nfs/pnfs_nfs.c                    |  12 +-
>  include/linux/sunrpc/clnt.h          |  12 +-
>  include/linux/sunrpc/xprt.h          |   3 +
>  include/linux/sunrpc/xprtmultipath.h |   6 +
>  net/sunrpc/clnt.c                    |  39 +--
>  net/sunrpc/sunrpc_syms.c             |   2 +
>  net/sunrpc/sysfs.c                   | 403 +++++++++++++++++++++++----
>  net/sunrpc/sysfs.h                   |  21 +-
>  net/sunrpc/xprt.c                    |  29 ++
>  net/sunrpc/xprtmultipath.c           |  37 +++
>  10 files changed, 487 insertions(+), 77 deletions(-)
>
> --
> 2.26.2
>

Re: [PATCH v1 0/8] sysfs files for multipath transport control

[Dan Aloni]

On Mon, Mar 01, 2021 at 10:56:22PM -0500, Olga Kornievskaia wrote:
> Hi Dan,
> 
> On Mon, Feb 15, 2021 at 12:43 PM Dan Aloni <dan@kernelim.com> wrote:
> >
> > Hi Anna,
> >
> > This patchset builds ontop v2 of your 'sysfs files for changing IP' changeset.
> >
> > - The patchset adds two more sysfs objects, for one for transport and another
> >   for multipath.
> > - Also, `net` renamed to `client`, and `client` now has symlink to its principal
> >   transport. A client also has a symlink to its `multipath` object.
> > - The transport interface lets you change `dstaddr` of individual transports,
> >   when `nconnect` is used (or if it wasn't used and these were added with the
> >   new interface).
> > - The interface to add a transport is using a single string written to 'add',
> >   for example:
> >
> >        echo 'dstaddr 192.168.40.8 kind rdma' \
> >                > /sys/kernel/sunrpc/client/0/multipath/add
> >
> > These changes are independent of the method used to obtain a sunrpc ID for a
> > mountpoint. For that I've sent a concept patch showing an fspick-based
> > implementation: https://marc.info/?l=linux-nfs&m=161332454821849&w=4
> 
> I'm confused: does this allow adding arbitrary connections between a
> client and some server IP to an existing RPC client? Given the above
> description, that's how it reads to me, can you clarify please. I
> thought it was something specifically for v3 (because it has no
> concept of trunking). As for NFSv4 there is a notion of getting server
> locations via FS_LOCATION and doing trunking (ie multipathing)? I
> don't see how this code restricts the addition of transports to v3.

Indeed, there's no restriction to NFSv3.

There can be potential uses for this for NFSv4 too. FS_LOCATIONS serving
as recommendation to which hosts the client can connect, while smart
load-balancing logic in userspace can determine to which subset of these
servers each client in a cluster should actually connect (a full mesh
is not always desired).

At any case, if this restriction is desired, we can add a new sunrpc
client flag for that and pass it only in NFSv3 client init.

-- 
Dan Aloni

Re: [PATCH v1 0/8] sysfs files for multipath transport control

[Chuck Lever]

> On Mar 4, 2021, at 6:58 AM, Dan Aloni <dan@kernelim.com> wrote:
> 
> On Mon, Mar 01, 2021 at 10:56:22PM -0500, Olga Kornievskaia wrote:
>> Hi Dan,
>> 
>> On Mon, Feb 15, 2021 at 12:43 PM Dan Aloni <dan@kernelim.com> wrote:
>>> 
>>> Hi Anna,
>>> 
>>> This patchset builds ontop v2 of your 'sysfs files for changing IP' changeset.
>>> 
>>> - The patchset adds two more sysfs objects, for one for transport and another
>>>  for multipath.
>>> - Also, `net` renamed to `client`, and `client` now has symlink to its principal
>>>  transport. A client also has a symlink to its `multipath` object.
>>> - The transport interface lets you change `dstaddr` of individual transports,
>>>  when `nconnect` is used (or if it wasn't used and these were added with the
>>>  new interface).
>>> - The interface to add a transport is using a single string written to 'add',
>>>  for example:
>>> 
>>>       echo 'dstaddr 192.168.40.8 kind rdma' \
>>>> /sys/kernel/sunrpc/client/0/multipath/add
>>> 
>>> These changes are independent of the method used to obtain a sunrpc ID for a
>>> mountpoint. For that I've sent a concept patch showing an fspick-based
>>> implementation: https://marc.info/?l=linux-nfs&m=161332454821849&w=4
>> 
>> I'm confused: does this allow adding arbitrary connections between a
>> client and some server IP to an existing RPC client? Given the above
>> description, that's how it reads to me, can you clarify please. I
>> thought it was something specifically for v3 (because it has no
>> concept of trunking). As for NFSv4 there is a notion of getting server
>> locations via FS_LOCATION and doing trunking (ie multipathing)? I
>> don't see how this code restricts the addition of transports to v3.
> 
> Indeed, there's no restriction to NFSv3.
> 
> There can be potential uses for this for NFSv4 too. FS_LOCATIONS serving
> as recommendation to which hosts the client can connect, while smart
> load-balancing logic in userspace can determine to which subset of these
> servers each client in a cluster should actually connect (a full mesh
> is not always desired).
> 
> At any case, if this restriction is desired, we can add a new sunrpc
> client flag for that and pass it only in NFSv3 client init.

IMO an NFSv3-only policy should not be built into this API.

This is a user-space / kernel API, not something that is an administrative
interface. The administrative interface, which is the place to apply an
NFSv3-only policy, would use this sysfs API. So would smart load-
balancing logic based on fs_locations.

If the API is under /sys/kernel/sunrpc/client, then including NFS-specific
controls is a layering violation. Consider that the kernel can send
multiple protocols over the same connection (NFSv3 and NFSACL, eg).


--
Chuck Lever