[RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

It's possible for an NFS server to go down but come back up with a
different IP address. These patches provide a way for administrators to
handle this issue by providing a new IP address for xprt sockets to
connect to.

This is a first draft of the code, so any thoughts or suggestions would
be greatly appreciated!

Anna


Anna Schumaker (7):
  net: Add a /sys/net directory to sysfs
  sunrpc: Create a sunrpc directory under /sys/net/
  sunrpc: Create a net/ subdirectory in the sunrpc sysfs
  sunrpc: Create per-rpc_clnt sysfs kobjects
  sunrpc: Create a per-rpc_clnt file for managing the IP address
  sunrpc: Prepare xs_connect() for taking NULL tasks
  sunrpc: Connect to a new IP address provided by the user

 include/linux/sunrpc/clnt.h |   1 +
 include/net/sock.h          |   4 +
 net/socket.c                |   8 ++
 net/sunrpc/Makefile         |   2 +-
 net/sunrpc/clnt.c           |   5 ++
 net/sunrpc/sunrpc_syms.c    |   8 ++
 net/sunrpc/sysfs.c          | 160 ++++++++++++++++++++++++++++++++++++
 net/sunrpc/sysfs.h          |  22 +++++
 net/sunrpc/xprtsock.c       |   3 +-
 9 files changed, 211 insertions(+), 2 deletions(-)
 create mode 100644 net/sunrpc/sysfs.c
 create mode 100644 net/sunrpc/sysfs.h

-- 
2.29.2

[RFC PATCH 1/7] net: Add a /sys/net directory to sysfs

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

Sunrpc will want to place its files under this directory

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 include/net/sock.h | 4 ++++
 net/socket.c       | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index bdc4323ce53c..cd986c6fbc6f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1765,6 +1765,10 @@ void sk_common_release(struct sock *sk);
 /* Initialise core socket variables */
 void sock_init_data(struct socket *sock, struct sock *sk);
 
+
+/* /sys/net */
+extern struct kobject *net_kobj;
+
 /*
  * Socket reference counting postulates.
  *
diff --git a/net/socket.c b/net/socket.c
index 33e8b6c4e1d3..43986419d673 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -141,6 +141,10 @@ static void sock_show_fdinfo(struct seq_file *m, struct file *f)
 #define sock_show_fdinfo NULL
 #endif
 
+/* /sys/net */
+struct kobject *net_kobj;
+EXPORT_SYMBOL_GPL(net_kobj);
+
 /*
  *	Socket files have a set of 'special' operations as well as the generic file ones. These don't appear
  *	in the operation structures but are done directly via the socketcall() multiplexor.
@@ -3060,6 +3064,10 @@ static int __init sock_init(void)
 		goto out_mount;
 	}
 
+	net_kobj = kobject_create_and_add("net", NULL);
+	if (!net_kobj)
+		printk(KERN_WARNING "%s: kobj create error\n", __func__);
+
 	/* The real protocol initialization is performed in later initcalls.
 	 */
 
-- 
2.29.2

[RFC PATCH 2/7] sunrpc: Create a sunrpc directory under /sys/net/

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

This is where we'll put per-rpc_client related files

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 net/sunrpc/Makefile      |  2 +-
 net/sunrpc/sunrpc_syms.c |  8 ++++++++
 net/sunrpc/sysfs.c       | 21 +++++++++++++++++++++
 net/sunrpc/sysfs.h       | 13 +++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 net/sunrpc/sysfs.c
 create mode 100644 net/sunrpc/sysfs.h

diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile
index 9488600451e8..1c8de397d6ad 100644
--- a/net/sunrpc/Makefile
+++ b/net/sunrpc/Makefile
@@ -12,7 +12,7 @@ sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \
 	    auth.o auth_null.o auth_unix.o \
 	    svc.o svcsock.o svcauth.o svcauth_unix.o \
 	    addr.o rpcb_clnt.o timer.o xdr.o \
-	    sunrpc_syms.o cache.o rpc_pipe.o \
+	    sunrpc_syms.o cache.o rpc_pipe.o sysfs.o \
 	    svc_xprt.o \
 	    xprtmultipath.o
 sunrpc-$(CONFIG_SUNRPC_DEBUG) += debugfs.o
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index 236fadc4a439..3b57efc692ec 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -24,6 +24,7 @@
 #include <linux/sunrpc/xprtsock.h>
 
 #include "sunrpc.h"
+#include "sysfs.h"
 #include "netns.h"
 
 unsigned int sunrpc_net_id;
@@ -103,6 +104,10 @@ init_sunrpc(void)
 	if (err)
 		goto out4;
 
+	err = rpc_sysfs_init();
+	if (err)
+		goto out5;
+
 	sunrpc_debugfs_init();
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 	rpc_register_sysctl();
@@ -111,6 +116,8 @@ init_sunrpc(void)
 	init_socket_xprt();	/* clnt sock transport */
 	return 0;
 
+out5:
+	unregister_rpc_pipefs();
 out4:
 	unregister_pernet_subsys(&sunrpc_net_ops);
 out3:
@@ -124,6 +131,7 @@ init_sunrpc(void)
 static void __exit
 cleanup_sunrpc(void)
 {
+	rpc_sysfs_exit();
 	rpc_cleanup_clids();
 	rpcauth_remove_module();
 	cleanup_socket_xprt();
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
new file mode 100644
index 000000000000..efff6977095c
--- /dev/null
+++ b/net/sunrpc/sysfs.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Anna Schumaker <Anna.Schumaker@Netapp.com>
+ */
+#include <net/sock.h>
+
+//struct kobject *rpc_client_kobj;
+static struct kset *rpc_client_kset;
+
+int rpc_sysfs_init(void)
+{
+	rpc_client_kset = kset_create_and_add("sunrpc", NULL, net_kobj);
+	if (!rpc_client_kset)
+		return -ENOMEM;
+	return 0;
+}
+
+void rpc_sysfs_exit(void)
+{
+	kset_unregister(rpc_client_kset);
+}
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
new file mode 100644
index 000000000000..93c3cd220506
--- /dev/null
+++ b/net/sunrpc/sysfs.h
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Anna Schumaker <Anna.Schumaker@Netapp.com>
+ */
+#ifndef __SUNRPC_SYSFS_H
+#define __SUNRPC_SYSFS_H
+
+extern struct kobject *rpc_client_kobj;
+
+extern int rpc_sysfs_init(void);
+extern void rpc_sysfs_exit(void);
+
+#endif
-- 
2.29.2

[RFC PATCH 3/7] sunrpc: Create a net/ subdirectory in the sunrpc sysfs

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

For network namespace separation.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 net/sunrpc/sysfs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index efff6977095c..afeaec79a9c7 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -4,18 +4,57 @@
  */
 #include <net/sock.h>
 
-//struct kobject *rpc_client_kobj;
+struct kobject *rpc_client_kobj;
 static struct kset *rpc_client_kset;
 
+static void rpc_netns_object_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
+static const struct kobj_ns_type_operations *rpc_netns_object_child_ns_type(
+		struct kobject *kobj)
+{
+	return &net_ns_type_operations;
+}
+
+static struct kobj_type rpc_netns_object_type = {
+	.release = rpc_netns_object_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.child_ns_type = rpc_netns_object_child_ns_type,
+};
+
+static struct kobject *rpc_netns_object_alloc(const char *name,
+		struct kset *kset, struct kobject *parent)
+{
+	struct kobject *kobj;
+	kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
+	if (kobj) {
+		kobj->kset = kset;
+		if (kobject_init_and_add(kobj, &rpc_netns_object_type,
+					parent, "%s", name) == 0)
+			return kobj;
+		kobject_put(kobj);
+	}
+	return NULL;
+}
+
 int rpc_sysfs_init(void)
 {
 	rpc_client_kset = kset_create_and_add("sunrpc", NULL, net_kobj);
 	if (!rpc_client_kset)
 		return -ENOMEM;
+	rpc_client_kobj = rpc_netns_object_alloc("net", rpc_client_kset, NULL);
+	if (!rpc_client_kobj) {
+		kset_unregister(rpc_client_kset);
+		rpc_client_kset = NULL;
+		return -ENOMEM;
+	}
 	return 0;
 }
 
 void rpc_sysfs_exit(void)
 {
+	kobject_put(rpc_client_kobj);
 	kset_unregister(rpc_client_kset);
 }
-- 
2.29.2

[RFC PATCH 4/7] sunrpc: Create per-rpc_clnt sysfs kobjects

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

These will eventually have files placed under them for sysfs operations.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 include/linux/sunrpc/clnt.h |  1 +
 net/sunrpc/clnt.c           |  5 +++
 net/sunrpc/sysfs.c          | 61 +++++++++++++++++++++++++++++++++++++
 net/sunrpc/sysfs.h          |  8 +++++
 4 files changed, 75 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 02e7a5863d28..503653720e18 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -71,6 +71,7 @@ struct rpc_clnt {
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 	struct dentry		*cl_debugfs;	/* debugfs directory */
 #endif
+	void			*cl_sysfs;	/* sysfs directory */
 	/* cl_work is only needed after cl_xpi is no longer used,
 	 * and that are of similar size
 	 */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 612f0a641f4c..02905eae5c0a 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -41,6 +41,7 @@
 #include <trace/events/sunrpc.h>
 
 #include "sunrpc.h"
+#include "sysfs.h"
 #include "netns.h"
 
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
@@ -300,6 +301,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
 	int err;
 
 	rpc_clnt_debugfs_register(clnt);
+	rpc_netns_sysfs_setup(clnt, net);
 
 	pipefs_sb = rpc_get_sb_net(net);
 	if (pipefs_sb) {
@@ -327,6 +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_clnt_debugfs_unregister(clnt);
 	return err;
 }
@@ -733,6 +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_clnt_debugfs_unregister(clnt);
 
 	/*
@@ -879,6 +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_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 afeaec79a9c7..dd298b9c13e8 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -2,7 +2,9 @@
 /*
  * Copyright (c) 2020 Anna Schumaker <Anna.Schumaker@Netapp.com>
  */
+#include <linux/sunrpc/clnt.h>
 #include <net/sock.h>
+#include "sysfs.h"
 
 struct kobject *rpc_client_kobj;
 static struct kset *rpc_client_kset;
@@ -53,8 +55,67 @@ int rpc_sysfs_init(void)
 	return 0;
 }
 
+static void rpc_netns_client_release(struct kobject *kobj)
+{
+	struct rpc_netns_client *c;
+
+	c = container_of(kobj, struct rpc_netns_client, kobject);
+	kfree(c);
+}
+
+static const void *rpc_netns_client_namespace(struct kobject *kobj)
+{
+	return container_of(kobj, struct rpc_netns_client, kobject)->net;
+}
+
+static struct kobj_type rpc_netns_client_type = {
+	.release = rpc_netns_client_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.namespace = rpc_netns_client_namespace,
+};
+
 void rpc_sysfs_exit(void)
 {
 	kobject_put(rpc_client_kobj);
 	kset_unregister(rpc_client_kset);
 }
+
+static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
+						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;
+		if (kobject_init_and_add(&p->kobject, &rpc_netns_client_type,
+					parent, "%d", clid) == 0)
+			return p;
+		kobject_put(&p->kobject);
+	}
+	return NULL;
+}
+
+void rpc_netns_sysfs_setup(struct rpc_clnt *clnt, struct net *net)
+{
+	struct rpc_netns_client *rpc_client;
+
+	rpc_client = rpc_netns_client_alloc(rpc_client_kobj, net, clnt->cl_clid);
+	if (rpc_client) {
+		clnt->cl_sysfs = rpc_client;
+		kobject_uevent(&rpc_client->kobject, KOBJ_ADD);
+	}
+}
+
+void rpc_netns_sysfs_destroy(struct rpc_clnt *clnt)
+{
+	struct rpc_netns_client *rpc_client = clnt->cl_sysfs;
+
+	if (rpc_client) {
+		kobject_uevent(&rpc_client->kobject, KOBJ_REMOVE);
+		kobject_del(&rpc_client->kobject);
+		kobject_put(&rpc_client->kobject);
+		clnt->cl_sysfs = NULL;
+	}
+}
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
index 93c3cd220506..279a836594e7 100644
--- a/net/sunrpc/sysfs.h
+++ b/net/sunrpc/sysfs.h
@@ -5,9 +5,17 @@
 #ifndef __SUNRPC_SYSFS_H
 #define __SUNRPC_SYSFS_H
 
