[MPTCP] Re: [RFC PATCH net] mptcp: select CRYPTO

[MPTCP] Re: [RFC PATCH net] mptcp: select CRYPTO

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

On Wed, 12 Feb 2020, Matthieu Baerts wrote:

> Without this modification and if CRYPTO is not selected, we have this
> warning:
>
>  WARNING: unmet direct dependencies detected for CRYPTO_LIB_SHA256
>    Depends on [n]: CRYPTO [=n]
>    Selected by [y]:
>    - MPTCP [=y] && NET [=y] && INET [=y]
>
> MPTCP selects CRYPTO_LIB_SHA256 which depends on CRYPTO. CRYPTO is now
> selected to avoid this issue.
>
> Fixes: 65492c5a6ab5 (mptcp: move from sha1 (v0) to sha256 (v1))
> Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> ---
> net/mptcp/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mptcp/Kconfig b/net/mptcp/Kconfig
> index 49f6054e7f4e..a9ed3bf1d93f 100644
> --- a/net/mptcp/Kconfig
> +++ b/net/mptcp/Kconfig
> @@ -4,6 +4,7 @@ config MPTCP
> 	depends on INET
> 	select SKB_EXTENSIONS
> 	select CRYPTO_LIB_SHA256
> +	select CRYPTO
> 	help
> 	  Multipath TCP (MPTCP) connections send and receive data over multiple
> 	  subflows in order to utilize multiple network paths. Each subflow
> -- 
> 2.25.0

Even though the config system prints that warning, it looks like sha256 is 
compiled and linked even without CONFIG_CRYPTO. Since we end up needing 
CONFIG_CRYPTO anyway for crypto_memneq, it's not bad to include it now, 
but it doesn't seem to be strictly necessary.

--
Mat Martineau
Intel

[MPTCP] Re: [RFC PATCH net] mptcp: select CRYPTO

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]

Hi Mat,

On 12/02/2020 20:31, Mat Martineau wrote:
> On Wed, 12 Feb 2020, Matthieu Baerts wrote:
> 
>> Without this modification and if CRYPTO is not selected, we have this
>> warning:
>>
>>  WARNING: unmet direct dependencies detected for CRYPTO_LIB_SHA256
>>    Depends on [n]: CRYPTO [=n]
>>    Selected by [y]:
>>    - MPTCP [=y] && NET [=y] && INET [=y]
>>
>> MPTCP selects CRYPTO_LIB_SHA256 which depends on CRYPTO. CRYPTO is now
>> selected to avoid this issue.
>>
>> Fixes: 65492c5a6ab5 (mptcp: move from sha1 (v0) to sha256 (v1))
>> Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
>> ---
>> net/mptcp/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/mptcp/Kconfig b/net/mptcp/Kconfig
>> index 49f6054e7f4e..a9ed3bf1d93f 100644
>> --- a/net/mptcp/Kconfig
>> +++ b/net/mptcp/Kconfig
>> @@ -4,6 +4,7 @@ config MPTCP
>>     depends on INET
>>     select SKB_EXTENSIONS
>>     select CRYPTO_LIB_SHA256
>> +    select CRYPTO
>>     help
>>       Multipath TCP (MPTCP) connections send and receive data over 
>> multiple
>>       subflows in order to utilize multiple network paths. Each subflow
>> -- 
>> 2.25.0
> 
> Even though the config system prints that warning, it looks like sha256 
> is compiled and linked even without CONFIG_CRYPTO. Since we end up 
> needing CONFIG_CRYPTO anyway for crypto_memneq, it's not bad to include 
> it now, but it doesn't seem to be strictly necessary.

Thank you for the review.

I agree with you. I don't even know where the dependence is added. In 
the code, we can see:

   config CRYPTO_LIB_SHA256
        tristate

But "make menuconfig" shows the dependence with CRYPTO.

Do you think we don't need to send it to "net"?
Maybe I should add something like this in the commit message?

   Even though the config system prints that warning, it looks like 
sha256 is compiled and linked even without CONFIG_CRYPTO. Since we will 
end up needing CONFIG_CRYPTO anyway in future commits that will be sent 
for the next version, we propose to add it now to fix the warning and 
prepare for the next commits.

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium