[PATCH v2] Expose RDS features via sysfs.

[PATCH v2] Expose RDS features via sysfs.

[Konrad Rzeszutek Wilk]

Hi folks,

Changelog:
+ Since v2:
 - Changed it to use sysfs and expose a 'features' file with the data.
 
This patch addresses an issue where we have applications compiled against
against older (or newer) kernels. RDS does not have any ioctls to query
for what version of ABIs it has or what features it has. Hence this solution
that proposes to put this ABI information in sysfs space.

Please take a look and thank you for spending your time reading over the
patch and providing feedback on the right way forward.

Thank you!

Konrad Rzeszutek Wilk (1):
      rds: Expose feature parameters via sysfs

 net/rds/af_rds.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

[PATCH v2] rds: Expose feature parameters via sysfs

[Konrad Rzeszutek Wilk]

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>
---
 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)
-- 
2.43.5

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

[Konrad Rzeszutek Wilk]

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
> 
> which would contain string values of what the RDS driver supports.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Argh, forgot to include:

Suggested-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  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)
> -- 
> 2.43.5
> 

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

[Andrew Lunn]

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

---
pw-bot: cr

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

[Allison Henderson]

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)

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

[Konrad Rzeszutek Wilk]

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

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

[Andrew Lunn]

> 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:

Please take a read of:

https://docs.kernel.org/process/submitting-patches.html

and

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

You need a new thread for every version of the patch. You should also
put the tree into the Subject: line, etc.

> diff --git a/Documentation/ABI/stable/sysfs-driver-rds b/Documentation/ABI/stable/sysfs-driver-rds

I could be bike shedding too much, but RDS is not a driver. It is a
socket protocol, which you can layer on top of a few different
transport protocols. So i don't think it should have -driver- in the
filename.

> 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.

Is that enough Documentation for somebody to make use of this file
without having to do a deep dive into the kernel sources? If i need to
do a deep dive, i might just as well handle the EOPNOTSUPP return
values.

> +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");

This is ABI. User space is going to start parsing this. Maybe we
should add both here, and in the documentation, something like:

  New lines will be added at random places within the file as new
  features are added.

This makes it clear that any code which tests line 4 for
"socket_free_mr" could break in the future. The whole file needs to be
searched for a feature.

	Andrew