diff mbox series

drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO

Message ID 21525878.NYvzQUHefP@ubuntu-mate-laptop (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO | expand

Commit Message

Julian Braha Feb. 19, 2021, 11:32 p.m. UTC 
commit 6e61907779ba99af785f5b2397a84077c289888a
Author: Julian Braha <julianbraha@gmail.com>
Date:   Fri Feb 19 18:20:57 2021 -0500

    drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
    
    When RDMA_RXE is enabled and CRYPTO is disabled,
    Kbuild gives the following warning:
    
    WARNING: unmet direct dependencies detected for CRYPTO_CRC32
      Depends on [n]: CRYPTO [=n]
      Selected by [y]:
      - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
    
    This is because RDMA_RXE selects CRYPTO_CRC32,
    without depending on or selecting CRYPTO, despite that config option
    being subordinate to CRYPTO.
    
    Signed-off-by: Julian Braha <julianbraha@gmail.com>

Comments

Leon Romanovsky Feb. 21, 2021, 6:48 a.m. UTC | #1 
On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> commit 6e61907779ba99af785f5b2397a84077c289888a
> Author: Julian Braha <julianbraha@gmail.com>
> Date:   Fri Feb 19 18:20:57 2021 -0500
>
>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
>
>     When RDMA_RXE is enabled and CRYPTO is disabled,
>     Kbuild gives the following warning:
>
>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
>       Depends on [n]: CRYPTO [=n]
>       Selected by [y]:
>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
>
>     This is because RDMA_RXE selects CRYPTO_CRC32,
>     without depending on or selecting CRYPTO, despite that config option
>     being subordinate to CRYPTO.
>
>     Signed-off-by: Julian Braha <julianbraha@gmail.com>

Please use git sent-email to send patches and please fix crypto Kconfig
to enable CRYPTO if CRYPTO_* selected.

It is a little bit awkward to request all users of CRYPTO_* to request
select CRYPTO too.

Thanks

>
> diff --git a/drivers/infiniband/sw/rxe/Kconfig b/drivers/infiniband/sw/rxe/Kconfig
> index 452149066792..06b8dc5093f7 100644
> --- a/drivers/infiniband/sw/rxe/Kconfig
> +++ b/drivers/infiniband/sw/rxe/Kconfig
> @@ -4,6 +4,7 @@ config RDMA_RXE
>         depends on INET && PCI && INFINIBAND
>         depends on INFINIBAND_VIRT_DMA
>         select NET_UDP_TUNNEL
> +      select CRYPTO
>         select CRYPTO_CRC32
>         help
>         This driver implements the InfiniBand RDMA transport over
>
>
>
Zhu Yanjun Feb. 22, 2021, 2:39 a.m. UTC | #2 
On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> > commit 6e61907779ba99af785f5b2397a84077c289888a
> > Author: Julian Braha <julianbraha@gmail.com>
> > Date:   Fri Feb 19 18:20:57 2021 -0500
> >
> >     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
> >
> >     When RDMA_RXE is enabled and CRYPTO is disabled,
> >     Kbuild gives the following warning:
> >
> >     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
> >       Depends on [n]: CRYPTO [=n]
> >       Selected by [y]:
> >       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
> >
> >     This is because RDMA_RXE selects CRYPTO_CRC32,
> >     without depending on or selecting CRYPTO, despite that config option
> >     being subordinate to CRYPTO.
> >
> >     Signed-off-by: Julian Braha <julianbraha@gmail.com>
>
> Please use git sent-email to send patches and please fix crypto Kconfig
> to enable CRYPTO if CRYPTO_* selected.
>
> It is a little bit awkward to request all users of CRYPTO_* to request
> select CRYPTO too.

The same issue and similar patch is in this link:

https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747

Zhu Yanjun

>
> Thanks
>
> >
> > diff --git a/drivers/infiniband/sw/rxe/Kconfig b/drivers/infiniband/sw/rxe/Kconfig
> > index 452149066792..06b8dc5093f7 100644
> > --- a/drivers/infiniband/sw/rxe/Kconfig
> > +++ b/drivers/infiniband/sw/rxe/Kconfig
> > @@ -4,6 +4,7 @@ config RDMA_RXE
> >         depends on INET && PCI && INFINIBAND
> >         depends on INFINIBAND_VIRT_DMA
> >         select NET_UDP_TUNNEL
> > +      select CRYPTO
> >         select CRYPTO_CRC32
> >         help
> >         This driver implements the InfiniBand RDMA transport over
> >
> >
> >
Leon Romanovsky Feb. 22, 2021, 1 p.m. UTC | #3 
On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
> On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> > > commit 6e61907779ba99af785f5b2397a84077c289888a
> > > Author: Julian Braha <julianbraha@gmail.com>
> > > Date:   Fri Feb 19 18:20:57 2021 -0500
> > >
> > >     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
> > >
> > >     When RDMA_RXE is enabled and CRYPTO is disabled,
> > >     Kbuild gives the following warning:
> > >
> > >     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
> > >       Depends on [n]: CRYPTO [=n]
> > >       Selected by [y]:
> > >       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
> > >
> > >     This is because RDMA_RXE selects CRYPTO_CRC32,
> > >     without depending on or selecting CRYPTO, despite that config option
> > >     being subordinate to CRYPTO.
> > >
> > >     Signed-off-by: Julian Braha <julianbraha@gmail.com>
> >
> > Please use git sent-email to send patches and please fix crypto Kconfig
> > to enable CRYPTO if CRYPTO_* selected.
> >
> > It is a little bit awkward to request all users of CRYPTO_* to request
> > select CRYPTO too.
>
> The same issue and similar patch is in this link:
>
> https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747

So what prevents us from fixing CRYPTO Kconfig?

Thanks

>
> Zhu Yanjun
>
> >
> > Thanks
> >
> > >
> > > diff --git a/drivers/infiniband/sw/rxe/Kconfig b/drivers/infiniband/sw/rxe/Kconfig
> > > index 452149066792..06b8dc5093f7 100644
> > > --- a/drivers/infiniband/sw/rxe/Kconfig
> > > +++ b/drivers/infiniband/sw/rxe/Kconfig
> > > @@ -4,6 +4,7 @@ config RDMA_RXE
> > >         depends on INET && PCI && INFINIBAND
> > >         depends on INFINIBAND_VIRT_DMA
> > >         select NET_UDP_TUNNEL
> > > +      select CRYPTO
> > >         select CRYPTO_CRC32
> > >         help
> > >         This driver implements the InfiniBand RDMA transport over
> > >
> > >
> > >
Jason Gunthorpe Feb. 22, 2021, 3:58 p.m. UTC | #4 
On Mon, Feb 22, 2021 at 03:00:03PM +0200, Leon Romanovsky wrote:
> On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
> > On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> > > > commit 6e61907779ba99af785f5b2397a84077c289888a
> > > > Author: Julian Braha <julianbraha@gmail.com>
> > > > Date:   Fri Feb 19 18:20:57 2021 -0500
> > > >
> > > >     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
> > > >
> > > >     When RDMA_RXE is enabled and CRYPTO is disabled,
> > > >     Kbuild gives the following warning:
> > > >
> > > >     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
> > > >       Depends on [n]: CRYPTO [=n]
> > > >       Selected by [y]:
> > > >       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
> > > >
> > > >     This is because RDMA_RXE selects CRYPTO_CRC32,
> > > >     without depending on or selecting CRYPTO, despite that config option
> > > >     being subordinate to CRYPTO.
> > > >
> > > >     Signed-off-by: Julian Braha <julianbraha@gmail.com>
> > >
> > > Please use git sent-email to send patches and please fix crypto Kconfig
> > > to enable CRYPTO if CRYPTO_* selected.
> > >
> > > It is a little bit awkward to request all users of CRYPTO_* to request
> > > select CRYPTO too.
> >
> > The same issue and similar patch is in this link:
> >
> > https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747
> 
> So what prevents us from fixing CRYPTO Kconfig?

Yes, I would like to see someone deal with this properly, either every
place doing select CRYPTO_XX needs fixing or something needs to be
done in the crypto layer.

I have no idea about kconfig to give advice, I've added Arnd since he
always seems to know :)

