Re: [PATCH] nvme-tcp: add basic support for the C2HTermReq PDU

[Hannes Reinecke <hare@suse.de>]

On 2/14/25 20:00, Maurizio Lombardi wrote:
> Previously, the NVMe/TCP host driver did not handle the C2HTermReq PDU,
> instead printing "unsupported pdu type (3)" when received. This patch adds
> support for processing the C2HTermReq PDU, allowing the driver
> to print the Fatal Error Status field.
> 
> Example of output:
> nvme nvme4: Received C2HTermReq (FES = Invalid PDU Header Field)
> 
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
>   drivers/nvme/host/tcp.c  | 37 +++++++++++++++++++++++++++++++++++++
>   include/linux/nvme-tcp.h |  2 ++
>   2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 841238f38fdd..8f783185575d 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -763,6 +763,40 @@ static int nvme_tcp_handle_r2t(struct nvme_tcp_queue *queue,
>   	return 0;
>   }
>   
> +static void nvme_tcp_handle_c2h_term(struct nvme_tcp_queue *queue,
> +		struct nvme_tcp_term_pdu *pdu)
> +{
> +	u16 fes;
> +	const char *msg;
> +	u32 plen = le32_to_cpu(pdu->hdr.plen);
> +
> +	static const char * const msg_table[] = {
> +		[NVME_TCP_FES_INVALID_PDU_HDR] = "Invalid PDU Header Field",
> +		[NVME_TCP_FES_PDU_SEQ_ERR] = "PDU Sequence Error",
> +		[NVME_TCP_FES_HDR_DIGEST_ERR] = "Header Digest Error",
> +		[NVME_TCP_FES_DATA_OUT_OF_RANGE] = "Data Transfer Out Of Range",
> +		[NVME_TCP_FES_R2T_LIMIT_EXCEEDED] = "R2T Limit Exceeded",
> +		[NVME_TCP_FES_UNSUPPORTED_PARAM] = "Unsupported Parameter",
> +	};
> +
> +	if (plen < NVME_TCP_MIN_C2HTERM_PLEN ||
> +	    plen > NVME_TCP_MAX_C2HTERM_PLEN) {
> +		dev_err(queue->ctrl->ctrl.device,
> +			"Received a malformed C2HTermReq PDU (plen = %u)\n",
> +			plen);
> +		return;
> +	}
> +
> +	fes = le16_to_cpu(pdu->fes);
> +	if (fes && fes < ARRAY_SIZE(msg_table))
> +		msg = msg_table[fes];
> +	else
> +		msg = "N/A";
> +
> +	dev_err(queue->ctrl->ctrl.device,
> +		"Received C2HTermReq (FES = %s)\n", msg);
> +}
> +
>   static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,
>   		unsigned int *offset, size_t *len)
>   {
> @@ -806,6 +840,9 @@ static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,
>   	case nvme_tcp_r2t:
>   		nvme_tcp_init_recv_ctx(queue);
>   		return nvme_tcp_handle_r2t(queue, (void *)queue->pdu);
> +	case nvme_tcp_c2h_term:
> +		nvme_tcp_handle_c2h_term(queue, (void *)queue->pdu);
> +		return -EINVAL;
>   	default:
>   		dev_err(queue->ctrl->ctrl.device,
>   			"unsupported pdu type (%d)\n", hdr->type);
> diff --git a/include/linux/nvme-tcp.h b/include/linux/nvme-tcp.h
> index e07e8978d691..e435250fcb4d 100644
> --- a/include/linux/nvme-tcp.h
> +++ b/include/linux/nvme-tcp.h
> @@ -13,6 +13,8 @@
>   #define NVME_TCP_ADMIN_CCSZ	SZ_8K
>   #define NVME_TCP_DIGEST_LENGTH	4
>   #define NVME_TCP_MIN_MAXH2CDATA 4096
> +#define NVME_TCP_MIN_C2HTERM_PLEN	24
> +#define NVME_TCP_MAX_C2HTERM_PLEN	152
>   
>   enum nvme_tcp_pfv {
>   	NVME_TCP_PFV_1_0 = 0x0,

Can you add support for nvmet, too, such that we can test the patch?
(And maybe even a blktest script for it?)

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