From cc1146a8b5999f3a5718034fd8313b267d2e7aca Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Mon, 31 Oct 2016 10:39:59 -0700 Subject: [PATCH] iscsi: move iSCSI-specific SenseLength into PDU This removes the 2 bytes of SenseLength from the beginning of the SCSI sense_data buffer, so now the offsets within sense.data match up to the expected values from the SCSI spec. Change-Id: I9188560096a9ec5a8fcf83bec95201521b127494 Signed-off-by: Daniel Verkamp --- lib/iscsi/conn.c | 1 + lib/iscsi/iscsi.c | 10 ++++++++-- lib/iscsi/iscsi.h | 5 +++++ lib/iscsi/iscsi_subsystem.c | 2 +- lib/scsi/scsi_bdev.c | 5 ++--- lib/scsi/task.c | 10 ++-------- test/lib/scsi/scsi_bdev/scsi_bdev_ut.c | 22 ++++++++-------------- 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index e3548df1e..64a133dde 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -914,6 +914,7 @@ void process_task_completion(spdk_event_t event) (task->scsi.status != SPDK_SCSI_STATUS_GOOD)) { memcpy(primary->scsi.sense_data, task->scsi.sense_data, task->scsi.sense_data_len); + primary->scsi.sense_data_len = task->scsi.sense_data_len; primary->scsi.status = task->scsi.status; } diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 7432e783f..3beaad3d8 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -3131,7 +3131,10 @@ void spdk_iscsi_task_response(struct spdk_iscsi_conn *conn, /* response PDU */ rsp_pdu = spdk_get_pdu(); rsph = (struct iscsi_bhs_scsi_resp *)&rsp_pdu->bhs; - rsp_pdu->data = task->scsi.sense_data; + assert(task->scsi.sense_data_len <= sizeof(rsp_pdu->sense.data)); + memcpy(rsp_pdu->sense.data, task->scsi.sense_data, task->scsi.sense_data_len); + to_be16(&rsp_pdu->sense.length, task->scsi.sense_data_len); + rsp_pdu->data = (uint8_t *)&rsp_pdu->sense; rsp_pdu->data_from_mempool = true; /* @@ -3157,7 +3160,10 @@ void spdk_iscsi_task_response(struct spdk_iscsi_conn *conn, rsph->flags |= ISCSI_SCSI_UNDERFLOW; rsph->status = task->scsi.status; - DSET24(rsph->data_segment_len, task->scsi.sense_data_len); + if (task->scsi.sense_data_len) { + /* SenseLength (2 bytes) + SenseData */ + DSET24(rsph->data_segment_len, 2 + task->scsi.sense_data_len); + } to_be32(&rsph->itt, task_tag); to_be32(&rsph->stat_sn, conn->StatSN); diff --git a/lib/iscsi/iscsi.h b/lib/iscsi/iscsi.h index c47822437..b6d0716c4 100644 --- a/lib/iscsi/iscsi.h +++ b/lib/iscsi/iscsi.h @@ -183,6 +183,11 @@ struct spdk_iscsi_pdu { * we need to not zero this out when doing memory clear. */ uint8_t ahs_data[ISCSI_AHS_LEN]; + + struct { + uint16_t length; /* iSCSI SenseLength (big-endian) */ + uint8_t data[32]; + } sense; }; enum iscsi_connection_state { diff --git a/lib/iscsi/iscsi_subsystem.c b/lib/iscsi/iscsi_subsystem.c index 158ffaf4f..2b12be36b 100644 --- a/lib/iscsi/iscsi_subsystem.c +++ b/lib/iscsi/iscsi_subsystem.c @@ -515,7 +515,7 @@ struct spdk_iscsi_pdu *spdk_get_pdu(void) rte_panic("no memory\n"); } - /* we do not want to zero out the last 60 bytes reserved for AHS */ + /* we do not want to zero out the last part of the structure reserved for AHS and sense data */ memset(pdu, 0, offsetof(struct spdk_iscsi_pdu, ahs_data)); pdu->ref = 1; diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index cb8e22f77..a94a08e0f 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -1841,9 +1841,8 @@ spdk_bdev_scsi_process_primary(struct spdk_bdev *bdev, spdk_scsi_task_build_sense_data(task, sk, asc, ascq); - /* omit SenseLength */ - data_len = task->sense_data_len - 2; - memcpy(data, &task->sense_data[2], data_len); + data_len = task->sense_data_len; + memcpy(data, task->sense_data, data_len); task->data_transferred = (uint64_t)data_len; task->status = SPDK_SCSI_STATUS_GOOD; break; diff --git a/lib/scsi/task.c b/lib/scsi/task.c index d0b65258b..8e2a994c5 100644 --- a/lib/scsi/task.c +++ b/lib/scsi/task.c @@ -133,18 +133,13 @@ spdk_scsi_task_alloc_data(struct spdk_scsi_task *task, uint32_t alloc_len, void spdk_scsi_task_build_sense_data(struct spdk_scsi_task *task, int sk, int asc, int ascq) { - uint8_t *data; uint8_t *cp; int resp_code; - data = task->sense_data; resp_code = 0x70; /* Current + Fixed format */ - /* SenseLength */ - memset(data, 0, 2); - /* Sense Data */ - cp = &data[2]; + cp = task->sense_data; /* VALID(7) RESPONSE CODE(6-0) */ cp[0] = 0x80 | resp_code; @@ -173,8 +168,7 @@ spdk_scsi_task_build_sense_data(struct spdk_scsi_task *task, int sk, int asc, in cp[17] = 0; /* SenseLength */ - to_be16(data, 18); - task->sense_data_len = 20; + task->sense_data_len = 18; } void diff --git a/test/lib/scsi/scsi_bdev/scsi_bdev_ut.c b/test/lib/scsi/scsi_bdev/scsi_bdev_ut.c index 697dcdaca..6e64060ff 100644 --- a/test/lib/scsi/scsi_bdev/scsi_bdev_ut.c +++ b/test/lib/scsi/scsi_bdev/scsi_bdev_ut.c @@ -76,18 +76,13 @@ spdk_scsi_task_alloc_data(struct spdk_scsi_task *task, uint32_t alloc_len, void spdk_scsi_task_build_sense_data(struct spdk_scsi_task *task, int sk, int asc, int ascq) { - uint8_t *data; uint8_t *cp; int resp_code; - data = task->sense_data; resp_code = 0x70; /* Current + Fixed format */ - /* SenseLength */ - memset(data, 0, 2); - /* Sense Data */ - cp = &data[2]; + cp = task->sense_data; /* VALID(7) RESPONSE CODE(6-0) */ cp[0] = 0x80 | resp_code; @@ -116,8 +111,7 @@ spdk_scsi_task_build_sense_data(struct spdk_scsi_task *task, int sk, int asc, in cp[17] = 0; /* SenseLength */ - to_be16(data, 18); - task->sense_data_len = 20; + task->sense_data_len = 18; } void @@ -370,9 +364,9 @@ inquiry_evpd_test(void) rc = spdk_bdev_scsi_execute(&bdev, &task); CU_ASSERT_EQUAL(task.status, SPDK_SCSI_STATUS_CHECK_CONDITION); - CU_ASSERT_EQUAL(task.sense_data[4], (SPDK_SCSI_SENSE_ILLEGAL_REQUEST & 0xf)); - CU_ASSERT_EQUAL(task.sense_data[14], 0x24); - CU_ASSERT_EQUAL(task.sense_data[15], 0x0); + CU_ASSERT_EQUAL(task.sense_data[2] & 0xf, SPDK_SCSI_SENSE_ILLEGAL_REQUEST); + CU_ASSERT_EQUAL(task.sense_data[12], 0x24); + CU_ASSERT_EQUAL(task.sense_data[13], 0x0); CU_ASSERT_EQUAL(rc, 0); } @@ -492,9 +486,9 @@ task_complete_test(void) bdev_io.status = SPDK_BDEV_IO_STATUS_FAILED; spdk_bdev_scsi_task_complete(&event); CU_ASSERT_EQUAL(task.status, SPDK_SCSI_STATUS_CHECK_CONDITION); - CU_ASSERT_EQUAL(task.sense_data[4], SPDK_SCSI_SENSE_ABORTED_COMMAND); - CU_ASSERT_EQUAL(task.sense_data[14], SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE); - CU_ASSERT_EQUAL(task.sense_data[15], SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); + CU_ASSERT_EQUAL(task.sense_data[2], SPDK_SCSI_SENSE_ABORTED_COMMAND); + CU_ASSERT_EQUAL(task.sense_data[12], SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE); + CU_ASSERT_EQUAL(task.sense_data[13], SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); } int