+struct rpc_netns_client {
+	struct kobject kobject;
+	struct net *net;
+};
+
 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);
+
 #endif
-- 
2.29.2

[RFC PATCH 5/7] sunrpc: Create a per-rpc_clnt file for managing the IP address

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

And since we're using IP here, restrict to only creating sysfs files for
TCP and RDMA connections.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 net/sunrpc/sysfs.c | 32 ++++++++++++++++++++++++++++++++
 net/sunrpc/sysfs.h |  1 +
 2 files changed, 33 insertions(+)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index dd298b9c13e8..537d83635670 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2020 Anna Schumaker <Anna.Schumaker@Netapp.com>
  */
 #include <linux/sunrpc/clnt.h>
+#include <linux/sunrpc/addr.h>
 #include <net/sock.h>
 #include "sysfs.h"
 
@@ -55,6 +56,23 @@ int rpc_sysfs_init(void)
 	return 0;
 }
 
+static ssize_t rpc_netns_address_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_address_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return count;
+}
+
 static void rpc_netns_client_release(struct kobject *kobj)
 {
 	struct rpc_netns_client *c;
@@ -68,8 +86,17 @@ 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_address = __ATTR(address,
+		0644, rpc_netns_address_show, rpc_netns_address_store);
+
+static struct attribute *rpc_netns_client_attrs[] = {
+	&rpc_netns_client_address.attr,
+	NULL,
+};
+
 static struct kobj_type rpc_netns_client_type = {
 	.release = rpc_netns_client_release,
+	.default_attrs = rpc_netns_client_attrs,
 	.sysfs_ops = &kobj_sysfs_ops,
 	.namespace = rpc_netns_client_namespace,
 };
@@ -100,10 +127,15 @@ static struct rpc_netns_client *rpc_netns_client_alloc(struct kobject *parent,
 void rpc_netns_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;
 
 	rpc_client = rpc_netns_client_alloc(rpc_client_kobj, net, clnt->cl_clid);
 	if (rpc_client) {
 		clnt->cl_sysfs = rpc_client;
+		rpc_client->clnt = clnt;
 		kobject_uevent(&rpc_client->kobject, KOBJ_ADD);
 	}
 }
diff --git a/net/sunrpc/sysfs.h b/net/sunrpc/sysfs.h
index 279a836594e7..137a12c87954 100644
--- a/net/sunrpc/sysfs.h
+++ b/net/sunrpc/sysfs.h
@@ -8,6 +8,7 @@
 struct rpc_netns_client {
 	struct kobject kobject;
 	struct net *net;
+	struct rpc_clnt *clnt;
 };
 
 extern struct kobject *rpc_client_kobj;
-- 
2.29.2

[RFC PATCH 6/7] sunrpc: Prepare xs_connect() for taking NULL tasks

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

We won't have a task structure when we go to change IP addresses, so
check for one before calling the WARN_ON() to avoid crashing.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 net/sunrpc/xprtsock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index c56a66cdf4ac..250abf1aa018 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2311,7 +2311,8 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 	struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
 	unsigned long delay = 0;
 
-	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
+	if (task)
+		WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
 	if (transport->sock != NULL) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
-- 
2.29.2

[RFC PATCH 7/7] sunrpc: Connect to a new IP address provided by the user

[schumaker.anna]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

We preserve the same port number, rather than providing a way to change
it. This keeps the implementation simpler for now.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 net/sunrpc/sysfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 537d83635670..47a7c9b8b143 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -70,6 +70,13 @@ static ssize_t rpc_netns_address_show(struct kobject *kobj,
 static ssize_t rpc_netns_address_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;
+
+	xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr, sizeof(*saddr));
 	return count;
 }
 
