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

[Maurizio Lombardi]

On Fri Mar 13, 2026 at 8:20 AM CET, Hannes Reinecke wrote:
> 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 ...

I've just rebased my branch on top of master and I can still
see nvmet_tcp_socket_error() calling nvmet_tcp_fatal_error()

nvmet_tcp_socket_error() is called by both nvmet_tcp_try_send()
and nvmet_tcp_try_recv() in case of errors, so it's still here.
with us.

Maurizio