Hi Olga,
On Thu, Jun 3, 2021 at 6:23 PM Olga Kornievskaia
<olga.kornievskaia@xxxxxxxxx> wrote:
>
> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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.
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
>
