Re: [PATCH 07/10] nvme-tcp: request secure channel concatenation

[Hannes Reinecke <hare@suse.de>]

On 1/28/25 10:11, Christoph Hellwig wrote:
> On Wed, Jan 22, 2025 at 05:58:26PM +0100, 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.
> 
> That's a very sparse commit message.  What is the point of doing this?
> What is the spec reference for the implementation?  What is the user
> interface?  Why does this now always select NVME_KEYRING?
> 
Okay, I'll add a spec reference and improve the description.
And NVME_KEYRING is selected due to a kbuild failure for
some odd combination of build-in vs modular in host and target.

>> +	if (!ctrl->opts->concat || chap->qid != 0)
>> +		data->sc_c = NVME_AUTH_SECP_NOSC;
>> +	else if (ctrl->opts->tls_key)
>> +		data->sc_c = NVME_AUTH_SECP_REPLACETLSPSK;
>> +	else
>> +		data->sc_c = NVME_AUTH_SECP_NEWTLSPSK;
> 
> Took me a while to unwind this.  Why not make this a little easier as:
> 
> 	if (ctrl->opts->concat && chap->qid == 0) {
> 		if (ctrl->opts->tls_key)
> 			data->sc_c = NVME_AUTH_SECP_REPLACETLSPSK;
> 		else
> 			data->sc_c = NVME_AUTH_SECP_NEWTLSPSK;
> 	} else {
> 		data->sc_c = NVME_AUTH_SECP_NOSC;
> 	}
> 
> ?
> 
Okay.

[ .. ]
>> --- a/include/linux/nvme.h
>> +++ b/include/linux/nvme.h
>> @@ -1746,6 +1746,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,
>> +};
> 
> Comments please to explain what fields this applies to.  Also we
> usuall try to split protocol definition additions into separate
> patches.
> 
Sure, I can split it into a separate patch.

And promise to fixup of formatting :-)

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