-- 
2.29.2

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Chuck Lever]

Hi Anna-

> On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
> 
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> It's possible for an NFS server to go down but come back up with a
> different IP address. These patches provide a way for administrators to
> handle this issue by providing a new IP address for xprt sockets to
> connect to.
> 
> This is a first draft of the code, so any thoughts or suggestions would
> be greatly appreciated!

One implementation question, one future question.

Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?

Do you have a plan to integrate support for fs_locations to probe
servers for alternate IP addresses? Would that be a userspace
utility that would plug values into this new /sys API?


> Anna
> 
> 
> Anna Schumaker (7):
>  net: Add a /sys/net directory to sysfs
>  sunrpc: Create a sunrpc directory under /sys/net/
>  sunrpc: Create a net/ subdirectory in the sunrpc sysfs
>  sunrpc: Create per-rpc_clnt sysfs kobjects
>  sunrpc: Create a per-rpc_clnt file for managing the IP address
>  sunrpc: Prepare xs_connect() for taking NULL tasks
>  sunrpc: Connect to a new IP address provided by the user
> 
> include/linux/sunrpc/clnt.h |   1 +
> include/net/sock.h          |   4 +
> net/socket.c                |   8 ++
> net/sunrpc/Makefile         |   2 +-
> net/sunrpc/clnt.c           |   5 ++
> net/sunrpc/sunrpc_syms.c    |   8 ++
> net/sunrpc/sysfs.c          | 160 ++++++++++++++++++++++++++++++++++++
> net/sunrpc/sysfs.h          |  22 +++++
> net/sunrpc/xprtsock.c       |   3 +-
> 9 files changed, 211 insertions(+), 2 deletions(-)
> create mode 100644 net/sunrpc/sysfs.c
> create mode 100644 net/sunrpc/sysfs.h
> 
> -- 
> 2.29.2
> 

--
Chuck Lever

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[J. Bruce Fields]

On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
> Hi Anna-
> 
> > On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
> > 
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > 
> > It's possible for an NFS server to go down but come back up with a
> > different IP address. These patches provide a way for administrators to
> > handle this issue by providing a new IP address for xprt sockets to
> > connect to.
> > 
> > This is a first draft of the code, so any thoughts or suggestions would
> > be greatly appreciated!
> 
> One implementation question, one future question.
> 
> Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?
> 
> Do you have a plan to integrate support for fs_locations to probe
> servers for alternate IP addresses? Would that be a userspace
> utility that would plug values into this new /sys API?

We already have dns resolution for fs_locations, right?  Why can't we
use that here?  Is it that the mount call doesn't give us a host name?
Or we don't trust dns to have the updated IP address for some reason?

--b.

> 
> 
> > Anna
> > 
> > 
> > Anna Schumaker (7):
> >  net: Add a /sys/net directory to sysfs
> >  sunrpc: Create a sunrpc directory under /sys/net/
> >  sunrpc: Create a net/ subdirectory in the sunrpc sysfs
> >  sunrpc: Create per-rpc_clnt sysfs kobjects
> >  sunrpc: Create a per-rpc_clnt file for managing the IP address
> >  sunrpc: Prepare xs_connect() for taking NULL tasks
> >  sunrpc: Connect to a new IP address provided by the user
> > 
> > include/linux/sunrpc/clnt.h |   1 +
> > include/net/sock.h          |   4 +
> > net/socket.c                |   8 ++
> > net/sunrpc/Makefile         |   2 +-
> > net/sunrpc/clnt.c           |   5 ++
> > net/sunrpc/sunrpc_syms.c    |   8 ++
> > net/sunrpc/sysfs.c          | 160 ++++++++++++++++++++++++++++++++++++
> > net/sunrpc/sysfs.h          |  22 +++++
> > net/sunrpc/xprtsock.c       |   3 +-
> > 9 files changed, 211 insertions(+), 2 deletions(-)
> > create mode 100644 net/sunrpc/sysfs.c
> > create mode 100644 net/sunrpc/sysfs.h
> > 
> > -- 
> > 2.29.2
> > 
> 
> --
> Chuck Lever
> 
> 

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Anna Schumaker]

