Re: [PATCH] RDMA/siw: Add missing Kconfig selections

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Sep 01, 2022 at 08:50:12AM -0400, Tom Talpey wrote:
> On 9/1/2022 8:32 AM, Leon Romanovsky wrote:
> > On Thu, Sep 01, 2022 at 09:25:39AM -0300, Jason Gunthorpe wrote:
> > > On Wed, Aug 31, 2022 at 12:30:48PM -0400, Tom Talpey wrote:
> > > > The SoftiWARP Kconfig is missing "select" for CRYPTO and CRYPTO_CRC32C.
> > > > 
> > > > In addition, it improperly "depends on" LIBCRC32C, this should be a
> > > > "select", similar to net/sctp and others. As a dependency, SIW fails
> > > > to appear in generic configurations.
> > > > 
> > > > Signed-off-by: Tom Talpey <tom@talpey.com>
> > > > ---
> > > >   drivers/infiniband/sw/siw/Kconfig | 5 ++++-
> > > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/infiniband/sw/siw/Kconfig
> > > > b/drivers/infiniband/sw/siw/Kconfig
> > > > index 1b5105cbabae..81b70a3eeb87 100644
> > > > --- a/drivers/infiniband/sw/siw/Kconfig
> > > > +++ b/drivers/infiniband/sw/siw/Kconfig
> > > > @@ -1,7 +1,10 @@
> > > >   config RDMA_SIW
> > > >          tristate "Software RDMA over TCP/IP (iWARP) driver"
> > > > -       depends on INET && INFINIBAND && LIBCRC32C
> > > > +       depends on INET && INFINIBAND
> > > >          depends on INFINIBAND_VIRT_DMA
> > > > +       select LIBCRC32C
> > > > +       select CRYPTO
> > > > +       select CRYPTO_CRC32C
> > > 
> > > This is against the kconfig instructions Documentation/kbuild/kconfig-language.rst:
> > > 
> > >    Note:
> > >          select should be used with care. select will force
> > >          a symbol to a value without visiting the dependencies.
> > >          By abusing select you are able to select a symbol FOO even
> > >          if FOO depends on BAR that is not set.
> > >          In general use select only for non-visible symbols
> > >          (no prompts anywhere) and for symbols with no dependencies.
> > >          That will limit the usefulness but on the other hand avoid
> > >          the illegal configurations all over.
> > > 
> > > None of them meet that criteria even though other places do abuse
> > > select like this as well.
> > > 
> > > It looked fine to me the way it was, you are supposed to have to
> > > select libcrc32c manually to make siw appear, and it already brings in
> > > the other symbols.
> > 
> > He took his snippet from RXE.
> 
> RXE, net/sctp and many others, actually. It seems backwards for a
> subsystem to depend on a library, shouldn't libraries be there for
> selecting? If that's invalid, there are a LOT of subsystems to fix.

kconfig is a mess unfortunately, and the crypto stuff is kind of
weirdly done to be both a library and a user selectable pluggable..

Jason