netdev - Re: [PATCH net-next v4.1] rds: Expose feature parameters via sysfs (and ELF)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF87ATSJH3Q9rkju@char.us.oracle.com>
Date: Fri, 27 Jun 2025 20:44:49 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: allison.henderson@...cle.com, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, rds-devel@....oracle.com, tj@...nel.org,
        andrew@...n.ch, hannes@...xchg.org, mkoutny@...e.com,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v4.1] rds: Expose feature parameters via sysfs
 (and ELF)

On Wed, Jun 25, 2025 at 04:30:09PM -0700, Jakub Kicinski wrote:
> On Mon, 23 Jun 2025 11:51:36 -0400 Konrad Rzeszutek Wilk wrote:
> > With the value of 'supported' in them. In the future this value
> > could change to say 'deprecated' or have other values (for example
> > different versions) or can be runtime changed.
> 
> I'm curious how this theoretical 'deprecated' value would work
> in context of uAPI which can never regress..

Kind of sad considering there are some APIs that should really
be fixed. Perhaps more of 'it-is-busted-use-this-other-API'?

> 
> > The choice to use sysfs and this particular way is modeled on the
> > filesystems usage exposing their features.
> > 
> > Alternative solution such as exposing one file ('features') with
> > each feature enumerated (which cgroup does) is a bit limited in
> > that it does not provide means to provide extra content in the future
> > for each feature. For example if one of the features had three
> > modes and one wanted to set a particular one at runtime - that
> > does not exist in cgroup (albeit it can be implemented but it would
> > be quite hectic to have just one single attribute).
> > 
> > Another solution of using an ioctl to expose a bitmask has the
> > disadvantage of being less flexible in the future and while it can
> > have a bit of supported/unsupported, it is not clear how one would
> > change modes or expose versions. It is most certainly feasible
> > but it can get seriously complex fast.
> > 
> > As such this mechanism offers the basic support we require
> > now and offers the flexibility for the future.
> > 
> > Lastly, we also utilize the ELF note macro to expose these via

.. <missing>

> > so that applications that have not yet initialized RDS transport
> > can inspect the kernel module to see if they have the appropiate
> > support and choose an alternative protocol if they wish so.
> 
> Looks like this paragraph had a line starting with #, presumably
> talking about the ELF note and it got eaten by git? Please fix.

Yup
> 
> 
> FWIW to me this series has a strong whiff of "we have an OOT module
> which has much more functionality and want to support a degraded /
> upstream-only mode in the user space stack". I'm probably over-
> -interpreting, and you could argue this will help you make real
> use of the upstream RDS. I OTOH would argue that it's a technical
> solution to a non-technical problem of not giving upstreaming 
> sufficient priority; I'd prefer to see code flowing upstream _first_ 
> and then worry about compatibility.

The goal here was to lay the groundwork for another patch series that
Allison had in her backlog which was to introduce the reset functionality.

Let me work with Allison on adding this to that patch series.

> 
> $ git log --oneline --since='6 months ago' -- net/rds/ 
> 433dce0692a0 rds: Correct spelling
> 6e307a873d30 rds: Correct endian annotation of port and addr assignments
> 5bccdc51f90c replace strncpy with strscpy_pad
> c50d295c37f2 rds: Use nested-BH locking for rds_page_remainder
> 0af5928f358c rds: Acquire per-CPU pointer within BH disabled section
> aaaaa6639cf5 rds: Disable only bottom halves in rds_page_remainder_alloc()
> 357660d7596b Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
> 5c70eb5c593d net: better track kernel sockets lifetime
> c451715d78e3 net/rds: Replace deprecated strncpy() with strscpy_pad()
> 7f5611cbc487 rds: sysctl: rds_tcp_{rcv,snd}buf: avoid using current->nsproxy
> $
> 
> IOW applying this patch is a bit of a leap of faith that RDS
> upstreaming will restart. I don't have anything against the patch

It has to. We have to make the RDS TCP be bug-free as there are
customers demanding that. 

> per se, but neither do I have much faith in this. So if v5 is taking 
> a long time to get applied it will be because its waiting for DaveM or
> Paolo to take it.