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

[Christoph Hellwig <hch@lst.de>]

On Wed, Jan 22, 2025 at 05:58:28PM +0100, Hannes Reinecke wrote:
> Evaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert
> the generated PSK once negotiation has finished.

Same as for the host side, please write a much more detailed commit log.

> +	if (ctrl->dh_gid == NVME_AUTH_DHGROUP_NULL &&
> +	    ctrl->concat) {

This conditional easily fits onto a single line.

> @@ -247,6 +263,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
>  	struct nvmet_ctrl *ctrl = NULL;
>  	struct nvmet_alloc_ctrl_args args = {
>  		.port = req->port,
> +		.sq = req->sq,

So this now needs to pass a sq in alloc_ctrl_args?  That needs proper
explanation and really should be in a prep patch.  Also please Cc
Damien to make sure this doesn't break the nvme PCIe endpoint code.

>  	cancel_work_sync(&queue->io_work);
> @@ -1806,6 +1808,23 @@ static void nvmet_tcp_tls_handshake_done(void *data, int status,
>  	spin_unlock_bh(&queue->state_lock);
>  
>  	cancel_delayed_work_sync(&queue->tls_handshake_tmo_work);
> +
> +	if (!status) {
> +		struct key *tls_key = nvme_tls_key_lookup(peerid);
> +
> +		if (IS_ERR(tls_key)) {
> +			pr_warn("%s: queue %d failed to lookup key %x\n",
> +				__func__, queue->idx, peerid);
> +			spin_lock_bh(&queue->state_lock);
> +			queue->state = NVMET_TCP_Q_FAILED;
> +			spin_unlock_bh(&queue->state_lock);
> +			status = PTR_ERR(tls_key);
> +		} else {
> +			pr_debug("%s: queue %d using TLS PSK %x\n",
> +				 __func__, queue->idx, peerid);
> +			queue->nvme_sq.tls_key = tls_key;
> +		}

This is almost begging for a separate helper..