Re: [PATCH v2] rds: Expose feature parameters via sysfs

[Allison Henderson <allison.henderson@oracle.com>]

On Wed, 2025-06-11 at 18:39 -0400, Konrad Rzeszutek Wilk wrote:
> We would like to have a programatic way for applications
> to query which of the features defined in include/uapi/linux/rds.h
> are actually implemented by the kernel.
> 
> The problem is that applications can be built against newer
> kernel (or older) and they may have the feature implemented or not.
> 
> The lack of a certain feature would signify that the kernel
> does not support it. The presence of it signifies the existence
> of it.
> 
> This would provide the application to query the sysfs and figure
> out what is supported.
> 
> This patch would expose this extra sysfs file:
> 
> /sys/kernel/rds/features
> 
> which would contain string values of what the RDS driver supports.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Hi Konrad,

Thanks for the v2!  I think the simplicity of the /sys/kernel/rds/features is very appealing.  I also explored the idea
of using an ioctl that return a bit mask, but I think I'd prefer to keep the solution as simple as possible given the
use case.  If we do move forward with this solution, we should definitely add some documentation as Andrew suggests. 
Maybe a quick description of the features under Documentation/ABI/stable/sysfs-kernel-rds.  #ifdef nits aside, think
this patch is looking pretty good.  Thanks for working on this!

Allison

> ---
>  net/rds/af_rds.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index 8435a20968ef..46cb8655df20 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c
> @@ -33,6 +33,7 @@
>  #include <linux/module.h>
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
> +#include <linux/kobject.h>
>  #include <linux/gfp.h>
>  #include <linux/in.h>
>  #include <linux/ipv6.h>
> @@ -871,6 +872,33 @@ static void rds6_sock_info(struct socket *sock, unsigned int len,
>  }
>  #endif
>  
> +#ifdef CONFIG_SYSFS
> +static ssize_t features_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			     char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "get_tos\n"
> +			"set_tos\n"
> +			"socket_cancel_sent_to\n"
> +			"socket_get_mr\n"
> +			"socket_free_mr\n"
> +			"socket_recverr\n"
> +			"socket_cong_monitor\n"
> +			"socket_get_mr_for_dest\n"
> +			"socket_so_transport\n"
> +			"socket_so_rxpath_latency\n");
> +}
> +static struct kobj_attribute rds_features_attr = __ATTR_RO(features);
> +
> +static struct attribute *rds_sysfs_attrs[] = {
> +	&rds_features_attr.attr,
> +	NULL,
> +};
> +static const struct attribute_group rds_sysfs_attr_group = {
> +	.attrs = rds_sysfs_attrs,
> +	.name = "rds",
> +};
> +#endif
> +
>  static void rds_exit(void)
>  {
>  	sock_unregister(rds_family_ops.family);
> @@ -882,6 +910,9 @@ static void rds_exit(void)
>  	rds_stats_exit();
>  	rds_page_exit();
>  	rds_bind_lock_destroy();
> +#ifdef CONFIG_SYSFS
> +	sysfs_remove_group(kernel_kobj, &rds_sysfs_attr_group);
> +#endif
>  	rds_info_deregister_func(RDS_INFO_SOCKETS, rds_sock_info);
>  	rds_info_deregister_func(RDS_INFO_RECV_MESSAGES, rds_sock_inc_info);
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -923,6 +954,12 @@ static int __init rds_init(void)
>  	if (ret)
>  		goto out_proto;
>  
> +#ifdef CONFIG_SYSFS
> +	ret = sysfs_create_group(kernel_kobj, &rds_sysfs_attr_group);
> +	if (ret)
> +		goto out_proto;
> +#endif
> +
>  	rds_info_register_func(RDS_INFO_SOCKETS, rds_sock_info);
>  	rds_info_register_func(RDS_INFO_RECV_MESSAGES, rds_sock_inc_info);
>  #if IS_ENABLED(CONFIG_IPV6)