On Tue, Jan 12, 2021 at 11:59 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
> > Hi Anna-
> >
> > > On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
> > >
> > > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > >
> > > It's possible for an NFS server to go down but come back up with a
> > > different IP address. These patches provide a way for administrators to
> > > handle this issue by providing a new IP address for xprt sockets to
> > > connect to.
> > >
> > > This is a first draft of the code, so any thoughts or suggestions would
> > > be greatly appreciated!
> >
> > One implementation question, one future question.
> >
> > Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?

Possibly! I was trying to match /sys/fs/nfs, but I can definitely
change this if another location is better.

> >
> > Do you have a plan to integrate support for fs_locations to probe
> > servers for alternate IP addresses? Would that be a userspace
> > utility that would plug values into this new /sys API?

Yeah, I would expect there to be a new utility to help with assigning
new values. I haven't given any thought to using fs_locations yet, but
it could probably work.
>
> We already have dns resolution for fs_locations, right?  Why can't we
> use that here?  Is it that the mount call doesn't give us a host name?
> Or we don't trust dns to have the updated IP address for some reason?

The mount call doesn't give us a host name (that I can find, at
least). By the time we get to the sunrpc layer we're dealing with just
the IP address anyway. I'd expect there to be a userland utility to
translate the dns name to the new IP address and pass it along to the
new API.

