On 1/9/25 10:10 AM, Benjamin Coddington wrote:
> On 8 Jan 2025, at 16:36, Anna Schumaker wrote:
>
>> From: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
>>
>> Writing to this file will clone the 'main' xprt of an xprt_switch and
>> add it to be used as an additional connection.
>
> I like this too! I'd prefer it was ./add_xprt instead of
> ./xprt_switch_add_xprt, since the directory already gives the context that
> you're operating on the xprt_switch.
I'd prefer that too, actually. I don't know what I was thinking going with the longer name, and I've made that change for v2.
>
> After happily adding a bunch of xprts, I did have to look up the source to
> re-learn how to remove them which wasn't obvious from the sysfs structure.
> You have to write "offline", then "remove" to the xprt_state file. This
> made me wish there was just a ./xprt-N-tcp/del_xprt that would do both of
> those..
`rpcctl xprt remove` will already do both of those in one step, if that helps. But I can look at adding in 'del_xprt' if you still think it would help.
>
> .. and in stark contrast to my previous preference on less dynamic sysfs, I
> think that the ./del_xprt shouldn't appear for the "main" xprt (since you
> can't remove/delete them).
>
> .. all just thoughts, these look good!
Thanks!
Anna
>
>>
>> Signed-off-by: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
>> ---
>> include/linux/sunrpc/xprtmultipath.h | 1 +
>> net/sunrpc/sysfs.c | 54 ++++++++++++++++++++++++++++
>> net/sunrpc/xprtmultipath.c | 21 +++++++++++
>> 3 files changed, 76 insertions(+)
>>
>> diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
>> index c0514c684b2c..c827c6ef0bc5 100644
>> --- a/include/linux/sunrpc/xprtmultipath.h
>> +++ b/include/linux/sunrpc/xprtmultipath.h
>> @@ -56,6 +56,7 @@ extern void rpc_xprt_switch_add_xprt(struct rpc_xprt_switch *xps,
>> struct rpc_xprt *xprt);
>> extern void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
>> struct rpc_xprt *xprt, bool offline);
>> +extern struct rpc_xprt *rpc_xprt_switch_get_main_xprt(struct rpc_xprt_switch *xps);
>>
>> extern void xprt_iter_init(struct rpc_xprt_iter *xpi,
>> struct rpc_xprt_switch *xps);
>> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
>> index dc3b7cd70000..ce7dae140770 100644
>> --- a/net/sunrpc/sysfs.c
>> +++ b/net/sunrpc/sysfs.c
>> @@ -250,6 +250,55 @@ static ssize_t rpc_sysfs_xprt_switch_info_show(struct kobject *kobj,
>> return ret;
>> }
>>
>> +static ssize_t rpc_sysfs_xprt_switch_add_xprt_show(struct kobject *kobj,
>> + struct kobj_attribute *attr,
>> + char *buf)
>> +{
>> + return sprintf(buf, "# add one xprt to this xprt_switch\n");
>> +}
>> +
>> +static ssize_t rpc_sysfs_xprt_switch_add_xprt_store(struct kobject *kobj,
>> + struct kobj_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct rpc_xprt_switch *xprt_switch =
>> + rpc_sysfs_xprt_switch_kobj_get_xprt(kobj);
>> + struct xprt_create xprt_create_args;
>> + struct rpc_xprt *xprt, *new;
>> +
>> + if (!xprt_switch)
>> + return 0;
>> +
>> + xprt = rpc_xprt_switch_get_main_xprt(xprt_switch);
>> + if (!xprt)
>> + goto out;
>> +
>> + xprt_create_args.ident = xprt->xprt_class->ident;
>> + xprt_create_args.net = xprt->xprt_net;
>> + xprt_create_args.dstaddr = (struct sockaddr *)&xprt->addr;
>> + xprt_create_args.addrlen = xprt->addrlen;
>> + xprt_create_args.servername = xprt->servername;
>> + xprt_create_args.bc_xprt = xprt->bc_xprt;
>> + xprt_create_args.xprtsec = xprt->xprtsec;
>> + xprt_create_args.connect_timeout = xprt->connect_timeout;
>> + xprt_create_args.reconnect_timeout = xprt->max_reconnect_timeout;
>> +
>> + new = xprt_create_transport(&xprt_create_args);
>> + if (IS_ERR_OR_NULL(new)) {
>> + count = PTR_ERR(new);
>> + goto out_put_xprt;
>> + }
>> +
>> + rpc_xprt_switch_add_xprt(xprt_switch, new);
>> + xprt_put(new);
>> +
>> +out_put_xprt:
>> + xprt_put(xprt);
>> +out:
>> + xprt_switch_put(xprt_switch);
>> + return count;
>> +}
>> +
>> static ssize_t rpc_sysfs_xprt_dstaddr_store(struct kobject *kobj,
>> struct kobj_attribute *attr,
>> const char *buf, size_t count)
>> @@ -451,8 +500,13 @@ ATTRIBUTE_GROUPS(rpc_sysfs_xprt);
>> static struct kobj_attribute rpc_sysfs_xprt_switch_info =
>> __ATTR(xprt_switch_info, 0444, rpc_sysfs_xprt_switch_info_show, NULL);
>>
>> +static struct kobj_attribute rpc_sysfs_xprt_switch_add_xprt =
>> + __ATTR(xprt_switch_add_xprt, 0644, rpc_sysfs_xprt_switch_add_xprt_show,
>> + rpc_sysfs_xprt_switch_add_xprt_store);
>> +
>> static struct attribute *rpc_sysfs_xprt_switch_attrs[] = {
>> &rpc_sysfs_xprt_switch_info.attr,
>> + &rpc_sysfs_xprt_switch_add_xprt.attr,
>> NULL,
>> };
>> ATTRIBUTE_GROUPS(rpc_sysfs_xprt_switch);
>> diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
>> index 720d3ba742ec..a07b81ce93c3 100644
>> --- a/net/sunrpc/xprtmultipath.c
>> +++ b/net/sunrpc/xprtmultipath.c
>> @@ -92,6 +92,27 @@ void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
>> xprt_put(xprt);
>> }
>>
>> +/**
>> + * rpc_xprt_switch_get_main_xprt - Get the 'main' xprt for an xprt switch.
>> + * @xps: pointer to struct rpc_xprt_switch.
>> + */
>> +struct rpc_xprt *rpc_xprt_switch_get_main_xprt(struct rpc_xprt_switch *xps)
>> +{
>> + struct rpc_xprt_iter xpi;
>> + struct rpc_xprt *xprt;
>> +
>> + xprt_iter_init_listall(&xpi, xps);
>> +
>> + xprt = xprt_iter_get_xprt(&xpi);
>> + while (xprt && !xprt->main) {
>> + xprt_put(xprt);
>> + xprt = xprt_iter_get_next(&xpi);
>> + }
>> +
>> + xprt_iter_destroy(&xpi);
>> + return xprt;
>> +}
>> +
>> static DEFINE_IDA(rpc_xprtswitch_ids);
>>
>> void xprt_multipath_cleanup_ids(void)
>> --
>> 2.47.1
>
