2021-06-03 22:23:30

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH v9 10/13] sunrpc: add dst_attr attributes to the sysfs xprt directory

From: Olga Kornievskaia <[email protected]>

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 <[email protected]>
---
include/linux/sunrpc/xprt.h | 1 +
net/sunrpc/sysfs.c | 99 +++++++++++++++++++++++++++++++++++++
net/sunrpc/xprt.c | 4 +-
net/sunrpc/xprtmultipath.c | 2 -
4 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 8360db664e5f..13a4eaf385cf 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -414,6 +414,7 @@ void xprt_conditional_disconnect(struct rpc_xprt *xprt, unsigned int cookie);

bool xprt_lock_connect(struct rpc_xprt *, struct rpc_task *, void *);
void xprt_unlock_connect(struct rpc_xprt *, void *);
+void xprt_release_write(struct rpc_xprt *, struct rpc_task *);

/*
* Reserved bit positions in xprt->state
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index 20f75708594f..4a14342e4d4e 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -4,8 +4,23 @@
*/
#include <linux/sunrpc/clnt.h>
#include <linux/kobject.h>
+#include <linux/sunrpc/addr.h>
+
#include "sysfs.h"

+struct xprt_addr {
+ const char *addr;
+ struct rcu_head rcu;
+};
+
+static void free_xprt_addr(struct rcu_head *head)
+{
+ struct xprt_addr *addr = container_of(head, struct xprt_addr, rcu);
+
+ kfree(addr->addr);
+ kfree(addr);
+}
+
static struct kset *rpc_sunrpc_kset;
static struct kobject *rpc_sunrpc_client_kobj, *rpc_sunrpc_xprt_switch_kobj;

@@ -43,6 +58,81 @@ 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 *x = container_of(kobj,
+ struct rpc_sysfs_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;
+ ret = sprintf(buf, "%s\n", xprt->address_strings[RPC_DISPLAY_ADDR]);
+ 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;
+ char *dst_addr;
+ int port;
+ struct xprt_addr *saved_addr;
+
+ if (!xprt)
+ return 0;
+ if (!(xprt->xprt_class->ident == XPRT_TRANSPORT_TCP ||
+ xprt->xprt_class->ident == XPRT_TRANSPORT_RDMA)) {
+ xprt_put(xprt);
+ return -EOPNOTSUPP;
+ }
+
+ if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED, TASK_KILLABLE)) {
+ count = -EINTR;
+ goto out_put;
+ }
+ saddr = (struct sockaddr *)&xprt->addr;
+ port = rpc_get_port(saddr);
+
+ dst_addr = kstrndup(buf, count - 1, GFP_KERNEL);
+ if (!dst_addr)
+ goto out_err;
+ saved_addr = kzalloc(sizeof(*saved_addr), GFP_KERNEL);
+ if (!saved_addr)
+ goto out_err_free;
+ saved_addr->addr =
+ rcu_dereference_raw(xprt->address_strings[RPC_DISPLAY_ADDR]);
+ rcu_assign_pointer(xprt->address_strings[RPC_DISPLAY_ADDR], dst_addr);
+ call_rcu(&saved_addr->rcu, free_xprt_addr);
+ xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr,
+ sizeof(*saddr));
+ rpc_set_port(saddr, port);
+
+ xprt_force_disconnect(xprt);
+out:
+ xprt_release_write(xprt, NULL);
+out_put:
+ xprt_put(xprt);
+ return count;
+out_err_free:
+ kfree(dst_addr);
+out_err:
+ count = -ENOMEM;
+ goto out;
+}
+
int rpc_sysfs_init(void)
{
rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL, kernel_kobj);
@@ -106,6 +196,14 @@ static const void *rpc_sysfs_xprt_namespace(struct kobject *kobj)
kobject)->xprt->xprt_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,
@@ -120,6 +218,7 @@ static struct kobj_type rpc_sysfs_xprt_switch_type = {

static struct kobj_type rpc_sysfs_xprt_type = {
.release = rpc_sysfs_xprt_release,
+ .default_attrs = rpc_sysfs_xprt_attrs,
.sysfs_ops = &kobj_sysfs_ops,
.namespace = rpc_sysfs_xprt_namespace,
};
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 20b9bd705014..fb6db09725c7 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
@@ -443,7 +444,7 @@ void xprt_release_xprt_cong(struct rpc_xprt *xprt, struct rpc_task *task)
}
EXPORT_SYMBOL_GPL(xprt_release_xprt_cong);