Anna
>
> --b.
>
> >
> >
> > > Anna
> > >
> > >
> > > Anna Schumaker (7):
> > >  net: Add a /sys/net directory to sysfs
> > >  sunrpc: Create a sunrpc directory under /sys/net/
> > >  sunrpc: Create a net/ subdirectory in the sunrpc sysfs
> > >  sunrpc: Create per-rpc_clnt sysfs kobjects
> > >  sunrpc: Create a per-rpc_clnt file for managing the IP address
> > >  sunrpc: Prepare xs_connect() for taking NULL tasks
> > >  sunrpc: Connect to a new IP address provided by the user
> > >
> > > include/linux/sunrpc/clnt.h |   1 +
> > > include/net/sock.h          |   4 +
> > > net/socket.c                |   8 ++
> > > net/sunrpc/Makefile         |   2 +-
> > > net/sunrpc/clnt.c           |   5 ++
> > > net/sunrpc/sunrpc_syms.c    |   8 ++
> > > net/sunrpc/sysfs.c          | 160 ++++++++++++++++++++++++++++++++++++
> > > net/sunrpc/sysfs.h          |  22 +++++
> > > net/sunrpc/xprtsock.c       |   3 +-
> > > 9 files changed, 211 insertions(+), 2 deletions(-)
> > > create mode 100644 net/sunrpc/sysfs.c
> > > create mode 100644 net/sunrpc/sysfs.h
> > >
> > > --
> > > 2.29.2
> > >
> >
> > --
> > Chuck Lever
> >
> >

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Chuck Lever]

> On Jan 13, 2021, at 2:23 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
> 
> On Tue, Jan 12, 2021 at 11:59 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>> 
>> On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
>>> Hi Anna-
>>> 
>>>> On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
>>>> 
>>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>> 
>>>> It's possible for an NFS server to go down but come back up with a
>>>> different IP address. These patches provide a way for administrators to
>>>> handle this issue by providing a new IP address for xprt sockets to
>>>> connect to.
>>>> 
>>>> This is a first draft of the code, so any thoughts or suggestions would
>>>> be greatly appreciated!
>>> 
>>> One implementation question, one future question.
>>> 
>>> Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?
> 
> Possibly! I was trying to match /sys/fs/nfs, but I can definitely
> change this if another location is better.

Ah... since this is a supplement to the mount() interface, maybe
placing this new API under /sys/fs/nfs/ might make some sense.


>>> Do you have a plan to integrate support for fs_locations to probe
>>> servers for alternate IP addresses? Would that be a userspace
>>> utility that would plug values into this new /sys API?
> 
> Yeah, I would expect there to be a new utility to help with assigning
> new values. I haven't given any thought to using fs_locations yet, but
> it could probably work.

I could see a tool that performs an fs_locations from user space
and plugs that information into the kernel NFS client as needed.
Future work!


--
Chuck Lever

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Chuck Lever]