Thanks,
Jason
Randy Dunlap Feb. 22, 2021, 4:26 p.m. UTC | #5 
On 2/22/21 7:58 AM, Jason Gunthorpe wrote:
> On Mon, Feb 22, 2021 at 03:00:03PM +0200, Leon Romanovsky wrote:
>> On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
>>> On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
>>>>
>>>> On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
>>>>> commit 6e61907779ba99af785f5b2397a84077c289888a
>>>>> Author: Julian Braha <julianbraha@gmail.com>
>>>>> Date:   Fri Feb 19 18:20:57 2021 -0500
>>>>>
>>>>>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
>>>>>
>>>>>     When RDMA_RXE is enabled and CRYPTO is disabled,
>>>>>     Kbuild gives the following warning:
>>>>>
>>>>>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
>>>>>       Depends on [n]: CRYPTO [=n]
>>>>>       Selected by [y]:
>>>>>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
>>>>>
>>>>>     This is because RDMA_RXE selects CRYPTO_CRC32,
>>>>>     without depending on or selecting CRYPTO, despite that config option
>>>>>     being subordinate to CRYPTO.
>>>>>
>>>>>     Signed-off-by: Julian Braha <julianbraha@gmail.com>
>>>>
>>>> Please use git sent-email to send patches and please fix crypto Kconfig
>>>> to enable CRYPTO if CRYPTO_* selected.
>>>>
>>>> It is a little bit awkward to request all users of CRYPTO_* to request
>>>> select CRYPTO too.
>>>
>>> The same issue and similar patch is in this link:
>>>
>>> https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747
>>
>> So what prevents us from fixing CRYPTO Kconfig?
> 
> Yes, I would like to see someone deal with this properly, either every
> place doing select CRYPTO_XX needs fixing or something needs to be
> done in the crypto layer.
> 
> I have no idea about kconfig to give advice, I've added Arnd since he
> always seems to know :)

