From e7f6ff2db6b50bcfd9ce2e644f2c3dd82977e6f1 Mon Sep 17 00:00:00 2001 From: Wenhua Liu Date: Thu, 4 Feb 2021 08:28:22 +0000 Subject: [PATCH] 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: Wenhua Liu 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: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- include/spdk_internal/nvme_tcp.h | 12 ++++++------ lib/nvmf/tcp.c | 11 ++++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/spdk_internal/nvme_tcp.h b/include/spdk_internal/nvme_tcp.h index 8df9fb59e..a1a05c4b6 100644 --- a/include/spdk_internal/nvme_tcp.h +++ b/include/spdk_internal/nvme_tcp.h @@ -618,12 +618,12 @@ 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; - if (padding_len > 0) { - psh_len = pdo; - } + } + if (pdu->hdr.common.plen > psh_len) { + pdo = pdu->hdr.common.pdo; + padding_len = pdo - psh_len; + if (padding_len > 0) { + psh_len = pdo; } } diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index 2bd34aae3..ef9799c4a 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -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; } }