From: Arnd Bergmann <[email protected]>
There are still a couple of link failures that I tried to address
with a previous patch. I've split up the missing bits into smaller
patches and tried to explain the bugs in more detail.
With these applied, randconfig builds work again. Please either
merge them or treat them as bug reports and find a different fix,
I won't do another version.
Arnd Bergmann (3):
nvme: target: fix nvme_keyring_id() references
nvme: target: fix Kconfig select statements
nvme: tcp: fix compile-time checks for TLS mode
drivers/nvme/host/tcp.c | 31 ++++++++++++++-----------------
drivers/nvme/target/Kconfig | 4 ++--
drivers/nvme/target/configfs.c | 2 +-
3 files changed, 17 insertions(+), 20 deletions(-)
--
2.39.2
Cc: Keith Busch <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: Chaitanya Kulkarni <[email protected]>
Cc: [email protected]
Cc: [email protected]
From: Arnd Bergmann <[email protected]>
When the NVME target code is built-in but its TCP frontend is a loadable
module, enabling keyring support causes a link failure:
x86_64-linux-ld: vmlinux.o: in function `nvmet_ports_make':
configfs.c:(.text+0x100a211): undefined reference to `nvme_keyring_id'
The problem is that CONFIG_NVME_TARGET_TCP_TLS is a 'bool' symbol that
depends on the tristate CONFIG_NVME_TARGET_TCP, so any 'select' from
it inherits the state of the tristate symbol rather than the intended
CONFIG_NVME_TARGET one that contains the actual call.
The same thing is true for CONFIG_KEYS, which itself is required for
NVME_KEYRING.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/nvme/target/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
index 31633da9427c..e1ebc73f3e5e 100644
--- a/drivers/nvme/target/Kconfig
+++ b/drivers/nvme/target/Kconfig
@@ -4,6 +4,8 @@ config NVME_TARGET
tristate "NVMe Target support"
depends on BLOCK
depends on CONFIGFS_FS
+ select NVME_KEYRING if NVME_TARGET_TCP_TLS
+ select KEYS if NVME_TARGET_TCP_TLS
select BLK_DEV_INTEGRITY_T10 if BLK_DEV_INTEGRITY
select SGL_ALLOC
help
@@ -87,9 +89,7 @@ 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_KEYRING
select NET_HANDSHAKE
- select KEYS
help
Enables TLS encryption for the NVMe TCP target using the netlink handshake API.
--
2.39.2
From: Arnd Bergmann <[email protected]>
When CONFIG_NVME_KEYRING is enabled as a loadable module, but the TCP
host code is built-in, it fails to link:
arm-linux-gnueabi-ld: drivers/nvme/host/tcp.o: in function `nvme_tcp_setup_ctrl':
tcp.c:(.text+0x1940): undefined reference to `nvme_tls_psk_default'
The problem is that the compile-time conditionals are inconsistent here,
using a mix of #ifdef CONFIG_NVME_TCP_TLS, IS_ENABLED(CONFIG_NVME_TCP_TLS)
and IS_ENABLED(CONFIG_NVME_KEYRING) checks, with CONFIG_NVME_KEYRING
controlling whether the implementation is actually built.
Change it to use IS_ENABLED(CONFIG_NVME_KEYRING) checks consistently,
which should help readability and make it less error-prone. Combining
it with the check for the ctrl->opts->tls flag lets the compiler drop
all the TLS code in configurations without this feature, which also
helps runtime behavior in addition to avoiding the link failure.
To make it possible for the compiler to build the dead code, both
the tls_handshake_timeout variable and the TLS specific members
of nvme_tcp_queue need to be moved out of the #ifdef block as well,
but at least the former of these gets optimized out again.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/nvme/host/tcp.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 89661a9cf850..ee7aa8478cfb 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -36,11 +36,11 @@ static int so_priority;
module_param(so_priority, int, 0644);
MODULE_PARM_DESC(so_priority, "nvme tcp socket optimize priority");
-#ifdef CONFIG_NVME_TCP_TLS
/*
* TLS handshake timeout
*/
static int tls_handshake_timeout = 10;
+#ifdef CONFIG_NVME_TCP_TLS
module_param(tls_handshake_timeout, int, 0644);
MODULE_PARM_DESC(tls_handshake_timeout,
"nvme TLS handshake timeout in seconds (default 10)");
@@ -161,10 +161,8 @@ struct nvme_tcp_queue {
struct ahash_request *snd_hash;
__le32 exp_ddgst;
__le32 recv_ddgst;
-#ifdef CONFIG_NVME_TCP_TLS
struct completion tls_complete;
int tls_err;
-#endif
struct page_frag_cache pf_cache;
void (*state_change)(struct sock *);
@@ -207,6 +205,14 @@ static inline int nvme_tcp_queue_id(struct nvme_tcp_queue *queue)
return queue - queue->ctrl->queues;
}
+static inline bool nvme_tcp_tls(struct nvme_ctrl *ctrl)
+{
+ if (!IS_ENABLED(CONFIG_NVME_TCP_TLS))
+ return 0;
+
+ return ctrl->opts->tls;
+}
+
static inline struct blk_mq_tags *nvme_tcp_tagset(struct nvme_tcp_queue *queue)
{
u32 queue_idx = nvme_tcp_queue_id(queue);
@@ -1412,7 +1418,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
memset(&msg, 0, sizeof(msg));
iov.iov_base = icresp;
iov.iov_len = sizeof(*icresp);
- if (queue->ctrl->ctrl.opts->tls) {
+ if (nvme_tcp_tls(&queue->ctrl->ctrl)) {
msg.msg_control = cbuf;
msg.msg_controllen = sizeof(cbuf);
}
@@ -1424,7 +1430,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
goto free_icresp;
}
ret = -ENOTCONN;
- if (queue->ctrl->ctrl.opts->tls) {
+ if (nvme_tcp_tls(&queue->ctrl->ctrl)) {
ctype = tls_get_record_type(queue->sock->sk,
(struct cmsghdr *)cbuf);
if (ctype != TLS_RECORD_TYPE_DATA) {
@@ -1548,7 +1554,6 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue)
queue->io_cpu = cpumask_next_wrap(n - 1, cpu_online_mask, -1, false);
}
-#ifdef CONFIG_NVME_TCP_TLS
static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid)
{
struct nvme_tcp_queue *queue = data;
@@ -1625,14 +1630,6 @@ static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
}
return ret;
}
-#else
-static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
- struct nvme_tcp_queue *queue,
- key_serial_t pskid)
-{
- return -EPROTONOSUPPORT;
-}
-#endif
static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
key_serial_t pskid)
@@ -1759,7 +1756,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
}
/* If PSKs are configured try to start TLS */
- if (pskid) {
+ if (IS_ENABLED(CONFIG_NVME_TCP_TLS) && pskid) {
ret = nvme_tcp_start_tls(nctrl, queue, pskid);
if (ret)
goto err_init_connect;
@@ -1916,7 +1913,7 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl)
int ret;
key_serial_t pskid = 0;
- if (ctrl->opts->tls) {
+ if (nvme_tcp_tls(ctrl)) {
if (ctrl->opts->tls_key)
pskid = key_serial(ctrl->opts->tls_key);
else
@@ -1949,7 +1946,7 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl)
{
int i, ret;
- if (ctrl->opts->tls && !ctrl->tls_key) {
+ if (nvme_tcp_tls(ctrl) && !ctrl->tls_key) {
dev_err(ctrl->device, "no PSK negotiated\n");
return -ENOKEY;
}
--
2.39.2
On 11/22/23 3:47 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> There are still a couple of link failures that I tried to address
> with a previous patch. I've split up the missing bits into smaller
> patches and tried to explain the bugs in more detail.
>
> With these applied, randconfig builds work again. Please either
> merge them or treat them as bug reports and find a different fix,
> I won't do another version.
Applied, but had to hand-apply hunk 9 of patch 3 due to a previous
attempt at this:
commit 23441536b63677cb2ed9b1637d8ca70315e44bd0
Author: Hannes Reinecke <[email protected]>
Date: Tue Nov 14 14:18:21 2023 +0100
nvme-tcp: only evaluate 'tls' option if TLS is selected
--
Jens Axboe
On Wed, Nov 22, 2023 at 11:47:16PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> There are still a couple of link failures that I tried to address
> with a previous patch. I've split up the missing bits into smaller
> patches and tried to explain the bugs in more detail.
>
> With these applied, randconfig builds work again. Please either
> merge them or treat them as bug reports and find a different fix,
> I won't do another version.
Jens is taking these directly, but for the record: looks good to me and
thanks for the fixes.
Acked-by: Keith Busch <[email protected]>
On Thu, Nov 23, 2023, at 02:42, Jens Axboe wrote:
> On 11/22/23 3:47 PM, Arnd Bergmann wrote:
>> From: Arnd Bergmann <[email protected]>
>>
>> There are still a couple of link failures that I tried to address
>> with a previous patch. I've split up the missing bits into smaller
>> patches and tried to explain the bugs in more detail.
>>
>> With these applied, randconfig builds work again. Please either
>> merge them or treat them as bug reports and find a different fix,
>> I won't do another version.
>
> Applied, but had to hand-apply hunk 9 of patch 3 due to a previous
> attempt at this:
>
> commit 23441536b63677cb2ed9b1637d8ca70315e44bd0
> Author: Hannes Reinecke <[email protected]>
> Date: Tue Nov 14 14:18:21 2023 +0100
>
> nvme-tcp: only evaluate 'tls' option if TLS is selected
Ok, thanks for merging my changes! The commit from Hannes
is what I had in my v1 for this, and it was a correct fix
as well, my patch 3/3 was just a more elaborate way to do
the same thing that I did since Hannes did not like my
version at first.
The 23441536b6 commit was not in linux-next yesterday, so
it looks like our patches crossed paths on the same day.
Arnd
On Wed, 22 Nov 2023 23:47:16 +0100, Arnd Bergmann wrote:
> There are still a couple of link failures that I tried to address
> with a previous patch. I've split up the missing bits into smaller
> patches and tried to explain the bugs in more detail.
>
> With these applied, randconfig builds work again. Please either
> merge them or treat them as bug reports and find a different fix,
> I won't do another version.
>
> [...]
Applied, thanks!
[1/3] nvme: target: fix nvme_keyring_id() references
commit: d78abcbabe7e98bb4baa4dea87550806944790ed
[2/3] nvme: target: fix Kconfig select statements
commit: 65e2a74c44ddfa174b700f5da2d1d29b4ba6639b
[3/3] nvme: tcp: fix compile-time checks for TLS mode
commit: 0e6c4fe782e683ff55a27fbb10e9c6b5c241533b
Best regards,
--
Jens Axboe
On 11/22/23 11:25 PM, Arnd Bergmann wrote:
> On Thu, Nov 23, 2023, at 02:42, Jens Axboe wrote:
>> On 11/22/23 3:47 PM, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <[email protected]>
>>>
>>> There are still a couple of link failures that I tried to address
>>> with a previous patch. I've split up the missing bits into smaller
>>> patches and tried to explain the bugs in more detail.
>>>
>>> With these applied, randconfig builds work again. Please either
>>> merge them or treat them as bug reports and find a different fix,
>>> I won't do another version.
>>
>> Applied, but had to hand-apply hunk 9 of patch 3 due to a previous
>> attempt at this:
>>
>> commit 23441536b63677cb2ed9b1637d8ca70315e44bd0
>> Author: Hannes Reinecke <[email protected]>
>> Date: Tue Nov 14 14:18:21 2023 +0100
>>
>> nvme-tcp: only evaluate 'tls' option if TLS is selected
>
> Ok, thanks for merging my changes! The commit from Hannes
> is what I had in my v1 for this, and it was a correct fix
> as well, my patch 3/3 was just a more elaborate way to do
> the same thing that I did since Hannes did not like my
> version at first.
>
> The 23441536b6 commit was not in linux-next yesterday, so
> it looks like our patches crossed paths on the same day.
yeah, it was just recently merged, which is probably why it wasn't there
yet and you didn't see it. Not a big deal, easy enough to resolve - just
wanted to make sure you knew about it. It should all be heading upstream
shortly, just sent out the pull.
--
Jens Axboe