2021-02-15 17:42:06

by Dan Aloni

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

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 <[email protected]>
---
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


2021-02-16 21:50:10

by Anna Schumaker

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

Hi Dan,

On Mon, Feb 15, 2021 at 12:42 PM Dan Aloni <[email protected]> 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 <[email protected]>
> ---
> 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
>

2021-02-17 21:03:53

by Dan Aloni

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

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