> On Jan 13, 2021, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> On Jan 13, 2021, at 2:23 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
>> 
>> On Tue, Jan 12, 2021 at 11:59 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>>> 
>>> On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
>>>> Hi Anna-
>>>> 
>>>>> On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
>>>>> 
>>>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>> 
>>>>> It's possible for an NFS server to go down but come back up with a
>>>>> different IP address. These patches provide a way for administrators to
>>>>> handle this issue by providing a new IP address for xprt sockets to
>>>>> connect to.
>>>>> 
>>>>> This is a first draft of the code, so any thoughts or suggestions would
>>>>> be greatly appreciated!
>>>> 
>>>> One implementation question, one future question.
>>>> 
>>>> Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?
>> 
>> Possibly! I was trying to match /sys/fs/nfs, but I can definitely
>> change this if another location is better.
> 
> Ah... since this is a supplement to the mount() interface, maybe
> placing this new API under /sys/fs/nfs/ might make some sense.

Or you could implement it with "-o remount,addr=new-address".


--
Chuck Lever

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Olga Kornievskaia]

On Wed, Jan 13, 2021 at 9:18 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
>
> > On Jan 13, 2021, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> >
> >> On Jan 13, 2021, at 2:23 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
> >>
> >> On Tue, Jan 12, 2021 at 11:59 AM J. Bruce Fields <bfields@fieldses.org> wrote:
> >>>
> >>> On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
> >>>> Hi Anna-
> >>>>
> >>>>> On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
> >>>>>
> >>>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >>>>>
> >>>>> It's possible for an NFS server to go down but come back up with a
> >>>>> different IP address. These patches provide a way for administrators to
> >>>>> handle this issue by providing a new IP address for xprt sockets to
> >>>>> connect to.
> >>>>>
> >>>>> This is a first draft of the code, so any thoughts or suggestions would
> >>>>> be greatly appreciated!
> >>>>
> >>>> One implementation question, one future question.
> >>>>
> >>>> Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?
> >>
> >> Possibly! I was trying to match /sys/fs/nfs, but I can definitely
> >> change this if another location is better.
> >
> > Ah... since this is a supplement to the mount() interface, maybe
> > placing this new API under /sys/fs/nfs/ might make some sense.
>
> Or you could implement it with "-o remount,addr=new-address".

A change of address is currently not allowed by the NFS because
multiple mounts might be sharing a superblock and change of one
mount's option would not be correct. The way things work from this new
mechanism is system wide and all mounts are affected.


>
>
> --
> Chuck Lever
>
>
>

Re: [RFC PATCH 0/7] SUNRPC: Create sysfs files for changing IP

[Chuck Lever]

> On Jan 14, 2021, at 3:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> 
> On Wed, Jan 13, 2021 at 9:18 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>> 
>>> On Jan 13, 2021, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>>> On Jan 13, 2021, at 2:23 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
>>>> 
>>>> On Tue, Jan 12, 2021 at 11:59 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>>>>> 
>>>>> On Tue, Jan 12, 2021 at 08:09:09AM -0500, Chuck Lever wrote:
>>>>>> Hi Anna-
>>>>>> 
>>>>>>> On Jan 11, 2021, at 4:41 PM, schumaker.anna@gmail.com wrote:
>>>>>>> 
>>>>>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>>>> 
>>>>>>> It's possible for an NFS server to go down but come back up with a
>>>>>>> different IP address. These patches provide a way for administrators to
>>>>>>> handle this issue by providing a new IP address for xprt sockets to
>>>>>>> connect to.
>>>>>>> 
>>>>>>> This is a first draft of the code, so any thoughts or suggestions would
>>>>>>> be greatly appreciated!
>>>>>> 
>>>>>> One implementation question, one future question.
>>>>>> 
>>>>>> Would /sys/kernel/net be a little better? or /sys/kernel/sunrpc ?
>>>> 
>>>> Possibly! I was trying to match /sys/fs/nfs, but I can definitely
>>>> change this if another location is better.
>>> 
>>> Ah... since this is a supplement to the mount() interface, maybe
>>> placing this new API under /sys/fs/nfs/ might make some sense.
>> 
>> Or you could implement it with "-o remount,addr=new-address".
> 
> A change of address is currently not allowed by the NFS because
> multiple mounts might be sharing a superblock and change of one
> mount's option would not be correct. The way things work from this new
> mechanism is system wide and all mounts are affected.

OK, well, if we're going with an API based on /sys that shows
underlying transport connections, is there a way to expose
whether the connection is established or closed? Maybe also
last traffic or last connect attempt?

Can it support RPC/RDMA connections too?


--
Chuck Lever