From 1c51dfdb6e727443f09256eee0ac6ee9a495452e Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 31 Oct 2019 13:48:40 -0400 Subject: [PATCH] lib/iscsi: Remove iSCSI task left in PDU receive process due to connection exit Previously iSCSI task was created after allocating data buffer and reading all data, and hence creating iSCSI task and processing iSCSI task were not separated. However, the recent refactoring separate PDU header handling and PDU payload handling, and then inserted allocating data buffer and reading data segment in the middle. If any critical error occurs during allocating data buffer or reading data segment, PDU payload handling is not done, and hence created iSCSI task is left in PDU receive process. If any critical error occurs, the current connection starts exiting and there is no way to continue PDU receive process. The task left in PDU receive process is never freed, and hence LUN hotplug or exiting connection never complete. This patch do the following: - Consolidate freeing pre-allocated PDU to spdk_iscsi_conn_destruct() because this is the only path to exit connection. - Abort SCSI task of the task left in PDU receive process if found when freeing pre-allocated PDU. If the task is not SCSI or Data Out, remove it simply. Fix issues #1018. Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472896 (master) (cherry picked from commit cd654cc5127515da5c113778c15f7c4666acefd7) Change-Id: I8a2464c446c43bf4cfb5afbc0cd78b5bdef7d080 Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472988 Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris Tested-by: SPDK CI Jenkins --- lib/iscsi/conn.c | 36 ++++++++++++++++++++++++++++++------ lib/iscsi/iscsi.c | 2 -- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 8af08fce4..9b5267147 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -377,12 +377,6 @@ _iscsi_conn_free(struct spdk_iscsi_conn *conn) spdk_iscsi_param_free(conn->params); - /* - * Each connection pre-allocates its next PDU - make sure these get - * freed here. - */ - spdk_put_pdu(conn->pdu_in_progress); - free_conn(conn); } @@ -675,6 +669,10 @@ _iscsi_conn_check_pending_tasks(void *arg) void spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) { + struct spdk_iscsi_pdu *pdu; + struct spdk_iscsi_task *task; + int opcode; + /* If a connection is already in exited status, just return */ if (conn->state >= ISCSI_CONN_STATE_EXITED) { return; @@ -682,6 +680,32 @@ spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) conn->state = ISCSI_CONN_STATE_EXITED; + /* + * Each connection pre-allocates its next PDU - make sure these get + * freed here. + */ + pdu = conn->pdu_in_progress; + if (pdu) { + /* remove the task left in the PDU too. */ + task = pdu->task; + if (task) { + opcode = pdu->bhs.opcode; + switch (opcode) { + case ISCSI_OP_SCSI: + case ISCSI_OP_SCSI_DATAOUT: + spdk_scsi_task_process_abort(&task->scsi); + spdk_iscsi_task_cpl(&task->scsi); + break; + default: + SPDK_ERRLOG("unexpected opcode %x\n", opcode); + spdk_iscsi_task_put(task); + break; + } + } + spdk_put_pdu(pdu); + conn->pdu_in_progress = NULL; + } + if (conn->sess != NULL && conn->pending_task_cnt > 0) { iscsi_conn_cleanup_backend(conn); } diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index c23382272..d8305ac37 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -4970,8 +4970,6 @@ iscsi_read_pdu(struct spdk_iscsi_conn *conn) } break; case ISCSI_PDU_RECV_STATE_ERROR: - spdk_put_pdu(pdu); - conn->pdu_in_progress = NULL; return SPDK_ISCSI_CONNECTION_FATAL; default: assert(false);