Re: [PATCH 09/10] nvmet-tcp: support secure channel concatenation

Hannes Reinecke
2025-01-13T09:37:55Z

On 1/12/25 23:38, Keith Busch wrote:
> On Thu, Jan 09, 2025 at 08:33:51AM +0100, Hannes Reinecke wrote:
>> On 1/8/25 17:33, Keith Busch wrote:
>>> On Tue, Dec 03, 2024 at 12:02:37PM +0100, Hannes Reinecke wrote:
>>>> Evaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert
>>>> the generated PSK once negotiation has finished.
>>>
>>> ...
>>>
>>>> @@ -251,7 +267,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
>>>> uuid_copy(&ctrl->hostid, &d->hostid);
>>>> - dhchap_status = nvmet_setup_auth(ctrl);
>>>> + dhchap_status = nvmet_setup_auth(ctrl, req);
>>>> if (dhchap_status) {
>>>> pr_err("Failed to setup authentication, dhchap status %u\n",
>>>> dhchap_status);
>>>> @@ -269,12 +285,13 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
>>>> goto out;
>>>> }
>>>
>>> This one had some merge conflicts after applying the pci endpoint
>>> series from Damien. I tried to resolve it, the result is here:
>>>
>>> https://git.infradead.org/?p=nvme.git;a=commitdiff;h=11cb42c0f4f4450b325e38c8f0f7d77f5e1a0eb0
>>>
>>> The main conflict was from moving the nvmet_setup_auth() call from
>>> nvmet_execute_admin_connect() to nvmet_alloc_ctrl().
>>
>> I'll give it a spin and check how it holds up.
>
> Sorry, I had to drop this from 6.14 for now. The build bot tagged us
> with the following error. It looks easy enough to fix but I can't do it
> over the weekened before the first merge window pull :)
>
> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> branch HEAD: f28201922a357663d4a2a258e024481e19269c2c Merge branch 'for-6.14/block' into for-next
>
> Error/Warning (recently discovered and may have been fixed):
>
> https://lore.kernel.org/oe-kbuild-all/202501120730.Nix2qru3-lkp@intel.com
>
> auth.c:(.text+0x986): undefined reference to `nvme_tls_psk_refresh'
> csky-linux-ld: auth.c:(.text+0xa00): undefined reference to `nvme_tls_psk_refresh'
>
> Error/Warning ids grouped by kconfigs:
>
> recent_errors
> `-- csky-randconfig-001-20250112
> |-- auth.c:(.text):undefined-reference-to-nvme_tls_psk_refresh
> `-- csky-linux-ld:auth.c:(.text):undefined-reference-to-nvme_tls_psk_refresh

Don't you also love kbuild robot ...

Fix is quite easy:

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 486afe598184..10e453b2436e 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -109,7 +109,7 @@ config NVME_HOST_AUTH
bool "NVMe over Fabrics In-Band Authentication in host side"
depends on NVME_CORE
select NVME_AUTH
- select NVME_KEYRING if NVME_TCP_TLS
+ select NVME_KEYRING
help
This provides support for NVMe over Fabrics In-Band
Authentication in
host side.

which obviously needs to be folded into patch 'nvme-tcp: request
secure channel concatenation' (the cited patch is a red herring;
it only exposes the issue, but the issue got introduced with the
patch to nvme-tcp).

Can you fold it in or shall I resubmit?

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich