diff mbox series

[v3,11/13] sunrpc: add dst_attr attributes to the sysfs xprt directory

Message ID 20210426171947.99233-12-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series create sysfs files for changing IP address | expand

Commit Message

Olga Kornievskaia April 26, 2021, 5:19 p.m. UTC
From: Olga Kornievskaia <kolga@netapp.com>

Allow to query and set the destination's address of a transport.
Setting of the destination address is allowed only for TCP or RDMA
based connections.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 net/sunrpc/sysfs.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

Trond Myklebust April 27, 2021, 1:26 p.m. UTC | #1
On Mon, 2021-04-26 at 13:19 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Allow to query and set the destination's address of a transport.
> Setting of the destination address is allowed only for TCP or RDMA
> based connections.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  net/sunrpc/sysfs.c | 72
> ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index 682def4293f2..076f777db3cd 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -4,6 +4,9 @@
>   */
>  #include <linux/sunrpc/clnt.h>
>  #include <linux/kobject.h>
> +#include <linux/sunrpc/addr.h>
> +#include <linux/sunrpc/xprtsock.h>
> +
>  #include "sysfs.h"
>  
>  static struct kset *rpc_sunrpc_kset;
> @@ -42,6 +45,66 @@ static struct kobject
> *rpc_sysfs_object_alloc(const char *name,
>         return NULL;
>  }
>  
> +static inline struct rpc_xprt *
> +rpc_sysfs_xprt_kobj_get_xprt(struct kobject *kobj)
> +{
> +       struct rpc_sysfs_xprt_switch_xprt *x = container_of(kobj,
> +               struct rpc_sysfs_xprt_switch_xprt, kobject);
> +
> +       return xprt_get(x->xprt);
> +}
> +
> +static ssize_t rpc_sysfs_xprt_dstaddr_show(struct kobject *kobj,
> +                                          struct kobj_attribute
> *attr,
> +                                          char *buf)
> +{
> +       struct rpc_xprt *xprt = rpc_sysfs_xprt_kobj_get_xprt(kobj);
> +       ssize_t ret;
> +
> +       if (!xprt)
> +               return 0;
> +       if (xprt->xprt_class->ident == XPRT_TRANSPORT_LOCAL)
> +               ret = sprintf(buf, "localhost");

This makes it look like it is a loopback socket, whereas in reality it
is a named socket. Why not just use the xprt-
>address_strings[RPC_DISPLAY_ADDR] here?

> +       else
> +               ret = rpc_ntop((struct sockaddr *)&xprt->addr, buf,
> PAGE_SIZE);
> +       buf[ret] = '\n';
> +       xprt_put(xprt);
> +       return ret + 1;
> +}
> +
> +static ssize_t rpc_sysfs_xprt_dstaddr_store(struct kobject *kobj,
> +                                           struct kobj_attribute
> *attr,
> +                                           const char *buf, size_t
> count)
> +{
> +       struct rpc_xprt *xprt = rpc_sysfs_xprt_kobj_get_xprt(kobj);
> +       struct sockaddr *saddr;
> +       int port;
> +
> +       if (!xprt)
> +               return 0;
> +       if (!(xprt->xprt_class->ident == XPRT_TRANSPORT_TCP ||
> +             xprt->xprt_class->ident == XPRT_TRANSPORT_RDMA)) {
> +               xprt_put(xprt);
> +               return -EOPNOTSUPP;
> +       }
> +
> +       wait_on_bit_lock(&xprt->state, XPRT_LOCKED, TASK_KILLABLE);
> +       saddr = (struct sockaddr *)&xprt->addr;
> +       port = rpc_get_port(saddr);
> +
> +       kfree(xprt->address_strings[RPC_DISPLAY_ADDR]);

This is not allowed. The xprt->address_strings can only be freed in a
manner that is safe w.r.t. the rcu_read_lock(). Otherwise, various
users of rpc_peeraddr2str() are prone to crash.

So if you're going to replace these strings, then you have to replace
them using something like rcu_replace_pointer(), then you have to free
them using call_rcu() or kfree_rcu}().

> +       xprt->address_strings[RPC_DISPLAY_ADDR] = kstrndup(buf, count
> - 1,
> +                                                         
> GFP_KERNEL);
> +       xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1,
> saddr,
> +                                sizeof(*saddr));
> +       rpc_set_port(saddr, port);

The above is also a bit dodgy w.r.t. rpc_peeraddr() since it is non-
atomic. However it looks like it should be OK in practice.

