diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index d19188f9c..22747d932 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -1076,25 +1076,6 @@ iscsi_task_mgmt_cpl(struct spdk_scsi_task *scsi_task) iscsi_task_put(task); } -static void -iscsi_task_copy_to_rsp_scsi_status(struct spdk_iscsi_task *primary, - struct spdk_scsi_task *task) -{ - memcpy(primary->rsp_sense_data, task->sense_data, task->sense_data_len); - primary->rsp_sense_data_len = task->sense_data_len; - primary->rsp_scsi_status = task->status; -} - -static void -iscsi_task_copy_from_rsp_scsi_status(struct spdk_scsi_task *task, - struct spdk_iscsi_task *primary) -{ - memcpy(task->sense_data, primary->rsp_sense_data, - primary->rsp_sense_data_len); - task->sense_data_len = primary->rsp_sense_data_len; - task->status = primary->rsp_scsi_status; -} - static void process_completed_read_subtask_list_in_order(struct spdk_iscsi_conn *conn, struct spdk_iscsi_task *primary) @@ -1189,45 +1170,33 @@ process_non_read_task_completion(struct spdk_iscsi_conn *conn, { primary->bytes_completed += task->scsi.length; - /* If the status of the subtask is the first failure, remember it as - * the status of the command and set it to the status of the primary - * task later. - * - * If the first failed task is the primary, two copies can be avoided - * but code simplicity is prioritized. - */ + if (task == primary) { + /* This was a small write with no R2T. */ + iscsi_task_response(conn, task); + iscsi_task_put(task); + return; + } + if (task->scsi.status == SPDK_SCSI_STATUS_GOOD) { - if (task != primary) { - primary->scsi.data_transferred += task->scsi.data_transferred; - } - } else if (primary->rsp_scsi_status == SPDK_SCSI_STATUS_GOOD) { - iscsi_task_copy_to_rsp_scsi_status(primary, &task->scsi); + primary->scsi.data_transferred += task->scsi.data_transferred; + } else if (primary->scsi.status == SPDK_SCSI_STATUS_GOOD) { + /* If the status of this subtask is the first failure, copy it to + * the primary task. + */ + spdk_scsi_task_copy_status(&primary->scsi, &task->scsi); } if (primary->bytes_completed == primary->scsi.transfer_len) { - /* - * Check if this is the last task completed for an iSCSI write - * that required child subtasks. If task != primary, we know - * for sure that it was part of an iSCSI write with child subtasks. - * The trickier case is when the last task completed was the initial - * task - in this case the task will have a smaller length than - * the overall transfer length. + /* If LUN is removed in the middle of the iSCSI write sequence, + * primary might complete the write to the initiator because it is not + * ensured that the initiator will send all data requested by R2Ts. + * + * We check it and skip the following if primary is completed. (see + * iscsi_clear_all_transfer_task() in iscsi.c.) */ - if (task != primary || task->scsi.length != task->scsi.transfer_len) { - /* If LUN is removed in the middle of the iSCSI write sequence, - * primary might complete the write to the initiator because it is not - * ensured that the initiator will send all data requested by R2Ts. - * - * We check it and skip the following if primary is completed. (see - * iscsi_clear_all_transfer_task() in iscsi.c.) - */ - if (primary->is_r2t_active) { - if (primary->rsp_scsi_status != SPDK_SCSI_STATUS_GOOD) { - iscsi_task_copy_from_rsp_scsi_status(&primary->scsi, primary); - } - iscsi_task_response(conn, primary); - iscsi_del_transfer_task(conn, primary->tag); - } + if (primary->is_r2t_active) { + iscsi_task_response(conn, primary); + iscsi_del_transfer_task(conn, primary->tag); } else { iscsi_task_response(conn, task); } diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 3a8c0a352..5faf707c4 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -3257,6 +3257,7 @@ iscsi_pdu_payload_op_scsi_write(struct spdk_iscsi_conn *conn, struct spdk_iscsi_ struct iscsi_bhs_scsi_req *reqh; uint32_t transfer_len; uint32_t scsi_data_len; + struct spdk_iscsi_task *subtask; int rc; pdu = iscsi_task_get_pdu(task); @@ -3281,14 +3282,18 @@ iscsi_pdu_payload_op_scsi_write(struct spdk_iscsi_conn *conn, struct spdk_iscsi_ } /* Non-immediate writes */ - if (pdu->data_segment_len == 0) { - return 0; - } else { + if (pdu->data_segment_len != 0) { /* we are doing the first partial write task */ - task->scsi.ref++; - spdk_scsi_task_set_data(&task->scsi, pdu->data, scsi_data_len); - task->scsi.length = pdu->data_segment_len; + subtask = iscsi_task_get(conn, task, iscsi_task_cpl); + assert(subtask != NULL); + + spdk_scsi_task_set_data(&subtask->scsi, pdu->data, scsi_data_len); + subtask->scsi.length = pdu->data_segment_len; + iscsi_task_associate_pdu(subtask, pdu); + + iscsi_queue_task(conn, subtask); } + return 0; } if (pdu->data_segment_len == transfer_len) { @@ -3354,7 +3359,7 @@ iscsi_pdu_hdr_op_scsi(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) task->scsi.target_port = conn->target_port; task->scsi.initiator_port = conn->initiator_port; task->parent = NULL; - task->rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; + task->scsi.status = SPDK_SCSI_STATUS_GOOD; if (task->scsi.lun == NULL) { spdk_scsi_task_process_null_lun(&task->scsi); diff --git a/lib/iscsi/task.h b/lib/iscsi/task.h index 0ef48599a..989454c48 100644 --- a/lib/iscsi/task.h +++ b/lib/iscsi/task.h @@ -44,10 +44,6 @@ struct spdk_iscsi_task { struct spdk_iscsi_task *parent; - uint8_t rsp_scsi_status; - uint8_t rsp_sense_data[32]; - size_t rsp_sense_data_len; - struct spdk_iscsi_conn *conn; struct spdk_iscsi_pdu *pdu; uint32_t outstanding_r2t; diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index daf3f62ed..567ce925b 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -387,7 +387,7 @@ propagate_scsi_error_status_for_split_read_tasks(void) conn.sess->DataSequenceInOrder = true; primary.scsi.transfer_len = 512 * 6; - primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; + primary.scsi.status = SPDK_SCSI_STATUS_GOOD; TAILQ_INIT(&primary.subtask_list); primary.scsi.ref = 7; @@ -467,7 +467,7 @@ process_non_read_task_completion_test(void) primary.bytes_completed = 0; primary.scsi.transfer_len = 4096 * 3; - primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; + primary.scsi.status = SPDK_SCSI_STATUS_GOOD; primary.scsi.ref = 1; TAILQ_INSERT_TAIL(&conn.active_r2t_tasks, &primary, link); primary.is_r2t_active = true; @@ -485,7 +485,7 @@ process_non_read_task_completion_test(void) CU_ASSERT(!TAILQ_EMPTY(&conn.active_r2t_tasks)); CU_ASSERT(primary.bytes_completed == 4096); CU_ASSERT(primary.scsi.data_transferred == 0); - CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(primary.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); CU_ASSERT(task.scsi.ref == 0); CU_ASSERT(primary.scsi.ref == 1); @@ -501,7 +501,7 @@ process_non_read_task_completion_test(void) CU_ASSERT(!TAILQ_EMPTY(&conn.active_r2t_tasks)); CU_ASSERT(primary.bytes_completed == 4096 * 2); CU_ASSERT(primary.scsi.data_transferred == 4096); - CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(primary.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); CU_ASSERT(task.scsi.ref == 0); CU_ASSERT(primary.scsi.ref == 1); @@ -517,44 +517,28 @@ process_non_read_task_completion_test(void) CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks)); CU_ASSERT(primary.bytes_completed == 4096 * 3); CU_ASSERT(primary.scsi.data_transferred == 4096 * 2); - CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(primary.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); CU_ASSERT(task.scsi.ref == 0); CU_ASSERT(primary.scsi.ref == 0); - /* Tricky case when the last task completed was the initial task. */ - primary.scsi.length = 4096; + /* A tricky case that the R2T was already terminated when the last task completed. */ + primary.scsi.ref = 0; primary.bytes_completed = 4096 * 2; primary.scsi.data_transferred = 4096 * 2; primary.scsi.transfer_len = 4096 * 3; - primary.scsi.status = SPDK_SCSI_STATUS_GOOD; - primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; - primary.scsi.ref = 2; - TAILQ_INSERT_TAIL(&conn.active_r2t_tasks, &primary, link); - primary.is_r2t_active = true; - - process_non_read_task_completion(&conn, &primary, &primary); - CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks)); - CU_ASSERT(primary.bytes_completed == 4096 * 3); - CU_ASSERT(primary.scsi.data_transferred == 4096 * 2); - CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_GOOD); - CU_ASSERT(primary.scsi.ref == 0); - - /* Further tricky case when the last task completed ws the initial task, - * and the R2T was already terminated. - */ - primary.scsi.ref = 1; - primary.scsi.length = 4096; - primary.bytes_completed = 4096 * 2; - primary.scsi.data_transferred = 4096 * 2; - primary.scsi.transfer_len = 4096 * 3; - primary.scsi.status = SPDK_SCSI_STATUS_GOOD; - primary.rsp_scsi_status = SPDK_SCSI_STATUS_GOOD; + primary.scsi.status = SPDK_SCSI_STATUS_CHECK_CONDITION; primary.is_r2t_active = false; + task.scsi.length = 4096; + task.scsi.data_transferred = 4096; + task.scsi.status = SPDK_SCSI_STATUS_GOOD; + task.scsi.ref = 1; + task.parent = &primary; + primary.scsi.ref++; - process_non_read_task_completion(&conn, &primary, &primary); + process_non_read_task_completion(&conn, &task, &primary); CU_ASSERT(primary.bytes_completed == 4096 * 3); - CU_ASSERT(primary.scsi.data_transferred == 4096 * 2); - CU_ASSERT(primary.rsp_scsi_status == SPDK_SCSI_STATUS_GOOD); + CU_ASSERT(primary.scsi.data_transferred == 4096 * 3); + CU_ASSERT(primary.scsi.status == SPDK_SCSI_STATUS_CHECK_CONDITION); CU_ASSERT(primary.scsi.ref == 0); }