I will Ack the original patch in this thread.

How many Mellanox drivers are you concerned about?
You don't have to fix any other drivers that have a similar issue.
Jason Gunthorpe Feb. 22, 2021, 4:46 p.m. UTC | #6 
On Mon, Feb 22, 2021 at 08:26:10AM -0800, Randy Dunlap wrote:
> On 2/22/21 7:58 AM, Jason Gunthorpe wrote:
> > On Mon, Feb 22, 2021 at 03:00:03PM +0200, Leon Romanovsky wrote:
> >> On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
> >>> On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
> >>>>
> >>>> On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> >>>>> commit 6e61907779ba99af785f5b2397a84077c289888a
> >>>>> Author: Julian Braha <julianbraha@gmail.com>
> >>>>> Date:   Fri Feb 19 18:20:57 2021 -0500
> >>>>>
> >>>>>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
> >>>>>
> >>>>>     When RDMA_RXE is enabled and CRYPTO is disabled,
> >>>>>     Kbuild gives the following warning:
> >>>>>
> >>>>>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
> >>>>>       Depends on [n]: CRYPTO [=n]
> >>>>>       Selected by [y]:
> >>>>>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
> >>>>>
> >>>>>     This is because RDMA_RXE selects CRYPTO_CRC32,
> >>>>>     without depending on or selecting CRYPTO, despite that config option
> >>>>>     being subordinate to CRYPTO.
> >>>>>
> >>>>>     Signed-off-by: Julian Braha <julianbraha@gmail.com>
> >>>>
> >>>> Please use git sent-email to send patches and please fix crypto Kconfig
> >>>> to enable CRYPTO if CRYPTO_* selected.
> >>>>
> >>>> It is a little bit awkward to request all users of CRYPTO_* to request
> >>>> select CRYPTO too.
> >>>
> >>> The same issue and similar patch is in this link:
> >>>
> >>> https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747
> >>
> >> So what prevents us from fixing CRYPTO Kconfig?
> > 
> > Yes, I would like to see someone deal with this properly, either every
> > place doing select CRYPTO_XX needs fixing or something needs to be
> > done in the crypto layer.
> > 
> > I have no idea about kconfig to give advice, I've added Arnd since he
> > always seems to know :)
> 
> I will Ack the original patch in this thread.