> +
> +       xprt->ops->connect(xprt, NULL);
> +       clear_bit(XPRT_LOCKED, &xprt->state);
> +       xprt_put(xprt);
> +       return count;
> +}
> +
>  int rpc_sysfs_init(void)
>  {
>         rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL,
> kernel_kobj);
> @@ -105,6 +168,14 @@ static const void
> *rpc_sysfs_xprt_switch_xprt_namespace(struct kobject *kobj)
>                             kobject)->net;
>  }
>  
> +static struct kobj_attribute rpc_sysfs_xprt_dstaddr =
> __ATTR(dstaddr,
> +       0644, rpc_sysfs_xprt_dstaddr_show,
> rpc_sysfs_xprt_dstaddr_store);
> +
> +static struct attribute *rpc_sysfs_xprt_attrs[] = {
> +       &rpc_sysfs_xprt_dstaddr.attr,
> +       NULL,
> +};
> +
>  static struct kobj_type rpc_sysfs_client_type = {
>         .release = rpc_sysfs_client_release,
>         .sysfs_ops = &kobj_sysfs_ops,
> @@ -119,6 +190,7 @@ static struct kobj_type
> rpc_sysfs_xprt_switch_type = {
>  
>  static struct kobj_type rpc_sysfs_xprt_switch_xprt_type = {
>         .release = rpc_sysfs_xprt_switch_xprt_release,
> +       .default_attrs = rpc_sysfs_xprt_attrs,
>         .sysfs_ops = &kobj_sysfs_ops,
>         .namespace = rpc_sysfs_xprt_switch_xprt_namespace,
>  };
diff mbox series

Patch

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 682def4293f2..076f777db3cd 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -4,6 +4,9 @@ 
  */
 #include <linux/sunrpc/clnt.h>
 #include <linux/kobject.h>
+#include <linux/sunrpc/addr.h>
+#include <linux/sunrpc/xprtsock.h>
+
 #include "sysfs.h"
 
 static struct kset *rpc_sunrpc_kset;
@@ -42,6 +45,66 @@  static struct kobject *rpc_sysfs_object_alloc(const char *name,
 	return NULL;
 }
 
+static inline struct rpc_xprt *
+rpc_sysfs_xprt_kobj_get_xprt(struct kobject *kobj)
+{
+	struct rpc_sysfs_xprt_switch_xprt *x = container_of(kobj,
+		struct rpc_sysfs_xprt_switch_xprt, kobject);
+
+	return xprt_get(x->xprt);
+}
+
+static ssize_t rpc_sysfs_xprt_dstaddr_show(struct kobject *kobj,
+					   struct kobj_attribute *attr,
+					   char *buf)
+{
+	struct rpc_xprt *xprt = rpc_sysfs_xprt_kobj_get_xprt(kobj);
+	ssize_t ret;
+
+	if (!xprt)
+		return 0;
+	if (xprt->xprt_class->ident == XPRT_TRANSPORT_LOCAL)
+		ret = sprintf(buf, "localhost");
+	else
+		ret = rpc_ntop((struct sockaddr *)&xprt->addr, buf, PAGE_SIZE);
+	buf[ret] = '\n';
+	xprt_put(xprt);
+	return ret + 1;
+}
+
+static ssize_t rpc_sysfs_xprt_dstaddr_store(struct kobject *kobj,
+					    struct kobj_attribute *attr,
+					    const char *buf, size_t count)
+{
+	struct rpc_xprt *xprt = rpc_sysfs_xprt_kobj_get_xprt(kobj);
+	struct sockaddr *saddr;
+	int port;
+
+	if (!xprt)
+		return 0;
+	if (!(xprt->xprt_class->ident == XPRT_TRANSPORT_TCP ||
+	      xprt->xprt_class->ident == XPRT_TRANSPORT_RDMA)) {
+		xprt_put(xprt);
+		return -EOPNOTSUPP;
+	}
+
+	wait_on_bit_lock(&xprt->state, XPRT_LOCKED, TASK_KILLABLE);
+	saddr = (struct sockaddr *)&xprt->addr;
+	port = rpc_get_port(saddr);
+
+	kfree(xprt->address_strings[RPC_DISPLAY_ADDR]);
+	xprt->address_strings[RPC_DISPLAY_ADDR] = kstrndup(buf, count - 1,
+							   GFP_KERNEL);
+	xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr,
+				 sizeof(*saddr));
+	rpc_set_port(saddr, port);
+
+	xprt->ops->connect(xprt, NULL);
+	clear_bit(XPRT_LOCKED, &xprt->state);
+	xprt_put(xprt);
+	return count;
+}
+
 int rpc_sysfs_init(void)
 {
 	rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
@@ -105,6 +168,14 @@  static const void *rpc_sysfs_xprt_switch_xprt_namespace(struct kobject *kobj)
 			    kobject)->net;
 }
 
+static struct kobj_attribute rpc_sysfs_xprt_dstaddr = __ATTR(dstaddr,
+	0644, rpc_sysfs_xprt_dstaddr_show, rpc_sysfs_xprt_dstaddr_store);
+
+static struct attribute *rpc_sysfs_xprt_attrs[] = {
+	&rpc_sysfs_xprt_dstaddr.attr,
+	NULL,
+};
+
 static struct kobj_type rpc_sysfs_client_type = {
 	.release = rpc_sysfs_client_release,
 	.sysfs_ops = &kobj_sysfs_ops,
@@ -119,6 +190,7 @@  static struct kobj_type rpc_sysfs_xprt_switch_type = {
 
 static struct kobj_type rpc_sysfs_xprt_switch_xprt_type = {
 	.release = rpc_sysfs_xprt_switch_xprt_release,
+	.default_attrs = rpc_sysfs_xprt_attrs,
 	.sysfs_ops = &kobj_sysfs_ops,
 	.namespace = rpc_sysfs_xprt_switch_xprt_namespace,
 };