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

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

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
>