Re: [PATCH 2/2] nvme: keyring: fix conditional compilation

[Hannes Reinecke <hare@suse.de>]

On 10/25/23 14:16, Arnd Bergmann wrote:
> On Wed, Oct 25, 2023, at 11:11, Hannes Reinecke wrote:
>> On 10/25/23 10:20, Arnd Bergmann wrote:
>>> On Wed, Oct 25, 2023, at 10:12, Hannes Reinecke wrote:
>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>
>>>> The keyring and auth functions can be called from both the host and
>>>> the target side and are controlled by Kconfig options for each of the
>>>> combinations, but the declarations are controlled by #ifdef checks
>>>> on the shared Kconfig symbols.
>>>>
>>>> This leads to link failures in combinations where one of the frontends
>>>> is built-in and the other one is a module, and the keyring code
>>>> ends up in a module that is not reachable from the builtin code:
>>>>
>>>> ld: drivers/nvme/host/core.o: in function `nvme_core_exit':
>>>> core.c:(.exit.text+0x4): undefined reference to `nvme_keyring_exit'
>>>> ld: drivers/nvme/host/core.o: in function `nvme_core_init':
>>>> core.c:(.init.text+0x94): undefined reference to `nvme_keyring_init
>>>>
>>>> ld: drivers/nvme/host/tcp.o: in function `nvme_tcp_setup_ctrl':
>>>> tcp.c:(.text+0x4c18): undefined reference to `nvme_tls_psk_default'
>>>>
>>>> Address this by moving nvme_auth_init()/nvme_auth_exit() into
>>>> module init/exit functions for the keyring module.
>>>>
>>>> Fixes: be8e82caa6859 ("nvme-tcp: enable TLS handshake upcall")
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>>
>>> Your patch looks good to me, and I think this fixes a
>>> separate problem with missing the initialization of the
>>> keyring when the kernel has only the target driver but
>>> no host support, but it has nothing to do with the changelog
>>> I wrote above and does not fix the build regression
>>> I described.
>>>
>> Hmm. Can you send me the .config file (or the config options for NVMe
>> where there failure occurred)? I tried the configurations mentioned, and
>> it worked for me.
>> But apparently I didn't get the right combination...
> 
> Sorry if my explanations were not clear enough. The two
> broken cases can be triggered using:
> 
> https://pastebin.com/aVQ1UbTv
> CONFIG_NVME_KEYRING=m
> CONFIG_NVME_AUTH=m
> CONFIG_NVME_CORE=y
> CONFIG_NVME_MULTIPATH=y
> CONFIG_NVME_VERBOSE_ERRORS=y
> CONFIG_NVME_FABRICS=y
> CONFIG_NVME_FC=y
> CONFIG_NVME_TCP=y
> # CONFIG_NVME_TCP_TLS is not set
> # CONFIG_NVME_HOST_AUTH is not set
> CONFIG_NVME_TARGET=m
> # CONFIG_NVME_TARGET_PASSTHRU is not set
> # CONFIG_NVME_TARGET_LOOP is not set
> CONFIG_NVME_TARGET_FC=m
> CONFIG_NVME_TARGET_FCLOOP=m
> CONFIG_NVME_TARGET_TCP=m
> CONFIG_NVME_TARGET_TCP_TLS=y
> CONFIG_NVME_TARGET_AUTH=y
> ld: drivers/nvme/host/tcp.o: in function `nvme_tcp_setup_ctrl':
> tcp.c:(.text+0x1099): undefined reference to `nvme_tls_psk_default'
> 
> and
> 
> https://pastebin.com/jXQ71vn9
> CONFIG_NVME_KEYRING=m
> CONFIG_NVME_AUTH=m
> CONFIG_NVME_CORE=m
> CONFIG_NVME_MULTIPATH=y
> CONFIG_NVME_VERBOSE_ERRORS=y
> CONFIG_NVME_FABRICS=m
> CONFIG_NVME_FC=m
> CONFIG_NVME_TCP=m
> CONFIG_NVME_TCP_TLS=y
> CONFIG_NVME_HOST_AUTH=y
> CONFIG_NVME_TARGET=y
> # CONFIG_NVME_TARGET_LOOP is not set
> CONFIG_NVME_TARGET_FC=m
> CONFIG_NVME_TARGET_FCLOOP=m
> CONFIG_NVME_TARGET_TCP=y
> # CONFIG_NVME_TARGET_TCP_TLS is not set
> # CONFIG_NVME_TARGET_AUTH is not set
> ld: drivers/nvme/target/configfs.o: in function `nvmet_ports_make':
> configfs.c:(.text+0x13b8): undefined reference to `nvme_keyring_id'
> 
> The problem is the same in each case: one of the two callers
> (target or host) is set to =m and enables keyring support, while
> the other one is built-in but doesn't use the keyring. This
> leads to the keyring code being built as a loadable module, but
> include/linux/nvme-keyring.h contains broken stub functions
> that lead to it still getting referenced from the built-in
> code.
> 
Ah. RTFM.

'IS_REACHABLE' is the keyword here; if we use that in nvme-keyring.h
things are resolved.

Will be sending an updated patch.

Cheers.

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman