[PATCH 0/2] [v3]: nvme: fixup module compilation

[PATCH 0/2] [v3]: nvme: fixup module compilation

[Hannes Reinecke]

Hi all,

Arnd noticed that the module selection between keyring and
host/target code fails under certain combinations.
This patchset addresses this by making 'keyring' into a
'proper' module by adding module_init()/module_exit() functions.

Arnd Bergmann (1):
  nvme: common: make keyring and auth separate modules
  nvme: keyring: fix conditional compilation

 drivers/nvme/Makefile         |  2 +-
 drivers/nvme/common/Kconfig   |  7 ++-----
 drivers/nvme/common/Makefile  |  7 ++++---
 drivers/nvme/common/keyring.c | 11 +++++++----
 drivers/nvme/host/Kconfig     |  2 --
 drivers/nvme/host/core.c      |  9 +--------
 drivers/nvme/target/Kconfig   |  2 --
 include/linux/nvme-keyring.h  | 10 +---------
 8 files changed, 16 insertions(+), 34 deletions(-)

-- 
2.35.3

[PATCH 1/2] nvme: common: make keyring and auth separate modules

[Hannes Reinecke]

From: Arnd Bergmann <arnd@arndb.de>

When only the keyring module is included but auth is not, modpost
complains about the lack of a module license tag:

ERROR: modpost: missing MODULE_LICENSE() in drivers/nvme/common/nvme-common.o

Address this by making both modules buildable standalone,
removing the now unnecessary CONFIG_NVME_COMMON symbol
in the process.

Fixes: 9d77eb5277849 ("nvme-keyring: register '.nvme' keyring")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/Makefile         | 2 +-
 drivers/nvme/common/Kconfig   | 7 ++-----
 drivers/nvme/common/Makefile  | 7 ++++---
 drivers/nvme/common/keyring.c | 2 ++
 drivers/nvme/host/Kconfig     | 2 --
 drivers/nvme/target/Kconfig   | 2 --
 include/linux/nvme-keyring.h  | 2 +-
 7 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/nvme/Makefile b/drivers/nvme/Makefile
index eedca8c72098..74f59ceed3d5 100644
--- a/drivers/nvme/Makefile
+++ b/drivers/nvme/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-obj-$(CONFIG_NVME_COMMON)		+= common/
+obj-y		+= common/
 obj-y		+= host/
 obj-y		+= target/
diff --git a/drivers/nvme/common/Kconfig b/drivers/nvme/common/Kconfig
index 06c8df00d1e2..244432e0b73d 100644
--- a/drivers/nvme/common/Kconfig
+++ b/drivers/nvme/common/Kconfig
@@ -1,14 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-config NVME_COMMON
-       tristate
-
 config NVME_KEYRING
-       bool
+       tristate
        select KEYS
 
 config NVME_AUTH
-	bool
+	tristate
 	select CRYPTO
 	select CRYPTO_HMAC
 	select CRYPTO_SHA256
diff --git a/drivers/nvme/common/Makefile b/drivers/nvme/common/Makefile
index 0cbd0b0b8d49..681514cf2e2f 100644
--- a/drivers/nvme/common/Makefile
+++ b/drivers/nvme/common/Makefile
@@ -2,7 +2,8 @@
 
 ccflags-y			+= -I$(src)
 
-obj-$(CONFIG_NVME_COMMON)	+= nvme-common.o
+obj-$(CONFIG_NVME_AUTH)		+= nvme-auth.o
+obj-$(CONFIG_NVME_KEYRING)	+= nvme-keyring.o
 
-nvme-common-$(CONFIG_NVME_AUTH)	+= auth.o
-nvme-common-$(CONFIG_NVME_KEYRING) += keyring.o
+nvme-auth-y			+= auth.o
+nvme-keyring-y			+= keyring.o
diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index f8d9a208397b..46d7a537dbc2 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -180,3 +180,5 @@ void nvme_keyring_exit(void)
 	key_put(nvme_keyring);
 }
 EXPORT_SYMBOL_GPL(nvme_keyring_exit);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 48f7d72de5e9..8fe2dd619e80 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -95,7 +95,6 @@ config NVME_TCP
 config NVME_TCP_TLS
 	bool "NVMe over Fabrics TCP TLS encryption support"
 	depends on NVME_TCP
-	select NVME_COMMON
 	select NVME_KEYRING
 	select NET_HANDSHAKE
 	select KEYS
@@ -110,7 +109,6 @@ config NVME_TCP_TLS
 config NVME_HOST_AUTH
 	bool "NVM Express over Fabrics In-Band Authentication"
 	depends on NVME_CORE
-	select NVME_COMMON
 	select NVME_AUTH
 	help
 	  This provides support for NVMe over Fabrics In-Band Authentication.
diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
index fa479c9f5c3d..31633da9427c 100644
--- a/drivers/nvme/target/Kconfig
+++ b/drivers/nvme/target/Kconfig
@@ -87,7 +87,6 @@ config NVME_TARGET_TCP
 config NVME_TARGET_TCP_TLS
 	bool "NVMe over Fabrics TCP target TLS encryption support"
 	depends on NVME_TARGET_TCP
-	select NVME_COMMON
 	select NVME_KEYRING
 	select NET_HANDSHAKE
 	select KEYS
@@ -102,7 +101,6 @@ config NVME_TARGET_TCP_TLS
 config NVME_TARGET_AUTH
 	bool "NVMe over Fabrics In-band Authentication support"
 	depends on NVME_TARGET
-	select NVME_COMMON
 	select NVME_AUTH
 	help
 	  This enables support for NVMe over Fabrics In-band Authentication
diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h
index 4efea9dd967c..6cc0696625f3 100644
--- a/include/linux/nvme-keyring.h
+++ b/include/linux/nvme-keyring.h
@@ -6,7 +6,7 @@
 #ifndef _NVME_KEYRING_H
 #define _NVME_KEYRING_H
 
-#ifdef CONFIG_NVME_KEYRING
+#if IS_ENABLED(CONFIG_NVME_KEYRING)
 
 key_serial_t nvme_tls_psk_default(struct key *keyring,
 		const char *hostnqn, const char *subnqn);
-- 
2.35.3

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

[Hannes Reinecke]

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>
---
 drivers/nvme/common/keyring.c | 9 +++++----
 drivers/nvme/host/core.c      | 9 +--------
 include/linux/nvme-keyring.h  | 8 --------
 3 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 46d7a537dbc2..ee341b83eeba 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -151,7 +151,7 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 }
 EXPORT_SYMBOL_GPL(nvme_tls_psk_default);
 
-int nvme_keyring_init(void)
+static int __init nvme_keyring_init(void)
 {
 	int err;
 
@@ -171,14 +171,15 @@ int nvme_keyring_init(void)
 	}
 	return 0;
 }
-EXPORT_SYMBOL_GPL(nvme_keyring_init);
 
-void nvme_keyring_exit(void)
+static void __exit nvme_keyring_exit(void)
 {
 	unregister_key_type(&nvme_tls_psk_key_type);
 	key_revoke(nvme_keyring);
 	key_put(nvme_keyring);
 }
-EXPORT_SYMBOL_GPL(nvme_keyring_exit);
 
 MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hannes Reinecke <hare@suse.de>");
+module_init(nvme_keyring_init);
+module_exit(nvme_keyring_exit);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f48b4f735d2d..47645a219128 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -25,7 +25,6 @@
 #include "nvme.h"
 #include "fabrics.h"
 #include <linux/nvme-auth.h>
-#include <linux/nvme-keyring.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -4724,16 +4723,11 @@ static int __init nvme_core_init(void)
 		result = PTR_ERR(nvme_ns_chr_class);
 		goto unregister_generic_ns;
 	}
-	result = nvme_keyring_init();
-	if (result)
-		goto destroy_ns_chr;
 	result = nvme_init_auth();
 	if (result)
-		goto keyring_exit;
+		goto destroy_ns_chr;
 	return 0;
 
-keyring_exit:
-	nvme_keyring_exit();
 destroy_ns_chr:
 	class_destroy(nvme_ns_chr_class);
 unregister_generic_ns:
@@ -4757,7 +4751,6 @@ static int __init nvme_core_init(void)
 static void __exit nvme_core_exit(void)
 {
 	nvme_exit_auth();
-	nvme_keyring_exit();
 	class_destroy(nvme_ns_chr_class);
 	class_destroy(nvme_subsys_class);
 	class_destroy(nvme_class);
diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h
index 6cc0696625f3..e10333d78dbb 100644
--- a/include/linux/nvme-keyring.h
+++ b/include/linux/nvme-keyring.h
@@ -12,8 +12,6 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 		const char *hostnqn, const char *subnqn);
 
 key_serial_t nvme_keyring_id(void);
-int nvme_keyring_init(void);
-void nvme_keyring_exit(void);
 
 #else
 
@@ -26,11 +24,5 @@ static inline key_serial_t nvme_keyring_id(void)
 {
 	return 0;
 }
-static inline int nvme_keyring_init(void)
-{
-	return 0;
-}
-static inline void nvme_keyring_exit(void) {}
-
 #endif /* !CONFIG_NVME_KEYRING */
 #endif /* _NVME_KEYRING_H */
-- 
2.35.3

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

[Arnd Bergmann]

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.

     Arnd

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

[Hannes Reinecke]

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...

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

Re: [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@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.

     Arnd

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

[Hannes Reinecke]

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

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

[Arnd Bergmann]

On Wed, Oct 25, 2023, at 17:00, Hannes Reinecke wrote:
> 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:
>
> 'IS_REACHABLE' is the keyword here; if we use that in nvme-keyring.h
> things are resolved.
>
> Will be sending an updated patch.

Please don't use IS_REACHABLE(), it's almost never the right
solution, even if it avoids this particular build failure,
it tends to cause hard to debug problems when features that
are enabled as modules end up not working correctly.

I still think my original patch or a variation of it is
the best option since it keys the usage of the module
directly on whether the caller actually wants to use it.

Another option would be to use Kconfig or Makefile logic
to force it to be built-in if at least one of the two
callers is built-in and at least one of them tries to
use it.

     Arnd

Re: [PATCH 1/2] nvme: common: make keyring and auth separate modules

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Christoph Hellwig]

On Wed, Oct 25, 2023 at 05:35:12PM +0200, Arnd Bergmann wrote:
> On Wed, Oct 25, 2023, at 17:00, Hannes Reinecke wrote:
> > 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:
> >
> > 'IS_REACHABLE' is the keyword here; if we use that in nvme-keyring.h
> > things are resolved.
> >
> > Will be sending an updated patch.
> 
> Please don't use IS_REACHABLE(), it's almost never the right
> solution, even if it avoids this particular build failure,
> it tends to cause hard to debug problems when features that
> are enabled as modules end up not working correctly.

Agreed.

> Another option would be to use Kconfig or Makefile logic
> to force it to be built-in if at least one of the two
> callers is built-in and at least one of them tries to
> use it.

That's the only sensible thing to do, a that is what the user
really wants.