Commit 2ef97692 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

lib/iscsi: Create subtask after reading data segment for Data-OUT PDU 



The following patches will change the handler for Data-OUT PDU to
submit subtask only when 64KB data is read or F bit is set.

Previously, we had created a subtask when processing header and
before reading data segment.  Creating a subtask beforehand is not
convenient for the following changes.

Hence create a subtask after reading data segment.

If LUN is removed while processing the Data-OUT PDU, the corresponding
primary task will be terminated by iscsi_clear_all_transfer_task(),
and any subtask completion is not sent to initiator. Hence we can
reject the received Data-OUT PDU safely.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ifb6d6988676080b458b31d12fef065f3c1de0cb6
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6415


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 59237d22

lib/iscsi/iscsi.c

+25 −40
Original line number Diff line number Diff line
@@ -4142,7 +4142,7 @@ iscsi_pdu_hdr_op_snack(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
static int

iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)

{

	struct spdk_iscsi_task	*task, *subtask;

	struct spdk_iscsi_task	*task;

	struct iscsi_bhs_data_out *reqh;

	struct spdk_scsi_lun	*lun_dev;

	uint32_t transfer_tag;
@@ -4225,15 +4225,6 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
		task->current_r2t_length = 0;

	}



	subtask = iscsi_task_get(conn, task, iscsi_task_cpl);

	if (subtask == NULL) {

		SPDK_ERRLOG("Unable to acquire subtask\n");

		return SPDK_ISCSI_CONNECTION_FATAL;

	}

	subtask->scsi.offset = buffer_offset;

	subtask->scsi.length = pdu->data_segment_len;

	iscsi_task_associate_pdu(subtask, pdu);



	if (task->next_expected_r2t_offset == transfer_len) {

		task->acked_r2tsn++;

	} else if (F_bit && (task->next_r2t_offset < transfer_len)) {
@@ -4249,45 +4240,48 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
	}



	if (lun_dev == NULL) {

		SPDK_DEBUGLOG(iscsi, "LUN %d is removed, complete the task immediately\n",

		SPDK_DEBUGLOG(iscsi, "LUN %d is removed, reject this PDU.\n",

			      task->lun_id);

		subtask->scsi.transfer_len = subtask->scsi.length;

		spdk_scsi_task_process_null_lun(&subtask->scsi);

		iscsi_task_cpl(&subtask->scsi);

		return 0;

	}



	if (spdk_unlikely(spdk_scsi_lun_get_dif_ctx(lun_dev, &subtask->scsi, &pdu->dif_ctx))) {

		return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR);

	} else if (spdk_unlikely(spdk_scsi_lun_get_dif_ctx(lun_dev, &task->scsi, &pdu->dif_ctx))) {

		pdu->dif_insert_or_strip = true;

	}



	pdu->task = subtask;

	return 0;

}



static int

iscsi_pdu_payload_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)

{

	struct spdk_iscsi_task *subtask;

	struct spdk_iscsi_task *task, *subtask;

	struct iscsi_bhs_data_out *reqh;

	uint32_t transfer_tag;



	if (pdu->task == NULL) {

		return 0;

	}



	subtask = pdu->task;

	uint32_t buffer_offset;



	reqh = (struct iscsi_bhs_data_out *)&pdu->bhs;

	transfer_tag = from_be32(&reqh->ttt);

	buffer_offset = from_be32(&reqh->buffer_offset);



	if (get_transfer_task(conn, transfer_tag) == NULL) {

	task = get_transfer_task(conn, transfer_tag);

	if (spdk_unlikely(task == NULL)) {

		SPDK_ERRLOG("Not found for transfer_tag=%x\n", transfer_tag);

		subtask->scsi.transfer_len = subtask->scsi.length;

		spdk_scsi_task_process_abort(&subtask->scsi);

		iscsi_task_cpl(&subtask->scsi);

		return 0;

		return iscsi_reject(conn, pdu, ISCSI_REASON_INVALID_PDU_FIELD);

	}



	if (spdk_scsi_dev_get_lun(conn->dev, task->lun_id) == NULL) {

		SPDK_DEBUGLOG(iscsi, "LUN %d is removed, reject this PDU.\n",

			      task->lun_id);

		return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR);

	}



	subtask = iscsi_task_get(conn, task, iscsi_task_cpl);

	if (subtask == NULL) {

		SPDK_ERRLOG("Unable to acquire subtask\n");

		return SPDK_ISCSI_CONNECTION_FATAL;

	}

	subtask->scsi.offset = buffer_offset;

	subtask->scsi.length = pdu->data_segment_len;

	iscsi_task_associate_pdu(subtask, pdu);



	if (spdk_likely(!pdu->dif_insert_or_strip)) {

		spdk_scsi_task_set_data(&subtask->scsi, pdu->data, pdu->data_segment_len);
@@ -4295,15 +4289,6 @@ iscsi_pdu_payload_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *p 
		spdk_scsi_task_set_data(&subtask->scsi, pdu->data, pdu->data_buf_len);

	}



	if (spdk_scsi_dev_get_lun(conn->dev, subtask->lun_id) == NULL) {

		SPDK_DEBUGLOG(iscsi, "LUN %d is removed, complete the task immediately\n",

			      subtask->lun_id);

		subtask->scsi.transfer_len = subtask->scsi.length;

		spdk_scsi_task_process_null_lun(&subtask->scsi);

		iscsi_task_cpl(&subtask->scsi);

		return 0;

	}



	iscsi_queue_task(conn, subtask);

	return 0;

}

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+9 −21
Original line number Diff line number Diff line
@@ -1858,7 +1858,7 @@ check_iscsi_r2t(struct spdk_iscsi_task *task, uint32_t len) 
	rsph = (struct iscsi_bhs_r2t *)&rsp_pdu->bhs;

	CU_ASSERT(rsph->opcode == ISCSI_OP_R2T);

	CU_ASSERT(from_be64(&rsph->lun) == spdk_scsi_lun_id_int_to_fmt(task->lun_id));

	CU_ASSERT(from_be32(&rsph->buffer_offset) == task->next_r2t_offset);

	CU_ASSERT(from_be32(&rsph->buffer_offset) + len == task->next_r2t_offset);

	CU_ASSERT(from_be32(&rsph->desired_xfer_len) == len);



	TAILQ_REMOVE(&g_write_pdu_list, rsp_pdu, tailq);
@@ -1955,10 +1955,10 @@ pdu_hdr_op_data_test(void) 


	rc = iscsi_pdu_hdr_op_data(&conn, &pdu);

	CU_ASSERT(rc == 0);

	CU_ASSERT(pdu.task == NULL);

	check_iscsi_reject(&pdu, ISCSI_REASON_PROTOCOL_ERROR);



	/* Case 10 - SCSI Data-Out PDU is correct and processed. Created task is held

	 * to the PDU, but its F bit is 0 and hence R2T is not sent.

	/* Case 10 - SCSI Data-Out PDU is correct and processed. Its F bit is 0 and hence

	 * R2T is not sent.

	 */

	dev.lun[0] = &lun;

	to_be32(&data_reqh->data_sn, primary.r2t_datasn);
@@ -1966,12 +1966,11 @@ pdu_hdr_op_data_test(void) 


	rc = iscsi_pdu_hdr_op_data(&conn, &pdu);

	CU_ASSERT(rc == 0);

	CU_ASSERT(pdu.task != NULL);

	iscsi_task_put(pdu.task);

	CU_ASSERT(!pdu.is_rejected);

	pdu.task = NULL;



	/* Case 11 - SCSI Data-Out PDU is correct and processed. Created task is held

	 * to the PDU, and Its F bit is 1 and hence R2T is sent.

	/* Case 11 - SCSI Data-Out PDU is correct and processed. Its F bit is 1 and hence

	 * R2T is sent.

	 */

	data_reqh->flags |= ISCSI_FLAG_FINAL;

	to_be32(&data_reqh->data_sn, primary.r2t_datasn);
@@ -1980,19 +1979,8 @@ pdu_hdr_op_data_test(void) 


	rc = iscsi_pdu_hdr_op_data(&conn, &pdu);

	CU_ASSERT(rc == 0);

	CU_ASSERT(pdu.task != NULL);

	check_iscsi_r2t(pdu.task, pdu.data_segment_len * 4);

	iscsi_task_put(pdu.task);



	/* Case 12 - Task pool is empty. */

	to_be32(&data_reqh->data_sn, primary.r2t_datasn);

	to_be32(&data_reqh->buffer_offset, primary.next_expected_r2t_offset);

	g_task_pool_is_empty = true;



	rc = iscsi_pdu_hdr_op_data(&conn, &pdu);

	CU_ASSERT(rc == SPDK_ISCSI_CONNECTION_FATAL);



	g_task_pool_is_empty = false;

	CU_ASSERT(!pdu.is_rejected);

	check_iscsi_r2t(&primary, pdu.data_segment_len * 4);

}



/* Test an ISCSI_OP_TEXT PDU with CONTINUE bit set but