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

[Hannes Reinecke]

On 8/13/24 21:45, Sagi Grimberg wrote:
> 
> On 13/08/2024 14:15, 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    | 108 +++++++++++++++++++++++++++++++++++-
>>   drivers/nvme/host/fabrics.c |  34 +++++++++++-
>>   drivers/nvme/host/fabrics.h |   3 +
>>   drivers/nvme/host/nvme.h    |   2 +
>>   drivers/nvme/host/sysfs.c   |   4 +-
>>   drivers/nvme/host/tcp.c     |  47 ++++++++++++++--
>>   include/linux/nvme.h        |   7 +++
>>   7 files changed, 191 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
>> index 371e14f0a203..d2fdaefd236f 100644
>> --- a/drivers/nvme/host/auth.c
>> +++ b/drivers/nvme/host/auth.c
[ .. ]
>> @@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct 
>> work_struct *work)
>>       }
>>       if (!ret) {
>>           chap->error = 0;
>> +        /* Secure concatenation can only be enabled on the admin 
>> queue */
>> +        if (!chap->qid && ctrl->opts->concat &&
>> +            (ret = nvme_auth_secure_concat(ctrl, chap))) {
>> +            dev_warn(ctrl->device,
>> +                 "%s: qid %d failed to enable secure concatenation\n",
>> +                 __func__, chap->qid);
>> +            chap->error = ret;
>> +        }
> 
> Lets break up the if condition:
> 
>      if (!chap->qid && ctrl->opts->concat) {
>          ret = nvme_auth_secure_concat(ctrl, chap);
>          if (!ret)
>              return;
>          dev_warn(ctrl->device,
>              "%s: qid %d failed to enable secure concatenation\n",
>               __func__, chap->qid);
>          chap->error = ret;
>      }
> 
> 
> 
Okay.

>>           return;
> 
> No need for the return statement.
> 
[ .. ]
>> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
>> index 7b2ae2e43544..8072edf3c842 100644
>> --- a/include/linux/nvme.h
>> +++ b/include/linux/nvme.h
>> @@ -1678,6 +1678,13 @@ enum {
>>       NVME_AUTH_DHGROUP_INVALID    = 0xff,
>>   };
>> +enum {
>> +    NVME_AUTH_SECP_NOSC        = 0x00,
>> +    NVME_AUTH_SECP_SC        = 0x01,
>> +    NVME_AUTH_SECP_NEWTLSPSK    = 0x02,
>> +    NVME_AUTH_SECP_REPLACETLSPSK    = 0x03,
>> +};
>> +
>>   union nvmf_auth_protocol {
>>       struct nvmf_auth_dhchap_protocol_descriptor dhchap;
>>   };
> 
> Wandering if splitting core/fabrics from tcp would be viable here...

Well, authentication is a base functionality, not a tcp one.
Even though it's pretty much TCP-centric.
So I'd rather keep in in nvme.h for now.

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