The one from Julian?

> How many Mellanox drivers are you concerned about?

?? This is about rxe

> You don't have to fix any other drivers that have a similar issue.

Why shouldn't they be fixed too?

There is nearly 1000 places that use a 'select CRYPTO_*' in the
kernel.

I see only 60 'select CRYPTO' statements.

Jason
Leon Romanovsky Feb. 22, 2021, 4:50 p.m. UTC | #7 
On Mon, Feb 22, 2021 at 12:46:45PM -0400, Jason Gunthorpe wrote:
> On Mon, Feb 22, 2021 at 08:26:10AM -0800, Randy Dunlap wrote:
> > On 2/22/21 7:58 AM, Jason Gunthorpe wrote:
> > > On Mon, Feb 22, 2021 at 03:00:03PM +0200, Leon Romanovsky wrote:
> > >> On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
> > >>> On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >>>>
> > >>>> On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> > >>>>> commit 6e61907779ba99af785f5b2397a84077c289888a
> > >>>>> Author: Julian Braha <julianbraha@gmail.com>
> > >>>>> Date:   Fri Feb 19 18:20:57 2021 -0500
> > >>>>>
> > >>>>>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
> > >>>>>
> > >>>>>     When RDMA_RXE is enabled and CRYPTO is disabled,
> > >>>>>     Kbuild gives the following warning:
> > >>>>>
> > >>>>>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
> > >>>>>       Depends on [n]: CRYPTO [=n]
> > >>>>>       Selected by [y]:
> > >>>>>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
> > >>>>>
> > >>>>>     This is because RDMA_RXE selects CRYPTO_CRC32,
> > >>>>>     without depending on or selecting CRYPTO, despite that config option
> > >>>>>     being subordinate to CRYPTO.
> > >>>>>
> > >>>>>     Signed-off-by: Julian Braha <julianbraha@gmail.com>
> > >>>>
> > >>>> Please use git sent-email to send patches and please fix crypto Kconfig
> > >>>> to enable CRYPTO if CRYPTO_* selected.
> > >>>>
> > >>>> It is a little bit awkward to request all users of CRYPTO_* to request
> > >>>> select CRYPTO too.
> > >>>
> > >>> The same issue and similar patch is in this link:
> > >>>
> > >>> https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747
> > >>
> > >> So what prevents us from fixing CRYPTO Kconfig?
> > >
> > > Yes, I would like to see someone deal with this properly, either every
> > > place doing select CRYPTO_XX needs fixing or something needs to be
> > > done in the crypto layer.
> > >
> > > I have no idea about kconfig to give advice, I've added Arnd since he
> > > always seems to know :)
> >
> > I will Ack the original patch in this thread.
>
> The one from Julian?
>
> > How many Mellanox drivers are you concerned about?
>
> ?? This is about rxe
>
> > You don't have to fix any other drivers that have a similar issue.
>
> Why shouldn't they be fixed too?
>
> There is nearly 1000 places that use a 'select CRYPTO_*' in the
> kernel.
>
> I see only 60 'select CRYPTO' statements.

I don't like the suggestion to ack and not fix either.
All CRYPTO_CRC32C users need CRYPTO and it means that CRYPTO Kconfig
should be fixed.

