[PATCH 6/8] nvme-tcp: request secure channel concatenation

[Hannes Reinecke]

On 7/22/24 08:36, Hannes Reinecke wrote:
> On 7/21/24 13:27, Sagi Grimberg wrote:
>>
>>
>>
>> On 18/07/2024 18:06, Hannes Reinecke wrote:
>>> Add a fabrics option 'concat' to request secure channel concatenation.
>>> When secure channel concatenation is enabled a 'generated PSK' is 
>>> inserted
>>> into the keyring such that it's available after reset.
>>>
>>> Signed-off-by: Hannes Reinecke <hare at kernel.org>
>>> ---
>>>   drivers/nvme/host/auth.c    | 100 ++++++++++++++++++++++++++++++++++--
>>>   drivers/nvme/host/fabrics.c |  34 ++++++++++--
>>>   drivers/nvme/host/fabrics.h |   3 ++
>>>   drivers/nvme/host/tcp.c     |  56 +++++++++++++++++---
>>>   include/linux/nvme.h        |   7 +++
>>>   5 files changed, 188 insertions(+), 12 deletions(-)
>>>
[ .. ]
>>> +
>>>   static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
>>>   {
>>>       struct nvmf_ctrl_options *opts = ctrl->opts;
>>> @@ -2308,6 +2348,8 @@ static void nvme_tcp_error_recovery_work(struct 
>>> work_struct *work)
>>>                   struct nvme_tcp_ctrl, err_work);
>>>       struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
>>> +    if (nvme_tcp_key_revoke_needed(ctrl))
>>> +        nvme_tcp_revoke_tls_key(ctrl);
>>
>> Should this maybe move to nvme_tcp_teardown_admin_qeueue ?
>>
> 
> Good point, will do.
> 
Actually, no.

That's precisely where we _cannot_ do it.
(Cf patch to reset the admin queue for secure concatenation).
We have three stages during secure concatenation:
1. authentication:
    ctrl->opts->concat = true, ctrl->opts->tls_key = NULL
    queue->tls_enabled = false
2. reset after authentication:
    ctrl->opts->concat = true, ctrl->opts->tls_key = <set>
    queue->tls_enabled = false
3. TLS enabled on the queue:
    ctrl->opts->concat = true, ctrl->opts->tls_key = <set>
    queue->tls_enabled = true

As we are calling 'nvme_teardown_admin_queue' in step 2)
we cannot clear the tls_key in that function.

Cheers,

Hannes
>>>       nvme_stop_keep_alive(ctrl);
>>>       flush_work(&ctrl->async_event_work);
>>>       nvme_tcp_teardown_io_queues(ctrl, false);
>>> @@ -2348,6 +2390,8 @@ static void nvme_reset_ctrl_work(struct 
>>> work_struct *work)
>>>           container_of(work, struct nvme_ctrl, reset_work);
>>>       int ret;
>>> +    if (nvme_tcp_key_revoke_needed(ctrl))
>>> +        nvme_tcp_revoke_tls_key(ctrl);
>>
>> Would eliminate this call...
> 
> Cheers,
> 
> Hannes

-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at 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