[PATCH RFC v1] Feature reporting of RDS driver.

[PATCH RFC v1] Feature reporting of RDS driver.

[Konrad Rzeszutek Wilk]

Hi folks,

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 user-accessible way.

The patch does it two ways:

1) Power of the ELF .note section to put in the module so that
applications can discern before installing whether the kernel
has the right support.

2) The sysfs way - in which the /sys/modules/ was appealing since
it was simple and non invasive means of delivering this functionality,
and requires no changes to existing APIs or kernel logic.

I am not tied to any specific ways - hence the request for collaboration.

And as such questions, comments and discussion are appreciated and if you
have read to this part - then thank you for spending your time reading over
this cover letter and I am looking forward to your feedback on the patch!

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

 Documentation/ABI/stable/sysfs-driver-rds | 92 +++++++++++++++++++++++++++++++
 net/rds/af_rds.c                          | 33 +++++++++++
 2 files changed, 125 insertions(+)

[PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[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 (and which ones are deprecated) and also
what ioctl number to use for the specific feature (albeit that
is already in include/uapi/linux/rds.h but this is an extra
check if someone messed up).

This patch would expose these extra sysfs values:

/sys/module/rds/parameters/rds_ioctl_get_tos: 35297
/sys/module/rds/parameters/rds_ioctl_set_tos: 35296
/sys/module/rds/parameters/rds_socket_cancel_sent_to: 1
/sys/module/rds/parameters/rds_socket_cong_monitor: 6
/sys/module/rds/parameters/rds_socket_free_mr: 3
/sys/module/rds/parameters/rds_socket_get_mr: 2
/sys/module/rds/parameters/rds_socket_get_mr_for_dest: 7
/sys/module/rds/parameters/rds_socket_recverr: 5
/sys/module/rds/parameters/rds_socket_so_rxpath_latency: 9
/sys/module/rds/parameters/rds_socket_so_transport: 8
/sys/module/rds/parameters/rds_so_transport_ib: 0
/sys/module/rds/parameters/rds_so_transport_tcp: 2

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 Documentation/ABI/stable/sysfs-driver-rds | 92 +++++++++++++++++++++++
 net/rds/af_rds.c                          | 33 ++++++++
 2 files changed, 125 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..dcb1a335c5d6
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-rds
@@ -0,0 +1,92 @@
+What:		/sys/module/rds/parameters/rds_ioctl_set_tos
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to set on a socket
+		the Quality of Service.
+
+		The returned value is the socket ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_ioctl_get_tos
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to get on a socket
+		the Quality of Service.
+
+		The returned value is the socket ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_cancel_sent_to
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to cancel all pending
+		messages to a given destination.
+
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_get_mr
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to retrieve the memory
+		ranges for the RDMA calls to setsockopt.
+
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_free_mr
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to release the memory
+		ranges for the RDMA calls to setsockopt.
+
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_recverr
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports the mechanism to send RDMA notifications
+		for any RDMA operation that fails.
+
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_cong_monitor
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The RDS driver supports mechanism to provide Congestion updates via
+		RDS_CMSG_CONG_UPDATE control messages.
+
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_get_mr_for_dest
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_so_transport
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_socket_so_rxpath_latency
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The returned value is the ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_so_transport_ib
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The returned value for the IB transport ioctl number and this is read-only.
+
+What:		/sys/module/rds/parameters/rds_so_transport_tcp
+Date:		Jun 2025
+Contact:	rds-devel@oss.oracle.com
+Description:
+		The returned value is the TCP transport number and this is read-only.
diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index 8435a20968ef..15c8ded02dfb 100644
--- a/net/rds/af_rds.c
+++ b/net/rds/af_rds.c
@@ -31,6 +31,7 @@
  *
  */
 #include <linux/module.h>
+#include <linux/elfnote.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/gfp.h>
@@ -960,3 +961,35 @@ MODULE_DESCRIPTION("RDS: Reliable Datagram Sockets"
 MODULE_VERSION(DRV_VERSION);
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS_NETPROTO(PF_RDS);
+
+#define RDS_IOCTL(feature, val) ELFNOTE64("rds.ioctl_" #feature, 0, val); \
+				unsigned int rds_ioctl_##feature = val; \
+				module_param(rds_ioctl_##feature, int, 0444)
+
+#define RDS_SOCKET(feature, val) ELFNOTE64("rds.socket_" #feature, 0, val); \
+				unsigned int rds_socket_##feature = val; \
+				module_param(rds_socket_##feature, int, 0444)
+
+#define RDS_SO_TRANSPORT(feature, val) ELFNOTE64("rds.so_transport_" #feature, 0, val); \
+				unsigned int rds_so_transport_##feature = val; \
+				module_param(rds_so_transport_##feature, int, 0444)
+
+/* The values used here correspond to include/uapi/linux/rds.h values */
+
+RDS_IOCTL(set_tos, SIOCRDSSETTOS);
+RDS_IOCTL(get_tos, SIOCRDSGETTOS);
+
+/* Advertise setsocket/getsocket options. */
+
+RDS_SOCKET(cancel_sent_to, RDS_CANCEL_SENT_TO);
+RDS_SOCKET(get_mr, RDS_GET_MR);
+RDS_SOCKET(free_mr, RDS_FREE_MR);
+RDS_SOCKET(recverr, RDS_RECVERR);
+RDS_SOCKET(cong_monitor, RDS_CONG_MONITOR);
+RDS_SOCKET(get_mr_for_dest, RDS_GET_MR_FOR_DEST);
+RDS_SOCKET(so_transport, SO_RDS_TRANSPORT);
+RDS_SOCKET(so_rxpath_latency, SO_RDS_MSG_RXPATH_LATENCY);
+
+/* The transport mechanisms. */
+RDS_SO_TRANSPORT(ib, RDS_TRANS_IB);
+RDS_SO_TRANSPORT(tcp, RDS_TRANS_TCP);
-- 
2.43.5

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Andrew Lunn]

> +What:		/sys/module/rds/parameters/rds_ioctl_set_tos
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to set on a socket
> +		the Quality of Service.
> +
> +		The returned value is the socket ioctl number and this is read-only.

From this, can i imply that the IOCTL number has changed sometime in
the past? That would be an ABI break, which is not good.

More likely, none of the IOCTL numbers have changed. All you need to
know is if the file exists or not. So i would makes the files the same
as /dev/null.

Also, these are not actually parameters to the module, so putting them
in parameters seems confusing at best.

I doubt this is the first time this problem has been addressed in the
kernel. Maybe look at:

$ find /sys -name "*features*"
/sys/kernel/security/apparmor/features
/sys/kernel/cgroup/features

and see if you can copy/paste ideas/code from there. And also ask them
if this was a good idea, or they say not don't do that, it was a bad
idea, just call the IOCTL and test for ENOIOCTLCMD.

	Andrew

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Konrad Rzeszutek Wilk]

On Tue, Jun 10, 2025 at 10:30:27PM +0200, Andrew Lunn wrote:
> > +What:		/sys/module/rds/parameters/rds_ioctl_set_tos
> > +Date:		Jun 2025
> > +Contact:	rds-devel@oss.oracle.com
> > +Description:
> > +		The RDS driver supports the mechanism to set on a socket
> > +		the Quality of Service.
> > +
> > +		The returned value is the socket ioctl number and this is read-only.
> 
> From this, can i imply that the IOCTL number has changed sometime in
> the past? That would be an ABI break, which is not good.

Not that I am aware of. But this is more of putting all of the different
exposed ABIs that exist in the driver instead of listing specific ones.

> 
> More likely, none of the IOCTL numbers have changed. All you need to
> know is if the file exists or not. So i would makes the files the same

Right..
> as /dev/null.

Not sure I follow. Make them the same a /dev/null? Meaning link them to
/dev/null when the module is loaded?
> 
> Also, these are not actually parameters to the module, so putting them
> in parameters seems confusing at best.

Right, that is the one part I was not sure about - another approach was
to create a new sysfs tree. But it would be more code as we have to
register all of that during initialization, while the module_param does it
very nicely.

> 
> I doubt this is the first time this problem has been addressed in the
> kernel. Maybe look at:
> 
> $ find /sys -name "*features*"
> /sys/kernel/security/apparmor/features
> /sys/kernel/cgroup/features
> 
> and see if you can copy/paste ideas/code from there. And also ask them
> if this was a good idea, or they say not don't do that, it was a bad
> idea, just call the IOCTL and test for ENOIOCTLCMD.

Good thinking - let me dig in that and also CC the authors of those to
get their feedback.

Thank you!
> 
> 	Andrew

Re: [rds-devel] [PATCH RFC v1] Feature reporting of RDS driver.

[Konrad Rzeszutek Wilk]

On Tue, Jun 10, 2025 at 12:27:24PM -0400, Konrad Rzeszutek Wilk via rds-devel wrote:
> Hi folks,

Hi cgroup folks,

Andrew suggested that I reach out to you all since you had implemented
something very similar via:

3958e2d0c34e1
01ee6cfb1483f

And I was wondering if you have have feedback on what worked for you,
best practices, etc.

Manually CCing you so you will shortly be copied on the patch too.
> 
> 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 user-accessible way.
> 
> The patch does it two ways:
> 
> 1) Power of the ELF .note section to put in the module so that
> applications can discern before installing whether the kernel
> has the right support.
> 
> 2) The sysfs way - in which the /sys/modules/ was appealing since
> it was simple and non invasive means of delivering this functionality,
> and requires no changes to existing APIs or kernel logic.
> 
> I am not tied to any specific ways - hence the request for collaboration.
> 
> And as such questions, comments and discussion are appreciated and if you
> have read to this part - then thank you for spending your time reading over
> this cover letter and I am looking forward to your feedback on the patch!
> 
> Konrad Rzeszutek Wilk (1):
>       rds: Expose feature parameters via sysfs and ELF note
> 
>  Documentation/ABI/stable/sysfs-driver-rds | 92 +++++++++++++++++++++++++++++++
>  net/rds/af_rds.c                          | 33 +++++++++++
>  2 files changed, 125 insertions(+)
> 
> _______________________________________________
> rds-devel mailing list
> rds-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/rds-devel

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Andrew Lunn]

> Right..
> > as /dev/null.
> 
> Not sure I follow. Make them the same a /dev/null? Meaning link them to
> /dev/null when the module is loaded?

I mean the contents of the file does not matter. All that matters is
if the file exists or not. The IOCTL etc should never change, and if
they do, the patch which changes them will be reverted. So i would
make them a 0 byte file. You can even make it have mode 0000.

However, i would first try to copy existing ideas...

	Andrew

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Konrad Rzeszutek Wilk]