-static inline void xprt_release_write(struct rpc_xprt *xprt, struct rpc_task *task)
+void xprt_release_write(struct rpc_xprt *xprt, struct rpc_task *task)
{
if (xprt->snd_task != task)
return;
@@ -1812,6 +1813,7 @@ void xprt_free(struct rpc_xprt *xprt)
put_net(xprt->xprt_net);
xprt_free_all_slots(xprt);
xprt_free_id(xprt);
+ rpc_sysfs_xprt_destroy(xprt);
kfree_rcu(xprt, rcu);
}
EXPORT_SYMBOL_GPL(xprt_free);
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index e7973c1ff70c..07e76ae1028a 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -86,7 +86,6 @@ 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);
- rpc_sysfs_xprt_destroy(xprt);
xprt_put(xprt);
}

@@ -155,7 +154,6 @@ static void xprt_switch_free_entries(struct rpc_xprt_switch *xps)
struct rpc_xprt, xprt_switch);
xprt_switch_remove_xprt_locked(xps, xprt);
spin_unlock(&xps->xps_lock);
- rpc_sysfs_xprt_destroy(xprt);
xprt_put(xprt);
spin_lock(&xps->xps_lock);
}
--
2.27.0


2021-06-07 22:00:09

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v9 10/13] sunrpc: add dst_attr attributes to the sysfs xprt directory

On Mon, 2021-06-07 at 17:03 -0400, Anna Schumaker wrote:
> Hi Olga,
>
> On Thu, Jun 3, 2021 at 6:23 PM Olga Kornievskaia
> <[email protected]> wrote:
> >
> > From: Olga Kornievskaia <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >  include/linux/sunrpc/xprt.h |  1 +
> >  net/sunrpc/sysfs.c          | 99
> > +++++++++++++++++++++++++++++++++++++
> >  net/sunrpc/xprt.c           |  4 +-
> >  net/sunrpc/xprtmultipath.c  |  2 -
> >  4 files changed, 103 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/xprt.h
> > b/include/linux/sunrpc/xprt.h
> > index 8360db664e5f..13a4eaf385cf 100644
> > --- a/include/linux/sunrpc/xprt.h
> > +++ b/include/linux/sunrpc/xprt.h
> > @@ -414,6 +414,7 @@ void                       
> > xprt_conditional_disconnect(struct rpc_xprt *xprt, unsigned int
> > cookie);
> >
> >  bool                   xprt_lock_connect(struct rpc_xprt *, struct
> > rpc_task *, void *);
> >  void                   xprt_unlock_connect(struct rpc_xprt *, void
> > *);
> > +void                   xprt_release_write(struct rpc_xprt *,
> > struct rpc_task *);
> >
> >  /*
> >   * Reserved bit positions in xprt->state
> > diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> > index 20f75708594f..4a14342e4d4e 100644
> > --- a/net/sunrpc/sysfs.c
> > +++ b/net/sunrpc/sysfs.c
> > @@ -4,8 +4,23 @@
> >   */
> >  #include <linux/sunrpc/clnt.h>
> >  #include <linux/kobject.h>
> > +#include <linux/sunrpc/addr.h>
> > +
> >  #include "sysfs.h"
> >
> > +struct xprt_addr {
> > +       const char *addr;
> > +       struct rcu_head rcu;
> > +};
> > +
> > +static void free_xprt_addr(struct rcu_head *head)
> > +{
> > +       struct xprt_addr *addr = container_of(head, struct
> > xprt_addr, rcu);
> > +
> > +       kfree(addr->addr);
> > +       kfree(addr);
> > +}
> > +
> >  static struct kset *rpc_sunrpc_kset;
> >  static struct kobject *rpc_sunrpc_client_kobj,
> > *rpc_sunrpc_xprt_switch_kobj;
> >
> > @@ -43,6 +58,81 @@ 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 *x = container_of(kobj,
> > +               struct rpc_sysfs_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;
> > +       ret = sprintf(buf, "%s\n", xprt-
> > >address_strings[RPC_DISPLAY_ADDR]);
> > +       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;
> > +       char *dst_addr;
> > +       int port;
> > +       struct xprt_addr *saved_addr;
> > +
> > +       if (!xprt)
> > +               return 0;
> > +       if (!(xprt->xprt_class->ident == XPRT_TRANSPORT_TCP ||
> > +             xprt->xprt_class->ident == XPRT_TRANSPORT_RDMA)) {
> > +               xprt_put(xprt);
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> > +       if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED,
> > TASK_KILLABLE)) {
> > +               count = -EINTR;
> > +               goto out_put;
> > +       }
> > +       saddr = (struct sockaddr *)&xprt->addr;
> > +       port = rpc_get_port(saddr);
> > +
> > +       dst_addr = kstrndup(buf, count - 1, GFP_KERNEL);
> > +       if (!dst_addr)
> > +               goto out_err;
> > +       saved_addr = kzalloc(sizeof(*saved_addr), GFP_KERNEL);
> > +       if (!saved_addr)
> > +               goto out_err_free;
> > +       saved_addr->addr =
> > +               rcu_dereference_raw(xprt-
> > >address_strings[RPC_DISPLAY_ADDR]);
> > +       rcu_assign_pointer(xprt->address_strings[RPC_DISPLAY_ADDR],
> > dst_addr);
> > +       call_rcu(&saved_addr->rcu, free_xprt_addr);
> > +       xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1,
> > saddr,
>
> The "count - 1" has been there since the beginning to account for
> NULL
> terminated strings (like those passed using `echo` into the file).
> Now
> that I'm working on the userspace side of things, I've realized this
> isn't necessarily always the case. I'm wondering if it would make
> sense to only decrement count if buf[count - 1] is "\0", but
> otherwise
> leave it alone?
>
> I found this by writing "192.168.111.186" through python, but the
> resulting dstaddr was "192.168.111.18" when I tried reading it back.

