Commit e7f6ff2d authored by Wenhua Liu's avatar Wenhua Liu Committed by Tomasz Zawadzki
Browse files

Fix incorrect implementation of HPDA/CPDA in NVMe/TCP target code. 



The current implementation treats HPDA/CPDA as the absolute offset
to the beginning of the PDU where the payload data starts. This is
incorrect. The HPDA/CPDA actually specify where the payload data
should start such that the starting location is a multiple of HPDA
(for C2H PDU) or CPDA (for H2C PDU or CapsuleCmd PDU).

The other issue fixed is that the current implementation calculates
padding only when header digest is enabled. This is also incorrect.

Signed-off-by: default avatarWenhua Liu <liuw@vmware.com>
Change-Id: If7a3896a4c1d73f6d062bd3dbe6a912d31771180
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6256


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 77573e83

include/spdk_internal/nvme_tcp.h

+6 −6
Original line number Diff line number Diff line
@@ -618,6 +618,7 @@ nvme_tcp_pdu_calc_psh_len(struct nvme_tcp_pdu *pdu, bool hdgst_enable) 
	if (g_nvme_tcp_hdgst[pdu->hdr.common.pdu_type] && hdgst_enable) {

		pdu->has_hdgst = true;

		psh_len += SPDK_NVME_TCP_DIGEST_LEN;

	}

	if (pdu->hdr.common.plen > psh_len) {

		pdo = pdu->hdr.common.pdo;

		padding_len = pdo - psh_len;
@@ -625,7 +626,6 @@ nvme_tcp_pdu_calc_psh_len(struct nvme_tcp_pdu *pdu, bool hdgst_enable) 
			psh_len = pdo;

		}

	}

	}



	psh_len -= sizeof(struct spdk_nvme_tcp_common_pdu_hdr);

	pdu->psh_len = psh_len;

lib/nvmf/tcp.c

+6 −5
Original line number Diff line number Diff line
@@ -1824,7 +1824,7 @@ nvmf_tcp_pdu_ch_handle(struct spdk_nvmf_tcp_qpair *tqpair) 
		case SPDK_NVME_TCP_PDU_TYPE_CAPSULE_CMD:

			expected_hlen = sizeof(struct spdk_nvme_tcp_cmd);

			pdo = pdu->hdr.common.pdo;

			if ((tqpair->cpda != 0) && (pdo != ((tqpair->cpda + 1) << 2))) {

			if ((tqpair->cpda != 0) && (pdo % ((tqpair->cpda + 1) << 2) != 0)) {

				pdo_error = true;

				break;

			}
@@ -1836,7 +1836,7 @@ nvmf_tcp_pdu_ch_handle(struct spdk_nvmf_tcp_qpair *tqpair) 
		case SPDK_NVME_TCP_PDU_TYPE_H2C_DATA:

			expected_hlen = sizeof(struct spdk_nvme_tcp_h2c_data_hdr);

			pdo = pdu->hdr.common.pdo;

			if ((tqpair->cpda != 0) && (pdo != ((tqpair->cpda + 1) << 2))) {

			if ((tqpair->cpda != 0) && (pdo % ((tqpair->cpda + 1) << 2) != 0)) {

				pdo_error = true;

				break;

			}
@@ -2216,9 +2216,10 @@ _nvmf_tcp_send_c2h_data(struct spdk_nvmf_tcp_qpair *tqpair, 
	pdo = plen;

	if (tqpair->cpda) {

		alignment = (tqpair->cpda + 1) << 2;

		if (alignment > plen) {

			rsp_pdu->padding_len = alignment - plen;

			pdo = plen = alignment;

		if (plen % alignment != 0) {

			pdo = (plen + alignment) / alignment * alignment;

			rsp_pdu->padding_len = pdo - plen;

			plen = pdo;

		}

	}