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 <daniel.verkamp@intel.com>

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;
 		}
 

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);

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 {

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;
 

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;
-		data_len = task->sense_data_len - 2;
+		memcpy(data, task->sense_data, data_len);
-		memcpy(data, &task->sense_data[2], data_len);
 		task->data_transferred = (uint64_t)data_len;
 		task->status = SPDK_SCSI_STATUS_GOOD;
 		break;

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 = 18;
-	task->sense_data_len = 20;
 }
 
 void

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 = 18;
-	task->sense_data_len = 20;
 }
 
 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[2] & 0xf, SPDK_SCSI_SENSE_ILLEGAL_REQUEST);
-	CU_ASSERT_EQUAL(task.sense_data[14], 0x24);
+	CU_ASSERT_EQUAL(task.sense_data[12], 0x24);
-	CU_ASSERT_EQUAL(task.sense_data[15], 0x0);
+	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[2], SPDK_SCSI_SENSE_ABORTED_COMMAND);
-	CU_ASSERT_EQUAL(task.sense_data[14], SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE);
+	CU_ASSERT_EQUAL(task.sense_data[12], 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[13], SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
 }
 
 int