➜  kernel git:(queue-next) git grep -B 1 "select CRYPTO_CRC32"
drivers/infiniband/sw/rxe/Kconfig-      select NET_UDP_TUNNEL
drivers/infiniband/sw/rxe/Kconfig:      select CRYPTO_CRC32
--
drivers/nvme/host/Kconfig-      select CRYPTO
drivers/nvme/host/Kconfig:      select CRYPTO_CRC32C
--
drivers/scsi/Kconfig-   select CRYPTO_MD5
drivers/scsi/Kconfig:   select CRYPTO_CRC32C
--
drivers/target/iscsi/Kconfig-   select CRYPTO
drivers/target/iscsi/Kconfig:   select CRYPTO_CRC32C
drivers/target/iscsi/Kconfig:   select CRYPTO_CRC32C_INTEL if X86
--
fs/btrfs/Kconfig-       select CRYPTO
fs/btrfs/Kconfig:       select CRYPTO_CRC32C
--
fs/ext4/Kconfig-        select CRYPTO
fs/ext4/Kconfig:        select CRYPTO_CRC32C
--
fs/f2fs/Kconfig-        select CRYPTO
fs/f2fs/Kconfig:        select CRYPTO_CRC32
--
fs/jbd2/Kconfig-        select CRYPTO
fs/jbd2/Kconfig:        select CRYPTO_CRC32C
--
lib/Kconfig-    select CRYPTO
lib/Kconfig:    select CRYPTO_CRC32C


>
> Jason
Randy Dunlap Feb. 22, 2021, 4:53 p.m. UTC | #8 
On 2/22/21 8:46 AM, Jason Gunthorpe wrote:
> On Mon, Feb 22, 2021 at 08:26:10AM -0800, Randy Dunlap wrote:
>> On 2/22/21 7:58 AM, Jason Gunthorpe wrote:
>>> On Mon, Feb 22, 2021 at 03:00:03PM +0200, Leon Romanovsky wrote:
>>>> On Mon, Feb 22, 2021 at 10:39:20AM +0800, Zhu Yanjun wrote:
>>>>> On Sun, Feb 21, 2021 at 2:49 PM Leon Romanovsky <leon@kernel.org> wrote:
>>>>>>
>>>>>> On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
>>>>>>> commit 6e61907779ba99af785f5b2397a84077c289888a
>>>>>>> Author: Julian Braha <julianbraha@gmail.com>
>>>>>>> Date:   Fri Feb 19 18:20:57 2021 -0500
>>>>>>>
>>>>>>>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
>>>>>>>
>>>>>>>     When RDMA_RXE is enabled and CRYPTO is disabled,
>>>>>>>     Kbuild gives the following warning:
>>>>>>>
>>>>>>>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
>>>>>>>       Depends on [n]: CRYPTO [=n]
>>>>>>>       Selected by [y]:
>>>>>>>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
>>>>>>>
>>>>>>>     This is because RDMA_RXE selects CRYPTO_CRC32,
>>>>>>>     without depending on or selecting CRYPTO, despite that config option
>>>>>>>     being subordinate to CRYPTO.
>>>>>>>
>>>>>>>     Signed-off-by: Julian Braha <julianbraha@gmail.com>
>>>>>>
>>>>>> Please use git sent-email to send patches and please fix crypto Kconfig
>>>>>> to enable CRYPTO if CRYPTO_* selected.
>>>>>>
>>>>>> It is a little bit awkward to request all users of CRYPTO_* to request
>>>>>> select CRYPTO too.
>>>>>
>>>>> The same issue and similar patch is in this link:
>>>>>
>>>>> https://patchwork.kernel.org/project/linux-rdma/patch/20200915101559.33292-1-fazilyildiran@gmail.com/#23615747
>>>>
>>>> So what prevents us from fixing CRYPTO Kconfig?
>>>
>>> Yes, I would like to see someone deal with this properly, either every
>>> place doing select CRYPTO_XX needs fixing or something needs to be
>>> done in the crypto layer.
>>>
>>> I have no idea about kconfig to give advice, I've added Arnd since he
>>> always seems to know :)
>>
>> I will Ack the original patch in this thread.
> 
> The one from Julian?

Yes.

> 
>> How many Mellanox drivers are you concerned about?
> 
> ?? This is about rxe

OK.

>> You don't have to fix any other drivers that have a similar issue.
> 
> Why shouldn't they be fixed too?

Of course, but it's not just on you and/or Leon to fix them.

> There is nearly 1000 places that use a 'select CRYPTO_*' in the
> kernel.
> 
> I see only 60 'select CRYPTO' statements.

