d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

iscsi: Fix the case that incoming data is split between data segment and data digest

Browse Source 
When data segment size is 64KB and data digest is enabled, if
data segment and data digest are split into different two packets,
- pdu->mobj[0] became full first when reading data semgment,
- pdu->mobj[1] was allocated but unused and data digest was read.

In this case, two SCSI write tasks were submitted by mistake and
the second SCSI write task had no data.

Fix the bug in this patch.

When iscsi_pdu_payload_read() is called and pdu->mobj[0] is full,
allocate pdu->mobj[1] only if any of data segment remains to read.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9a0c36c05f90092c3c2122a7eb91e10976830b40
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9965
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This commit is contained in:
Shuhei Matsumoto 2021-10-21 15:29:34 +09:00 committed by Jim Harris
parent e536464357
commit eb739d0364
3 changed files with 46 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/iscsi/iscsi.c
View File

@ -4622,6 +4622,7 @@ iscsi_pdu_payload_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)
int rc;
data_len = pdu->data_segment_len;
read_len = data_len - pdu->data_valid_bytes;
mobj = pdu->mobj[0];
if (mobj == NULL) {
@ -4643,7 +4644,7 @@ iscsi_pdu_payload_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)
pdu->mobj[0] = mobj;
pdu->data = mobj->buf;
pdu->data_from_mempool = true;
} else if (mobj->data_len == SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH) {
} else if (mobj->data_len == SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH && read_len > 0) {
mobj = pdu->mobj[1];
if (mobj == NULL) {
/* The first data buffer just ran out. Allocate the second data buffer and
@ -4667,8 +4668,8 @@ iscsi_pdu_payload_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)
}
/* copy the actual data into local buffer */
read_len = spdk_min(data_len - pdu->data_valid_bytes,
SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH - mobj->data_len);
read_len = spdk_min(read_len, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH - mobj->data_len);
if (read_len > 0) {
rc = iscsi_conn_read_data_segment(conn,
pdu,

13
test/unit/lib/iscsi/common.c
View File

@ -18,6 +18,8 @@ TAILQ_HEAD(, spdk_iscsi_pdu) g_write_pdu_list = TAILQ_HEAD_INITIALIZER(g_write_p
static bool g_task_pool_is_empty = false;
static bool g_pdu_pool_is_empty = false;
static uint32_t g_conn_read_len;
static bool g_conn_read_data_digest = false;
static uint32_t g_data_digest;
struct spdk_iscsi_task *
iscsi_task_get(struct spdk_iscsi_conn *conn,
@ -181,12 +183,23 @@ iscsi_task_cpl(struct spdk_scsi_task *scsi_task)
DEFINE_STUB_V(iscsi_task_mgmt_cpl, (struct spdk_scsi_task *scsi_task));
#define MAKE_DIGEST_WORD(BUF, CRC32C) \
( ((*((uint8_t *)(BUF)+0)) = (uint8_t)((uint32_t)(CRC32C) >> 0)), \
((*((uint8_t *)(BUF)+1)) = (uint8_t)((uint32_t)(CRC32C) >> 8)), \
((*((uint8_t *)(BUF)+2)) = (uint8_t)((uint32_t)(CRC32C) >> 16)), \
((*((uint8_t *)(BUF)+3)) = (uint8_t)((uint32_t)(CRC32C) >> 24)))
int
iscsi_conn_read_data(struct spdk_iscsi_conn *conn, int bytes, void *buf)
{
uint32_t *data = buf;
int i;
if (g_conn_read_data_digest) {
MAKE_DIGEST_WORD(buf, g_data_digest);
return ISCSI_DIGEST_LEN;
}
/* Limit the length to 4 bytes multiples. */
SPDK_CU_ASSERT_FATAL((bytes % 4) == 0);

29
test/unit/lib/iscsi/iscsi.c/iscsi_ut.c
View File

@ -2130,6 +2130,35 @@ pdu_payload_read_test(void)
rc = iscsi_pdu_payload_read(&conn, &pdu);
check_pdu_payload_read(&pdu, &mobj2, rc, 1, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);
/* Case 5: data segment size is SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH, data digest
* is enabled, and reading PDU data is split between data segment and data digest.
*/
conn.data_digest = true;
memset(&pdu, 0, sizeof(pdu));
pdu.crc32c = SPDK_CRC32C_INITIAL;
pdu.data = mobj1.buf;
pdu.data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;
pdu.mobj[0] = &mobj1;
pdu.data_valid_bytes = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;
mobj1.data_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;
/* generate data digest. */
g_data_digest = spdk_crc32c_update(mobj1.buf, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH,
SPDK_CRC32C_INITIAL);
g_data_digest ^= SPDK_CRC32C_XOR;
g_conn_read_data_digest = true;
rc = iscsi_pdu_payload_read(&conn, &pdu);
CU_ASSERT(rc == 0);
CU_ASSERT(pdu.ddigest_valid_bytes == ISCSI_DIGEST_LEN);
CU_ASSERT(pdu.mobj[1] == NULL);
g_conn_read_data_digest = false;
g_conn_read_len = 0;
MOCK_SET(spdk_mempool_get, &mobj1);
mobj1.data_len = 0;
g_conn_read_len = 0;
MOCK_CLEAR(spdk_mempool_get);
free(mobj1.buf);