On Tue, Jun 10, 2025 at 12:27:25PM -0400, Konrad Rzeszutek Wilk wrote:

Hi!
I've added some extra folks on the To: list to solicit feedback on this
idea since something similar was done via

3958e2d0c34e1 cgroup: make per-cgroup pressure stall tracking configurable
01ee6cfb1483f cgroup: export list of delegatable control files using sysfs
5f2e673405b74 cgroup: export list of cgroups v2 features using sysfs

Thank you for you time!
> 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 (and which ones are deprecated) and also
> what ioctl number to use for the specific feature (albeit that
> is already in include/uapi/linux/rds.h but this is an extra
> check if someone messed up).
> 
> This patch would expose these extra sysfs values:
> 
> /sys/module/rds/parameters/rds_ioctl_get_tos: 35297
> /sys/module/rds/parameters/rds_ioctl_set_tos: 35296
> /sys/module/rds/parameters/rds_socket_cancel_sent_to: 1
> /sys/module/rds/parameters/rds_socket_cong_monitor: 6
> /sys/module/rds/parameters/rds_socket_free_mr: 3
> /sys/module/rds/parameters/rds_socket_get_mr: 2
> /sys/module/rds/parameters/rds_socket_get_mr_for_dest: 7
> /sys/module/rds/parameters/rds_socket_recverr: 5
> /sys/module/rds/parameters/rds_socket_so_rxpath_latency: 9
> /sys/module/rds/parameters/rds_socket_so_transport: 8
> /sys/module/rds/parameters/rds_so_transport_ib: 0
> /sys/module/rds/parameters/rds_so_transport_tcp: 2
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  Documentation/ABI/stable/sysfs-driver-rds | 92 +++++++++++++++++++++++
>  net/rds/af_rds.c                          | 33 ++++++++
>  2 files changed, 125 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..dcb1a335c5d6
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-rds
> @@ -0,0 +1,92 @@
> +What:		/sys/module/rds/parameters/rds_ioctl_set_tos
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to set on a socket
> +		the Quality of Service.
> +
> +		The returned value is the socket ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_ioctl_get_tos
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to get on a socket
> +		the Quality of Service.
> +
> +		The returned value is the socket ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_cancel_sent_to
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to cancel all pending
> +		messages to a given destination.
> +
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_get_mr
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to retrieve the memory
> +		ranges for the RDMA calls to setsockopt.
> +
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_free_mr
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to release the memory
> +		ranges for the RDMA calls to setsockopt.
> +
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_recverr
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports the mechanism to send RDMA notifications
> +		for any RDMA operation that fails.
> +
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_cong_monitor
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The RDS driver supports mechanism to provide Congestion updates via
> +		RDS_CMSG_CONG_UPDATE control messages.
> +
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_get_mr_for_dest
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_so_transport
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_socket_so_rxpath_latency
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The returned value is the ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_so_transport_ib
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The returned value for the IB transport ioctl number and this is read-only.
> +
> +What:		/sys/module/rds/parameters/rds_so_transport_tcp
> +Date:		Jun 2025
> +Contact:	rds-devel@oss.oracle.com
> +Description:
> +		The returned value is the TCP transport number and this is read-only.
> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index 8435a20968ef..15c8ded02dfb 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c
> @@ -31,6 +31,7 @@
>   *
>   */
>  #include <linux/module.h>
> +#include <linux/elfnote.h>
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/gfp.h>
> @@ -960,3 +961,35 @@ MODULE_DESCRIPTION("RDS: Reliable Datagram Sockets"
>  MODULE_VERSION(DRV_VERSION);
>  MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_ALIAS_NETPROTO(PF_RDS);
> +
> +#define RDS_IOCTL(feature, val) ELFNOTE64("rds.ioctl_" #feature, 0, val); \
> +				unsigned int rds_ioctl_##feature = val; \
> +				module_param(rds_ioctl_##feature, int, 0444)
> +
> +#define RDS_SOCKET(feature, val) ELFNOTE64("rds.socket_" #feature, 0, val); \
> +				unsigned int rds_socket_##feature = val; \
> +				module_param(rds_socket_##feature, int, 0444)
> +
> +#define RDS_SO_TRANSPORT(feature, val) ELFNOTE64("rds.so_transport_" #feature, 0, val); \
> +				unsigned int rds_so_transport_##feature = val; \
> +				module_param(rds_so_transport_##feature, int, 0444)
> +
> +/* The values used here correspond to include/uapi/linux/rds.h values */
> +
> +RDS_IOCTL(set_tos, SIOCRDSSETTOS);
> +RDS_IOCTL(get_tos, SIOCRDSGETTOS);
> +
> +/* Advertise setsocket/getsocket options. */
> +
> +RDS_SOCKET(cancel_sent_to, RDS_CANCEL_SENT_TO);
> +RDS_SOCKET(get_mr, RDS_GET_MR);
> +RDS_SOCKET(free_mr, RDS_FREE_MR);
> +RDS_SOCKET(recverr, RDS_RECVERR);
> +RDS_SOCKET(cong_monitor, RDS_CONG_MONITOR);
> +RDS_SOCKET(get_mr_for_dest, RDS_GET_MR_FOR_DEST);
> +RDS_SOCKET(so_transport, SO_RDS_TRANSPORT);
> +RDS_SOCKET(so_rxpath_latency, SO_RDS_MSG_RXPATH_LATENCY);
> +
> +/* The transport mechanisms. */
> +RDS_SO_TRANSPORT(ib, RDS_TRANS_IB);
> +RDS_SO_TRANSPORT(tcp, RDS_TRANS_TCP);
> -- 
> 2.43.5
> 

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Konrad Rzeszutek Wilk]

On Tue, Jun 10, 2025 at 10:50:06PM +0200, Andrew Lunn wrote:
> > Right..
> > > as /dev/null.
> > 
> > Not sure I follow. Make them the same a /dev/null? Meaning link them to
> > /dev/null when the module is loaded?
> 
> I mean the contents of the file does not matter. All that matters is
> if the file exists or not. The IOCTL etc should never change, and if
> they do, the patch which changes them will be reverted. So i would
> make them a 0 byte file. You can even make it have mode 0000.

Gotcha! That seems to run counter to what the sysfs expects?

It has to have _something_ in them. But let me play around and see.

Thanks for the feedback!
> 
> However, i would first try to copy existing ideas...

Yup, lets see what the other folks have to say.
> 
> 	Andrew

Re: [PATCH RFC v1] Feature reporting of RDS driver.

[Allison Henderson]

On Tue, 2025-06-10 at 12:27 -0400, Konrad Rzeszutek Wilk wrote:
> Hi folks,
> 
> 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 user-accessible way.
> 
> The patch does it two ways:
> 
> 1) Power of the ELF .note section to put in the module so that
> applications can discern before installing whether the kernel
> has the right support.
> 
> 2) The sysfs way - in which the /sys/modules/ was appealing since
> it was simple and non invasive means of delivering this functionality,
> and requires no changes to existing APIs or kernel logic.
> 
> I am not tied to any specific ways - hence the request for collaboration.
> 
> And as such questions, comments and discussion are appreciated and if you
> have read to this part - then thank you for spending your time reading over
> this cover letter and I am looking forward to your feedback on the patch!
> 
> Konrad Rzeszutek Wilk (1):
>       rds: Expose feature parameters via sysfs and ELF note

Hi Konrad,

Thanks for the rfc,  I think this is a great way to start discussion for exploring solutions and collecting opinions. 
The elfnotes are creative, but I think the module parameters is more commonly seen.  Andrew makes a good point that
these are not actually module parameters though, so I think the feature search suggestion is worth exploring.  I'll do
some digging on existing patterns too, as I am not sure the best practice myself.  

Thanks again for getting this started!

Allison


> 
>  Documentation/ABI/stable/sysfs-driver-rds | 92 +++++++++++++++++++++++++++++++
>  net/rds/af_rds.c                          | 33 +++++++++++
>  2 files changed, 125 insertions(+)

Re: [rds-devel] [PATCH RFC v1] Feature reporting of RDS driver.

[Tejun Heo]

Hello,

On Tue, Jun 10, 2025 at 04:47:23PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 10, 2025 at 12:27:24PM -0400, Konrad Rzeszutek Wilk via rds-devel wrote:
> > Hi folks,
> 
> Hi cgroup folks,
> 
> Andrew suggested that I reach out to you all since you had implemented
> something very similar via:
> 
> 3958e2d0c34e1
> 01ee6cfb1483f
> 
> And I was wondering if you have have feedback on what worked for you,
> best practices, etc.

I don't know RDS at all, so please take what I say with a big grain of salt.
That said, the sysfs approach is pretty straightforward and has worked well
for us. One thing which we didn't do (yet) but maybe useful is defining some
conventions to tell whether a given feature or option should be enabled by
default so that most users don't have to know which features to use and
follow whatever the kernel release thinks is the best default combination.

Thanks.

-- 
tejun

Re: [rds-devel] [PATCH RFC v1] Feature reporting of RDS driver.

[Konrad Rzeszutek Wilk]

On Tue, Jun 10, 2025 at 11:32:40AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Tue, Jun 10, 2025 at 04:47:23PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Tue, Jun 10, 2025 at 12:27:24PM -0400, Konrad Rzeszutek Wilk via rds-devel wrote:
> > > Hi folks,
> > 
> > Hi cgroup folks,
> > 
> > Andrew suggested that I reach out to you all since you had implemented
> > something very similar via:
> > 
> > 3958e2d0c34e1
> > 01ee6cfb1483f
> > 
> > And I was wondering if you have have feedback on what worked for you,
> > best practices, etc.
> 
> I don't know RDS at all, so please take what I say with a big grain of salt.

It is just a driver. One talks to it via socket.. But it can do extra
things based on setsocket/getseocket and such.

> That said, the sysfs approach is pretty straightforward and has worked well
> for us. One thing which we didn't do (yet) but maybe useful is defining some
> conventions to tell whether a given feature or option should be enabled by
> default so that most users don't have to know which features to use and
> follow whatever the kernel release thinks is the best default combination.

I see. With that in mind, would it have helped if each feature had its
own sysfs file with a tri-state or such?

In regards to the existing 'feature' sysfs attribute:

How were you thinking to address API/ABI semantic breakage? Say older
versions implemented a "foobar" feature but never kernels implement a
much better way, but with a change the semantics (say require extra parameters,
etc).  Would you expose both of them via the 'feature' sysfs attribute: "foobar\nfoobar_v2" ?

What would be then the path for removing the old one? Would you just
drop "foobar" and only expose "foobar_v2" ?

Thank you!
> 
> Thanks.
> 
> -- 
> tejun

Re: [PATCH RFC v1] rds: Expose feature parameters via sysfs and ELF note

[Leon Romanovsky]

On Tue, Jun 10, 2025 at 12:27:25PM -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.

If you are interested in programmatic way, the IOCTL/netlink/syscall/...
are the right approach for it and not sysfs files.

Why don't you follow standard way of doing it by creating new query IOCTL
command? Which will return declared feature bitmask, which application can
parse and understand?

Thanks

Re: [rds-devel] [PATCH RFC v1] Feature reporting of RDS driver.

[Tejun Heo]

Hello,

On Tue, Jun 10, 2025 at 07:15:31PM -0400, Konrad Rzeszutek Wilk wrote:
...
> > That said, the sysfs approach is pretty straightforward and has worked well
> > for us. One thing which we didn't do (yet) but maybe useful is defining some
> > conventions to tell whether a given feature or option should be enabled by
> > default so that most users don't have to know which features to use and
> > follow whatever the kernel release thinks is the best default combination.
> 
> I see. With that in mind, would it have helped if each feature had its
> own sysfs file with a tri-state or such?

I don't see why that wouldn't work but maybe a bit too elaborate?

> In regards to the existing 'feature' sysfs attribute:
> 
> How were you thinking to address API/ABI semantic breakage? Say older
> versions implemented a "foobar" feature but never kernels implement a
> much better way, but with a change the semantics (say require extra parameters,
> etc).  Would you expose both of them via the 'feature' sysfs attribute: "foobar\nfoobar_v2" ?
> 
> What would be then the path for removing the old one? Would you just
> drop "foobar" and only expose "foobar_v2" ?

I don't think there's one good answer but here's one:

- Each token in the files represents an optional feature.

- A feature preceded by + is expected to be enabled (or used) by default. A
  feature preced by - is expected to be not used.

- When introducing v2, make v2 +, the old one -.

- After users are reasoanbly migrated, start generating warning on v1 usages.

- Remove v1.

Thanks.

-- 
tejun