Correct. :(
We (community) tend to fix bug reports, not do global audits
to see what needs to be fixed (with a few exceptions).


I'll gladly wait to see what Arnd says.
Arnd Bergmann Feb. 23, 2021, 8:36 p.m. UTC | #9 
On Mon, Feb 22, 2021 at 5:53 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> On 2/22/21 8:46 AM, Jason Gunthorpe wrote:
>
> > There is nearly 1000 places that use a 'select CRYPTO_*' in the
> > kernel.
> >
> > I see only 60 'select CRYPTO' statements.

I think most of these are correct, as you typically have a single
'select CRYPTO'
in combination with a couple of 'select CRYPTO_*' ones for the specific
algorithms. I just added a lot of 'select CRC32' statements to deal with
all the build failures in drivers that require this but fail to have an extra
select statement. The way I got the list was to start with 'make allmodconfig'
and then recursively disable everything that had an explicit select, until
I was left with all the modules that need it without selecting it.

The same method could be used for CONFIG_CRYPTO, but it's a few
hours of work.

> Correct. :(
> We (community) tend to fix bug reports, not do global audits
> to see what needs to be fixed (with a few exceptions).
>
>
> I'll gladly wait to see what Arnd says.

For the specific case of CRC32, it might actually a good idea to change
the code to call into the CRC32 code directly instead of the CRYPTO_CRC32
abstraction. Would that work for RDMA_RXE?

       Arnd
Arnd Bergmann Feb. 23, 2021, 8:40 p.m. UTC | #10 
On Tue, Feb 23, 2021 at 9:36 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> For the specific case of CRC32, it might actually a good idea to change
> the code to call into the CRC32 code directly instead of the CRYPTO_CRC32
> abstraction. Would that work for RDMA_RXE?

On the more general question of whether a driver should 'select CRYPTO',
this is how it's currently done for the other users, but I don't
actually like this,
and in general recommend against force-enabling another subsystem when
a particular driver is enabled.

My preference would be to change all drivers that require crypto services
of some kind to use 'depends on CRYPTO' in combination with 'select CRYPTO_*',
as this is what we do for other cross-subsystem dependencies. However,
it seems unlikely that we can change it anytime soon, as the current method
is widespread and changing the dependencies would break users that do
'make oldconfig' on an old configuration.

       Arnd
Arnd Bergmann Feb. 23, 2021, 9:26 p.m. UTC | #11 
On Tue, Feb 23, 2021 at 9:46 PM Julian Braha <julianbraha@gmail.com> wrote:
>
> I have other similar patches that I intend to submit. What should I do,
> going forward? Should I use "depends on CRYPTO" for cases like these?
> Should I resubmit this patch with that change?

No, we should not mix the two methods, that just leads to circular dependencies.

How many more patches do you have that need to get merged?

If it's only a few, I'd suggest merging them first before we consider a
broader change. If the problem is very common, we may want to
think about alternative approaches first, and then change everything
at once.

       Arnd
Julian Braha Feb. 23, 2021, 9:54 p.m. UTC | #12 
On Tuesday, February 23, 2021 4:26:44 PM EST Arnd Bergmann wrote:
> On Tue, Feb 23, 2021 at 9:46 PM Julian Braha <julianbraha@gmail.com> wrote:
> >
> > I have other similar patches that I intend to submit. What should I do,
> > going forward? Should I use "depends on CRYPTO" for cases like these?
> > Should I resubmit this patch with that change?
> 
> No, we should not mix the two methods, that just leads to circular dependencies.
> 
> How many more patches do you have that need to get merged?
> 
> If it's only a few, I'd suggest merging them first before we consider a
> broader change. If the problem is very common, we may want to
> think about alternative approaches first, and then change everything
> at once.
> 
>        Arnd
> 

Sorry, I don't have a specific number, but it's certainly under a dozen patches.
Arnd Bergmann Feb. 23, 2021, 10:05 p.m. UTC | #13 
On Tue, Feb 23, 2021 at 10:54 PM Julian Braha <julianbraha@gmail.com> wrote:
> On Tuesday, February 23, 2021 4:26:44 PM EST Arnd Bergmann wrote:
> > On Tue, Feb 23, 2021 at 9:46 PM Julian Braha <julianbraha@gmail.com> wrote:
> > >
> > > I have other similar patches that I intend to submit. What should I do,
> > > going forward? Should I use "depends on CRYPTO" for cases like these?
> > > Should I resubmit this patch with that change?
> >
> > No, we should not mix the two methods, that just leads to circular dependencies.
> >
> > How many more patches do you have that need to get merged?
> >
> > If it's only a few, I'd suggest merging them first before we consider a
> > broader change. If the problem is very common, we may want to
> > think about alternative approaches first, and then change everything
> > at once.
>
> Sorry, I don't have a specific number, but it's certainly under a dozen patches.

It's probably best to continue pushing those first then.

        Arnd
Leon Romanovsky Feb. 24, 2021, 10:04 a.m. UTC | #14 
On Tue, Feb 23, 2021 at 11:05:45PM +0100, Arnd Bergmann wrote:
> On Tue, Feb 23, 2021 at 10:54 PM Julian Braha <julianbraha@gmail.com> wrote:
> > On Tuesday, February 23, 2021 4:26:44 PM EST Arnd Bergmann wrote:
> > > On Tue, Feb 23, 2021 at 9:46 PM Julian Braha <julianbraha@gmail.com> wrote:
> > > >
> > > > I have other similar patches that I intend to submit. What should I do,
> > > > going forward? Should I use "depends on CRYPTO" for cases like these?
> > > > Should I resubmit this patch with that change?
> > >
> > > No, we should not mix the two methods, that just leads to circular dependencies.
> > >
> > > How many more patches do you have that need to get merged?
> > >
> > > If it's only a few, I'd suggest merging them first before we consider a
> > > broader change. If the problem is very common, we may want to
> > > think about alternative approaches first, and then change everything
> > > at once.
> >
> > Sorry, I don't have a specific number, but it's certainly under a dozen patches.
>
> It's probably best to continue pushing those first then.

Arnd,

I'm hearing it over and over for a long time already. People are focused
on micro-solutions instead of one, powerful change, which is not hard to
do.

Thanks

>
>         Arnd
Jason Gunthorpe March 1, 2021, 6:48 p.m. UTC | #15 
On Fri, Feb 19, 2021 at 06:32:26PM -0500, Julian Braha wrote:
> commit 6e61907779ba99af785f5b2397a84077c289888a
> Author: Julian Braha <julianbraha@gmail.com>
> Date:   Fri Feb 19 18:20:57 2021 -0500
> 
>     drivers: infiniband: sw: rxe: fix kconfig dependency on CRYPTO
>     
>     When RDMA_RXE is enabled and CRYPTO is disabled,
>     Kbuild gives the following warning:
>     
>     WARNING: unmet direct dependencies detected for CRYPTO_CRC32
>       Depends on [n]: CRYPTO [=n]
>       Selected by [y]:
>       - RDMA_RXE [=y] && (INFINIBAND_USER_ACCESS [=y] || !INFINIBAND_USER_ACCESS [=y]) && INET [=y] && PCI [=y] && INFINIBAND [=y] && INFINIBAND_VIRT_DMA [=y]
>     
>     This is because RDMA_RXE selects CRYPTO_CRC32,
>     without depending on or selecting CRYPTO, despite that config option
>     being subordinate to CRYPTO.
>     
>     Signed-off-by: Julian Braha <julianbraha@gmail.com>

Your patch was horribly mangled in patchworks, I fixed it, but you
need to send patches properly if you intend to send more to the kernel

Applied to for-rc

Thanks,
Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/Kconfig b/drivers/infiniband/sw/rxe/Kconfig
index 452149066792..06b8dc5093f7 100644
--- a/drivers/infiniband/sw/rxe/Kconfig
+++ b/drivers/infiniband/sw/rxe/Kconfig
@@ -4,6 +4,7 @@  config RDMA_RXE
        depends on INET && PCI && INFINIBAND
        depends on INFINIBAND_VIRT_DMA
        select NET_UDP_TUNNEL
+      select CRYPTO
        select CRYPTO_CRC32
        help
        This driver implements the InfiniBand RDMA transport over