[PATCH V2 2/2] nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()

[Hannes Reinecke]

On 3/12/26 14:40, Maurizio Lombardi wrote:
> Executing nvmet_tcp_fatal_error() is generally the responsibility
> of the caller (nvmet_tcp_try_recv); all other functions should
> just return the error code.
> 
> Signed-off-by: Maurizio Lombardi <mlombard at redhat.com>
> ---
>   drivers/nvme/target/tcp.c | 22 +++++-----------------
>   1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 1fbf12df1183..b250b13854b4 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -885,7 +885,6 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
>   	if (le32_to_cpu(icreq->hdr.plen) != sizeof(struct nvme_tcp_icreq_pdu)) {
>   		pr_err("bad nvme-tcp pdu length (%d)\n",
>   			le32_to_cpu(icreq->hdr.plen));
> -		nvmet_tcp_fatal_error(queue);
>   		return -EPROTO;
>   	}
>   
> @@ -951,7 +950,6 @@ static int nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
>   	ret = nvmet_tcp_map_data(cmd);
>   	if (unlikely(ret)) {
>   		pr_err("queue %d: failed to map data\n", queue->idx);
> -		nvmet_tcp_fatal_error(queue);
>   		return -EPROTO;
>   	}
>   
> @@ -1024,7 +1022,6 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
>   
>   err_proto:
>   	/* FIXME: use proper transport errors */
> -	nvmet_tcp_fatal_error(queue);
>   	return -EPROTO;
>   }
>   
> @@ -1039,7 +1036,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>   		if (hdr->type != nvme_tcp_icreq) {
>   			pr_err("unexpected pdu type (%d) before icreq\n",
>   				hdr->type);
> -			nvmet_tcp_fatal_error(queue);
>   			return -EPROTO;
>   		}
>   		return nvmet_tcp_handle_icreq(queue);
> @@ -1048,7 +1044,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>   	if (unlikely(hdr->type == nvme_tcp_icreq)) {
>   		pr_err("queue %d: received icreq pdu in state %d\n",
>   			queue->idx, queue->state);
> -		nvmet_tcp_fatal_error(queue);
>   		return -EPROTO;
>   	}
>   
> @@ -1065,7 +1060,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>   		pr_err("queue %d: out of commands (%d) send_list_len: %d, opcode: %d",
>   			queue->idx, queue->nr_cmds, queue->send_list_len,
>   			nvme_cmd->common.opcode);
> -		nvmet_tcp_fatal_error(queue);
>   		return -ENOMEM;
>   	}
>   
> @@ -1086,9 +1080,9 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>   	if (unlikely(ret)) {
>   		pr_err("queue %d: failed to map data\n", queue->idx);
>   		if (nvmet_tcp_has_inline_data(queue->cmd))
> -			nvmet_tcp_fatal_error(queue);
> -		else
> -			nvmet_req_complete(req, ret);
> +			return -EPROTO;
> +
> +		nvmet_req_complete(req, ret);
>   		ret = -EAGAIN;
>   		goto out;
>   	}
> @@ -1211,7 +1205,6 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
>   
>   		if (unlikely(!nvmet_tcp_pdu_valid(hdr->type))) {
>   			pr_err("unexpected pdu type %d\n", hdr->type);
> -			nvmet_tcp_fatal_error(queue);
>   			return -EIO;
>   		}
>   
> @@ -1225,16 +1218,12 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
>   	}
>   
>   	if (queue->hdr_digest &&
> -	    nvmet_tcp_verify_hdgst(queue, &queue->pdu, hdr->hlen)) {
> -		nvmet_tcp_fatal_error(queue); /* fatal */
> +	    nvmet_tcp_verify_hdgst(queue, &queue->pdu, hdr->hlen))
>   		return -EPROTO;
> -	}
>   
>   	if (queue->data_digest &&
> -	    nvmet_tcp_check_ddgst(queue, &queue->pdu)) {
> -		nvmet_tcp_fatal_error(queue); /* fatal */
> +	    nvmet_tcp_check_ddgst(queue, &queue->pdu))
>   		return -EPROTO;
> -	}
>   
>   	return nvmet_tcp_done_recv_pdu(queue);
>   }
> @@ -1319,7 +1308,6 @@ static int nvmet_tcp_try_recv_ddgst(struct nvmet_tcp_queue *queue)
>   			le32_to_cpu(cmd->exp_ddgst));
>   		nvmet_req_uninit(&cmd->req);
>   		nvmet_tcp_free_cmd_buffers(cmd);
> -		nvmet_tcp_fatal_error(queue);
>   		ret = -EPROTO;
>   		goto out;
>   	}

I seem to be missing something crucial here. But after applying this
patch _all_ invocations to nvmet_tcp_fatal_error() are gone.
(Applying the patches on top of linus tree with topmost commit 
0257f64bdac7fdca30fa3cae0df8b9ecbec7733a)

I must be missing something crucial here ...

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