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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Jun 13, 2025 at 05:21:15PM +0200, Andrew Lunn wrote:
> On Wed, Jun 11, 2025 at 06:39:19PM -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
> 
> This should probably be documented somewhere under
> Documentation/ABI/stable.
> 
> > which would contain string values of what the RDS driver supports.
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  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
> 
> include/linux/sysfs.h has a stub for when SYSFS is not enabled. So you
> should not need any #ifdefs
> 
>     Andrew

I could not for the life of me get the kernel to compile without
CONFIG_SYSFS, but here is the patch with the modifications you
enumerated:

From 46550ddbd78c878924e4398f07811aac63402ecf Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 11 Jun 2025 15:29:39 -0400
Subject: [PATCH] rds: Expose feature parameters via sysfs

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.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v3: Add the missing documentation
    Remove the CONFIG_SYSFS #ifdef machinations
---
 Documentation/ABI/stable/sysfs-driver-rds | 10 ++++++++
 net/rds/af_rds.c                          | 31 +++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-rds

diff --git a/Documentation/ABI/stable/sysfs-driver-rds b/Documentation/ABI/stable/sysfs-driver-rds
new file mode 100644
index 000000000000..d0b4fe0d3ce4
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-rds
@@ -0,0 +1,10 @@
+What:          /sys/kernel/rds/features
+Date:          June 2025
+KernelVersion: 6.17
+Contact:       rds-devel@oss.oracle.com 
+Description:   This file will contain the features that correspond
+               to the include/uapi/linux/rds.h in a string format.
+
+	       The intent is for applications compiled against rds.h
+	       to be able to query and find out what features the
+	       driver supports.
diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index 8435a20968ef..cc9ade29c58f 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,31 @@ static void rds6_sock_info(struct socket *sock, unsigned int len,
 }
 #endif
 
+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",
+};
+
 static void rds_exit(void)
 {
 	sock_unregister(rds_family_ops.family);
@@ -882,6 +908,7 @@ static void rds_exit(void)
 	rds_stats_exit();
 	rds_page_exit();
 	rds_bind_lock_destroy();
+	sysfs_remove_group(kernel_kobj, &rds_sysfs_attr_group);
 	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 +950,10 @@ static int __init rds_init(void)
 	if (ret)
 		goto out_proto;
 
+	ret = sysfs_create_group(kernel_kobj, &rds_sysfs_attr_group);
+	if (ret)
+		goto out_proto;
+
 	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)
-- 
2.43.5