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

[Arnd Bergmann]

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 at 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 at arndb.de>
>>> Signed-off-by: Hannes Reinecke <hare at 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.

     Arnd