kstrndup(buf, count, GFP_KERNEL) would suffice to strip an excess '\0'.
I'm assuming the real problem here is the '\n'?

>
> Anna
>
> > +                                sizeof(*saddr));
> > +       rpc_set_port(saddr, port);
> > +
> > +       xprt_force_disconnect(xprt);
> > +out:
> > +       xprt_release_write(xprt, NULL);
> > +out_put:
> > +       xprt_put(xprt);
> > +       return count;
> > +out_err_free:
> > +       kfree(dst_addr);
> > +out_err:
> > +       count = -ENOMEM;
> > +       goto out;
> > +}
> > +
> >  int rpc_sysfs_init(void)
> >  {
> >         rpc_sunrpc_kset = kset_create_and_add("sunrpc", NULL,
> > kernel_kobj);
> > @@ -106,6 +196,14 @@ static const void
> > *rpc_sysfs_xprt_namespace(struct kobject *kobj)
> >                             kobject)->xprt->xprt_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,
> > @@ -120,6 +218,7 @@ static struct kobj_type
> > rpc_sysfs_xprt_switch_type = {
> >
> >  static struct kobj_type rpc_sysfs_xprt_type = {
> >         .release = rpc_sysfs_xprt_release,
> > +       .default_attrs = rpc_sysfs_xprt_attrs,
> >         .sysfs_ops = &kobj_sysfs_ops,
> >         .namespace = rpc_sysfs_xprt_namespace,
> >  };
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > index 20b9bd705014..fb6db09725c7 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
> > @@ -443,7 +444,7 @@ void xprt_release_xprt_cong(struct rpc_xprt
> > *xprt, struct rpc_task *task)
> >  }
> >  EXPORT_SYMBOL_GPL(xprt_release_xprt_cong);
> >
> > -static inline void xprt_release_write(struct rpc_xprt *xprt,
> > struct rpc_task *task)
> > +void xprt_release_write(struct rpc_xprt *xprt, struct rpc_task
> > *task)
> >  {
> >         if (xprt->snd_task != task)
> >                 return;
> > @@ -1812,6 +1813,7 @@ void xprt_free(struct rpc_xprt *xprt)
> >         put_net(xprt->xprt_net);
> >         xprt_free_all_slots(xprt);
> >         xprt_free_id(xprt);
> > +       rpc_sysfs_xprt_destroy(xprt);
> >         kfree_rcu(xprt, rcu);
> >  }
> >  EXPORT_SYMBOL_GPL(xprt_free);
> > diff --git a/net/sunrpc/xprtmultipath.c
> > b/net/sunrpc/xprtmultipath.c
> > index e7973c1ff70c..07e76ae1028a 100644
> > --- a/net/sunrpc/xprtmultipath.c
> > +++ b/net/sunrpc/xprtmultipath.c
> > @@ -86,7 +86,6 @@ 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);
> > -       rpc_sysfs_xprt_destroy(xprt);
> >         xprt_put(xprt);
> >  }
> >
> > @@ -155,7 +154,6 @@ static void xprt_switch_free_entries(struct
> > rpc_xprt_switch *xps)
> >                                 struct rpc_xprt, xprt_switch);
> >                 xprt_switch_remove_xprt_locked(xps, xprt);
> >                 spin_unlock(&xps->xps_lock);
> > -               rpc_sysfs_xprt_destroy(xprt);
> >                 xprt_put(xprt);
> >                 spin_lock(&xps->xps_lock);
> >         }
> > --
> > 2.27.0
> >

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]